Skip to content

Commit

Permalink
Merge branch 'ccip-develop' into CCIP-2837-multi-on-ramp-make-router-…
Browse files Browse the repository at this point in the history 
…destination-chain-specific-2
  • Loading branch information
@RyanRHall
RyanRHall committed Aug 5, 2024
2 parents 430e19d + b446f45 commit 0f1ae75
Show file tree
Hide file tree
Showing 33 changed files with 1,120 additions and 381 deletions.
5 changes: 5 additions & 0 deletions .changeset/cool-mirrors-beg.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#added L3X Config
202 changes: 101 additions & 101 deletions contracts/gas-snapshots/ccip.gas-snapshot
View file

Large diffs are not rendered by default.

20 changes: 10 additions & 10 deletions contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base {
using ERC165Checker for address;
using EnumerableMapAddresses for EnumerableMapAddresses.AddressToAddressMap;

error AlreadyAttempted(uint64 sourceChainSelector, uint64 sequenceNumber);
error AlreadyExecuted(uint64 sourceChainSelector, uint64 sequenceNumber);
error ZeroChainSelectorNotAllowed();
error ExecutionError(bytes32 messageId, bytes err);
Expand Down Expand Up @@ -74,6 +73,7 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base {
event SourceChainSelectorAdded(uint64 sourceChainSelector);
event SourceChainConfigSet(uint64 indexed sourceChainSelector, SourceChainConfig sourceConfig);
event SkippedAlreadyExecutedMessage(uint64 sourceChainSelector, uint64 sequenceNumber);
event AlreadyAttempted(uint64 sourceChainSelector, uint64 sequenceNumber);
/// @dev RMN depends on this event, if changing, please notify the RMN maintainers.
event CommitReportAccepted(CommitReport report);
event RootRemoved(bytes32 root);
Expand Down Expand Up @@ -376,13 +376,6 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base {

Internal.MessageExecutionState originalState =
getExecutionState(sourceChainSelector, message.header.sequenceNumber);
if (originalState == Internal.MessageExecutionState.SUCCESS) {
// If the message has already been executed, we skip it. We want to not revert on race conditions between
// executing parties. This will allow us to open up manual exec while also attempting with the DON, without
// reverting an entire DON batch when a user manually executes while the tx is inflight.
emit SkippedAlreadyExecutedMessage(sourceChainSelector, message.header.sequenceNumber);
continue;
}
// Two valid cases here, we either have never touched this message before, or we tried to execute
// and failed. This check protects against reentry and re-execution because the other state is
// IN_PROGRESS which should not be allowed to execute.
Expand All @@ -391,7 +384,13 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base {
originalState == Internal.MessageExecutionState.UNTOUCHED
|| originalState == Internal.MessageExecutionState.FAILURE
)
) revert AlreadyExecuted(sourceChainSelector, message.header.sequenceNumber);
) {
// If the message has already been executed, we skip it. We want to not revert on race conditions between
// executing parties. This will allow us to open up manual exec while also attempting with the DON, without
// reverting an entire DON batch when a user manually executes while the tx is inflight.
emit SkippedAlreadyExecutedMessage(sourceChainSelector, message.header.sequenceNumber);
continue;
}

if (manualExecution) {
bool isOldCommitReport =
Expand All @@ -410,7 +409,8 @@ contract EVM2EVMMultiOffRamp is ITypeAndVersion, MultiOCR3Base {
// DON can only execute a message once
// Acceptable state transitions: UNTOUCHED->SUCCESS, UNTOUCHED->FAILURE
if (originalState != Internal.MessageExecutionState.UNTOUCHED) {
revert AlreadyAttempted(sourceChainSelector, message.header.sequenceNumber);
emit AlreadyAttempted(sourceChainSelector, message.header.sequenceNumber);
continue;
}
}

Expand Down
22 changes: 12 additions & 10 deletions contracts/src/v0.8/ccip/offRamp/EVM2EVMOffRamp.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
using ERC165Checker for address;
using EnumerableMapAddresses for EnumerableMapAddresses.AddressToAddressMap;

error AlreadyAttempted(uint64 sequenceNumber);
error AlreadyExecuted(uint64 sequenceNumber);
error ZeroAddressNotAllowed();
error CommitStoreAlreadyInUse();
Expand Down Expand Up @@ -69,6 +68,7 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
event TokenAggregateRateLimitAdded(address sourceToken, address destToken);
event TokenAggregateRateLimitRemoved(address sourceToken, address destToken);
event SkippedAlreadyExecutedMessage(uint64 indexed sequenceNumber);
event AlreadyAttempted(uint64 sequenceNumber);

/// @notice Static offRamp config
/// @dev RMN depends on this struct, if changing, please notify the RMN maintainers.
Expand Down Expand Up @@ -276,13 +276,6 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
for (uint256 i = 0; i < numMsgs; ++i) {
Internal.EVM2EVMMessage memory message = report.messages[i];
Internal.MessageExecutionState originalState = getExecutionState(message.sequenceNumber);
if (originalState == Internal.MessageExecutionState.SUCCESS) {
// If the message has already been executed, we skip it. We want to not revert on race conditions between
// executing parties. This will allow us to open up manual exec while also attempting with the DON, without
// reverting an entire DON batch when a user manually executes while the tx is inflight.
emit SkippedAlreadyExecutedMessage(message.sequenceNumber);
continue;
}
// Two valid cases here, we either have never touched this message before, or we tried to execute
// and failed. This check protects against reentry and re-execution because the other state is
// IN_PROGRESS which should not be allowed to execute.
Expand All @@ -291,7 +284,13 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
originalState == Internal.MessageExecutionState.UNTOUCHED
|| originalState == Internal.MessageExecutionState.FAILURE
)
) revert AlreadyExecuted(message.sequenceNumber);
) {
// If the message has already been executed, we skip it. We want to not revert on race conditions between
// executing parties. This will allow us to open up manual exec while also attempting with the DON, without
// reverting an entire DON batch when a user manually executes while the tx is inflight.
emit SkippedAlreadyExecutedMessage(message.sequenceNumber);
continue;
}

