Skip to content

feat(solana): support V1 transactions and map the full Agave RPC error surface - #2336

Merged
EliasiOfir merged 2 commits into
mainfrom
ofir/tsk-45-solana-v1-tx
Sep 10, 2026
Merged

EliasiOfir merged 2 commits into
mainfrom
ofir/tsk-45-solana-v1-tx

Conversation

@EliasiOfir

@EliasiOfir EliasiOfir commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Closes: N/A (Notion TSK-45 / Lava board "Solana Spec Upgrade – Support Upcoming V1 Transactions")

Three commits from one review pass over Solana's upgrade path. Everything is additive; nothing is removed or renamed.

  1. feat(specs) — bump maxSupportedTransactionVersion 0 → 1 in the SOLANA GET_BLOCK_BY_NUM parse directive.
  2. feat(errors) — map the 9 Agave JSON-RPC error codes Lava was missing, including -32020 which is new in Agave v4.0.
  3. test(solana) — pin the spec value and the per-code retry policy, in response to the Qodo review.

Neither of the first two was prompted by an observed failure. Both come from Solana's published upgrade material and Agave's own source, checked against the repo.

1. V1 transaction support

Solana is activating V1 transactions (SIMD-0385): new wire format (discriminator byte 0x81), max size 4,096 bytes (was 1,232), no address-lookup tables. Active on Devnet and Testnet, pending on Mainnet.

The bump lands in both specs/mainnet-1/specs/solana.json and specs/testnet-2/specs/solana.json (otherwise identical files). SOLANAT and KOII import SOLANA and inherit it.

Honest scope note: the directive uses "transactionDetails":"none", and the node never applies the version check in none/signatures mode, so the chain tracker would not have broken at 0 either. This is a correctness fix for the declared value, not a live-bug fix. It matters for any future directive that requests transaction details, and it removes the footgun before Mainnet activation.

The rest of the spec was reviewed against the current RPC docs and found complete:

Area Finding
maxSupportedTransactionVersion Only occurrence in the repo is this directive. Nothing else pins a transaction version.
HTTP method coverage 52 in spec, 52 in docs. Exact match.
WebSocket method coverage 18 in spec, 18 in docs. Exact match.
Agave v4.0 method removals All 16 removed legacy methods (getConfirmedBlock, getRecentBlockhash, getFees, …) are absent from the spec.
getBlock / getTransaction / blockSubscribe Pass-through. Clients opt in with their own maxSupportedTransactionVersion; Lava does not inject or rewrite params.
sendTransaction / simulateTransaction Pass-through. Over 1,232 bytes the node requires base64.
Payload limits Consumer HTTP body limit 4 MB (fiber default), provider gRPC 512 MB. A 4,096-byte tx is ~5.5 KB base64. No change needed.
Rent reduction No rent assumptions in protocol code. No change needed.

2. Agave RPC error-code coverage

Agave defines a contiguous block of JSON-RPC server error codes, -32001 through -32021, in rpc-client-api/src/custom_error.rs. Lava mapped 12. The other 9 fell through to UNKNOWN_ERROR, whose retry behaviour is the default rather than a choice — and the default is to retry, so a permanent, deterministic node answer was rotated across every provider in the pairing before failing.

-32020 is the one reachable on mainnet today: Agave v4.0 changed getSignaturesForAddress to return it when the before/until signature is not found, instead of returning an empty array. The other 8 are mapped alongside it so the gap does not reopen.

Agave code Meaning Mapped to Retryable
-32006 Transaction precompile verification failure CHAIN_INVALID_SIGNATURE (3016) No
-32008 No snapshot NODE_RESOURCE_UNAVAILABLE (2013) Yes
-32011 Transaction history not available from this node CHAIN_DATA_NOT_AVAILABLE (3205) Yes
-32012 Scan error (account scan aborted) NODE_INTERNAL_ERROR (2003) Yes
-32017 Epoch rewards period still active CHAIN_SOLANA_EPOCH_REWARDS_ACTIVE (3312, new) No
-32018 Slot is not an epoch boundary USER_INVALID_PARAMS (4003) No
-32019 Long-term storage unreachable NODE_SERVICE_UNAVAILABLE (2006) Yes
-32020 Filter transaction not found CHAIN_TX_NOT_FOUND (3202) Yes
-32021 No slot history CHAIN_DATA_NOT_AVAILABLE (3205) Yes

Eight reuse existing generic errors, per the design doc's rule that a Tier-2 code is warranted only where retryability differs from the generic pattern.

-32017 is the one new code. The epoch rewards distribution window is chain state — identical on every provider — so rotating during it is always wasted work. No existing error carries "data unavailable and non-retryable", hence code 3312.

3. Tests added in response to review

Two real gaps, both now closed. Tests only; no production behaviour changed by this commit.

The spec bump had no test. TestSolanaSpec_GetBlockByNumSupportsV1Transactions is table-driven over both deployed copies. It decodes the rendered request rather than substring-matching the template, so it survives key reordering and rejects the quoted "1" form Solana does not accept, and it asserts the two networks' directives are byte-identical. Nothing previously stopped mainnet-1 and testnet-2 drifting.

The classifier tests never asserted retryability. They pinned which error each code maps to, but most targets are generic errors shared with other chain families, so flipping Retryable on one would have changed Solana's provider rotation with every test still green. TestClassifyError_SolanaRetryPolicy now pins the retry decision for all 21 codes with the policy argument recorded beside each value.

-32020 stays retryable, against the review's suggestion. The before/until signature may be absent from one node's history but present on a provider with a deeper ledger or long-term storage enabled, and the error is byte-identical in both cases, so the classifier cannot distinguish them. CHAIN_TX_NOT_FOUND and CHAIN_RECEIPT_NOT_FOUND are already retryable for this same ambiguity on every other chain, and the costs are asymmetric: a wasted rotation costs latency, whereas suppressing the retry reports "not found" for data another provider holds. Reasoning and the one open question are on that review thread, left unresolved so the decision stays visible.

Verification

Unit tests (all pass):

go test ./x/spec/keeper/ -run 'TestSolanaSpecAddMultipleSubscribe|TestSpecProposal|TestSpecAdd' -count=1   ok
go test ./protocol/common/... ./protocol/relaycore/... ./protocol/parser/... ./protocol/chaintracker/...   ok

Every new test was mutation-tested — proven to fail, not just to pass. Each guard was broken deliberately, then restored:

Mutation Test that caught it
specs/testnet-2 reverted to 0 (half-applied edit) spec test, on both the value and the parity assertion
CHAIN_TX_NOT_FOUND flipped to non-retryable retry-policy test, quoting the policy argument
-32021 mapping deleted coverage test and retry-policy test

For example:

Solana code -32021 is defined by agave rpc-client-api/src/custom_error.rs but is unmapped in
chainErrorMappings[ChainFamilySolana]; it would classify as UNKNOWN_ERROR and be retried across providers

Lava chain fetcher against live clusters (temporary test, not committed): built a jsonrpc chain parser + router + fetcher from the modified spec and ran GET_BLOCKNUM then GET_BLOCK_BY_NUM end to end.

devnet  (solana-core 4.3.0-beta.3, V1 active):  latest slot 495199468, slot 495199428 -> 3RTWHSJnoJN8sU7rv4n2MPNdZjXS9G4HZ8QkMZpUfiVD
mainnet (solana-core 4.2.2, V1 not active):    latest slot 445388103, slot 445388063 -> 8GNYTCri7ALtAKsn51byPg3H5QiJ2Azkq7CjF5s38zXn

Mainnet accepts maxSupportedTransactionVersion: 1 (and higher) in the raw getBlock call today, so the bump is safe to propose before Mainnet activation.

V1 transaction flows on Devnet (direct RPC, exercising what Lava passes through):

  • Built a real V1 transaction with solders 0.29: version 1, first byte 0x81, 1,983 bytes.
  • simulateTransaction with encoding: base64: err: null, memo program executed.
  • Same transaction with encoding: base58: -32602 "... too large: 2708 bytes (max: encoded/raw 1683/1232)", confirming base64 is mandatory above 1,232 bytes.
  • -32020 reproduced on both clusters with a well-formed signature that exists nowhere, confirming the error shape and message.

