Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ctb): Pass msg.sender to dispute games from the factory #10149

Merged
merged 4 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion op-bindings/bindings/disputegamefactory.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion op-bindings/bindings/disputegamefactory_more.go

Large diffs are not rendered by default.

35 changes: 33 additions & 2 deletions op-bindings/bindings/faultdisputegame.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion op-bindings/bindings/faultdisputegame_more.go

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions packages/contracts-bedrock/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,12 @@
"sourceCodeHash": "0xba941ad1f941f5a4a066182d50634fa9b190085ed82779decef71c019ba963c5"
},
"src/dispute/DisputeGameFactory.sol": {
"initCodeHash": "0xf4b8bc7cfaa1e741bc6bf1f9c8e5e20e3f77d603dd6f7f58af9fea7ebf0b070a",
"sourceCodeHash": "0x8545910bdb40f5e706a0ae5ed274cabc6d1f17be92d497a490d5732d74ac9c59"
"initCodeHash": "0xcbf28ecd117cbcbb9cea34dd75d2d0afbdc42865311b6fa94f23958c26186ab3",
"sourceCodeHash": "0x08f34cec56d58ea6ee7a47b5adcbeca6a68a5dd1daa949330b4bde86c2e605f5"
},
"src/dispute/FaultDisputeGame.sol": {
"initCodeHash": "0x4ea53ec8274b7a25012aab6655cd84a60b4cbcdba95ad199085fd81910731bee",
"sourceCodeHash": "0x778aafed19b2d8dddd61a44d39e94fb3139a89e416a314572534064ab8823ee1"
"initCodeHash": "0x98544bd91d1d777bfcfa0d71b81a9ed75f9feb31536aad934b92dcf16bb21ce7",
"sourceCodeHash": "0x06c7f5206e9c121c5c7fa2e6e114f9a80781025954c830bc7641f3ab0a5b2583"
},
"src/dispute/weth/DelayedWETH.sol": {
"initCodeHash": "0x7b6ec89eaec09e369426e73161a9c6932223bb1f974377190c3f6f552995da35",
Expand Down
13 changes: 13 additions & 0 deletions packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,19 @@
"stateMutability": "pure",
"type": "function"
},
{
"inputs": [],
"name": "gameCreator",
"outputs": [
{
"internalType": "address",
"name": "creator_",
"type": "address"
}
],
"stateMutability": "pure",
"type": "function"
},
{
"inputs": [],
"name": "gameData",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,19 @@
"stateMutability": "pure",
"type": "function"
},
{
"inputs": [],
"name": "gameCreator",
"outputs": [
{
"internalType": "address",
"name": "creator_",
"type": "address"
}
],
"stateMutability": "pure",
"type": "function"
},
{
"inputs": [],
"name": "gameData",
Expand Down
16 changes: 13 additions & 3 deletions packages/contracts-bedrock/src/dispute/DisputeGameFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ contract DisputeGameFactory is OwnableUpgradeable, IDisputeGameFactory, ISemver
using LibClone for address;

/// @notice Semantic version.
/// @custom:semver 0.4.0
string public constant version = "0.4.0";
/// @custom:semver 0.5.0
string public constant version = "0.5.0";

/// @inheritdoc IDisputeGameFactory
mapping(GameType => IDisputeGame) public gameImpls;
Expand Down Expand Up @@ -103,7 +103,17 @@ contract DisputeGameFactory is OwnableUpgradeable, IDisputeGameFactory, ISemver
bytes32 parentHash = blockhash(block.number - 1);

// Clone the implementation contract and initialize it with the given parameters.
proxy_ = IDisputeGame(address(impl).clone(abi.encodePacked(_rootClaim, parentHash, _extraData)));
//
// CWIA Calldata Layout:
// ┌──────────────┬────────────────────────────────────┐
// │ Bytes │ Description │
// ├──────────────┼────────────────────────────────────┤
// │ [0, 20) │ Game creator address │
// │ [20, 52) │ Root claim │
// │ [52, 84) │ Parent block hash at creation time │
// │ [84, 84 + n) │ Extra data (opaque) │
// └──────────────┴────────────────────────────────────┘
proxy_ = IDisputeGame(address(impl).clone(abi.encodePacked(msg.sender, _rootClaim, parentHash, _extraData)));
proxy_.initialize{ value: msg.value }();

// Compute the unique identifier for the dispute game.
Expand Down
51 changes: 32 additions & 19 deletions packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, ISemver {
OutputRoot public startingOutputRoot;

/// @notice Semantic version.
/// @custom:semver 0.11.0
string public constant version = "0.11.0";
/// @custom:semver 0.12.0
string public constant version = "0.12.0";

/// @param _gameType The type ID of the game.
/// @param _absolutePrestate The absolute prestate of the instruction trace.
Expand Down Expand Up @@ -345,14 +345,9 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, ISemver {
}
}

/// @inheritdoc IFaultDisputeGame
function l1Head() public pure returns (Hash l1Head_) {
l1Head_ = Hash.wrap(_getArgBytes32(0x20));
}

/// @inheritdoc IFaultDisputeGame
function l2BlockNumber() public pure returns (uint256 l2BlockNumber_) {
l2BlockNumber_ = _getArgUint256(0x40);
l2BlockNumber_ = _getArgUint256(0x54);
clabby marked this conversation as resolved.
Show resolved Hide resolved
}

////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -451,16 +446,26 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, ISemver {
resolvedSubgames[_claimIndex] = true;
}

