Skip to content

Commit

Permalink
Port v1.5 fixes to v1.6 (#1286)
Browse files Browse the repository at this point in the history
## Motivation
Porting the fixes done in
#1212

1. Add ~~uint32~~ `bytes destExecData` to the `struct RampTokenAmount`
to allow us to send the amount we billed on source to dest
2. removed `defaultTokenDestBytesOverhead` and used
`CCIP_LOCK_OR_BURN_V1_RET_BYTES` as default value

---------

Signed-off-by: 0xsuryansh <[email protected]>
Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 757cbf3 commit c638767
Show file tree
Hide file tree
Showing 20 changed files with 583 additions and 513 deletions.
686 changes: 342 additions & 344 deletions contracts/gas-snapshots/ccip.gas-snapshot

Large diffs are not rendered by default.

23 changes: 16 additions & 7 deletions contracts/src/v0.8/ccip/PriceRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ contract PriceRegistry is
// The following three properties are defaults, they can be overridden by setting the TokenTransferFeeConfig for a token
uint16 defaultTokenFeeUSDCents; // │ Default token fee charged per token transfer
uint32 defaultTokenDestGasOverhead; // ──────╯ Default gas charged to execute the token transfer on the destination chain
uint32 defaultTokenDestBytesOverhead; // ────╮ Default extra data availability bytes charged per token transfer
uint32 defaultTxGasLimit; // │ Default gas limit for a tx
uint32 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
bool enforceOutOfOrder; // │ Whether to enforce the allowOutOfOrderExecution extraArg value to be true.
Expand Down Expand Up @@ -601,7 +600,7 @@ contract PriceRegistry is
if (!transferFeeConfig.isEnabled) {
tokenTransferFeeUSDWei += uint256(destChainConfig.defaultTokenFeeUSDCents) * 1e16;
tokenTransferGas += destChainConfig.defaultTokenDestGasOverhead;
tokenTransferBytesOverhead += destChainConfig.defaultTokenDestBytesOverhead;
tokenTransferBytesOverhead += Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES;
continue;
}

Expand Down Expand Up @@ -867,13 +866,13 @@ contract PriceRegistry is

/// @inheritdoc IPriceRegistry
/// @dev precondition - rampTokenAmounts and sourceTokenAmounts lengths must be equal
function validatePoolReturnData(
function processPoolReturnData(
uint64 destChainSelector,
Internal.RampTokenAmount[] calldata rampTokenAmounts,
Client.EVMTokenAmount[] calldata sourceTokenAmounts
) external view {
) external view returns (bytes[] memory destExecDataPerToken) {
bytes4 chainFamilySelector = s_destChainConfigs[destChainSelector].chainFamilySelector;

destExecDataPerToken = new bytes[](rampTokenAmounts.length);
for (uint256 i = 0; i < rampTokenAmounts.length; ++i) {
address sourceToken = sourceTokenAmounts[i].token;

Expand All @@ -888,7 +887,18 @@ contract PriceRegistry is
}

_validateDestFamilyAddress(chainFamilySelector, rampTokenAmounts[i].destTokenAddress);
PriceRegistry.TokenTransferFeeConfig memory tokenTransferFeeConfig =
s_tokenTransferFeeConfig[destChainSelector][sourceToken];
uint32 defaultGasOverhead = s_destChainConfigs[destChainSelector].defaultTokenDestGasOverhead;
// NOTE: Revisit this when adding new non-EVM chain family selector support
uint32 destGasAmount =
tokenTransferFeeConfig.isEnabled ? tokenTransferFeeConfig.destGasOverhead : defaultGasOverhead;

// The user will be billed either the default or the override, so we send the exact amount that we billed for
// to the destination chain to be used for the token releaseOrMint and transfer.
destExecDataPerToken[i] = abi.encode(destGasAmount);
}
return destExecDataPerToken;
}

// ================================================================
Expand Down Expand Up @@ -919,7 +929,6 @@ contract PriceRegistry is
if (
destChainSelector == 0 || destChainConfig.defaultTxGasLimit == 0
|| destChainConfig.chainFamilySelector != Internal.CHAIN_FAMILY_SELECTOR_EVM
|| destChainConfig.defaultTokenDestBytesOverhead < Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES
|| destChainConfig.defaultTxGasLimit > destChainConfig.maxPerMsgGasLimit
) {
revert InvalidDestChainConfig(destChainSelector);
Expand Down
7 changes: 4 additions & 3 deletions contracts/src/v0.8/ccip/interfaces/IPriceRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,10 @@ interface IPriceRegistry {
/// @param destChainSelector Destination chain selector to which the token amounts are sent to
/// @param rampTokenAmounts Token amounts with populated pool return data
/// @param sourceTokenAmounts Token amounts originally sent in a Client.EVM2AnyMessage message
function validatePoolReturnData(
/// @return destExecData Destination chain execution data
function processPoolReturnData(
uint64 destChainSelector,
Internal.RampTokenAmount[] calldata rampTokenAmounts,
Internal.RampTokenAmount[] memory rampTokenAmounts,
Client.EVMTokenAmount[] calldata sourceTokenAmounts
) external view;
) external view returns (bytes[] memory);
}
12 changes: 8 additions & 4 deletions contracts/src/v0.8/ccip/libraries/Internal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,11 @@ library Internal {
uint256 public constant ANY_2_EVM_MESSAGE_FIXED_BYTES = 32 * 14;

/// @dev Each token transfer adds 1 RampTokenAmount
/// RampTokenAmount has 4 fields, including 3 bytes.
/// Each bytes takes 1 more slot to store its length, and one slot to store the offset.
/// When abi encoded, each token transfer takes up 10 slots, excl bytes contents.
uint256 public constant ANY_2_EVM_MESSAGE_FIXED_BYTES_PER_TOKEN = 32 * 10;
/// RampTokenAmount has 5 fields, 3 of which are bytes type, 1 uint256 and 1 uint32.
/// Each bytes type takes 1 slot for length, 1 slot for data and 1 slot for the offset.
/// uint256 amount takes 1 slot.
/// uint32 destGasAmount takes 1 slot.
uint256 public constant ANY_2_EVM_MESSAGE_FIXED_BYTES_PER_TOKEN = 32 * ((3 * 3) + 2);

bytes32 internal constant EVM_2_EVM_MESSAGE_HASH = keccak256("EVM2EVMMessageHashV2");

Expand Down Expand Up @@ -294,6 +295,9 @@ library Internal {
// has to be set for the specific token.
bytes extraData;
uint256 amount; // Amount of tokens.
// Destination chain specific execution data encoded in bytes
//(for EVM destination it consists of the amount of gas available for the releaseOrMint and transfer calls on the offRamp
bytes destExecData;
}

/// @notice Family-agnostic header for OnRamp & OffRamp messages.
Expand Down
9 changes: 7 additions & 2 deletions contracts/src/v0.8/ccip/onRamp/OnRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,14 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator {
}

// Validate pool return data after it is populated (view function - no state changes)
IPriceRegistry(s_dynamicConfig.priceRegistry).validatePoolReturnData(
bytes[] memory destExecDataPerToken = IPriceRegistry(s_dynamicConfig.priceRegistry).processPoolReturnData(
destChainSelector, newMessage.tokenAmounts, message.tokenAmounts
);

for (uint256 i = 0; i < newMessage.tokenAmounts.length; ++i) {
newMessage.tokenAmounts[i].destExecData = destExecDataPerToken[i];
}

// Override extraArgs with latest version
newMessage.extraArgs = convertedExtraArgs;

Expand Down Expand Up @@ -251,7 +255,8 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator {
sourcePoolAddress: abi.encode(sourcePool),
destTokenAddress: poolReturnData.destTokenAddress,
extraData: poolReturnData.destPoolData,
amount: tokenAndAmount.amount
amount: tokenAndAmount.amount,
destExecData: "" // This is set in the processPoolReturnData function
});
}

Expand Down
40 changes: 26 additions & 14 deletions contracts/src/v0.8/ccip/test/offRamp/OffRamp.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1307,12 +1307,12 @@ contract OffRamp_manuallyExecute is OffRampSetup {
s_offRamp.batchExecute(_generateBatchReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages), new uint256[][](1));

s_reverting_receiver.setRevert(false);

uint256[][] memory gasLimitOverrides = new uint256[][](1);
gasLimitOverrides[0] = _getGasLimitsFromMessages(messages);
gasLimitOverrides[0][0] += 1;

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

assertExecutionStateChangedEventLogs(
SOURCE_CHAIN_SELECTOR_1,
messages[0].header.sequenceNumber,
Expand Down Expand Up @@ -1664,7 +1664,8 @@ contract OffRamp_manuallyExecute is OffRampSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[s_sourceFeeToken]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[s_sourceFeeToken]),
extraData: "",
amount: tokenAmount
amount: tokenAmount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

messages[0].receiver = address(receiver);
Expand Down Expand Up @@ -2216,7 +2217,8 @@ contract OffRamp_trialExecute is OffRampSetup {
sourcePoolAddress: abi.encode(address(0)),
destTokenAddress: abi.encode(address(0)),
extraData: "",
amount: message.tokenAmounts[0].amount
amount: message.tokenAmounts[0].amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

message.header.messageId = Internal._hash(message, ON_RAMP_ADDRESS_1);
Expand All @@ -2233,7 +2235,8 @@ contract OffRamp_trialExecute is OffRampSetup {
sourcePoolAddress: abi.encode(address(0)),
destTokenAddress: abi.encode(notAContract),
extraData: "",
amount: message.tokenAmounts[0].amount
amount: message.tokenAmounts[0].amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

message.header.messageId = Internal._hash(message, ON_RAMP_ADDRESS_1);
Expand Down Expand Up @@ -2264,7 +2267,8 @@ contract OffRamp__releaseOrMintSingleToken is OffRampSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[token]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[token]),
extraData: "",
amount: amount
amount: amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

vm.expectCall(
Expand Down Expand Up @@ -2297,7 +2301,8 @@ contract OffRamp__releaseOrMintSingleToken is OffRampSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[token]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[token]),
extraData: "",
amount: amount
amount: amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

// Mock the call so returns 2 slots of data
Expand All @@ -2318,7 +2323,8 @@ contract OffRamp__releaseOrMintSingleToken is OffRampSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[token]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[token]),
extraData: "",
amount: amount
amount: amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

bytes memory revertData = "failed to balanceOf";
Expand All @@ -2342,7 +2348,8 @@ contract OffRamp__releaseOrMintSingleToken is OffRampSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[token]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[token]),
extraData: "",
amount: amount
amount: amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

vm.mockCall(
Expand All @@ -2369,7 +2376,8 @@ contract OffRamp__releaseOrMintSingleToken is OffRampSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[token]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[token]),
extraData: "",
amount: amount
amount: amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

// This should make the call fail if it does not skip the check
Expand All @@ -2396,7 +2404,8 @@ contract OffRamp__releaseOrMintSingleToken is OffRampSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[token]),
destTokenAddress: abi.encode(destToken),
extraData: "",
amount: amount
amount: amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

