Merge pull request #104 from ethstorage/fix_xshard

Opening a new shard fix

contracts/DecentralizedKV.sol

@@ -114,8 +114,8 @@ contract DecentralizedKV is OwnableUpgradeable {
         return _upfrontPayment(block.timestamp);
     }
 
-    /// @notice Checks before appending the key-value.
-    function _prepareAppend(uint256 _batchSize) internal virtual {
+    /// @notice Checks while appending the key-value.
+    function _checkAppend(uint256 _batchSize) internal virtual {
         require(msg.value >= upfrontPayment() * _batchSize, "DecentralizedKV: not enough batch payment");
     }
 
@@ -152,7 +152,7 @@ contract DecentralizedKV is OwnableUpgradeable {
             res[i] = paddr.kvIdx;
         }
 
-        _prepareAppend(batchPaymentSize);
+        _checkAppend(batchPaymentSize);
 
         return res;
     }

contracts/EthStorageContractL2.sol

@@ -28,62 +28,29 @@ contract EthStorageContractL2 is EthStorageContract2 {
     IL1Block internal constant L1_BLOCK = IL1Block(0x4200000000000000000000000000000000000015);
 
     /// @notice Constructs the EthStorageContractL2 contract.
-    constructor(
-        Config memory _config,
-        uint256 _startTime,
-        uint256 _storageCost,
-        uint256 _dcfFactor
-    ) EthStorageContract2(_config, _startTime, _storageCost, _dcfFactor) {}
+    constructor(Config memory _config, uint256 _startTime, uint256 _storageCost, uint256 _dcfFactor)
+        EthStorageContract2(_config, _startTime, _storageCost, _dcfFactor)
+    {}
+
+    /// @notice Get the current block number
+    function _blockNumber() internal view override returns (uint256) {
+        return L1_BLOCK.number();
+    }
+
+    /// @notice Get the current block timestamp
+    function _blockTs() internal view override returns (uint256) {
+        return L1_BLOCK.timestamp();
+    }
 
     /// @notice Get the randao value from the L1 blockhash.
-    function _getRandao(uint256 _l1BlockNumber, bytes calldata _headerRlpBytes) internal view returns (bytes32) {
+    function _getRandao(uint256 _l1BlockNumber, bytes calldata _headerRlpBytes)
+        internal
+        view
+        override
+        returns (bytes32)
+    {
         bytes32 bh = L1_BLOCK.blockHash(_l1BlockNumber);
         require(bh != bytes32(0), "EthStorageContractL2: failed to obtain blockhash");
-
         return RandaoLib.verifyHeaderAndGetRandao(bh, _headerRlpBytes);
     }
-
-    /// @notice We are still using L1 block number, timestamp, and blockhash to mine eventhough we are on L2.
-    /// @param _blockNumber  L1 blocknumber.
-    /// @param _shardId  Shard ID.
-    /// @param _miner  Miner address.
-    /// @param _nonce  Nonce.
-    /// @param _encodedSamples  Encoded samples.
-    /// @param _masks  Sample masks.
-    /// @param _randaoProof  L1 block header RLP bytes.
-    /// @param _inclusiveProofs  Sample inclusive proofs.
-    /// @param _decodeProof  Mask decode proof.
-    function _mine(
-        uint256 _blockNumber,
-        uint256 _shardId,
-        address _miner,
-        uint256 _nonce,
-        bytes32[] memory _encodedSamples,
-        uint256[] memory _masks,
-        bytes calldata _randaoProof,
-        bytes[] calldata _inclusiveProofs,
-        bytes[] calldata _decodeProof
-    ) internal override {
-        // Obtain the blockhash of the block number of recent blocks
-        require(L1_BLOCK.number() - _blockNumber <= MAX_L1_MINING_DRIFT, "EthStorageContractL2: block number too old");
-        // To avoid stack too deep, we resue the hash0 instead of using randao
-
-        bytes32 hash0 = _getRandao(_blockNumber, _randaoProof);
-        // Estimate block timestamp
-        uint256 mineTs = L1_BLOCK.timestamp() - (L1_BLOCK.number() - _blockNumber) * 12;
-
-        // Given a blockhash and a miner, we only allow sampling up to nonce limit times.
-        require(_nonce < nonceLimit, "EthStorageContractL2: nonce too big");
-
-        // Check if the data matches the hash in metadata and obtain the solution hash.
-        hash0 = keccak256(abi.encode(_miner, hash0, _nonce));
-        hash0 = verifySamples(_shardId, hash0, _miner, _encodedSamples, _masks, _inclusiveProofs, _decodeProof);
-
-        // Check difficulty
-        uint256 diff = _calculateDiffAndInitHashSingleShard(_shardId, mineTs);
-        uint256 required = uint256(2 ** 256 - 1) / diff;
-        require(uint256(hash0) <= required, "EthStorageContractL2: diff not match");
-
-        _rewardMiner(_shardId, _miner, mineTs, diff);
-    }
 }

contracts/StorageContract.sol

@@ -145,44 +145,45 @@ abstract contract StorageContract is DecentralizedKV {
     /// @notice People can sent ETH to the contract.
     function sendValue() public payable {}
 
-    /// @notice Checks the payment using the last mine time.
-    function _prepareAppendWithTimestamp(uint256 _timestamp, uint256 _batchSize) internal {
-        uint256 totalEntries = kvEntryCount + 1; // include the one to be put
-        uint256 shardId = kvEntryCount >> SHARD_ENTRY_BITS; // shard id of the new KV
-        if ((totalEntries % (1 << SHARD_ENTRY_BITS)) == 1) {
-            // Open a new shard if the KV is the first one of the shard
-            // and mark the shard is ready to mine.
-            // (TODO): Setup shard difficulty as current difficulty / factor?
-            if (shardId != 0) {
-                // shard0 is already opened in constructor
-                infos[shardId].lastMineTime = _timestamp;
-            }
-        }
+    /// @notice Upfront payment for the next insertion
+    function upfrontPayment() public view virtual override returns (uint256) {
+        return _upfrontPaymentInBatch(kvEntryCount, 1);
+    }
 
-        require(
-            msg.value >= _upfrontPayment(infos[shardId].lastMineTime) * _batchSize,
-            "StorageContract: not enough batch payment"
-        );
+    /// @notice Upfront payment for a batch insertion
+    /// @param _batchSize The blob count for a batch insertion.
+    /// @return The total payment for a batch insertion.
+    function upfrontPaymentInBatch(uint256 _batchSize) public view returns (uint256) {
+        return _upfrontPaymentInBatch(kvEntryCount, _batchSize);
     }
 
-    /// @notice Upfront payment for the next insertion
-    function upfrontPayment() public view virtual override returns (uint256) {
-        uint256 totalEntries = kvEntryCount + 1; // include the one to be put
-        uint256 shardId = kvEntryCount >> SHARD_ENTRY_BITS; // shard id of the new KV
-        // shard0 is already opened in constructor
-        if ((totalEntries % (1 << SHARD_ENTRY_BITS)) == 1 && shardId != 0) {
-            // Open a new shard if the KV is the first one of the shard
-            // and mark the shard is ready to mine.
-            // (TODO): Setup shard difficulty as current difficulty / factor?
-            return _upfrontPayment(block.timestamp);
+    /// @notice Upfront payment for a batch insertion
+    function _upfrontPaymentInBatch(uint256 _kvEntryCount, uint256 _batchSize) private view returns (uint256) {
+        uint256 shardId = _kvEntryCount >> SHARD_ENTRY_BITS;
+        uint256 totalEntries = _kvEntryCount + _batchSize; // include the batch to be put
+        uint256 totalPayment = 0;
+        if ((totalEntries >> SHARD_ENTRY_BITS) > shardId) {
+            uint256 kvCountNew = totalEntries % (1 << SHARD_ENTRY_BITS);
+            totalPayment += _upfrontPayment(_blockTs()) * kvCountNew;
+            totalPayment += _upfrontPayment(infos[shardId].lastMineTime) * (_batchSize - kvCountNew);
         } else {
-            return _upfrontPayment(infos[shardId].lastMineTime);
+            totalPayment += _upfrontPayment(infos[shardId].lastMineTime) * _batchSize;
         }
+        return totalPayment;
     }
 
     /// @inheritdoc DecentralizedKV
-    function _prepareAppend(uint256 _batchSize) internal virtual override {
-        return _prepareAppendWithTimestamp(block.timestamp, _batchSize);
+    function _checkAppend(uint256 _batchSize) internal virtual override {
+        uint256 kvEntryCountPrev = kvEntryCount - _batchSize; // kvEntryCount already increased
+        uint256 totalPayment = _upfrontPaymentInBatch(kvEntryCountPrev, _batchSize);
+        require(msg.value >= totalPayment, "StorageContract: not enough batch payment");
+
+        uint256 shardId = kvEntryCount >> SHARD_ENTRY_BITS; // shard id after the batch
+        if (shardId > (kvEntryCountPrev >> SHARD_ENTRY_BITS)) {
+            // Open a new shard and mark the shard is ready to mine.
+            // (TODO): Setup shard difficulty as current difficulty / factor?
+            infos[shardId].lastMineTime = _blockTs();
+        }
     }
 
     /// @notice Verify the samples of the BLOBs by the miner (storage provider) including
@@ -278,16 +279,16 @@ abstract contract StorageContract is DecentralizedKV {
 
     /// @notice Get the mining reward.
     /// @param _shardId     The shard id.
-    /// @param _blockNumber The block number.
+    /// @param _blockNum The block number.
     /// @return The mining reward.
-    function miningReward(uint256 _shardId, uint256 _blockNumber) public view returns (uint256) {
-        uint256 minedTs = block.timestamp - (block.number - _blockNumber) * 12;
+    function miningReward(uint256 _shardId, uint256 _blockNum) public view returns (uint256) {
+        uint256 minedTs = _getMinedTs(_blockNum);
         (,, uint256 minerReward) = _miningReward(_shardId, minedTs);
         return minerReward;
     }
 
     /// @notice Mine a block.
-    /// @param _blockNumber     The block number.
+    /// @param _blockNum     The block number.
     /// @param _shardId         The shard id.
     /// @param _miner           The miner address.
     /// @param _nonce           The nonce.
@@ -297,7 +298,7 @@ abstract contract StorageContract is DecentralizedKV {
     /// @param _inclusiveProofs The inclusive proofs.
     /// @param _decodeProof     The decode proof.
     function mine(
-        uint256 _blockNumber,
+        uint256 _blockNum,
         uint256 _shardId,
         address _miner,
         uint256 _nonce,
@@ -308,15 +309,7 @@ abstract contract StorageContract is DecentralizedKV {
         bytes[] calldata _decodeProof
     ) public virtual {
         _mine(
-            _blockNumber,
-            _shardId,
-            _miner,
-            _nonce,
-            _encodedSamples,
-            _masks,
-            _randaoProof,
-            _inclusiveProofs,
-            _decodeProof
+            _blockNum, _shardId, _miner, _nonce, _encodedSamples, _masks, _randaoProof, _inclusiveProofs, _decodeProof
         );
     }
 
@@ -340,7 +333,7 @@ abstract contract StorageContract is DecentralizedKV {
     ///         to decoded one. The decoded samples will be used to perform inclusive check with on-chain datahashes.
     ///         The encoded samples will be used to calculate the solution hash, and if the hash passes the difficulty check,
     ///         the miner, or say the storage provider, shall be rewarded by the token number from out economic models
-    /// @param _blockNumber     The block number.
+    /// @param _blockNum     The block number.
     /// @param _shardId         The shard id.
     /// @param _miner           The miner address.
     /// @param _nonce           The nonce.
@@ -350,7 +343,7 @@ abstract contract StorageContract is DecentralizedKV {
     /// @param _inclusiveProofs The inclusive proofs.
     /// @param _decodeProof     The decode proof.
     function _mine(
-        uint256 _blockNumber,
+        uint256 _blockNum,
         uint256 _shardId,
         address _miner,
         uint256 _nonce,
@@ -360,12 +353,11 @@ abstract contract StorageContract is DecentralizedKV {
         bytes[] calldata _inclusiveProofs,
         bytes[] calldata _decodeProof
     ) internal virtual {
-        // Obtain the blockhash of the block number of recent blocks
-        require(block.number - _blockNumber <= MAX_L1_MINING_DRIFT, "StorageContract: block number too old");
+        require(_blockNumber() - _blockNum <= MAX_L1_MINING_DRIFT, "StorageContract: block number too old");
         // To avoid stack too deep, we resue the hash0 instead of using randao
-        bytes32 hash0 = RandaoLib.verifyHistoricalRandao(_blockNumber, _randaoProof);
+        bytes32 hash0 = _getRandao(_blockNum, _randaoProof);
         // Estimate block timestamp
-        uint256 mineTs = block.timestamp - (block.number - _blockNumber) * 12;
+        uint256 mineTs = _getMinedTs(_blockNum);
 
         // Given a blockhash and a miner, we only allow sampling up to nonce limit times.
         require(_nonce < nonceLimit, "StorageContract: nonce too big");
@@ -382,6 +374,28 @@ abstract contract StorageContract is DecentralizedKV {
         _rewardMiner(_shardId, _miner, mineTs, diff);
     }
 
+    /// @notice Get the current block number
+    function _blockNumber() internal view virtual returns (uint256) {
+        return block.number;
+    }
+
+    /// @notice Get the current block timestamp
+    function _blockTs() internal view virtual returns (uint256) {
+        return block.timestamp;
+    }
+
+    /// @notice Get the randao value by block number.
+    function _getRandao(uint256 _blockNum, bytes calldata _headerRlpBytes) internal view virtual returns (bytes32) {
+        bytes32 bh = blockhash(_blockNum);
+        require(bh != bytes32(0), "StorageContract: failed to obtain blockhash");
+        return RandaoLib.verifyHeaderAndGetRandao(bh, _headerRlpBytes);
+    }
+
+    /// @notice Get the mined timestamp
+    function _getMinedTs(uint256 _blockNum) internal view returns (uint256) {
+        return _blockTs() - (_blockNumber() - _blockNum) * 12;
+    }
+
     /// @notice Return the sample size bits.
     function sampleSizeBits() public pure returns (uint256) {
         return SAMPLE_SIZE_BITS;

contracts/libraries/RandaoLib.sol

@@ -35,17 +35,4 @@ library RandaoLib {
         require(_headerHash == item.rlpBytesKeccak256(), "RandaoLib: header hash mismatch");
         return getRandaoFromHeader(item);
     }
-
-    /// @notice Get the historical Randao mixDigest by block number
-    /// @param _blockNumber The block number
-    /// @param _headerRlpBytes The RLP data of the header
-    /// @return The Randao mixDigest
-    function verifyHistoricalRandao(
-        uint256 _blockNumber,
-        bytes memory _headerRlpBytes
-    ) internal view returns (bytes32) {
-        bytes32 bh = blockhash(_blockNumber);
-        require(bh != bytes32(0), "RandaoLib: failed to obtain blockhash");
-        return verifyHeaderAndGetRandao(bh, _headerRlpBytes);
-    }
 }

contracts/test/EthStorageContractTest.t.sol

@@ -35,7 +35,7 @@ contract EthStorageContractTest is Test {
 
         uint256 insufficientCost = storageContract.upfrontPayment() - 1;
 
-        // Expect the specific revert reason from _prepareAppend due to insufficient msg.value
+        // Expect the specific revert reason from _checkAppend due to insufficient msg.value
         vm.expectRevert("StorageContract: not enough batch payment");
         storageContract.putBlob{value: insufficientCost}(key, blobIdx, length);
 
@@ -75,7 +75,7 @@ contract EthStorageContractTest is Test {
         storageContract.putBlobs{value: insufficientCost}(keys, blobIdxs, lengths);
 
         // Enough storage cost
-        uint256 sufficientCost = 2 * storageContract.upfrontPayment();
+        uint256 sufficientCost = storageContract.upfrontPaymentInBatch(2);
         storageContract.putBlobs{value: sufficientCost}(keys, blobIdxs, lengths);
 
         assertEq(storageContract.kvEntryCount(), 2);
@@ -99,4 +99,27 @@ contract EthStorageContractTest is Test {
         assertEq(storageContract.size(bytes32(uint256(1))), 20);
         assertEq(storageContract.size(bytes32(uint256(2))), 30);
     }
+
+    function testPutBlobsXshard() public {
+        uint256 size = 6;
+        bytes32[] memory keys = new bytes32[](size);
+        uint256[] memory blobIdxs = new uint256[](size);
+        uint256[] memory lengths = new uint256[](size);
+        for (uint256 i = 0; i < size; i++) {
+            keys[i] = bytes32(uint256(i));
+            blobIdxs[i] = i;
+            lengths[i] = 10 + i * 10;
+        }
+
+        uint256 sufficientCost = storageContract.upfrontPaymentInBatch(size);
+        uint256 insufficientCost = sufficientCost - 1;
+
+        // Expect the specific revert reason from _prepareBatchAppend due to insufficient msg.value
+        vm.expectRevert("StorageContract: not enough batch payment");
+        storageContract.putBlobs{value: insufficientCost}(keys, blobIdxs, lengths);
+
+        // Enough storage cost
+        storageContract.putBlobs{value: sufficientCost}(keys, blobIdxs, lengths);
+        assertEq(storageContract.kvEntryCount(), size);
+    }
 }

contracts/test/TestEthStorageContract.sol

@@ -145,12 +145,9 @@ contract TestEthStorageContract is EthStorageContract {
         bytes[] calldata inclusiveProofs,
         bytes[] calldata decodeProof
     ) internal {
-        // Obtain the blockhash of the block number of recent blocks
-        require(block.number - blockNumber <= 64, "block number too old");
-        // To avoid stack too deep, we resue the hash0 instead of using randao
-        bytes32 hash0 = RandaoLib.verifyHistoricalRandao(blockNumber, randaoProof);
-        // Estimate block timestamp
-        uint256 mineTs = block.timestamp - (block.number - blockNumber) * 12;
+        require(_blockNumber() - blockNumber <= MAX_L1_MINING_DRIFT, "block number too old");
+        bytes32 hash0 = _getRandao(blockNumber, randaoProof);
+        uint256 mineTs = _getMinedTs(blockNumber);
 
         // Given a blockhash and a miner, we only allow sampling up to nonce limit times.
         require(nonce < nonceLimit, "nonce too big");

contracts/test/TestRandao.sol

@@ -7,8 +7,4 @@ contract TestRandao {
     function verifyHeaderAndGetRandao(bytes32 headerHash, bytes memory headerRlpBytes) public pure returns (bytes32) {
         return RandaoLib.verifyHeaderAndGetRandao(headerHash, headerRlpBytes);
     }
-
-    function verifyHistoricalRandao(uint256 blockNumber, bytes memory proof) public view returns (bytes32) {
-        return RandaoLib.verifyHistoricalRandao(blockNumber, proof);
-    }
 }

test/randao-test.js

@@ -58,8 +58,8 @@ describe("Randao Test", function () {
       const Randao = await ethers.getContractFactory("TestRandao");
       const rd = await Randao.deploy();
       await rd.deployed();
-
-      let randao = await rd.verifyHistoricalRandao(bn, encodedHeader);
+      
+      let randao = await rd.verifyHeaderAndGetRandao(hash, encodedHeader);
       expect(randao).to.equal(block.mixHash);
     });
 });