Skip to content

N
nebula
Commit a272058b authored by OptimismBot's avatar OptimismBot Committed by GitHub
Browse files
Options

Merge pull request #5444 from ethereum-optimism/jm/nonreentrant-with-replays 

feat(ctb): Fail relayed XDM messages on reentry
parents 85075b94 aae9e545
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 189 additions and 386 deletions
packages/contracts-bedrock/.gas-snapshot
View file @ a272058b
......@@ -7,14 +7,14 @@ Bytes_toNibbles_Test:test_toNibbles_expectedResult5Bytes_works() (gas: 6132)
Bytes_toNibbles_Test:test_toNibbles_zeroLengthInput_works() (gas: 944)
CrossDomainMessenger_BaseGas_Test:test_baseGas_succeeds() (gas: 20097)
CrossDomainOwnable2_Test:test_onlyOwner_notMessenger_reverts() (gas: 8416)
CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 61872)
CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 57254)
CrossDomainOwnable2_Test:test_onlyOwner_notOwner_reverts() (gas: 16566)
CrossDomainOwnable2_Test:test_onlyOwner_succeeds() (gas: 75933)
CrossDomainOwnable2_Test:test_onlyOwner_succeeds() (gas: 73282)
CrossDomainOwnable3_Test:test_constructor_succeeds() (gas: 10554)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notMessenger_reverts() (gas: 28334)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner2_reverts() (gas: 76381)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner2_reverts() (gas: 73730)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner_reverts() (gas: 31978)
CrossDomainOwnable3_Test:test_crossDomainTransferOwnership_succeeds() (gas: 93938)
CrossDomainOwnable3_Test:test_crossDomainTransferOwnership_succeeds() (gas: 91287)
CrossDomainOwnable3_Test:test_localOnlyOwner_notOwner_reverts() (gas: 13193)
CrossDomainOwnable3_Test:test_localOnlyOwner_succeeds() (gas: 35220)
CrossDomainOwnable3_Test:test_localTransferOwnership_succeeds() (gas: 52128)
......@@ -70,20 +70,18 @@ L1BlockTest:test_timestamp_succeeds() (gas: 7640)
L1BlockTest:test_updateValues_succeeds() (gas: 60482)
L1CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 24715)
L1CrossDomainMessenger_Test:test_relayMessage_legacyOldReplay_reverts() (gas: 49394)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 232952)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 206446)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterSuccess_reverts() (gas: 146819)
L1CrossDomainMessenger_Test:test_relayMessage_legacy_succeeds() (gas: 79729)
L1CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 725335)
L1CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 662298)
L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 200353)
L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 76665)
L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 101282)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 209286)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 203184)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterSuccess_reverts() (gas: 123784)
L1CrossDomainMessenger_Test:test_relayMessage_legacy_succeeds() (gas: 77098)
L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 197091)
L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 74034)
L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 56540)
L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 12365)
L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 53445)
L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 31063)
L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 304740)
L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1496124)
L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 87194)
L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 84563)
L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24296)
L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52707)
L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27310)
......@@ -120,15 +118,13 @@ L1StandardBridge_Getter_Test:test_getters_succeeds() (gas: 32173)
L1StandardBridge_Initialize_Test:test_initialize_succeeds() (gas: 22050)
L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 525438)
L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8411)
L2CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 680395)
L2CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 626456)
L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 166461)
L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 54980)
L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 51448)
L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 163159)
L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 48640)
L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 29021)
L2CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 11711)
L2CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 122508)
L2CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 134826)
L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 54580)
L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 48139)
L2CrossDomainMessenger_Test:test_xDomainSender_senderNotSet_reverts() (gas: 10590)
L2ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 26431)
L2ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 21814)
......
packages/contracts-bedrock/.storage-layout
View file @ a272058b
......@@ -24,8 +24,7 @@
| xDomainMsgSender | address | 204 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| msgNonce | uint240 | 205 | 0 | 30 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| failedMessages | mapping(bytes32 => bool) | 206 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| reentrancyLocks | mapping(bytes32 => bool) | 207 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap | uint256[41] | 208 | 0 | 1312 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap | uint256[42] | 207 | 0 | 1344 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
=======================
➡ contracts/L1/L1StandardBridge.sol:L1StandardBridge
......@@ -135,8 +134,7 @@
| xDomainMsgSender | address | 204 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| msgNonce | uint240 | 205 | 0 | 30 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| failedMessages | mapping(bytes32 => bool) | 206 | 0 | 32 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| reentrancyLocks | mapping(bytes32 => bool) | 207 | 0 | 32 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap | uint256[41] | 208 | 0 | 1312 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap | uint256[42] | 207 | 0 | 1344 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
=======================
➡ contracts/L2/L2StandardBridge.sol:L2StandardBridge
......
packages/contracts-bedrock/contracts/test/CrossDomainMessenger.t.sol
View file @ a272058b
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
import { Messenger_Initializer, Reverter, CallerCaller } from "./CommonTest.t.sol";
import { Messenger_Initializer, Reverter, CallerCaller, CommonTest } from "./CommonTest.t.sol";
import { L1CrossDomainMessenger } from "../L1/L1CrossDomainMessenger.sol";
// Libraries
import { Predeploys } from "../libraries/Predeploys.sol";
import { Hashing } from "../libraries/Hashing.sol";
import { Encoding } from "../libraries/Encoding.sol";
// CrossDomainMessenger_Test is for testing functionality which is common to both the L1 and L2
// CrossDomainMessenger contracts. For simplicity, we use the L1 Messenger as the test contract.
......@@ -17,3 +23,149 @@ contract CrossDomainMessenger_BaseGas_Test is Messenger_Initializer {
L1Messenger.baseGas(hex"ff", _minGasLimit);
}
}
/**
* @title ExternalRelay
* @notice A mock external contract called via the SafeCall inside
* the CrossDomainMessenger's `relayMessage` function.
*/
contract ExternalRelay is CommonTest {
address internal op;
address internal fuzzedSender;
L1CrossDomainMessenger internal L1Messenger;
event FailedRelayedMessage(bytes32 indexed msgHash);
constructor(L1CrossDomainMessenger _l1Messenger, address _op) {
L1Messenger = _l1Messenger;
op = _op;
}
/**
* @notice Internal helper function to relay a message and perform assertions.
*/
function _internalRelay(address _innerSender) internal {
address initialSender = L1Messenger.xDomainMessageSender();
bytes memory callMessage = getCallData();
bytes32 hash = Hashing.hashCrossDomainMessage({
_nonce: Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
_sender: _innerSender,
_target: address(this),
_value: 0,
_gasLimit: 0,
_data: callMessage
});
vm.expectEmit(true, true, true, true);
emit FailedRelayedMessage(hash);
vm.prank(address(op));
L1Messenger.relayMessage({
_nonce: Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
_sender: _innerSender,
_target: address(this),
_value: 0,
_minGasLimit: 0,
_message: callMessage
});
assertTrue(L1Messenger.failedMessages(hash));
assertFalse(L1Messenger.successfulMessages(hash));
assertEq(initialSender, L1Messenger.xDomainMessageSender());
}
/**
* @notice externalCallWithMinGas is called by the CrossDomainMessenger.
*/
function externalCallWithMinGas() external payable {
for (uint256 i = 0; i < 10; i++) {
address _innerSender;
unchecked {
_innerSender = address(uint160(uint256(uint160(fuzzedSender)) + i));
}
_internalRelay(_innerSender);
}
}
/**
* @notice Helper function to get the callData for an `externalCallWithMinGas
*/
function getCallData() public returns (bytes memory) {
return abi.encodeWithSelector(ExternalRelay.externalCallWithMinGas.selector);
}
/**
* @notice Helper function to set the fuzzed sender
*/
function setFuzzedSender(address _fuzzedSender) public {
fuzzedSender = _fuzzedSender;
}
}
/**
* @title CrossDomainMessenger_RelayMessage_Test
* @notice Fuzz tests re-entrancy into the CrossDomainMessenger relayMessage function.
*/
contract CrossDomainMessenger_RelayMessage_Test is Messenger_Initializer {
// Storage slot of the l2Sender
uint256 constant senderSlotIndex = 50;
ExternalRelay public er;
function setUp() public override {
super.setUp();
er = new ExternalRelay(L1Messenger, address(op));
}
/**
* @dev This test mocks an OptimismPortal call to the L1CrossDomainMessenger via
* the relayMessage function. The relayMessage function will then use SafeCall's
* callWithMinGas to call the target with call data packed in the callMessage.
* For this test, the callWithMinGas will call the mock ExternalRelay test contract
* defined above, executing the externalCallWithMinGas function which will try to
* re-enter the CrossDomainMessenger's relayMessage function, resulting in that message
* being recorded as failed.
*/
function testFuzz_relayMessageReenter_succeeds(address _sender, uint256 _gasLimit) external {
vm.assume(_sender != Predeploys.L2_CROSS_DOMAIN_MESSENGER);
address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
er.setFuzzedSender(_sender);
address target = address(er);
bytes memory callMessage = er.getCallData();
vm.expectCall(target, callMessage);
uint64 gasLimit = uint64(bound(_gasLimit, 0, 30_000_000));
bytes32 hash = Hashing.hashCrossDomainMessage({
_nonce: Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
_sender: sender,
_target: target,
_value: 0,
_gasLimit: gasLimit,
_data: callMessage
});
// set the value of op.l2Sender() to be the L2 Cross Domain Messenger.
vm.store(address(op), bytes32(senderSlotIndex), bytes32(abi.encode(sender)));
vm.prank(address(op));
L1Messenger.relayMessage({
_nonce: Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
_sender: sender,
_target: target,
_value: 0,
_minGasLimit: gasLimit,
_message: callMessage
});
assertTrue(L1Messenger.successfulMessages(hash));
assertEq(L1Messenger.failedMessages(hash), false);
// Ensures that the `xDomainMsgSender` is set back to `Predeploys.L2_CROSS_DOMAIN_MESSENGER`
vm.expectRevert("CrossDomainMessenger: xDomainMessageSender is not set");
L1Messenger.xDomainMessageSender();
}
}
packages/contracts-bedrock/contracts/test/L1CrossDomainMessenger.t.sol
View file @ a272058b
......@@ -100,10 +100,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
L1Messenger.xDomainMessageSender();
}
// xDomainMessageSender: should return the xDomainMsgSender address
// TODO: might need a test contract
// function test_xDomainSenderSetCorrectly() external {}
function test_relayMessage_v2_reverts() external {
address target = address(0xabcd);
address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
......@@ -295,173 +291,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
assertEq(L1Messenger.failedMessages(hash), true);
}
// relayMessage: Should revert if the recipient is trying to reenter with the
// same message.
function test_relayMessage_reentrancySameMessage_reverts() external {
ConfigurableCaller caller = new ConfigurableCaller();
address target = address(caller);
address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
bytes memory callMessage = abi.encodeWithSelector(caller.call.selector);
bytes32 hash = Hashing.hashCrossDomainMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
callMessage
);
// Set the portal's `l2Sender` to the `sender`.
vm.store(address(op), bytes32(senderSlotIndex), bytes32(uint256(uint160(sender))));
// Act as the portal and call the `relayMessage` function with the `innerMessage`.
vm.prank(address(op));
vm.expectCall(target, callMessage);
L1Messenger.relayMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
callMessage
);
// Assert that the message failed to be relayed
assertFalse(L1Messenger.successfulMessages(hash));
assertTrue(L1Messenger.failedMessages(hash));
// Set the configurable caller's target to `L1Messenger` and set the payload to `relayMessage(...)`.
caller.setDoRevert(false);
caller.setTarget(address(L1Messenger));
caller.setPayload(
abi.encodeWithSelector(
L1Messenger.relayMessage.selector,
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
callMessage
)
);
// Attempt to replay the failed message, which will *not* immediately revert this time around,
// but attempt to reenter `relayMessage` with the same message hash. The reentrancy attempt should
// revert.
vm.expectEmit(true, true, true, true, target);
emit WhatHappened(
false,
abi.encodeWithSignature("Error(string)", "ReentrancyGuard: reentrant call")
);
L1Messenger.relayMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }), // nonce
sender,
target,
0,
0,
callMessage
);
// Assert that the message still failed to be relayed.
assertFalse(L1Messenger.successfulMessages(hash));
assertTrue(L1Messenger.failedMessages(hash));
}
// relayMessage: should not revert if the recipient reenters `relayMessage` with a different
// message hash.
function test_relayMessage_reentrancyDiffMessage_succeeds() external {
ConfigurableCaller caller = new ConfigurableCaller();
address target = address(caller);
address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
bytes memory messageA = abi.encodeWithSelector(caller.call.selector);
bytes memory messageB = hex"";
bytes32 hashA = Hashing.hashCrossDomainMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
messageA
);
bytes32 hashB = Hashing.hashCrossDomainMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
messageB
);
// Set the portal's `l2Sender` to the `sender`.
vm.store(address(op), bytes32(senderSlotIndex), bytes32(uint256(uint160(sender))));
// Act as the portal and call the `relayMessage` function with both `messageA` and `messageB`.
vm.startPrank(address(op));
vm.expectCall(target, messageA);
L1Messenger.relayMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
messageA
);
vm.expectCall(target, messageB);
L1Messenger.relayMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
messageB
);
// Stop acting as the portal
vm.stopPrank();
// Assert that both messages failed to be relayed
assertFalse(L1Messenger.successfulMessages(hashA));
assertFalse(L1Messenger.successfulMessages(hashB));
assertTrue(L1Messenger.failedMessages(hashA));
assertTrue(L1Messenger.failedMessages(hashB));
// Set the configurable caller's target to `L1Messenger` and set the payload to `relayMessage(...)`.
caller.setDoRevert(false);
caller.setTarget(address(L1Messenger));
caller.setPayload(
abi.encodeWithSelector(
L1Messenger.relayMessage.selector,
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
messageB
)
);
// Attempt to replay the failed message, which will *not* immediately revert this time around,
// but attempt to reenter `relayMessage` with messageB. The reentrancy attempt should succeed
// because the message hashes are different.
vm.expectEmit(true, true, true, true, target);
emit WhatHappened(true, hex"");
L1Messenger.relayMessage(
Encoding.encodeVersionedNonce({ _nonce: 0, _version: 1 }),
sender,
target,
0,
0,
messageA
);
// Assert that both messages are now in the `successfulMessages` mapping.
assertTrue(L1Messenger.successfulMessages(hashA));
assertTrue(L1Messenger.successfulMessages(hashB));
}
function test_relayMessage_legacy_succeeds() external {
address target = address(0xabcd);
address sender = Predeploys.L2_CROSS_DOMAIN_MESSENGER;
......
packages/contracts-bedrock/contracts/test/L2CrossDomainMessenger.t.sol
View file @ a272058b
......@@ -230,168 +230,4 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
assertEq(L2Messenger.successfulMessages(hash), true);
assertEq(L2Messenger.failedMessages(hash), true);
}
// relayMessage: Should revert if the recipient is trying to reenter with the
// same message.
function test_relayMessage_reentrancySameMessage_reverts() external {
ConfigurableCaller caller = new ConfigurableCaller();
address target = address(caller);
address sender = address(L1Messenger);
address l1XDMAlias = AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger));
bytes memory callMessage = abi.encodeWithSelector(caller.call.selector);
bytes32 hash = Hashing.hashCrossDomainMessage(
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
callMessage
);
// Act as the L1XDM and call the `relayMessage` function with the `innerMessage`.
vm.prank(l1XDMAlias);
vm.expectCall(target, callMessage);
L2Messenger.relayMessage(
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
callMessage
);
// Assert that the message failed to be relayed
assertFalse(L2Messenger.successfulMessages(hash));
assertTrue(L2Messenger.failedMessages(hash));
// Set the configurable caller's target to `L2Messenger` and set the payload to `relayMessage(...)`.
caller.setDoRevert(false);
caller.setTarget(address(L2Messenger));
caller.setPayload(
abi.encodeWithSelector(
L2Messenger.relayMessage.selector,
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
callMessage
)
);
// Attempt to replay the failed message, which will *not* immediately revert this time around,
// but attempt to reenter `relayMessage` with the same message hash. The reentrancy attempt should
// revert.
vm.expectEmit(true, true, true, true, target);
emit WhatHappened(
false,
abi.encodeWithSignature("Error(string)", "ReentrancyGuard: reentrant call")
);
L2Messenger.relayMessage(
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
callMessage
);
// Assert that the message still failed to be relayed.
assertFalse(L2Messenger.successfulMessages(hash));
assertTrue(L2Messenger.failedMessages(hash));
}
// relayMessage: should not revert if the recipient reenters `relayMessage` with a different
// message hash.
function test_relayMessage_reentrancyDiffMessage_succeeds() external {
ConfigurableCaller caller = new ConfigurableCaller();
address target = address(caller);
address sender = address(L1Messenger);
address l1XDMAlias = AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger));
bytes memory messageA = abi.encodeWithSelector(caller.call.selector);
bytes memory messageB = hex"";
bytes32 hashA = Hashing.hashCrossDomainMessage(
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
messageA
);
bytes32 hashB = Hashing.hashCrossDomainMessage(
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
messageB
);
// Act as the L1XDM and call the `relayMessage` function with both `messageA` and `messageB`.
vm.startPrank(l1XDMAlias);
vm.expectCall(target, messageA);
L2Messenger.relayMessage(
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
messageA
);
vm.expectCall(target, messageB);
L2Messenger.relayMessage(
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
messageB
);
// Stop acting as the L1XDM
vm.stopPrank();
// Assert that both messages failed to be relayed
assertFalse(L2Messenger.successfulMessages(hashA));
assertFalse(L2Messenger.successfulMessages(hashB));
assertTrue(L2Messenger.failedMessages(hashA));
assertTrue(L2Messenger.failedMessages(hashB));
// Set the configurable caller's target to `L2Messenger` and set the payload to `relayMessage(...)`.
caller.setDoRevert(false);
caller.setTarget(address(L2Messenger));
caller.setPayload(
abi.encodeWithSelector(
L2Messenger.relayMessage.selector,
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
messageB
)
);
// Attempt to replay the failed message, which will *not* immediately revert this time around,
// but attempt to reenter `relayMessage` with messageB. The reentrancy attempt should succeed
// because the message hashes are different.
vm.expectEmit(true, true, true, true, target);
emit WhatHappened(true, hex"");
L2Messenger.relayMessage(
Encoding.encodeVersionedNonce(0, 1),
sender,
target,
0,
0,
messageA
);
// Assert that both messages are now in the `successfulMessages` mapping.
assertTrue(L2Messenger.successfulMessages(hashA));
assertTrue(L2Messenger.successfulMessages(hashB));
}
}
packages/contracts-bedrock/contracts/test/OptimismPortal.t.sol
View file @ a272058b
......@@ -1019,7 +1019,7 @@ contract OptimismPortal_FinalizeWithdrawal_Test is Portal_Initializer {
assertEq(outputRoot, Hashing.hashOutputRootProof(proof));
assertEq(withdrawalHash, Hashing.hashWithdrawal(_tx));
// Mock the call to the oracle
// Setup the Oracle to return the outputRoot
vm.mockCall(
address(oracle),
abi.encodeWithSelector(oracle.getL2Output.selector),
......@@ -1039,8 +1039,6 @@ contract OptimismPortal_FinalizeWithdrawal_Test is Portal_Initializer {
// Warp past the finalization period
vm.warp(block.timestamp + oracle.FINALIZATION_PERIOD_SECONDS() + 1);
uint256 targetBalanceBefore = _target.balance;
// Finalize the withdrawal transaction
vm.expectCallMinGas(_tx.target, _tx.value, uint64(_tx.gasLimit), _tx.data);
op.finalizeWithdrawalTransaction(_tx);
......
packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol
View file @ a272058b
......@@ -175,17 +175,12 @@ abstract contract CrossDomainMessenger is
*/
mapping(bytes32 => bool) public failedMessages;
/**
* @notice A mapping of hashes to reentrancy locks.
*/
mapping(bytes32 => bool) internal reentrancyLocks;
/**
* @notice Reserve extra slots in the storage layout for future upgrades.
* A gap size of 41 was chosen here, so that the first slot used in a child contract
* would be a multiple of 50.
*/
uint256[41] private __gap;
uint256[42] private __gap;
/**
* @notice Emitted whenever a message is sent to the other chain.
......@@ -323,13 +318,6 @@ abstract contract CrossDomainMessenger is
_message
);
// Check if the reentrancy lock for the `versionedHash` is already set.
if (reentrancyLocks[versionedHash]) {
revert("ReentrancyGuard: reentrant call");
}
// Trigger the reentrancy lock for `versionedHash`
reentrancyLocks[versionedHash] = true;
if (_isOtherMessenger()) {
// These properties should always hold when the message is first submitted (as
// opposed to being replayed).
......@@ -357,6 +345,15 @@ abstract contract CrossDomainMessenger is
"CrossDomainMessenger: message has already been relayed"
);
// If `xDomainMsgSender` is not the default L2 sender, this function
// is being re-entered. This marks the message as failed to allow it
// to be replayed.
if (xDomainMsgSender != Constants.DEFAULT_L2_SENDER) {
failedMessages[versionedHash] = true;
emit FailedRelayedMessage(versionedHash);
return;
}
xDomainMsgSender = _sender;
bool success = SafeCall.callWithMinGas(_target, _minGasLimit, _value, _message);
xDomainMsgSender = Constants.DEFAULT_L2_SENDER;
......@@ -377,9 +374,6 @@ abstract contract CrossDomainMessenger is
revert("CrossDomainMessenger: failed to relay message");
}
}
// Clear the reentrancy lock for `versionedHash`
reentrancyLocks[versionedHash] = false;
}
/**
......
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