Chain writer config for Solana plugin oracle creator logic #15848
Conversation
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
huangzhen1997
requested review from
george-dorin,
winder,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara,
0xAustinWang,
0xnogo and
prashantkumar1982
| switch chainFamily { | ||
| case relay.NetworkSolana: | ||
| var solConfig chainwriter.ChainWriterConfig | ||
| if solConfig, err = getSolanaChainWriterConfig(transmitter[0]); err == nil { |
There was a problem hiding this comment.
Looking at how this transmitter map is initialized, the addresses are still EVM-specific. I assume CCIP team will update it, right?
There was a problem hiding this comment.
Good point, that too needs to be changed to support solana keystore too.
I think we need to do it.
It seems similar to the previous PR @archseer sent to support solana keystore.
We need a switch statement there per chain family, and for solana, do this:
solKeys, err := d.keystore.Solana().GetAll()
Plz check with @archseer. 1 thing I see missing is the solana chain-id here.
There was a problem hiding this comment.
@archseer Shall we Implement EnabledAddressesForChain for Solana keystore as well ?
| @@ -483,6 +482,30 @@ func isUSDCEnabled(ofc offChainConfig) bool { | |||
| return ofc.exec().IsUSDCEnabled() | |||
| } | |||
|
|
|||
| // TODO once on-chain account lookup address are available, the config will be updated | |||
| func getSolanaChainWriterConfig(fromAddress string) (chainwriter.ChainWriterConfig, error) { | |||
There was a problem hiding this comment.
Please move this solana specific function to a new package ccip.configs.solana, similar to how the evm configs are structured in the ccip/configs/evm/chain_writer.go file
| InputModifications: nil, | ||
| ChainSpecificName: "commit"}, | ||
| }, | ||
| // TODO this IDL definition configured statically somewhere or passed in as parameter |
There was a problem hiding this comment.
Shouldn't this be picked from the generated IDL files?
In this case, I think it should be this:
https://github.com/smartcontractkit/chainlink-ccip/blob/17b5e617d2c8ae96f29da55346c17680df853f89/chains/solana/contracts/target/idl/ccip_router.json
| switch chainFamily { | ||
| case relay.NetworkSolana: | ||
| var solConfig chainwriter.ChainWriterConfig | ||
| if solConfig, err = getSolanaChainWriterConfig(transmitter[0]); err == nil { |
There was a problem hiding this comment.
Good point, that too needs to be changed to support solana keystore too.
I think we need to do it.
It seems similar to the previous PR @archseer sent to support solana keystore.
We need a switch statement there per chain family, and for solana, do this:
solKeys, err := d.keystore.Solana().GetAll()
Plz check with @archseer. 1 thing I see missing is the solana chain-id here.
| return chainwriter.ChainWriterConfig{}, fmt.Errorf("unexpected error: invalid CCIP Router IDL, error: %w", err) | ||
| } | ||
|
|
||
| solConfig := chainwriter.ChainWriterConfig{ |
There was a problem hiding this comment.
When the rest of the address like routerProgram and commonAddressesLookupTable etc. become available, will they be hard coded here, or provided by tooling team ?
There was a problem hiding this comment.
I think should be provided by some sort of tooling since addresses will be different for testnets than mainnet
There was a problem hiding this comment.
Is it not possible to directly refer to this file in the chainlink-ccip repo?
I worry if we create a copy here, it won't keep updated if things change on the source of truth.
There was a problem hiding this comment.
I tried to, but for some reason the generated IDL json doesn't exist in the module github.com/smartcontractkit/chainlink-ccip. It's excluded from compilation. https://chainlink-core.slack.com/archives/C0779R3AH8R/p1736537328721779?thread_ts=1736526205.277789&cid=C0779R3AH8R
core/capabilities/ccip/delegate.go
Outdated
| return | ||
| }() | ||
| case relay.NetworkSolana: | ||
| // Implement EnabledAddressesForChain for Solana as well ? |
There was a problem hiding this comment.
This interface has been EVM-only, so I'd keep the comment and maybe tackle this in the future
| } | ||
| return | ||
| }() | ||
| case relay.NetworkSolana: |
There was a problem hiding this comment.
Can we also add NetworkAptos, NetworkCosmos and NetworkStarknet definitions? They'll all look identical to the Solana case and it'll save us time in the future
|
Jira ticket https://smartcontract-it.atlassian.net/browse/NONEVM-934
This PR remove EVM specific dependency, and add Solana support for chain writer config in plugin logic. At the moment, the Solana program address required for the plugin are not yet available. They will be configured later in a different PR, and an empty place holder will be used now for unblocking E2E test. https://chainlink-core.slack.com/archives/C0779R3AH8R/p1736289787018579?thread_ts=1736269231.525509&cid=C0779R3AH8R