if (manualExecution) {
bool isOldCommitReport =
Expand All @@ -309,7 +308,10 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
} else {
// DON can only execute a message once
// Acceptable state transitions: UNTOUCHED->SUCCESS, UNTOUCHED->FAILURE
if (originalState != Internal.MessageExecutionState.UNTOUCHED) revert AlreadyAttempted(message.sequenceNumber);
if (originalState != Internal.MessageExecutionState.UNTOUCHED) {
emit AlreadyAttempted(message.sequenceNumber);
continue;
}
}

if (message.nonce != 0) {
Expand Down
34 changes: 15 additions & 19 deletions contracts/src/v0.8/ccip/test/offRamp/EVM2EVMMultiOffRamp.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -952,11 +952,10 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup {
);
s_offRamp.executeSingleReport(_generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[](0));

vm.expectRevert(
abi.encodeWithSelector(
EVM2EVMMultiOffRamp.AlreadyAttempted.selector, SOURCE_CHAIN_SELECTOR_1, messages[0].header.sequenceNumber
)
);
// The second time should skip the msg
vm.expectEmit();
emit EVM2EVMMultiOffRamp.AlreadyAttempted(SOURCE_CHAIN_SELECTOR_1, messages[0].header.sequenceNumber);

s_offRamp.executeSingleReport(_generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[](0));
}

Expand Down Expand Up @@ -1688,7 +1687,7 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup {
s_offRamp.manuallyExecute(_generateBatchReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), gasLimitOverrides);
}

function test_manuallyExecute_ReentrancyFails() public {
function test_manuallyExecute_ReentrancyFails_Success() public {
uint256 tokenAmount = 1e9;
IERC20 tokenToAbuse = IERC20(s_destFeeToken);

Expand Down Expand Up @@ -1722,29 +1721,26 @@ contract EVM2EVMMultiOffRamp_manuallyExecute is EVM2EVMMultiOffRampSetup {
uint256[][] memory gasLimitOverrides = new uint256[][](1);
gasLimitOverrides[0] = _getGasLimitsFromMessages(messages);

// The first entry should be fine and triggers the second entry. This one fails
// but since it's an inner tx of the first one it is caught in the try-catch.
// This means the first tx is marked `FAILURE` with the error message of the second tx.
// The first entry should be fine and triggers the second entry which is skipped. Due to the reentrancy
// the second completes first, so we expect the skip event before the success event.
vm.expectEmit();
emit EVM2EVMMultiOffRamp.SkippedAlreadyExecutedMessage(
messages[0].header.sourceChainSelector, messages[0].header.sequenceNumber
);

vm.expectEmit();
emit EVM2EVMMultiOffRamp.ExecutionStateChanged(
messages[0].header.sourceChainSelector,
messages[0].header.sequenceNumber,
messages[0].header.messageId,
Internal.MessageExecutionState.FAILURE,
abi.encodeWithSelector(
EVM2EVMMultiOffRamp.ReceiverError.selector,
abi.encodeWithSelector(
EVM2EVMMultiOffRamp.AlreadyExecuted.selector,
messages[0].header.sourceChainSelector,
messages[0].header.sequenceNumber
)
)
Internal.MessageExecutionState.SUCCESS,
""
);

s_offRamp.manuallyExecute(_generateBatchReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), gasLimitOverrides);

// Since the tx failed we don't release the tokens
assertEq(tokenToAbuse.balanceOf(address(receiver)), balancePre);
assertEq(tokenToAbuse.balanceOf(address(receiver)), balancePre + tokenAmount);
}
}

