-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
Changes from all 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 dde241d
[Bot] Update changeset file with jira issues
app-token-issuer-infra-releng[bot] 23c28fc
rm duplicate extra args validation
RensR 19b296e
validate svm bitmap
RensR 8dd0952
fix solhint
RensR File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,8 @@ 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); | ||
error InvalidSVMExtraArgsWritableBitmap(uint64 accountIsWritableBitmap, uint256 numAccounts); | ||
|
||
event FeeTokenAdded(address indexed feeToken); | ||
event FeeTokenRemoved(address indexed feeToken); | ||
|
@@ -1004,6 +1006,12 @@ 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); | ||
} | ||
if (svmExtraArgsV1.accountIsWritableBitmap >> svmExtraArgsV1.accounts.length != 0) { | ||
revert InvalidSVMExtraArgsWritableBitmap(svmExtraArgsV1.accountIsWritableBitmap, svmExtraArgsV1.accounts.length); | ||
} | ||
gasLimit = svmExtraArgsV1.computeUnits; | ||
} else { | ||
revert InvalidChainFamilySelector(destChainConfig.chainFamilySelector); | ||
|
@@ -1042,8 +1050,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, | |
|
||
if (msgFeeJuels > i_maxFeeJuelsPerMsg) revert MessageFeeTooHigh(msgFeeJuels, i_maxFeeJuelsPerMsg); | ||
|
||
(convertedExtraArgs, isOutOfOrderExecution) = | ||
_processChainFamilySelector(destChainSelector, sourceTokenAmounts.length > 0, extraArgs); | ||
(convertedExtraArgs, isOutOfOrderExecution) = _processChainFamilySelector(destChainSelector, extraArgs); | ||
|
||
destExecDataPerToken = _processPoolReturnData(destChainSelector, onRampTokenTransfers, sourceTokenAmounts); | ||
|
||
|
@@ -1052,26 +1059,22 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver, | |
|
||
/// @notice Parses the extra Args based on the chain family selector. Isolated into a separate function | ||
/// as it was the only way to prevent a stack too deep error, and makes future chain family additions easier. | ||
// solhint-disable-next-line chainlink-solidity/explicit-returns | ||
function _processChainFamilySelector( | ||
uint64 destChainSelector, | ||
bool isMessageWithTokenTransfer, | ||
bytes calldata extraArgs | ||
) internal view returns (bytes memory, bool) { | ||
) internal view returns (bytes memory validatedExtraArgs, bool allowOutOfOrderExecution) { | ||
// Since this function is called after getFee, which already validates the params, no validation is necessary. | ||
DestChainConfig memory destChainConfig = s_destChainConfigs[destChainSelector]; | ||
if (destChainConfig.chainFamilySelector == Internal.CHAIN_FAMILY_SELECTOR_EVM) { | ||
// Since the message is called after getFee, which already validates the params, no validation is necessary. | ||
Client.EVMExtraArgsV2 memory parsedExtraArgs = | ||
_parseUnvalidatedEVMExtraArgsFromBytes(extraArgs, destChainConfig.defaultTxGasLimit); | ||
|
||
return (Client._argsToBytes(parsedExtraArgs), parsedExtraArgs.allowOutOfOrderExecution); | ||
} | ||
if (destChainConfig.chainFamilySelector == Internal.CHAIN_FAMILY_SELECTOR_SVM) { | ||
bytes32 tokenReceiver = _parseSVMExtraArgsFromBytes( | ||
extraArgs, destChainConfig.maxPerMsgGasLimit, destChainConfig.enforceOutOfOrder | ||
).tokenReceiver; | ||
if (isMessageWithTokenTransfer && tokenReceiver == bytes32(0)) { | ||
revert InvalidTokenReceiver(); | ||
} | ||
// If extraArgs passes the parsing it's valid and can be returned unchanged. | ||
_parseSVMExtraArgsFromBytes(extraArgs, destChainConfig.maxPerMsgGasLimit, destChainConfig.enforceOutOfOrder); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice! 👍🏼 |
||
|
||
// ExtraArgs are required on SVM, meaning the supplied extraArgs are either invalid and we would have reverted | ||
// or we have valid extraArgs and we can return them without having to re-encode them. | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice