Skip to content

Conversation

@Unheilbar
Copy link
Contributor

@Unheilbar Unheilbar commented Oct 23, 2025

Supported by chainlink-solana

@github-actions
Copy link

github-actions bot commented Oct 23, 2025

⚠️ API Diff Results - Breaking changes detected

📦 Module: github-com-smartcontractkit-chainlink-common

🔴 Breaking Changes (3)

pkg/types.Relayer (1)
  • Solana — ➕ Added
pkg/types/chains/solana.AccountMeta (1)
  • PublicKey — Type changed:
  - [32]byte
  + PublicKey
)
pkg/types/core.Relayer (1)
  • Solana — ➕ Added

📄 View full apidiff report

silaslenihan
silaslenihan previously approved these changes Nov 20, 2025
jmank88
jmank88 previously approved these changes Nov 20, 2025
@pavel-raykov
Copy link
Contributor

Hi, this seems like a massive PR: 11k lines of code where only ~50% (pg.go + mocks) are directly marked as generated. Could you please say how much of this PR is generated using the go tools (including genAI)? I am asking since it seems that there are random code pieces which are unused, e.g., like half of pkg/types/chains/solana/lp_types.go : its const part, type EventIdl EventIDLTypes and its descendants; e.g., most of const entries in pkg/types/chains/solana/solana.go.

@Unheilbar
Copy link
Contributor Author

Unheilbar commented Nov 21, 2025

Hi, this seems like a massive PR: 11k lines of code where only ~50% (pg.go + mocks) are directly marked as generated. Could you please say how much of this PR is generated using the go tools (including genAI)? I am asking since it seems that there are random code pieces which are unused, e.g., like half of pkg/types/chains/solana/lp_types.go : its const part, type EventIdl EventIDLTypes and its descendants; e.g., most of const entries in pkg/types/chains/solana/solana.go.

proto converters were generated by my own generator, and then brushed up manually
EventIdl, EventIDLTypes are copied from chainlink-solana, to be used from the chain capability implementation.
The meaningful part is SolanaService interface definition, the rest is mostly boilerplate for piping it through the relayer.
Actually, now when I think about it, solana chain capability can use log poller's types directly. gonna remove event idls

@pavel-raykov
Copy link
Contributor

proto converters were generated by my own generator, and then brushed up manually

this seems like a confusing state - if someone wants to change proto converters, how should one proceed?

@Unheilbar
Copy link
Contributor Author

this seems like a confusing state - if someone wants to change proto converters, how should one proceed?

Just change them manually, like I did. I automated the process of initial scaffolding, but I don't think it should be fully automated

@pavel-raykov
Copy link
Contributor

this seems like a confusing state - if someone wants to change proto converters, how should one proceed?

Just change them manually, like I did. I automated the process of initial scaffolding, but I don't think it should be fully automated

ok, can you at least put the generation code in the top comments of the proto converters file so that there is at least a hint about the generation process?

@Unheilbar
Copy link
Contributor Author

Unheilbar commented Nov 21, 2025

ok, can you at least put the generation code in the top comments of the proto converters file so that there is at least a hint about the generation process?

Yes, the generator itself is currently WIP, I'll open up a PR with it shortly with the readme on how to use it

@pavel-raykov
Copy link
Contributor

ok, can you at least put the generation code in the top comments of the proto converters file so that there is at least a hint about the generation process?

Yes, the generator itself is currently WIP, I'll open up a PR with it shortly with the readme on how to use it

that is fine, but let's at least mark the files as generated, otherwise, this info will be lost.

@Unheilbar
Copy link
Contributor Author

that is fine, but let's at least mark the files as generated, otherwise, this info will be lost.

But they aren't fully generated, there is still manual work needed and I think "generated by" mark will be confusing.
Plus, the way the generator works it generates both proto + converters based on the interface surface. Whoever will have to extend this interface can't use generator for that, because it will change the protos.
Generator is used to make the initial work easier, but once its merged it shouldn't be used for extending the interface.

@jmank88
Copy link
Contributor

jmank88 commented Nov 21, 2025

ok, can you at least put the generation code in the top comments of the proto converters file so that there is at least a hint about the generation process?

Yes, the generator itself is currently WIP, I'll open up a PR with it shortly with the readme on how to use it

that is fine, but let's at least mark the files as generated, otherwise, this info will be lost.

but IIUC we do not intend to re-generate these files. They were generated once to bootstrap things, and then manually edited, and cannot be reproduced deterministically. That is fine, but they should not be confused with files that are actually re-generated. They are now regular source files, and the code owners are responsible for them - not the generator.

@Unheilbar Unheilbar changed the title [WIP] Define SolanaService for the Relayer Define SolanaService for the Relayer Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants