feat: Add SVM destination support for CCTP strategy #2049
Conversation
ashwinrava
requested review from
0xjorgen,
NikolasHaimerl,
amateima,
dohaki,
gsteenkamp89,
melisaguevara,
mrice32,
nicholaspai and
pxrl
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
api/_bridges/cctp/utils/constants.ts
Outdated
| export const DEFAULT_CCTP_ACROSS_FINALIZER_ADDRESS = | ||
| "0x72adB07A487f38321b6665c02D289C413610B081"; | ||
|
|
||
| const CCTP_FINALIZER_ADDRESS_OVERRIDES: Record<number, string> = { |
There was a problem hiding this comment.
Is this an override or should we simply have a chainId to address mapping for the finalizer?
There was a problem hiding this comment.
Hmm so I'm following the same override pattern that we do for getCctpTokenMessengerAddress and getCctpMessageTransmitterAddress: https://github.com/across-protocol/frontend/blob/master/api/_bridges/cctp/utils/constants.ts#L52-L73
I think we don't need a full mapping because all the EVM chain ids will use the same address so we'd have a bunch of redundant entries. Thoughts?
There was a problem hiding this comment.
I think in this case is better if we use chainIsSvm() check to decide which address to return. Wdyt?
There was a problem hiding this comment.
Yea I also think just adding DEFAULT_CCTP_ACROSS_FINALIZER_ADDRESS_SVM should be sufficient for now
dohaki
left a comment
dohaki
left a comment
There was a problem hiding this comment.
LGTM 👍 Just one nit about the ongoing conversation
api/_bridges/cctp/utils/constants.ts
Outdated
| export const DEFAULT_CCTP_ACROSS_FINALIZER_ADDRESS = | ||
| "0x72adB07A487f38321b6665c02D289C413610B081"; | ||
|
|
||
| const CCTP_FINALIZER_ADDRESS_OVERRIDES: Record<number, string> = { |
There was a problem hiding this comment.
Yea I also think just adding DEFAULT_CCTP_ACROSS_FINALIZER_ADDRESS_SVM should be sufficient for now
dohaki
left a comment
dohaki
left a comment
There was a problem hiding this comment.
LGTM! Is the SVM finalizer in prod already though? We should probably enable this in the API after the finalizer is ready right?
Yup I'll merge them in order. Going to push the finalizer changes first to my release branch and if everything looks good, I'll merge this. |
amateima
left a comment
amateima
left a comment
There was a problem hiding this comment.
Before merging this we should test the entire flow with the indexer and finalizer bot
Yeah I tested it end to end: swap API -> indexer -> finalizer and it's working well. Here is one finalize transaction for example. |
Changes:
test/api/_bridges/cctp/strategy.test.ts.Testing: I tested locally by creating a deposit transaction from the swap API (without marker to avoid the prod finalizer from picking it up) and ran the finalizer code locally to confirm the mint works as expected.