search

mosaicdao / mosaic-1

Mosaic-1 : meta-blockchains on Ethereum 1.x
8 stars 12 forks source link

Implementation of ERC20Gateway.proveCogateway #291

Closed abhayks1 closed 4 years ago

abhayks1 commented 4 years ago

PR addresses below points:

Note: Method has been updated to ERC20Gateway.proveGateway which is aligned with ConsensusCogateway.proveConsensusGateway method naming convention. https://github.com/mosaicdao/mosaic-1/blob/develop/contracts/consensus-gateway/ConsensusCogateway.sol#L189 It makes sense since here we are proving ERC20Cogateway contract. I have updated the same in the ticket comment.

Fixes #276

deepesh-kn commented 4 years ago

PR addresses below points:

  • Implementation of ERC20Gateway::proveCogateway
  • Unit tests with proof data

Note: Method has been updated to ERC20Gateway.proveCogateway which is aligned with ConsensusCogateway.proveConsensusGateway method naming convention. https://github.com/mosaicdao/mosaic-1/blob/develop/contracts/consensus-gateway/ConsensusCogateway.sol#L189 It makes sense since here we are proving ERC20Cogateway contract. I have updated the same in the ticket comment.

Fixes #276

We can change the ConsensusCogateway::proveConsensusGateway to ConsensusCogateway::proveGateway instead of the proposed change, else you need to make separate services in the facilitator.

Also as we were discussing about moving the message bus and proveGateway function in to ERC20GatewayBase, at that time we can make the change related to ConsensusCogateway.

abhayks1 commented 4 years ago 
Also as we were discussing about moving the message bus and proveGateway function in to ERC20GatewayBase, at that time we can make the change related to ConsensusCogateway.

This will reduce lot of code duplication as proveGateway can be shared by ConsensusGateway/ConsensusCogateway/ERC20Gateway/ERC20Cogateway contracts 👍.

abhayks1 commented 4 years ago

Available for review again 🍏