Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cap max accounts in svm extra args #16175

Merged
merged 5 commits into from
Feb 4, 2025
Merged

Cap max accounts in svm extra args #16175

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9143f7f
cap max accounts in svm extra args
RensR Jan 31, 2025
dde241d
[Bot] Update changeset file with jira issues
app-token-issuer-infra-releng[bot] Jan 31, 2025
23c28fc
rm duplicate extra args validation
RensR Feb 3, 2025
19b296e
validate svm bitmap
RensR Feb 3, 2025
8dd0952
fix solhint
RensR Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions contracts/.changeset/slow-gorillas-notice.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@chainlink/contracts': patch
---

#internal cap max accounts in svm extra args


PR issue: CCIP-5111

Solidity Review issue: CCIP-3966
10 changes: 5 additions & 5 deletions contracts/gas-snapshots/ccip.gas-snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ FeeQuoter_applyPremiumMultiplierWeiPerEthUpdates:test_applyPremiumMultiplierWeiP
FeeQuoter_applyPremiumMultiplierWeiPerEthUpdates:test_applyPremiumMultiplierWeiPerEthUpdatesZeroInput() (gas: 12468)
FeeQuoter_applyTokenTransferFeeConfigUpdates:test_ApplyTokenTransferFeeConfig() (gas: 88736)
FeeQuoter_applyTokenTransferFeeConfigUpdates:test_ApplyTokenTransferFeeZeroInput() (gas: 13218)
FeeQuoter_constructor:test_Setup() (gas: 5447819)
FeeQuoter_constructor:test_Setup() (gas: 5458858)
FeeQuoter_convertTokenAmount:test_ConvertTokenAmount() (gas: 68417)
FeeQuoter_getDataAvailabilityCost:test_EmptyMessageCalculatesDataAvailabilityCost() (gas: 98860)
FeeQuoter_getDataAvailabilityCost:test_SimpleMessageCalculatesDataAvailabilityCost() (gas: 21439)
Expand All @@ -88,11 +88,11 @@ FeeQuoter_getTokenTransferCost:test_SmallTokenTransferChargesMinFeeAndGas() (gas
FeeQuoter_getTokenTransferCost:test_ZeroAmountTokenTransferChargesMinFeeAndGas() (gas: 25230)
FeeQuoter_getTokenTransferCost:test_ZeroFeeConfigChargesMinFee() (gas: 37788)
FeeQuoter_getTokenTransferCost:test_getTokenTransferCost_selfServeUsesDefaults() (gas: 26924)
FeeQuoter_getValidatedFee:test_EmptyMessage() (gas: 84896)
FeeQuoter_getValidatedFee:test_EmptyMessage() (gas: 84963)
FeeQuoter_getValidatedFee:test_HighGasMessage() (gas: 242832)
FeeQuoter_getValidatedFee:test_MessageWithDataAndTokenTransfer() (gas: 143454)
FeeQuoter_getValidatedFee:test_SingleTokenMessage() (gas: 114959)
FeeQuoter_getValidatedFee:test_SolChainFamilySelector() (gas: 61180)
FeeQuoter_getValidatedFee:test_MessageWithDataAndTokenTransfer() (gas: 143432)
FeeQuoter_getValidatedFee:test_SingleTokenMessage() (gas: 114937)
FeeQuoter_getValidatedFee:test_SolChainFamilySelector() (gas: 61283)
FeeQuoter_getValidatedFee:test_ZeroDataAvailabilityMultiplier() (gas: 66298)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPrice() (gas: 58927)
FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeed() (gas: 65137)
Expand Down
4 changes: 4 additions & 0 deletions contracts/src/v0.8/ccip/FeeQuoter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
error InvalidFeeRange(uint256 minFeeUSDCents, uint256 maxFeeUSDCents);
error InvalidChainFamilySelector(bytes4 chainFamilySelector);
error InvalidTokenReceiver();
error TooManySVMExtraArgsAccounts(uint256 numAccounts, uint256 maxAccounts);

event FeeTokenAdded(address indexed feeToken);
event FeeTokenRemoved(address indexed feeToken);
Expand Down Expand Up @@ -1004,6 +1005,9 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
if (numberOfTokens > 0 && svmExtraArgsV1.tokenReceiver == bytes32(0)) {
revert InvalidTokenReceiver();
}
if (svmExtraArgsV1.accounts.length > Client.SVM_EXTRA_ARGS_MAX_ACCOUNTS) {
revert TooManySVMExtraArgsAccounts(svmExtraArgsV1.accounts.length, Client.SVM_EXTRA_ARGS_MAX_ACCOUNTS);
}
gasLimit = svmExtraArgsV1.computeUnits;
} else {
revert InvalidChainFamilySelector(destChainConfig.chainFamilySelector);
Expand Down
3 changes: 3 additions & 0 deletions contracts/src/v0.8/ccip/libraries/Client.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ library Client {
// bytes4(keccak256("CCIP SVMExtraArgsV1"));
bytes4 public constant SVM_EXTRA_ARGS_V1_TAG = 0x1f3b3aba;

/// @dev The maximum number of accounts that can be passed in SVMExtraArgs.
uint256 public constant SVM_EXTRA_ARGS_MAX_ACCOUNTS = 64;

/// @param gasLimit: gas limit for the callback on the destination chain.
/// @param allowOutOfOrderExecution: if true, it indicates that the message can be executed in any order relative to
/// other messages from the same sender. This value's default varies by chain. On some chains, a particular value is
Expand Down
6 changes: 3 additions & 3 deletions contracts/src/v0.8/ccip/pools/SiloedLockReleaseTokenPool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,15 @@ contract SiloedLockReleaseTokenPool is TokenPool, ITypeAndVersion {
SiloConfig storage remoteConfig = s_chainConfigs[releaseOrMintIn.remoteChainSelector];

// Since remoteConfig.isSiloed is used more than once, caching in memory saves gas instead of multiple SLOADs.
bool isSiloed = remoteConfig.isSiloed;
bool chainIsSiloed = remoteConfig.isSiloed;

// Prevent A silent underflow by explicitly ensuring that enough funds are available to release
uint256 availableLiquidity = isSiloed ? remoteConfig.tokenBalance : s_unsiloedTokenBalance;
uint256 availableLiquidity = chainIsSiloed ? remoteConfig.tokenBalance : s_unsiloedTokenBalance;
if (localAmount > availableLiquidity) revert InsufficientLiquidity(availableLiquidity, localAmount);

// Tracking balances independently by chain is a security measure to prevent liquidity for one chain from being
// released by another chain.
if (isSiloed) {
if (chainIsSiloed) {
remoteConfig.tokenBalance -= localAmount;
} else {
s_unsiloedTokenBalance -= localAmount;
Expand Down
28 changes: 24 additions & 4 deletions contracts/src/v0.8/ccip/test/feeQuoter/FeeQuoter.getValidatedFee.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,15 +290,11 @@ contract FeeQuoter_getValidatedFee is FeeQuoterFeeSetup {
}

function test_RevertWhen_SVMMessageWithTokenTransferAndInvalidTokenReceiver() public {
vm.stopPrank();
//setup to set chainFamilySelector for SVM so that token receiver's check flow is enabled
vm.startPrank(OWNER);

FeeQuoter.DestChainConfigArgs[] memory destChainConfigArgs = _generateFeeQuoterDestChainConfigArgs();
destChainConfigArgs[0].destChainConfig.chainFamilySelector = Internal.CHAIN_FAMILY_SELECTOR_SVM;

s_feeQuoter.applyDestChainConfigUpdates(destChainConfigArgs);
vm.stopPrank();

Client.EVM2AnyMessage memory message = _generateSingleTokenMessage(s_sourceFeeToken, 1);
// replace with SVM Extra Args
Expand All @@ -314,4 +310,28 @@ contract FeeQuoter_getValidatedFee is FeeQuoterFeeSetup {
vm.expectRevert(FeeQuoter.InvalidTokenReceiver.selector);
s_feeQuoter.getValidatedFee(DEST_CHAIN_SELECTOR, message);
}

function test_getValidatedFee_RevertWhen_TooManySVMExtraArgsAccounts() public {
FeeQuoter.DestChainConfigArgs[] memory destChainConfigArgs = _generateFeeQuoterDestChainConfigArgs();
destChainConfigArgs[0].destChainConfig.chainFamilySelector = Internal.CHAIN_FAMILY_SELECTOR_SVM;

s_feeQuoter.applyDestChainConfigUpdates(destChainConfigArgs);

uint256 maxAccounts = Client.SVM_EXTRA_ARGS_MAX_ACCOUNTS;

Client.EVM2AnyMessage memory message = _generateSingleTokenMessage(s_sourceFeeToken, 1);
message.extraArgs = Client._svmArgsToBytes(
Client.SVMExtraArgsV1({
computeUnits: GAS_LIMIT,
accountIsWritableBitmap: 0,
allowOutOfOrderExecution: true,
tokenReceiver: bytes32(uint256(1)),
accounts: new bytes32[](maxAccounts + 1)
})
);
vm.expectRevert(
abi.encodeWithSelector(FeeQuoter.TooManySVMExtraArgsAccounts.selector, maxAccounts + 1, maxAccounts)
);
s_feeQuoter.getValidatedFee(DEST_CHAIN_SELECTOR, message);
}
}
4 changes: 2 additions & 2 deletions core/gethwrappers/ccip/generated/fee_quoter/fee_quoter.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ ccip_encoding_utils: ../../../contracts/solc/ccip/EncodingUtils/EncodingUtils.so
ccip_home: ../../../contracts/solc/ccip/CCIPHome/CCIPHome.sol/CCIPHome.abi.json ../../../contracts/solc/ccip/CCIPHome/CCIPHome.sol/CCIPHome.bin 39de1fbc907a2b573e9358e716803bf5ac3b0a2e622d5bc0069ab60daf38949b
ccip_reader_tester: ../../../contracts/solc/ccip/CCIPReaderTester/CCIPReaderTester.sol/CCIPReaderTester.abi.json ../../../contracts/solc/ccip/CCIPReaderTester/CCIPReaderTester.sol/CCIPReaderTester.bin b8e597d175ec5ff4990d98b4e3b8a8cf06c6ae22977dd6f0e58c0f4107639e8f
ether_sender_receiver: ../../../contracts/solc/ccip/EtherSenderReceiver/EtherSenderReceiver.sol/EtherSenderReceiver.abi.json ../../../contracts/solc/ccip/EtherSenderReceiver/EtherSenderReceiver.sol/EtherSenderReceiver.bin 88973abc1bfbca23a23704e20087ef46f2e20581a13477806308c8f2e664844e
fee_quoter: ../../../contracts/solc/ccip/FeeQuoter/FeeQuoter.sol/FeeQuoter.abi.json ../../../contracts/solc/ccip/FeeQuoter/FeeQuoter.sol/FeeQuoter.bin 3efd18088f1a27b497ec5b0936f54931e0e151033d5cb32649d497525a3964fa
fee_quoter: ../../../contracts/solc/ccip/FeeQuoter/FeeQuoter.sol/FeeQuoter.abi.json ../../../contracts/solc/ccip/FeeQuoter/FeeQuoter.sol/FeeQuoter.bin 8e0afdeaa7a732e4378d7b35c7d7af361ffa27d758262881a88f92e0a0148f41
lock_release_token_pool: ../../../contracts/solc/ccip/LockReleaseTokenPool/LockReleaseTokenPool.sol/LockReleaseTokenPool.abi.json ../../../contracts/solc/ccip/LockReleaseTokenPool/LockReleaseTokenPool.sol/LockReleaseTokenPool.bin 2e73ee0da6f9a9a5722294289b969e4202476706e5d7cdb623e728831c79c28b
log_message_data_receiver: ../../../contracts/solc/ccip/LogMessageDataReceiver/LogMessageDataReceiver.sol/LogMessageDataReceiver.abi.json ../../../contracts/solc/ccip/LogMessageDataReceiver/LogMessageDataReceiver.sol/LogMessageDataReceiver.bin 6fe60e48711884eae82dd95cabb1c66a5644336719fa1219df1ceceec11e6bce
maybe_revert_message_receiver: ../../../contracts/solc/ccip/MaybeRevertMessageReceiver/MaybeRevertMessageReceiver.sol/MaybeRevertMessageReceiver.abi.json ../../../contracts/solc/ccip/MaybeRevertMessageReceiver/MaybeRevertMessageReceiver.sol/MaybeRevertMessageReceiver.bin ee264f67a2356cc4eebe839a5a88367cbcdc27a7520cca56263319e9afe97a1a
Expand Down
Loading