Skip to content
This repository was archived by the owner on Jun 29, 2023. It is now read-only.

Commit f1c13fe

Browse files
authored
Merge pull request #678 from b-tarczynski/contracts/invalid-post-state-result
Pass correct result on dispute invalid post state root
2 parents 47e6681 + 196d7a4 commit f1c13fe

13 files changed

+218
-131
lines changed

contracts/Create2Transfer.sol

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,17 @@ contract Create2Transfer {
2222
* @notice processes the state transition of a commitment
2323
* */
2424
function processCreate2TransferCommit(
25-
bytes32 stateRoot,
25+
bytes32 currentStateRoot,
26+
bytes32 postStateRoot,
2627
uint256 maxTxSize,
2728
uint256 feeReceiver,
2829
bytes memory txs,
2930
Types.StateMerkleProof[] memory proofs
30-
) public pure returns (bytes32, Types.Result result) {
31+
) public pure returns (Types.Result result) {
3132
if (txs.create2TransferHasExcessData())
32-
return (stateRoot, Types.Result.BadCompression);
33+
return Types.Result.BadCompression;
3334
uint256 size = txs.create2TransferSize();
34-
if (size > maxTxSize) return (stateRoot, Types.Result.TooManyTx);
35+
if (size > maxTxSize) return Types.Result.TooManyTx;
3536

3637
uint256 fees = 0;
3738
// tokenID should be the same for all states in this commit
@@ -40,25 +41,28 @@ contract Create2Transfer {
4041

4142
for (uint256 i = 0; i < size; i++) {
4243
_tx = txs.create2TransferDecode(i);
43-
(stateRoot, result) = Transition.processCreate2Transfer(
44-
stateRoot,
44+
(currentStateRoot, result) = Transition.processCreate2Transfer(
45+
currentStateRoot,
4546
_tx,
4647
tokenID,
4748
proofs[i * 2],
4849
proofs[i * 2 + 1]
4950
);
50-
if (result != Types.Result.Ok) return (stateRoot, result);
51+
if (result != Types.Result.Ok) return result;
5152
// Only trust fees when the result is good
5253
fees = fees.add(_tx.fee);
5354
}
54-
(stateRoot, result) = Transition.processReceiver(
55-
stateRoot,
55+
(currentStateRoot, result) = Transition.processReceiver(
56+
currentStateRoot,
5657
feeReceiver,
5758
tokenID,
5859
fees,
5960
proofs[size * 2]
6061
);
6162

62-
return (stateRoot, result);
63+
if (result != Types.Result.Ok) return result;
64+
if (currentStateRoot != postStateRoot)
65+
return Types.Result.InvalidPostStateRoot;
66+
return result;
6367
}
6468
}

contracts/MassMigrations.sol

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,20 @@ contract MassMigration {
2323

2424
/**
2525
* @notice processes the state transition of a commitment
26-
* @param stateRoot represents the state before the state transition
26+
* @param currentStateRoot represents the state before the state transition
2727
* */
2828
function processMassMigrationCommit(
29-
bytes32 stateRoot,
29+
bytes32 currentStateRoot,
30+
bytes32 postStateRoot,
3031
uint256 maxTxSize,
3132
Types.MassMigrationBody memory committed,
3233
Types.StateMerkleProof[] memory proofs
33-
) public pure returns (bytes32, Types.Result result) {
34+
) public pure returns (Types.Result result) {
3435
if (committed.txs.massMigrationHasExcessData())
35-
return (stateRoot, Types.Result.BadCompression);
36+
return Types.Result.BadCompression;
3637

3738
uint256 size = committed.txs.massMigrationSize();
38-
if (size > maxTxSize) return (stateRoot, Types.Result.TooManyTx);
39+
if (size > maxTxSize) return Types.Result.TooManyTx;
3940

4041
Tx.MassMigration memory _tx;
4142
uint256 totalAmount = 0;
@@ -45,34 +46,38 @@ contract MassMigration {
4546

4647
for (uint256 i = 0; i < size; i++) {
4748
_tx = committed.txs.massMigrationDecode(i);
48-
(stateRoot, freshState, result) = Transition.processMassMigration(
49-
stateRoot,
49+
(currentStateRoot, freshState, result) = Transition
50+
.processMassMigration(
51+
currentStateRoot,
5052
_tx,
5153
committed.tokenID,
5254
proofs[i]
5355
);
54-
if (result != Types.Result.Ok) return (stateRoot, result);
56+
if (result != Types.Result.Ok) return result;
5557

5658
// Only trust these variables when the result is good
5759
totalAmount += _tx.amount;
5860
fees += _tx.fee;
5961
withdrawLeaves[i] = keccak256(freshState);
6062
}
61-
(stateRoot, result) = Transition.processReceiver(
62-
stateRoot,
63+
(currentStateRoot, result) = Transition.processReceiver(
64+
currentStateRoot,
6365
committed.feeReceiver,
6466
committed.tokenID,
6567
fees,
6668
proofs[size]
6769
);
68-
if (result != Types.Result.Ok) return (stateRoot, result);
70+
if (result != Types.Result.Ok) return result;
6971

7072
if (totalAmount != committed.amount)
71-
return (stateRoot, Types.Result.MismatchedAmount);
73+
return Types.Result.MismatchedAmount;
7274

7375
if (MerkleTree.merklize(withdrawLeaves) != committed.withdrawRoot)
74-
return (stateRoot, Types.Result.BadWithdrawRoot);
76+
return Types.Result.BadWithdrawRoot;
7577

76-
return (stateRoot, result);
78+
if (currentStateRoot != postStateRoot)
79+
return Types.Result.InvalidPostStateRoot;
80+
81+
return result;
7782
}
7883
}

contracts/Transfer.sol

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,17 @@ contract Transfer {
2323
* @notice processes the state transition of a commitment
2424
* */
2525
function processTransferCommit(
26-
bytes32 stateRoot,
26+
bytes32 currentStateRoot,
27+
bytes32 postStateRoot,
2728
uint256 maxTxSize,
2829
uint256 feeReceiver,
2930
bytes memory txs,
3031
Types.StateMerkleProof[] memory proofs
31-
) public pure returns (bytes32, Types.Result result) {
32-
if (txs.transferHasExcessData())
33-
return (stateRoot, Types.Result.BadCompression);
32+
) public pure returns (Types.Result result) {
33+
if (txs.transferHasExcessData()) return Types.Result.BadCompression;
3434

3535
uint256 size = txs.transferSize();
36-
if (size > maxTxSize) return (stateRoot, Types.Result.TooManyTx);
36+
if (size > maxTxSize) return Types.Result.TooManyTx;
3737

3838
uint256 fees = 0;
3939
// tokenID should be the same for all states in this commit
@@ -42,25 +42,28 @@ contract Transfer {
4242

4343
for (uint256 i = 0; i < size; i++) {
4444
_tx = txs.transferDecode(i);
45-
(stateRoot, result) = Transition.processTransfer(
46-
stateRoot,
45+
(currentStateRoot, result) = Transition.processTransfer(
46+
currentStateRoot,
4747
_tx,
4848
tokenID,
4949
proofs[i * 2],
5050
proofs[i * 2 + 1]
5151
);
52-
if (result != Types.Result.Ok) return (stateRoot, result);
52+
if (result != Types.Result.Ok) return result;
5353
// Only trust fees when the result is good
5454
fees = fees.add(_tx.fee);
5555
}
56-
(stateRoot, result) = Transition.processReceiver(
57-
stateRoot,
56+
(currentStateRoot, result) = Transition.processReceiver(
57+
currentStateRoot,
5858
feeReceiver,
5959
tokenID,
6060
fees,
6161
proofs[size * 2]
6262
);
6363

64-
return (stateRoot, result);
64+
if (result != Types.Result.Ok) return result;
65+
if (currentStateRoot != postStateRoot)
66+
return Types.Result.InvalidPostStateRoot;
67+
return result;
6568
}
6669
}

contracts/libs/Types.sol

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ library Types {
251251
BadCompression,
252252
TooManyTx,
253253
BadPrecompileCall,
254-
NonexistentReceiver
254+
NonexistentReceiver,
255+
InvalidPostStateRoot
255256
}
256257
}

contracts/rollup/Rollup.sol

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -377,19 +377,17 @@ contract Rollup is BatchManager, EIP712, IEIP712 {
377377
"Target commitment is absent in the batch"
378378
);
379379

380-
(bytes32 processedStateRoot, Types.Result result) =
380+
Types.Result result =
381381
transfer.processTransferCommit(
382382
previous.commitment.stateRoot,
383+
target.commitment.stateRoot,
383384
paramMaxTxsPerCommit,
384385
target.commitment.body.feeReceiver,
385386
target.commitment.body.txs,
386387
proofs
387388
);
388389

389-
if (
390-
result != Types.Result.Ok ||
391-
(processedStateRoot != target.commitment.stateRoot)
392-
) startRollingBack(batchID, result);
390+
if (result != Types.Result.Ok) startRollingBack(batchID, result);
393391
}
394392

395393
function disputeTransitionMassMigration(
@@ -407,18 +405,16 @@ contract Rollup is BatchManager, EIP712, IEIP712 {
407405
"Target commitment is absent in the batch"
408406
);
409407

410-
(bytes32 processedStateRoot, Types.Result result) =
408+
Types.Result result =
411409
massMigration.processMassMigrationCommit(
412410
previous.commitment.stateRoot,
411+
target.commitment.stateRoot,
413412
paramMaxTxsPerCommit,
414413
target.commitment.body,
415414
proofs
416415
);
417416

418-
if (
419-
result != Types.Result.Ok ||
420-
(processedStateRoot != target.commitment.stateRoot)
421-
) startRollingBack(batchID, result);
417+
if (result != Types.Result.Ok) startRollingBack(batchID, result);
422418
}
423419

424420
function disputeTransitionCreate2Transfer(
@@ -436,19 +432,17 @@ contract Rollup is BatchManager, EIP712, IEIP712 {
436432
"Target commitment is absent in the batch"
437433
);
438434

439-
(bytes32 processedStateRoot, Types.Result result) =
435+
Types.Result result =
440436
create2Transfer.processCreate2TransferCommit(
441437
previous.commitment.stateRoot,
438+
target.commitment.stateRoot,
442439
paramMaxTxsPerCommit,
443440
target.commitment.body.feeReceiver,
444441
target.commitment.body.txs,
445442
proofs
446443
);
447444

448-
if (
449-
result != Types.Result.Ok ||
450-
(processedStateRoot != target.commitment.stateRoot)
451-
) startRollingBack(batchID, result);
445+
if (result != Types.Result.Ok) startRollingBack(batchID, result);
452446
}
453447

454448
function disputeSignatureTransfer(

contracts/test/TestCreate2Transfer.sol

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,21 +47,23 @@ contract TestCreate2Transfer is Create2Transfer {
4747
}
4848

4949
function testProcessCreate2TransferCommit(
50-
bytes32 stateRoot,
50+
bytes32 currentStateRoot,
51+
bytes32 postStateRoot,
5152
uint256 maxTxSize,
5253
uint256 feeReceiver,
5354
bytes memory txs,
5455
Types.StateMerkleProof[] memory proofs
55-
) public returns (bytes32, uint256) {
56-
bytes32 newRoot;
57-
uint256 operationCost = gasleft();
58-
(newRoot, ) = processCreate2TransferCommit(
59-
stateRoot,
60-
maxTxSize,
61-
feeReceiver,
62-
txs,
63-
proofs
64-
);
65-
return (newRoot, operationCost - gasleft());
56+
) public returns (uint256 gasCost, Types.Result) {
57+
gasCost = gasleft();
58+
Types.Result result =
59+
processCreate2TransferCommit(
60+
currentStateRoot,
61+
postStateRoot,
62+
maxTxSize,
63+
feeReceiver,
64+
txs,
65+
proofs
66+
);
67+
return (gasCost - gasleft(), result);
6668
}
6769
}

contracts/test/TestMassMigration.sol

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,27 +29,21 @@ contract TestMassMigration is MassMigration {
2929
}
3030

3131
function testProcessMassMigrationCommit(
32-
bytes32 stateRoot,
32+
bytes32 currentStateRoot,
33+
bytes32 postStateRoot,
3334
uint256 maxTxSize,
3435
Types.MassMigrationBody memory commitmentBody,
3536
Types.StateMerkleProof[] memory proofs
36-
)
37-
public
38-
view
39-
returns (
40-
uint256 gasCost,
41-
bytes32,
42-
Types.Result
43-
)
44-
{
37+
) public view returns (uint256 gasCost, Types.Result) {
4538
gasCost = gasleft();
46-
(bytes32 postRoot, Types.Result result) =
39+
Types.Result result =
4740
processMassMigrationCommit(
48-
stateRoot,
41+
currentStateRoot,
42+
postStateRoot,
4943
maxTxSize,
5044
commitmentBody,
5145
proofs
5246
);
53-
return (gasCost - gasleft(), postRoot, result);
47+
return (gasCost - gasleft(), result);
5448
}
5549
}

contracts/test/TestTransfer.sol

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,23 @@ contract TestTransfer is Transfer {
4040
}
4141

4242
function testProcessTransferCommit(
43-
bytes32 stateRoot,
43+
bytes32 currentStateRoot,
44+
bytes32 postStateRoot,
4445
uint256 maxTxSize,
4546
uint256 feeReceiver,
4647
bytes memory txs,
4748
Types.StateMerkleProof[] memory proofs
48-
) public returns (bytes32, uint256) {
49-
bytes32 newRoot;
50-
uint256 operationCost = gasleft();
51-
(newRoot, ) = processTransferCommit(
52-
stateRoot,
53-
maxTxSize,
54-
feeReceiver,
55-
txs,
56-
proofs
57-
);
58-
return (newRoot, operationCost - gasleft());
49+
) public returns (uint256 gasCost, Types.Result) {
50+
gasCost = gasleft();
51+
Types.Result result =
52+
processTransferCommit(
53+
currentStateRoot,
54+
postStateRoot,
55+
maxTxSize,
56+
feeReceiver,
57+
txs,
58+
proofs
59+
);
60+
return (gasCost - gasleft(), result);
5961
}
6062
}

0 commit comments

Comments
 (0)