search

omni / token-bridge

Oracle service for TokenBridge. TB is an interoperability solution between Ethereum networks for native to ERC20 and ERC20 to ERC20 cross chain transfers
109 stars 54 forks source link

Enhance check for duplicated transactions handling #24

Closed akolotov closed 6 years ago

akolotov commented 6 years ago

Depends on #37

Watchers implemented:

Currently the bridge uses estimateGas() JSON-RPC API to check if the transaction was already processed. This check is not always accurate. For example, the exception will be raised by estimateGas() if the bridge instance operates with the private key which does not belong to the actual validator (e.g. the validator was removed from the contract), or if the message which is signed by the validator is corrupted etc. In such kind of cases the bridge will report that transaction was already processed but it is not true. It will slow down investigation of the issues since the log message does not reflect actual error.

That's why the check used estimateGas() could be extended with usage of methods provided by the bridge contract to provide more accurate logs:

  1. The validator contract on the corresponding side could be inspected to make sure that the bridge is still is a validator by calling isValidator():

https://github.com/poanetwork/poa-bridge-contracts/blob/93969cd934190649d42691f054676d0d16a9777e/contracts/upgradeable_contracts/U_BridgeValidators.sol#L71

  1. processDeposits could be enhanced to check:

    • that the message was already handled by this validator by calling messagesSigned from Home Bridge contract
    • that the message was already relayed by calling isAlreadyProcessed from Home Bridge contract

  2. processCollectedSignatures could be enhanced to check:

    • that enough number of signatures collected on the Home side by comparing the value from the CollectedSignatures event and value returned by requiredSignatures from the Validator contract on Foreign side.
    • that the message was already relayed by calling deposits from Foreign Bridge contract

  3. processWithdraw could be enhanced to check:

    • that the message was already handled by this validator by calling withdrawalsSigned from Home Bridge contract
    • that the message was already completely relayed by calling isAlreadyProcessed from Home Bridge contract
akolotov commented 6 years ago

@pablofullana we need to decrease priority of this issue and do it after switching to v2.1 bridge contracts.

pablofullana commented 6 years ago

@fvictorio @patitonar can suggest list below the set of changes needed to be done as per new contracts (refactor_v1 branch)

akolotov commented 6 years ago

Addressed in #71