Not covered here

  • getTransaction/getBlock at version 0 against a real V1 transaction. No V1 transaction was observed in the last 400 Devnet slots or the 2,000 most recent Memo/ComputeBudget signatures — only legacy and v0. Per the docs getTransaction returns -32015 and getBlock fails the whole call in full/accounts mode; Lava already maps -32015 to CHAIN_SOLANA_TX_VERSION_UNSUPPORTED (non-retryable) and passes it through.
  • The retryable branch of -32020 could not be demonstrated live, as no pruned Solana node was available to query. That argument rests on Agave's lookup path plus repo convention.
  • Full local chain run (scripts/pre_setups/init_solana_only_with_node.sh against Devnet): not executed in the sandbox, which has no screen/lavad.
  • sendTransaction of a signed V1 tx: the Devnet faucet returned -32603 Internal error on every requestAirdrop, so no funded key was available. Simulation covers the same encoding and size path.
  • Governance proposal for the updated spec is a separate ops step.

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 breaking — additive only)
  • targeted the main branch
  • provided a link to the relevant issue or specification (V1 transactions, agave custom_error.rs)
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests (spec value and parity, retry policy for all 21 codes, contiguous coverage — each mutation-tested)
  • updated the relevant documentation or specification (error-registry-design.md table; the spec JSON is its own documentation)
  • 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

Bump maxSupportedTransactionVersion from 0 to 1 in the SOLANA
GET_BLOCK_BY_NUM parse directive (mainnet-1 and testnet-2). V1
transactions are active on Devnet/Testnet and pending on Mainnet;
nodes that do not yet know V1 accept the higher value unchanged.

Claude-Session: https://claude.ai/code/session_01BmDq4Jk3Xh9EJhC9UdJoy3
@codecov

codecov Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
consensus 8.97% <ø> (ø)
protocol 38.83% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
protocol/common/error_classifier.go 73.54% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

Test Results

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

Results for commit 1e42c13. ± Comparison against base commit 9ef694a.

♻️ This comment has been updated with latest results.

Agave defines JSON-RPC server error codes -32001..-32021 in
rpc-client-api/src/custom_error.rs. Lava mapped 12 of them; the other 9
fell through to UNKNOWN_ERROR, which is retryable by default, so a
permanent node answer was retried across every provider in the pairing
before failing.

-32020 ("Transaction N not found" on getSignaturesForAddress before/until)
is new in Agave v4.0 and was the one actually reachable on mainnet today.
The rest are mapped alongside it so the gap does not reopen.

Eight map onto existing generic errors. -32017 gets a new Tier-2 code:
the epoch rewards window is chain state, identical on every provider, so
retrying it is always wasted work.

A coverage test now walks the whole -32001..-32021 block and fails on any
unmapped code, so a future Agave addition cannot silently regress to a
retryable UNKNOWN_ERROR.

Claude-Session: https://claude.ai/code/session_01BmDq4Jk3Xh9EJhC9UdJoy3
@pull-request-size pull-request-size Bot added size/M and removed size/XS labels Sep 9, 2026
@EliasiOfir EliasiOfir changed the title feat(specs): support Solana V1 transactions in GET_BLOCK_BY_NUM feat(solana): support V1 transactions and map the full Agave RPC error surface Sep 9, 2026
@EliasiOfir
EliasiOfir marked this pull request as ready for review September 10, 2026 04:29
@EliasiOfir
EliasiOfir merged commit 04596ee into main Sep 10, 2026
31 checks passed
@EliasiOfir
EliasiOfir deleted the ofir/tsk-45-solana-v1-tx branch September 10, 2026 04:29
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Support Solana V1 transactions and complete Agave RPC error mapping

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Enables Solana V1 block requests across mainnet and testnet specifications.
• Classifies all Agave RPC errors to prevent inappropriate unknown-error retries.
• Adds epoch-reward error documentation and contiguous Agave code coverage tests.
Diagram

