Commit 379cdc6d authored by mergify[bot]'s avatar mergify[bot] Committed by GitHub

Merge branch 'develop' into jm/ctb-style

parents fc2c1501 f3f2e2ca
---
'@eth-optimism/message-relayer': major
'@eth-optimism/fault-detector': major
'@eth-optimism/sdk': major
---
......
---
'@eth-optimism/fault-detector': patch
---
Fix false error to warning
......@@ -24,6 +24,7 @@ on:
- proxyd
- indexer
- fault-detector
- ci-builder
prerelease:
description: Increment major/minor/patch as prerelease?
required: false
......
......@@ -31,7 +31,9 @@ bindings: l1block-bindings \
l1-fee-vault-bindings \
basefee-vault-bindings \
legacy-erc20-eth-bindings \
dispute-game-factory-bindings
dispute-game-factory-bindings \
standard-bridge-bindings \
cross-domain-messenger-bindings
version:
forge --version
......@@ -126,6 +128,12 @@ l1-blocknumber-bindings: compile
dispute-game-factory-bindings: compile
./gen_bindings.sh contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory $(pkg)
standard-bridge-bindings: compile
./gen_bindings.sh contracts/universal/StandardBridge.sol:StandardBridge $(pkg)
cross-domain-messenger-bindings: compile
./gen_bindings.sh contracts/universal/CrossDomainMessenger.sol:CrossDomainMessenger $(pkg)
more:
go run ./gen/main.go \
-artifacts ../packages/contracts-bedrock/artifacts \
......
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
......@@ -6,7 +6,7 @@ DOCKER_REPO=$1
GIT_TAG=$2
GIT_SHA=$3
IMAGE_NAME=$(echo "$GIT_TAG" | grep -Eow '^(fault-detector|proxyd|indexer|op-[a-z0-9\-]*)' || true)
IMAGE_NAME=$(echo "$GIT_TAG" | grep -Eow '^(ci-builder|fault-detector|proxyd|indexer|op-[a-z0-9\-]*)' || true)
if [ -z "$IMAGE_NAME" ]; then
echo "image name could not be parsed from git tag '$GIT_TAG'"
exit 1
......
......@@ -15,7 +15,8 @@ MIN_VERSIONS = {
'op-proposer': '0.10.14',
'proxyd': '3.16.0',
'indexer': '0.5.0',
'fault-detector': '0.6.3'
'fault-detector': '0.6.3',
'ci-builder': '0.6.0'
}
VALID_BUMPS = ('major', 'minor', 'patch', 'prerelease', 'finalize-prerelease')
......
......@@ -27,11 +27,11 @@ CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10597)
CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34883)
DeployerWhitelist_Test:test_owner_succeeds() (gas: 7582)
DeployerWhitelist_Test:test_storageSlots_succeeds() (gas: 33395)
DisputeGameFactory_Test:test_owner_succeeds() (gas: 12610)
DisputeGameFactory_Test:test_setImplementation_notOwner_reverts() (gas: 16099)
DisputeGameFactory_Test:test_setImplementation_succeeds() (gas: 44302)
DisputeGameFactory_Test:test_transferOwnership_notOwner_reverts() (gas: 15974)
DisputeGameFactory_Test:test_transferOwnership_succeeds() (gas: 18694)
DisputeGameFactory_Owner_Test:test_owner_succeeds() (gas: 12559)
DisputeGameFactory_SetImplementation_Test:test_setImplementation_notOwner_reverts() (gas: 16042)
DisputeGameFactory_SetImplementation_Test:test_setImplementation_succeeds() (gas: 44243)
DisputeGameFactory_TransferOwnership_Test:test_transferOwnership_notOwner_reverts() (gas: 15950)
DisputeGameFactory_TransferOwnership_Test:test_transferOwnership_succeeds() (gas: 18642)
FaultDisputeGame_Test:test_clockTimeExceeded_reverts() (gas: 26413)
FaultDisputeGame_Test:test_defendRoot_reverts() (gas: 13258)
FaultDisputeGame_Test:test_duplicateClaim_reverts() (gas: 103259)
......
......@@ -253,13 +253,13 @@
➡ contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory
=======================
| Name | Type | Slot | Offset | Bytes | Contract |
|-----------------|--------------------------------------------|------|--------|-------|-------------------------------------------------------------|
| _initialized | uint8 | 0 | 0 | 1 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| _initializing | bool | 0 | 1 | 1 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| __gap | uint256[50] | 1 | 0 | 1600 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| _owner | address | 51 | 0 | 20 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| __gap | uint256[49] | 52 | 0 | 1568 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| gameImpls | mapping(GameType => contract IDisputeGame) | 101 | 0 | 32 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| disputeGames | mapping(Hash => contract IDisputeGame) | 102 | 0 | 32 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| disputeGameList | contract IDisputeGame[] | 103 | 0 | 32 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| Name | Type | Slot | Offset | Bytes | Contract |
|------------------|--------------------------------------------|------|--------|-------|-------------------------------------------------------------|
| _initialized | uint8 | 0 | 0 | 1 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| _initializing | bool | 0 | 1 | 1 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| __gap | uint256[50] | 1 | 0 | 1600 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| _owner | address | 51 | 0 | 20 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| __gap | uint256[49] | 52 | 0 | 1568 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| gameImpls | mapping(GameType => contract IDisputeGame) | 101 | 0 | 32 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| _disputeGames | mapping(Hash => GameId) | 102 | 0 | 32 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
| _disputeGameList | GameId[] | 103 | 0 | 32 | contracts/dispute/DisputeGameFactory.sol:DisputeGameFactory |
......@@ -15,7 +15,11 @@ import { IVersioned } from "./interfaces/IVersioned.sol";
/**
* @title DisputeGameFactory
* @notice A factory contract for creating `IDisputeGame` contracts.
* @notice A factory contract for creating `IDisputeGame` contracts. All created dispute games
* are stored in both a mapping and an append only array. The timestamp of the creation
* time of the dispute game is packed tightly into the storage slot with the address of
* the dispute game. This is to make offchain discoverability of playable dispute games
* easier.
*/
contract DisputeGameFactory is OwnableUpgradeable, IDisputeGameFactory, IVersioned {
/**
......@@ -29,18 +33,18 @@ contract DisputeGameFactory is OwnableUpgradeable, IDisputeGameFactory, IVersion
mapping(GameType => IDisputeGame) public gameImpls;
/**
* @notice Mapping of a hash of `gameType . rootClaim . extraData` to
* @notice Mapping of a hash of `gameType || rootClaim || extraData` to
* the deployed `IDisputeGame` clone.
* @dev Note: `.` denotes concatenation.
* @dev Note: `||` denotes concatenation.
*/
mapping(Hash => IDisputeGame) internal disputeGames;
mapping(Hash => GameId) internal _disputeGames;
/**
* @notice An append-only array of disputeGames that have been created.
* @dev This accessor is used by offchain game solvers to efficiently
* track dispute games
*/
IDisputeGame[] public disputeGameList;
GameId[] internal _disputeGameList;
/**
* @notice Constructs a new DisputeGameFactory contract.
......@@ -60,27 +64,46 @@ contract DisputeGameFactory is OwnableUpgradeable, IDisputeGameFactory, IVersion
/**
* @inheritdoc IVersioned
* @custom:semver 0.0.2
*/
function version() external pure returns (string memory) {
return "0.0.1";
return "0.0.2";
}
/**
* @inheritdoc IDisputeGameFactory
*/
function gameCount() external view returns (uint256 _gameCount) {
_gameCount = disputeGameList.length;
function gameCount() external view returns (uint256 gameCount_) {
gameCount_ = _disputeGameList.length;
}
/**
* @inheritdoc IDisputeGameFactory
*/
function games(
GameType gameType,
Claim rootClaim,
bytes calldata extraData
) external view returns (IDisputeGame _proxy) {
return disputeGames[getGameUUID(gameType, rootClaim, extraData)];
GameType _gameType,
Claim _rootClaim,
bytes calldata _extraData
) external view returns (IDisputeGame proxy_, uint256 timestamp_) {
Hash uuid = getGameUUID(_gameType, _rootClaim, _extraData);
GameId slot = _disputeGames[uuid];
(address addr, uint256 timestamp) = _unpackSlot(slot);
proxy_ = IDisputeGame(addr);
timestamp_ = timestamp;
}
/**
* @inheritdoc IDisputeGameFactory
*/
function gameAtIndex(uint256 _index)
external
view
returns (IDisputeGame proxy_, uint256 timestamp_)
{
GameId slot = _disputeGameList[_index];
(address addr, uint256 timestamp) = _unpackSlot(slot);
proxy_ = IDisputeGame(addr);
timestamp_ = timestamp;
}
/**
......@@ -107,13 +130,15 @@ contract DisputeGameFactory is OwnableUpgradeable, IDisputeGameFactory, IVersion
Hash uuid = getGameUUID(gameType, rootClaim, extraData);
// If a dispute game with the same UUID already exists, revert.
if (address(disputeGames[uuid]) != address(0)) {
if (GameId.unwrap(_disputeGames[uuid]) != bytes32(0)) {
revert GameAlreadyExists(uuid);
}
GameId slot = _packSlot(address(proxy), block.timestamp);
// Store the dispute game in the mapping & emit the `DisputeGameCreated` event.
disputeGames[uuid] = proxy;
disputeGameList.push(proxy);
_disputeGames[uuid] = slot;
_disputeGameList.push(slot);
emit DisputeGameCreated(address(proxy), gameType, rootClaim);
}
......@@ -163,4 +188,24 @@ contract DisputeGameFactory is OwnableUpgradeable, IDisputeGameFactory, IVersion
gameImpls[gameType] = impl;
emit ImplementationSet(address(impl), gameType);
}
/**
* @dev Packs an address and a uint256 into a single bytes32 slot. This
* is only safe for up to uint96 values.
*/
function _packSlot(address _addr, uint256 _num) internal pure returns (GameId slot_) {
assembly {
slot_ := or(shl(0xa0, _num), _addr)
}
}
/**
* @dev Unpacks an address and a uint256 from a single bytes32 slot.
*/
function _unpackSlot(GameId _slot) internal pure returns (address addr_, uint256 num_) {
assembly {
addr_ := and(_slot, 0xffffffffffffffffffffffffffffffffffffffff)
num_ := shr(0xa0, _slot)
}
}
}
......@@ -44,12 +44,26 @@ interface IDisputeGameFactory {
* @param extraData Any extra data that should be provided to the created dispute game.
* @return _proxy The clone of the `DisputeGame` created with the given parameters.
* Returns `address(0)` if nonexistent.
* @return _timestamp The timestamp of the creation of the dispute game.
*/
function games(
GameType gameType,
Claim rootClaim,
bytes calldata extraData
) external view returns (IDisputeGame _proxy);
) external view returns (IDisputeGame _proxy, uint256 _timestamp);
/**
* @notice `gameAtIndex` returns the dispute game contract address and its creation timestamp
* at the given index. Each created dispute game increments the underlying index.
* @param _index The index of the dispute game.
* @return _proxy The clone of the `DisputeGame` created with the given parameters.
* Returns `address(0)` if nonexistent.
* @return _timestamp The timestamp of the creation of the dispute game.
*/
function gameAtIndex(uint256 _index)
external
view
returns (IDisputeGame _proxy, uint256 _timestamp);
/**
* @notice `gameImpls` is a mapping that maps `GameType`s to their respective
......
......@@ -33,6 +33,18 @@ type Timestamp is uint64;
*/
type Duration is uint64;
/**
* @notice A `GameId` represents a packed 12 byte timestamp and a 20 byte address.
* @dev The packed layout of this type is as follows:
* ┌────────────┬────────────────┐
* │ Bits │ Value │
* ├────────────┼────────────────┤
* │ [0, 96) │ Timestamp │
* │ [96, 256) │ Address │
* └────────────┴────────────────┘
*/
type GameId is bytes32;
/**
* @notice A `Clock` represents a packed `Duration` and `Timestamp`
* @dev The packed layout of this type is as follows:
......
......@@ -9,8 +9,9 @@ import { DisputeGameFactory } from "../dispute/DisputeGameFactory.sol";
import { IDisputeGame } from "../dispute/interfaces/IDisputeGame.sol";
import { Proxy } from "../universal/Proxy.sol";
contract DisputeGameFactory_Initializer is Test {
contract DisputeGameFactory_Init is Test {
DisputeGameFactory factory;
FakeClone fakeClone;
event DisputeGameCreated(
address indexed disputeProxy,
......@@ -30,17 +31,12 @@ contract DisputeGameFactory_Initializer is Test {
});
factory = DisputeGameFactory(address(proxy));
vm.label(address(factory), "DisputeGameFactoryProxy");
}
}
contract DisputeGameFactory_Test is DisputeGameFactory_Initializer {
FakeClone fakeClone;
function setUp() public override {
super.setUp();
fakeClone = new FakeClone();
}
}
contract DisputeGameFactory_Create_Test is DisputeGameFactory_Init {
/**
* @dev Tests that the `create` function succeeds when creating a new dispute game
* with a `GameType` that has an implementation set.
......@@ -62,10 +58,16 @@ contract DisputeGameFactory_Test is DisputeGameFactory_Initializer {
emit DisputeGameCreated(address(0), gt, rootClaim);
IDisputeGame proxy = factory.create(gt, rootClaim, extraData);
(IDisputeGame game, uint256 timestamp) = factory.games(gt, rootClaim, extraData);
// Ensure that the dispute game was assigned to the `disputeGames` mapping.
assertEq(address(factory.games(gt, rootClaim, extraData)), address(proxy));
assertEq(address(game), address(proxy));
assertEq(timestamp, block.timestamp);
assertEq(factory.gameCount(), 1);
assertEq(address(factory.disputeGameList(0)), address(proxy));
(IDisputeGame game2, uint256 timestamp2) = factory.gameAtIndex(0);
assertEq(address(game2), address(proxy));
assertEq(timestamp2, block.timestamp);
}
/**
......@@ -105,8 +107,10 @@ contract DisputeGameFactory_Test is DisputeGameFactory_Initializer {
emit DisputeGameCreated(address(0), gt, rootClaim);
IDisputeGame proxy = factory.create(gt, rootClaim, extraData);
(IDisputeGame game, uint256 timestamp) = factory.games(gt, rootClaim, extraData);
// Ensure that the dispute game was assigned to the `disputeGames` mapping.
assertEq(address(factory.games(gt, rootClaim, extraData)), address(proxy));
assertEq(address(game), address(proxy));
assertEq(timestamp, block.timestamp);
// Ensure that the `create` function reverts when called with parameters that would result in the same UUID.
vm.expectRevert(
......@@ -117,7 +121,9 @@ contract DisputeGameFactory_Test is DisputeGameFactory_Initializer {
);
factory.create(gt, rootClaim, extraData);
}
}
contract DisputeGameFactory_SetImplementation_Test is DisputeGameFactory_Init {
/**
* @dev Tests that the `setImplementation` function properly sets the implementation for a given `GameType`.
*/
......@@ -144,7 +150,9 @@ contract DisputeGameFactory_Test is DisputeGameFactory_Initializer {
vm.expectRevert("Ownable: caller is not the owner");
factory.setImplementation(GameTypes.FAULT, IDisputeGame(address(1)));
}
}
contract DisputeGameFactory_GetGameUUID_Test is DisputeGameFactory_Init {
/**
* @dev Tests that the `getGameUUID` function returns the correct hash when comparing
* against the keccak256 hash of the abi-encoded parameters.
......@@ -162,14 +170,18 @@ contract DisputeGameFactory_Test is DisputeGameFactory_Initializer {
keccak256(abi.encode(gt, rootClaim, extraData))
);
}
}
contract DisputeGameFactory_Owner_Test is DisputeGameFactory_Init {
/**
* @dev Tests that the `owner` function returns the correct address after deployment.
*/
function test_owner_succeeds() public {
assertEq(factory.owner(), address(this));
}
}
contract DisputeGameFactory_TransferOwnership_Test is DisputeGameFactory_Init {
/**
* @dev Tests that the `transferOwnership` function succeeds when called by the owner.
*/
......@@ -188,6 +200,43 @@ contract DisputeGameFactory_Test is DisputeGameFactory_Initializer {
}
}
/**
* @title PackingTester
* @notice Exposes the internal packing functions so that they can be fuzzed
* in a roundtrip manner.
*/
contract PackingTester is DisputeGameFactory {
function packSlot(address _addr, uint256 _num) external pure returns (GameId) {
return _packSlot(_addr, _num);
}
function unpackSlot(GameId _slot) external pure returns (address, uint256) {
return _unpackSlot(_slot);
}
}
/**
* @title DisputeGameFactory_PackSlot_Test
* @notice Fuzzes the PackingTester contract
*/
contract DisputeGameFactory_PackSlot_Test is Test {
PackingTester tester;
function setUp() public {
tester = new PackingTester();
}
/**
* @dev Tests that the `packSlot` and `unpackSlot` functions roundtrip correctly.
*/
function testFuzz_packSlot_succeeds(address _addr, uint96 _num) public {
GameId slot = tester.packSlot(_addr, uint256(_num));
(address addr, uint256 num) = tester.unpackSlot(slot);
assertEq(addr, _addr);
assertEq(num, _num);
}
}
/**
* @dev A fake clone used for testing the `DisputeGameFactory` contract's `create` function.
*/
......
......@@ -2,7 +2,7 @@
pragma solidity ^0.8.15;
import { Test } from "forge-std/Test.sol";
import { DisputeGameFactory_Initializer } from "./DisputeGameFactory.t.sol";
import { DisputeGameFactory_Init } from "./DisputeGameFactory.t.sol";
import { DisputeGameFactory } from "../dispute/DisputeGameFactory.sol";
import { FaultDisputeGame } from "../dispute/FaultDisputeGame.sol";
......@@ -11,7 +11,7 @@ import "../libraries/DisputeErrors.sol";
import { LibClock } from "../dispute/lib/LibClock.sol";
import { LibPosition } from "../dispute/lib/LibPosition.sol";
contract FaultDisputeGame_Test is DisputeGameFactory_Initializer {
contract FaultDisputeGame_Test is DisputeGameFactory_Init {
/**
* @dev The root claim of the game.
*/
......
......@@ -66,7 +66,7 @@ export const updateOracleCache = async <TSubmissionEventArgs>(
latestBlock: endBlock,
})
let failures = 0
let failures = []
let currentBlock = cache.highestBlock + 1
let step = endBlock - currentBlock
while (currentBlock < endBlock) {
......@@ -98,7 +98,7 @@ export const updateOracleCache = async <TSubmissionEventArgs>(
currentBlock += step
step = Math.ceil(step * 2)
} catch (err) {
logger?.error('error fetching events', {
logger?.warn('error fetching events', {
err,
node: 'l1',
section: 'getLogs',
......@@ -110,14 +110,14 @@ export const updateOracleCache = async <TSubmissionEventArgs>(
// When the step gets down to zero, we're pretty much guaranteed that range size isn't the
// problem. If we get three failures like this in a row then we should just give up.
if (step === 0) {
failures++
failures.push(err)
} else {
failures = 0
failures = []
}
// We've failed 3 times in a row, we're probably stuck.
if (failures >= 3) {
logger?.fatal('unable to fetch oracle events', { err })
// We've failed 5 times in a row, we're probably stuck.
if (failures.length >= 5) {
logger?.fatal('unable to fetch oracle events', { errors: failures })
throw new Error('failed to update event cache')
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment