Efficient extraArgs & non-evm support #1359
Open
Conversation
This file contains hidden or 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
# Conflicts: # chains/evm/.gas-snapshot # chains/evm/contracts/onRamp/OnRamp.sol # chains/evm/contracts/test/helpers/OnRampTestHelper.sol # chains/evm/contracts/test/onRamp/OnRamp/OnRampSetup.t.sol
RensR
commented
Nov 7, 2025
| uint64 destChainSelector, | ||
| uint16 requestedBlockDepth, | ||
| Client.CCV[] calldata ccvs, | ||
| address[] calldata ccvs, |
Collaborator
Author
There was a problem hiding this comment.
It never needed the ccv args but it was more efficient to pass in the CCVs struct
RensR
force-pushed
the
genericextraArgs
branch
from
be9e438 to
40b310f
Compare
RensR
force-pushed
the
genericextraArgs
branch
from
5a9021f to
796bf24
Compare
add fields
RensR
force-pushed
the
genericextraArgs
branch
from
796bf24 to
22c5e88
Compare
|
kylesmartin
reviewed
Nov 7, 2025
| /// a block depth. CCVs, Pools and the executor may all reject this value by reverting the transaction on the source | ||
| /// chain if they do not want to take on the risk of the block depth specified. | ||
| /// @dev May be zero to indicate waiting for finality is desired. | ||
| uint16 finalityConfig; |
Contributor
There was a problem hiding this comment.
Do we want to change this to match the new blockConfirmations naming?
| if (extraArgs.length >= 4 && bytes4(extraArgs[0:4]) == Client.GENERIC_EXTRA_ARGS_V3_TAG) { | ||
| resolvedArgs = abi.decode(extraArgs[4:], (Client.GenericExtraArgsV3)); | ||
| ) internal view returns (ExtraArgsCodec.GenericExtraArgsV3 memory resolvedArgs) { | ||
| if (extraArgs.length >= 4 && bytes4(extraArgs[0:4]) == ExtraArgsCodec.GENERIC_EXTRA_ARGS_V3_TAG) { |
Contributor
There was a problem hiding this comment.
Suggested change
| if (extraArgs.length >= 4 && bytes4(extraArgs[0:4]) == ExtraArgsCodec.GENERIC_EXTRA_ARGS_V3_TAG) { | |
| if (extraArgs.length >= 4 && bytes4(extraArgs[:4]) == ExtraArgsCodec.GENERIC_EXTRA_ARGS_V3_TAG) { |
| uint256 public constant SUI_EXECUTOR_ARGS_V1_BASE_SIZE = 4 + 1; | ||
|
|
||
| // Enum to indicate specific error locations during encoding/decoding. | ||
| enum EncodingErrorLocation { |
Contributor
There was a problem hiding this comment.
Is part of the intention here to prevent having to update this for new non-EVMs?
If so, how is space for params that new non-EVMs may need accounted for?
0xsuryansh
reviewed
Nov 7, 2025
| bytes memory encoded = ExtraArgsCodec._encodeSVMExecutorArgsV1(args); | ||
| ExtraArgsCodec.SVMExecutorArgsV1 memory decoded = s_helper._decodeSVMExecutorArgsV1(encoded); | ||
|
|
||
| assertEq(decoded.accountIsWritableBitmap, type(uint64).max); |
Member
There was a problem hiding this comment.
assertEq(uint256(decoded.useATA), uint256(args.useATA));
adding this assertion test fails..
0xsuryansh
reviewed
Nov 7, 2025
| s_helper = new ExtraArgsCodecHelper(); | ||
| } | ||
|
|
||
| function test_DecodeSVMExecutorArgsV1_Empty() public view { |
Member
There was a problem hiding this comment.
Suggested change
| function test_DecodeSVMExecutorArgsV1_Empty() public view { | |
| function test_decodeSVMExecutorArgsV1_Empty() public view { |
or intuitionally kept uppercase for all the tests ?
0xsuryansh
reviewed
Nov 7, 2025
| assembly { | ||
| // Read useATA (1 byte). | ||
| let useATA := byte(0, calldataload(add(encoded.offset, 4))) | ||
| mstore(executorArgs, iszero(iszero(useATA))) |
Member
There was a problem hiding this comment.
Doing this would means both enum cases DERIVE_ATA_DONT_CREATE (1) and USE_AS_IS (2) decode to the same 1.
is this done to intentionally ?
if so please add comments
0xsuryansh
reviewed
Nov 7, 2025
| /// @notice Decodes bytes into a SVMExecutorArgsV1 struct using assembly. | ||
| /// @param encoded The encoded bytes to decode. | ||
| /// @return executorArgs The decoded SVMExecutorArgsV1 struct. | ||
| function _decodeSVMExecutorArgsV1( |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
No description provided.