Make sure the contract upgradeability works smoothly in binance and polygon #60
Conversation
There was a problem hiding this comment.
@softskillpro Niice, we're almost there, please fix the issues requested, rebase and i'll merge
| binance: { | ||
| url: "https://bsc-dataseed.binance.org/", | ||
| chainId: 56, | ||
| gasPrice: 20000000000, |
There was a problem hiding this comment.
Remove this, not necessary
|
|
||
| const netinfo = await ethers.provider.getNetwork(); | ||
| network = netinfo.name; | ||
| if (network === "unknown") |
There was a problem hiding this comment.
Change this to
if (network === "unknown"){
network = 'mainnet';
// put the rest of the code here
}
There was a problem hiding this comment.
I have a question here, do you mean that put the same code here for mainnet?
In my opinion, it's not good we have duplicated code for mainnet, binance and polygon.
so, how about to use if/else for only places where needs a special process for a specific network?
Or we can define configs for each networks, it will makes generalized code.
Forexample, in deploy.js and config.json
const wrapper = await deployWithProxy(
Wrapper,
OwnableProxy,
'WrapAndUnWrap',
addr.tokens.WETH[network],
addr.swaps.uniswap[network],
addr.swaps.uniswapFactory[network],
addr.tokens.DAI[network],
addr.tokens.USDT[network],
addr.tokens.USDC[network]
);
The more generalized code, easier to understand and more clean.
What do you think?
There was a problem hiding this comment.
@softskillpro I agree, please implement it this way :)
| console.log("Deploying from account: " + ownerAddress); | ||
| console.log(""); | ||
|
|
||
| if (network === "binance") { |
There was a problem hiding this comment.
Add the same flow for Polygon/Matic
@terminator0x
#50
you can run "npx hardhat run scripts/upgradeChecks.js"
Please review this PR.
Thank you.