Solana devenv support #1023
Conversation
huangzhen1997
changed the title
There was a problem hiding this comment.
Pull request overview
Adds support plumbing for SVM v1 extraArgs and a new lane-profile knob to allow non-empty tokenReceiver for destinations that require it.
Changes:
- Propagate
TokenReceiverAllowedfromChainLaneProfileintoPartialRemoteChainConfigwhen building lane configs. - Add
TokenReceiverAllowed *booltoChainLaneProfile(with behavior documentation). - Introduce an ABI-based
serializeExtraArgsSVMV1helper for SVM v1 extraArgs encoding.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| build/devenv/implcommon.go | Wires TokenReceiverAllowed into the per-remote lane config builder. |
| build/devenv/evm/impl.go | Adds serializeExtraArgsSVMV1 helper for SVM v1 extraArgs packing. |
| build/devenv/cciptestinterfaces/interface.go | Extends ChainLaneProfile with TokenReceiverAllowed and documents expected behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
huangzhen1997
changed the title
huangzhen1997
changed the title
huangzhen1997
changed the title
| } | ||
|
|
||
| // SerializeSVMExtraArgs is the Solana family's ExtraArgsSerializer, handling versions 1. | ||
| func SerializeSVMExtraArgs(opts cciptestinterfaces.MessageOptions) []byte { |
There was a problem hiding this comment.
This is 1.6 though, right? From 2.0 onwards the extra args are encoded by the destination chain format
There was a problem hiding this comment.
Ideally yes, but devenv uses these chain-family serializer registries, it needs to be abi encoded otherwise fq cannot decode it. I spoke to Terry, the Devenv is still evolving, and this is just for unblocking us. CC @tt-cll
There was a problem hiding this comment.
I think ideally the registration we're doing above happens in the integrating repo, Solana in this instance.
cciptestinterfaces.RegisterExtraArgsSerializer(chainsel.FamilySolana, SerializeSVMExtraArgs)
That repo impl would return something to the EVM (or whoever calls it) like a byte array and anything else required for serialization to occur. fyi @winder @makramkd
huangzhen1997
force-pushed
the
jh/implement-serializeExtraArgsSVMV1-for-evm-devenv
branch
from
444bfbf to
dc8bdd1
Compare
huangzhen1997
force-pushed
the
jh/implement-serializeExtraArgsSVMV1-for-evm-devenv
branch
2 times, most recently
from
c7c32e8 to
dc8bdd1
Compare
| {Name: "accounts", Type: "bytes32[]"}, | ||
| }) | ||
| if err != nil { | ||
| panic(fmt.Sprintf("failed to create SVMExtraArgsV1 tuple type: %v", err)) |
There was a problem hiding this comment.
Can we not panic here? I don't see a clear path to adding test, asserting valid/intended errors from upstream caller
There was a problem hiding this comment.
This is supposed to be running in devenv test via a call with SendMessage. And that's the pattern with all other serialize functions
| // filterOutputsByFamily returns only the blockchain outputs matching the given chain family. | ||
| func filterOutputsByFamily(outputs []*blockchain.Output, family string) []*blockchain.Output { | ||
| var filtered []*blockchain.Output | ||
| for _, out := range outputs { | ||
| if out.Family == family { | ||
| filtered = append(filtered, out) | ||
| } | ||
| } | ||
| return filtered | ||
| } |
There was a problem hiding this comment.
This shouldn't be necessary, you can filter in the implementation and the GenericConfig object has a helper for that:
… jh/implement-serializeExtraArgsSVMV1-for-evm-devenv
|
Code coverage report:
|
huangzhen1997
deleted the
jh/implement-serializeExtraArgsSVMV1-for-evm-devenv
branch
Related PR.
Summary
Test plan