diff --git a/contracts/gateway/SafeCore.sol b/contracts/gateway/SafeCore.sol index e335703c..e20b2b49 100644 --- a/contracts/gateway/SafeCore.sol +++ b/contracts/gateway/SafeCore.sol @@ -23,6 +23,7 @@ pragma solidity ^0.5.0; import "./WorkersInterface.sol"; import "../StateRootInterface.sol"; +import "../lib/CircularBufferUint.sol"; import "../lib/IsMemberInterface.sol"; import "../lib/MerklePatriciaProof.sol"; import "../lib/Organized.sol"; @@ -38,7 +39,7 @@ import "../lib/SafeMath.sol"; * by the workers that are registered as part of the `Organized` * interface. */ -contract SafeCore is StateRootInterface, Organized { +contract SafeCore is StateRootInterface, Organized, CircularBufferUint { using SafeMath for uint256; @@ -58,9 +59,6 @@ contract SafeCore is StateRootInterface, Organized { */ uint256 private remoteChainId; - /** Latest block height of block for which state root was committed. */ - uint256 private latestStateRootBlockHeight; - /** Address of the core on the auxiliary chain. Can be zero. */ address public coCore; @@ -68,21 +66,25 @@ contract SafeCore is StateRootInterface, Organized { /* Constructor */ /** - * @notice Contract constructor. + * @notice Contract constructor. * - * @param _remoteChainId _remoteChainId is the chain id of the chain that - * this core tracks. - * @param _blockHeight Block height at which _stateRoot needs to store. - * @param _stateRoot State root hash of given _blockHeight. - * @param _membersManager Address of a members manager contract. + * @param _remoteChainId _remoteChainId is the chain id of the chain that + * this core tracks. + * @param _blockHeight Block height at which _stateRoot needs to store. + * @param _stateRoot State root hash of given _blockHeight. + * @param _maxStateRoots The max number of state roots to store in the + * circular buffer. + * @param _membersManager Address of a members manager contract. */ constructor( uint256 _remoteChainId, uint256 _blockHeight, bytes32 _stateRoot, + uint256 _maxStateRoots, IsMemberInterface _membersManager ) Organized(_membersManager) + CircularBufferUint(_maxStateRoots) public { require( @@ -92,8 +94,8 @@ contract SafeCore is StateRootInterface, Organized { remoteChainId = _remoteChainId; - latestStateRootBlockHeight = _blockHeight; - stateRoots[latestStateRootBlockHeight] = _stateRoot; + stateRoots[_blockHeight] = _stateRoot; + CircularBufferUint.store(_blockHeight); } @@ -152,7 +154,7 @@ contract SafeCore is StateRootInterface, Organized { view returns (uint256 height_) { - height_ = latestStateRootBlockHeight; + height_ = CircularBufferUint.head(); } /** @@ -180,12 +182,13 @@ contract SafeCore is StateRootInterface, Organized { // Input block height should be valid require( - _blockHeight > latestStateRootBlockHeight, + _blockHeight > CircularBufferUint.head(), "Given block height is lower or equal to highest committed state root block height." ); stateRoots[_blockHeight] = _stateRoot; - latestStateRootBlockHeight = _blockHeight; + uint256 oldestStoredBlockHeight = CircularBufferUint.store(_blockHeight); + delete stateRoots[oldestStoredBlockHeight]; emit StateRootAvailable(_blockHeight, _stateRoot); diff --git a/contracts/lib/CircularBufferUint.sol b/contracts/lib/CircularBufferUint.sol new file mode 100644 index 00000000..9433e44e --- /dev/null +++ b/contracts/lib/CircularBufferUint.sol @@ -0,0 +1,107 @@ +pragma solidity ^0.5.0; + +// Copyright 2017 OpenST Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// ---------------------------------------------------------------------------- +// +// http://www.simpletoken.org/ +// +// ---------------------------------------------------------------------------- + +contract CircularBufferUint { + + + /** Storage */ + + /** + * The circular buffer that stores the latest `items.length` items. Once + * `items.length` items were stored, items will be overwritten starting at + * zero. + */ + uint256[] private items; + + /** + * The current index in the items array. The index increases up to + * `items.length - 1` and then resets to zero in an endless loop. This + * means that a new item will always overwrite the oldest item. + */ + uint256 private index; + + + /* Constructor */ + + /** + * @notice Create a new buffer with the size `_maxItems`. + * + * @param _maxItems Defines how many items this buffer stores before + * overwriting older items. + */ + constructor(uint256 _maxItems) public { + require( + _maxItems > 0, + "The max number of items to store in a circular buffer must be greater than 0." + ); + + items.length = _maxItems; + } + + + /* Internal functions */ + + /** + * @notice Store a new item in the circular buffer. + * + * @param _item The item to store in the circular buffer. + * + * @return overwrittenItem_ The item that was in the circular buffer's + * position where the new item is now stored. The + * overwritten item is no longer available in the + * circular buffer. + */ + function store(uint256 _item) internal returns(uint256 overwrittenItem_) { + nextIndex(); + + /* + * Retrieve the old item from the circular buffer before overwriting it + * with the new item. + */ + overwrittenItem_ = items[index]; + items[index] = _item; + } + + /** + * @notice Get the most recent item that was stored in the circular buffer. + * + * @return head_ The most recently stored item. + */ + function head() internal view returns(uint256 head_) { + head_ = items[index]; + } + + + /* Private functions */ + + /** + * @notice Updates the index of the circular buffer to point to the next + * slot of where to store an item. Resets to zero if it gets to the + * end of the array that represents the circular. + */ + function nextIndex() private { + index++; + if (index == items.length) { + index = 0; + } + } +} diff --git a/contracts/test/core/MockSafeCore.sol b/contracts/test/core/MockSafeCore.sol index 3b58d3e7..a7252fe4 100644 --- a/contracts/test/core/MockSafeCore.sol +++ b/contracts/test/core/MockSafeCore.sol @@ -13,24 +13,28 @@ contract MockSafeCore is SafeCore { /* Public functions */ /** - * @notice Contract constructor + * @notice Contract constructor. * - * @param _remoteChainId _remoteChainId is the chain id of the chain that - * this core tracks. - * @param _blockHeight block height at which _stateRoot needs to store. - * @param _stateRoot state root hash of given _blockHeight. - * @param _membersManager Address of a members manager contract. + * @param _remoteChainId _remoteChainId is the chain id of the chain that + * this core tracks. + * @param _blockHeight Block height at which _stateRoot needs to store. + * @param _stateRoot State root hash of given _blockHeight. + * @param _maxStateRoots The max number of state roots to store in the + * circular buffer. + * @param _membersManager Address of a members manager contract. */ constructor( uint256 _remoteChainId, uint256 _blockHeight, bytes32 _stateRoot, + uint256 _maxStateRoots, IsMemberInterface _membersManager ) SafeCore( _remoteChainId, _blockHeight, _stateRoot, + _maxStateRoots, _membersManager ) public diff --git a/contracts/test/lib/TestCircularBufferUint.sol b/contracts/test/lib/TestCircularBufferUint.sol new file mode 100644 index 00000000..5ead7226 --- /dev/null +++ b/contracts/test/lib/TestCircularBufferUint.sol @@ -0,0 +1,64 @@ +pragma solidity ^0.5.0; + +// Copyright 2017 OpenST Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// ---------------------------------------------------------------------------- +// +// http://www.simpletoken.org/ +// +// ---------------------------------------------------------------------------- + +import "../../lib/CircularBufferUint.sol"; + +contract TestCircularBufferUint is CircularBufferUint { + + + /* Constructor */ + + /** + * @notice Create a new test buffer with the size `_maxItems`. + * + * @param _maxItems Defines how many items this test buffer stores before + * overwriting older items. + */ + constructor(uint256 _maxItems) public CircularBufferUint(_maxItems) {} + + + /* Internal functions */ + + /** + * @notice Store a new item in the circular test buffer. + * + * @param _item The item to store in the circular test buffer. + * + * @return overwrittenItem_ The item that was in the circular test buffer's + * position where the new item is now stored. The + * overwritten item is no longer available in the + * circular test buffer. + */ + function storeExternal(uint256 _item) external returns(uint256 overwrittenItem_) { + overwrittenItem_ = CircularBufferUint.store(_item); + } + + /** + * @notice Get the most recent item that was stored in the circular test + * buffer. + * + * @return head_ The most recently stored item. + */ + function headExternal() external view returns(uint256 head_) { + head_ = CircularBufferUint.head(); + } +} diff --git a/test/gateway/safe_core.js b/test/gateway/safe_core.js index 5952b6ec..57c5d6b9 100644 --- a/test/gateway/safe_core.js +++ b/test/gateway/safe_core.js @@ -105,16 +105,28 @@ contract('Core', function (accounts) { }); describe('commitStateRoot', async () => { - // Before all. - before(async () => { + let core; + let worker; + let blockHeight; + let stateRoot; + let maxNumberOfStateRoots; + + beforeEach(async () => { blockHeight = 5; - contractsData = await coreUtils.deployCore(artifacts, accounts); + contractsData = await coreUtils.deployCore( + artifacts, + accounts, + blockHeight, + ); core = contractsData.core; worker = contractsData.worker; stateRoot = proof.account.stateRoot; + maxNumberOfStateRoots = contractsData.numberOfStateRoots; }); it('should be able to commit state root and getStateRoot for given block height', async () => { + blockHeight++; + let response = await core.commitStateRoot.call( blockHeight, stateRoot, @@ -151,6 +163,55 @@ contract('Core', function (accounts) { assert.equal(latestStateRootBlockHeight.toNumber(), blockHeight); }); + it('should store only the given number of max store roots', async () => { + /* + * It should store the given state roots and they should be + * available for querying afterwards. After the max number of state + * roots has been exceeded, the old state roots should no longer be + * available. + */ + + let iterations = maxNumberOfStateRoots * 2; + for (let i = 0; i < iterations; i++) { + blockHeight++; + await core.commitStateRoot( + blockHeight, + stateRoot, + { from: worker }, + ); + + // Check that the older state root has been deleted. + if (i > maxNumberOfStateRoots) { + let prunedBlockHeight = blockHeight - maxNumberOfStateRoots; + let storedStateRoot = await core.getStateRoot.call( + new BN(prunedBlockHeight), + ); + + assert.strictEqual( + storedStateRoot, + '0x0000000000000000000000000000000000000000000000000000000000000000', + 'There should not be any state root stored at a ' + + 'pruned height. It should have been reset by now.', + ); + + /* + * The state root that is one block younger than the pruned + * one should still be available. + */ + let existingBlockHeight = prunedBlockHeight + 1; + storedStateRoot = await core.getStateRoot.call( + new BN(existingBlockHeight), + ); + assert.strictEqual( + storedStateRoot, + stateRoot, + 'The stored state root should still exist.', + ); + + } + } + }); + it('should not be able to commit state root of block height which is equal to latest block height', async () => { await utils.expectThrow( core.commitStateRoot(blockHeight, stateRoot, { from: worker }), @@ -158,20 +219,23 @@ contract('Core', function (accounts) { }); it('should not be able to commit state root of block height which is less than latest block height', async () => { + blockHeight--; await utils.expectThrow( - core.commitStateRoot(3, stateRoot, { from: worker }) + core.commitStateRoot(blockHeight, stateRoot, { from: worker }) ); }); it('should not be able to commit state root of block height if non worker commits root', async () => { + blockHeight++; await utils.expectThrow( - core.commitStateRoot(6, stateRoot, { from: accounts[0] }) + core.commitStateRoot(blockHeight, stateRoot, { from: accounts[0] }) ); }); it('should not be able to commit state root when state root is empty', async () => { + blockHeight++; await utils.expectThrow( - core.commitStateRoot(6, '0x', { from: worker }) + core.commitStateRoot(blockHeight, '0x', { from: worker }) ); }); diff --git a/test/gateway/safe_core_utils.js b/test/gateway/safe_core_utils.js index 0d601300..71ebdfec 100644 --- a/test/gateway/safe_core_utils.js +++ b/test/gateway/safe_core_utils.js @@ -19,23 +19,32 @@ // // ---------------------------------------------------------------------------- +const BN = require('bn.js'); + const SafeCore = artifacts.require("./SafeCore.sol") , Organization = artifacts.require("MockOrganization.sol") , proof = require('../data/proof'); ; /// @dev Deploy -module.exports.deployCore = async (artifacts, accounts) => { +module.exports.deployCore = async (artifacts, accounts, blockHeight) => { + if (blockHeight === undefined) { + blockHeight = 0; + } + const remoteChainId = 1410; const organizationOwner = accounts[7]; const worker = accounts[8]; + const numberOfStateRoots = new BN(10); + // Deploy worker contract const organization = await Organization.new(organizationOwner, worker); const core = await SafeCore.new( remoteChainId, - 0, + blockHeight, proof.account.stateRoot, + numberOfStateRoots, organization.address, { from: accounts[0] }, ); @@ -44,6 +53,7 @@ module.exports.deployCore = async (artifacts, accounts) => { owner: organizationOwner, worker: worker, remoteChainId: remoteChainId, + numberOfStateRoots: numberOfStateRoots, }; }; diff --git a/test/lib/circular_buffer_uint/constructor.js b/test/lib/circular_buffer_uint/constructor.js new file mode 100644 index 00000000..8c3a5d33 --- /dev/null +++ b/test/lib/circular_buffer_uint/constructor.js @@ -0,0 +1,28 @@ +// Copyright 2018 OpenST Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +const Utils = require('../../test_lib/utils.js'); +const BN = require('bn.js'); + +const CircularBufferUint = artifacts.require('CircularBufferUint'); + +contract('CircularBufferUint.constructor()', async (accounts) => { + + it('reverts if the given buffer length is 0', async () => { + await Utils.expectRevert( + CircularBufferUint.new(new BN(0)), + 'The max number of items to store in a circular buffer must be greater than 0\\.', + ); + }); +}); diff --git a/test/lib/circular_buffer_uint/store.js b/test/lib/circular_buffer_uint/store.js new file mode 100644 index 00000000..fceb6183 --- /dev/null +++ b/test/lib/circular_buffer_uint/store.js @@ -0,0 +1,77 @@ +// Copyright 2018 OpenST Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +const BN = require('bn.js'); + +const CircularBufferUint = artifacts.require('TestCircularBufferUint'); + +contract('CircularBufferUint.store()', async (accounts) => { + + it('stores the given number of max items', async () => { + let data = [ + new BN(0), + new BN(0), + new BN(0), + new BN(0), + new BN(0), + new BN(0), + new BN(0), + new BN(0), + new BN(0), + new BN(0), + new BN(1), + new BN(2), + new BN(3), + new BN(4), + new BN(5), + new BN(6), + new BN(7), + new BN(8), + new BN(9), + new BN(10), + new BN(11), + new BN(12), + new BN(13), + new BN(14), + new BN(15), + new BN(16), + new BN(17), + new BN(18), + new BN(19), + new BN(20), + ]; + + let bufferLength = 10; + let buffer = await CircularBufferUint.new(new BN(bufferLength)); + + let count = data.length; + for (let i = bufferLength; i < count; i++) { + let previousItem = await buffer.storeExternal.call(data[i]); + let expectedPreviousItem = data[i - bufferLength]; + assert( + previousItem.eq(expectedPreviousItem), + 'The contract did not return the expected item that should ' + + 'have been overwritten.', + ); + + await buffer.storeExternal(data[i]); + let head = await buffer.headExternal.call(); + let expectedHead = data[i]; + assert( + head.eq(expectedHead), + 'The contract did not update the head to the latest stored item.', + ); + } + }); +});