Skip to content

N
nebula
Commit 1a4a80e5 authored by clabby's avatar clabby Committed by GitHub
Browse files
Options

feat(ctb): Key withdrawal proofs by address (#9948) 

* feat(ctb): Key withdrawal proofs by address

* Use recovered public key in ToB tests

* reviews + reverse lookup mapping for proof submitters

* Update natspec
parent 7f3afe38
Hide whitespace changes
Inline Side-by-side
Showing with 343 additions and 55 deletions
op-bindings/bindingspreview/optimismportal2.go
View file @ 1a4a80e5
This source diff could not be displayed because it is too large. You can view the blob instead.
op-bindings/bindingspreview/optimismportal2_more.go
View file @ 1a4a80e5
This source diff could not be displayed because it is too large. You can view the blob instead.
op-e2e/actions/user.go
View file @ 1a4a80e5
......@@ -465,7 +465,7 @@ func (s *CrossLayerUser) getDisputeGame(t Testing, params withdrawals.ProvenWith
wdHash, err := wd.Hash()
require.Nil(t, err)
game, err := portal2.ProvenWithdrawals(&bind.CallOpts{}, wdHash)
game, err := portal2.ProvenWithdrawals(&bind.CallOpts{}, wdHash, s.L1.address)
require.Nil(t, err)
require.NotNil(t, game, "withdrawal should be proven")
......
op-e2e/e2eutils/wait/withdrawals.go
View file @ 1a4a80e5
......@@ -110,7 +110,7 @@ func ForGamePublished(ctx context.Context, client *ethclient.Client, optimismPor
}
// ForWithdrawalCheck waits until the withdrawal check in the portal succeeds.
func ForWithdrawalCheck(ctx context.Context, client *ethclient.Client, withdrawal crossdomain.Withdrawal, optimismPortalAddr common.Address) error {
func ForWithdrawalCheck(ctx context.Context, client *ethclient.Client, withdrawal crossdomain.Withdrawal, optimismPortalAddr common.Address, proofSubmitter common.Address) error {
ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
defer cancel()
opts := &bind.CallOpts{Context: ctx}
......@@ -125,7 +125,7 @@ func ForWithdrawalCheck(ctx context.Context, client *ethclient.Client, withdrawa
return false, fmt.Errorf("hash withdrawal: %w", err)
}
err = portal.CheckWithdrawal(opts, wdHash)
err = portal.CheckWithdrawal(opts, wdHash, proofSubmitter)
return err == nil, nil
})
}
op-e2e/system_tob_test.go
View file @ 1a4a80e5
......@@ -682,7 +682,7 @@ func TestMixedWithdrawalValidity(t *testing.T) {
ctx, withdrawalCancel := context.WithTimeout(context.Background(), 60*time.Duration(cfg.DeployConfig.L1BlockTime)*time.Second)
defer withdrawalCancel()
if e2eutils.UseFPAC() {
err = wait.ForWithdrawalCheck(ctx, l1Client, withdrawal, cfg.L1Deployments.OptimismPortalProxy)
err = wait.ForWithdrawalCheck(ctx, l1Client, withdrawal, cfg.L1Deployments.OptimismPortalProxy, transactor.Account.L1Opts.From)
require.NoError(t, err)
} else {
err = wait.ForFinalizationPeriod(ctx, l1Client, header.Number, cfg.L1Deployments.L2OutputOracleProxy)
......
op-e2e/withdrawal_helper.go
View file @ 1a4a80e5
......@@ -193,7 +193,7 @@ func FinalizeWithdrawal(t *testing.T, cfg SystemConfig, l1Client *ethclient.Clie
wdHash, err := wd.Hash()
require.Nil(t, err)
game, err := portal2.ProvenWithdrawals(&bind.CallOpts{}, wdHash)
game, err := portal2.ProvenWithdrawals(&bind.CallOpts{}, wdHash, opts.From)
require.Nil(t, err)
require.NotNil(t, game, "withdrawal should be proven")
......@@ -220,7 +220,7 @@ func FinalizeWithdrawal(t *testing.T, cfg SystemConfig, l1Client *ethclient.Clie
}
if e2eutils.UseFPAC() {
err := wait.ForWithdrawalCheck(ctx, l1Client, wd, config.L1Deployments.OptimismPortalProxy)
err := wait.ForWithdrawalCheck(ctx, l1Client, wd, config.L1Deployments.OptimismPortalProxy, opts.From)
require.Nil(t, err)
} else {
err := wait.ForFinalizationPeriod(ctx, l1Client, withdrawalProofReceipt.BlockNumber, config.L1Deployments.L2OutputOracleProxy)
......
packages/contracts-bedrock/semver-lock.json
View file @ 1a4a80e5
......@@ -36,8 +36,8 @@
"sourceCodeHash": "0xf549ae16033b63e7cb3e032898a6495e1a13090dc8dd1422f7f650076ae973f8"
},
"src/L1/OptimismPortal2.sol": {
"initCodeHash": "0x3501f67714a63fe67ce1b7530bcaa2593f149f595a702034837006494ee8cb64",
"sourceCodeHash": "0x11451fdf6a9c60331fb7413d946dae556410a64bcf761fe48f82e56812743424"
"initCodeHash": "0x88d44d2c53c9e6dd5b561c200c3bb57ffc0bca58dfa2b386046c332e13b58280",
"sourceCodeHash": "0xd66cfc9b4bfb451d3ba1001b85ae695ff30c042f7ebfe0dcf9f96587559bf81e"
},
"src/L1/ProtocolVersions.sol": {
"initCodeHash": "0x72cd467e8bcf019c02675d72ab762e088bcc9cc0f1a4e9f587fa4589f7fdd1b8",
......
packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json
View file @ 1a4a80e5
......@@ -69,6 +69,11 @@
"internalType": "bytes32",
"name": "_withdrawalHash",
"type": "bytes32"
},
{
"internalType": "address",
"name": "_proofSubmitter",
"type": "address"
}
],
"name": "checkWithdrawal",
......@@ -206,6 +211,56 @@
"stateMutability": "nonpayable",
"type": "function"
},
{
"inputs": [
{
"components": [
{
"internalType": "uint256",
"name": "nonce",
"type": "uint256"
},
{
"internalType": "address",
"name": "sender",
"type": "address"
},
{
"internalType": "address",
"name": "target",
"type": "address"
},
{
"internalType": "uint256",
"name": "value",
"type": "uint256"
},
{
"internalType": "uint256",
"name": "gasLimit",
"type": "uint256"
},
{
"internalType": "bytes",
"name": "data",
"type": "bytes"
}
],
"internalType": "struct Types.WithdrawalTransaction",
"name": "_tx",
"type": "tuple"
},
{
"internalType": "address",
"name": "_proofSubmitter",
"type": "address"
}
],
"name": "finalizeWithdrawalTransactionExternalProof",
"outputs": [],
"stateMutability": "nonpayable",
"type": "function"
},
{
"inputs": [
{
......@@ -293,6 +348,25 @@
"stateMutability": "pure",
"type": "function"
},
{
"inputs": [
{
"internalType": "bytes32",
"name": "_withdrawalHash",
"type": "bytes32"
}
],
"name": "numProofSubmitters",
"outputs": [
{
"internalType": "uint256",
"name": "",
"type": "uint256"
}
],
"stateMutability": "view",
"type": "function"
},
{
"inputs": [],
"name": "params",
......@@ -342,6 +416,30 @@
"stateMutability": "view",
"type": "function"
},
{
"inputs": [
{
"internalType": "bytes32",
"name": "",
"type": "bytes32"
},
{
"internalType": "uint256",
"name": "",
"type": "uint256"
}
],
"name": "proofSubmitters",
"outputs": [
{
"internalType": "address",
"name": "",
"type": "address"
}
],
"stateMutability": "view",
"type": "function"
},
{
"inputs": [
{
......@@ -430,6 +528,11 @@
"internalType": "bytes32",
"name": "",
"type": "bytes32"
},
{
"internalType": "address",
"name": "",
"type": "address"
}
],
"name": "provenWithdrawals",
......
packages/contracts-bedrock/snapshots/storageLayout/OptimismPortal2.json
View file @ 1a4a80e5
......@@ -88,7 +88,7 @@
"label": "provenWithdrawals",
"offset": 0,
"slot": "57",
"type": "mapping(bytes32 => struct OptimismPortal2.ProvenWithdrawal)"
"type": "mapping(bytes32 => mapping(address => struct OptimismPortal2.ProvenWithdrawal))"
},
{
"bytes": "32",
......@@ -110,5 +110,12 @@
"offset": 4,
"slot": "59",
"type": "uint64"
},
{
"bytes": "32",
"label": "proofSubmitters",
"offset": 0,
"slot": "60",
"type": "mapping(bytes32 => address[])"
}
]
\ No newline at end of file
packages/contracts-bedrock/src/L1/OptimismPortal2.sol
View file @ 1a4a80e5
......@@ -78,8 +78,8 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
/// @custom:network-specific
DisputeGameFactory public disputeGameFactory;
/// @notice A mapping of withdrawal hashes to `ProvenWithdrawal` data.
mapping(bytes32 => ProvenWithdrawal) public provenWithdrawals;
/// @notice A mapping of withdrawal hashes to proof submitters to `ProvenWithdrawal` data.
mapping(bytes32 => mapping(address => ProvenWithdrawal)) public provenWithdrawals;
/// @notice A mapping of dispute game addresses to whether or not they are blacklisted.
mapping(IDisputeGame => bool) public disputeGameBlacklist;
......@@ -90,6 +90,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
/// @notice The timestamp at which the respected game type was last updated.
uint64 public respectedGameTypeUpdatedAt;
/// @notice Mapping of withdrawal hashes to addresses that have submitted a proof for the withdrawal.
mapping(bytes32 => address[]) public proofSubmitters;
/// @notice Emitted when a transaction is deposited from L1 to L2.
/// The parameters of this event are read by the rollup node and used to derive deposit
/// transactions on L2.
......@@ -117,8 +120,8 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
}
/// @notice Semantic version.
/// @custom:semver 3.4.0
string public constant version = "3.4.0";
/// @custom:semver 3.5.0
string public constant version = "3.5.0";
/// @notice Constructs the OptimismPortal contract.
constructor(
......@@ -265,7 +268,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
// Load the ProvenWithdrawal into memory, using the withdrawal hash as a unique identifier.
bytes32 withdrawalHash = Hashing.hashWithdrawal(_tx);
ProvenWithdrawal memory provenWithdrawal = provenWithdrawals[withdrawalHash];
ProvenWithdrawal memory provenWithdrawal = provenWithdrawals[withdrawalHash][msg.sender];
// We do not allow for proving withdrawals against dispute games that have resolved against the favor
// of the root claim.
......@@ -311,16 +314,32 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
// Designate the withdrawalHash as proven by storing the `disputeGameProxy` & `timestamp` in the
// `provenWithdrawals` mapping. A `withdrawalHash` can only be proven once unless the dispute game it proved
// against resolves against the favor of the root claim.
provenWithdrawals[withdrawalHash] =
provenWithdrawals[withdrawalHash][msg.sender] =
ProvenWithdrawal({ disputeGameProxy: gameProxy, timestamp: uint64(block.timestamp) });
// Emit a `WithdrawalProven` event.
emit WithdrawalProven(withdrawalHash, _tx.sender, _tx.target);
// Add the proof submitter to the list of proof submitters for this withdrawal hash.
proofSubmitters[withdrawalHash].push(msg.sender);
}
/// @notice Finalizes a withdrawal transaction.
/// @param _tx Withdrawal transaction to finalize.
function finalizeWithdrawalTransaction(Types.WithdrawalTransaction memory _tx) external whenNotPaused {
finalizeWithdrawalTransactionExternalProof(_tx, msg.sender);
}
/// @notice Finalizes a withdrawal transaction, using an external proof submitter.
/// @param _tx Withdrawal transaction to finalize.
/// @param _proofSubmitter Address of the proof submitter.
function finalizeWithdrawalTransactionExternalProof(
Types.WithdrawalTransaction memory _tx,
address _proofSubmitter
)
public
whenNotPaused
{
// Make sure that the l2Sender has not yet been set. The l2Sender is set to a value other
// than the default value when a withdrawal transaction is being finalized. This check is
// a defacto reentrancy guard.
......@@ -332,7 +351,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
bytes32 withdrawalHash = Hashing.hashWithdrawal(_tx);
// Check that the withdrawal can be finalized.
checkWithdrawal(withdrawalHash);
checkWithdrawal(withdrawalHash, _proofSubmitter);
// Mark the withdrawal as finalized so it can't be replayed.
finalizedWithdrawals[withdrawalHash] = true;
......@@ -435,8 +454,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
/// @notice Checks if a withdrawal can be finalized. This function will revert if the withdrawal cannot be
/// finalized, and otherwise has no side-effects.
/// @param _withdrawalHash Hash of the withdrawal to check.
function checkWithdrawal(bytes32 _withdrawalHash) public view {
ProvenWithdrawal memory provenWithdrawal = provenWithdrawals[_withdrawalHash];
/// @param _proofSubmitter The submitter of the proof for the withdrawal hash
function checkWithdrawal(bytes32 _withdrawalHash, address _proofSubmitter) public view {
ProvenWithdrawal memory provenWithdrawal = provenWithdrawals[_withdrawalHash][_proofSubmitter];
IDisputeGame disputeGameProxy = provenWithdrawal.disputeGameProxy;
// The dispute game must not be blacklisted.
......@@ -445,7 +465,10 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
// A withdrawal can only be finalized if it has been proven. We know that a withdrawal has
// been proven at least once when its timestamp is non-zero. Unproven withdrawals will have
// a timestamp of zero.
require(provenWithdrawal.timestamp != 0, "OptimismPortal: withdrawal has not been proven yet");
require(
provenWithdrawal.timestamp != 0,
"OptimismPortal: withdrawal has not been proven by proof submitter address yet"
);
uint64 createdAt = disputeGameProxy.createdAt().raw();
......@@ -468,7 +491,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
// from finalizing withdrawals proven against non-finalized output roots.
require(
disputeGameProxy.status() == GameStatus.DEFENDER_WINS,
"OptimismPortal: output proposal has not been finalized yet"
"OptimismPortal: output proposal has not been validated"
);
// The game type of the dispute game must be the respected game type. This was also checked in
......@@ -494,4 +517,11 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver {
// Check that this withdrawal has not already been finalized, this is replay protection.
require(!finalizedWithdrawals[_withdrawalHash], "OptimismPortal: withdrawal has already been finalized");
}
/// @notice External getter for the number of proof submitters for a withdrawal hash.
/// @param _withdrawalHash Hash of the withdrawal.
/// @return The number of proof submitters for the withdrawal hash.
function numProofSubmitters(bytes32 _withdrawalHash) external view returns (uint256) {
return proofSubmitters[_withdrawalHash].length;
}
}
packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol
View file @ 1a4a80e5
......@@ -683,6 +683,63 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
assert(address(bob).balance == bobBalanceBefore + 100);
}
/// @dev Tests that `finalizeWithdrawalTransaction` succeeds using a different proof than an earlier one by another
/// party.
function test_finalizeWithdrawalTransaction_secondaryProof_succeeds() external {
uint256 bobBalanceBefore = address(bob).balance;
// Create a secondary dispute game.
IDisputeGame secondGame = disputeGameFactory.create(
optimismPortal2.respectedGameType(), Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber + 1)
);
// Warp 1 second into the future so that the proof is submitted after the timestamp of game creation.
vm.warp(block.timestamp + 1 seconds);
// Prove the withdrawal transaction against the invalid dispute game, as 0xb0b.
vm.expectEmit(true, true, true, true);
emit WithdrawalProven(_withdrawalHash, alice, bob);
vm.prank(address(0xb0b));
optimismPortal2.proveWithdrawalTransaction({
_tx: _defaultTx,
_disputeGameIndex: _proposedGameIndex + 1,
_outputRootProof: _outputRootProof,
_withdrawalProof: _withdrawalProof
});
// Mock the status of the dispute game 0xb0b proves against to be CHALLENGER_WINS.
vm.mockCall(address(secondGame), abi.encodeCall(game.status, ()), abi.encode(GameStatus.CHALLENGER_WINS));
// Prove the withdrawal transaction against the invalid dispute game, as the test contract, against the original
// game.
vm.expectEmit(true, true, true, true);
emit WithdrawalProven(_withdrawalHash, alice, bob);
optimismPortal2.proveWithdrawalTransaction({
_tx: _defaultTx,
_disputeGameIndex: _proposedGameIndex,
_outputRootProof: _outputRootProof,
_withdrawalProof: _withdrawalProof
});
// Warp and resolve the original dispute game.
game.resolveClaim(0);
game.resolve();
vm.warp(block.timestamp + optimismPortal2.proofMaturityDelaySeconds() + 1 seconds);
// Ensure both proofs are registered successfully.
assertEq(optimismPortal2.numProofSubmitters(_withdrawalHash), 2);
vm.expectRevert("OptimismPortal: output proposal has not been validated");
vm.prank(address(0xb0b));
optimismPortal2.finalizeWithdrawalTransaction(_defaultTx);
vm.expectEmit(true, true, false, true);
emit WithdrawalFinalized(_withdrawalHash, true);
optimismPortal2.finalizeWithdrawalTransaction(_defaultTx);
assert(address(bob).balance == bobBalanceBefore + 100);
}
/// @dev Tests that `finalizeWithdrawalTransaction` reverts if the contract is paused.
function test_finalizeWithdrawalTransaction_paused_reverts() external {
vm.prank(optimismPortal2.GUARDIAN());
......@@ -696,7 +753,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
function test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() external {
uint256 bobBalanceBefore = address(bob).balance;
vm.expectRevert("OptimismPortal: withdrawal has not been proven yet");
vm.expectRevert("OptimismPortal: withdrawal has not been proven by proof submitter address yet");
optimismPortal2.finalizeWithdrawalTransaction(_defaultTx);
assert(address(bob).balance == bobBalanceBefore);
......@@ -770,7 +827,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
vm.warp(block.timestamp + optimismPortal2.proofMaturityDelaySeconds() + 1);
// Attempt to finalize the withdrawal
vm.expectRevert("OptimismPortal: output proposal has not been finalized yet");
vm.expectRevert("OptimismPortal: output proposal has not been validated");
optimismPortal2.finalizeWithdrawalTransaction(_defaultTx);
// Ensure that bob's balance has remained the same
......@@ -977,7 +1034,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
// Prove the withdrawal transaction
optimismPortal2.proveWithdrawalTransaction(_tx, _proposedGameIndex, proof, withdrawalProof);
(IDisputeGame _game,) = optimismPortal2.provenWithdrawals(withdrawalHash);
(IDisputeGame _game,) = optimismPortal2.provenWithdrawals(withdrawalHash, address(this));
assertTrue(_game.rootClaim().raw() != bytes32(0));
// Resolve the dispute game
......@@ -1123,7 +1180,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest {
// Warp 1 second in the future, past the proof maturity delay, and attempt to finalize the withdrawal.
// This should also fail, since the dispute game has not resolved yet.
vm.warp(block.timestamp + 1 seconds);
vm.expectRevert("OptimismPortal: output proposal has not been finalized yet");
vm.expectRevert("OptimismPortal: output proposal has not been validated");
optimismPortal2.finalizeWithdrawalTransaction(_defaultTx);
// Finalize the dispute game and attempt to finalize the withdrawal again. This should also fail, since the
......
packages/contracts-bedrock/test/Specs.t.sol
View file @ 1a4a80e5
......@@ -277,6 +277,11 @@ contract Specification_Test is CommonTest {
_sel: OptimismPortal2.finalizeWithdrawalTransaction.selector,
_pausable: true
});
_addSpec({
_name: "OptimismPortal2",
_sel: OptimismPortal2.finalizeWithdrawalTransactionExternalProof.selector,
_pausable: true
});
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("finalizedWithdrawals(bytes32)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("guardian()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("initialize(address,address,address)") });
......@@ -285,7 +290,7 @@ contract Specification_Test is CommonTest {
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("params()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("paused()") });
_addSpec({ _name: "OptimismPortal2", _sel: OptimismPortal2.proveWithdrawalTransaction.selector, _pausable: true });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("provenWithdrawals(bytes32)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("provenWithdrawals(bytes32,address)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("superchainConfig()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("systemConfig()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("version()") });
......@@ -294,10 +299,12 @@ contract Specification_Test is CommonTest {
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("respectedGameType()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("blacklistDisputeGame(address)"), _auth: Role.GUARDIAN });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("setRespectedGameType(uint32)"), _auth: Role.GUARDIAN });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("checkWithdrawal(bytes32)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("checkWithdrawal(bytes32,address)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("proofMaturityDelaySeconds()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("disputeGameFinalityDelaySeconds()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("respectedGameTypeUpdatedAt()") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("proofSubmitters(bytes32,uint256)") });
_addSpec({ _name: "OptimismPortal2", _sel: _getSel("numProofSubmitters(bytes32)") });
// ProtocolVersions
_addSpec({ _name: "ProtocolVersions", _sel: _getSel("RECOMMENDED_SLOT()") });
......
packages/sdk/src/cross-chain-messenger.ts
View file @ 1a4a80e5
......@@ -829,7 +829,8 @@ export class CrossChainMessenger {
try {
// If this doesn't revert then we should be fine to relay.
await this.contracts.l1.OptimismPortal2.checkWithdrawal(
hashLowLevelMessage(withdrawal)
hashLowLevelMessage(withdrawal),
await this.l1Signer.getAddress()
)
return MessageStatus.READY_FOR_RELAY
......
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