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: add game type event #14196

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol";
import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.sol";
import { IAddressManager } from "interfaces/legacy/IAddressManager.sol";
import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol";
import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol";
import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol";
import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol";
import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol";
import { IPermissionedDisputeGame } from "interfaces/dispute/IPermissionedDisputeGame.sol";
Expand Down Expand Up @@ -166,6 +166,13 @@ interface IOPContractsManager {
/// @param upgrader Address that initiated the upgrade
event Upgraded(uint256 indexed l2ChainId, ISystemConfig indexed systemConfig, address indexed upgrader);

/// @notice Emitted when a new game type is added to a chain
/// @param l2ChainId Chain ID of the chain
/// @param gameType Type of the game being added
/// @param newDisputeGame Address of the deployed dispute game
/// @param oldDisputeGame Address of the old dispute game
event GameTypeAdded(uint256 indexed l2ChainId, GameType indexed gameType, IDisputeGame newDisputeGame, IDisputeGame oldDisputeGame);

// -------- Errors --------

error BytesArrayTooLong();
Expand Down
11 changes: 11 additions & 0 deletions packages/contracts-bedrock/interfaces/L1/IOPPrestateUpdater.sol
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

// Libraries
import { GameType } from "src/dispute/lib/Types.sol";

// Interfaces
import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol";
import { ISuperchainConfig } from "interfaces/L1/ISuperchainConfig.sol";
import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol";
import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol";
import { IOPContractsManager } from "interfaces/L1/IOPContractsManager.sol";
import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol";

interface IOPPrestateUpdater {
// -------- Constants and Variables --------
Expand Down Expand Up @@ -39,6 +43,13 @@ interface IOPPrestateUpdater {
/// @param upgrader Address that initiated the upgrade
event Upgraded(uint256 indexed l2ChainId, ISystemConfig indexed systemConfig, address indexed upgrader);

/// @notice Emitted when a new game type is added to a chain
/// @param l2ChainId Chain ID of the chain
/// @param gameType Type of the game being added
/// @param newDisputeGame Address of the deployed dispute game
/// @param oldDisputeGame Address of the old dispute game
event GameTypeAdded(uint256 indexed l2ChainId, GameType indexed gameType, IDisputeGame newDisputeGame, IDisputeGame oldDisputeGame);

// -------- Errors --------

error BytesArrayTooLong();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,37 @@
"name": "Deployed",
"type": "event"
},
{
"anonymous": false,
"inputs": [
{
"indexed": true,
"internalType": "uint256",
"name": "l2ChainId",
"type": "uint256"
},
{
"indexed": true,
"internalType": "GameType",
"name": "gameType",
"type": "uint32"
},
{
"indexed": false,
"internalType": "contract IDisputeGame",
"name": "newDisputeGame",
"type": "address"
},
{
"indexed": false,
"internalType": "contract IDisputeGame",
"name": "oldDisputeGame",
"type": "address"
}
],
"name": "GameTypeAdded",
"type": "event"
},
{
"anonymous": false,
"inputs": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,37 @@
"name": "Deployed",
"type": "event"
},
{
"anonymous": false,
"inputs": [
{
"indexed": true,
"internalType": "uint256",
"name": "l2ChainId",
"type": "uint256"
},
{
"indexed": true,
"internalType": "GameType",
"name": "gameType",
"type": "uint32"
},
{
"indexed": false,
"internalType": "contract IDisputeGame",
"name": "newDisputeGame",
"type": "address"
},
{
"indexed": false,
"internalType": "contract IDisputeGame",
"name": "oldDisputeGame",
"type": "address"
}
],
"name": "GameTypeAdded",
"type": "event"
},
{
"anonymous": false,
"inputs": [
Expand Down
31 changes: 31 additions & 0 deletions packages/contracts-bedrock/snapshots/abi/OPPrestateUpdater.json
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,37 @@
"name": "Deployed",
"type": "event"
},
{
"anonymous": false,
"inputs": [
{
"indexed": true,
"internalType": "uint256",
"name": "l2ChainId",
"type": "uint256"
},
{
"indexed": true,
"internalType": "GameType",
"name": "gameType",
"type": "uint32"
},
{
"indexed": false,
"internalType": "contract IDisputeGame",
"name": "newDisputeGame",
"type": "address"
},
{
"indexed": false,
"internalType": "contract IDisputeGame",
"name": "oldDisputeGame",
"type": "address"
}
],
"name": "GameTypeAdded",
"type": "event"
},
{
"anonymous": false,
"inputs": [
Expand Down
10 changes: 5 additions & 5 deletions packages/contracts-bedrock/snapshots/semver-lock.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semver lock check failing since OPPU needs a semver change

Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@
"sourceCodeHash": "0xf9ba98657dc235355146e381b654fe3ed766feb7cd87636ec0c9d4c6dd3e1973"
},
"src/L1/OPContractsManager.sol": {
"initCodeHash": "0x997aa1eb8c9323ece8942344d0d27bac71f6a4f4ccf7dac7b9ab7232f90bed93",
"sourceCodeHash": "0xc9214c3ac97f51d3e3435500a987346518eaa04cf2cb672827d295b8bd226a61"
"initCodeHash": "0x436c3c76531b8c1e462badaa60933ed61f510343f16cf847b1251320225f2eef",
"sourceCodeHash": "0x55b66f495da8a316d85e51e0e685f5395f5f8aa3ab6c1de0ed6d543745ddbfeb"
},
"src/L1/OPContractsManagerInterop.sol": {
"initCodeHash": "0xef5ec182708fa5c29cf2f42a6109d748cac653981f8a773ce1d6453aff863362",
"sourceCodeHash": "0xf9e820880250d9a1889d07e87f3dfc2c86397cd743f6e6ac9101e621b90f912d"
"initCodeHash": "0xf993868ede15c1cfb6be09be935617ff0c7263cac923616cdb846d9beea5591e",
"sourceCodeHash": "0xa5f8b31a02896d2e8ba6c5b64a8a7c1d78bd95af998184cdd11d66760fdedabd"
},
"src/L1/OPPrestateUpdater.sol": {
"initCodeHash": "0xcbf87fdc1409afb25e7c8a6a01edc56d4cbe4913b6bbc4922a0c7a441afd69c0",
"initCodeHash": "0x50894a2925c950623b4e3a37a71ea67496b8b47c7b53213b6066a63b2f5af38b",
"sourceCodeHash": "0x8a4eb72a9c0d5c73542239b6dfe25c7dfc29d3b26a1c0a8a9f2f5b16c01c9fd6"
},
"src/L1/OptimismPortal2.sol": {
Expand Down
48 changes: 38 additions & 10 deletions packages/contracts-bedrock/src/L1/OPContractsManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ contract OPContractsManager is ISemver {

// -------- Constants and Variables --------

/// @custom:semver 1.2.2
/// @custom:semver 1.3.0
function version() public pure virtual returns (string memory) {
return "1.2.2";
return "1.3.0";
}

/// @notice Address of the SuperchainConfig contract shared by all chains.
Expand Down Expand Up @@ -198,6 +198,15 @@ contract OPContractsManager is ISemver {
/// @param upgrader Address that initiated the upgrade
event Upgraded(uint256 indexed l2ChainId, ISystemConfig indexed systemConfig, address indexed upgrader);

/// @notice Emitted when a new game type is added to a chain
/// @param l2ChainId Chain ID of the chain
/// @param gameType Type of the game being
/// @param newDisputeGame Address of the deployed dispute game
/// @param oldDisputeGame Address of the old dispute game
event GameTypeAdded(
uint256 indexed l2ChainId, GameType indexed gameType, IDisputeGame newDisputeGame, IDisputeGame oldDisputeGame
);
Comment on lines +206 to +208
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this event signature was approved by proofs I've update the event based on proofs input.


// -------- Errors --------

/// @notice Thrown when an address other than the upgrade controller calls the setRC function.
Expand Down Expand Up @@ -623,14 +632,15 @@ contract OPContractsManager is ISemver {
if (lastGameConfig >= gameTypeInt) revert InvalidGameConfigs();
lastGameConfig = gameTypeInt;

// Grab the FDG from the SystemConfig.
IFaultDisputeGame fdg = IFaultDisputeGame(
// Grab the permissioned and fault dispute games from the SystemConfig.
// We keep the FDG type as it reduces casting below.
IFaultDisputeGame pdg = IFaultDisputeGame(
address(
getGameImplementation(getDisputeGameFactory(gameConfig.systemConfig), GameTypes.PERMISSIONED_CANNON)
Comment on lines +636 to 639
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old fdg varname was misleading since this the PERMISSIONED_CANNON game type is being retrieved.

)
);
// Pull out the chain ID.
uint256 l2ChainId = getL2ChainId(fdg);
uint256 l2ChainId = getL2ChainId(pdg);

// Deploy a new DelayedWETH proxy for this game if one hasn't already been specified. Leaving
/// gameConfig.delayedWETH as the zero address will cause a new DelayedWETH to be deployed for this game.
Expand All @@ -650,10 +660,13 @@ contract OPContractsManager is ISemver {
outputs[i].delayedWETH = gameConfig.delayedWETH;
}

// The FDG is only used for the event below, and only if it is being replaced,
// so we declare it here, but only assign it below if needed.
IFaultDisputeGame fdg;

// The below sections are functionally the same. Both deploy a new dispute game. The dispute game type is
// either permissioned or permissionless depending on game config.
if (gameConfig.permissioned) {
IPermissionedDisputeGame pdg = IPermissionedDisputeGame(address(fdg));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this rename is no longer necessary because of the change above.

outputs[i].faultDisputeGame = IFaultDisputeGame(
Blueprint.deployFrom(
bps.permissionedDisputeGame1,
Expand All @@ -669,15 +682,18 @@ contract OPContractsManager is ISemver {
gameConfig.disputeMaxClockDuration,
gameConfig.vm,
outputs[i].delayedWETH,
getAnchorStateRegistry(IFaultDisputeGame(address(pdg))),
getAnchorStateRegistry(pdg),
l2ChainId
),
getProposer(pdg),
getChallenger(pdg)
getProposer(IPermissionedDisputeGame(address(pdg))),
getChallenger(IPermissionedDisputeGame(address(pdg)))
)
)
);
} else {
fdg = IFaultDisputeGame(
address(getGameImplementation(getDisputeGameFactory(gameConfig.systemConfig), GameTypes.CANNON))
);
outputs[i].faultDisputeGame = IFaultDisputeGame(
Blueprint.deployFrom(
bps.permissionlessDisputeGame1,
Expand All @@ -693,7 +709,7 @@ contract OPContractsManager is ISemver {
gameConfig.disputeMaxClockDuration,
gameConfig.vm,
outputs[i].delayedWETH,
getAnchorStateRegistry(fdg),
getAnchorStateRegistry(pdg),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ASR for the PDG and FDG should be the same, so this change doesn't matter in practice, but curious why you didn't keep this as getAnchorStateRegistry(fdg) which feels more semantically clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this changed with the varname change above, so the semantic diff is actually smaller.

But now that we're getting fdg just above, I think that makes sense.

l2ChainId
)
)
Expand All @@ -706,6 +722,18 @@ contract OPContractsManager is ISemver {
IDisputeGameFactory dgf = getDisputeGameFactory(gameConfig.systemConfig);
setDGFImplementation(dgf, gameConfig.disputeGameType, IDisputeGame(address(outputs[i].faultDisputeGame)));
dgf.setInitBond(gameConfig.disputeGameType, gameConfig.initialBond);

if (gameConfig.permissioned) {
// Emit event for the newly added game type with the old permissioned dispute game
emit GameTypeAdded(
Copy link
Contributor

@mds1 mds1 Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, but can we do these event emissions in the above if (gameConfig.permissioned) / else block to simplify control flow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned that here: https://github.com/ethereum-optimism/optimism/pull/14196/files#r1953015117

If you think slightly odd event ordering is OK, we can do that.

l2ChainId, gameConfig.disputeGameType, outputs[i].faultDisputeGame, IDisputeGame(address(pdg))
);
} else {
// Emit event for the newly added game type with the old fault dispute game
emit GameTypeAdded(
l2ChainId, gameConfig.disputeGameType, outputs[i].faultDisputeGame, IDisputeGame(address(fdg))
);
}
Comment on lines +725 to +736
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a tradeoff made here. This approach has the benefit of emitting the GameTypeAdded event at the most accurate point, once all the necessary steps are completed, however it has the downside of requiring an additional branch.

}

return outputs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import { ISystemConfigInterop } from "interfaces/L1/ISystemConfigInterop.sol";
import { IProxyAdmin } from "interfaces/universal/IProxyAdmin.sol";

contract OPContractsManagerInterop is OPContractsManager {
/// @custom:semver +interop.6
/// @custom:semver +interop.7
function version() public pure override returns (string memory) {
return string.concat(super.version(), "+interop.6");
return string.concat(super.version(), "+interop.7");
}

constructor(
Expand Down
15 changes: 15 additions & 0 deletions packages/contracts-bedrock/test/L1/OPContractsManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol";
import { IPreimageOracle } from "interfaces/cannon/IPreimageOracle.sol";
import { IPermissionedDisputeGame } from "interfaces/dispute/IPermissionedDisputeGame.sol";
import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol";
import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol";
import { IDisputeGameFactory } from "interfaces/dispute/IDisputeGameFactory.sol";
import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol";
import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol";
import { IOPContractsManager } from "interfaces/L1/IOPContractsManager.sol";
Expand Down Expand Up @@ -529,6 +531,10 @@ contract OPContractsManager_AddGameType_Test is Test {

IOPContractsManager.DeployOutput internal chainDeployOutput;

event GameTypeAdded(
uint256 indexed l2ChainId, GameType indexed gameType, IDisputeGame newDisputeGame, IDisputeGame oldDisputeGame
);

function setUp() public {
ISuperchainConfig superchainConfigProxy = ISuperchainConfig(makeAddr("superchainConfig"));
IProtocolVersions protocolVersionsProxy = IProtocolVersions(makeAddr("protocolVersions"));
Expand Down Expand Up @@ -700,6 +706,15 @@ contract OPContractsManager_AddGameType_Test is Test {
IOPContractsManager.AddGameInput[] memory inputs = new IOPContractsManager.AddGameInput[](1);
inputs[0] = input;

uint256 l2ChainId = IFaultDisputeGame(
address(IDisputeGameFactory(input.systemConfig.disputeGameFactory()).gameImpls(GameType.wrap(1)))
).l2ChainId();

// Expect the GameTypeAdded event to be emitted.
vm.expectEmit(true, true, false, false, address(this));
maurelian marked this conversation as resolved.
Show resolved Hide resolved
emit GameTypeAdded(
l2ChainId, input.disputeGameType, IDisputeGame(payable(address(0))), IDisputeGame(payable(address(0)))
);
(bool success, bytes memory rawGameOut) =
address(opcm).delegatecall(abi.encodeCall(IOPContractsManager.addGameType, (inputs)));
assertTrue(success, "addGameType failed");
Expand Down