Skip to content

[Feature]Standardize JSON-RPC error handling(revert codes, LiteNode pruned-history responses, request fields validation) #6676

Description

@0xbigapple

Summary

Align java-tron's JSON-RPC behavior with the Ethereum Execution API standard in three areas: contract revert error codes, LiteNode pruned-history responses, and JSON-RPC 2.0 request validation. This improves compatibility with Ethereum tooling, makes LiteNode failure modes explicit, and adds an opt-in strict protocol validation mode.

Problem

Motivation

java-tron's JSON-RPC behavior currently diverges from the Ethereum Execution API standard in ways that can break compatibility with wallets, DApps, SDKs, and debugging tools that assume Ethereum-compatible semantics.

Current State

  1. eth_call and eth_estimateGas currently return -32000 for all execution failures, without distinguishing TVM revert from other failures such as OUT_OF_ENERGY.
  2. On LiteNode which historical blocks have been pruned, clients cannot distinguish between "data was pruned" and "data does not exist". The earliest tag is also treated as the genesis block rather than the lowest available block on the node.
  3. java-tron relies on jsonrpc4j's default behavior, which does not strictly validate the jsonrpc: "2.0" field. In addition, certain id values are handled in a way that is not suitable for blockchain node scenarios, for example treating "id": null as a notification and returning no response.

Limitations or Risks

  • DApps and tooling cannot reliably distinguish contract revert from internal execution failures, which affects error handling and UX.
  • LiteNode clients cannot make informed fallback decisions, such as retrying against an archive node, because pruned-history cases are not explicitly signaled.
  • Protocol-level request handling is less consistent with mainstream Ethereum clients such as geth, which increases cross-client compatibility risk.

Proposed Solution

Proposed Design

Introduce the following behavior changes:

  1. eth_call and eth_estimateGas return error code 3 for contract revert, and include the revert payload in the data field as hex. Non-revert execution failures continue to return -32000.
  2. On LiteNode, return error code 4444 with message Pruned history unavailable when the requested block range is deterministically below the node's lowest available block. This applies to:
    • eth_getBlockByNumber
    • eth_getBlockTransactionCountByNumber
    • eth_getTransactionByBlockNumberAndIndex
    • eth_getBlockReceipts when block number/tag is used
    • eth_getLogs
    • eth_newFilter
  3. Resolve earliest to the node's lowest available block number instead of always mapping it to the genesis block. For single-block queries, explicit 0x0 keeps its genesis-block meaning.
  4. Add a new config item node.jsonrpc.strictComplianceMode, disabled by default. When enabled:
    • requests missing jsonrpc: "2.0" or carrying an invalid version return -32600 Invalid Request
    • invalid id types return responses with id: null
  5. Limit pruned-history handling to selectors that can be deterministically evaluated by block number or tag. Hash-only methods such as eth_getBlockByHash and eth_getTransactionByHash remain out of scope.

Key Changes

  • Modules
    • JSON-RPC execution error mapping
    • LiteNode block selector / history availability handling
    • JSON-RPC Servlet request validation
  • Configuration
    • add node.jsonrpc.strictComplianceMode = false
  • API / Behavior Surface
    • eth_call
    • eth_estimateGas
    • eth_getBlockByNumber
    • eth_getBlockTransactionCountByNumber
    • eth_getTransactionByBlockNumberAndIndex
    • eth_getBlockReceipts
    • eth_getLogs
    • eth_newFilter
  • Testing
    • add regression coverage for error mapping, pruned-history handling, earliest resolution, and strict mode validation

Impact

This change primarily affects API compatibility and developer experience, with positive impact on interoperability with Ethereum tooling. Performance impact should be negligible: protocol validation is an O(1) field check, and pruned-history validation happens before deeper business logic, which can reduce unnecessary I/O on LiteNode.

Compatibility

  • Breaking Change: Yes
  • Default Behavior Change: Partial. eth_call / eth_estimateGas revert responses and LiteNode pruned-history responses change behavior immediately; strictComplianceMode is added but remains disabled by default.
  • Migration Required: Possibly. Clients that currently rely on -32000 for all execution failures or expect null for pruned LiteNode data may need to adjust. Clients that omit the jsonrpc field are unaffected unless strictComplianceMode is explicitly enabled.

References

Additional Notes

  • Do you have ideas regarding implementation? Yes
  • Are you willing to implement this feature? Yes

