Merge pull request #678 from b-tarczynski/contracts/invalid-post-stat…

…e-result

Pass correct result on dispute invalid post state root

contracts/Create2Transfer.sol

@@ -22,16 +22,17 @@ contract Create2Transfer {
      * @notice processes the state transition of a commitment
      * */
     function processCreate2TransferCommit(
-        bytes32 stateRoot,
+        bytes32 currentStateRoot,
+        bytes32 postStateRoot,
         uint256 maxTxSize,
         uint256 feeReceiver,
         bytes memory txs,
         Types.StateMerkleProof[] memory proofs
-    ) public pure returns (bytes32, Types.Result result) {
+    ) public pure returns (Types.Result result) {
         if (txs.create2TransferHasExcessData())
-            return (stateRoot, Types.Result.BadCompression);
+            return Types.Result.BadCompression;
         uint256 size = txs.create2TransferSize();
-        if (size > maxTxSize) return (stateRoot, Types.Result.TooManyTx);
+        if (size > maxTxSize) return Types.Result.TooManyTx;
 
         uint256 fees = 0;
         // tokenID should be the same for all states in this commit
@@ -40,25 +41,28 @@ contract Create2Transfer {
 
         for (uint256 i = 0; i < size; i++) {
             _tx = txs.create2TransferDecode(i);
-            (stateRoot, result) = Transition.processCreate2Transfer(
-                stateRoot,
+            (currentStateRoot, result) = Transition.processCreate2Transfer(
+                currentStateRoot,
                 _tx,
                 tokenID,
                 proofs[i * 2],
                 proofs[i * 2 + 1]
             );
-            if (result != Types.Result.Ok) return (stateRoot, result);
+            if (result != Types.Result.Ok) return result;
             // Only trust fees when the result is good
             fees = fees.add(_tx.fee);
         }
-        (stateRoot, result) = Transition.processReceiver(
-            stateRoot,
+        (currentStateRoot, result) = Transition.processReceiver(
+            currentStateRoot,
             feeReceiver,
             tokenID,
             fees,
             proofs[size * 2]
         );
 
-        return (stateRoot, result);
+        if (result != Types.Result.Ok) return result;
+        if (currentStateRoot != postStateRoot)
+            return Types.Result.InvalidPostStateRoot;
+        return result;
     }
 }

contracts/MassMigrations.sol

@@ -23,19 +23,20 @@ contract MassMigration {
 
     /**
      * @notice processes the state transition of a commitment
-     * @param stateRoot represents the state before the state transition
+     * @param currentStateRoot represents the state before the state transition
      * */
     function processMassMigrationCommit(
-        bytes32 stateRoot,
+        bytes32 currentStateRoot,
+        bytes32 postStateRoot,
         uint256 maxTxSize,
         Types.MassMigrationBody memory committed,
         Types.StateMerkleProof[] memory proofs
-    ) public pure returns (bytes32, Types.Result result) {
+    ) public pure returns (Types.Result result) {
         if (committed.txs.massMigrationHasExcessData())
-            return (stateRoot, Types.Result.BadCompression);
+            return Types.Result.BadCompression;
 
         uint256 size = committed.txs.massMigrationSize();
-        if (size > maxTxSize) return (stateRoot, Types.Result.TooManyTx);
+        if (size > maxTxSize) return Types.Result.TooManyTx;
 
         Tx.MassMigration memory _tx;
         uint256 totalAmount = 0;
@@ -45,34 +46,38 @@ contract MassMigration {
 
         for (uint256 i = 0; i < size; i++) {
             _tx = committed.txs.massMigrationDecode(i);
-            (stateRoot, freshState, result) = Transition.processMassMigration(
-                stateRoot,
+            (currentStateRoot, freshState, result) = Transition
+                .processMassMigration(
+                currentStateRoot,
                 _tx,
                 committed.tokenID,
                 proofs[i]
             );
-            if (result != Types.Result.Ok) return (stateRoot, result);
+            if (result != Types.Result.Ok) return result;
 
             // Only trust these variables when the result is good
             totalAmount += _tx.amount;
             fees += _tx.fee;
             withdrawLeaves[i] = keccak256(freshState);
         }
-        (stateRoot, result) = Transition.processReceiver(
-            stateRoot,
+        (currentStateRoot, result) = Transition.processReceiver(
+            currentStateRoot,
             committed.feeReceiver,
             committed.tokenID,
             fees,
             proofs[size]
         );
-        if (result != Types.Result.Ok) return (stateRoot, result);
+        if (result != Types.Result.Ok) return result;
 
         if (totalAmount != committed.amount)
-            return (stateRoot, Types.Result.MismatchedAmount);
+            return Types.Result.MismatchedAmount;
 
         if (MerkleTree.merklize(withdrawLeaves) != committed.withdrawRoot)
-            return (stateRoot, Types.Result.BadWithdrawRoot);
+            return Types.Result.BadWithdrawRoot;
 
-        return (stateRoot, result);
+        if (currentStateRoot != postStateRoot)
+            return Types.Result.InvalidPostStateRoot;
+
+        return result;
     }
 }

contracts/Transfer.sol

@@ -23,17 +23,17 @@ contract Transfer {
      * @notice processes the state transition of a commitment
      * */
     function processTransferCommit(
-        bytes32 stateRoot,
+        bytes32 currentStateRoot,
+        bytes32 postStateRoot,
         uint256 maxTxSize,
         uint256 feeReceiver,
         bytes memory txs,
         Types.StateMerkleProof[] memory proofs
-    ) public pure returns (bytes32, Types.Result result) {
-        if (txs.transferHasExcessData())
-            return (stateRoot, Types.Result.BadCompression);
+    ) public pure returns (Types.Result result) {
+        if (txs.transferHasExcessData()) return Types.Result.BadCompression;
 
         uint256 size = txs.transferSize();
-        if (size > maxTxSize) return (stateRoot, Types.Result.TooManyTx);
+        if (size > maxTxSize) return Types.Result.TooManyTx;
 
         uint256 fees = 0;
         // tokenID should be the same for all states in this commit
@@ -42,25 +42,28 @@ contract Transfer {
 
         for (uint256 i = 0; i < size; i++) {
             _tx = txs.transferDecode(i);
-            (stateRoot, result) = Transition.processTransfer(
-                stateRoot,
+            (currentStateRoot, result) = Transition.processTransfer(
+                currentStateRoot,
                 _tx,
                 tokenID,
                 proofs[i * 2],
                 proofs[i * 2 + 1]
             );
-            if (result != Types.Result.Ok) return (stateRoot, result);
+            if (result != Types.Result.Ok) return result;
             // Only trust fees when the result is good
             fees = fees.add(_tx.fee);
         }
-        (stateRoot, result) = Transition.processReceiver(
-            stateRoot,
+        (currentStateRoot, result) = Transition.processReceiver(
+            currentStateRoot,
             feeReceiver,
             tokenID,
             fees,
             proofs[size * 2]
         );
 
-        return (stateRoot, result);
+        if (result != Types.Result.Ok) return result;
+        if (currentStateRoot != postStateRoot)
+            return Types.Result.InvalidPostStateRoot;
+        return result;
     }
 }

contracts/libs/Types.sol

@@ -251,6 +251,7 @@ library Types {
         BadCompression,
         TooManyTx,
         BadPrecompileCall,
-        NonexistentReceiver
+        NonexistentReceiver,
+        InvalidPostStateRoot
     }
 }

contracts/rollup/Rollup.sol

@@ -377,19 +377,17 @@ contract Rollup is BatchManager, EIP712, IEIP712 {
             "Target commitment is absent in the batch"
         );
 
-        (bytes32 processedStateRoot, Types.Result result) =
+        Types.Result result =
             transfer.processTransferCommit(
                 previous.commitment.stateRoot,
+                target.commitment.stateRoot,
                 paramMaxTxsPerCommit,
                 target.commitment.body.feeReceiver,
                 target.commitment.body.txs,
                 proofs
             );
 
-        if (
-            result != Types.Result.Ok ||
-            (processedStateRoot != target.commitment.stateRoot)
-        ) startRollingBack(batchID, result);
+        if (result != Types.Result.Ok) startRollingBack(batchID, result);
     }
 
     function disputeTransitionMassMigration(
@@ -407,18 +405,16 @@ contract Rollup is BatchManager, EIP712, IEIP712 {
             "Target commitment is absent in the batch"
         );
 
-        (bytes32 processedStateRoot, Types.Result result) =
+        Types.Result result =
             massMigration.processMassMigrationCommit(
                 previous.commitment.stateRoot,
+                target.commitment.stateRoot,
                 paramMaxTxsPerCommit,
                 target.commitment.body,
                 proofs
             );
 
-        if (
-            result != Types.Result.Ok ||
-            (processedStateRoot != target.commitment.stateRoot)
-        ) startRollingBack(batchID, result);
+        if (result != Types.Result.Ok) startRollingBack(batchID, result);
     }
 
     function disputeTransitionCreate2Transfer(
@@ -436,19 +432,17 @@ contract Rollup is BatchManager, EIP712, IEIP712 {
             "Target commitment is absent in the batch"
         );
 
-        (bytes32 processedStateRoot, Types.Result result) =
+        Types.Result result =
             create2Transfer.processCreate2TransferCommit(
                 previous.commitment.stateRoot,
+                target.commitment.stateRoot,
                 paramMaxTxsPerCommit,
                 target.commitment.body.feeReceiver,
                 target.commitment.body.txs,
                 proofs
             );
 
-        if (
-            result != Types.Result.Ok ||
-            (processedStateRoot != target.commitment.stateRoot)
-        ) startRollingBack(batchID, result);
+        if (result != Types.Result.Ok) startRollingBack(batchID, result);
     }
 
     function disputeSignatureTransfer(

contracts/test/TestCreate2Transfer.sol

@@ -47,21 +47,23 @@ contract TestCreate2Transfer is Create2Transfer {
     }
 
     function testProcessCreate2TransferCommit(
-        bytes32 stateRoot,
+        bytes32 currentStateRoot,
+        bytes32 postStateRoot,
         uint256 maxTxSize,
         uint256 feeReceiver,
         bytes memory txs,
         Types.StateMerkleProof[] memory proofs
-    ) public returns (bytes32, uint256) {
-        bytes32 newRoot;
-        uint256 operationCost = gasleft();
-        (newRoot, ) = processCreate2TransferCommit(
-            stateRoot,
-            maxTxSize,
-            feeReceiver,
-            txs,
-            proofs
-        );
-        return (newRoot, operationCost - gasleft());
+    ) public returns (uint256 gasCost, Types.Result) {
+        gasCost = gasleft();
+        Types.Result result =
+            processCreate2TransferCommit(
+                currentStateRoot,
+                postStateRoot,
+                maxTxSize,
+                feeReceiver,
+                txs,
+                proofs
+            );
+        return (gasCost - gasleft(), result);
     }
 }

contracts/test/TestMassMigration.sol

@@ -29,27 +29,21 @@ contract TestMassMigration is MassMigration {
     }
 
     function testProcessMassMigrationCommit(
-        bytes32 stateRoot,
+        bytes32 currentStateRoot,
+        bytes32 postStateRoot,
         uint256 maxTxSize,
         Types.MassMigrationBody memory commitmentBody,
         Types.StateMerkleProof[] memory proofs
-    )
-        public
-        view
-        returns (
-            uint256 gasCost,
-            bytes32,
-            Types.Result
-        )
-    {
+    ) public view returns (uint256 gasCost, Types.Result) {
         gasCost = gasleft();
-        (bytes32 postRoot, Types.Result result) =
+        Types.Result result =
             processMassMigrationCommit(
-                stateRoot,
+                currentStateRoot,
+                postStateRoot,
                 maxTxSize,
                 commitmentBody,
                 proofs
             );
-        return (gasCost - gasleft(), postRoot, result);
+        return (gasCost - gasleft(), result);
     }
 }

contracts/test/TestTransfer.sol

@@ -40,21 +40,23 @@ contract TestTransfer is Transfer {
     }
 
     function testProcessTransferCommit(
-        bytes32 stateRoot,
+        bytes32 currentStateRoot,
+        bytes32 postStateRoot,
         uint256 maxTxSize,
         uint256 feeReceiver,
         bytes memory txs,
         Types.StateMerkleProof[] memory proofs
-    ) public returns (bytes32, uint256) {
-        bytes32 newRoot;
-        uint256 operationCost = gasleft();
-        (newRoot, ) = processTransferCommit(
-            stateRoot,
-            maxTxSize,
-            feeReceiver,
-            txs,
-            proofs
-        );
-        return (newRoot, operationCost - gasleft());
+    ) public returns (uint256 gasCost, Types.Result) {
+        gasCost = gasleft();
+        Types.Result result =
+            processTransferCommit(
+                currentStateRoot,
+                postStateRoot,
+                maxTxSize,
+                feeReceiver,
+                txs,
+                proofs
+            );
+        return (gasCost - gasleft(), result);
     }
 }