Commit 29f1c228 authored by Kelvin Fichter's avatar Kelvin Fichter

fix: correctly replay messages

parent 66f5af26
---
'@eth-optimism/contracts': patch
---
Fixes a bug that made replayMessage useless.
...@@ -51,13 +51,15 @@ interface IL1CrossDomainMessenger is ICrossDomainMessenger { ...@@ -51,13 +51,15 @@ interface IL1CrossDomainMessenger is ICrossDomainMessenger {
* @param _sender Original sender address. * @param _sender Original sender address.
* @param _message Message to send to the target. * @param _message Message to send to the target.
* @param _queueIndex CTC Queue index for the message to replay. * @param _queueIndex CTC Queue index for the message to replay.
* @param _gasLimit Gas limit for the provided message. * @param _oldGasLimit Original gas limit used to send the message.
* @param _newGasLimit New gas limit to be used for this message.
*/ */
function replayMessage( function replayMessage(
address _target, address _target,
address _sender, address _sender,
bytes memory _message, bytes memory _message,
uint256 _queueIndex, uint256 _queueIndex,
uint32 _gasLimit uint32 _oldGasLimit,
uint32 _newGasLimit
) external; ) external;
} }
...@@ -269,7 +269,8 @@ contract L1CrossDomainMessenger is ...@@ -269,7 +269,8 @@ contract L1CrossDomainMessenger is
address _sender, address _sender,
bytes memory _message, bytes memory _message,
uint256 _queueIndex, uint256 _queueIndex,
uint32 _gasLimit uint32 _oldGasLimit,
uint32 _newGasLimit
) )
public public
{ {
...@@ -278,32 +279,35 @@ contract L1CrossDomainMessenger is ...@@ -278,32 +279,35 @@ contract L1CrossDomainMessenger is
Lib_OVMCodec.QueueElement memory element = Lib_OVMCodec.QueueElement memory element =
ICanonicalTransactionChain(canonicalTransactionChain).getQueueElement(_queueIndex); ICanonicalTransactionChain(canonicalTransactionChain).getQueueElement(_queueIndex);
// Compute the calldata that was originally used to send the message.
bytes memory xDomainCalldata = Lib_CrossDomainUtils.encodeXDomainCalldata(
_target,
_sender,
_message,
_queueIndex
);
// Compute the transactionHash // Compute the transactionHash
bytes32 transactionHash = keccak256( bytes32 transactionHash = keccak256(
abi.encode( abi.encode(
AddressAliasHelper.applyL1ToL2Alias(address(this)), AddressAliasHelper.applyL1ToL2Alias(address(this)),
Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER, Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER,
_gasLimit, _oldGasLimit,
_message xDomainCalldata
) )
); );
// Now check that the provided message data matches the one in the queue element.
require( require(
transactionHash == element.transactionHash, transactionHash == element.transactionHash,
"Provided message has not been enqueued." "Provided message has not been enqueued."
); );
bytes memory xDomainCalldata = Lib_CrossDomainUtils.encodeXDomainCalldata( // Send the same message but with the new gas limit.
_target,
_sender,
_message,
_queueIndex
);
_sendXDomainMessage( _sendXDomainMessage(
canonicalTransactionChain, canonicalTransactionChain,
xDomainCalldata, xDomainCalldata,
_gasLimit _newGasLimit
); );
} }
......
...@@ -23,8 +23,11 @@ import { ...@@ -23,8 +23,11 @@ import {
TrieTestGenerator, TrieTestGenerator,
getNextBlockNumber, getNextBlockNumber,
encodeXDomainCalldata, encodeXDomainCalldata,
getEthTime,
setEthTime,
} from '../../../helpers' } from '../../../helpers'
import { predeploys } from '../../../../src' import { predeploys } from '../../../../src'
import { keccak256 } from 'ethers/lib/utils'
const MAX_GAS_LIMIT = 8_000_000 const MAX_GAS_LIMIT = 8_000_000
...@@ -206,40 +209,146 @@ describe('L1CrossDomainMessenger', () => { ...@@ -206,40 +209,146 @@ describe('L1CrossDomainMessenger', () => {
describe('replayMessage', () => { describe('replayMessage', () => {
const target = NON_ZERO_ADDRESS const target = NON_ZERO_ADDRESS
const message = NON_NULL_BYTES32 const message = NON_NULL_BYTES32
const gasLimit = 100_000 const oldGasLimit = 100_000
const newGasLimit = 200_000
it('should revert if given the wrong queue index', async () => { let sender: string
await L1CrossDomainMessenger.sendMessage(target, message, 100_001) before(async () => {
sender = await signer.getAddress()
})
let queueIndex: number
beforeEach(async () => {
await L1CrossDomainMessenger.connect(signer).sendMessage(
target,
message,
oldGasLimit
)
const queueLength = await CanonicalTransactionChain.getQueueLength() const queueLength = await CanonicalTransactionChain.getQueueLength()
await expect( queueIndex = queueLength - 1
L1CrossDomainMessenger.replayMessage( })
target,
await signer.getAddress(), describe('when giving some incorrect input value', async () => {
message, it('should revert if given the wrong target', async () => {
queueLength - 1, await expect(
gasLimit L1CrossDomainMessenger.replayMessage(
ethers.constants.AddressZero, // Wrong target
sender,
message,
queueIndex,
oldGasLimit,
newGasLimit
)
).to.be.revertedWith('Provided message has not been enqueued.')
})
it('should revert if given the wrong sender', async () => {
await expect(
L1CrossDomainMessenger.replayMessage(
target,
ethers.constants.AddressZero, // Wrong sender
message,
queueIndex,
oldGasLimit,
newGasLimit
)
).to.be.revertedWith('Provided message has not been enqueued.')
})
it('should revert if given the wrong message', async () => {
await expect(
L1CrossDomainMessenger.replayMessage(
target,
sender,
'0x', // Wrong message
queueIndex,
oldGasLimit,
newGasLimit
)
).to.be.revertedWith('Provided message has not been enqueued.')
})
it('should revert if given the wrong queue index', async () => {
await expect(
L1CrossDomainMessenger.replayMessage(
target,
sender,
message,
queueIndex - 1, // Wrong queue index
oldGasLimit,
newGasLimit
)
).to.be.revertedWith('Provided message has not been enqueued.')
})
it('should revert if given the wrong old gas limit', async () => {
await expect(
L1CrossDomainMessenger.replayMessage(
target,
sender,
message,
queueIndex,
oldGasLimit + 1, // Wrong gas limit
newGasLimit
)
).to.be.revertedWith('Provided message has not been enqueued.')
})
})
describe('when all input values are the same as the existing message', () => {
it('should succeed', async () => {
await expect(
L1CrossDomainMessenger.replayMessage(
target,
sender,
message,
queueIndex,
oldGasLimit,
newGasLimit
)
).to.not.be.reverted
})
it('should emit the TransactionEnqueued event', async () => {
const newQueueIndex = await CanonicalTransactionChain.getQueueLength()
const newTimestamp = (await getEthTime(ethers.provider)) + 100
await setEthTime(ethers.provider, newTimestamp)
await expect(
L1CrossDomainMessenger.replayMessage(
target,
sender,
message,
queueIndex,
oldGasLimit,
newGasLimit
)
) )
).to.be.revertedWith('Provided message has not been enqueued.') .to.emit(CanonicalTransactionChain, 'TransactionEnqueued')
.withArgs(
applyL1ToL2Alias(L1CrossDomainMessenger.address),
Mock__L2CrossDomainMessenger.address,
newGasLimit,
encodeXDomainCalldata(target, sender, message, queueIndex),
newQueueIndex,
newTimestamp
)
})
}) })
it('should succeed if the message exists', async () => { it('should succeed if all inputs are the same as the existing message', async () => {
await L1CrossDomainMessenger.sendMessage(target, message, gasLimit) await L1CrossDomainMessenger.sendMessage(target, message, oldGasLimit)
const queueLength = await CanonicalTransactionChain.getQueueLength() const queueLength = await CanonicalTransactionChain.getQueueLength()
const calldata = encodeXDomainCalldata(
target,
await signer.getAddress(),
message,
queueLength - 1
)
await expect( await expect(
L1CrossDomainMessenger.replayMessage( L1CrossDomainMessenger.replayMessage(
Mock__L2CrossDomainMessenger.address, target,
await signer.getAddress(), await signer.getAddress(),
calldata, message,
queueLength - 1, queueLength - 1,
gasLimit oldGasLimit,
newGasLimit
) )
).to.not.be.reverted ).to.not.be.reverted
}) })
......
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