Expand Down
159 changes: 79 additions & 80 deletions contracts/src/v0.8/ccip/test/offRamp/EVM2EVMOffRamp.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,36 @@ contract EVM2EVMOffRamp_execute is EVM2EVMOffRampSetup {
s_offRamp.execute(executionReport, new uint256[](0));
}

function test_RetryFailedMessageWithoutManualExecution_Success() public {
Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage();

bytes memory realError1 = new bytes(2);
realError1[0] = 0xbe;
realError1[1] = 0xef;
s_reverting_receiver.setErr(realError1);

messages[0].receiver = address(s_reverting_receiver);
messages[0].messageId = Internal._hash(messages[0], s_offRamp.metadataHash());

vm.expectEmit();
emit EVM2EVMOffRamp.ExecutionStateChanged(
messages[0].sequenceNumber,
messages[0].messageId,
Internal.MessageExecutionState.FAILURE,
abi.encodeWithSelector(
EVM2EVMOffRamp.ReceiverError.selector,
abi.encodeWithSelector(MaybeRevertMessageReceiver.CustomError.selector, realError1)
)
);
s_offRamp.execute(_generateReportFromMessages(messages), new uint256[](0));

// The second time should skip the msg
vm.expectEmit();
emit EVM2EVMOffRamp.AlreadyAttempted(messages[0].sequenceNumber);

s_offRamp.execute(_generateReportFromMessages(messages), new uint256[](0));
}

// Reverts

function test_InvalidMessageId_Revert() public {
Expand Down Expand Up @@ -744,33 +774,6 @@ contract EVM2EVMOffRamp_execute is EVM2EVMOffRampSetup {
);
s_offRamp.execute(executionReport, new uint256[](0));
}

function test_RetryFailedMessageWithoutManualExecution_Revert() public {
Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage();

bytes memory realError1 = new bytes(2);
realError1[0] = 0xbe;
realError1[1] = 0xef;
s_reverting_receiver.setErr(realError1);

messages[0].receiver = address(s_reverting_receiver);
messages[0].messageId = Internal._hash(messages[0], s_offRamp.metadataHash());

vm.expectEmit();
emit EVM2EVMOffRamp.ExecutionStateChanged(
messages[0].sequenceNumber,
messages[0].messageId,
Internal.MessageExecutionState.FAILURE,
abi.encodeWithSelector(
EVM2EVMOffRamp.ReceiverError.selector,
abi.encodeWithSelector(MaybeRevertMessageReceiver.CustomError.selector, realError1)
)
);
s_offRamp.execute(_generateReportFromMessages(messages), new uint256[](0));

vm.expectRevert(abi.encodeWithSelector(EVM2EVMOffRamp.AlreadyAttempted.selector, messages[0].sequenceNumber));
s_offRamp.execute(_generateReportFromMessages(messages), new uint256[](0));
}
}

