Merge pull request matter-labs#147 from matter-labs/baldyash/98/fix_d…

…eposits_for_exodus_mode cancelOutstandingDepositsForExodusMode will not run out of gas
MuxinFeng · Jan 22, 2020 · f7c0417 · f7c0417
2 parents 94c5c0d + 0080f9c
commit f7c0417
Show file tree

Hide file tree

Showing 8 changed files with 306 additions and 282 deletions.
diff --git a/bin/contracts-test.sh b/bin/contracts-test.sh
@@ -1,8 +1,9 @@
 #!/bin/bash
+set -e
 
 . .setup_env
 
 echo contracts-test
 cd contracts
-yarn test  | tee ../test.log
+yarn test
 cd ..
diff --git a/contracts/contracts/Franklin.sol b/contracts/contracts/Franklin.sol
@@ -58,9 +58,6 @@ contract Franklin {
     /// @notice Base gas for full exit transaction
     uint256 constant BASE_FULL_EXIT_GAS = 170000;
 
-    /// @notice Max amount of any token must fit into uint128
-    uint256 constant MAX_VALUE = 2 ** 112 - 1;
-
     /// @notice ETH blocks verification expectation
     uint256 constant EXPECT_VERIFICATION_IN = 8 * 60 * 100;
 
@@ -227,8 +224,14 @@ contract Franklin {
 
     /// @notice Accrues users balances from deposit priority requests in Exodus mode
     /// @dev WARNING: Only for Exodus mode
-    function cancelOutstandingDepositsForExodusMode() internal {
-        bytes memory depositsPubData = priorityQueue.getOutstandingDeposits();
+    /// @dev Canceling may take several separate transactions to be completed
+    /// @param _number Supposed number of requests to look at
+    function cancelOutstandingDepositsForExodusMode(uint64 _number) external {
+        require(
+            exodusMode,
+            "frс11"
+        ); // frс11 - exodus mode is not activated
+        bytes memory depositsPubData = priorityQueue.deletePriorityRequestsAndPopOutstandingDeposits(_number);
         uint64 i = 0;
         while (i < depositsPubData.length) {
             bytes memory deposit = Bytes.slice(depositsPubData, i, ETH_ADDR_BYTES+TOKEN_BYTES+AMOUNT_BYTES);
@@ -289,11 +292,6 @@ contract Franklin {
             msg.value >= fee + _amount,
             "fdh11"
         ); // fdh11 - Not enough ETH provided
-
-        require(
-            _amount <= MAX_VALUE,
-            "fdh12"
-        ); // fdh12 - deposit amount value is heigher than Franklin is able to process
 
         if (msg.value != fee + _amount) {
             msg.sender.transfer(msg.value-(fee + _amount));
@@ -839,7 +837,6 @@ contract Franklin {
     function triggerExodusIfNeeded() internal returns (bool) {
         if (priorityQueue.triggerExodusIfNeeded()) {
             exodusMode = true;
-            cancelOutstandingDepositsForExodusMode();
             emit ExodusMode();
             return true;
         } else {

diff --git a/contracts/contracts/PriorityQueue.sol b/contracts/contracts/PriorityQueue.sol
@@ -156,14 +156,26 @@ contract PriorityQueue {
         return totalFee;
     }
 
-    /// @notice Concates open (outstanding) deposit requests public data
-    /// @return concated deposits public data
-    function getOutstandingDeposits() external view returns (bytes memory depositsPubData) {
-        for (uint64 i = firstPriorityRequestId; i < firstPriorityRequestId + totalOpenPriorityRequests; i++) {
-            if (priorityRequests[i].opType == DEPOSIT_OP) {
-                depositsPubData = Bytes.concat(depositsPubData, priorityRequests[i].pubData);
+    /// @notice Concates open (outstanding) deposit requests public data up to defined deposits number
+    /// @dev Deletes processed requests.
+    /// @param _number Supposed number of open requests to look at and delete
+    /// @return concated deposits public data for limited number of deposits so as not to go beyond the block gas limit in the caller function
+    function deletePriorityRequestsAndPopOutstandingDeposits(uint64 _number) external returns (bytes memory depositsPubData) {
+        requireFranklin();
+        require(
+            totalOpenPriorityRequests > 0,
+            "pgs11"
+        ); // pgs11 - no one priority request left
+        uint64 toProcess = totalOpenPriorityRequests < _number ? totalOpenPriorityRequests : _number;
+        for (uint64 i = 0; i < toProcess; i++) {
+            uint64 id = firstPriorityRequestId + i;
+            if (priorityRequests[id].opType == DEPOSIT_OP) {
+                depositsPubData = Bytes.concat(depositsPubData, priorityRequests[id].pubData);
             }
+            delete priorityRequests[id];
         }
+        firstPriorityRequestId += toProcess;
+        totalOpenPriorityRequests -= toProcess;
     }
 
     /// @notice Compares Rollup operation with corresponding priority requests' operation

diff --git a/contracts/src.ts/deploy.ts b/contracts/src.ts/deploy.ts
@@ -29,7 +29,7 @@ export const priorityQueueContractSourceCode = fs.readFileSync('flat/PriorityQue
 export const franklinTestContractCode      = require('../build/FranklinTest');
 export const verifierTestContractCode      = require('../build/VerifierTest');
 export const governanceTestContractCode    = require('../build/GovernanceTest');
-export const priorityQueueTestContractCode = require('../build/PriorityQueueTest')
+export const priorityQueueTestContractCode = require('../build/PriorityQueueTest');
 
 export async function publishSourceCodeToEtherscan(contractname, contractaddress, sourceCode, compiled, constructorParams: any[]) {
     const network = process.env.ETH_NETWORK;