strategy to migrate an origin native-to-erc bridge to AMB

akolotov commented 4 years ago

This issue is being opened to make research to identify ability of migrating a native-to-erc bridge (e.g. WETC bridge) to be an extension of AMB bridge.

The following is required to be considered:

the existing pair of proxy contracts (Ethereum Classic: 0x073081832B4Ecdce79d4D6753565c85Ba4b3BeA9, Ethereum Mainnet: 0x0cB781EE62F815bdD9CD4c2210aE8600d43e7040) must be the proxy for AMB mediators contracts
the current bridge state must preserve (balances and completed operations)

Please provide an analysis if the migration is possible, what limitations need to be taken into account (e.g. due to limited number of functionality existing in Ethereum Classic), what special handling must be done in comparison with a generic native-to-erc20 extension for AMB.

The implementation stage will begin only after the review of the analysis described above.

patitonar commented 4 years ago

Here is the result of my review.

Security

AMB validators will listen to different events on different contracts with different types of messages and how they are processed. Also, validators could be different. So there is no risk to reprocess a completed message.

Functionality

All functionality from the bridge can be kept as a mediator. Regarding the fee distribution, the current fee manager implementation for this mode is intended to work with RewardableValidators contract. If the fees will be distributed to the AMB validators, which uses BridgeValidators contract, the fee manager should be adapted accordingly (i.e. the method getValidatorRewardAddress is not defined).

Bridge state

Upgrading the implementation contract will not affect the locked ETC value in the proxy contract nor the ERC677 tokens hold by the users.

If EternalStorage is also used for the mediators, the list of parameters currently stored in the contract should continue to be accessible if the same key is used.

Parameters in home bridge	Possible parameters needed in home mediator
requiredBlockConfirmations
deployedAtBlock
gasPrice
isInitialized	isInitialized
owner	owner
validatorContract	validatorContract
dailyLimit	dailyLimit
maxPerTx	maxPerTx
minPerTx	minPerTx
executionDailyLimit	executionDailyLimit
executionMaxPerTx	executionMaxPerTx
feeManagerContract	feeManagerContract
homeFee	homeFee
foreignFee	foreignFee
	bridgeContract
	mediatorContract
	requestGasLimit
	nonce
	decimalShift

Parameters in foreign bridge	Possible parameters needed in foreign mediator
requiredBlockConfirmations
deployedAtBlock
gasPrice
isInitialized	isInitialized
owner	owner
erc677token	erc677token
validatorContract	validatorContract
dailyLimit	dailyLimit
maxPerTx	maxPerTx
minPerTx	minPerTx
executionDailyLimit	executionDailyLimit
executionMaxPerTx	executionMaxPerTx
feeManagerContract	feeManagerContract
homeFee	homeFee
	bridgeContract
	mediatorContract
	requestGasLimit
	nonce
	decimalShift

Given that the contracts are already initialized, the following parameters need to be set/updated as part of the implementation contract upgrade:

bridgeContract
mediatorContract
requestGasLimit
nonce
decimalShift
validatorContract --> Update to select new validator contract to distribute the fees
feeManagerContract --> Update to select new feeManager contract compatible with mediators to distribute the fees

ETC special considerations

Previously, contracts needed to be adapted and compiled to the spuriousDragon EVM version to be able to deploy the contract and correctly work on ETC chain. This is no longer needed since ETC was upgraded:

Atlantis introduced Byzantium changes from Ethereum. https://ecips.ethereumclassic.org/ECIPs/ecip-1054
Agharta introduced Constantinople and Petersburg changes from Ethereum. https://ecips.ethereumclassic.org/ECIPs/ecip-1056

To confirm this, a native-to-erc bridge was deployed in Kotti-Kovan networks and tested along with all the components with successful results. Home Bridge: 0x12d756ee7c00fa819dd8baff787fd1fb66232be8 Foreign Bridge: 0xd0f06b4dfd32d7f9f56d31bff3c855f9ccdd66ea

Bridge UI

The UI will be needed to be adapted to work with the new mediator. So far there is no AMB mediator supported.

akolotov commented 4 years ago

@patitonar thanks for detailed analysis!

Please find my comments below:

All functionality from the bridge can be kept as a mediator. Regarding the fee distribution, the current fee manager implementation for this mode is intended to work with RewardableValidators contract. If the fees will be distributed to the AMB validators, which uses BridgeValidators contract, the fee manager should be adapted accordingly (i.e. the method getValidatorRewardAddress is not defined).

At this moment I suggest to consider the way to integrate the storage of reward recipient into the fee manager (or something like this). It looks logical since we assume that the mediator contracts could be owned by a different project rather than the project who deployed the Arbitrary Message Bridge.

Parameters in foreign bridge: requiredBlockConfirmations, deployedAtBlock, gasPrice.

Could we add a code to a migration method to release the storage for these parameters. It will allows us to refund some gas.

To confirm this, a native-to-erc bridge was deployed in Kotti-Kovan networks and tested along with all the components with successful results.

As far as I remember we didn't use Kotti for testing last time by some reason. Do you recall these reasons?

The UI will be needed to be adapted to work with the new mediator. So far there is no AMB mediator supported.

I discussed this with @igorbarinov and we agreed that it could be a good idea to start looking on more generic approach for the bridge UI with respect to AMB. The current view is that we can consider to develop a Burner Wallet plug-in (https://github.com/burner-wallet/sample-plugin). The rationale here is that it is already works with the xDai chain to bridge Dai to xDai (https://github.com/burner-wallet/burner-wallet-2/tree/develop/packages/exchange).

patitonar commented 4 years ago

After reviewing the structure and code of the Burner Wallet, I think the easiest way to integrate and interact with new mediators contracts is to create a plugin that adds a new Pair (ETC <-> WETC) in the exchange plugin, similar on how the xDai bridge works.

Here are some possibles steps to do this:

Fork https://github.com/burner-wallet/sample-plugin
Implement a custom plugin that defines:
- ETC Native Asset
- WETC ERC677 Asset
- ETC HTTPGateway
- WETCBridge Pair for exchange plugin.
Update basic-wallet and local-wallet (for dev) included in the repo:
- Use the exchange plugin. Add the implemented WETCBridge Pair plugin.
- Add the metamask plugin to support metamask/nifty wallet in case the user doesn't want to set their PK in the burner wallet. This step is optional or could be added later since the user can import their PK in the Burner Wallet and start using it.

omni / tokenbridge-contracts