fix(solana encode): make it compatible with the latest Gateway #436
Conversation
📝 WalkthroughWalkthroughDerives a connected PDA from a provided program address (seed "connected"), adds SYSVAR_INSTRUCTIONS_PUBKEY to base account lists in both mint and non-mint branches, adds a CLI Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (--connected, --accounts)
participant Encoder as solanaEncode
participant SDK as `@solana/web3` (findProgramAddressSync)
note right of CLI `#E8F3FF`: User provides <connected> program address and optional extra accounts
CLI->>Encoder: pass connected address, data, options, accounts...
Encoder->>SDK: parse connected -> create connectedProgramId
SDK-->>Encoder: connectedPdaAccount (findProgramAddressSync "connected")
Encoder->>Encoder: compute gateway/meta PDA(s)
Encoder->>Encoder: build baseAccounts (pda, gatewayPda, ..., token/systemProgram, instructionSysvar)
Encoder-->>CLI: return encoded transaction payload and accounts
note over Encoder `#F6FFF0`: instructionSysvar (SYSVAR_INSTRUCTIONS_PUBKEY) included in accounts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/client/src/solanaEncode.ts(3 hunks)packages/commands/src/solana/encode.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: fadeev
Repo: zeta-chain/toolkit PR: 346
File: packages/commands/src/ton/depositAndCall.ts:0-0
Timestamp: 2025-06-13T15:33:54.781Z
Learning: fadeev prefers responses in English.
Learnt from: fadeev
Repo: zeta-chain/toolkit PR: 346
File: packages/commands/src/ton/deposit.ts:0-0
Timestamp: 2025-06-13T15:32:09.225Z
Learning: User fadeev prefers that responses be in English; avoid replying in Russian in future interactions.
🪛 ESLint
packages/client/src/solanaEncode.ts
[error] 50-52: Replace ⏎······anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY.toBytes()⏎···· with anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY.toBytes()
(prettier/prettier)
🪛 GitHub Actions: Lint
packages/client/src/solanaEncode.ts
[error] 50-50: Prettier formatting error. Replace the newline-wrapped anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY.toBytes() with anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY.toBytes() according to prettier/prettier.
🪛 GitHub Check: build
packages/client/src/solanaEncode.ts
[failure] 50-50:
Replace ⏎······anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY.toBytes()⏎···· with anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY.toBytes()
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/commands/src/solana/encode.ts (1)
17-20: LGTM! Documentation accurately reflects the implementation change.The updated help text correctly describes that the
--connectedparameter is now a program address used to derive the PDA, which aligns with the implementation changes insolanaEncode.ts.packages/client/src/solanaEncode.ts (3)
79-87: LGTM! Instruction sysvar correctly added to mint path.Adding
SYSVAR_INSTRUCTIONS_PUBKEYto the base accounts enables the Gateway to introspect the current transaction's instructions, which is essential for security validations in the latest Gateway implementation.
88-90: LGTM! Instruction sysvar correctly added to non-mint path.Consistent with the mint path,
SYSVAR_INSTRUCTIONS_PUBKEYis properly added to ensure Gateway compatibility.
23-27: Verify seed "connected" against Gateway protocol documentation.The PDA derivation logic is implemented correctly and follows Solana best practices. However, codebase analysis reveals:
- The seed
"connected"appears only in solanaEncode.ts and is not referenced elsewhere- The derivation pattern (Buffer.from seed + programId) is consistent with other PDAs in the codebase (e.g., "meta", "whitelist")
- No contradictions found in the codebase
To confirm the seed is correct for Gateway compatibility, verify against the latest Solana Gateway protocol specification or implementation, as this validation point exists outside the repository.
|
@hernan-clich please, review as code owner. Thanks! |
skosito
left a comment
skosito
left a comment
There was a problem hiding this comment.
looks good to me, we can also work on supporting ALT, nothing needs to change on example programs, just in a way it is called
there are tests in gateway.ts in solana repo with (ALT) in the name
| .requiredOption("--connected <address>", "Connected PDA account address") | ||
| .requiredOption( | ||
| "--connected <address>", | ||
| "Connected program address used to derive the PDA" |
There was a problem hiding this comment.
should be just Connected program address i think
| expect(accounts[3].publicKey).toBe( | ||
| hexlifyPubkey(anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY) | ||
| ); | ||
| expect(accounts[3].isWritable).toBe(false); |
There was a problem hiding this comment.
should we also add some remainingAccounts?
There was a problem hiding this comment.
Not sure I understand what you mean 🤔
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
test/solanaEncode.test.ts (3)
185-197: Consider extracting the base account count as a constant.The hardcoded
7on line 194 assumes the base account count for mint scenarios. This could break if the base accounts change. Consider making this more explicit.+ const BASE_ACCOUNTS_WITH_MINT = 7; + additionalAccounts.forEach((expectedAccount, index) => { - const account = accounts[index + 7]; + const account = accounts[index + BASE_ACCOUNTS_WITH_MINT]; expect(account.publicKey).toBe(expectedAccount.publicKey); expect(account.isWritable).toBe(expectedAccount.isWritable); });
200-219: Consider more specific error assertions.The error tests verify that invalid inputs throw, but could be more specific about the expected error type or message for better test documentation and catching unintended errors.
Example:
await expect(solanaEncode(input)).rejects.toThrow(/Invalid account format/);
26-198: Consider extracting common PDA derivation logic.The three main test cases duplicate PDA and account derivation logic. Extracting this into a helper function would improve maintainability and make the tests more focused on their specific scenarios.
Example helper:
const deriveTestAccounts = async (input: any) => { const connectedProgramId = new anchor.web3.PublicKey(input.connected); const [connectedPdaAccount] = anchor.web3.PublicKey.findProgramAddressSync( [Buffer.from("connected", "utf-8")], connectedProgramId ); const [gatewayPdaAccount] = anchor.web3.PublicKey.findProgramAddressSync( [Buffer.from("meta", "utf-8")], new anchor.web3.PublicKey(input.gateway) ); let connectedPdaAta; let mintPubkey; if (input.mint) { mintPubkey = new anchor.web3.PublicKey(input.mint); connectedPdaAta = await getAssociatedTokenAddress( mintPubkey, connectedPdaAccount, true ); } return { connectedPdaAccount, gatewayPdaAccount, connectedPdaAta, mintPubkey }; };This would reduce duplication and make test updates easier.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/commands/src/solana/encode.ts(1 hunks)test/solanaEncode.test.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/commands/src/solana/encode.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: fadeev
Repo: zeta-chain/toolkit PR: 346
File: packages/commands/src/ton/depositAndCall.ts:0-0
Timestamp: 2025-06-13T15:33:54.781Z
Learning: fadeev prefers responses in English.
Learnt from: fadeev
Repo: zeta-chain/toolkit PR: 346
File: packages/commands/src/ton/deposit.ts:0-0
Timestamp: 2025-06-13T15:32:09.225Z
Learning: User fadeev prefers that responses be in English; avoid replying in Russian in future interactions.
🧬 Code graph analysis (1)
test/solanaEncode.test.ts (1)
packages/client/src/solanaEncode.ts (1)
solanaEncode(16-103)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (3)
test/solanaEncode.test.ts (3)
1-24: LGTM! Well-structured test utilities.The helper functions and type definitions are clean and promote code reuse throughout the test suite. The
decodeEncodedResultandhexlifyPubkeyhelpers effectively reduce duplication and improve test readability.
26-64: LGTM! Comprehensive validation of base encoding.The test properly validates both the encoded data and the account structure. The test at lines 124-198 addresses the previous review comment about
remainingAccountsby including additional accounts scenarios.
66-122: LGTM! Thorough validation of mint scenario.The test comprehensively validates the mint encoding path, including the associated token address derivation and all seven expected accounts with correct writability flags.
|
Merging this. Will be adding ALT support in follow up PR. |
Program:
https://solana.fm/address/GhVHQQt9R3tHKjsewDphTsXLwnoTkdaZNzTovEgWfTGP/transactions?cluster=devnet-solana
CCTX:
https://zetachain-athens.blockpi.network/lcd/v1/public/zeta-chain/crosschain/inboundHashToCct[…]5da0676493c959522b2b6015a6fb311d331b902d7c4587ecd5c8
Solana tx:
https://solana.fm/tx/4p9X2MRCfbA7XSERVrnzN9kMZfzEXfdyvJaa7NZqDFj7U62AZim2aGpZ7QhHTKBbNkYekHmzhS1z6zHKubcCyvty?cluster=devnet-solana
Tested with: zeta-chain/example-contracts#287
Associated docs: zeta-chain/docs#685
Summary by CodeRabbit
Documentation
--connectednow described as a program address.New Features
--accounts <accounts...>option to supply extra accounts with writability.Refactor
Tests