-
Notifications
You must be signed in to change notification settings - Fork 228
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
Extend Native-ERC20 limits functionality on contracts v1.1 #109
Conversation
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.
The comments added to the Foreign bridge contract are also applicable to the Home bridge contract as well.
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.
Please do the following:
- Port changes you made to the gists you prepared
- Deploy the bridge contracts (and run Rust bridge) to Sokol<->Kovan. The version of contracts must be the same as it is deployed now in ETH and POA.
- Make few transaction through the bridge in both directions. Make sure that the bridge balance is not zero after that.
- Upgrade contracts on both sides. The version of contracts must be taken from the gists.
- Set the validators limits.
- Make sure that the state of the bridge is the same: the same balance, the previous state of user limits (daily limit).
- Reset the revisited blocks and restart the bridge. Make sure that transactions which were transferred successfully at the step 3, are not transferred one more time (double spend).
- Make few transfers in both directions.
@akolotov Followed the steps you mentioned. Everything worked as expected. Here are some notes from my actions:
Used eth-cli tool for the following steps.
then
then
|
Ok. Great! I will take a look at the gists one more time and will initiate the procedure of the contracts upgrade. |
@patitonar I tried to describe the upgrade procedure and found that it is quite complicated and could lead to a situation when transactions will not be accepted for short period of time since we don't want to stop validators during upgrade. That's why my suggestion is to introduce an additional method in the contracts that are in your gist. This method will be responsible for migration from one version of contract to another. In the case of function upgradeFrom2To3() public {
requires(!boolStorage[keccak256("isUpgradedFrom2To3")]);
uintStorage[keccak256("foreignDailyLimit")] = 1500000 eth;
emit ForeignDailyLimit(1500000 eth);
uintStorage[keccak256("foreignMaxPerTx")] = 1500000 eth;
boolStorage[keccak256("isUpgradedFrom2To3")] = true;
} For the function upgradeFrom1To2() public {
requires(!boolStorage[keccak256("isUpgradedFrom1To2")]);
uintStorage[keccak256("homeDailyLimit")] = 3000000 eth;
emit HomeDailyLimit(3000000 eth);
uintStorage[keccak256("homeMaxPerTx")] = 1500000 eth;
boolStorage[keccak256("isUpgradedFrom1To2")] = true;
} It will allow us upgrade and initialize everything in one transaction by calling Please modify gists correspondingly and perform tests for upgrade similar to you did here: #109 (comment) |
@akolotov I made some corrections on the methods you suggested (replaced Then I followed the same steps as before, except at the moment of the upgrade of the contracts:
The state of the bridge was the same after the upgrade, no double spend was produced by Rust Bridge and transactions performed using bridge-ui worked as expected. |
The upgraded version of contracts was applied to the ETH mainnet and POA Core: |
Relates to issue #108
Gist for deployed HomeBridge: https://gist.github.com/patitonar/df585134034f5b6fbe01928fc6e00e36
Gist for deployed ForeignBridge: https://gist.github.com/patitonar/0ff940d070ef5c4d3dbd70e44eeae361