[EliasiOfir]

Description

Follow-up to #2336, which squash-merged as 04596ee about two hours before these tests were pushed to its branch, so they missed the merge.

Tests only. No production code, no behaviour change. Everything under test is already on main. Both guards are the two findings from the Qodo review on #2336.

Files to review: two _test.go files, +126 lines, nothing removed.

1. The spec bump had no test

TestSolanaSpec_GetBlockByNumSupportsV1Transactions in protocol/parser/parser_test.go, table-driven over both deployed copies of solana.json.

Two deliberate choices:

• It decodes the rendered request rather than substring-matching the template, so the assertion survives key reordering or whitespace, and the string "1" cannot pass as the integer 1. Solana rejects the quoted form, and a substring match on maxSupportedTransactionVersion would not catch it.
• It also asserts the two networks' templates are byte-identical. That is the more valuable half: specs/mainnet-1 and specs/testnet-2 are copies differing only in the proposal deposit, and nothing in the repo stopped them drifting. A half-applied two-file edit is a realistic failure mode.

2. The classifier tests never asserted retryability

They pinned which error each Agave code maps to, but most targets are generic errors shared with other chain families. Flipping Retryable on one of them would have changed Solana's provider rotation with every existing test still green.

TestClassifyError_SolanaRetryPolicy pins the decision for all 21 codes in the -32001..-32021 block and records the policy argument beside each value. Retryable means a different provider could plausibly answer; non-retryable means every provider returns the same thing and rotating only burns the pairing.

-32020 stays retryable, against the review's suggestion

This is the one open disagreement, carried over from the review thread on #2336, which was deliberately left unresolved.

-32020 is not deterministic across providers. Whether the before/until signature is found depends on the node's ledger depth and whether long-term storage is enabled, so a pruned node returns -32020 where an archive node answers successfully. The message is byte-identical in both cases, so the classifier cannot separate them. Reproduced live on both clusters with a well-formed signature that exists nowhere:

{"code":-32020,"message":"Transaction 5RkdZnHmvG95UqGHj9p5Vap8xEkbQCtBnkvLJYWfoD9Eu9azRhQmXTuzVFN1z21b385VdguUaYdunpZiBYGAZBB4 not found"}

Two further reasons: CHAIN_TX_NOT_FOUND (3202) and CHAIN_RECEIPT_NOT_FOUND (3203) are already retryable for exactly this ambiguity, so a non-retryable Solana variant would make this family inconsistent with every other one; and the costs are asymmetric, since a wasted rotation costs latency whereas suppressing the retry reports "not found" for data another provider holds.

What I could not prove. The retryable branch was not demonstrated live, as no pruned Solana node was available to query; that half rests on reading Agave's lookup path plus repo convention. It reduces to one question for reviewers: does Lava's Solana provider fleet actually vary in history depth? If it is uniform, the review is right and a dedicated non-retryable code is better, which is a one-line change on top of this PR. Note the Solana spec declares no archive extension, unlike ETH1, so Lava cannot route by history depth either way.

Verification

Both tests pass against merged main:

go test ./protocol/parser/ ./protocol/common/ -count=1     ok

Both were mutation-tested against merged main, not just run green. Each guard was broken deliberately, then restored:

Mutation	Result
specs/testnet-2 reverted to 0, a half-applied edit	spec test fails on both the value and the parity assertion
-32020 mapping deleted	coverage test and retry-policy test both fail
CHAIN_TX_NOT_FOUND flipped to non-retryable (checked pre-merge)	retry-policy test fails, quoting the policy argument

A test that has never been seen to fail is not yet known to test anything. For example:

Solana -32020 (CHAIN_TX_NOT_FOUND) must have Retryable=true
  — the before/until signature may exist on a provider with deeper history

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• read the contribution guide
• included the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change (not applicable — tests only)
• targeted the main branch
• provided a link to the relevant issue or specification (feat(solana): support V1 transactions and map the full Agave RPC error surface #2336, V1 transactions, agave custom_error.rs)
• reviewed "Files changed" and left comments if necessary
• included the necessary unit and integration tests (this PR is the tests; each was mutation-tested)
• updated the relevant documentation or specification (not applicable — no behaviour change; the error-registry-design.md row landed with feat(solana): support V1 transactions and map the full Agave RPC error surface #2336)
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed all author checklist items have been addressed
• reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

https://claude.ai/code/session_01BmDq4Jk3Xh9EJhC9UdJoy3

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
consensus	8.97% <ø> (ø)
protocol	38.85% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 204 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[qodo-code-review]

PR Summary by Qodo

Pin Solana V1 spec and retry policy in tests

🧪 Tests 🕐 10-20 Minutes

AI Description

• Pins Solana V1 getBlock configuration across mainnet and testnet specifications.
• Locks classification and retryability for all 21 Agave JSON-RPC errors.
• Documents why provider-dependent error -32020 remains retryable.

Diagram

graph TD
  CI["Go Test Suite"] --> SG["Spec Guard"] --> MS["Mainnet Spec"] --> RR["Rendered Request"]
  SG --> TS["Testnet Spec"] --> RR
  CI --> RG["Retry Guard"] --> EC["Error Classifier"] --> RP["Retry Policy"]

Loading

High-Level Assessment

The scoped regression-test approach is appropriate because the intended production behavior already exists. Decoding rendered JSON is safer than substring matching, and testing the public classifier result protects both mapping and retryability. Centralizing or generating the duplicated network specs could remove drift structurally, but that would introduce unrelated production/configuration changes beyond this follow-up.

Files changed (2) +126 / -4

Tests (2) +126 / -4

error_registry_test.goPin Solana error mappings and retry decisions +61/-4

Pin Solana error mappings and retry decisions

• Adds table-driven coverage for the mapped Lava error and retryability of every Agave code from -32001 through -32021. It records the policy rationale per code, including why -32020 remains retryable, and clarifies the existing coverage test's relationship to default retry behavior.

protocol/common/error_registry_test.go

parser_test.goVerify V1 transaction support in deployed Solana specs +65/-0

Verify V1 transaction support in deployed Solana specs

• Loads both deployed Solana specifications and verifies GET_BLOCK_BY_NUM renders valid JSON with numeric maxSupportedTransactionVersion set to 1. It also requires the mainnet and testnet templates to remain identical, preventing partial spec updates.

protocol/parser/parser_test.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[github-actions]

Test Results

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
7 files   +1   0 ❌ ±0 

Results for commit 2dd754e. ± Comparison against base commit b13b379.