// Address(0) should always revert
Expand Down Expand Up @@ -2438,7 +2447,8 @@ contract OffRamp__releaseOrMintSingleToken is OffRampSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[token]),
destTokenAddress: abi.encode(destToken),
extraData: "",
amount: amount
amount: amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

bytes memory revertData = "call reverted :o";
Expand Down Expand Up @@ -2594,7 +2604,8 @@ contract OffRamp_releaseOrMintTokens is OffRampSetup {
sourcePoolAddress: abi.encode(fakePoolAddress),
destTokenAddress: abi.encode(s_offRamp),
extraData: "",
amount: 1
amount: 1,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

vm.expectRevert(abi.encodeWithSelector(OffRamp.NotACompatiblePool.selector, address(0)));
Expand Down Expand Up @@ -2646,7 +2657,8 @@ contract OffRamp_releaseOrMintTokens is OffRampSetup {
sourcePoolAddress: unusedVar,
destTokenAddress: abi.encode(destPool),
extraData: unusedVar,
amount: 1
amount: 1,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});

try s_offRamp.releaseOrMintTokens(
Expand Down
6 changes: 4 additions & 2 deletions contracts/src/v0.8/ccip/test/offRamp/OffRampSetup.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ contract OffRampSetup is PriceRegistrySetup, MultiOCR3BaseSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[tokenAmounts[i].token]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[tokenAmounts[i].token]),
extraData: "",
amount: tokenAmounts[i].amount
amount: tokenAmounts[i].amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});
}

Expand Down Expand Up @@ -414,7 +415,8 @@ contract OffRampSetup is PriceRegistrySetup, MultiOCR3BaseSetup {
sourcePoolAddress: abi.encode(s_sourcePoolByToken[srcTokenAmounts[i].token]),
destTokenAddress: abi.encode(s_destTokenBySourceToken[srcTokenAmounts[i].token]),
extraData: "",
amount: srcTokenAmounts[i].amount
amount: srcTokenAmounts[i].amount,
destExecData: abi.encode(DEFAULT_TOKEN_DEST_GAS_OVERHEAD)
});
}
return sourceTokenData;
Expand Down
Loading

0 comments on commit c638767

Please sign in to comment.