Skip to content
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

STAKE token mediators #394

Merged
merged 18 commits into from
Apr 10, 2020
Merged

STAKE token mediators #394

merged 18 commits into from
Apr 10, 2020

Conversation

k1rill-fedoseev
Copy link
Member

This PR adds two smart-contracts that are intended to bridge STAKE token through AMB bridge between Mainnet and xDai chains.

From the implementation perspective, the regular AMB-ERC677-TO-ERC677 can be used, with some slight changes.

The STAKE token is supposed to have a dynamic supply in the xDai chain and a non-decreasing supply in the Mainnet chain.
Also, the configured fee will be collected for transfers in the xDai => Mainnet direction.

For that, the following bridging mechanics are being introduced:

  • For the Mainnet => xDai transfers, nothing is changed from the AMB-ERC677-TO-ERC677 logic. The tokens are locked on the mediator contract in the Mainnet and then minted in the xDai chain.
  • For the xDai => Mainnet transfers:
    1. Bridged tokens are burnt in the xDai chain.
    2. The fee is calculated and the blockReward.addBridgeTokenRewardReceivers(fee) is called. The collected fee will be distributed between validators later in the consensus contract.
    3. Value, after fee deduction, is bridged to the Mainnet.
    4. Tokens are unlocked in the Mainnet chain. If bridge balance is less than the bridged amount, the bridge will first mint insufficient tokens, as a consequence of this, the total supply will increase in the Mainnet.

@k1rill-fedoseev k1rill-fedoseev self-assigned this Mar 31, 2020
Copy link
Collaborator

@akolotov akolotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely need the deployment scripts for these mediators. Take into account that the deployment must work with pre-defined token contracts on both sides.

For testing purposes consider to use the token contract from https://github.com/xdaichain/stake-token.

@k1rill-fedoseev
Copy link
Member Author

k1rill-fedoseev commented Apr 3, 2020

I have tested the deployment scripts. Everything seems to work fine.
Here is the test performed:

  1. Existing Sokol-Kovan AMB bridge at 0xFe446bEF1DbF7AFE24E81e05BC8B271C1BA9a560.
  2. Deploy BlockReward mock in Home chain at 0x03E2b0876625D1Cd19194498D673cCeff8D60110
  3. Deploy ERC677BridgeTokenRewardable token in the Home chain. 0x3061B0f53FAdfB55075bDB1fF7A6961cb592997F
  4. Call setBlockRewardContract on deployed token contract.
  5. Call setToken on deployed block reward contract.
  6. Deploy stake token in the Foreign chain. 0x3cE43Ec076324E0bEcb6d9Ea47e264309f804CAc.
  7. Run deploy script using ./deploy.sh. Mediators were deployed successfully in both chains: 0xb5d68a0010F175cbced754E5635d67A4cB343388 and 0x2c34cc8c227d1fbcD76531528208587390352f40.
  8. Call addBridge(0xb5d68a0010F175cbced754E5635d67A4cB343388) on token in the Foreign chain.
  9. Call addBridge(0x2c34cc8c227d1fbcD76531528208587390352f40) and transferOwnership(0x2c34cc8c227d1fbcD76531528208587390352f40) on the token in the Home chain.
  10. Test that tokens can be bridged in the Foreign => Home direction, tx.
  11. Test that tokens can be bridged in the Home => Foreign direction, tx. 0.1% fee was collected and minted to the blockReward contract.

@varasev
Copy link
Contributor

varasev commented Apr 6, 2020

@k1rill-fedoseev Could you please measure gas consumption for deploying Foreign contract implementations to ensure the size of the compiled contracts doesn't exceed 24 Kb? (gas cannot exceed ~6 700 000 in this case).

@k1rill-fedoseev
Copy link
Member Author

For the foreign mediator implementation:

  • Optimized bytecode size is ~ 15 Kb
  • Deploy tx took ~ 3 200 000 gas

For the home mediator, the numbers are similar:

  • Optimized bytecode size is ~ 16 Kb

Copy link
Collaborator

@akolotov akolotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patitonar Gerardo, please take a look at the changes. Since there is no information when we can get the next audit we need to review the changes carefully.

@akolotov akolotov merged commit 393ef6c into develop Apr 10, 2020
@akolotov akolotov deleted the stake-token-mediators branch April 10, 2020 20:18
*/
function executeActionOnFixedTokens(address _recipient, uint256 _value) internal {
getMintHandler().mint(_recipient, _value);
}
Copy link
Contributor

@varasev varasev Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question regarding executeActionOnFixedTokens function: as far as I understand, it's used to return tokens back to sender if something is wrong when passing a message from bridge contract to mediator contract. I tried to analyze the following case (please correct me if I'm wrong somewhere):

For example, some sender Bob on Home side uses alternative receiver feature to send tokens to Alice. He sends his tokens to Home mediator contract, so bridgeSpecificActionsOnTokenTransfer is triggered (and the tokens are burnt on Home side) and then the message passed to Home bridge contract. Oracle catches the corresponding event, the bridge validators sign the message and request the Foreign bridge contract to pass the message to Foreign mediator contract, but _passMessage returns false for some reason.

Then someone calls the requestFailedMessageFix on the Foreign mediator contract. The Foreign mediator contract in turn through the Foreign bridge contract asks for fixFailedMessage to be called on Home side. The fixFailedMessage calls executeActionOnFixedTokens of the Home mediator contract which mints back the tokens for Alice (but not for Bob) on the Home side.

Is it intended behavior that in case of alternative receiver feature the failed tokens are returned back to Alice, but not to Bob on Home side?

@varasev
Copy link
Contributor

varasev commented Apr 17, 2020

Should onTokenTransfer on the bridge contract revert when someone sends tokens directly to bridge contract instead of sending them to mediator contract?

@akolotov
Copy link
Collaborator

Should onTokenTransfer on the bridge contract revert when someone sends tokens directly to bridge contract instead of sending them to mediator contract?

If I understood your question correctly, you consider the scenario when tokens (not only STAKE tokens) are being sent to a AMB bridge contract. But the AMB token contracts have no onTokenTransfer functionality at all. It is assumed that if some one occasionally sent the tokens to a wrong address the only thing they could do is to ask the bridge owners to use claimTokens functionality: https://github.com/poanetwork/tokenbridge-contracts/blob/fdaab2777efe2f48c20a690c9f59196fa89e3631/contracts/upgradeable_contracts/BasicBridge.sol#L38

@varasev
Copy link
Contributor

varasev commented Apr 20, 2020

I mean onTokenTransfer in BasicAMBErc677ToErc677: seems this contract is inherited by HomeAMBErc677ToErc677 and ForeignAMBErc677ToErc677 contracts. As far as I understand, these contracts are AMB bridge contracts and they can accept STAKE tokens. Is it OK, or I missed something?

My understanding is that if we use mediator contract, the mediator contract should accept tokens, but the bridge contract shouldn't (which is between mediator contract and oracle). Am I correct?

@akolotov
Copy link
Collaborator

My understanding is that if we use mediator contract, the mediator contract should accept tokens, but the bridge contract shouldn't (which is between mediator contract and oracle). Am I correct?

The token contract communicates with the mediator, the mediator contract communicates with the bridge contract. The oracle listens events emitted by the bridge contract.

@varasev
Copy link
Contributor

varasev commented Apr 20, 2020

OK, so in AMB scheme, what will happen if I send STAKE token to the bridge contract (not to the mediator contract)?

Does AMB allow sending STAKE tokens to bridge contract? (not mediator one)

@akolotov
Copy link
Collaborator

OK, so in AMB scheme, what will happen if I send STAKE token to the bridge contract (not to the mediator contract)?

Does AMB allow sending STAKE tokens to bridge contract? (not mediator one)

When we are saying the AMB we mean the bridge contract. The AMB contracts have no onTokenTransfer functionality at all. It is assumed that if some one occasionally sent the any tokens to the bridge address the only thing they could do is to ask the bridge owners to use claimTokens functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants