Skip to content

Commit 35131a8

Browse files
mbaxtermbaxter
authored
proofs: Move proposer/challenger params on v2 contract (#17463)
* Make calldata length check overridable * Rework calldata length check * Make calldata length helpers more granular * Update PDG to use CWIA for proposer and challenger * Add a few more tests * Cleanup - rework method name, comments * Run just lint * Fix test names * Add some more tests around invalid FDG immutable args data * Add tests around invalid immutable args when creating PDGv2 * Add some method documentation * Regenerate snapshots, run semver-lock * Simplify method name * Fix notice natspec comments * Clean up step tests * Reorder auth check and call to super.initialize * Add fuzz test to validate step() reverts for unauthorized actors * Run semver-lock
1 parent 6440220 commit 35131a8

File tree

9 files changed

+349
-116
lines changed

9 files changed

+349
-116
lines changed

‎packages/contracts-bedrock/interfaces/dispute/v2/IPermissionedDisputeGameV2.sol‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,11 @@ interface IPermissionedDisputeGameV2 is IDisputeGame {
130130

131131
error BadAuth();
132132

133-
function proposer() external view returns (address proposer_);
134-
function challenger() external view returns (address challenger_);
133+
function proposer() external pure returns (address proposer_);
134+
function challenger() external pure returns (address challenger_);
135135

136136
function __constructor__(
137-
IFaultDisputeGameV2.GameConstructorParams memory _params,
138-
address _proposer,
139-
address _challenger
137+
IFaultDisputeGameV2.GameConstructorParams memory _params
140138
)
141139
external;
142140
}

‎packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGameV2.json‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,6 @@
3232
"internalType": "struct FaultDisputeGameV2.GameConstructorParams",
3333
"name": "_params",
3434
"type": "tuple"
35-
},
36-
{
37-
"internalType": "address",
38-
"name": "_proposer",
39-
"type": "address"
40-
},
41-
{
42-
"internalType": "address",
43-
"name": "_challenger",
44-
"type": "address"
4535
}
4636
],
4737
"stateMutability": "nonpayable",
@@ -182,7 +172,7 @@
182172
"type": "address"
183173
}
184174
],
185-
"stateMutability": "view",
175+
"stateMutability": "pure",
186176
"type": "function"
187177
},
188178
{
@@ -660,7 +650,7 @@
660650
"type": "address"
661651
}
662652
],
663-
"stateMutability": "view",
653+
"stateMutability": "pure",
664654
"type": "function"
665655
},
666656
{

‎packages/contracts-bedrock/snapshots/semver-lock.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,12 @@
176176
"sourceCodeHash": "0x8fdd69d4bcd33a3d8b49a73ff5b6855f9ad5f7e2b7393e67cd755973b127b1e8"
177177
},
178178
"src/dispute/v2/FaultDisputeGameV2.sol:FaultDisputeGameV2": {
179-
"initCodeHash": "0x13ef27ad793c95be884dea8259ac06619bf13d10868c5fc9980bc402b59efb8d",
180-
"sourceCodeHash": "0x47c141889e647820759a2eaa84c31be8acdce6427cc4fe7c00a482ba44d62b87"
179+
"initCodeHash": "0xb5a7bfcbfcb445dc57fc88c07d7305191dc32cc0cf5580d50b4897229e4033c1",
180+
"sourceCodeHash": "0x38fd8878a22564cd63e2bdc6af51edead373e2c4a90e631d2774078bbaa54ea6"
181181
},
182182
"src/dispute/v2/PermissionedDisputeGameV2.sol:PermissionedDisputeGameV2": {
183-
"initCodeHash": "0x7b1385d347dce625d63f71c13201fd90e63fcaeae17416911bccd9d670b3afc4",
184-
"sourceCodeHash": "0xc5aa308a19fd9600607370693885155a3d6f9f7bf8a7c6ff93a389c37c136555"
183+
"initCodeHash": "0xcff1406639ff8a83a4c948f412329956104bc7ee30eb8da169a2ba5ef6d8848f",
184+
"sourceCodeHash": "0x53fdae5faf97beed5f23d4f285bed06766161ab15c88e3a388f84808471a73c3"
185185
},
186186
"src/legacy/DeployerWhitelist.sol:DeployerWhitelist": {
187187
"initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29",

‎packages/contracts-bedrock/src/dispute/v2/FaultDisputeGameV2.sol‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,9 @@ contract FaultDisputeGameV2 is Clone, ISemver {
151151
uint256 internal constant HEADER_BLOCK_NUMBER_INDEX = 8;
152152

153153
/// @notice Semantic version.
154-
/// @custom:semver 2.0.0
154+
/// @custom:semver 2.1.0
155155
function version() public pure virtual returns (string memory) {
156-
return "2.0.0";
156+
return "2.1.0";
157157
}
158158

159159
/// @notice The starting timestamp of the game
@@ -268,20 +268,7 @@ contract FaultDisputeGameV2 is Clone, ISemver {
268268
// This is to prevent adding extra or omitting bytes from to `extraData` that result in a different game UUID
269269
// in the factory, but are not used by the game, which would allow for multiple dispute games for the same
270270
// output proposal to be created.
271-
//
272-
// Expected length: 246 bytes
273-
// - 4 bytes: selector
274-
// - 2 bytes: CWIA length prefix
275-
// - 20 bytes: creator address
276-
// - 32 bytes: root claim
277-
// - 32 bytes: l1 head
278-
// - 32 bytes: extraData
279-
// - 32 bytes: absolutePrestate
280-
// - 20 bytes: vm address
281-
// - 20 bytes: anchorStateRegistry address
282-
// - 20 bytes: weth address
283-
// - 32 bytes: l2ChainId
284-
if (msg.data.length != 246) revert BadExtraData();
271+
if (msg.data.length != expectedInitCallDataLength()) revert BadExtraData();
285272

286273
// Grab the latest anchor root.
287274
(Hash root, uint256 rootBlockNumber) = anchorStateRegistry().getAnchorRoot();
@@ -340,6 +327,30 @@ contract FaultDisputeGameV2 is Clone, ISemver {
340327
GameType.unwrap(anchorStateRegistry().respectedGameType()) == GameType.unwrap(GAME_TYPE);
341328
}
342329

330+
/// @notice Returns the expected calldata length for the initialize method
331+
function expectedInitCallDataLength() internal pure returns (uint256) {
332+
// Expected length: 6 bytes + immutable args byte count
333+
// - 4 bytes: selector
334+
// - 2 bytes: CWIA length prefix
335+
// - n bytes: Immutable args data
336+
return 6 + immutableArgsByteCount();
337+
}
338+
339+
/// @notice Returns the byte count of the immutable args for this contract.
340+
function immutableArgsByteCount() internal pure virtual returns (uint256) {
341+
// Expected length: 240 bytes
342+
// - 20 bytes: creator address
343+
// - 32 bytes: root claim
344+
// - 32 bytes: l1 head
345+
// - 32 bytes: extraData
346+
// - 32 bytes: absolutePrestate
347+
// - 20 bytes: vm address
348+
// - 20 bytes: anchorStateRegistry address
349+
// - 20 bytes: weth address
350+
// - 32 bytes: l2ChainId
351+
return 240;
352+
}
353+
343354
////////////////////////////////////////////////////////////////
344355
// `IFaultDisputeGame` impl //
345356
////////////////////////////////////////////////////////////////

‎packages/contracts-bedrock/src/dispute/v2/PermissionedDisputeGameV2.sol‎

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,39 +17,22 @@ import { BadAuth } from "src/dispute/lib/Errors.sol";
1717
/// costs that certain networks may not wish to support. This contract can also be used as a fallback mechanism
1818
/// in case of a failure in the permissionless fault proof system in the stage one release.
1919
contract PermissionedDisputeGameV2 is FaultDisputeGameV2 {
20-
/// @notice The proposer role is allowed to create proposals and participate in the dispute game.
21-
address internal immutable PROPOSER;
22-
23-
/// @notice The challenger role is allowed to participate in the dispute game.
24-
address internal immutable CHALLENGER;
25-
2620
/// @notice Modifier that gates access to the `challenger` and `proposer` roles.
2721
modifier onlyAuthorized() {
28-
if (!(msg.sender == PROPOSER || msg.sender == CHALLENGER)) {
22+
if (!(msg.sender == proposer() || msg.sender == challenger())) {
2923
revert BadAuth();
3024
}
3125
_;
3226
}
3327

3428
/// @notice Semantic version.
35-
/// @custom:semver 2.0.0
29+
/// @custom:semver 2.1.0
3630
function version() public pure override returns (string memory) {
37-
return "2.0.0";
31+
return "2.1.0";
3832
}
3933

4034
/// @param _params Parameters for creating a new FaultDisputeGame.
41-
/// @param _proposer Address that is allowed to create instances of this contract.
42-
/// @param _challenger Address that is allowed to challenge instances of this contract.
43-
constructor(
44-
GameConstructorParams memory _params,
45-
address _proposer,
46-
address _challenger
47-
)
48-
FaultDisputeGameV2(_params)
49-
{
50-
PROPOSER = _proposer;
51-
CHALLENGER = _challenger;
52-
}
35+
constructor(GameConstructorParams memory _params) FaultDisputeGameV2(_params) { }
5336

5437
/// @inheritdoc FaultDisputeGameV2
5538
function step(
@@ -86,24 +69,31 @@ contract PermissionedDisputeGameV2 is FaultDisputeGameV2 {
8669

8770
/// @notice Initializes the contract.
8871
function initialize() public payable override {
72+
super.initialize();
73+
8974
// The creator of the dispute game must be the proposer EOA.
90-
if (tx.origin != PROPOSER) revert BadAuth();
75+
if (tx.origin != proposer()) revert BadAuth();
76+
}
9177

92-
// Fallthrough initialization.
93-
super.initialize();
78+
function immutableArgsByteCount() internal pure override returns (uint256) {
79+
// Extend expected data length to account for proposer and challenger addresses
80+
// - 20 bytes: proposer address
81+
// - 20 bytes: challenger address
82+
return super.immutableArgsByteCount() + 40;
9483
}
9584

9685
////////////////////////////////////////////////////////////////
9786
// IMMUTABLE GETTERS //
9887
////////////////////////////////////////////////////////////////
9988

100-
/// @notice Returns the proposer address.
101-
function proposer() external view returns (address proposer_) {
102-
proposer_ = PROPOSER;
89+
/// @notice Returns the proposer address. The proposer role is allowed to create proposals and participate in the
90+
/// dispute game.
91+
function proposer() public pure returns (address proposer_) {
92+
proposer_ = _getArgAddress(super.immutableArgsByteCount());
10393
}
10494

105-
/// @notice Returns the challenger address.
106-
function challenger() external view returns (address challenger_) {
107-
challenger_ = CHALLENGER;
95+
/// @notice Returns the challenger address. The challenger role is allowed to participate in the dispute game.
96+
function challenger() public pure returns (address challenger_) {
97+
challenger_ = _getArgAddress(super.immutableArgsByteCount() + 20);
10898
}
10999
}

‎packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol‎

Lines changed: 71 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,17 @@ contract DisputeGameFactory_TestInit is CommonTest {
129129
params_ = abi.decode(args, (ISuperFaultDisputeGame.GameConstructorParams));
130130
}
131131

132+
function _setGame(address _gameImpl, GameType _gameType) internal {
133+
_setGame(_gameImpl, _gameType, false, "");
134+
}
135+
132136
function _setGame(address _gameImpl, GameType _gameType, bytes memory _implArgs) internal {
137+
_setGame(_gameImpl, _gameType, true, _implArgs);
138+
}
139+
140+
function _setGame(address _gameImpl, GameType _gameType, bool _hasImplArgs, bytes memory _implArgs) internal {
133141
vm.startPrank(disputeGameFactory.owner());
134-
if (_implArgs.length > 0) {
142+
if (_hasImplArgs) {
135143
disputeGameFactory.setImplementation(_gameType, IDisputeGame(_gameImpl), _implArgs);
136144
} else {
137145
disputeGameFactory.setImplementation(_gameType, IDisputeGame(_gameImpl));
@@ -157,7 +165,7 @@ contract DisputeGameFactory_TestInit is CommonTest {
157165
)
158166
});
159167

160-
_setGame(gameImpl_, GameTypes.SUPER_CANNON, "");
168+
_setGame(gameImpl_, GameTypes.SUPER_CANNON);
161169
}
162170

163171
/// @notice Sets up a super permissioned game implementation
@@ -185,7 +193,7 @@ contract DisputeGameFactory_TestInit is CommonTest {
185193
)
186194
});
187195

188-
_setGame(gameImpl_, GameTypes.SUPER_PERMISSIONED_CANNON, "");
196+
_setGame(gameImpl_, GameTypes.SUPER_PERMISSIONED_CANNON);
189197
}
190198

191199
/// @notice Sets up a fault game implementation
@@ -203,32 +211,44 @@ contract DisputeGameFactory_TestInit is CommonTest {
203211
)
204212
});
205213

206-
_setGame(gameImpl_, GameTypes.CANNON, "");
214+
_setGame(gameImpl_, GameTypes.CANNON);
215+
}
216+
217+
/// @notice Sets up immutable data for fault game v2 implementation
218+
function getFaultDisputeGameV2ImmutableArgs(Claim _absolutePrestate)
219+
internal
220+
returns (bytes memory immutableArgs_, AlphabetVM vm_, IPreimageOracle preimageOracle_)
221+
{
222+
(vm_, preimageOracle_) = _createVM(_absolutePrestate);
223+
// Encode the implementation args for CWIA (tightly packed)
224+
immutableArgs_ = abi.encodePacked(
225+
_absolutePrestate, // 32 bytes
226+
vm_, // 20 bytes
227+
anchorStateRegistry, // 20 bytes
228+
delayedWeth, // 20 bytes
229+
l2ChainId // 32 bytes (l2ChainId)
230+
);
207231
}
208232

209233
/// @notice Sets up a fault game v2 implementation
210234
function setupFaultDisputeGameV2(Claim _absolutePrestate)
211235
internal
212236
returns (address gameImpl_, AlphabetVM vm_, IPreimageOracle preimageOracle_)
213237
{
214-
(vm_, preimageOracle_) = _createVM(_absolutePrestate);
238+
bytes memory immutableArgs;
239+
(immutableArgs, vm_, preimageOracle_) = getFaultDisputeGameV2ImmutableArgs(_absolutePrestate);
240+
gameImpl_ = setupFaultDisputeGameV2(immutableArgs);
241+
}
242+
243+
function setupFaultDisputeGameV2(bytes memory immutableArgs) internal returns (address gameImpl_) {
215244
gameImpl_ = DeployUtils.create1({
216245
_name: "FaultDisputeGameV2",
217246
_args: DeployUtils.encodeConstructor(
218247
abi.encodeCall(IFaultDisputeGameV2.__constructor__, (_getGameConstructorParamsV2(GameTypes.CANNON)))
219248
)
220249
});
221250

222-
// Encode the implementation args for CWIA (tightly packed)
223-
bytes memory implArgs = abi.encodePacked(
224-
_absolutePrestate, // 32 bytes
225-
vm_, // 20 bytes
226-
anchorStateRegistry, // 20 bytes
227-
delayedWeth, // 20 bytes
228-
l2ChainId // 32 bytes (l2ChainId)
229-
);
230-
231-
_setGame(gameImpl_, GameTypes.CANNON, implArgs);
251+
_setGame(gameImpl_, GameTypes.CANNON, immutableArgs);
232252
}
233253

234254
function setupPermissionedDisputeGame(
@@ -254,7 +274,7 @@ contract DisputeGameFactory_TestInit is CommonTest {
254274
)
255275
});
256276

257-
_setGame(gameImpl_, GameTypes.PERMISSIONED_CANNON, "");
277+
_setGame(gameImpl_, GameTypes.PERMISSIONED_CANNON);
258278
}
259279

260280
function changeClaimStatus(Claim _claim, VMStatus _status) public pure returns (Claim out_) {
@@ -263,6 +283,30 @@ contract DisputeGameFactory_TestInit is CommonTest {
263283
}
264284
}
265285

286+
/// @notice Sets up immutable args for PDG v2 implementation
287+
function getPermissionedDisputeGameV2ImmutableArgs(
288+
Claim _absolutePrestate,
289+
address _proposer,
290+
address _challenger
291+
)
292+
internal
293+
returns (bytes memory implArgs_, AlphabetVM vm_, IPreimageOracle preimageOracle_)
294+
{
295+
(vm_, preimageOracle_) = _createVM(_absolutePrestate);
296+
297+
// Encode the implementation args for CWIA (tightly packed)
298+
implArgs_ = abi.encodePacked(
299+
_absolutePrestate, // 32 bytes
300+
vm_, // 20 bytes
301+
anchorStateRegistry, // 20 bytes
302+
delayedWeth, // 20 bytes
303+
l2ChainId, // 32 bytes (l2ChainId),
304+
_proposer, // 20 bytes
305+
_challenger // 20 bytes
306+
);
307+
}
308+
309+
/// @notice Deploys PDG v2 implementation and sets it on the DGF
266310
function setupPermissionedDisputeGameV2(
267311
Claim _absolutePrestate,
268312
address _proposer,
@@ -271,27 +315,25 @@ contract DisputeGameFactory_TestInit is CommonTest {
271315
internal
272316
returns (address gameImpl_, AlphabetVM vm_, IPreimageOracle preimageOracle_)
273317
{
274-
(vm_, preimageOracle_) = _createVM(_absolutePrestate);
318+
bytes memory implArgs;
319+
(implArgs, vm_, preimageOracle_) =
320+
getPermissionedDisputeGameV2ImmutableArgs(_absolutePrestate, _proposer, _challenger);
321+
322+
gameImpl_ = setupPermissionedDisputeGameV2(implArgs);
323+
}
324+
325+
/// @notice Deploys PDG v2 implementation and sets it on the DGF
326+
function setupPermissionedDisputeGameV2(bytes memory _implArgs) internal returns (address gameImpl_) {
275327
gameImpl_ = DeployUtils.create1({
276328
_name: "PermissionedDisputeGameV2",
277329
_args: DeployUtils.encodeConstructor(
278330
abi.encodeCall(
279-
IPermissionedDisputeGameV2.__constructor__,
280-
(_getGameConstructorParamsV2(GameTypes.PERMISSIONED_CANNON), _proposer, _challenger)
331+
IPermissionedDisputeGameV2.__constructor__, (_getGameConstructorParamsV2(GameTypes.PERMISSIONED_CANNON))
281332
)
282333
)
283334
});
284335

285-
// Encode the implementation args for CWIA (tightly packed)
286-
bytes memory implArgs = abi.encodePacked(
287-
_absolutePrestate, // 32 bytes
288-
vm_, // 20 bytes
289-
anchorStateRegistry, // 20 bytes
290-
delayedWeth, // 20 bytes
291-
l2ChainId // 32 bytes (l2ChainId)
292-
);
293-
294-
_setGame(gameImpl_, GameTypes.PERMISSIONED_CANNON, implArgs);
336+
_setGame(gameImpl_, GameTypes.PERMISSIONED_CANNON, _implArgs);
295337
}
296338
}
297339

0 commit comments

Comments
 (0)