graph TD
  A["Solana Specs"] --> B["Block Directive"] --> C["Agave RPC"] --> D["Error Classifier"] --> E["Error Registry"] --> F["Retry Policy"]
  G["Coverage Tests"] --> D
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Generate mappings from Agave sources
  • ➕ Reduces manual drift when Agave introduces new error codes.
  • ➕ Could enforce exact upstream coverage automatically.
  • ➖ Introduces source parsing or generated-code maintenance.
  • ➖ Couples builds or release workflows to Agave implementation details.
2. Create dedicated errors for every Agave code
  • ➕ Preserves maximum Solana-specific observability and semantics.
  • ➕ Avoids grouping distinct conditions under generic Lava errors.
  • ➖ Expands the public registry with narrowly scoped error types.
  • ➖ Duplicates existing generic categories without changing retry policy.

Recommendation: Keep the explicit mappings, reuse generic Lava errors where semantics match, and add only the distinct epoch-rewards classification. The contiguous coverage test provides a lightweight drift guard without generation complexity or unnecessary registry expansion.

Files changed (6) +50 / -2

Enhancement (2) +16 / -0
error_classifier.goMap the complete Agave RPC error range +9/-0

Map the complete Agave RPC error range

• Adds mappings for the nine previously unclassified Agave codes between -32001 and -32021. The mappings distinguish user, chain-state, node-internal, and provider-availability conditions instead of falling back to retryable UNKNOWN_ERROR.

protocol/common/error_classifier.go

error_codes.goRegister the epoch-rewards-active error +7/-0

Register the epoch-rewards-active error

• Introduces CHAIN_SOLANA_EPOCH_REWARDS_ACTIVE as Tier-2 code 3312. It is intentionally non-retryable because the rewards distribution window is shared chain state across providers.

protocol/common/error_codes.go

Tests (1) +31 / -0
error_registry_test.goTest all Agave error mappings and coverage +31/-0

Test all Agave error mappings and coverage

• Adds cases for each newly mapped Agave error. A range-based regression test verifies every code from -32001 through -32021 avoids UNKNOWN_ERROR classification.

protocol/common/error_registry_test.go

Documentation (1) +1 / -0
error-registry-design.mdDocument the Solana epoch-rewards error +1/-0

Document the Solana epoch-rewards error

• Adds error code 3312 to the Tier-2 Solana registry table, including its non-retryable behavior and Agave source code.

error-registry-design.md

Other (2) +2 / -2
solana.jsonDeclare Solana V1 support on mainnet +1/-1

Declare Solana V1 support on mainnet

• Raises maxSupportedTransactionVersion from 0 to 1 in the GET_BLOCK_BY_NUM request template, preparing the mainnet specification for V1 transactions.

specs/mainnet-1/specs/solana.json

solana.jsonDeclare Solana V1 support on testnet +1/-1

Declare Solana V1 support on testnet

• Raises maxSupportedTransactionVersion from 0 to 1 in the GET_BLOCK_BY_NUM request template, matching V1 transaction availability on Solana test networks.

specs/testnet-2/specs/solana.json

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Version-one support can regress unseen 📘 Rule violation ☼ Reliability
Description
maxSupportedTransactionVersion is changed from 0 to 1 in both Solana specifications, but no
test asserts the new value in either directive. A later edit can restore the older value or let the
mainnet and testnet copies diverge without the affected test suites detecting it.
Code

specs/mainnet-1/specs/solana.json[994]

+                                "function_template": "{\"jsonrpc\":\"2.0\",\"method\":\"getBlock\",\"params\":[%d,{\"transactionDetails\":\"none\",\"rewards\":false,\"maxSupportedTransactionVersion\":1}],\"id\":1}",
Evidence
PR Compliance ID 5 requires changed behavior to have appropriate tests. Both specification copies
change the supported transaction version, while repository-wide inspection finds no test asserting
maxSupportedTransactionVersion or protecting parity between these directives.

AGENTS.md: Changes Must Maintain Adequate Unit and Integration Test Coverage
specs/mainnet-1/specs/solana.json[994-994]
specs/testnet-2/specs/solana.json[994-994]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Solana specifications now declare transaction version one support, but no test verifies the changed directive value in either copied specification.

## Fix Focus Areas
- specs/mainnet-1/specs/solana.json[994-994]
- specs/testnet-2/specs/solana.json[994-994]
- protocol/parser/parser_test.go[979-1025]

## Recommended Fix
Add a table-driven test that loads both Solana specification files, locates the `GET_BLOCK_BY_NUM` directive, and asserts that its request template sets `maxSupportedTransactionVersion` to `1`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Missing transactions still get retried 🐞 Bug ≡ Correctness
Description
The -32020 mapping uses LavaErrorChainTxNotFound, whose registry entry has Retryable: true. When
getSignaturesForAddress reports that a before or until transaction is not found, retry
handling therefore continues rotating providers despite the new coverage test's stated goal of
avoiding retries for this deterministic response.
Code

protocol/common/error_classifier.go[43]

+		{CodeEquals(-32020), LavaErrorChainTxNotFound},                                         // "Transaction N not found" (getSignaturesForAddress before/until)
Evidence
The new classifier maps -32020 to LavaErrorChainTxNotFound, while that error's registry definition
explicitly enables retries. ShouldRetryErrorWithContext returns the classification's Retryable
value, and the added coverage test only rejects LavaErrorUnknown, so it passes without correcting
the retry behavior described in its own comments.

protocol/common/error_classifier.go[40-44]
protocol/common/error_codes.go[387-390]
protocol/chainlib/node_error_handler.go[197-208]
protocol/common/error_registry_test.go[1198-1214]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Agave error -32020 is mapped to the generic transaction-not-found error, which is retryable and therefore does not prevent the provider retries this change intends to eliminate.

## Fix Focus Areas
- protocol/common/error_classifier.go[43-43]
- protocol/common/error_codes.go[387-390]
- protocol/common/error_registry_test.go[1207-1214]

## Recommended Fix
Register a dedicated non-retryable Solana error for -32020 and map the code to it rather than changing the shared transaction-not-found error. Extend the tests to assert that -32020 is classified as that error and is non-retryable, rather than checking only that it is not unknown.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This changes runtime Solana error classification and transaction-spec behavior across multiple code paths, with retryability and provider-routing implications that warrant a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread specs/mainnet-1/specs/solana.json Resolved
{CodeEquals(-32017), LavaErrorChainSolanaEpochRewardsActive}, // "Epoch rewards period still active at slot N"
{CodeEquals(-32018), LavaErrorUserInvalidParams}, // "Rewards cannot be found; slot N is not epoch boundary"
{CodeEquals(-32019), LavaErrorNodeServiceUnavailable}, // "Failed to query long-term storage; please try again"
{CodeEquals(-32020), LavaErrorChainTxNotFound}, // "Transaction N not found" (getSignaturesForAddress before/until)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Missing transactions still get retried 🐞 Bug ≡ Correctness

The -32020 mapping uses LavaErrorChainTxNotFound, whose registry entry has Retryable: true. When
getSignaturesForAddress reports that a before or until transaction is not found, retry
handling therefore continues rotating providers despite the new coverage test's stated goal of
avoiding retries for this deterministic response.
Agent Prompt
## Issue description
Agave error -32020 is mapped to the generic transaction-not-found error, which is retryable and therefore does not prevent the provider retries this change intends to eliminate.

## Fix Focus Areas
- protocol/common/error_classifier.go[43-43]
- protocol/common/error_codes.go[387-390]
- protocol/common/error_registry_test.go[1207-1214]

## Recommended Fix
Register a dedicated non-retryable Solana error for -32020 and map the code to it rather than changing the shared transaction-not-found error. Extend the tests to assert that -32020 is classified as that error and is non-retryable, rather than checking only that it is not unknown.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every factual claim here is correct, but I'm keeping -32020 on CHAIN_TX_NOT_FOUND and keeping it retryable. Flagging clearly since this is a deliberate disagreement, not an oversight.

Why not the suggested fix. -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 has no way to separate them. Confirmed 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 chain family inconsistent with every other one. And the costs are asymmetric: a wasted rotation costs latency, whereas suppressing the retry reports "not found" for data a different provider holds, which is a correctness regression and much harder to debug.

On the intent point, the comment being quoted describes why unmapped codes are dangerous, since UNKNOWN_ERROR retries by default. It was not a claim that -32020 must be non-retryable. That wording was genuinely misleading though, and 873c0f1 rewrites it: the problem with an unmapped code is that its retry behaviour is decided by the default rather than chosen.

Where this finding was right, and what changed. The tests asserted which error each code maps to but never whether it retries, so a reader could not tell deliberate from accidental. TestClassifyError_SolanaRetryPolicy now pins the retry decision for all 21 codes with the policy argument recorded beside each value. That matters more than it sounds: most targets are generic errors shared with other chain families, so flipping Retryable on one would have changed Solana's routing with every existing test still green. Verified by flipping CHAIN_TX_NOT_FOUND:

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

The open question, for human reviewers. I could not empirically demonstrate the retryable branch, since I have no pruned Solana node to point at; that half rests on reading Agave's lookup path plus repo convention. It reduces to whether Lava's Solana provider fleet actually varies in history depth. If it is uniform, this finding is right and a non-retryable code is better. Note the Solana spec declares no archive extension, unlike ETH1, so Lava cannot route by history depth either way. Leaving this thread open rather than resolving it so that decision stays visible.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up for anyone finding this thread after the merge.

This PR squash-merged as 04596ee before the test commit answering this finding reached its branch, so those tests missed the merge. They are now in #2338, which is tests only against the already-merged behaviour.

The -32020 decision is unchanged and the reasoning above still stands: it stays on CHAIN_TX_NOT_FOUND and stays retryable. TestClassifyError_SolanaRetryPolicy in #2338 pins that decision for all 21 codes, so it is now explicit rather than incidental, and flipping Retryable on a shared error fails loudly instead of silently changing Solana's provider rotation.

The open question travels to #2338 as well: whether Lava's Solana provider fleet actually varies in history depth. Uniform means this finding was right and a dedicated non-retryable code is the better call, which is a one-line change on top of that PR.


Generated by Claude Code

EliasiOfir added a commit that referenced this pull request Sep 10, 2026
Follow-up to #2336, which merged before these tests were pushed. The
behaviour is already on main; this only adds the guards for it, raised by
the Qodo review on that PR.

The spec bump had no test. TestSolanaSpec_GetBlockByNumSupportsV1Transactions
is table-driven over both deployed copies of solana.json, asserting that
GET_BLOCK_BY_NUM declares maxSupportedTransactionVersion 1 and that the
directive is identical across networks. It decodes the rendered request
rather than matching the template as a substring, so it survives key
reordering and rejects the quoted "1" form that Solana does not accept.
Nothing previously stopped the mainnet-1 and testnet-2 copies drifting.

The classifier tests asserted which error each Agave code maps to but not
whether that error retries. Most targets are generic errors shared with
other chain families, so flipping Retryable on one of them would have
changed Solana's provider rotation with every test still green.
TestClassifyError_SolanaRetryPolicy pins the decision for all 21 codes and
records the policy argument beside each value.

-32020 stays on CHAIN_TX_NOT_FOUND and stays retryable, against the
review's suggestion. The before/until signature may be absent from one
node's history but present on a provider with a deeper ledger, and the
error is byte-identical in both cases, so the classifier cannot
distinguish them. A wasted rotation costs latency; refusing to retry
would report "not found" for data another provider holds. This matches
CHAIN_TX_NOT_FOUND and CHAIN_RECEIPT_NOT_FOUND on every other chain.

Also corrects a comment that implied every code in the block should be
non-retryable; the point is that an unmapped code has its retry behaviour
decided by the default rather than chosen.

Claude-Session: https://claude.ai/code/session_01BmDq4Jk3Xh9EJhC9UdJoy3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants