omni / tokenbridge-contracts

Smart contracts for TokenBridge
http://docs.tokenbridge.net
GNU General Public License v3.0
228 stars 227 forks source link

Consortium Bridge Proposal #133

Open branciard opened 5 years ago

branciard commented 5 years ago

Hi, Here spec proposal to implement a consortium bridge as described here : https://forum.poa.network/t/consortium-bridge/1739.

  1. A consortium Bridge will introduce a whiteListing of ethereum address that will be allowed to transfer asset from the foreign chain to the home chain according to the current state in a foreign chain whitelist smart contract.

  2. Transfer affirmation request (erc20 or native) from home chain to foreign chain will be always possible. If a user has move asset before being removed from the whitelist by admin, The user will be able to withdraw asset (home to foreign) but not deposit asset any more (foreign to home).

  3. The deployment of this whiteListing should be optional. This will be materialized by a contract (like the optional feeContract for instance) that can be set to null (0x0) for an open bridge as today.

  4. Proposal for this contract name : Border.sol or BorderPoint.sol, BorderCrossing.sol, BorderCrossingPoint.sol, Frontier.sol ... We will name it Border Contract in the following text.

  5. The Border Contract must be upgradable. (removed (0x0) or replace by a new one)

  6. The Border Contract state will be stored in the foreign chain and not in the home chain. In order to have a non-repudiation assured. As the home chain state maybe not accessible by everybody.

  7. This whitelist in Border Contract will be checked and affirmationRequestWatcher bridge component may filter events received.

  8. The whitelisting check is activate by a configuration on bridge conf too: CHECK_BORDER_ACTIVE=true|false If the bridge cannot access the smart contract whitelist, filtering is done by default. So this is why a config can be set : CHECK_BORDER_ACTIVE = false. In this case, no check to retrieved the Border Contract form the foreign bridge contract and no check of the whitelist in the Border Contract. All this to be sure to not have rpc failure network from the foreign web3 that will ended to do a filtering by default... Admin must set CHECK_BORDER_ACTIVE to false in bridge config when a public bridge mode is wanted as today.

  9. AffirmationRequestWatcher filtering action will introduce a balance desynch detected by the bridgeMonitor ? In this case, all filtering must be records in logs, in order to administrator to detect if balance desynch is cause by normal filtering or real problems. In this log, Admin must be able to have all data to refund user in case of filtering done, if the user claimed his locked asset on the foreign bridge contract. In this case, the use of claimTokens to refund ?: https://github.com/poanetwork/poa-bridge-contracts/blob/119-Epic-rewards-for-bridge-validators/contracts/upgradeable_contracts/BasicBridge.sol#L130

  10. Note: This Border Contract is not a membership contract. It is an authorization to currently bridge asset in one direction foreign to home. By "currently", we means the current state of the whitelist in foreign chain, not the time of erc20 or native transfer transaction done by the user. Timing may differ a bit.

  11. Note : A blacklist strategy for the Border Contract is not usefull as a user can change wallet at any time to easily pass the bridge.

  12. Interface of BorderContract :

    interface IBorder {
    function isAllowed(address _recipient) public view returns(bool);
    function addRecipientAllowance(address _recipient) external onlyOwner returns (bool);
    function removeRecipientAllowance(address _recipient) external onlyOwner returns (bool);
    function owner() public view returns(address);
    }

    Must generate an event RecipientAllowanceAdded(_recipient) Must generate an event RecipientAllowanceRemoved(_recipient) addRecipientAllowance and removeRecipientAllowance methods access by the bridge admin account only. The method isAllowed(_recipient) is public.

  13. Border Contract implementation must have a allowedRecipient mapping.

    mapping(address=>bool) public allowedRecipient;

    Or list must be store in EternalStorage ? (like the validatorsList) and used of :

    mapping(bytes32 => bool) internal boolStorage;
  14. Or maybe a quick solution with no smart contract involved ? Only provide a config list of address used to filter or not in affirmationRequestWatcher bridgecomponent.

This is a starting point ideas for a consortium bridge. Open to others architecture proposals.

akolotov commented 5 years ago

Thanks for formulating this in such useful manner!

Do you want this feature to be work with the existing ERC20 tokens or we can assume that we always work with an ERC677 token on the Foreign side? It will get rid of necessity to change the token bridge side. Also it will imply that user will not need to claim tokens manually (no changes on the monitor side is required as well).

If we are going to operate with existing ERC20 tokens, we can consider to introduce a new functionality when the validators are looking for the Transfer for accounts that are not whitelisted and then provide confirmations to refund assets (minus a fee): as soon as N confirmations received, the assets are returned to the same account that sent them initially. The bridge monitor could observe a situations when only few confirmations received (e.g. the account was added to the white list just after transferring tokens) and report about such cases as so the admin could resolve the issue manually.

I have one question for the notes you published above:

Transfer affirmation request (erc20 or native) from home chain to foreign chain will be always possible.

Do you mean it will be possible by any account even if it is not whitelisted?

branciard commented 5 years ago

Thank you for your feedback!

Yes on the foreign side, it is an existing ERC20 token that does not implement ERC677 standard.

I was thinking of case when an address, previousely whitelisted, has a balance on the home side. If a this address is then remove from the whitelist. To not have his balance prisonned on the home side. Bridge must assured transfer from home to foreign as usual even if this address is not currently whitelisted for transfering from foreign to home.

akolotov commented 5 years ago

So, we are currently agreed on the vision of what we would like to achieve. Great!

It means that we need to implement: poa-bridge-contracts

  1. new Board Contract with the following methods:
    • add an address, emit an event, allowed to be run by a new role: custom inspector.
    • remove an address, emit an event, allowed to be run by the custom inspector
    • check if an address is whitelisted
    • return all whitelisted addresses
  2. implement new functionality on the Home and Foreign side to:
    • collect signatures on the Home side from the validators to confirm refund of assets from the address that is not the white list
    • accept collected signatures on the Foreign side and perform actual refund
  3. change the Foreign contract for native-to-erc20 mode:
    • onTokenTransfer should use the Board contract to check if the transaction sender is whitelisted and if it is not to revert the transaction.

We need to consider methodically cases

token-bridge

  1. Introduce a new watcher. The watcher must read all addresses from the contract as soon as it starts. Then it will look for new events appearing when an address is added/removed from the Border contract
  2. Change the a watcher that looks for Transfer events to:
    • handle Transfer events only from the watcher described in 1.
    • send signature to the Home bridge contract to refund if the address that transferred assets is not in the white list
  3. Introduce a new watcher that looks for events alarming that refund to particular address must happen

bridge-monitor

  1. Introduce new endpoing to monitor that some Transfer was not refund.

bridge-ui

  1. Disable the Transfer operations for the end user if it is not whitelisted.

Did I miss something?

branciard commented 5 years ago

Thank you for the synthesis, it looks to cover all things!

For token-bridge description part, for 1. and 2. point, is it only one watcher that cover both functionalities: watching the update of the whitelist and also watch transfer Event, or 2 different watchers containers that must, at some point, communicate to each other to know the filtering policy? And if so, how do they communicate with each other? For this new Transfer watcher, do we create a new different case like affirmation-request-filtered-watcher or do we configure the existing affirmation-request-watcher to cover all cases?

akolotov commented 5 years ago
  1. We can use Redis to keep the actual whitelist. The watcher that looks for the whitelist updates in the contract will do the following:
    • as soon as a request to add/remove address appear, it will lock the record in Redis.
    • update the record in Redis. Suggestion to keep all the addresses in one record.
    • unlock the record in Redis.

So, the watcher that monitors Transfer will get all corresponding new events. If new there are new events it will:

  1. I would vote to have one watcher covering both functionality: filtered and unfiltered event monitoring
branciard commented 5 years ago

ok, make sense to use Redis for the whitelist shared state between containers. ok for one transfer event watcher covering both.

akolotov commented 5 years ago

@branciard so actually the question is whether you are able to implement this?

branciard commented 5 years ago

Yes, I will try to implement those functionalities listed above. It may take me a bit more time ( maybe 2 months from now) as I must also continue to learn and master all part of bridge code to implement this feature correctly and test this new functionality fully then after. I will rebase on develop branches during this time to keep me updated. I will contact you if I have questions along the process. Is it ok for you?

akolotov commented 5 years ago

@branciard thanks for such involvement! Yes, me personally and @patitonar will support you as much as we can. I am in GMT+2 timezone, Gerardo is in GMT-3 (I think). You can use also Telegram (https://t.me/AlexanderKolotov) to contact to me quickly.

branciard commented 5 years ago

Great! thank you, I will let you informed.