From 75c755413f250afbdfb5e3b8f2d69ebc33f0cb4c Mon Sep 17 00:00:00 2001 From: billxu Date: Mon, 29 Jul 2024 12:46:19 +0800 Subject: [PATCH 01/11] fix the unit test of addLocalData --- .../src/dispute/FaultDisputeGameN.sol | 1 + .../test/dispute/FaultDisputeGameN.t.sol | 124 ++++++++++++------ 2 files changed, 84 insertions(+), 41 deletions(-) diff --git a/packages/contracts-bedrock/src/dispute/FaultDisputeGameN.sol b/packages/contracts-bedrock/src/dispute/FaultDisputeGameN.sol index 6d21a1a4ad6c..e09034f9818e 100644 --- a/packages/contracts-bedrock/src/dispute/FaultDisputeGameN.sol +++ b/packages/contracts-bedrock/src/dispute/FaultDisputeGameN.sol @@ -489,6 +489,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, ISemver { /// @inheritdoc IFaultDisputeGame function addLocalData(uint256 _ident, uint256 _execLeafIdx, uint256 _partOffset) external { + revert NotSupported(); } function addLocalData(uint256 _ident, uint256 _execLeafIdx, uint256 _partOffset, LibDA.DAItem memory _daItem) external returns (Hash uuid_, bytes32 value_) { diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index 33a8691c6dbe..6d8b813533f5 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -2035,20 +2035,26 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { function testFuzz_addLocalData_oob_reverts(uint256 _ident) public { Claim disputed; // Get a claim below the split depth so that we can add local data for an execution trace subgame. - for (uint256 i; i < 4; i++) { - uint256 bond = _getRequiredBond(i); + for (uint256 i; i < 2; i++) { + uint256 bond = _getRequiredBondV2(i, 0); (,,,, disputed,,) = gameProxy.claimData(i); - gameProxy.attack{ value: bond }(disputed, i, _dummyClaim()); + gameProxy.attackV2{ value: bond }(disputed, i, _dummyClaim(), 0); } - uint256 lastBond = _getRequiredBond(4); - (,,,, disputed,,) = gameProxy.claimData(4); - gameProxy.attack{ value: lastBond }(disputed, 4, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC)); + uint256 lastBond = _getRequiredBondV2(2, 0); + (,,,, disputed,,) = gameProxy.claimData(2); + gameProxy.attackV2{ value: lastBond }(disputed, 2, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC), 0); // [1, 5] are valid local data identifiers. if (_ident <= 5) _ident = 0; + // This variable is not used + LibDA.DAItem memory localDataItem = LibDA.DAItem({ + daType: LibDA.DA_TYPE_CALLDATA, + dataHash: '00000000000000000000000000000000', + proof: hex"" + }); vm.expectRevert(InvalidLocalIdent.selector); - gameProxy.addLocalData(_ident, 5, 0); + gameProxy.addLocalData(_ident, 3, 0, localDataItem); } /// @dev Tests that local data is loaded into the preimage oracle correctly in the subgame @@ -2056,37 +2062,52 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { function test_addLocalDataGenesisTransition_static_succeeds() public { IPreimageOracle oracle = IPreimageOracle(address(gameProxy.vm().oracle())); Claim disputed; + Claim[] memory claims = generateClaims(3); + Claim disputedClaim = Claim.wrap(LibDA.getClaimsHash(LibDA.DA_TYPE_CALLDATA, 3, abi.encodePacked(claims[0], claims[1], claims[2]))); - // Get a claim below the split depth so that we can add local data for an execution trace subgame. - for (uint256 i; i < 4; i++) { - uint256 bond = _getRequiredBond(i); - (,,,, disputed,,) = gameProxy.claimData(i); - gameProxy.attack{ value: bond }(disputed, i, Claim.wrap(bytes32(i))); - } - uint256 lastBond = _getRequiredBond(4); - (,,,, disputed,,) = gameProxy.claimData(4); - gameProxy.attack{ value: lastBond }(disputed, 4, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC)); + (,,,, disputed,,) = gameProxy.claimData(0); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); + + (,,,, disputed,,) = gameProxy.claimData(1); + gameProxy.attackV2{ value: _getRequiredBondV2(1, 0) }(disputed, 1, disputedClaim, 0); + + (,,,, disputed,,) = gameProxy.claimData(2); + gameProxy.attackV2{ value: _getRequiredBondV2(2, 0) }(disputed, 2, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC), 0); + + LibDA.DAItem memory startingDataItem = LibDA.DAItem({ + daType: LibDA.DA_TYPE_CALLDATA, + dataHash: '00000000000000000000000000000000', + proof: hex"" + }); + LibDA.DAItem memory disputedDataItem = LibDA.DAItem({ + daType: LibDA.DA_TYPE_CALLDATA, + dataHash: claims[0].raw(), + proof: abi.encodePacked(claims[1], claims[2]) + }); // Expected start/disputed claims (Hash root,) = gameProxy.startingOutputRoot(); bytes32 startingClaim = root.raw(); - bytes32 disputedClaim = bytes32(uint256(3)); Position disputedPos = LibPosition.wrap(4, 0); // Expected local data bytes32[5] memory data = [ gameProxy.l1Head().raw(), startingClaim, - disputedClaim, + disputedDataItem.dataHash, bytes32(uint256(1) << 0xC0), bytes32(gameProxy.l2ChainId() << 0xC0) ]; for (uint256 i = 1; i <= 5; i++) { uint256 expectedLen = i > 3 ? 8 : 32; - bytes32 key = _getKey(i, keccak256(abi.encode(disputedClaim, disputedPos))); + bytes32 key = _getKey(i, keccak256(abi.encode(disputedClaim.raw(), disputedPos))); - gameProxy.addLocalData(i, 5, 0); + if (LocalPreimageKey.DISPUTED_OUTPUT_ROOT == i) { + gameProxy.addLocalData(i, 3, 0, disputedDataItem); + } else { + gameProxy.addLocalData(i, 3, 0, startingDataItem); + } (bytes32 dat, uint256 datLen) = oracle.readPreimage(key, 0); assertEq(dat >> 0xC0, bytes32(expectedLen)); // Account for the length prefix if i > 3 (the data stored @@ -2096,7 +2117,11 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // total.) assertEq(datLen, expectedLen + (i > 3 ? 8 : 0)); - gameProxy.addLocalData(i, 5, 8); + if (LocalPreimageKey.DISPUTED_OUTPUT_ROOT == i) { + gameProxy.addLocalData(i, 3, 8, disputedDataItem); + } else { + gameProxy.addLocalData(i, 3, 8, startingDataItem); + } (dat, datLen) = oracle.readPreimage(key, 8); assertEq(dat, data[i - 1]); assertEq(datLen, expectedLen); @@ -2106,38 +2131,51 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { /// @dev Tests that local data is loaded into the preimage oracle correctly. function test_addLocalDataMiddle_static_succeeds() public { IPreimageOracle oracle = IPreimageOracle(address(gameProxy.vm().oracle())); - Claim disputed; + Claim[] memory claims = generateClaims(3); + Claim root = Claim.wrap(LibDA.getClaimsHash(LibDA.DA_TYPE_CALLDATA, 3, abi.encodePacked(claims[0], claims[1], claims[2]))); - // Get a claim below the split depth so that we can add local data for an execution trace subgame. - for (uint256 i; i < 4; i++) { - uint256 bond = _getRequiredBond(i); - (,,,, disputed,,) = gameProxy.claimData(i); - gameProxy.attack{ value: bond }(disputed, i, Claim.wrap(bytes32(i))); - } - uint256 lastBond = _getRequiredBond(4); - (,,,, disputed,,) = gameProxy.claimData(4); - gameProxy.defend{ value: lastBond }(disputed, 4, _changeClaimStatus(ROOT_CLAIM, VMStatuses.VALID)); + (,,,, Claim disputed,,) = gameProxy.claimData(0); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, root, 0); + + (,,,, disputed,,) = gameProxy.claimData(1); + gameProxy.attackV2{ value: _getRequiredBondV2(1, 0) }(disputed, 1, root, 0); + + (,,,, disputed,,) = gameProxy.claimData(2); + gameProxy.attackV2{ value: _getRequiredBondV2(2, 3) }(disputed, 2, _changeClaimStatus(ROOT_CLAIM, VMStatuses.VALID), 3); + + LibDA.DAItem memory startingDataItem = LibDA.DAItem({ + daType: LibDA.DA_TYPE_CALLDATA, + dataHash: claims[2].raw(), + proof: bytes.concat(keccak256(abi.encode(claims[0].raw(), claims[1].raw()))) + }); + LibDA.DAItem memory disputedDataItem = LibDA.DAItem({ + daType: LibDA.DA_TYPE_CALLDATA, + dataHash: claims[0].raw(), + proof: abi.encodePacked(claims[1], claims[2]) + }); // Expected start/disputed claims - bytes32 startingClaim = bytes32(uint256(3)); - Position startingPos = LibPosition.wrap(4, 0); - bytes32 disputedClaim = bytes32(uint256(2)); - Position disputedPos = LibPosition.wrap(3, 0); + Position startingPos = LibPosition.wrap(4, 2); + Position disputedPos = LibPosition.wrap(2, 0); // Expected local data bytes32[5] memory data = [ gameProxy.l1Head().raw(), - startingClaim, - disputedClaim, - bytes32(uint256(2) << 0xC0), + startingDataItem.dataHash, + disputedDataItem.dataHash, + bytes32(uint256(4) << 0xC0), bytes32(gameProxy.l2ChainId() << 0xC0) ]; for (uint256 i = 1; i <= 5; i++) { uint256 expectedLen = i > 3 ? 8 : 32; - bytes32 key = _getKey(i, keccak256(abi.encode(startingClaim, startingPos, disputedClaim, disputedPos))); + bytes32 key = _getKey(i, keccak256(abi.encode(root.raw(), startingPos, root.raw(), disputedPos))); - gameProxy.addLocalData(i, 5, 0); + if (LocalPreimageKey.DISPUTED_OUTPUT_ROOT == i) { + gameProxy.addLocalData(i, 3, 0, disputedDataItem); + } else { + gameProxy.addLocalData(i, 3, 0, startingDataItem); + } (bytes32 dat, uint256 datLen) = oracle.readPreimage(key, 0); assertEq(dat >> 0xC0, bytes32(expectedLen)); // Account for the length prefix if i > 3 (the data stored @@ -2147,7 +2185,11 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // total.) assertEq(datLen, expectedLen + (i > 3 ? 8 : 0)); - gameProxy.addLocalData(i, 5, 8); + if (LocalPreimageKey.DISPUTED_OUTPUT_ROOT == i) { + gameProxy.addLocalData(i, 3, 8, disputedDataItem); + } else { + gameProxy.addLocalData(i, 3, 8, startingDataItem); + } (dat, datLen) = oracle.readPreimage(key, 8); assertEq(dat, data[i - 1]); assertEq(datLen, expectedLen); From 9815ee68109e2cc029aa463497294c399111ff58 Mon Sep 17 00:00:00 2001 From: billxu Date: Mon, 29 Jul 2024 21:13:05 +0800 Subject: [PATCH 02/11] fix the unit test of move --- .../test/dispute/FaultDisputeGameN.t.sol | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index 116c32da40f2..3074d141b267 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -27,6 +27,10 @@ contract FaultDisputeGame_Init is DisputeGameFactory_Init { /// @dev The type of the game being tested. GameType internal constant GAME_TYPE = GameType.wrap(0); + uint256 internal constant N_BITS = 2; + + uint256 internal constant MAX_ATTACK_BRANCH = (1 << N_BITS) - 1; + /// @dev The implementation of the game. FaultDisputeGame internal gameImpl; /// @dev The `Clone` proxy of the game. @@ -105,7 +109,7 @@ contract FaultDisputeGame_Init is DisputeGameFactory_Init { return Claim.wrap(keccak256(abi.encode(gasleft()))); } - function generateClaims(uint256 total) public view returns (Claim[] memory) { + function generateClaims(uint256 total) public pure returns (Claim[] memory) { Claim[] memory newClaims = new Claim[](total); for (uint256 i = 0; i < total; i++) { bytes memory claimData = abi.encode(i + 1, i + 1); @@ -391,14 +395,14 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { (,,,, Claim root,,) = gameProxy.claimData(0); // Attempt to make a move. Should revert. vm.expectRevert(GameNotInProgress.selector); - gameProxy.attack(root, 0, Claim.wrap(0)); + gameProxy.attackV2(root, 0, Claim.wrap(0), 0); } /// @dev Tests that an attempt to defend the root claim reverts with the `CannotDefendRootClaim` error. function test_move_defendRoot_reverts() public { (,,,, Claim root,,) = gameProxy.claimData(0); vm.expectRevert(CannotDefendRootClaim.selector); - gameProxy.defend(root, 0, _dummyClaim()); + gameProxy.attackV2(root, 0, _dummyClaim(), uint64(MAX_ATTACK_BRANCH)); } /// @dev Tests that an attempt to move against a claim that does not exist reverts with the @@ -408,11 +412,11 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // Expect an out of bounds revert for an attack vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x32)); - gameProxy.attack(_dummyClaim(), 1, claim); + gameProxy.attackV2(_dummyClaim(), 1, claim, 0); // Expect an out of bounds revert for a defense vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x32)); - gameProxy.defend(_dummyClaim(), 1, claim); + gameProxy.attackV2(_dummyClaim(), 1, claim, uint64(MAX_ATTACK_BRANCH)); } /// @dev Tests that an attempt to move at the maximum game depth reverts with the @@ -422,15 +426,15 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { uint256 maxDepth = gameProxy.maxGameDepth(); - for (uint256 i = 0; i <= maxDepth; i++) { + for (uint256 i = 0; i * N_BITS <= maxDepth; i++) { (,,,, Claim disputed,,) = gameProxy.claimData(i); // At the max game depth, the `_move` function should revert with // the `GameDepthExceeded` error. - if (i == maxDepth) { + if (i * N_BITS == maxDepth) { vm.expectRevert(GameDepthExceeded.selector); - gameProxy.attack{ value: 100 ether }(disputed, i, claim); + gameProxy.attackV2{ value: 100 ether }(disputed, i, claim, 0); } else { - gameProxy.attack{ value: _getRequiredBond(i) }(disputed, i, claim); + gameProxy.attackV2{ value: _getRequiredBondV2(i, 0) }(disputed, i, claim, 0); } } } @@ -513,13 +517,13 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { Claim claim = _dummyClaim(); // Make the first move. This should succeed. - uint256 bond = _getRequiredBond(0); + uint256 bond = _getRequiredBondV2(0, 0); (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: bond }(disputed, 0, claim); + gameProxy.attackV2{ value: bond }(disputed, 0, claim, 0); // Attempt to make the same move again. vm.expectRevert(ClaimAlreadyExists.selector); - gameProxy.attack{ value: bond }(disputed, 0, claim); + gameProxy.attackV2{ value: bond }(disputed, 0, claim, 0); } /// @dev Static unit test asserting that identical claims at the same position can be made in different subgames. @@ -528,19 +532,19 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { Claim claimB = _dummyClaim(); // Make the first moves. This should succeed. - uint256 bond = _getRequiredBond(0); + uint256 bond = _getRequiredBondV2(0, 0); (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: bond }(disputed, 0, claimA); - gameProxy.attack{ value: bond }(disputed, 0, claimB); + gameProxy.attackV2{ value: bond }(disputed, 0, claimA, 0); + gameProxy.attackV2{ value: bond }(disputed, 0, claimB, 0); // Perform an attack at the same position with the same claim value in both subgames. // These both should succeed. - bond = _getRequiredBond(1); + bond = _getRequiredBondV2(1, 0); (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.attack{ value: bond }(disputed, 1, claimA); - bond = _getRequiredBond(2); + gameProxy.attackV2{ value: bond }(disputed, 1, claimA, 0); + bond = _getRequiredBondV2(2, 0); (,,,, disputed,,) = gameProxy.claimData(2); - gameProxy.attack{ value: bond }(disputed, 2, claimA); + gameProxy.attackV2{ value: bond }(disputed, 2, claimA, 0); } /// @dev Static unit test for the correctness of an opening attack. @@ -551,11 +555,11 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { Claim counter = _dummyClaim(); // Perform the attack. - uint256 reqBond = _getRequiredBond(0); + uint256 reqBond = _getRequiredBondV2(0, 0); vm.expectEmit(true, true, true, false); emit Move(0, counter, address(this)); (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: reqBond }(disputed, 0, counter); + gameProxy.attackV2{ value: reqBond }(disputed, 0, counter, 0); // Grab the claim data of the attack. ( @@ -574,7 +578,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { assertEq(claimant, address(this)); assertEq(bond, reqBond); assertEq(claim.raw(), counter.raw()); - assertEq(position.raw(), Position.wrap(1).move(true).raw()); + assertEq(position.raw(), Position.wrap(1).moveN(N_BITS, 0).raw()); assertEq(clock.raw(), LibClock.wrap(Duration.wrap(5), Timestamp.wrap(uint64(block.timestamp))).raw()); // Grab the claim data of the parent. @@ -630,7 +634,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { } uint256 lastBond = _getRequiredBondV2(2, 0); (,,,, disputed,,) = gameProxy.claimData(2); - gameProxy.attackV2{ value: lastBond }(disputed, 2, Claim.wrap(bytes32(0)), 3); + gameProxy.attackV2{ value: lastBond }(disputed, 2, Claim.wrap(bytes32(0)), uint64(MAX_ATTACK_BRANCH)); } /// @dev Static unit test asserting that a move reverts when the bonded amount is incorrect. @@ -643,10 +647,10 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { /// @dev Static unit test asserting that a move reverts when the disputed claim does not match its index. function test_move_incorrectDisputedIndex_reverts() public { (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); - uint256 bond = _getRequiredBond(1); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); + uint256 bond = _getRequiredBondV2(1, 0); vm.expectRevert(InvalidDisputedClaimIndex.selector); - gameProxy.attack{ value: bond }(disputed, 1, _dummyClaim()); + gameProxy.attackV2{ value: bond }(disputed, 1, _dummyClaim(), 0); } /// @dev Tests that challenging the root claim's L2 block number by providing the real preimage of the output root From 012467475848bd69de2527ffa8aafec5c55a63bf Mon Sep 17 00:00:00 2001 From: billxu Date: Wed, 31 Jul 2024 09:17:32 +0800 Subject: [PATCH 03/11] fix the unit test of resolve --- .../test/dispute/FaultDisputeGameN.t.sol | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index 3074d141b267..267ba82d2918 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -1450,10 +1450,10 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { function test_resolve_multiPart_succeeds() public { vm.deal(address(this), 10_000 ether); - uint256 bond = _getRequiredBond(0); + uint256 bond = _getRequiredBondV2(0, 0); for (uint256 i = 0; i < 2048; i++) { (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: bond }(disputed, 0, Claim.wrap(bytes32(i))); + gameProxy.attackV2{ value: bond }(disputed, 0, Claim.wrap(bytes32(i)), 0); } // Warp past the clock period. @@ -1472,7 +1472,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { gameProxy.resolutionCheckpoints(0); assertTrue(initCheckpoint); assertEq(subgameIndex, 1024); - assertEq(leftmostPosition.raw(), Position.wrap(1).move(true).raw()); + assertEq(leftmostPosition.raw(), Position.wrap(1).moveN(N_BITS, 0).raw()); assertEq(counteredBy, address(this)); // The root subgame should not be resolved. @@ -1489,7 +1489,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { (initCheckpoint, subgameIndex, leftmostPosition, counteredBy) = gameProxy.resolutionCheckpoints(0); assertTrue(initCheckpoint); assertEq(subgameIndex, 2048); - assertEq(leftmostPosition.raw(), Position.wrap(1).move(true).raw()); + assertEq(leftmostPosition.raw(), Position.wrap(1).moveN(N_BITS, 0).raw()); assertEq(counteredBy, address(this)); // The root subgame should now be resolved @@ -1525,7 +1525,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { /// @dev Static unit test for the correctness of resolving a single attack game state. function test_resolve_rootContested_succeeds() public { (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); vm.warp(block.timestamp + 3 days + 12 hours); @@ -1537,9 +1537,9 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { /// @dev Static unit test for the correctness of resolving a game with a contested challenge claim. function test_resolve_challengeContested_succeeds() public { (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.defend{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(1, 0) }(disputed, 1, _dummyClaim(), uint64(MAX_ATTACK_BRANCH)); vm.warp(block.timestamp + 3 days + 12 hours); @@ -1552,11 +1552,11 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { /// @dev Static unit test for the correctness of resolving a game with multiplayer moves. function test_resolve_teamDeathmatch_succeeds() public { (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.defend{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); - gameProxy.defend{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(1, uint64(MAX_ATTACK_BRANCH)) }(disputed, 1, _dummyClaim(), uint64(MAX_ATTACK_BRANCH)); + gameProxy.attackV2{ value: _getRequiredBondV2(1, uint64(MAX_ATTACK_BRANCH)) }(disputed, 1, _dummyClaim(), uint64(MAX_ATTACK_BRANCH)); vm.warp(block.timestamp + 3 days + 12 hours); @@ -1571,20 +1571,20 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { /// @dev Static unit test for the correctness of resolving a game that reaches max game depth. function test_resolve_stepReached_succeeds() public { Claim claim = _dummyClaim(); - for (uint256 i; i < gameProxy.splitDepth(); i++) { + for (uint256 i; i * N_BITS < gameProxy.splitDepth(); i++) { (,,,, Claim disputed,,) = gameProxy.claimData(i); - gameProxy.attack{ value: _getRequiredBond(i) }(disputed, i, claim); + gameProxy.attackV2{ value: _getRequiredBondV2(i, 0) }(disputed, i, claim, 0); } claim = _changeClaimStatus(claim, VMStatuses.PANIC); - for (uint256 i = gameProxy.claimDataLen() - 1; i < gameProxy.maxGameDepth(); i++) { + for (uint256 i = gameProxy.claimDataLen() - 1; i * N_BITS < gameProxy.maxGameDepth(); i++) { (,,,, Claim disputed,,) = gameProxy.claimData(i); - gameProxy.attack{ value: _getRequiredBond(i) }(disputed, i, claim); + gameProxy.attackV2{ value: _getRequiredBondV2(i, 0) }(disputed, i, claim, 0); } vm.warp(block.timestamp + 3 days + 12 hours); - for (uint256 i = 9; i > 0; i--) { + for (uint256 i = 5; i > 0; i--) { gameProxy.resolveClaim(i - 1, 0); } assertEq(uint8(gameProxy.resolve()), uint8(GameStatus.DEFENDER_WINS)); @@ -1593,14 +1593,14 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { /// @dev Static unit test asserting that resolve reverts when attempting to resolve a subgame multiple times function test_resolve_claimAlreadyResolved_reverts() public { Claim claim = _dummyClaim(); - uint256 firstBond = _getRequiredBond(0); + uint256 firstBond = _getRequiredBondV2(0, 0); vm.deal(address(this), firstBond); (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: firstBond }(disputed, 0, claim); - uint256 secondBond = _getRequiredBond(1); + gameProxy.attackV2{ value: firstBond }(disputed, 0, claim, 0); + uint256 secondBond = _getRequiredBondV2(1, 0); vm.deal(address(this), secondBond); (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.attack{ value: secondBond }(disputed, 1, claim); + gameProxy.attackV2{ value: secondBond }(disputed, 1, claim, 0); vm.warp(block.timestamp + 3 days + 12 hours); @@ -1622,40 +1622,40 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { /// @dev Static unit test asserting that resolve reverts when attempting to resolve a subgame at max depth function test_resolve_claimAtMaxDepthAlreadyResolved_reverts() public { Claim claim = _dummyClaim(); - for (uint256 i; i < gameProxy.splitDepth(); i++) { + for (uint256 i; i * N_BITS < gameProxy.splitDepth(); i++) { (,,,, Claim disputed,,) = gameProxy.claimData(i); - gameProxy.attack{ value: _getRequiredBond(i) }(disputed, i, claim); + gameProxy.attackV2{ value: _getRequiredBondV2(i, 0) }(disputed, i, claim, 0); } vm.deal(address(this), 10000 ether); claim = _changeClaimStatus(claim, VMStatuses.PANIC); - for (uint256 i = gameProxy.claimDataLen() - 1; i < gameProxy.maxGameDepth(); i++) { + for (uint256 i = gameProxy.claimDataLen() - 1; i * N_BITS < gameProxy.maxGameDepth(); i++) { (,,,, Claim disputed,,) = gameProxy.claimData(i); - gameProxy.attack{ value: _getRequiredBond(i) }(disputed, i, claim); + gameProxy.attackV2{ value: _getRequiredBondV2(i, 0) }(disputed, i, claim, 0); } vm.warp(block.timestamp + 3 days + 12 hours); // Resolve to claim bond uint256 balanceBefore = address(this).balance; - gameProxy.resolveClaim(8, 0); + gameProxy.resolveClaim(4, 0); // Wait for the withdrawal delay. vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); gameProxy.claimCredit(address(this)); - assertEq(address(this).balance, balanceBefore + _getRequiredBond(7)); + assertEq(address(this).balance, balanceBefore + _getRequiredBondV2(3, 0)); vm.expectRevert(ClaimAlreadyResolved.selector); - gameProxy.resolveClaim(8, 0); + gameProxy.resolveClaim(4, 0); } /// @dev Static unit test asserting that resolve reverts when attempting to resolve subgames out of order function test_resolve_outOfOrderResolution_reverts() public { (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.attack{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(1, 0) }(disputed, 1, _dummyClaim(), 0); vm.warp(block.timestamp + 3 days + 12 hours); @@ -1965,7 +1965,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // Challenge the claim and resolve it. (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); vm.warp(block.timestamp + 3 days + 12 hours); gameProxy.resolveClaim(1, 0); gameProxy.resolveClaim(0, 0); From d021cec111015dd8c972c38b0d5bf4efeecd2720 Mon Sep 17 00:00:00 2001 From: billxu Date: Thu, 1 Aug 2024 19:01:38 +0800 Subject: [PATCH 04/11] fix the unit test of resolve, add test_resolution_lastSecondDisputes_succeeds --- .../test/dispute/FaultDisputeGameN.t.sol | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index 267ba82d2918..98ad180ba50d 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -2207,7 +2207,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // Defender's turn vm.warp(block.timestamp + 3.5 days - 1 seconds); (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); // Chess clock time accumulated: assertEq(gameProxy.getChallengerDuration(0).raw(), 3.5 days - 1 seconds); assertEq(gameProxy.getChallengerDuration(1).raw(), 0); @@ -2215,9 +2215,9 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // Advance time by 1 second, so that the root claim challenger clock is expired. vm.warp(block.timestamp + 1 seconds); // Attempt a second attack against the root claim. This should revert since the challenger clock is expired. - uint256 expectedBond = _getRequiredBond(0); + uint256 expectedBond = _getRequiredBondV2(0, 0); vm.expectRevert(ClockTimeExceeded.selector); - gameProxy.attack{ value: expectedBond }(disputed, 0, _dummyClaim()); + gameProxy.attackV2{ value: expectedBond }(disputed, 0, _dummyClaim(), 0); // Chess clock time accumulated: assertEq(gameProxy.getChallengerDuration(0).raw(), 3.5 days); assertEq(gameProxy.getChallengerDuration(1).raw(), 1 seconds); @@ -2232,11 +2232,12 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { vm.warp(block.timestamp + 3.5 days - 2 seconds); // Attack the challenge to the root claim. This should succeed, since the defender clock is not expired. (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.attack{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); + gameProxy.attackV2{ value: _getRequiredBondV2(1, 0) }(disputed, 1, _dummyClaim(), 0); // Chess clock time accumulated: assertEq(gameProxy.getChallengerDuration(0).raw(), 3.5 days); assertEq(gameProxy.getChallengerDuration(1).raw(), 3.5 days - 1 seconds); - assertEq(gameProxy.getChallengerDuration(2).raw(), 3.5 days - gameProxy.clockExtension().raw()); + // todo, bill modify :2024-07-31 09:43:35 + assertEq(gameProxy.getChallengerDuration(2).raw(), 3.5 days - (gameProxy.clockExtension().raw() * 2)); // Should not be able to resolve any claims yet. vm.expectRevert(ClockNotExpired.selector); @@ -2259,17 +2260,17 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // Chess clock time accumulated: assertEq(gameProxy.getChallengerDuration(0).raw(), 3.5 days); assertEq(gameProxy.getChallengerDuration(1).raw(), 3.5 days); - assertEq(gameProxy.getChallengerDuration(2).raw(), 3.5 days - 1 seconds); + assertEq(gameProxy.getChallengerDuration(2).raw(), 3.5 days - 1 seconds - gameProxy.clockExtension().raw()); // Warp past the challenge period for the root claim defender. Defending the root claim should now revert. - vm.warp(block.timestamp + 1 seconds); - expectedBond = _getRequiredBond(1); + vm.warp(block.timestamp + 1 seconds + gameProxy.clockExtension().raw()); + expectedBond = _getRequiredBondV2(1, 0); vm.expectRevert(ClockTimeExceeded.selector); // no further move can be made - gameProxy.attack{ value: expectedBond }(disputed, 1, _dummyClaim()); - expectedBond = _getRequiredBond(2); + gameProxy.attackV2{ value: expectedBond }(disputed, 1, _dummyClaim(), 0); + expectedBond = _getRequiredBondV2(2, 0); (,,,, disputed,,) = gameProxy.claimData(2); vm.expectRevert(ClockTimeExceeded.selector); // no further move can be made - gameProxy.attack{ value: expectedBond }(disputed, 2, _dummyClaim()); + gameProxy.attackV2{ value: expectedBond }(disputed, 2, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC), 0); // Chess clock time accumulated: assertEq(gameProxy.getChallengerDuration(0).raw(), 3.5 days); assertEq(gameProxy.getChallengerDuration(1).raw(), 3.5 days); From 917a290b3dc72b747f78c3e1887ca5c4c0a4fa3d Mon Sep 17 00:00:00 2001 From: billxu Date: Sun, 4 Aug 2024 03:16:46 +0800 Subject: [PATCH 05/11] fix the unit test of stepAttackDummyClaim_attackBranch.* --- .../contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index 98ad180ba50d..257b9a5b447f 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -952,7 +952,6 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { (,,,, disputed,,) = gameProxy.claimData(3); gameProxy.attackV2{ value: _getRequiredBondV2(3, 0) }(disputed, 3, root, 0); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 1); LibDA.DAItem memory preStateItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA, @@ -970,6 +969,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { vmProof: hex"" }); + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 1, preStateItem); gameProxy.stepV2({_claimIndex: 4, _attackBranch: 1, _stateData: claimData1, _proof: stepProof}); } @@ -1000,7 +1000,6 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { (,,,, disputed,,) = gameProxy.claimData(3); gameProxy.attackV2{ value: _getRequiredBondV2(3, 0) }(disputed, 3, root, 0); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 2); LibDA.DAItem memory preStateItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA, @@ -1018,6 +1017,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { vmProof: hex"" }); + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 2, preStateItem); gameProxy.stepV2({_claimIndex: 4, _attackBranch: 2, _stateData: claimData2, _proof: stepProof}); } @@ -1048,7 +1048,6 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { (,,,, disputed,,) = gameProxy.claimData(3); gameProxy.attackV2{ value: _getRequiredBondV2(3, 0) }(disputed, 3, root, 0); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 3); LibDA.DAItem memory preStateItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA, @@ -1067,6 +1066,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { vmProof: hex"" }); + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 3, preStateItem); vm.expectRevert(ValidStep.selector); gameProxy.stepV2({_claimIndex: 4, _attackBranch: 3, _stateData: claimData3, _proof: stepProof}); } From b167664c4eaa189ad91286bb746aabd3284350dd Mon Sep 17 00:00:00 2001 From: billxu Date: Sun, 4 Aug 2024 14:23:49 +0800 Subject: [PATCH 06/11] fix the unit test of FaultDisputeGameN_LessSplitDepth_Test --- .../contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index 257b9a5b447f..8c133154865c 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -2392,7 +2392,6 @@ contract FaultDisputeGameN_LessSplitDepth_Test is FaultDisputeGame_Init { (,,,, disputed,,) = gameProxy.claimData(3); gameProxy.attackV2{ value: _getRequiredBondV2(3, 0) }(disputed, 3, mid, 2); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 3); LibDA.DAItem memory preStateItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA, @@ -2403,6 +2402,7 @@ contract FaultDisputeGameN_LessSplitDepth_Test is FaultDisputeGame_Init { FaultDisputeGame.StepProof memory stepProof = FaultDisputeGame.StepProof({ preStateItem: preStateItem, postStateItem: postStateItem, vmProof: hex"" }); + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 3, preStateItem); gameProxy.stepV2({ _claimIndex: 4, _attackBranch: 3, _stateData: claimData6, _proof: stepProof }); } @@ -2447,7 +2447,6 @@ contract FaultDisputeGameN_LessSplitDepth_Test is FaultDisputeGame_Init { (,,,, disputed,,) = gameProxy.claimData(3); gameProxy.attackV2{ value: _getRequiredBondV2(3, 0) }(disputed, 3, mid, 2); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 3); LibDA.DAItem memory preStateItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA, @@ -2458,6 +2457,7 @@ contract FaultDisputeGameN_LessSplitDepth_Test is FaultDisputeGame_Init { FaultDisputeGame.StepProof memory stepProof = FaultDisputeGame.StepProof({ preStateItem: preStateItem, postStateItem: postStateItem, vmProof: hex"" }); + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 3, preStateItem); vm.expectRevert(ValidStep.selector); gameProxy.stepV2({ _claimIndex: 4, _attackBranch: 3, _stateData: claimData6, _proof: stepProof }); } @@ -2499,7 +2499,6 @@ contract FaultDisputeGameN_LessSplitDepth_Test is FaultDisputeGame_Init { (,,,, disputed,,) = gameProxy.claimData(3); gameProxy.attackV2{ value: _getRequiredBondV2(3, 0) }(disputed, 3, mid, 3); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 3); LibDA.DAItem memory preStateItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA, @@ -2515,6 +2514,7 @@ contract FaultDisputeGameN_LessSplitDepth_Test is FaultDisputeGame_Init { FaultDisputeGame.StepProof memory stepProof = FaultDisputeGame.StepProof({ preStateItem: preStateItem, postStateItem: postStateItem, vmProof: hex"" }); + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 3, preStateItem); gameProxy.stepV2({ _claimIndex: 4, _attackBranch: 3, _stateData: claimData6, _proof: stepProof }); } From 9f0b4e6d6b2dace4a88a2fdf22593076073f8f6b Mon Sep 17 00:00:00 2001 From: billxu Date: Sun, 4 Aug 2024 16:15:13 +0800 Subject: [PATCH 07/11] fix the unit test of remove edundant unit tests --- .../test/dispute/FaultDisputeGameN.t.sol | 130 ------------------ 1 file changed, 130 deletions(-) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index 8c133154865c..894399053b38 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -841,35 +841,6 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // gameProxy.stepV2(4, 0, absolutePrestateData, hex""); // } - /// @dev Tests that successfully step with true attacking claim when there is a true defend claim(claim5) in the - /// middle of the dispute game. - function test_stepAttackDummyClaim_defendTrueClaimInTheMiddle_succeeds() public { - // Give the test contract some ether - vm.deal(address(this), 1000 ether); - - // Make claims all the way down the tree. - (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.attack{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(2); - gameProxy.attack{ value: _getRequiredBond(2) }(disputed, 2, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(3); - gameProxy.attack{ value: _getRequiredBond(3) }(disputed, 3, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(4); - gameProxy.attack{ value: _getRequiredBond(4) }(disputed, 4, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC)); - bytes memory claimData5 = abi.encode(5, 5); - Claim claim5 = Claim.wrap(keccak256(claimData5)); - (,,,, disputed,,) = gameProxy.claimData(5); - gameProxy.attack{ value: _getRequiredBond(5) }(disputed, 5, claim5); - (,,,, disputed,,) = gameProxy.claimData(6); - gameProxy.defend{ value: _getRequiredBond(6) }(disputed, 6, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(7); - gameProxy.attack{ value: _getRequiredBond(7) }(disputed, 7, _dummyClaim()); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 8, 0); - gameProxy.step(8, true, claimData5, hex""); - } - function test_stepAttackDummyClaim_attackBranch0_succeeds() public { // Give the test contract some ether vm.deal(address(this), 1000 ether); @@ -1071,38 +1042,6 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { gameProxy.stepV2({_claimIndex: 4, _attackBranch: 3, _stateData: claimData3, _proof: stepProof}); } - /// @dev Tests that step reverts with false attacking claim when there is a true defend claim(claim5) in the middle - /// of the dispute game. - function test_stepAttackTrueClaim_defendTrueClaimInTheMiddle_reverts() public { - // Give the test contract some ether - vm.deal(address(this), 1000 ether); - - // Make claims all the way down the tree. - (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.attack{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(2); - gameProxy.attack{ value: _getRequiredBond(2) }(disputed, 2, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(3); - gameProxy.attack{ value: _getRequiredBond(3) }(disputed, 3, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(4); - gameProxy.attack{ value: _getRequiredBond(4) }(disputed, 4, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC)); - bytes memory claimData5 = abi.encode(5, 5); - Claim claim5 = Claim.wrap(keccak256(claimData5)); - (,,,, disputed,,) = gameProxy.claimData(5); - gameProxy.attack{ value: _getRequiredBond(5) }(disputed, 5, claim5); - (,,,, disputed,,) = gameProxy.claimData(6); - gameProxy.defend{ value: _getRequiredBond(6) }(disputed, 6, _dummyClaim()); - Claim postState_ = Claim.wrap(gameImpl.vm().step(claimData5, hex"", bytes32(0))); - (,,,, disputed,,) = gameProxy.claimData(7); - gameProxy.attack{ value: _getRequiredBond(7) }(disputed, 7, postState_); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 8, 0); - - vm.expectRevert(ValidStep.selector); - gameProxy.step(8, true, claimData5, hex""); - } - function test_addLocalKey_AttackBranch0_succeeds() public { // Give the test contract some ether vm.deal(address(this), 1000 ether); @@ -1363,75 +1302,6 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { assertEq(disputedOutputRoot, ROOT_CLAIM.raw()); } - /// @dev Tests that step reverts with false defending claim when there is a true defend claim(postState_) in the - /// middle of the dispute game. - function test_stepDefendDummyClaim_defendTrueClaimInTheMiddle_reverts() public { - // Give the test contract some ether - vm.deal(address(this), 1000 ether); - - // Make claims all the way down the tree. - (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.attack{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(2); - gameProxy.attack{ value: _getRequiredBond(2) }(disputed, 2, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(3); - gameProxy.attack{ value: _getRequiredBond(3) }(disputed, 3, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(4); - gameProxy.attack{ value: _getRequiredBond(4) }(disputed, 4, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC)); - - bytes memory claimData7 = abi.encode(5, 5); - Claim postState_ = Claim.wrap(gameImpl.vm().step(claimData7, hex"", bytes32(0))); - - (,,,, disputed,,) = gameProxy.claimData(5); - gameProxy.attack{ value: _getRequiredBond(5) }(disputed, 5, postState_); - (,,,, disputed,,) = gameProxy.claimData(6); - gameProxy.defend{ value: _getRequiredBond(6) }(disputed, 6, _dummyClaim()); - - bytes memory _dummyClaimData = abi.encode(gasleft(), gasleft()); - Claim dummyClaim7 = Claim.wrap(keccak256(_dummyClaimData)); - (,,,, disputed,,) = gameProxy.claimData(7); - gameProxy.attack{ value: _getRequiredBond(7) }(disputed, 7, dummyClaim7); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 8, 0); - vm.expectRevert(ValidStep.selector); - gameProxy.step(8, false, _dummyClaimData, hex""); - } - - /// @dev Tests that step reverts with true defending claim when there is a true defend claim(postState_) in the - /// middle of the dispute game. - function test_stepDefendTrueClaim_defendTrueClaimInTheMiddle_reverts() public { - // Give the test contract some ether - vm.deal(address(this), 1000 ether); - - // Make claims all the way down the tree. - (,,,, Claim disputed,,) = gameProxy.claimData(0); - gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(1); - gameProxy.attack{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(2); - gameProxy.attack{ value: _getRequiredBond(2) }(disputed, 2, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(3); - gameProxy.attack{ value: _getRequiredBond(3) }(disputed, 3, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(4); - gameProxy.attack{ value: _getRequiredBond(4) }(disputed, 4, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC)); - - bytes memory claimData7 = abi.encode(5, 5); - Claim claim7 = Claim.wrap(keccak256(claimData7)); - Claim postState_ = Claim.wrap(gameImpl.vm().step(claimData7, hex"", bytes32(0))); - - (,,,, disputed,,) = gameProxy.claimData(5); - gameProxy.attack{ value: _getRequiredBond(5) }(disputed, 5, postState_); - (,,,, disputed,,) = gameProxy.claimData(6); - gameProxy.defend{ value: _getRequiredBond(6) }(disputed, 6, _dummyClaim()); - (,,,, disputed,,) = gameProxy.claimData(7); - gameProxy.attack{ value: _getRequiredBond(7) }(disputed, 7, claim7); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 8, 0); - - vm.expectRevert(ValidStep.selector); - gameProxy.step(8, false, claimData7, hex""); - } - /// @dev Static unit test for the correctness an uncontested root resolution. function test_resolve_rootUncontested_succeeds() public { vm.warp(block.timestamp + 3 days + 12 hours); From 61f97ab79a0ea01e59917277963394f2f64be3fc Mon Sep 17 00:00:00 2001 From: billxu Date: Wed, 7 Aug 2024 09:52:09 +0800 Subject: [PATCH 08/11] The attack has been modified to support da and the FaultDisputeGameTest contract has been added to minimize changes in the testing code --- .../src/dispute/FaultDisputeGameN.sol | 9 ++-- .../test/dispute/FaultDisputeGameN.t.sol | 43 ++++++++++--------- .../test/dispute/FaultDisputeGameNTest.sol | 43 +++++++++++++++++++ 3 files changed, 69 insertions(+), 26 deletions(-) create mode 100644 packages/contracts-bedrock/test/dispute/FaultDisputeGameNTest.sol diff --git a/packages/contracts-bedrock/src/dispute/FaultDisputeGameN.sol b/packages/contracts-bedrock/src/dispute/FaultDisputeGameN.sol index e09034f9818e..2de37bb021c0 100644 --- a/packages/contracts-bedrock/src/dispute/FaultDisputeGameN.sol +++ b/packages/contracts-bedrock/src/dispute/FaultDisputeGameN.sol @@ -367,9 +367,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, ISemver { Claim _claim, uint64 _attackBranch ) - public - payable - virtual + internal { // For N = 4 (bisec), // 1. _attackBranch == 0 (attack) @@ -1196,8 +1194,9 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, ISemver { } } - function attackV2(Claim _disputed, uint256 _parentIndex, Claim _claim, uint64 _attackBranch) public payable { - moveV2(_disputed, _parentIndex, _claim, _attackBranch); + function attackV2(Claim _disputed, uint256 _parentIndex, uint64 _attackBranch, uint256 _daType, bytes memory _claims) public payable { + Claim claim = Claim.wrap(LibDA.getClaimsHash(_daType, MAX_ATTACK_BRANCH, _claims)); + moveV2(_disputed, _parentIndex, claim, _attackBranch); } function step( diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index 894399053b38..a7a61e1a7fbe 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -6,6 +6,7 @@ import { Vm } from "forge-std/Vm.sol"; import { DisputeGameFactory_Init } from "test/dispute/DisputeGameFactory.t.sol"; import { DisputeGameFactory } from "src/dispute/DisputeGameFactory.sol"; import { FaultDisputeGame, IDisputeGame } from "src/dispute/FaultDisputeGameN.sol"; +import { FaultDisputeGameTest } from "test/dispute/FaultDisputeGameNTest.sol"; import { DelayedWETH } from "src/dispute/weth/DelayedWETH.sol"; import { PreimageOracle } from "src/cannon/PreimageOracle.sol"; @@ -32,9 +33,9 @@ contract FaultDisputeGame_Init is DisputeGameFactory_Init { uint256 internal constant MAX_ATTACK_BRANCH = (1 << N_BITS) - 1; /// @dev The implementation of the game. - FaultDisputeGame internal gameImpl; + FaultDisputeGameTest internal gameImpl; /// @dev The `Clone` proxy of the game. - FaultDisputeGame internal gameProxy; + FaultDisputeGameTest internal gameProxy; /// @dev The extra data passed to the game for initialization. bytes internal extraData; @@ -53,7 +54,7 @@ contract FaultDisputeGame_Init is DisputeGameFactory_Init { AlphabetVM _vm = new AlphabetVM(absolutePrestate, new PreimageOracle(0, 0)); // Deploy an implementation of the fault game - gameImpl = new FaultDisputeGame({ + gameImpl = new FaultDisputeGameTest({ _gameType: GAME_TYPE, _absolutePrestate: absolutePrestate, _maxGameDepth: 2 ** 3, @@ -69,7 +70,7 @@ contract FaultDisputeGame_Init is DisputeGameFactory_Init { // Register the game implementation with the factory. disputeGameFactory.setImplementation(GAME_TYPE, gameImpl); // Create a new game. - gameProxy = FaultDisputeGame(payable(address(disputeGameFactory.create(GAME_TYPE, rootClaim, extraData)))); + gameProxy = FaultDisputeGameTest(payable(address(disputeGameFactory.create(GAME_TYPE, rootClaim, extraData)))); // Check immutables assertEq(gameProxy.gameType().raw(), GAME_TYPE.raw()); @@ -141,14 +142,14 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // `IDisputeGame` Implementation Tests // //////////////////////////////////////////////////////////////// - /// @dev Tests that the constructor of the `FaultDisputeGame` reverts when the `MAX_GAME_DEPTH` parameter is + /// @dev Tests that the constructor of the `FaultDisputeGameTest` reverts when the `MAX_GAME_DEPTH` parameter is /// greater than `LibPosition.MAX_POSITION_BITLEN - 1`. function testFuzz_constructor_maxDepthTooLarge_reverts(uint256 _maxGameDepth) public { AlphabetVM alphabetVM = new AlphabetVM(absolutePrestate, new PreimageOracle(0, 0)); _maxGameDepth = bound(_maxGameDepth, LibPosition.MAX_POSITION_BITLEN, type(uint256).max - 1); vm.expectRevert(MaxDepthTooLarge.selector); - new FaultDisputeGame({ + new FaultDisputeGameTest({ _gameType: GAME_TYPE, _absolutePrestate: absolutePrestate, _maxGameDepth: _maxGameDepth, @@ -162,14 +163,14 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { }); } - /// @dev Tests that the constructor of the `FaultDisputeGame` reverts when the `_splitDepth` + /// @dev Tests that the constructor of the `FaultDisputeGameTest` reverts when the `_splitDepth` /// parameter is greater than or equal to the `MAX_GAME_DEPTH` function testFuzz_constructor_invalidSplitDepth_reverts(uint256 _splitDepth) public { AlphabetVM alphabetVM = new AlphabetVM(absolutePrestate, new PreimageOracle(0, 0)); _splitDepth = bound(_splitDepth, 2 ** 3, type(uint256).max); vm.expectRevert(InvalidSplitDepth.selector); - new FaultDisputeGame({ + new FaultDisputeGameTest({ _gameType: GAME_TYPE, _absolutePrestate: absolutePrestate, _maxGameDepth: 2 ** 3, @@ -183,7 +184,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { }); } - /// @dev Tests that the constructor of the `FaultDisputeGame` reverts when clock extension is greater than the + /// @dev Tests that the constructor of the `FaultDisputeGameTest` reverts when clock extension is greater than the /// max clock duration. function testFuzz_constructor_clockExtensionTooLong_reverts( uint64 _maxClockDuration, @@ -196,7 +197,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { _maxClockDuration = uint64(bound(_maxClockDuration, 0, type(uint64).max - 1)); _clockExtension = uint64(bound(_clockExtension, _maxClockDuration + 1, type(uint64).max)); vm.expectRevert(InvalidClockExtension.selector); - new FaultDisputeGame({ + new FaultDisputeGameTest({ _gameType: GAME_TYPE, _absolutePrestate: absolutePrestate, _maxGameDepth: 16, @@ -252,7 +253,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { Claim claim = _dummyClaim(); vm.expectRevert(abi.encodeWithSelector(UnexpectedRootClaim.selector, claim)); gameProxy = - FaultDisputeGame(payable(address(disputeGameFactory.create(GAME_TYPE, claim, abi.encode(_blockNumber))))); + FaultDisputeGameTest(payable(address(disputeGameFactory.create(GAME_TYPE, claim, abi.encode(_blockNumber))))); } /// @dev Tests that the proxy receives ETH from the dispute game factory. @@ -261,7 +262,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { vm.deal(address(this), _value); assertEq(address(gameProxy).balance, 0); - gameProxy = FaultDisputeGame( + gameProxy = FaultDisputeGameTest( payable(address(disputeGameFactory.create{ value: _value }(GAME_TYPE, ROOT_CLAIM, abi.encode(1)))) ); assertEq(address(gameProxy).balance, 0); @@ -289,7 +290,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { Claim claim = _dummyClaim(); vm.expectRevert(abi.encodeWithSelector(BadExtraData.selector)); - gameProxy = FaultDisputeGame(payable(address(disputeGameFactory.create(GAME_TYPE, claim, _extraData)))); + gameProxy = FaultDisputeGameTest(payable(address(disputeGameFactory.create(GAME_TYPE, claim, _extraData)))); } /// @dev Tests that the game is initialized with the correct data. @@ -671,7 +672,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { IDisputeGame game = disputeGameFactory.create(GAME_TYPE, Claim.wrap(outputRoot), abi.encode(_l2BlockNumber + 1)); // Challenge the L2 block number. - FaultDisputeGame fdg = FaultDisputeGame(address(game)); + FaultDisputeGameTest fdg = FaultDisputeGameTest(address(game)); fdg.challengeRootL2Block(outputRootProof, headerRLP); // Ensure that a duplicate challenge reverts. @@ -710,7 +711,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { IDisputeGame game = disputeGameFactory.create{ value: 0.1 ether }( GAME_TYPE, Claim.wrap(outputRoot), abi.encode(_l2BlockNumber + 1) ); - FaultDisputeGame fdg = FaultDisputeGame(address(game)); + FaultDisputeGameTest fdg = FaultDisputeGameTest(address(game)); // Attack the root as 0xb0b uint256 bond = _getRequiredBondV2(0, 0); @@ -767,7 +768,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { IDisputeGame game = disputeGameFactory.create(GAME_TYPE, Claim.wrap(outputRoot), abi.encode(_l2BlockNumber)); // Challenge the L2 block number. - FaultDisputeGame fdg = FaultDisputeGame(address(game)); + FaultDisputeGameTest fdg = FaultDisputeGameTest(address(game)); vm.expectRevert(BlockNumberMatches.selector); fdg.challengeRootL2Block(outputRootProof, headerRLP); @@ -797,7 +798,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // Create the dispute game with the output root at the wrong L2 block number. IDisputeGame game = disputeGameFactory.create(GAME_TYPE, Claim.wrap(outputRoot), abi.encode(1)); - FaultDisputeGame fdg = FaultDisputeGame(address(game)); + FaultDisputeGameTest fdg = FaultDisputeGameTest(address(game)); vm.expectRevert(InvalidHeaderRLP.selector); fdg.challengeRootL2Block(outputRootProof, hex""); @@ -810,7 +811,7 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { // Create the dispute game with the output root at the wrong L2 block number. IDisputeGame game = disputeGameFactory.create(GAME_TYPE, Claim.wrap(outputRoot), abi.encode(1)); - FaultDisputeGame fdg = FaultDisputeGame(address(game)); + FaultDisputeGameTest fdg = FaultDisputeGameTest(address(game)); vm.expectRevert(InvalidHeaderRLP.selector); fdg.challengeRootL2Block(outputRootProof, hex""); @@ -2416,7 +2417,7 @@ contract FaultDisputeN_1v1_Actors_Test is FaultDisputeGame_Init { DisputeActor internal dishonest; function setUp() public override { - // Setup the `FaultDisputeGame` + // Setup the `FaultDisputeGameTest` super.setUp(); } @@ -2952,10 +2953,10 @@ contract FaultDisputeN_1v1_Actors_Test is FaultDisputeGame_Init { contract ClaimCreditReenter { Vm internal immutable vm; - FaultDisputeGame internal immutable GAME; + FaultDisputeGameTest internal immutable GAME; uint256 public numCalls; - constructor(FaultDisputeGame _gameProxy, Vm _vm) { + constructor(FaultDisputeGameTest _gameProxy, Vm _vm) { GAME = _gameProxy; vm = _vm; } diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameNTest.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameNTest.sol new file mode 100644 index 000000000000..84e527cb70f9 --- /dev/null +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameNTest.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import { GameType, Claim, Duration } from "src/dispute/lib/LibUDT.sol"; +import { FaultDisputeGame } from "src/dispute/FaultDisputeGameN.sol"; +import { IAnchorStateRegistry } from "src/dispute/interfaces/IAnchorStateRegistry.sol"; +import { IDelayedWETH } from "src/dispute/interfaces/IDelayedWETH.sol"; +import { IBigStepper } from "src/dispute/interfaces/IBigStepper.sol"; + +contract FaultDisputeGameTest is FaultDisputeGame { + constructor( + GameType _gameType, + Claim _absolutePrestate, + uint256 _maxGameDepth, + uint256 _splitDepth, + Duration _clockExtension, + Duration _maxClockDuration, + IBigStepper _vm, + IDelayedWETH _weth, + IAnchorStateRegistry _anchorStateRegistry, + uint256 _l2ChainId + ) + FaultDisputeGame( + _gameType, + _absolutePrestate, + _maxGameDepth, + _splitDepth, + _clockExtension, + _maxClockDuration, + _vm, + _weth, + _anchorStateRegistry, + _l2ChainId + ) + { } + + // For testing convenience and to minimize changes in the testing code, the submission of the "claims" value is + // omitted during the attack. In contract testing, the value of "claims" is already known and does not need to be + // submitted via calldata or EIP-4844 during the attack. + function attackV2(Claim _disputed, uint256 _parentIndex, Claim _claim, uint64 _attackBranch) public payable { + moveV2(_disputed, _parentIndex, _claim, _attackBranch); + } +} From 8235f7713c0955f626e3ae4f9d2d857ae6c57532 Mon Sep 17 00:00:00 2001 From: billxu Date: Sun, 11 Aug 2024 16:33:21 +0800 Subject: [PATCH 09/11] Added an attack test with the da type as calldata. --- .../test/dispute/FaultDisputeGameN.t.sol | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index a7a61e1a7fbe..e03b7d48b753 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -2199,6 +2199,57 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { bytes32 h = keccak256(abi.encode(_ident | (1 << 248), address(gameProxy), _localContext)); return bytes32((uint256(h) & ~uint256(0xFF << 248)) | (1 << 248)); } + + function test_stepAttackDummyClaim_attackDACalldata_succeeds() public { + // Give the test contract some ether + vm.deal(address(this), 1000 ether); + + bytes memory claimData1 = abi.encode(1, 1); + bytes memory claimData2 = abi.encode(2, 2); + bytes memory claimData3 = abi.encode(3, 3); + Claim claim1 = Claim.wrap(keccak256(claimData1)); + Claim claim2 = Claim.wrap(keccak256(claimData2)); + Claim claim3 = Claim.wrap(keccak256(claimData3)); + + bytes memory input = abi.encodePacked(claim1, claim2, claim3); // bytes.concat(claim1.raw(), claim2.raw(), claim3.raw()); + bytes memory claims = abi.encodePacked(claim1, claim2, claim3); + + // Make claims all the way down the tree. + (,,,, Claim disputed,,) = gameProxy.claimData(0); + gameProxy.attackV2{ value: _getRequiredBondV2(0, 0) }(disputed, 0, _dummyClaim(), 0); + + (,,,, disputed,,) = gameProxy.claimData(1); + gameProxy.attackV2{ value: _getRequiredBondV2(1, 0) }(disputed, 1, _dummyClaim(), 0); + + (,,,, disputed,,) = gameProxy.claimData(2); + gameProxy.attackV2{ value: _getRequiredBondV2(2, 0) }(disputed, 2, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC), 0); + + (,,,, disputed,,) = gameProxy.claimData(3); + uint256 bond = _getRequiredBondV2(3, 0); + gameProxy.attackV2{ value: bond }(disputed, 3, 0, LibDA.DA_TYPE_CALLDATA, claims); + vm.expectRevert(ClaimAlreadyExists.selector); + gameProxy.attackV2{ value: bond }(disputed, 3, Claim.wrap(LibDA.getClaimsHash(LibDA.DA_TYPE_CALLDATA, 3, input)), 0); + + + LibDA.DAItem memory preStateItem = LibDA.DAItem({ + daType: LibDA.DA_TYPE_CALLDATA, + dataHash: claim2.raw(), + proof: abi.encodePacked(claim1, claim3) + }); + LibDA.DAItem memory postStateItem = LibDA.DAItem({ + daType: LibDA.DA_TYPE_CALLDATA, + dataHash: claim3.raw(), + proof: bytes.concat(keccak256(abi.encode(claim1.raw(), claim2.raw()))) + }); + FaultDisputeGame.StepProof memory stepProof = FaultDisputeGame.StepProof({ + preStateItem: preStateItem, + postStateItem: postStateItem, + vmProof: hex"" + }); + + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 2, preStateItem); + gameProxy.stepV2({_claimIndex: 4, _attackBranch: 2, _stateData: claimData2, _proof: stepProof}); + } } contract FaultDisputeGameN_LessSplitDepth_Test is FaultDisputeGame_Init { From 6af98dfa2b270ce85a0718a734cf1c71afb3c927 Mon Sep 17 00:00:00 2001 From: billxu Date: Tue, 20 Aug 2024 09:25:46 +0800 Subject: [PATCH 10/11] Add comments and remove redundant code --- .../test/dispute/FaultDisputeGameN.t.sol | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index e03b7d48b753..f86e18838c53 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -2211,7 +2211,6 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { Claim claim2 = Claim.wrap(keccak256(claimData2)); Claim claim3 = Claim.wrap(keccak256(claimData3)); - bytes memory input = abi.encodePacked(claim1, claim2, claim3); // bytes.concat(claim1.raw(), claim2.raw(), claim3.raw()); bytes memory claims = abi.encodePacked(claim1, claim2, claim3); // Make claims all the way down the tree. @@ -2228,8 +2227,15 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { uint256 bond = _getRequiredBondV2(3, 0); gameProxy.attackV2{ value: bond }(disputed, 3, 0, LibDA.DA_TYPE_CALLDATA, claims); vm.expectRevert(ClaimAlreadyExists.selector); - gameProxy.attackV2{ value: bond }(disputed, 3, Claim.wrap(LibDA.getClaimsHash(LibDA.DA_TYPE_CALLDATA, 3, input)), 0); + gameProxy.attackV2{ value: bond }(disputed, 3, Claim.wrap(LibDA.getClaimsHash(LibDA.DA_TYPE_CALLDATA, 3, claims)), 0); + // This variable is not used + LibDA.DAItem memory localDataItem = LibDA.DAItem({ + daType: LibDA.DA_TYPE_CALLDATA, + dataHash: '00000000000000000000000000000000', + proof: hex"" + }); + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 2, localDataItem); LibDA.DAItem memory preStateItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA, @@ -2246,8 +2252,6 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { postStateItem: postStateItem, vmProof: hex"" }); - - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 2, preStateItem); gameProxy.stepV2({_claimIndex: 4, _attackBranch: 2, _stateData: claimData2, _proof: stepProof}); } } From c1d29c533a77d1540466bf317a4855d1bca4b9d4 Mon Sep 17 00:00:00 2001 From: billxu Date: Wed, 21 Aug 2024 10:34:32 +0800 Subject: [PATCH 11/11] Modify the variable definition in the unit test --- .../contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol index f86e18838c53..7cd7dfb7675f 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGameN.t.sol @@ -2229,13 +2229,12 @@ contract FaultDisputeGameN_Test is FaultDisputeGame_Init { vm.expectRevert(ClaimAlreadyExists.selector); gameProxy.attackV2{ value: bond }(disputed, 3, Claim.wrap(LibDA.getClaimsHash(LibDA.DA_TYPE_CALLDATA, 3, claims)), 0); - // This variable is not used - LibDA.DAItem memory localDataItem = LibDA.DAItem({ + LibDA.DAItem memory dummyDataItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA, - dataHash: '00000000000000000000000000000000', + dataHash: _dummyClaim().raw(), proof: hex"" }); - gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 2, localDataItem); + gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 4, 2, dummyDataItem); LibDA.DAItem memory preStateItem = LibDA.DAItem({ daType: LibDA.DA_TYPE_CALLDATA,