Skip to content

Commit

Permalink
MultiRamps - out-of-order execution changes (#1085)
Browse files Browse the repository at this point in the history 
## Motivation
Pulls in the out-of-order execution changes from the single-lane ramps:
14c4979

## Solution
Applies the logic directly from the single lane ramps, with changes in
per-lane config (`enforceOutOfOrder` is now configured per-lane)
  • Loading branch information
@elatoskinas
elatoskinas authored Jun 26, 2024
1 parent fe203df commit ab3df7b
Show file tree
Hide file tree
Showing 13 changed files with 549 additions and 256 deletions.
300 changes: 153 additions & 147 deletions contracts/gas-snapshots/ccip.gas-snapshot
View file

Large diffs are not rendered by default.

55 changes: 29 additions & 26 deletions contracts/src/v0.8/ccip/offRamp/EVM2EVMMultiOffRamp.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,32 +437,34 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
}
}

// In the scenario where we upgrade offRamps, we still want to have sequential nonces.
// Referencing the old offRamp to check the expected nonce if none is set for a
// given sender allows us to skip the current message if it would not be the next according
// to the old offRamp. This preserves sequencing between updates.
(uint64 prevNonce, bool isFromPrevRamp) = _getSenderNonce(sourceChainSelector, message.sender);
if (isFromPrevRamp) {
if (prevNonce + 1 != message.nonce) {
// the starting v2 onramp nonce, i.e. the 1st message nonce v2 offramp is expected to receive,
// is guaranteed to equal (largest v1 onramp nonce + 1).
// if this message's nonce isn't (v1 offramp nonce + 1), then v1 offramp nonce != largest v1 onramp nonce,
// it tells us there are still messages inflight for v1 offramp
emit SkippedSenderWithPreviousRampMessageInflight(sourceChainSelector, message.nonce, message.sender);
continue;
if (message.nonce > 0) {
// In the scenario where we upgrade offRamps, we still want to have sequential nonces.
// Referencing the old offRamp to check the expected nonce if none is set for a
// given sender allows us to skip the current message if it would not be the next according
// to the old offRamp. This preserves sequencing between updates.
(uint64 prevNonce, bool isFromPrevRamp) = _getSenderNonce(sourceChainSelector, message.sender);
if (isFromPrevRamp) {
if (prevNonce + 1 != message.nonce) {
// the starting v2 onramp nonce, i.e. the 1st message nonce v2 offramp is expected to receive,
// is guaranteed to equal (largest v1 onramp nonce + 1).
// if this message's nonce isn't (v1 offramp nonce + 1), then v1 offramp nonce != largest v1 onramp nonce,
// it tells us there are still messages inflight for v1 offramp
emit SkippedSenderWithPreviousRampMessageInflight(sourceChainSelector, message.nonce, message.sender);
continue;
}
// Otherwise this nonce is indeed the "transitional nonce", that is
// all messages sent to v1 ramp have been executed by the DON and the sequence can resume in V2.
// Note if first time user in V2, then prevNonce will be 0, and message.nonce = 1, so this will be a no-op.
s_senderNonce[sourceChainSelector][message.sender] = prevNonce;
}
// Otherwise this nonce is indeed the "transitional nonce", that is
// all messages sent to v1 ramp have been executed by the DON and the sequence can resume in V2.
// Note if first time user in V2, then prevNonce will be 0, and message.nonce = 1, so this will be a no-op.
s_senderNonce[sourceChainSelector][message.sender] = prevNonce;
}

// UNTOUCHED messages MUST be executed in order always
if (originalState == Internal.MessageExecutionState.UNTOUCHED) {
if (prevNonce + 1 != message.nonce) {
// We skip the message if the nonce is incorrect
emit SkippedIncorrectNonce(sourceChainSelector, message.nonce, message.sender);
continue;
// UNTOUCHED messages MUST be executed in order always IF message.nonce > 0.
if (originalState == Internal.MessageExecutionState.UNTOUCHED) {
if (prevNonce + 1 != message.nonce) {
// We skip the message if the nonce is incorrect, since message.nonce > 0.
emit SkippedIncorrectNonce(sourceChainSelector, message.nonce, message.sender);
continue;
}
}
}

Expand Down Expand Up @@ -495,12 +497,13 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
revert InvalidNewState(sourceChainSelector, message.sequenceNumber, newState);
}

// Nonce changes per state transition
// Nonce changes per state transition.
// These only apply for ordered messages.
// UNTOUCHED -> FAILURE nonce bump
// UNTOUCHED -> SUCCESS nonce bump
// FAILURE -> FAILURE no nonce bump
// FAILURE -> SUCCESS no nonce bump
if (originalState == Internal.MessageExecutionState.UNTOUCHED) {
if (message.nonce > 0 && originalState == Internal.MessageExecutionState.UNTOUCHED) {
s_senderNonce[sourceChainSelector][message.sender]++;
}

Expand Down
66 changes: 48 additions & 18 deletions contracts/src/v0.8/ccip/onRamp/EVM2EVMMultiOnRamp.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre

error CannotSendZeroTokens();
error InvalidExtraArgsTag();
error ExtraArgOutOfOrderExecutionMustBeTrue();
error OnlyCallableByOwnerOrAdmin();
error MessageTooLarge(uint256 maxSize, uint256 actualSize);
error MessageGasLimitTooHigh();
Expand Down Expand Up @@ -138,7 +139,8 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre
uint32 defaultTokenDestBytesOverhead; // ────╮ Default extra data availability bytes charged per token transfer
uint64 defaultTxGasLimit; // │ Default gas limit for a tx
uint64 gasMultiplierWeiPerEth; // │ Multiplier for gas costs, 1e18 based so 11e17 = 10% extra cost.
uint32 networkFeeUSDCents; // ───────────────╯ Flat network fee to charge for messages, multiples of 0.01 USD
uint32 networkFeeUSDCents; // │ Flat network fee to charge for messages, multiples of 0.01 USD
bool enforceOutOfOrder; // ──────────────────╯ Whether to enforce the allowOutOfOrderExecution extraArg value to be true.
}

/// @dev Struct to hold the configs for a destination chain
Expand Down Expand Up @@ -327,12 +329,13 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre
if (msg.sender != s_dynamicConfig.router) revert MustBeCalledByRouter();
if (!destChainConfig.dynamicConfig.isEnabled) revert DestinationChainNotEnabled(destChainSelector);

uint256 gasLimit = message.extraArgs.length == 0
? destChainConfig.dynamicConfig.defaultTxGasLimit
: _gasLimitFromBytes(message.extraArgs);
Client.EVMExtraArgsV2 memory extraArgs =
_extraArgsFromBytes(message.extraArgs, destChainConfig.dynamicConfig.defaultTxGasLimit);
// Validate the message with various checks
uint256 numberOfTokens = message.tokenAmounts.length;
_validateMessage(destChainSelector, message.data.length, gasLimit, numberOfTokens);
_validateMessage(
destChainSelector, message.data.length, extraArgs.gasLimit, numberOfTokens, extraArgs.allowOutOfOrderExecution
);

// Only check token value if there are tokens
if (numberOfTokens > 0) {
Expand Down Expand Up @@ -366,9 +369,13 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre
// Not duplicately validated in `getFee`. Invalid address is uncommon, gas cost outweighs UX gain.
receiver: Internal._validateEVMAddress(message.receiver),
sequenceNumber: ++destChainConfig.sequenceNumber,
gasLimit: gasLimit,
gasLimit: extraArgs.gasLimit,
strict: false,
nonce: INonceManager(i_nonceManager).getIncrementedOutboundNonce(destChainSelector, originalSender),
// Only bump nonce for messages that specify allowOutOfOrderExecution == false. Otherwise, we
// may block ordered message nonces, which is not what we want.
nonce: extraArgs.allowOutOfOrderExecution
? 0
: INonceManager(i_nonceManager).getIncrementedOutboundNonce(destChainSelector, originalSender),
feeToken: message.feeToken,
feeTokenAmount: feeTokenAmount,
data: message.data,
Expand All @@ -380,12 +387,25 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre

/// @dev Convert the extra args bytes into a struct
/// @param extraArgs The extra args bytes
/// @return The gas limit from the extra args
function _gasLimitFromBytes(bytes calldata extraArgs) internal pure returns (uint256) {
if (bytes4(extraArgs) != Client.EVM_EXTRA_ARGS_V1_TAG) revert InvalidExtraArgsTag();
// EVMExtraArgsV1 originally included a second boolean (strict) field which we have deprecated entirely.
// Clients may still send that version but it will be ignored.
return abi.decode(extraArgs[4:], (Client.EVMExtraArgsV1)).gasLimit;
/// @return EVMExtraArgs the extra args struct
function _extraArgsFromBytes(
bytes calldata extraArgs,
uint64 defaultTxGasLimit
) internal pure returns (Client.EVMExtraArgsV2 memory) {
if (extraArgs.length == 0) {
return Client.EVMExtraArgsV2({gasLimit: defaultTxGasLimit, allowOutOfOrderExecution: false});
}

bytes4 extraArgsTag = bytes4(extraArgs);
if (extraArgsTag == Client.EVM_EXTRA_ARGS_V2_TAG) {
return abi.decode(extraArgs[4:], (Client.EVMExtraArgsV2));
} else if (extraArgsTag == Client.EVM_EXTRA_ARGS_V1_TAG) {
// EVMExtraArgsV1 originally included a second boolean (strict) field which has been deprecated.
// Clients may still include it but it will be ignored.
return Client.EVMExtraArgsV2({gasLimit: abi.decode(extraArgs[4:], (uint256)), allowOutOfOrderExecution: false});
}

revert InvalidExtraArgsTag();
}

/// @notice Validate the forwarded message with various checks.
Expand All @@ -399,7 +419,8 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre
uint64 destChainSelector,
uint256 dataLength,
uint256 gasLimit,
uint256 numberOfTokens
uint256 numberOfTokens,
bool allowOutOfOrderExecution
) internal view {
// Check that payload is formed correctly
DestChainDynamicConfig storage destChainDynamicConfig = s_destChainConfig[destChainSelector].dynamicConfig;
Expand All @@ -408,6 +429,9 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre
}
if (gasLimit > uint256(destChainDynamicConfig.maxPerMsgGasLimit)) revert MessageGasLimitTooHigh();
if (numberOfTokens > uint256(destChainDynamicConfig.maxNumberOfTokensPerMsg)) revert UnsupportedNumberOfTokens();
if (destChainDynamicConfig.enforceOutOfOrder && !allowOutOfOrderExecution) {
revert ExtraArgOutOfOrderExecutionMustBeTrue();
}
}

// ================================================================
Expand Down Expand Up @@ -491,10 +515,16 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre

if (!destChainDynamicConfig.isEnabled) revert DestinationChainNotEnabled(destChainSelector);

uint256 gasLimit =
message.extraArgs.length == 0 ? destChainDynamicConfig.defaultTxGasLimit : _gasLimitFromBytes(message.extraArgs);
Client.EVMExtraArgsV2 memory extraArgs =
_extraArgsFromBytes(message.extraArgs, destChainDynamicConfig.defaultTxGasLimit);
// Validate the message with various checks
_validateMessage(destChainSelector, message.data.length, gasLimit, message.tokenAmounts.length);
_validateMessage(
destChainSelector,
message.data.length,
extraArgs.gasLimit,
message.tokenAmounts.length,
extraArgs.allowOutOfOrderExecution
);

uint64 premiumMultiplierWeiPerEth = s_premiumMultiplierWeiPerEth[message.feeToken];

Expand Down Expand Up @@ -541,7 +571,7 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre
// uint112(packedGasPrice) = executionGasPrice
uint256 executionCost = uint112(packedGasPrice)
* (
gasLimit + destChainDynamicConfig.destGasOverhead
extraArgs.gasLimit + destChainDynamicConfig.destGasOverhead
+ (message.data.length * destChainDynamicConfig.destGasPerPayloadByte) + tokenTransferGas
) * destChainDynamicConfig.gasMultiplierWeiPerEth;

Expand Down
12 changes: 10 additions & 2 deletions contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ contract MultiRampsE2E is EVM2EVMMultiOnRampSetup, EVM2EVMMultiOffRampSetup {
reports[1] = _generateReportFromMessages(SOURCE_CHAIN_SELECTOR + 1, messages2);

vm.resumeGasMetering();
s_offRamp.batchExecute(reports, new uint256[][](0));
_execute(reports);
}

function _sendRequest(
Expand All @@ -223,7 +223,15 @@ contract MultiRampsE2E is EVM2EVMMultiOnRampSetup, EVM2EVMMultiOffRampSetup {

message.receiver = abi.encode(address(s_receiver));
Internal.EVM2EVMMessage memory msgEvent = _messageToEvent(
message, sourceChainSelector, expectedSeqNum, nonce, expectedFee, OWNER, metadataHash, tokenAdminRegistry
message,
sourceChainSelector,
DEST_CHAIN_SELECTOR,
expectedSeqNum,
nonce,
expectedFee,
OWNER,
metadataHash,
tokenAdminRegistry
);

vm.expectEmit();
Expand Down
7 changes: 7 additions & 0 deletions contracts/src/v0.8/ccip/test/helpers/EVM2EVMMultiOnRampHelper.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,11 @@ contract EVM2EVMMultiOnRampHelper is EVM2EVMMultiOnRamp, IgnoreContractSize {
) external view returns (uint256, uint32, uint32) {
return _getTokenTransferCost(destChainSelector, feeToken, feeTokenPrice, tokenAmounts);
}

function extraArgsFromBytes(
bytes calldata extraArgs,
uint64 destChainSelector
) external view returns (Client.EVMExtraArgsV2 memory) {
return _extraArgsFromBytes(extraArgs, s_destChainConfig[destChainSelector].dynamicConfig.defaultTxGasLimit);
}
}
113 changes: 113 additions & 0 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 @@ -428,6 +428,53 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup {
assertGt(s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, messages[0].sender), nonceBefore);
}

function test_SingleMessageNoTokensUnordered_Success() public {
Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1);
messages[0].nonce = 0;
messages[0].messageId =
Internal._hash(messages[0], s_offRamp.metadataHash(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1));

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

// Nonce never increments on unordered messages.
uint64 nonceBefore = s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, messages[0].sender);
s_offRamp.executeSingleReport(_generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[](0));
assertEq(
s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, messages[0].sender),
nonceBefore,
"nonce must remain unchanged on unordered messages"
);

