Skip to content

Commit

Permalink
Get rid of modifier
Browse files Browse the repository at this point in the history
  • Loading branch information
clabby committed Feb 21, 2023
1 parent 5313991 commit 6e34593
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 55 deletions.
2 changes: 1 addition & 1 deletion op-bindings/bindings/l1crossdomainmessenger.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion op-bindings/bindings/l1crossdomainmessenger_more.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion op-bindings/bindings/l2crossdomainmessenger.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion op-bindings/bindings/l2crossdomainmessenger_more.go

Large diffs are not rendered by default.

52 changes: 26 additions & 26 deletions packages/contracts-bedrock/.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_su
CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10530)
CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34861)
CrossDomainOwnable2_Test:test_onlyOwner_notMessenger_reverts() (gas: 8416)
CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 64616)
CrossDomainOwnable2_Test:test_onlyOwner_notOwner2_reverts() (gas: 63695)
CrossDomainOwnable2_Test:test_onlyOwner_notOwner_reverts() (gas: 16588)
CrossDomainOwnable2_Test:test_onlyOwner_succeeds() (gas: 79364)
CrossDomainOwnable2_Test:test_onlyOwner_succeeds() (gas: 78212)
CrossDomainOwnable3_Test:test_constructor_succeeds() (gas: 10576)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notMessenger_reverts() (gas: 28289)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner2_reverts() (gas: 79746)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner2_reverts() (gas: 78594)
CrossDomainOwnable3_Test:test_crossDomainOnlyOwner_notOwner_reverts() (gas: 31978)
CrossDomainOwnable3_Test:test_crossDomainTransferOwnership_succeeds() (gas: 97324)
CrossDomainOwnable3_Test:test_crossDomainTransferOwnership_succeeds() (gas: 96172)
CrossDomainOwnable3_Test:test_localOnlyOwner_notOwner_reverts() (gas: 13215)
CrossDomainOwnable3_Test:test_localOnlyOwner_succeeds() (gas: 35220)
CrossDomainOwnable3_Test:test_localTransferOwnership_succeeds() (gas: 52128)
Expand Down Expand Up @@ -71,24 +71,24 @@ L1BlockNumberTest:test_receive_succeeds() (gas: 25340)
L1CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 24781)
L1CrossDomainMessenger_Test:test_pause_callerIsNotOwner_reverts() (gas: 24517)
L1CrossDomainMessenger_Test:test_pause_succeeds() (gas: 52964)
L1CrossDomainMessenger_Test:test_relayMessage_legacyOldReplay_reverts() (gas: 74933)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 233723)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 206083)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterSuccess_reverts() (gas: 146213)
L1CrossDomainMessenger_Test:test_relayMessage_legacy_succeeds() (gas: 78034)
L1CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 78734)
L1CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 727261)
L1CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 662167)
L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 200174)
L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 75073)
L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 102686)
L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 12562)
L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 56661)
L1CrossDomainMessenger_Test:test_relayMessage_legacyOldReplay_reverts() (gas: 51545)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailureThenSuccess_reverts() (gas: 230701)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterFailure_succeeds() (gas: 204067)
L1CrossDomainMessenger_Test:test_relayMessage_legacyRetryAfterSuccess_reverts() (gas: 144199)
L1CrossDomainMessenger_Test:test_relayMessage_legacy_succeeds() (gas: 77026)
L1CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 55447)
L1CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 723099)
L1CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 660069)
L1CrossDomainMessenger_Test:test_relayMessage_retryAfterFailure_succeeds() (gas: 197996)
L1CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 73984)
L1CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 100511)
L1CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 14471)
L1CrossDomainMessenger_Test:test_replayMessage_withValue_reverts() (gas: 55573)
L1CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 299710)
L1CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 1490566)
L1CrossDomainMessenger_Test:test_unpause_callerIsNotOwner_reverts() (gas: 24538)
L1CrossDomainMessenger_Test:test_unpause_succeeds() (gas: 45185)
L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 85438)
L1CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 84446)
L1CrossDomainMessenger_Test:test_xDomainSender_notSet_reverts() (gas: 24274)
L1ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 52730)
L1ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 27332)
Expand Down Expand Up @@ -127,16 +127,16 @@ L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 520198)
L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8412)
L2CrossDomainMessenger_Test:test_pause_notOwner_reverts() (gas: 10860)
L2CrossDomainMessenger_Test:test_pause_succeeds() (gas: 31846)
L2CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 59787)
L2CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 687927)
L2CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 631400)
L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 171357)
L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 57712)
L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 54619)
L2CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 11925)
L2CrossDomainMessenger_Test:test_relayMessage_paused_reverts() (gas: 36500)
L2CrossDomainMessenger_Test:test_relayMessage_reentrancyDiffMessage_succeeds() (gas: 683845)
L2CrossDomainMessenger_Test:test_relayMessage_reentrancySameMessage_reverts() (gas: 629342)
L2CrossDomainMessenger_Test:test_relayMessage_retry_succeeds() (gas: 169219)
L2CrossDomainMessenger_Test:test_relayMessage_succeeds() (gas: 56856)
L2CrossDomainMessenger_Test:test_relayMessage_toSystemContract_reverts() (gas: 53532)
L2CrossDomainMessenger_Test:test_relayMessage_v2_reverts() (gas: 13839)
L2CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 122621)
L2CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 134738)
L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 57144)
L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 56367)
L2CrossDomainMessenger_Test:test_xDomainSender_senderNotSet_reverts() (gas: 10524)
L2ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 26454)
L2ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 21770)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
vm.store(address(op), bytes32(senderSlotIndex), bytes32(abi.encode(sender)));

// Expect a revert.
vm.expectRevert("Hashing: unknown cross domain message version");
vm.expectRevert(
"CrossDomainMessenger: only version 0 or 1 messages are supported at this time"
);

// Try to relay a v2 message.
vm.prank(address(op));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
address caller = AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger));

// Expect a revert.
vm.expectRevert("Hashing: unknown cross domain message version");
vm.expectRevert(
"CrossDomainMessenger: only version 0 or 1 messages are supported at this time"
);

// Try to relay a v2 message.
vm.prank(caller);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,21 +181,6 @@ abstract contract CrossDomainMessenger is
*/
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.
*/
Expand Down Expand Up @@ -280,15 +265,13 @@ abstract contract CrossDomainMessenger is
uint256 _value,
uint256 _minGasLimit,
bytes calldata _message
)
external
payable
perMessageNonReentrant(
Hashing.hashCrossDomainMessage(_nonce, _sender, _target, _value, _minGasLimit, _message)
)
whenNotPaused
{
) external payable whenNotPaused {
(, uint16 version) = Encoding.decodeVersionedNonce(_nonce);
require(
version < 2,
"CrossDomainMessenger: only version 0 or 1 messages are supported at this time"
);

// If the message is version 0, then it's a migrated legacy withdrawal. We therefore need
// to check that the legacy version of the message has not already been relayed.
if (version == 0) {
Expand All @@ -310,6 +293,13 @@ 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).
Expand Down Expand Up @@ -362,6 +352,9 @@ abstract contract CrossDomainMessenger is
revert("CrossDomainMessenger: failed to relay message");
}
}

// Clear the reentrancy lock for `versionedHash`
reentrancyLocks[versionedHash] = false;
}

/**
Expand Down

0 comments on commit 6e34593

Please sign in to comment.