contract EVM2EVMOffRamp_execute_upgrade is EVM2EVMOffRampSetup {
Expand Down Expand Up @@ -1144,6 +1147,55 @@ contract EVM2EVMOffRamp_manuallyExecute is EVM2EVMOffRampSetup {
s_offRamp.manuallyExecute(_generateReportFromMessages(messages), gasLimitOverrides);
}

function test_ReentrancyManualExecuteFails_Success() public {
uint256 tokenAmount = 1e9;
IERC20 tokenToAbuse = IERC20(s_destFeeToken);

// This needs to be deployed before the source chain message is sent
// because we need the address for the receiver.
ReentrancyAbuser receiver = new ReentrancyAbuser(address(s_destRouter), s_offRamp);
uint256 balancePre = tokenToAbuse.balanceOf(address(receiver));

// For this test any message will be flagged as correct by the
// commitStore. In a real scenario the abuser would have to actually
// send the message that they want to replay.
Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage();
messages[0].tokenAmounts = new Client.EVMTokenAmount[](1);
messages[0].tokenAmounts[0] = Client.EVMTokenAmount({token: s_sourceFeeToken, amount: tokenAmount});
messages[0].receiver = address(receiver);
messages[0].sourceTokenData = new bytes[](1);
messages[0].sourceTokenData[0] = abi.encode(
Internal.SourceTokenData({
sourcePoolAddress: abi.encode(s_sourcePoolByToken[s_sourceFeeToken]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[s_sourceFeeToken]),
extraData: "",
destGasAmount: DEFAULT_TOKEN_DEST_GAS_OVERHEAD
})
);

messages[0].messageId = Internal._hash(messages[0], s_offRamp.metadataHash());

Internal.ExecutionReport memory report = _generateReportFromMessages(messages);

// sets the report to be repeated on the ReentrancyAbuser to be able to replay
receiver.setPayload(report);

// The first entry should be fine and triggers the second entry which is skipped. Due to the reentrancy
// the second completes first, so we expect the skip event before the success event.
vm.expectEmit();
emit EVM2EVMOffRamp.SkippedAlreadyExecutedMessage(messages[0].sequenceNumber);

vm.expectEmit();
emit EVM2EVMOffRamp.ExecutionStateChanged(
messages[0].sequenceNumber, messages[0].messageId, Internal.MessageExecutionState.SUCCESS, ""
);

s_offRamp.manuallyExecute(report, _getGasLimitsFromMessages(messages));

// Assert that they only got the tokens once, not twice
assertEq(tokenToAbuse.balanceOf(address(receiver)), balancePre + tokenAmount);
}

function test_ManualExecForkedChain_Revert() public {
Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage();

Expand Down Expand Up @@ -1200,59 +1252,6 @@ contract EVM2EVMOffRamp_manuallyExecute is EVM2EVMOffRampSetup {
);
s_offRamp.manuallyExecute(_generateReportFromMessages(messages), _getGasLimitsFromMessages(messages));
}

function test_ReentrancyManualExecuteFails() public {
uint256 tokenAmount = 1e9;
IERC20 tokenToAbuse = IERC20(s_destFeeToken);

// This needs to be deployed before the source chain message is sent
// because we need the address for the receiver.
ReentrancyAbuser receiver = new ReentrancyAbuser(address(s_destRouter), s_offRamp);
uint256 balancePre = tokenToAbuse.balanceOf(address(receiver));

// For this test any message will be flagged as correct by the
// commitStore. In a real scenario the abuser would have to actually
// send the message that they want to replay.
Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage();
messages[0].tokenAmounts = new Client.EVMTokenAmount[](1);
messages[0].tokenAmounts[0] = Client.EVMTokenAmount({token: s_sourceFeeToken, amount: tokenAmount});
messages[0].receiver = address(receiver);
messages[0].sourceTokenData = new bytes[](1);
messages[0].sourceTokenData[0] = abi.encode(
Internal.SourceTokenData({
sourcePoolAddress: abi.encode(s_sourcePoolByToken[s_sourceFeeToken]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[s_sourceFeeToken]),
extraData: "",
destGasAmount: DEFAULT_TOKEN_DEST_GAS_OVERHEAD
})
);

messages[0].messageId = Internal._hash(messages[0], s_offRamp.metadataHash());

Internal.ExecutionReport memory report = _generateReportFromMessages(messages);

// sets the report to be repeated on the ReentrancyAbuser to be able to replay
receiver.setPayload(report);

// The first entry should be fine and triggers the second entry. This one fails
// but since it's an inner tx of the first one it is caught in the try-catch.
// This means the first tx is marked `FAILURE` with the error message of the second tx.
vm.expectEmit();
emit EVM2EVMOffRamp.ExecutionStateChanged(
messages[0].sequenceNumber,
messages[0].messageId,
Internal.MessageExecutionState.FAILURE,
abi.encodeWithSelector(
EVM2EVMOffRamp.ReceiverError.selector,
abi.encodeWithSelector(EVM2EVMOffRamp.AlreadyExecuted.selector, messages[0].sequenceNumber)
)
);

s_offRamp.manuallyExecute(report, _getGasLimitsFromMessages(messages));

// Since the tx failed we don't release the tokens
assertEq(tokenToAbuse.balanceOf(address(receiver)), balancePre);
}
}

contract EVM2EVMOffRamp_getExecutionState is EVM2EVMOffRampSetup {
Expand Down
Loading

0 comments on commit 0f1ae75

Please sign in to comment.