Commit 8e9ea053 authored by Georgios Konstantopoulos's avatar Georgios Konstantopoulos Committed by GitHub

perf: reset xDomainMessageSender to default value after external call (#316)

* perf: reset xDomainMessageSender to default value after external call

Fixes https://github.com/ethereum-optimism/contracts/issues/309

We also set `xDomainMessageSender in the `initialize` function in L1, and in the L2 constructor.

* test: ensure that xDomainMessageSender is always 0xdead before/after calls

* chore: use parent constructor for initializing the L2 xdomain sender

* fix: require that the xDomainSender is not the default value
parent 2e16f392
...@@ -18,6 +18,14 @@ import { Lib_ReentrancyGuard } from "../../../libraries/utils/Lib_ReentrancyGuar ...@@ -18,6 +18,14 @@ import { Lib_ReentrancyGuard } from "../../../libraries/utils/Lib_ReentrancyGuar
* Runtime target: defined by child contract * Runtime target: defined by child contract
*/ */
abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger, Lib_ReentrancyGuard { abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger, Lib_ReentrancyGuard {
/**************
* Constants *
**************/
// The default x-domain message sender being set to a non-zero value makes
// deployment a bit more expensive, but in exchange the refund on every call to
// `relayMessage` by the L1 and L2 messengers will be higher.
address internal constant DEFAULT_XDOMAIN_SENDER = 0x000000000000000000000000000000000000dEaD;
/********************** /**********************
* Contract Variables * * Contract Variables *
...@@ -27,13 +35,18 @@ abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger, ...@@ -27,13 +35,18 @@ abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger,
mapping (bytes32 => bool) public successfulMessages; mapping (bytes32 => bool) public successfulMessages;
mapping (bytes32 => bool) public sentMessages; mapping (bytes32 => bool) public sentMessages;
uint256 public messageNonce; uint256 public messageNonce;
address override public xDomainMessageSender; address internal xDomainMsgSender = DEFAULT_XDOMAIN_SENDER;
/******************** /********************
* Public Functions * * Public Functions *
********************/ ********************/
constructor() Lib_ReentrancyGuard() {} constructor() Lib_ReentrancyGuard() internal {}
function xDomainMessageSender() public override view returns (address) {
require(xDomainMsgSender != DEFAULT_XDOMAIN_SENDER, "xDomainMessageSender is not set");
return xDomainMsgSender;
}
/** /**
* Sends a cross domain message to the target messenger. * Sends a cross domain message to the target messenger.
......
...@@ -49,6 +49,7 @@ contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCros ...@@ -49,6 +49,7 @@ contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCros
{ {
require(address(libAddressManager) == address(0), "L1CrossDomainMessenger already intialized."); require(address(libAddressManager) == address(0), "L1CrossDomainMessenger already intialized.");
libAddressManager = Lib_AddressManager(_libAddressManager); libAddressManager = Lib_AddressManager(_libAddressManager);
xDomainMsgSender = DEFAULT_XDOMAIN_SENDER;
} }
...@@ -113,8 +114,9 @@ contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCros ...@@ -113,8 +114,9 @@ contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCros
"Provided message has already been received." "Provided message has already been received."
); );
xDomainMessageSender = _sender; xDomainMsgSender = _sender;
(bool success, ) = _target.call(_message); (bool success, ) = _target.call(_message);
xDomainMsgSender = DEFAULT_XDOMAIN_SENDER;
// Mark the message as received if the call was successful. Ensures that a message can be // Mark the message as received if the call was successful. Ensures that a message can be
// relayed multiple times in the case that the call reverted. // relayed multiple times in the case that the call reverted.
......
...@@ -75,8 +75,9 @@ contract OVM_L2CrossDomainMessenger is iOVM_L2CrossDomainMessenger, Abs_BaseCros ...@@ -75,8 +75,9 @@ contract OVM_L2CrossDomainMessenger is iOVM_L2CrossDomainMessenger, Abs_BaseCros
"Provided message has already been received." "Provided message has already been received."
); );
xDomainMessageSender = _sender; xDomainMsgSender = _sender;
(bool success, ) = _target.call(_message); (bool success, ) = _target.call(_message);
xDomainMsgSender = DEFAULT_XDOMAIN_SENDER;
// Mark the message as received if the call was successful. Ensures that a message can be // Mark the message as received if the call was successful. Ensures that a message can be
// relayed multiple times in the case that the call reverted. // relayed multiple times in the case that the call reverted.
......
...@@ -320,6 +320,22 @@ describe('OVM_L1CrossDomainMessenger', () => { ...@@ -320,6 +320,22 @@ describe('OVM_L1CrossDomainMessenger', () => {
).to.equal(true) ).to.equal(true)
}) })
it('the xDomainMessageSender is reset to the original value', async () => {
await expect(
OVM_L1CrossDomainMessenger.xDomainMessageSender()
).to.be.revertedWith('xDomainMessageSender is not set')
await OVM_L1CrossDomainMessenger.relayMessage(
target,
sender,
message,
0,
proof
)
await expect(
OVM_L1CrossDomainMessenger.xDomainMessageSender()
).to.be.revertedWith('xDomainMessageSender is not set')
})
it('should revert if trying to send the same message twice', async () => { it('should revert if trying to send the same message twice', async () => {
await OVM_L1CrossDomainMessenger.relayMessage( await OVM_L1CrossDomainMessenger.relayMessage(
target, target,
......
...@@ -137,6 +137,16 @@ describe('OVM_L2CrossDomainMessenger', () => { ...@@ -137,6 +137,16 @@ describe('OVM_L2CrossDomainMessenger', () => {
]) ])
}) })
it('the xDomainMessageSender is reset to the original value', async () => {
await expect(
OVM_L2CrossDomainMessenger.xDomainMessageSender()
).to.be.revertedWith('xDomainMessageSender is not set')
await OVM_L2CrossDomainMessenger.relayMessage(target, sender, message, 0)
await expect(
OVM_L2CrossDomainMessenger.xDomainMessageSender()
).to.be.revertedWith('xDomainMessageSender is not set')
})
it('should revert if trying to send the same message twice', async () => { it('should revert if trying to send the same message twice', async () => {
Mock__OVM_L1MessageSender.smocked.getL1MessageSender.will.return.with( Mock__OVM_L1MessageSender.smocked.getL1MessageSender.will.return.with(
Mock__OVM_L1CrossDomainMessenger.address Mock__OVM_L1CrossDomainMessenger.address
......
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