Commit ee308ead authored by clabby's avatar clabby

De-racecar `perMessageNonReentrant` modifier

parent d247694a
...@@ -21,14 +21,14 @@ CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_su ...@@ -21,14 +21,14 @@ CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_su
CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10530) CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10530)
CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34861) CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34861)
CrossDomainOwnable2_Test:test_onlyOwner_notMessenger_reverts() (gas: 8416) CrossDomainOwnable2_Test:test_onlyOwner_notMessenger_reverts() (gas: 8416)
CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 64332) CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 64616)
CrossDomainOwnable2_Test:test_onlyOwner_notOwner_reverts() (gas: 16588) CrossDomainOwnable2_Test:test_onlyOwner_notOwner_reverts() (gas: 16588)
CrossDomainOwnable2_Test:test_onlyOwner_succeeds() (gas: 79009) CrossDomainOwnable2_Test:test_onlyOwner_succeeds() (gas: 79364)
CrossDomainOwnable3_Test:test_constructor_succeeds() (gas: 10576) CrossDomainOwnable3_Test:test_constructor_succeeds() (gas: 10576)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notMessenger_reverts() (gas: 28289) CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notMessenger_reverts() (gas: 28289)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner2_reverts() (gas: 79391) CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner2_reverts() (gas: 79746)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner_reverts() (gas: 31978) CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner_reverts() (gas: 31978)
CrossDomainOwnable3_Test:test_crossDomainTransferOwnership_succeeds() (gas: 96969) CrossDomainOwnable3_Test:test_crossDomainTransferOwnership_succeeds() (gas: 97324)
CrossDomainOwnable3_Test:test_localOnlyOwner_notOwner_reverts() (gas: 13215) CrossDomainOwnable3_Test:test_localOnlyOwner_notOwner_reverts() (gas: 13215)
CrossDomainOwnable3_Test:test_localOnlyOwner_succeeds() (gas: 35220) CrossDomainOwnable3_Test:test_localOnlyOwner_succeeds() (gas: 35220)
CrossDomainOwnable3_Test:test_localTransferOwnership_succeeds() (gas: 52128) CrossDomainOwnable3_Test:test_localTransferOwnership_succeeds() (gas: 52128)
...@@ -71,24 +71,24 @@ L1BlockNumberTest:test_receive_succeeds() (gas: 25340) ...@@ -71,24 +71,24 @@ L1BlockNumberTest:test_receive_succeeds() (gas: 25340)
L1CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 24781) L1CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 24781)
L1CrossDomainMessenger_Test:test_pause_callerIsNotOwner_reverts() (gas: 24517) L1CrossDomainMessenger_Test:test_pause_callerIsNotOwner_reverts() (gas: 24517)
L1CrossDomainMessenger_Test:test_pause_succeeds() (gas: 52964) L1CrossDomainMessenger_Test:test_pause_succeeds() (gas: 52964)
L1CrossDomainMessenger_Test:test_relayMessage_legacyOldReplay_reverts() (gas: 74749) L1CrossDomainMessenger_Test:test_relayMessage_legacyOldReplay_reverts() (gas: 74933)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 232829) L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 233723)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 205373) L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 206083)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterSuccess_reverts() (gas: 145674) L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterSuccess_reverts() (gas: 146213)
L1CrossDomainMessenger_Test:test_relayMessage_legacy_succeeds() (gas: 77679) L1CrossDomainMessenger_Test:test_relayMessage_legacy_succeeds() (gas: 78034)
L1CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 78550) L1CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 78734)
L1CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 725841) L1CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 727261)
L1CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 661391) L1CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 662167)
L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 199464) L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 200174)
L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 74718) L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 75073)
L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 102318) L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 102686)
L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 12562) L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 12562)
L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 56477) L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 56661)
L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 299710) L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 299710)
L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1490566) L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1490566)
L1CrossDomainMessenger_Test:test_unpause_callerIsNotOwner_reverts() (gas: 24538) L1CrossDomainMessenger_Test:test_unpause_callerIsNotOwner_reverts() (gas: 24538)
L1CrossDomainMessenger_Test:test_unpause_succeeds() (gas: 45185) L1CrossDomainMessenger_Test:test_unpause_succeeds() (gas: 45185)
L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 85083) L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 85438)
L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24274) L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24274)
L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52730) L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52730)
L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27332) L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27332)
...@@ -127,16 +127,16 @@ L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 520198) ...@@ -127,16 +127,16 @@ L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 520198)
L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8412) L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8412)
L2CrossDomainMessenger_Test:test_pause_notOwner_reverts() (gas: 10860) L2CrossDomainMessenger_Test:test_pause_notOwner_reverts() (gas: 10860)
L2CrossDomainMessenger_Test:test_pause_succeeds() (gas: 31846) L2CrossDomainMessenger_Test:test_pause_succeeds() (gas: 31846)
L2CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 59603) L2CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 59787)
L2CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 686507) L2CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 687927)
L2CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 630624) L2CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 631400)
L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 170647) L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 171357)
L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 57428) L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 57712)
L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 54435) L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 54619)
L2CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 11925) L2CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 11925)
L2CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 122621) L2CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 122621)
L2CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 134738) L2CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 134738)
L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 56860) L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 57144)
L2CrossDomainMessenger_Test:test_xDomainSender_senderNotSet_reverts() (gas: 10524) L2CrossDomainMessenger_Test:test_xDomainSender_senderNotSet_reverts() (gas: 10524)
L2ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 26454) L2ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 26454)
L2ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 21770) L2ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 21770)
...@@ -394,8 +394,6 @@ RLPWriter_writeUint_Test:test_writeUint_smallint3_succeeds() (gas: 7256) ...@@ -394,8 +394,6 @@ RLPWriter_writeUint_Test:test_writeUint_smallint3_succeeds() (gas: 7256)
RLPWriter_writeUint_Test:test_writeUint_smallint4_succeeds() (gas: 7280) RLPWriter_writeUint_Test:test_writeUint_smallint4_succeeds() (gas: 7280)
RLPWriter_writeUint_Test:test_writeUint_smallint_succeeds() (gas: 7258) RLPWriter_writeUint_Test:test_writeUint_smallint_succeeds() (gas: 7258)
RLPWriter_writeUint_Test:test_writeUint_zero_succeeds() (gas: 7726) RLPWriter_writeUint_Test:test_writeUint_zero_succeeds() (gas: 7726)
ReentrancyGuard_Test:test_perMessageNonReentrant_diffHash_succeeds() (gas: 54413)
ReentrancyGuard_Test:test_perMessageNonReentrant_sameHash_reverts() (gas: 54179)
ResourceMetering_Test:test_meter_initialResourceParams_succeeds() (gas: 8983) ResourceMetering_Test:test_meter_initialResourceParams_succeeds() (gas: 8983)
ResourceMetering_Test:test_meter_updateNoGasDelta_succeeds() (gas: 2008119) ResourceMetering_Test:test_meter_updateNoGasDelta_succeeds() (gas: 2008119)
ResourceMetering_Test:test_meter_updateOneEmptyBlock_succeeds() (gas: 18148) ResourceMetering_Test:test_meter_updateOneEmptyBlock_succeeds() (gas: 18148)
......
...@@ -24,7 +24,8 @@ ...@@ -24,7 +24,8 @@
| xDomainMsgSender | address | 204 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger | | xDomainMsgSender | address | 204 | 0 | 20 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| msgNonce | uint240 | 205 | 0 | 30 | 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 | | failedMessages | mapping(bytes32 => bool) | 206 | 0 | 32 | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
| __gap | uint256[42] | 207 | 0 | 1344 | 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 |
======================= =======================
➡ contracts/L1/L1StandardBridge.sol:L1StandardBridge ➡ contracts/L1/L1StandardBridge.sol:L1StandardBridge
...@@ -132,7 +133,8 @@ ...@@ -132,7 +133,8 @@
| xDomainMsgSender | address | 204 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger | | xDomainMsgSender | address | 204 | 0 | 20 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| msgNonce | uint240 | 205 | 0 | 30 | 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 | | failedMessages | mapping(bytes32 => bool) | 206 | 0 | 32 | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
| __gap | uint256[42] | 207 | 0 | 1344 | 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 |
======================= =======================
➡ contracts/L2/L2StandardBridge.sol:L2StandardBridge ➡ contracts/L2/L2StandardBridge.sol:L2StandardBridge
......
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
/**
* @title ReentrancyGuard
* @notice A contract that provides custom reentrancy guard modifiers.
*/
contract ReentrancyGuard {
/**
* @notice Modifier for a per-message reentrancy guard.
*/
modifier perMessageNonReentrant(bytes32 _msgHash) {
bytes32 _hashMsgHash;
assembly {
// Re-hash the `_msgHash` with the `0xcafebabe` salt to reduce the possibility
// of collisions with existing storage slots.
mstore(0x00, _msgHash)
mstore(0x20, 0xcafebabe)
_hashMsgHash := keccak256(0x00, 0x40)
// Check if the reentrancy lock for the `_msgHash` is set. If so, revert.
if sload(_hashMsgHash) {
// MEMORY SAFETY: We're reverting, so it's fine that we're clobbering the free
// memory pointer.
// Store selector for "Error(string)" in scratch space
mstore(0x00, 0x08c379a0)
// Store pointer to the string in scratch space
mstore(0x20, 0x20)
// Add the length of the "ReentrancyGuard: reentrant call" string (31 bytes)
mstore(0x40, 0x1f)
// Store "ReentrancyGuard: reentrant call" in the zero slot
// (plus a 0 byte for padding)
mstore(0x60, 0x5265656e7472616e637947756172643a207265656e7472616e742063616c6c00)
// Revert with 'Error("ReentrancyGuard: reentrant call")'
revert(0x1c, 0x64)
}
// Trigger the reentrancy lock for `_msgHash`.
sstore(_hashMsgHash, 0x01)
}
_;
assembly {
// Clear the reentrancy lock for `_msgHash`
sstore(_hashMsgHash, 0x00)
}
}
}
// SPDX-License-Identifier: Unlicense
pragma solidity 0.8.15;
import { Test } from "forge-std/Test.sol";
import { ReentrancyGuard } from "../libraries/ReentrancyGuard.sol";
contract ReentrancyGuard_Test is Test {
/// @dev The message hash passed to `noReentrance`
bytes32 internal constant MSG_HASH = keccak256(abi.encode("MESSAGE_HASH"));
NonReentrant internal reentrant;
function setUp() public {
reentrant = new NonReentrant();
}
function test_perMessageNonReentrant_diffHash_succeeds() public {
reentrant.noReentrance(bytes32(0));
}
function test_perMessageNonReentrant_sameHash_reverts() public {
vm.expectRevert("ReentrancyGuard: reentrant call");
reentrant.noReentrance(MSG_HASH);
}
fallback() external {
reentrant.noReentrance(MSG_HASH);
}
}
contract NonReentrant is ReentrancyGuard {
bool onlyOnce;
function noReentrance(bytes32 _hash) external perMessageNonReentrant(_hash) {
// Only allow the following call back to the sender occur once.
if (onlyOnce) return;
onlyOnce = true;
assembly {
let success := call(gas(), caller(), 0, 0, 0, 0, 0)
returndatacopy(0x00, 0x00, returndatasize())
switch success
case 0 {
revert(0x00, returndatasize())
}
default {
return(0x00, returndatasize())
}
}
}
}
...@@ -14,7 +14,6 @@ import { SafeCall } from "../libraries/SafeCall.sol"; ...@@ -14,7 +14,6 @@ import { SafeCall } from "../libraries/SafeCall.sol";
import { Hashing } from "../libraries/Hashing.sol"; import { Hashing } from "../libraries/Hashing.sol";
import { Encoding } from "../libraries/Encoding.sol"; import { Encoding } from "../libraries/Encoding.sol";
import { Constants } from "../libraries/Constants.sol"; import { Constants } from "../libraries/Constants.sol";
import { ReentrancyGuard } from "../libraries/ReentrancyGuard.sol";
/** /**
* @custom:legacy * @custom:legacy
...@@ -45,8 +44,7 @@ abstract contract CrossDomainMessenger is ...@@ -45,8 +44,7 @@ abstract contract CrossDomainMessenger is
CrossDomainMessengerLegacySpacer, CrossDomainMessengerLegacySpacer,
OwnableUpgradeable, OwnableUpgradeable,
PausableUpgradeable, PausableUpgradeable,
ReentrancyGuardUpgradeable, ReentrancyGuardUpgradeable
ReentrancyGuard
{ {
/** /**
* @notice Current message version identifier. * @notice Current message version identifier.
...@@ -131,12 +129,17 @@ abstract contract CrossDomainMessenger is ...@@ -131,12 +129,17 @@ abstract contract CrossDomainMessenger is
*/ */
mapping(bytes32 => bool) public failedMessages; 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. * @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 * A gap size of 41 was chosen here, so that the first slot used in a child contract
* would be a multiple of 50. * would be a multiple of 50.
*/ */
uint256[42] private __gap; uint256[41] private __gap;
/** /**
* @notice Emitted whenever a message is sent to the other chain. * @notice Emitted whenever a message is sent to the other chain.
...@@ -178,6 +181,21 @@ abstract contract CrossDomainMessenger is ...@@ -178,6 +181,21 @@ abstract contract CrossDomainMessenger is
*/ */
event FailedRelayedMessage(bytes32 indexed msgHash); event FailedRelayedMessage(bytes32 indexed msgHash);
/**
* @notice Modifier for a per-message reentrancy guard.
*/
modifier perMessageNonReentrant(bytes32 _msgHash) {
// Check if the reentrancy lock for the `_msgHash` is already set.
if (reentrancyLocks[_msgHash]) {
revert("ReentrancyGuard: reentrant call");
}
// Trigger the reentrancy lock for the `_msgHash`.
reentrancyLocks[_msgHash] = true;
_;
// Clear the reentrancy lock for the `_msgHash`.
reentrancyLocks[_msgHash] = false;
}
/** /**
* @param _otherMessenger Address of the messenger on the paired chain. * @param _otherMessenger Address of the messenger on the paired chain.
*/ */
......
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