/// @inheritdoc IDisputeGame
function gameCreator() public pure returns (address creator_) {
creator_ = _getArgAddress(0x00);
}

/// @inheritdoc IDisputeGame
function rootClaim() public pure returns (Claim rootClaim_) {
rootClaim_ = Claim.wrap(_getArgBytes32(0x00));
rootClaim_ = Claim.wrap(_getArgBytes32(0x14));
}

/// @inheritdoc IDisputeGame
function l1Head() public pure returns (Hash l1Head_) {
l1Head_ = Hash.wrap(_getArgBytes32(0x34));
}

/// @inheritdoc IDisputeGame
function extraData() public pure returns (bytes memory extraData_) {
// The extra data starts at the second word within the cwia calldata and
// is 32 bytes long.
extraData_ = _getArgBytes(0x40, 0x20);
extraData_ = _getArgBytes(0x54, 0x20);
}

/// @inheritdoc IDisputeGame
Expand Down Expand Up @@ -513,15 +518,23 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, ISemver {
// configured starting block number.
if (l2BlockNumber() <= rootBlockNumber) revert UnexpectedRootClaim(rootClaim());

// Revert if the calldata size is too large, which signals that the `extraData` contains more than expected.
// This is to prevent adding extra bytes to the `extraData` that result in a different game UUID in the factory,
// but are not used by the game, which would allow for multiple dispute games for the same output proposal to
// be created.
// Expected length: 0x66 (0x04 selector + 0x20 root claim + 0x20 l1 head + 0x20 extraData + 0x02 CWIA bytes)
// Revert if the calldata size is not the expected length.
//
// This is to prevent adding extra or omitting bytes from to `extraData` that result in a different game UUID
// in the factory, but are not used by the game, which would allow for multiple dispute games for the same
// output proposal to be created.
//
// Expected length: 0x7A
// - 0x04 selector
// - 0x14 creator address
// - 0x20 root claim
// - 0x20 l1 head
// - 0x20 extraData
// - 0x02 CWIA bytes
assembly {
if gt(calldatasize(), 0x66) {
// Store the selector for `ExtraDataTooLong()` & revert
mstore(0x00, 0xc407e025)
if iszero(eq(calldatasize(), 0x7A)) {
// Store the selector for `BadExtraData()` & revert
mstore(0x00, 0x9824bdab)
revert(0x1C, 0x04)
}
}
Expand All @@ -531,7 +544,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, ISemver {
ClaimData({
parentIndex: type(uint32).max,
counteredBy: address(0),
claimant: tx.origin,
claimant: gameCreator(),
bond: uint128(msg.value),
claim: rootClaim(),
position: ROOT_POSITION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,23 @@ interface IDisputeGame is IInitializable {
/// @return gameType_ The type of proof system being used.
function gameType() external view returns (GameType gameType_);

/// @notice Getter for the root claim.
/// @notice Getter for the creator of the dispute game.
/// @dev `clones-with-immutable-args` argument #1
/// @return creator_ The creator of the dispute game.
function gameCreator() external pure returns (address creator_);

/// @notice Getter for the root claim.
/// @dev `clones-with-immutable-args` argument #2
/// @return rootClaim_ The root claim of the DisputeGame.
function rootClaim() external pure returns (Claim rootClaim_);

/// @notice Getter for the parent hash of the L1 block when the dispute game was created.
/// @dev `clones-with-immutable-args` argument #3
refcell marked this conversation as resolved.
Show resolved Hide resolved
/// @return l1Head_ The parent hash of the L1 block when the dispute game was created.
function l1Head() external pure returns (Hash l1Head_);

/// @notice Getter for the extra data.
/// @dev `clones-with-immutable-args` argument #2
/// @dev `clones-with-immutable-args` argument #4
refcell marked this conversation as resolved.
Show resolved Hide resolved
/// @return extraData_ Any extra data supplied to the dispute game contract by the creator.
function extraData() external pure returns (bytes memory extraData_);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ interface IFaultDisputeGame is IDisputeGame {
/// @param _claimIndex The index of the subgame root claim to resolve.
function resolveClaim(uint256 _claimIndex) external payable;

/// @notice A block hash on the L1 that contains the disputed output root.
clabby marked this conversation as resolved.
Show resolved Hide resolved
function l1Head() external view returns (Hash l1Head_);

/// @notice The l2BlockNumber of the disputed output root in the `L2OutputOracle`.
clabby marked this conversation as resolved.
Show resolved Hide resolved
function l2BlockNumber() external view returns (uint256 l2BlockNumber_);

Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/src/libraries/DisputeErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ error NoCreditToClaim();
/// @notice Thrown when the transfer of credit to a recipient account reverts.
error BondTransferFailed();

/// @notice Thrown when the `extraData` passed to the CWIA proxy is too long for the `FaultDisputeGame`.
error ExtraDataTooLong();
/// @notice Thrown when the `extraData` passed to the CWIA proxy is of improper length, or contains invalid information.
error BadExtraData();

/// @notice Thrown when a defense against the root claim is attempted.
error CannotDefendRootClaim();
Expand Down
18 changes: 10 additions & 8 deletions packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,17 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init {
assertEq(delayedWeth.balanceOf(address(gameProxy)), _value);
}

/// @dev Tests that the game cannot be initialized with extra data > 64 bytes long (root claim + l2 block number
/// concatenated)
function testFuzz_initialize_extraDataTooLong_reverts(uint256 _extraDataLen) public {
/// @dev Tests that the game cannot be initialized with extra data of the incorrect length (must be 32 bytes)
function testFuzz_initialize_badExtraData_reverts(uint256 _extraDataLen) public {
// The `DisputeGameFactory` will pack the root claim and the extra data into a single array, which is enforced
// to be at least 64 bytes long.
// We bound the upper end to 23.5KB to ensure that the minimal proxy never surpasses the contract size limit
// in this test, as CWIA proxies store the immutable args in their bytecode.
// [33 bytes, 23.5 KB]
_extraDataLen = bound(_extraDataLen, 33, 23_500);
// [0 bytes, 31 bytes] u [33 bytes, 23.5 KB]
_extraDataLen = bound(_extraDataLen, 0, 23_500);
if (_extraDataLen == 32) {
_extraDataLen++;
}
bytes memory _extraData = new bytes(_extraDataLen);

// Assign the first 32 bytes in `extraData` to a valid L2 block number passed the starting block.
Expand All @@ -196,7 +198,7 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init {
}

Claim claim = _dummyClaim();
vm.expectRevert(abi.encodeWithSelector(ExtraDataTooLong.selector));
vm.expectRevert(abi.encodeWithSelector(BadExtraData.selector));
gameProxy = FaultDisputeGame(payable(address(disputeGameFactory.create(GAME_TYPE, claim, _extraData))));
}

Expand All @@ -214,7 +216,7 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init {
) = gameProxy.claimData(0);
assertEq(parentIndex, type(uint32).max);
assertEq(counteredBy, address(0));
assertEq(claimant, DEFAULT_SENDER);
assertEq(claimant, address(this));
assertEq(bond, 0);
assertEq(claim.raw(), ROOT_CLAIM.raw());
assertEq(position.raw(), 1);
Expand Down Expand Up @@ -435,7 +437,7 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init {
// Assert correctness of the parent claim's data.
assertEq(parentIndex, type(uint32).max);
assertEq(counteredBy, address(0));
assertEq(claimant, DEFAULT_SENDER);
assertEq(claimant, address(this));
assertEq(bond, 0);
assertEq(claim.raw(), ROOT_CLAIM.raw());
assertEq(position.raw(), 1);
Expand Down