Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
andrevmatos
force-pushed
the
api/get_execution_input
branch
from
0176a1f to
2c87434
Compare
andrevmatos
force-pushed
the
api/get_execution_input
branch
from
a54e4fd to
3347926
Compare
PabloMansanet
previously approved these changes
Feb 27, 2026
Contributor
PabloMansanet
left a comment
PabloMansanet
left a comment
There was a problem hiding this comment.
Looking good, just two minor comments. I'll handle the fork test early Monday.
And use it in EVMChain.[generateUnsigned]Execute({ messageId }) method
useful if you're executing with `dest.execute({ messageId })` only
andrevmatos
force-pushed
the
api/get_execution_input
branch
from
7d3e5c2 to
76310cf
Compare
Coverage Report |
andrevmatos
force-pushed
the
api/get_execution_input
branch
from
12abb78 to
9dd1853
Compare
aelmanaa
requested changes
Feb 27, 2026
| } | ||
|
|
||
| const messagesInBatch = raw.messageBatch.map(decodeMessage) | ||
| const message = messagesInBatch.find((message) => message.messageId === messageId)! |
Collaborator
There was a problem hiding this comment.
What's the behaviour if the message is not found? (in runtime this will become undefined)
Collaborator
Author
There was a problem hiding this comment.
It should NEVER not be found, if API is doing what it should; if it can't, then it's expected the API itself will return an HTTP error. It's basically an invariant of the function of the endpoint, that it should include the message we're looking in the batch for its messageId
|
|
||
| const proof = calculateManualExecProof(messagesInBatch, lane, messageId, raw.merkleRoot, this) | ||
|
|
||
| const rawMessage = raw.messageBatch.find((message) => message.messageId === messageId)! |
Collaborator
There was a problem hiding this comment.
What's the behaviour if the rawMessage is not found? (in runtime this will become undefined)
Collaborator
Author
There was a problem hiding this comment.
Same. The message MUST be there, if the API does what it's supposed to; any error should be thrown at http response level
| offRamp, | ||
| version: CCIPVersion.V2_0, | ||
| encodedMessage: raw.encodedMessage, | ||
| verifications: (raw.ccvData ?? []).map((ccvData, i) => ({ |
Collaborator
There was a problem hiding this comment.
I'd suggest some defensive coding in case there is a bug in the API, which will raise an error earlier instead of passing it throgh the layers (which will be harder to investigate) . imho, we should validate that ccvData.length === verifierAddresses.length
Collaborator
Author
There was a problem hiding this comment.
Hmm, I'd say it's not sdk's responsibility to verify behavior of the API, but just to assume and use it. It's more or less the same with RPC: we must assume a correctly working data source layer, otherwise nothing will ever be able to be achieved here. At most, we validate for things that CAN happen in normal operation
| destChainSelector: bigint | ||
| version: string | ||
| } | ||
| | object |
Collaborator
Author
There was a problem hiding this comment.
It's linter lang for {}, but it doesn't like the empty type; the difference between this union and just making all those properties optional, is that it tells TS that, if any is present, all of them are, while optionals I'd need to check one by one
| sourceChainSelector: raw.sourceChainSelector, | ||
| destChainSelector: raw.destChainSelector, | ||
| onRamp: raw.onramp, | ||
| version: raw.version as CCIPVersion, |
Collaborator
There was a problem hiding this comment.
Why don't you validate the value like we do for other part in the code (validateChainFamily / validateMessageStatus) just for the sake of defensive coding
Collaborator
Author
There was a problem hiding this comment.
We don't do much validation; in this case, I'd argue we WANT this endpoint to support "unexpected" versions, as this may allow for things to continue working optimistically in case the SDK is outdated with CCIP's version
Collaborator
Author
There was a problem hiding this comment.
But I agree, we do some validation elsewhere, also coming from the API, so maybe we should do it here. I'll implement
| CCIPTimeoutError, | ||
| CCIPUnexpectedPaginationError, | ||
| } from '../errors/index.ts' | ||
| import { decodeMessageV1 } from '../evm/messages.ts' |
Collaborator
There was a problem hiding this comment.
can we move decodeMessageV1 out of evm/messages.ts to another file that doesn't import ./const.ts? Otherwise it will pull ABI files in the bundle even for users who are not using EVM (even users who only use CCIPAPIClient)
Collaborator
Author
There was a problem hiding this comment.
Sure, good catch!
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
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.
/execution-inputsAPI endpoint for both v1 and v2 messagesdest.execute({ messageId })manual-execwhen given amessageIdsource: