-
Notifications
You must be signed in to change notification settings - Fork 71
chore: deploy new BSC contracts #1091
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Ihor Farion <[email protected]>
Signed-off-by: Ihor Farion <[email protected]>
hardhat.config.ts
Outdated
// ! Notice. Params below helped deploy Universal_Spoke on BSC, but might not be desirable always | ||
// gas: "auto", | ||
// gasPrice: 3e8, // 0.3 GWEI | ||
// allowUnlimitedContractSize: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why allowUnlimitedContractSize
was needed here? That seems like it specifically should cause the deployment to fail when actually sending the create transaction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea it might be a local error...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why allowUnlimitedContractSize was needed here?
Hmm, perhaps it wasn't needed actually. I was trying to throw everything at this config to make it not revert with the "deployment out of gas" error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure it wasn't needed. Removed here ca5816e
@@ -18,6 +27,10 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) { | |||
throw new Error(`Wrapped native token not found for ${wrappedNativeSymbol} on chainId ${spokeChainId}`); | |||
} | |||
|
|||
const oftEid = getOftEid(hubChainId); | |||
// ! Notice. Deployed has to adjust this fee cap based on dst chain's native token. 4.4 BNB for BSC | |||
const oftFeeCap = toWei(4.4); // ~1 ETH fee cap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would happen if the BNB:ETH exchange rate moved sharply? Would we need to redeploy or does our fee cap give us a wide room for error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is extremely wide. A normal quote is never > $10. We give a HUGE buffer here
The fee cap is meant to prevent a single incorreect quote from draining our (potentially big?) native token balance, we cap it at like $3-4K. IMO 4.4 BNB is fine here
Signed-off-by: Ihor Farion <[email protected]>
This deployment is required to update us to using a new
SP1Helios
deployment from here, but also bumps BSC deployment with OFT functionality.This PR also deploys
Universal_Adapter
for BSC in order to keep parity of functionality between Adapter and Spoke for BSC (i.e. both support OFT now)