Activity

  1. 317787106 commented on Apr 13, 2026

    @317787106
    Collaborator

    @0xbigapple Excellent optimization! Maintaining compatibility with the Ethereum ecosystem helps reduce maintenance costs for TRON developers, and keeping track of already resolved bugs is also crucial for service stability as that #6632.

    Is it necessary to set the default value of node.jsonrpc.strictComplianceMode to true? Because, in fact, the specification requires id and version to follow fixed formats. This requirement for id and version is not created recently — it has been in place since the specification was first introduced.

  2. lxcmyf commented on Apr 13, 2026

    @lxcmyf
    Collaborator

    I checked the current JSON-RPC implementation and the proposal matches several real gaps in the code path:

    • eth_call / eth_estimateGas currently collapse all execution failures into JsonRpcInternalException, and the interface annotation maps that to -32000. Even when revert data is present, the current code only puts it into data; it does not switch the code to 3 for revert.
    • earliest is currently hard-wired to block 0 in both JsonRpcApiUtil.getByJsonBlockId(...) and Wallet.getByJsonBlockId(...), so the issue description about genesis behavior is accurate.
    • For number/tag-based queries such as eth_getBlockByNumber, eth_getTransactionByBlockNumberAndIndex, and eth_getBlockReceipts, the current behavior is mostly null when the resolved block is unavailable. That means LiteNode pruned-history and true not-found are not distinguishable at the API layer.

    One implementation detail that seems important: JsonRpcServlet currently hands the request directly to jsonrpc4j (rpcServer.handle(req, resp)). So if strictComplianceMode is added, the jsonrpc / id validation likely has to happen before normal dispatch in the servlet layer, not just in JsonRpcErrorResolver, because notification handling and request-shape parsing already happen inside jsonrpc4j.

    For the collaborator question about the default value: based on the current implementation, I think strictComplianceMode = false is the safer rollout. Switching it to true by default would immediately change protocol-level behavior for existing clients that rely on the current jsonrpc4j tolerance, while the revert-code and pruned-history changes can be scoped more narrowly and verified method by method.

    A practical structure might be:

    • add a dedicated exception for EVM-compatible revert responses and map that one to code 3
    • add a dedicated pruned-history exception and map it to 4444
    • centralize block tag resolution so earliest can resolve to lowest available in LiteNode-aware paths
    • gate request-shape validation behind strictComplianceMode in JsonRpcServlet
  3. 0xbigapple commented on Apr 13, 2026

    @0xbigapple
    CollaboratorAuthor

    @0xbigapple Excellent optimization! Maintaining compatibility with the Ethereum ecosystem helps reduce maintenance costs for TRON developers, and keeping track of already resolved bugs is also crucial for service stability as that #6632.

    Is it necessary to set the default value of node.jsonrpc.strictComplianceMode to true? Because, in fact, the specification requires id and version to follow fixed formats. This requirement for id and version is created recently — it has been in place since the specification was first introduced.

    @317787106 Thanks for the feedback.
    On the default value question: I'd prefer to keep strictComplianceMode = false for the initial rollout. While the spec has always required jsonrpc: "2.0", the reality is that java-tron has been tolerant of these deviations since day one, and some existing clients may depend on that tolerance. Flipping the default to true immediately would be a silent breaking change for those clients with no migration path.
    Also, if downstream feedback shows most clients are already compliant, we can accelerate the timeline for defaulting to true. We can bring this topic to the developer meeting for broader community input on the right default.

  4. 0xbigapple commented on Apr 13, 2026

    @0xbigapple
    CollaboratorAuthor

    I checked the current JSON-RPC implementation and the proposal matches several real gaps in the code path:

    • eth_call / eth_estimateGas currently collapse all execution failures into JsonRpcInternalException, and the interface annotation maps that to -32000. Even when revert data is present, the current code only puts it into data; it does not switch the code to 3 for revert.
    • earliest is currently hard-wired to block 0 in both JsonRpcApiUtil.getByJsonBlockId(...) and Wallet.getByJsonBlockId(...), so the issue description about genesis behavior is accurate.
    • For number/tag-based queries such as eth_getBlockByNumber, eth_getTransactionByBlockNumberAndIndex, and eth_getBlockReceipts, the current behavior is mostly null when the resolved block is unavailable. That means LiteNode pruned-history and true not-found are not distinguishable at the API layer.

    One implementation detail that seems important: JsonRpcServlet currently hands the request directly to jsonrpc4j (rpcServer.handle(req, resp)). So if strictComplianceMode is added, the jsonrpc / id validation likely has to happen before normal dispatch in the servlet layer, not just in JsonRpcErrorResolver, because notification handling and request-shape parsing already happen inside jsonrpc4j.

    For the collaborator question about the default value: based on the current implementation, I think strictComplianceMode = false is the safer rollout. Switching it to true by default would immediately change protocol-level behavior for existing clients that rely on the current jsonrpc4j tolerance, while the revert-code and pruned-history changes can be scoped more narrowly and verified method by method.

    A practical structure might be:

    • add a dedicated exception for EVM-compatible revert responses and map that one to code 3
    • add a dedicated pruned-history exception and map it to 4444
    • centralize block tag resolution so earliest can resolve to lowest available in LiteNode-aware paths
    • gate request-shape validation behind strictComplianceMode in JsonRpcServlet

    @lxcmyf Great analysis — the code-level verification is very helpful, especially the point about JsonRpcServlet and jsonrpc4j dispatch ordering.

    I agree with the implementation structure you outlined. A few thoughts on each piece:

    Revert error code 3: Yes, a dedicated exception (e.g. JsonRpcExecutionRevertedException) that carries the raw revert payload and maps to code 3 is the right approach. The current JsonRpcInternalException catch-all is the root cause of the -32000 conflation. The key implementation detail is detecting revert vs other failures — we can check for the REVERT result code from TVM execution and extract the revert data from the transaction result.

    Pruned-history 4444: Agreed on a dedicated JsonRpcPrunedHistoryException. The check should happen early — right after block tag resolution and before any store lookups — to avoid unnecessary I/O on LiteNode.

    earliest resolution: Centralizing block tag resolution is important. On FullNode earliest = genesis, on LiteNode earliest = lowest available. This should be a single utility method that both JsonRpcApiUtil.getByJsonBlockId() and Wallet.getByJsonBlockId() delegate to.

    Servlet-layer validation: Good catch on the dispatch ordering. The jsonrpc/id validation must intercept the request before jsonrpc4j's handle(), since jsonrpc4j already makes its own assumptions about notifications and request shape. A pre-dispatch filter in JsonRpcServlet gated by strictComplianceMode is the right place.

  5. lvs0075 commented on Apr 14, 2026

    @lvs0075
    Collaborator

    The direction of this proposal is sound. JSON-RPC behavioral divergence from the Ethereum standard has been a consistent friction point for DApp and tooling integration.

    Error code 3 (revert distinction) has the broadest impact and the highest return. All execution failures currently returning -32000 makes it impossible for SDKs and debugging tools to distinguish contract revert from out-of-energy failures, directly hurting UX and error handling. This is a real behavior change: clients that currently rely on -32000 to detect revert will need to adapt. I'd recommend including a migration example in the PR description and calling this out prominently in release notes for at least one or two versions.

    Error code 4444 (LiteNode pruned history) is logically sound, but 4444 falls outside the standard JSON-RPC error code range. It would help to explain in the PR why existing codes like -32000 or -32002 weren't reused, and whether any other Ethereum clients have used a similar custom code — that context matters for evaluating cross-client interoperability.

    strictComplianceMode off by default is the right conservative default. Supportive.

  6. 0xbigapple commented on Apr 14, 2026

    @0xbigapple
    CollaboratorAuthor

    The direction of this proposal is sound. JSON-RPC behavioral divergence from the Ethereum standard has been a consistent friction point for DApp and tooling integration.

    Error code 3 (revert distinction) has the broadest impact and the highest return. All execution failures currently returning -32000 makes it impossible for SDKs and debugging tools to distinguish contract revert from out-of-energy failures, directly hurting UX and error handling. This is a real behavior change: clients that currently rely on -32000 to detect revert will need to adapt. I'd recommend including a migration example in the PR description and calling this out prominently in release notes for at least one or two versions.

    Error code 4444 (LiteNode pruned history) is logically sound, but 4444 falls outside the standard JSON-RPC error code range. It would help to explain in the PR why existing codes like -32000 or -32002 weren't reused, and whether any other Ethereum clients have used a similar custom code — that context matters for evaluating cross-client interoperability.

    strictComplianceMode off by default is the right conservative default. Supportive.

    Thanks for the review! Regarding the concern about error code 4444, here is some additional context:

    1. JSON-RPC 2.0 Specification: Only the negative range -32768 to -32000 is reserved for pre-defined errors. Positive error codes are fully available for application-defined use (spec).

    2. Ethereum Official Execution API Specification: Error code 4444 has been standardized as "Pruned history unavailable" in the official Execution API docs (reference).

    3. Geth: PR #31361 implements full support, covering all historical query endpoints including eth_getBlockByNumber, eth_getBlockReceipts, eth_getLogs, and more.

    4. Besu: PR #9643 also uses PRUNED_HISTORY_UNAVAILABLE(4444).

    In summary, 4444 is a standard error code established across the Ethereum ecosystem — from specification to implementation — aligned with EIP-4444 (Bound Historical Data in Execution Clients).

  7. waynercheung commented on Apr 14, 2026

    @waynercheung
    Collaborator

    Building on @lvs0075’s points about migration notes for revert code 3 and keeping strictComplianceMode off by default, I checked the current code path locally and there are a few implementation details worth locking down early:

    • Distinguishing REVERT likely needs a Wallet-side change, not only a new JSON-RPC exception. TVM/runtime already distinguishes REVERT vs OUT_OF_ENERGY, but the current constant-call path in Wallet.callConstantContract(...) only writes ret = SUCESS/FAILED into the returned Transaction. So JSON-RPC currently sees “failed + optional revert payload”, but not a preserved contractRet.
    • On LiteNode, I agree it would help to explain why 4444 is used instead of reusing an existing generic server code. From the implementation side, the bigger semantic question is what should happen for ranges that cross lowestBlockNum. Today LogFilterWrapper resolves the numeric range first, and LogBlockQuery scans it without a LiteNode floor check, so requests like fromBlock: 0x0, toBlock: latest can still degrade into silent empty/incomplete results.
    • Because of that, it would help to define whether 4444 should be returned whenever the resolved lower bound is below lowestBlockNum, or only when the whole requested range is below it. If we only gate on the latter, cross-boundary eth_getLogs / eth_newFilter requests can still return incomplete data silently.
    • On strict validation, I agree with default-off for the initial rollout. I also checked the jsonrpc4j 1.6 integration path locally: java-tron currently passes requests straight into JsonRpcServlet -> rpcServer.handle(...), and jsonrpc4j still defaults to backwards-compatible request handling; id: null is treated as a notification, and even non-backwards-compatible mode only checks that a jsonrpc field exists, not that it equals "2.0". So strict validation needs to happen before normal dispatch in JsonRpcServlet, rather than relying on JsonRpcErrorResolver or jsonrpc4j defaults.
    • One small resolver detail to watch: JsonRpcErrorResolver matches @JsonRpcError entries in declaration order using isInstance(...). If the new revert/pruned exceptions subclass JsonRpcInternalException, they need to be declared before the generic JsonRpcInternalException mapping, otherwise they will still resolve to -32000.
  8. waynercheung commented on Apr 14, 2026

    @waynercheung
    Collaborator

    @0xbigapple Thanks for the clarification on 4444 - if it is already defined in the Execution API docs and implemented in geth / Besu, then using it here is clearly the compatibility path rather than a java-tron-specific extension.

    After checking the current code path locally, the remaining questions seem to be mostly about scope and edge semantics:

    • The current proposal still scopes pruned-history handling more narrowly than the latest Execution API surface, especially by keeping hash-only methods out of scope for now.
    • For LiteNode ranges, especially eth_getLogs / eth_newFilter, it would help to define the cross-boundary rule explicitly: if fromBlock < lowestBlockNum <= toBlock, should the request fail with 4444, or return only the available suffix? The current path can otherwise degrade into silent empty/incomplete results.
    • On the revert side, TVM/runtime already distinguishes REVERT vs other execution failures, but the current constant-call path only writes ret = SUCESS/FAILED back into the returned Transaction. So clean code = 3 behavior may require a small Wallet-side change, not only a new JSON-RPC exception mapping.

    For strictComplianceMode, I agree that default-off is the safer initial rollout. One implementation question here: should this be introduced as a real config switch with default false, or effectively hard-coded to false in the first PR? A config-controlled switch seems more compatibility-friendly, since operators can opt in early, downstream tooling can test against it before any future default change, and it gives a clearer migration path if the project ever wants to default it to true later.

    So from my perspective the main direction looks largely settled on code = 3 for execution reverted and code = 4444 for pruned history, while the rollout shape for strictComplianceMode still seems worth confirming - especially whether it should be introduced as a config-controlled, default-off switch.

    The main things still worth pinning down before implementation are the LiteNode boundary behavior and whether the first PR should intentionally stay narrower than the current Execution API coverage.

  9. 0xbigapple commented on Apr 14, 2026

    @0xbigapple
    CollaboratorAuthor

    Building on @lvs0075’s points about migration notes for revert code 3 and keeping strictComplianceMode off by default, I checked the current code path locally and there are a few implementation details worth locking down early:

    • Distinguishing REVERT likely needs a Wallet-side change, not only a new JSON-RPC exception. TVM/runtime already distinguishes REVERT vs OUT_OF_ENERGY, but the current constant-call path in Wallet.callConstantContract(...) only writes ret = SUCESS/FAILED into the returned Transaction. So JSON-RPC currently sees “failed + optional revert payload”, but not a preserved contractRet.
    • On LiteNode, I agree it would help to explain why 4444 is used instead of reusing an existing generic server code. From the implementation side, the bigger semantic question is what should happen for ranges that cross lowestBlockNum. Today LogFilterWrapper resolves the numeric range first, and LogBlockQuery scans it without a LiteNode floor check, so requests like fromBlock: 0x0, toBlock: latest can still degrade into silent empty/incomplete results.
    • Because of that, it would help to define whether 4444 should be returned whenever the resolved lower bound is below lowestBlockNum, or only when the whole requested range is below it. If we only gate on the latter, cross-boundary eth_getLogs / eth_newFilter requests can still return incomplete data silently.
    • On strict validation, I agree with default-off for the initial rollout. I also checked the jsonrpc4j 1.6 integration path locally: java-tron currently passes requests straight into JsonRpcServlet -> rpcServer.handle(...), and jsonrpc4j still defaults to backwards-compatible request handling; id: null is treated as a notification, and even non-backwards-compatible mode only checks that a jsonrpc field exists, not that it equals "2.0". So strict validation needs to happen before normal dispatch in JsonRpcServlet, rather than relying on JsonRpcErrorResolver or jsonrpc4j defaults.
    • One small resolver detail to watch: JsonRpcErrorResolver matches @JsonRpcError entries in declaration order using isInstance(...). If the new revert/pruned exceptions subclass JsonRpcInternalException, they need to be declared before the generic JsonRpcInternalException mapping, otherwise they will still resolve to -32000.

    @0xbigapple Thanks for the clarification on 4444 - if it is already defined in the Execution API docs and implemented in geth / Besu, then using it here is clearly the compatibility path rather than a java-tron-specific extension.

    After checking the current code path locally, the remaining questions seem to be mostly about scope and edge semantics:

    • The current proposal still scopes pruned-history handling more narrowly than the latest Execution API surface, especially by keeping hash-only methods out of scope for now.
    • For LiteNode ranges, especially eth_getLogs / eth_newFilter, it would help to define the cross-boundary rule explicitly: if fromBlock < lowestBlockNum <= toBlock, should the request fail with 4444, or return only the available suffix? The current path can otherwise degrade into silent empty/incomplete results.
    • On the revert side, TVM/runtime already distinguishes REVERT vs other execution failures, but the current constant-call path only writes ret = SUCESS/FAILED back into the returned Transaction. So clean code = 3 behavior may require a small Wallet-side change, not only a new JSON-RPC exception mapping.

    For strictComplianceMode, I agree that default-off is the safer initial rollout. One implementation question here: should this be introduced as a real config switch with default false, or effectively hard-coded to false in the first PR? A config-controlled switch seems more compatibility-friendly, since operators can opt in early, downstream tooling can test against it before any future default change, and it gives a clearer migration path if the project ever wants to default it to true later.

    So from my perspective the main direction looks largely settled on code = 3 for execution reverted and code = 4444 for pruned history, while the rollout shape for strictComplianceMode still seems worth confirming - especially whether it should be introduced as a config-controlled, default-off switch.

    The main things still worth pinning down before implementation are the LiteNode boundary behavior and whether the first PR should intentionally stay narrower than the current Execution API coverage.

    @waynercheung Thanks for the follow-up! We're largely aligned on direction. Responding to the remaining questions:

    1. Cross-boundary query semantics

    The rule is: return 4444 whenever fromBlock < lowestBlockNum. This is consistent with Geth. earliest on LiteNode resolves to lowestBlockNum, so fromBlock: earliest, toBlock: latest works correctly, while an explicit fromBlock: 0x0 below the pruning cutoff returns 4444.

    2. Hash-based method coverage

    This is not an intentional scope reduction — it's a technical limitation:

    • Block hash queries: java-tron's LiteNode has no hash → block number mapping for pruned data. Although TRON's BlockId embeds the block number in its first 8 bytes, we cannot verify whether the input is a valid BlockId — external tools may pass arbitrary 32-byte hex, and the first 8 bytes would be parsed as a meaningless number, potentially causing false positives (e.g., returning 4444 instead of null for a non-existent hash).
    • Transaction hash queries: Pure hashes with no way to associate them with a block number. Geth also does not return 4444 for eth_getTransactionByHash — it simply returns nil when not found.

    So hash-based queries can only return null for now.

    3. Revert Wallet-side change

    Agreed. The plan is to set contractRet (e.g. REVERT, OUT_OF_ENERGY, etc.) in TransactionResultCapsule within Wallet.callConstantContract(), so the JSON-RPC layer can directly check contractRet == REVERT instead of relying on error message string matching. A side effect is that gRPC/HTTP triggerConstantContract responses will also include the contractRet field, but this is filling in previously missing information — not a breaking change.

    4. strictComplianceMode

    Agreed — it should be introduced as a real config switch with default false, not hard-coded.

  10. waynercheung commented on Apr 14, 2026

    @waynercheung
    Collaborator

    @0xbigapple Thanks, this clarifies the remaining semantics well.

    The 4444 cross-boundary rule, the Wallet-side contractRet propagation plan, and strictComplianceMode as a config-controlled default-off switch all sound right. The main thing left is to make sure the PR implementation and tests match these agreed semantics.

  11. lxcmyf commented on Apr 15, 2026

    @lxcmyf
    Collaborator

    @0xbigapple Thanks, we’re aligned on the implementation shape.

    The two points I especially agree with are:

    • REVERT probably needs to be preserved in the Wallet-side constant-call path first, otherwise JSON-RPC still has to infer too much from FAILED + optional data.
    • strict validation has to happen before jsonrpc4j dispatch in JsonRpcServlet, not in the resolver layer.

    On the LiteNode side, I’d also lean toward making the boundary rule explicit in tests from the start: if the resolved lower bound is below lowestBlockNum, return 4444 rather than silently serving a partial suffix. That keeps eth_getLogs / eth_newFilter from degrading into incomplete results.

    So from my side the remaining work is mostly making sure the PR matches these semantics cleanly in code and tests.

  12. halibobo1205 commented on May 15, 2026

    @halibobo1205
    Collaborator

    Noticed this feature has been removed from the 4.8.2 scope — wanted to check on the current progress. The design converged in mid-April (revert code 3 / pruned history 4444 / earliest resolution / strictComplianceMode as a config switch / cross-boundary rule / Wallet-side contractRet change). What's the current state of the PR, and which release is it targeting next?

  13. 0xbigapple commented on May 15, 2026

    @0xbigapple
    CollaboratorAuthor

    Noticed this feature has been removed from the 4.8.2 scope — wanted to check on the current progress. The design converged in mid-April (revert code 3 / pruned history 4444 / earliest resolution / strictComplianceMode as a config switch / cross-boundary rule / Wallet-side contractRet change). What's the current state of the PR, and which release is it targeting next?

    Thanks for the follow-up. This feature was deferred from 4.8.2 because the release ended up including a number of overlapping refactors and adjustments in related code paths. Building on top of an in-flux base would have made both the implementation and the review harder than necessary.

    The plan is to pick this back up against the post-4.8.2 codebase, so the work can be built on the cleaned-up logic. We're targeting the next release after 4.8.2 for landing this feature. The mid-April design conclusions still stand.

  14. 0xbigapple commented on Aug 11, 2026

    @0xbigapple
    CollaboratorAuthor

    While implementing the design from the discussion above, two things surfaced that I'd like to settle here before opening the PR.

    1. On a LiteNode, receipt availability lags block availability

    lowestBlockNum (derived from block-index) only marks block-body availability. A DbLite snapshot backfills the recent 65,536 block bodies into block / block-index / trans, but transactionRetStore / transactionHistoryStore are excluded and never backfilled — so the entire retained window passes the 4444 check while having no receipts or logs, permanently.

    Verified on nodes bootstrapped from the official snapshots:

    network body floor receipts start from
    Nile (backup 2026-04-13) 66,475,399 66,540,934
    Mainnet (backup 2026-03-16) 80,919,127 80,984,662

    Inside that ~65k-block gap:

    • eth_getBlockReceipts → -32000 "TransactionList size mismatch: block has N transactions, but transactionInfoList has 0"
    • eth_getLogs → silently incomplete results (LogMatch skips blocks whose info list is empty)
    • eth_getTransactionReceipt → null forever, while eth_getTransactionByHash returns the mined transaction

    — exactly the failure modes this issue set out to eliminate, one level above the cutoff.

    Proposed amendment (implemented, will be part of the PR):

    • Probe a lowestReceiptBlockNum at startup: the first key of transactionRetStore; an empty store means receipts begin with the next executed block. It is probed from the data rather than derived from snapshot metadata or arithmetic — the semantics of info.properties may vary across tool generations, the 65,536-block window (RECENT_BLKS) may be adjusted in the future, and only the store itself knows where its receipts start. With storage.transHistory.switch = off receipts are never persisted at all, so receipt-dependent endpoints answer 4444 unconditionally instead of promising a floor that never materializes.
    • Receipt/log endpoints (eth_getLogs, eth_newFilter, eth_getBlockReceipts) check the lowestReceiptBlockNum ; block-body endpoints keep lowestBlockNum, so bodies in the gap stay servable by explicit number.
    • earliest anchors to the start of the node's available history. With receipts persisted, this refines the earlier "lowest available block" rule into the lowestReceiptBlockNum (lowest block with complete data); with storage.transHistory.switch = off no such block exists, so it falls back to lowestBlockNum and receipt-dependent ranges anchored there fail loudly with 4444. Either way earliest never yields silently-incomplete results.
    • Hash-based selectors stay null-only, as agreed.

    Alternatively, we could also consider having the Toolkit keep the retained window's receipts when generating the snapshot — the way geth prunes block bodies and receipts on a single boundary. That direction probably deserves a separate issue.

    2. Strict-mode validation: what jsonrpc4j actually does with id / params / method

    Implementation-time verification against a live node established the actual baseline for each request shape, and the strict-mode decision:

    request shape JSON-RPC 2.0 jsonrpc4j today (verified) strict mode
    missing / wrong jsonrpc version invalid tolerated -32600
    missing / non-string method invalid (§5 example) -32601 -32600
    scalar params (e.g. "params": "x") invalid (§4.2) no response -32600
    params: null invalid (§4.2: must be Array/Object when present) served (zero-argument dispatch) -32600
    id: null permitted but discouraged (§4 [1]) treated as a notification → no response -32600
    array / object id invalid no response -32600 with id: null

    On the id: null row: the spec permits it but explicitly discourages it (§4 note [1] — Null collides with the id used in Responses to unknown-id requests, and with JSON-RPC 1.0 notification semantics), and the dispatcher additionally treats a null id as a notification and answers nothing. Rejecting a discouraged shape with a loud -32600 seems better than the silent no-response.

    Everything above is gated behind strictComplianceMode (default false); flag-off behavior stays byte-identical to today.

    Looking forward to your thoughts.

  15. 0xbigapple commented on Aug 24, 2026

    @0xbigapple
    CollaboratorAuthor

    @317787106 @lxcmyf @lvs0075 @waynercheung I’m very much looking forward to your suggestions and feedback.

  16. waynercheung commented on Aug 26, 2026

    @waynercheung
    Collaborator

    @0xbigapple Thanks for the ping, and sorry for the slow reply. I had a look at feature/jsonrpc-error-handling on your fork so the comments below point at actual lines rather than at the design sketch. Two parts: the LiteNode receipt floor, then strict mode, where this overlaps work I have in flight.

    1. Receipt floor

    Probing the store itself rather than deriving the floor from info.properties or RECENT_BLKS arithmetic is the right call, and the three failure modes you list inside the gap are exactly the ones this issue set out to remove. Four points on the implementation.

    (a) transHistory.switch = off does not reach FullNodes. Your comment says receipt-dependent endpoints should answer 4444 unconditionally when receipts are never persisted. checkPrunedReceiptHistory returns early on !wallet.isLiteNode(), but TransactionRetStore.put gates on storage.transHistory.switch for every node type, not just LiteNodes:

    public void put(byte[] key, TransactionRetCapsule item) {
      if (BooleanUtils.toBoolean(CommonParameter.getInstance()
          .getStorage().getTransactionHistorySwitch())) {
        super.put(key, item);
      }
    }

    So a FullNode with receipts disabled still answers eth_getLogs with silently incomplete results. ChainBaseManager.init() already sets lowestReceiptBlockNum = Long.MAX_VALUE for that case, so the information is there; it is just behind the isLiteNode() gate. Suggest moving the "no floor exists" branch ahead of the node-type check.

    (b) earliest resolution is the one place the body/receipt split is not applied. You already made that distinction for the checks - checkPrunedHistory on lowestBlockNum, checkPrunedReceiptHistory on the receipt floor - but parseBlockTag resolves earliest to the receipt floor globally, including for body-only endpoints. With your own Nile numbers (body floor 66,475,399, receipt floor 66,540,934), eth_getBlockByNumber("earliest") skips about 65k blocks that the same endpoint still serves by explicit number, so earliest stops meaning "the earliest block this endpoint can return".

    My preference is endpoint-specific resolution: body-only endpoints resolve earliest to lowestBlockNum, receipt and log endpoints to lowestReceiptBlockNum. A single global receipt floor makes body endpoints skip blocks they can still serve; a single global body floor makes receipt endpoints fail or return incomplete data. An explicit parameter (something like HistoryKind.BODY / HistoryKind.RECEIPT) would keep callers from picking implicitly. This is completing a distinction the branch already makes rather than adding a new concept.

    Related: your alternative of having the Toolkit retain the window's receipts when generating the snapshot would collapse the two floors into one and make this whole question disappear, which is roughly what geth gets by pruning bodies and receipts on a single boundary. If that is on the table, endpoint-specific resolution is the interim answer rather than the end state, and it would be worth saying so in the PR.

    (c) The probe runs before checkpoint recovery. The comment in ChainBaseManager.init() accepts that the first session can be one block conservative and that restarts self-correct. Requiring an operator restart to correct an RPC boundary is a rough edge for a public API. lowestReceiptBlockNum already has a setter, so refreshing it once recovery completes looks cheap. Alternatively, establish and test an invariant showing no actually available receipt block is ever rejected during that first session.

    (d) Keep the wire message exactly as specified. The Execution API defines the message verbatim as Pruned history unavailable. prunedMessage() currently appends the floor to it, and there is a second variant for the not-persisted case, so clients matching the documented string will miss both. Suggest keeping message exact and moving the floor into data - that is what data is for, and a client deciding whether to fall back to an archive node needs the boundary value, so a log line does not serve it. JsonRpcExecutionRevertedException already carries a data constructor; a symmetric one on JsonRpcPrunedHistoryException would cover this.

    2. Strict mode

    (a) The servlet hooks collide with a change I have in flight.

    I have an unmerged change that hardens the JSON-RPC error path: it replaces jsonrpc4j's -32001 unmapped fallback with -32603, lets VirtualMachineError / ThreadDeath / LinkageError escape rather than be dressed up as a normal error response, and for that last part moves the single-request path from handle(req, resp) to handleRequest(InputStream, OutputStream), because JsonRpcServer.handle catches Throwable. It also rejects, before dispatch, any request id that is not a String, Number or Null.

    That lands on the same two lines yours does:

    • in doPost, immediately after the empty-batch check, where you insert violatesStrictCompliance(rootNode);
    • in handleBatch, on the if (!subRequest.isObject()) line, which you extend with violatesStrictCompliance(subRequest) and I extend with an id-validity check.

    Two differences worth settling now rather than at merge time:

    • My id check is unconditional; yours is inside the flag. If both land as-is, the id row of violatesStrictCompliance becomes unreachable, and we carry two copies of the same predicate (mine in an isValidRequestId helper, yours inlined twice).
    • I moved the malformed-sub-request check ahead of the if (overflow) branch, so that once the response budget is exhausted a malformed sub-request is still reported as -32600 rather than -32003. Yours stays after it. Mine is the better order, but either way we should pick one.

    Two smaller side effects of the transport switch, since they touch files you may rebase over: CachedBodyRequestWrapper and the HttpStatusCodeProvider become unreachable and are removed.

    (b) Which rows actually need the flag.

    This is the part I would most like your view on. The six rows are not homogeneous in compatibility risk. Grouped by what a client sees today with the flag off:

    row behavior today client contract to preserve?
    missing / wrong jsonrpc request served normally yes, real
    params: null served for zero-arg methods; -32602 via arity for methods with parameters yes, real
    non-string method -32601 error either way, low risk
    scalar params no response no
    array / object id no response no
    id: null no response (treated as a notification) no, but see (c)

    For the two rows in the middle group with no response at all - scalar params and non-scalar id - there is nothing for a client to depend on, and returning nothing to a request that carries an id is itself the 2.0 deviation. Gating those behind a default-off flag preserves the deviation without buying any compatibility, which is why my change makes the id one unconditional. My suggestion is to treat those two the same way and keep the flag for the rows that change a response clients actually receive today.

    Two geth data points that may help with the top row, since they are not in the table: isCall() requires hasValidVersion(), so geth already rejects a missing or wrong jsonrpc field - making that row strict moves toward geth rather than away from it. Conversely hasValidID() only excludes { and [, so geth accepts a boolean id; both our implementations reject it. That is correct per §4 (id MUST be String, Number or Null) but it is a divergence from geth and belongs in the release note.

    (c) id: null is the one row where strict mode is stricter than the spec and than geth.

    Your table already marks it permitted-but-discouraged and the code comment registers the deviation, so this is confirmation rather than a new objection. Adding the geth behavior: ID is a json.RawMessage, so an explicit null is four bytes rather than nil. isNotification() requires msg.ID == nil and therefore does not match, while hasValidID() only rejects a leading { or [. So geth dispatches it as an ordinary call and echoes "id": null in the response. rpc/json.go L56-L73

    So under the flag we would deviate from both the specification and geth on this row, which I think argues for two things: keep this row behind the flag even if the other two silent rows go unconditional, and record it in the release note rather than only in a code comment.

    I looked at whether answering normally is reachable instead. It is, but not cheaply: jsonrpc4j decides notification-ness internally, so the servlet would have to rewrite the id to a sentinel before dispatch and restore null on the way out. I do not think that is worth doing in this PR - I mention it only so the choice is on the record as a constraint of the framework rather than a preference.

    (d) scalar params in a batch. Under the flag your per-sub-request check already does the right thing: -32600 for the offending sub-request, valid siblings still executed. The gap is the flag-off path, which is pre-existing on develop rather than anything you introduced: handleBatch catches RuntimeException and returns, replacing the whole batch with a single error object. In our testing scalar params is one of the shapes that produces that exception, so today one malformed sub-request discards the entire batch. That is an additional argument for making this row unconditional per (b).

    3. Scope boundary and the resolver

    I have been working on a related but distinct layer: null values inside the params array - "params": [null] reaching a method body after arity and dispatch (filter objects, call objects, filter ids, fullTransactionObjects, DTO fields) - plus error mapping and internal-detail sanitization. That never touches the request envelope. Proposed split:

    • request envelope (jsonrpc, method, params container type, id type) -> this issue, with the rollout policy decided per row: compatibility-sensitive legacy behavior stays behind strictComplianceMode, while malformed requests that currently receive no response can be corrected unconditionally;
    • values inside params, plus error mapping and sanitization -> a separate tracking issue I am preparing.

    I will cross-link both ways when I open it so the boundary is visible from either side.

    On the resolver specifically: your branch does not modify the production JsonRpcErrorResolver, only the annotation mappings on TronJsonRpc plus the resolver tests, while my change rewrites the resolver but preserves declaration-order matching. So the runtime semantics stay compatible and only the annotation and test changes need care during rebasing.

    One durable improvement while you are there. Both JsonRpcExecutionRevertedException and JsonRpcPrunedHistoryException extend JsonRpcInternalException, which is what creates the ordering dependency I raised back in April. The order is correct on the branch today - I checked estimateGas, getCall, getBlockReceipts and getLogs, and both are declared ahead of the generic JsonRpcInternalException mapping - so this is not a live bug. But the dependency is invisible at the call site, and a future reorder or a third subclass would regress it silently. Since both carry wire codes distinct from -32000, extending JsonRpcException directly would remove the dependency entirely, and it would match every other exception in org.tron.core.exception.jsonrpc, all of which extend the base class rather than each other.

  17. 0xbigapple commented on Aug 27, 2026

    @0xbigapple
    CollaboratorAuthor

    @0xbigapple Thanks for the ping, and sorry for the slow reply. I had a look at feature/jsonrpc-error-handling on your fork so the comments below point at actual lines rather than at the design sketch. Two parts: the LiteNode receipt floor, then strict mode, where this overlaps work I have in flight.

    1. Receipt floor

    Probing the store itself rather than deriving the floor from info.properties or RECENT_BLKS arithmetic is the right call, and the three failure modes you list inside the gap are exactly the ones this issue set out to remove. Four points on the implementation.

    (a) transHistory.switch = off does not reach FullNodes. Your comment says receipt-dependent endpoints should answer 4444 unconditionally when receipts are never persisted. checkPrunedReceiptHistory returns early on !wallet.isLiteNode(), but TransactionRetStore.put gates on storage.transHistory.switch for every node type, not just LiteNodes:

    public void put(byte[] key, TransactionRetCapsule item) {
    if (BooleanUtils.toBoolean(CommonParameter.getInstance()
    .getStorage().getTransactionHistorySwitch())) {
    super.put(key, item);
    }
    }
    So a FullNode with receipts disabled still answers eth_getLogs with silently incomplete results. ChainBaseManager.init() already sets lowestReceiptBlockNum = Long.MAX_VALUE for that case, so the information is there; it is just behind the isLiteNode() gate. Suggest moving the "no floor exists" branch ahead of the node-type check.

    (b) earliest resolution is the one place the body/receipt split is not applied. You already made that distinction for the checks - checkPrunedHistory on lowestBlockNum, checkPrunedReceiptHistory on the receipt floor - but parseBlockTag resolves earliest to the receipt floor globally, including for body-only endpoints. With your own Nile numbers (body floor 66,475,399, receipt floor 66,540,934), eth_getBlockByNumber("earliest") skips about 65k blocks that the same endpoint still serves by explicit number, so earliest stops meaning "the earliest block this endpoint can return".

    My preference is endpoint-specific resolution: body-only endpoints resolve earliest to lowestBlockNum, receipt and log endpoints to lowestReceiptBlockNum. A single global receipt floor makes body endpoints skip blocks they can still serve; a single global body floor makes receipt endpoints fail or return incomplete data. An explicit parameter (something like HistoryKind.BODY / HistoryKind.RECEIPT) would keep callers from picking implicitly. This is completing a distinction the branch already makes rather than adding a new concept.

    Related: your alternative of having the Toolkit retain the window's receipts when generating the snapshot would collapse the two floors into one and make this whole question disappear, which is roughly what geth gets by pruning bodies and receipts on a single boundary. If that is on the table, endpoint-specific resolution is the interim answer rather than the end state, and it would be worth saying so in the PR.

    (c) The probe runs before checkpoint recovery. The comment in ChainBaseManager.init() accepts that the first session can be one block conservative and that restarts self-correct. Requiring an operator restart to correct an RPC boundary is a rough edge for a public API. lowestReceiptBlockNum already has a setter, so refreshing it once recovery completes looks cheap. Alternatively, establish and test an invariant showing no actually available receipt block is ever rejected during that first session.

    (d) Keep the wire message exactly as specified. The Execution API defines the message verbatim as Pruned history unavailable. prunedMessage() currently appends the floor to it, and there is a second variant for the not-persisted case, so clients matching the documented string will miss both. Suggest keeping message exact and moving the floor into data - that is what data is for, and a client deciding whether to fall back to an archive node needs the boundary value, so a log line does not serve it. JsonRpcExecutionRevertedException already carries a data constructor; a symmetric one on JsonRpcPrunedHistoryException would cover this.

    2. Strict mode

    (a) The servlet hooks collide with a change I have in flight.

    I have an unmerged change that hardens the JSON-RPC error path: it replaces jsonrpc4j's -32001 unmapped fallback with -32603, lets VirtualMachineError / ThreadDeath / LinkageError escape rather than be dressed up as a normal error response, and for that last part moves the single-request path from handle(req, resp) to handleRequest(InputStream, OutputStream), because JsonRpcServer.handle catches Throwable. It also rejects, before dispatch, any request id that is not a String, Number or Null.

    That lands on the same two lines yours does:

    • in doPost, immediately after the empty-batch check, where you insert violatesStrictCompliance(rootNode);
    • in handleBatch, on the if (!subRequest.isObject()) line, which you extend with violatesStrictCompliance(subRequest) and I extend with an id-validity check.

    Two differences worth settling now rather than at merge time:

    • My id check is unconditional; yours is inside the flag. If both land as-is, the id row of violatesStrictCompliance becomes unreachable, and we carry two copies of the same predicate (mine in an isValidRequestId helper, yours inlined twice).
    • I moved the malformed-sub-request check ahead of the if (overflow) branch, so that once the response budget is exhausted a malformed sub-request is still reported as -32600 rather than -32003. Yours stays after it. Mine is the better order, but either way we should pick one.

    Two smaller side effects of the transport switch, since they touch files you may rebase over: CachedBodyRequestWrapper and the HttpStatusCodeProvider become unreachable and are removed.

    (b) Which rows actually need the flag.

    This is the part I would most like your view on. The six rows are not homogeneous in compatibility risk. Grouped by what a client sees today with the flag off:

    row behavior today client contract to preserve?
    missing / wrong jsonrpc request served normally yes, real
    params: null served for zero-arg methods; -32602 via arity for methods with parameters yes, real
    non-string method -32601 error either way, low risk
    scalar params no response no
    array / object id no response no
    id: null no response (treated as a notification) no, but see (c)
    For the two rows in the middle group with no response at all - scalar params and non-scalar id - there is nothing for a client to depend on, and returning nothing to a request that carries an id is itself the 2.0 deviation. Gating those behind a default-off flag preserves the deviation without buying any compatibility, which is why my change makes the id one unconditional. My suggestion is to treat those two the same way and keep the flag for the rows that change a response clients actually receive today.

    Two geth data points that may help with the top row, since they are not in the table: isCall() requires hasValidVersion(), so geth already rejects a missing or wrong jsonrpc field - making that row strict moves toward geth rather than away from it. Conversely hasValidID() only excludes { and [, so geth accepts a boolean id; both our implementations reject it. That is correct per §4 (id MUST be String, Number or Null) but it is a divergence from geth and belongs in the release note.

    (c) id: null is the one row where strict mode is stricter than the spec and than geth.

    Your table already marks it permitted-but-discouraged and the code comment registers the deviation, so this is confirmation rather than a new objection. Adding the geth behavior: ID is a json.RawMessage, so an explicit null is four bytes rather than nil. isNotification() requires msg.ID == nil and therefore does not match, while hasValidID() only rejects a leading { or [. So geth dispatches it as an ordinary call and echoes "id": null in the response. rpc/json.go L56-L73

    So under the flag we would deviate from both the specification and geth on this row, which I think argues for two things: keep this row behind the flag even if the other two silent rows go unconditional, and record it in the release note rather than only in a code comment.

    I looked at whether answering normally is reachable instead. It is, but not cheaply: jsonrpc4j decides notification-ness internally, so the servlet would have to rewrite the id to a sentinel before dispatch and restore null on the way out. I do not think that is worth doing in this PR - I mention it only so the choice is on the record as a constraint of the framework rather than a preference.

    (d) scalar params in a batch. Under the flag your per-sub-request check already does the right thing: -32600 for the offending sub-request, valid siblings still executed. The gap is the flag-off path, which is pre-existing on develop rather than anything you introduced: handleBatch catches RuntimeException and returns, replacing the whole batch with a single error object. In our testing scalar params is one of the shapes that produces that exception, so today one malformed sub-request discards the entire batch. That is an additional argument for making this row unconditional per (b).

    3. Scope boundary and the resolver

    I have been working on a related but distinct layer: null values inside the params array - "params": [null] reaching a method body after arity and dispatch (filter objects, call objects, filter ids, fullTransactionObjects, DTO fields) - plus error mapping and internal-detail sanitization. That never touches the request envelope. Proposed split:

    • request envelope (jsonrpc, method, params container type, id type) -> this issue, with the rollout policy decided per row: compatibility-sensitive legacy behavior stays behind strictComplianceMode, while malformed requests that currently receive no response can be corrected unconditionally;
    • values inside params, plus error mapping and sanitization -> a separate tracking issue I am preparing.

    I will cross-link both ways when I open it so the boundary is visible from either side.

    On the resolver specifically: your branch does not modify the production JsonRpcErrorResolver, only the annotation mappings on TronJsonRpc plus the resolver tests, while my change rewrites the resolver but preserves declaration-order matching. So the runtime semantics stay compatible and only the annotation and test changes need care during rebasing.

    One durable improvement while you are there. Both JsonRpcExecutionRevertedException and JsonRpcPrunedHistoryException extend JsonRpcInternalException, which is what creates the ordering dependency I raised back in April. The order is correct on the branch today - I checked estimateGas, getCall, getBlockReceipts and getLogs, and both are declared ahead of the generic JsonRpcInternalException mapping - so this is not a live bug. But the dependency is invisible at the call site, and a future reorder or a third subclass would regress it silently. Since both carry wire codes distinct from -32000, extending JsonRpcException directly would remove the dependency entirely, and it would match every other exception in org.tron.core.exception.jsonrpc, all of which extend the base class rather than each other.

    @waynercheung Thanks for the detailed review, and for checking the branch itself.

    1. Receipt floor

    I agree with (a), (c) and (d). One thing to confirm on (a).
    When transHistory.switch = off, receipt and log queries that need persisted receipt data return 4444 on every node type, whatever is left in the store; When the switch is on, the receipt-floor check applies only to LiteNodes, not to FullNodes. The reason: on a FullNode the first key of the ret store is not where receipts start. On a database that predates June 2019, receipts from before then are still in TransactionHistoryStore (the ret store replaced it in eefe9c1b7), and getTransactionInfoById / getTransactionInfoByBlockNum fall back to it. A floor check based on the first key would return 4444 for blocks the node can actually serve. On a FullNode, receipt completeness is up to the operator. Gaps caused by toggling the switch or an incomplete migration are not treated as pruned history; behavior stays as on develop: getBlockReceipts returns -32000 size mismatch, getLogs returns no error.

    On (b), the definition of earliest. The Execution API has one shared definition, "the lowest numbered block the client has available"; I read that as a node-level boundary rather than an endpoint-specific one. If it is resolved per endpoint, eth_getBlockByNumber("earliest") returns a block for which eth_getBlockReceipts then returns 4444, which is confusing. With a single receipt floor, a client walking from earliest works on the first try. And as discussed earlier in this thread, the real fix is for the Toolkit to keep the window's receipts, so bodies and receipts share one boundary like geth.
    So my preference is to use a shared earliest boundary for these history methods on LiteNodes. On a FullNode it is 0 for every endpoint.

    2. Strict mode

    Before the row-by-row handling, two questions I would like to settle first, because the rows depend on them.

    Is the config item needed at all? Your table and 2(a)-(d), plus running the same shapes against a local geth --dev (v1.17.5), reduce the question to a single shape: only a missing or wrong jsonrpc turns a request that succeeds today into an error. Every other shape either already fails, gets no response, or - for params: null - behaves exactly as geth does and stays as is. This jsonrpc interface exists for mainstream Ethereum tooling, and that tooling always sends "jsonrpc": "2.0", so strictComplianceMode would in practice only exist for a hand-written TRON-only client that may or may not exist. I lean to dropping the flag, handling every shape unconditionally, and carrying the break in the release note, with an error message that says jsonrpc must be "2.0". This reverses what I told @317787106 in April.

    With the flag gone, should the id be validated in one place? Yes. Your isValidRequestId should be the single id check, and I rebase onto it rather than carry a second copy. The one thing I would add to it is the explicit null id: spec-valid, but jsonrpc4j treats it as a notification and never answers, so the helper should reject it too; a missing id stays a notification. If you would rather keep the helper spec-only, I will add that right after it - either way, one place. And the malformed sub-request check moves ahead of if (overflow), as you have it.

    If we agree on both, I will follow up with the per-row handling - codes, batch behavior, release-note items. Given how much the request validation overlaps with your error-path change, I would also split it out of the current PR into its own, built on top of yours; could you share the PR or branch for that change, so the landing order is settled once?

    3. Scope and resolver

    The split between the request envelope and values inside params is the right boundary. Please cross-link once the tracking issue is up; I will reference it from the PR.

    On the hierarchy: agreed. In this PR both exceptions will extend JsonRpcException directly, which removes the declaration-order dependency; subclassing JsonRpcInternalException was only a shortcut around throws clauses, and the change is throws declarations with no other effect.

  18. waynercheung commented on Sep 1, 2026

    @waynercheung
    Collaborator

    @0xbigapple Thanks - this settles most of it. Taking the two questions you wanted decided first, then scope, then the receipt floor.

    Dropping the flag

    Agreed, and your reduction is the right one: after 2(a)-(d) the only shape that turns a currently-succeeding request into an error is a missing or wrong jsonrpc, and mainstream tooling always sends it. Handling every shape unconditionally with jsonrpc must be "2.0" in the message, and carrying the break in the release note, is simpler than a switch almost nobody would flip.

    One id check

    Agreed, and the malformed sub-request check moving ahead of if (overflow) is settled.

    On the explicit null id I would rather take the second option you offered: keep isValidRequestId spec-only and add the rejection right after it. The reason is not a preference about the outcome but about where the deviation is recorded. That helper is a literal reading of section 4 - id MUST be String, Number or Null - and nothing else, so anyone can check it against one paragraph. Note the direction is not uniform in this area: we reject a boolean id, which geth accepts, and we would now also reject null, which both the spec permits and geth answers normally. That makes id: null the only row that deviates from the specification and from geth at once. With the flag gone it also becomes unconditional, so it deserves its own release-note line rather than being folded into a general "stricter validation" note.

    I do not disagree that answering -32600 beats answering nothing - today's silence is itself the section 4 violation, so both options are improvements. I only want the two decisions separable, so that if we later work around jsonrpc4j and answer normally with "id": null, exactly one place changes.

    params container type

    One adjustment to the envelope split I proposed, on the scalar params row. My change now validates the params container type before dispatch, on the same lines as the id check: a non-null scalar params (string, number or boolean) gets -32600 "Invalid Request", the id is echoed when valid and null otherwise, and in a batch only that element is affected while its siblings still run. Two reasons for taking it there rather than leaving it to your PR:

    • it is the same predicate, at the same two insertion points, and it is a "no response today" row per your 2(b), so it goes unconditional either way;
    • the exception that aborts a batch today (your 2(d): handleBatch catching the IllegalArgumentException and replacing the whole batch with one error object) also surfaces on the single-request path once it goes through handleRequest. Without the container check the switch would turn today's empty body into a -32603 with the id echoed, the wrong code for a malformed request, so the check has to land in the same change.

    params: null stays as it is (jsonrpc4j treats it as absent), and jsonrpc, method and the explicit id: null policy stay with this issue. For the release note: geth classifies the same shape as -32602 at argument parsing and does not answer when the request has no id; we classify it at the envelope as -32600 and answer with id: null, per the section 4.2 example.

    Scope and landing order

    Rather than one tracking issue there will be two, one per PR, so each can close on its own:

    • [Feature] Standardize JSON-RPC error mapping and exception boundaries #6941 - the framework layer: -32603 for unmapped exceptions, the fatal-error boundary, the handle -> handleRequest switch, and the pre-dispatch id and params container checks above. This one is up.
    • the second covers null values inside params reaching method bodies: the 10 parameter positions and 7 DTO fields, method-level only, no envelope changes. It goes up next and I will link it here.

    The PR for #6941 is based on develop and follows the issue; I will link it here as soon as it is open. Stacking your request-validation PR on top of it is the order I would pick as well, for the reason you give: it can build on isValidRequestId and the container check instead of carrying a second copy. Neither of the two PRs depends on the other, and the null-parameter one only overlaps this issue in JsonRpcApiUtil / TronJsonRpcImpl.

    earliest

    You are right and I withdraw the endpoint-specific proposal. The Execution API defines the tag once, in the shared Block tag schema that every method references: "earliest: The lowest numbered block the client has available". That is a client-level boundary, not a per-method one, so resolving it differently per endpoint would be reading a distinction into the schema that is not there. A single receipt floor on LiteNodes, and 0 on FullNodes, is the right call, and your point about a client walking up from earliest succeeding on the first try is the practical argument for it. Agreed too that the durable fix is the Toolkit keeping the window's receipts so bodies and receipts share one boundary.

    Receipt floor on FullNodes

    Thanks for the correction - I had not accounted for TransactionHistoryStore. If receipts from before the ret store replaced it in eefe9c1b7 are still served through the fallback in getTransactionInfoById / getTransactionInfoByBlockNum, then a floor derived from the ret store's first key would return 4444 for blocks a FullNode can actually serve, which is worse than the gap I was pointing at. Gating the floor check on LiteNodes when the switch is on is right, and answering 4444 unconditionally on every node type when the switch is off covers the case I was actually worried about.

    Exception hierarchy

    Thanks for taking it - extending JsonRpcException directly removes the ordering dependency entirely.

  19. waynercheung commented on Sep 21, 2026

    @waynercheung
    Collaborator

    @0xbigapple Following up on the two links I owed you here.

    The second issue is #6951, covering null-parameter handling and eth_uninstallFilter lookup-miss normalization; its PR is #6984. The previously identified overlap with this issue is in JsonRpcApiUtil and TronJsonRpcImpl, and neither side depends on the other.

    The PR for #6941 is #6985. One correction to what I wrote on 1 September: both PRs are based on release_v4.8.3 rather than develop, since that is where the 4.8.3 work is landing, including the dependency upgrade in #6950 and the HTTP error sanitization in #6954. release_v4.8.3 currently contains everything on develop, so this does not change the stacking order we discussed.

    For your request-validation change, the pieces in #6985 that it can build on are all in JsonRpcServlet: isValidRequestId is the literal section 4 check, hasScalarParams is the container check, and both are applied at the same two insertion points, the single-request path and each batch element, with buildErrorNode producing the -32600 envelope and the id: null fallback. The jsonrpc and method members, params: null, and the explicit id: null policy are untouched there, as agreed.

    One item to confirm on this issue. In the #6941 discussion, @lxcmyf and @halibobo1205 both preferred not to add a narrow notification classifier there, and proposed that notification normalization be handled together with the envelope validation here. #6985 preserves the existing notification response-suppression rules; its error-mapping and recovery changes still apply. Characterization tests cover forwarded jsonrpc4j errors for unknown methods, arity mismatches and unmapped exceptions without an id. In the single-request servlet catch, a missing id stays silent, while explicit id: null gets an error with id: null. A recoverable batch failure without an id also produces an error with id: null. Any normalization would need to cover the forwarded responses and the servlet-synthesized errors together. Does that belong in this issue's scope from your side? It is not a prerequisite for either PR.

  20. removed this from the GreatVoyage-v4.8.3 milestone on Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    • Status
      No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions