fix(canton): prod testnet send and manual-exec for ccip-cli and ccip-sdk#278
Open
SyedAsadKazmi wants to merge 3 commits into
Open
fix(canton): prod testnet send and manual-exec for ccip-cli and ccip-sdk#278SyedAsadKazmi wants to merge 3 commits into
SyedAsadKazmi wants to merge 3 commits into
Conversation
SyedAsadKazmi
requested review from
a team,
PabloMansanet and
aelmanaa
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
SyedAsadKazmi
force-pushed
the
fix/canton-prod-testnet-cli-sdk
branch
from
cc12ebd to
7d8f66b
Compare
Coverage Report |
RodrigoAD
reviewed
Jun 26, 2026
RodrigoAD
left a comment
RodrigoAD
left a comment
Member
There was a problem hiding this comment.
Thanks @SyedAsadKazmi for the fixes! Left you some comments. Although we should get a review from @andrevmatos
| const sourceNetwork = networkInfo(argv.source) | ||
| const destNetwork = networkInfo(argv.dest) | ||
| const cantonConfig = loadCantonConfig(argv.cantonConfig, logger) | ||
| const router = resolveCliRouter( |
Member
There was a problem hiding this comment.
It's probably best to not have chain specific logic inside the CLI commands. This affects to other resolve... functions.
Something like this, that follows other patterns like fetchChainsFromRpcs might be better
// providers/index.ts
function resolveRouter(argv, sourceNetwork) {
const cantonConfig = loadCantonConfig(argv.cantonConfig)
// ...
@andrevmatos can you give us your thoughts here?
Contributor
Author
There was a problem hiding this comment.
Moved router/indexer resolution into providers/index.ts as resolveRouter / resolveIndexer, following the same pattern as fetchChainsFromRpcs (commands pass argv + network/chain context; canton-config loading stays in the providers layer). Low-level helpers remain in canton.ts for unit tests.
| } | ||
|
|
||
| /** Extract RawInstanceAddress.unpack strings from a list field on createArgument. */ | ||
| function extractStringListField(createArgument: unknown, fieldName: string): string[] { |
Member
There was a problem hiding this comment.
This function is quite hard to understand, what's its purpose? Can we simplify it?
Contributor
Author
There was a problem hiding this comment.
Split the monolithic extractStringListField into smaller helpers:
readCreateArgumentField- flat object or{ fields: [{ label, value }] }asLedgerList- bare array vs{ list }/{ elements }extractRawInstanceAddressUnpack- oneRawInstanceAddressentry ->unpackhexextractRequiredCCVs- composes the above for CCIPReceiverrequiredCCVs
Removed the separate requiredCCVsLength helper; callers use extractRequiredCCVs(...).length when needed.
| import { id as keccak256Utf8 } from 'ethers' | ||
|
|
||
| /** Normalize a hex CCV InstanceAddress for comparison (lowercase, no 0x). */ | ||
| export function normalizeCcvHex(value: string): string { |
Member
There was a problem hiding this comment.
I've seen this in acs.ts too. Let's avoid the duplication. Also, I think there's some general utils in the repo for this
Contributor
Author
There was a problem hiding this comment.
Consolidated the duplicated helpers into utils.ts:
- Exported
normalizeHex,hashedUtf8Hex, andisCantonPartyId acs.tsandccv-addresses.tsnow import these instead of local copies (removed the duplicateisCantonPartyId/normalizeHexinacs.tsandnormalizeCcvHexinccv-addresses.ts)getAddressBytesuseshashedUtf8Hexfor Canton party hashing as well
| tokenMetadataClient: TokenMetadataClient, | ||
| ccipParty: string, | ||
| indexerUrl = MAINNET_INDEXER_URLS[0]!, | ||
| ledgerParty?: string, |
Member
There was a problem hiding this comment.
We should probably make this required, defaulting to ccipParty doesn't make too much sense for users
Contributor
Author
There was a problem hiding this comment.
Made ledgerParty required on the constructor and fromClient, removed the ?? ccipParty fallback, and updated the CantonConfig docs to clarify party (user ledger / actAs) vs ccipParty (CCIP operator). fromUrl now validates that ctx.cantonConfig.party is set and passes it through explicitly.
| } | ||
|
|
||
| /** Daml party ID: `hint::1220<64-hex-fingerprint>` (not a 3-part instrument id). */ | ||
| function isCantonPartyId(address: string): boolean { |
Member
There was a problem hiding this comment.
Same as other utilities, I've seen them duplicated in different places.
SyedAsadKazmi
force-pushed
the
fix/canton-prod-testnet-cli-sdk
branch
from
24f5530 to
300a186
Compare
RodrigoAD
approved these changes
Jun 26, 2026
RodrigoAD
left a comment
RodrigoAD
left a comment
Member
There was a problem hiding this comment.
@SyedAsadKazmi LGTM, let's wait for @andrevmatos 👍
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
2 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.
Fix Canton prod testnet CCIP flows (Sepolia ↔ Canton) in ccip-tools-ts. Canton support already exists; this patch set addresses gaps that blocked prod testnet lanes.
SDK fixes
ccip-runtime, etc.)requiredCCVsmatching on executechainIdin canton-config when synchronizer alias is generic (global) so prod testnet resolves tocanton:TestNetinstead of defaulting tocanton:DevNetCLI fixes
-r/--indexer/ccvsfrom canton-config where appropriate--rpclists so EVM endpoints aren’t raced against Canton ledger URLsmanual-execby defaultshowaftersendKnown limitations / workarounds
--only-get-feeon Canton source — not useful today; Canton has no scalar upfront fee (getFeereturns0). Real cost is computed in the on-chain send command.Gas estimation on Canton source —
estimateReceiveExecutioncallstypeAndVersionon the source OnRamp/router;CantonChain.typeAndVersionis not implemented, so positive--estimate-gas-limitfails for Canton→EVM sends. **typeAndVersionor a Canton-specific gas estimator]Workaround: pass
--estimate-gas-limit -101(skips estimation when<= -100) and set gas explicitly, e.g.-x gasLimit=200000Tested
--estimate-gas-limit -101+-x gasLimit=…)