From 75c755413f250afbdfb5e3b8f2d69ebc33f0cb4c Mon Sep 17 00:00:00 2001 From: billxu Date: Mon, 29 Jul 2024 12:46:19 +0800 Subject: [PATCH 1/2] 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 2/2] 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