search

OpenST / mosaic-contracts

Mosaic-0: Gateways and anchors on top of Ethereum to scale DApps
https://discuss.openst.org/c/mosaic
MIT License
114 stars 31 forks source link

Gateways should store roots in ring buffer (Contract approach) #726

Closed deepesh-kn closed 5 years ago

deepesh-kn commented 5 years ago

Fixes: #698 After the discussion on library vs contract approach (2:3), it was decided to go with the contract approach. https://github.com/OpenST/mosaic-contracts/pull/722

This is done with the contract approach.

This has interface change. Mosaic.js need to provide backward compatibility.

schemar commented 5 years ago

Having a look 👁

@deepesh-kn what are the differences to #669 ? Would help to know, I think.

deepesh-kn commented 5 years ago

Having a look 👁

@deepesh-kn what are the differences to #669 ? Would help to know, I think.

Code changes are similar except for that uses the constant. uint256 private constant MAX_STORAGE_ROOTS = 256;

In this PR, the constructor accepts max storage item size in the constructor as a parameter.

deepesh-kn commented 5 years ago

Good catch 👍

Looks good to me 🐱. However, additional opinions are required.

Some general questions:

  • Should there not be a test that asserts that older than max state roots are deleted by GatewayBase?

Added unit test.

  • Are no other updates required for MockGatewayBase?

Update the code.

  • In which test cases is the actual gateway base used for storing (I am confused by the amount of mock and test replacements for the various smart contracts of the gateway)?

I too get confused. its in test/gateway/gateway_base/prove_gateway.js