fix: fixed the deployV1FactoryContracts script #301
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the deployV1FactoryContracts.ts deployment script from ethers v5 to ethers v6, updating all API calls to use the new ethers v6 syntax. The script was tested on opBNB testnet.
Key Changes:
- Updated from BigNumber arithmetic to native bigint operations for gas price calculations
- Migrated contract deployment and address retrieval methods to ethers v6 APIs
- Added fallback logic for signer selection when a second signer is unavailable
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const gasParams = { | ||
| gasPrice: feeData.gasPrice.mul(2) // Use BigNumber arithmetic for ethers v5 | ||
| gasPrice: (feeData.gasPrice ?? 0n) * 2n // Use bigint arithmetic for ethers v6 |
There was a problem hiding this comment.
The fallback value of 0n for gasPrice could cause transactions to fail. If feeData.gasPrice is null or undefined, multiplying 0n by 2n will result in a gasPrice of 0n, which would make transactions not get mined. Consider throwing an error or using a more appropriate fallback strategy when gasPrice is unavailable.
| const gasParams = { | ||
| gasPrice: feeData.gasPrice.mul(2) // Use BigNumber arithmetic for ethers v5 | ||
| gasPrice: (feeData.gasPrice ?? 0n) * 2n // Use bigint arithmetic for ethers v6 | ||
| }; |
There was a problem hiding this comment.
The gasPrice parameter is being set as a bigint value, but ethers v6 typically expects gas parameters to be in standard number format or properly formatted. Verify that the gasParams object with bigint gasPrice is compatible with the deploy and sendTransaction methods in ethers v6, as this may cause type errors or transaction failures.
| output.walletImplementation = await walletImplementation.getAddress(); // ethers v6 | ||
| console.log( | ||
| `${walletImplementationContractName} deployed at ` + | ||
| walletImplementation.address // ethers v5 | ||
| (await walletImplementation.getAddress()) // ethers v6 |
There was a problem hiding this comment.
Redundant async call: the address is fetched twice - once on line 48 for assignment and again on line 51 for logging. Consider storing the address in a variable to avoid the duplicate async call.
| console.log( | ||
| `${walletFactoryContractName} deployed at ` + walletFactory.address // ethers v5 | ||
| `${walletFactoryContractName} deployed at ` + | ||
| (await walletFactory.getAddress()) // ethers v6 |
There was a problem hiding this comment.
Redundant async call: the address is fetched twice - once on line 63 for assignment and again on line 66 for logging. Consider reusing the address stored in output.walletFactory to avoid the duplicate async call.
| (await walletFactory.getAddress()) // ethers v6 | |
| output.walletFactory // ethers v6 |
| console.log( | ||
| `${forwarderImplementationContractName} deployed at ` + | ||
| forwarderImplementation.address // ethers v5 | ||
| (await forwarderImplementation.getAddress()) // ethers v6 |
There was a problem hiding this comment.
Redundant async call: the address is fetched twice - once on line 99 for assignment and again on line 103 for logging. Consider reusing the address stored in output.forwarderImplementation to avoid the duplicate async call.
| (await forwarderImplementation.getAddress()) // ethers v6 | |
| output.forwarderImplementation // ethers v6 |
| console.log( | ||
| `${forwarderFactoryContractName} deployed at ` + forwarderFactory.address // ethers v5 | ||
| `${forwarderFactoryContractName} deployed at ` + | ||
| (await forwarderFactory.getAddress()) // ethers v6 |
There was a problem hiding this comment.
Redundant async call: the address is fetched twice - once on line 117 for assignment and again on line 120 for logging. Consider reusing the address stored in output.forwarderFactory to avoid the duplicate async call.
| (await forwarderFactory.getAddress()) // ethers v6 | |
| output.forwarderFactory // ethers v6 |
Ticket: COIN-6850
Tested locally you can check here
https://opbnb-testnet.bscscan.com/address/0x8d18cbb2e5b198a6a0a2366474d80a1ea08c69e8