messages[0].sequenceNumber++;
messages[0].messageId =
Internal._hash(messages[0], s_offRamp.metadataHash(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1));

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

// Nonce never increments on unordered messages.
nonceBefore = s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, messages[0].sender);
s_offRamp.executeSingleReport(_generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[](0));
assertEq(
s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, messages[0].sender),
nonceBefore,
"nonce must remain unchanged on unordered messages"
);
}

function test_SingleMessageNoTokensOtherChain_Success() public {
Internal.EVM2EVMMessage[] memory messagesChain1 =
_generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1);
Expand Down Expand Up @@ -537,6 +584,29 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup {
s_offRamp.executeSingleReport(_generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[](0));
}

function test__execute_SkippedAlreadyExecutedMessageUnordered_Success() public {
Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1);
messages[0].nonce = 0;
messages[0].messageId =
Internal._hash(messages[0], s_offRamp.metadataHash(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1));

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

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

vm.expectEmit();
emit EVM2EVMMultiOffRamp.SkippedAlreadyExecutedMessage(SOURCE_CHAIN_SELECTOR_1, messages[0].sequenceNumber);

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

// Send a message to a contract that does not implement the CCIPReceiver interface
// This should execute successfully.
function test_SingleMessageToNonCCIPReceiver_Success() public {
Expand Down Expand Up @@ -641,6 +711,49 @@ contract EVM2EVMMultiOffRamp_executeSingleReport is EVM2EVMMultiOffRampSetup {
assertEq(uint64(2), s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, OWNER));
}

function test_Fuzz_InterleavingOrderedAndUnorderedMessages_Success(bool[7] memory orderings) public {
Internal.EVM2EVMMessage[] memory messages = new Internal.EVM2EVMMessage[](orderings.length);
// number of tokens needs to be capped otherwise we hit UnsupportedNumberOfTokens.
Client.EVMTokenAmount[] memory tokenAmounts = new Client.EVMTokenAmount[](3);
for (uint256 i = 0; i < 3; ++i) {
tokenAmounts[i].token = s_sourceTokens[i % s_sourceTokens.length];
tokenAmounts[i].amount = 1e18;
}
uint64 expectedNonce = 0;
for (uint256 i = 0; i < orderings.length; ++i) {
messages[i] =
_generateAny2EVMMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1, uint64(i + 1), tokenAmounts, !orderings[i]);
if (orderings[i]) {
messages[i].nonce = ++expectedNonce;
}
messages[i].messageId =
Internal._hash(messages[i], s_offRamp.metadataHash(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1));

vm.expectEmit();
emit EVM2EVMMultiOffRamp.ExecutionStateChanged(
SOURCE_CHAIN_SELECTOR_1,
messages[i].sequenceNumber,
messages[i].messageId,
Internal.MessageExecutionState.SUCCESS,
""
);
}

uint64 nonceBefore = s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, OWNER);
assertEq(uint64(0), nonceBefore, "nonce before exec should be 0");
s_offRamp.executeSingleReport(
_generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), _getGasLimitsFromMessages(messages)
);
// all executions should succeed.
for (uint256 i = 0; i < orderings.length; ++i) {
assertEq(
uint256(s_offRamp.getExecutionState(SOURCE_CHAIN_SELECTOR_1, messages[i].sequenceNumber)),
uint256(Internal.MessageExecutionState.SUCCESS)
);
}
assertEq(nonceBefore + expectedNonce, s_offRamp.getSenderNonce(SOURCE_CHAIN_SELECTOR_1, OWNER));
}

function test_InvalidSourcePoolAddress_Success() public {
address fakePoolAddress = address(0x0000000000333333);

Expand Down
Loading

0 comments on commit ab3df7b

Please sign in to comment.