feat(jsonrpc): standardize JSON-RPC error handling - #18
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR adds retained block and receipt cutoffs, pruned-history JSON-RPC errors, execution-revert handling, and optional strict JSON-RPC request validation. It also adds configuration wiring and tests for these behaviors. ChangesJSON-RPC history, execution, and compliance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant JsonRpcServlet
participant TronJsonRpcImpl
participant JsonRpcApiUtil
Client->>JsonRpcServlet: send JSON-RPC request
JsonRpcServlet->>JsonRpcServlet: validate request in strict mode
JsonRpcServlet->>TronJsonRpcImpl: dispatch valid request
TronJsonRpcImpl->>JsonRpcApiUtil: validate retained history
JsonRpcApiUtil-->>TronJsonRpcImpl: allow request or return pruning error
TronJsonRpcImpl-->>JsonRpcServlet: return result or mapped exception
JsonRpcServlet-->>Client: send JSON-RPC response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
1 issue found across 24 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java">
<violation number="1" location="framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java:305">
P3: Strict compliance mode rejects an explicit JSON `null` `id` with -32600, but JSON-RPC 2.0 §4.3 explicitly permits an id of String, Number, **or NULL**. Because a null id arrives as a Jackson NullNode, the final `id != null && !id.isTextual() && !id.isNumber()` check flags it as non-compliant even though it is spec-valid. This is inconsistent with the feature's stated 'align with the JSON-RPC 2.0 spec' goal and would reject spec-conforming clients that send `"id":null`. The anti-hang motivation for structured ids (array/object) is reasonable; consider distinguishing null ids (spec-valid, deterministic) from structured ids (undeterminable) rather than treating both identically, e.g. by rejecting only array/object ids, or by ensuring a -32600/null-id reply is written for null ids without claiming they are malformed.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // becomes a notification, an array or an object makes its parseId throw — which leaves | ||
| // the client waiting forever, so a -32600 beats a silent hang. | ||
| JsonNode id = node.get("id"); | ||
| return id != null && !id.isTextual() && !id.isNumber(); |
There was a problem hiding this comment.
P3: Strict compliance mode rejects an explicit JSON null id with -32600, but JSON-RPC 2.0 §4.3 explicitly permits an id of String, Number, or NULL. Because a null id arrives as a Jackson NullNode, the final id != null && !id.isTextual() && !id.isNumber() check flags it as non-compliant even though it is spec-valid. This is inconsistent with the feature's stated 'align with the JSON-RPC 2.0 spec' goal and would reject spec-conforming clients that send "id":null. The anti-hang motivation for structured ids (array/object) is reasonable; consider distinguishing null ids (spec-valid, deterministic) from structured ids (undeterminable) rather than treating both identically, e.g. by rejecting only array/object ids, or by ensuring a -32600/null-id reply is written for null ids without claiming they are malformed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java, line 305:
<comment>Strict compliance mode rejects an explicit JSON `null` `id` with -32600, but JSON-RPC 2.0 §4.3 explicitly permits an id of String, Number, **or NULL**. Because a null id arrives as a Jackson NullNode, the final `id != null && !id.isTextual() && !id.isNumber()` check flags it as non-compliant even though it is spec-valid. This is inconsistent with the feature's stated 'align with the JSON-RPC 2.0 spec' goal and would reject spec-conforming clients that send `"id":null`. The anti-hang motivation for structured ids (array/object) is reasonable; consider distinguishing null ids (spec-valid, deterministic) from structured ids (undeterminable) rather than treating both identically, e.g. by rejecting only array/object ids, or by ensuring a -32600/null-id reply is written for null ids without claiming they are malformed.</comment>
<file context>
@@ -265,6 +273,38 @@ private void handleBatch(HttpServletResponse resp, JsonNode rootNode, int maxRes
+ // becomes a notification, an array or an object makes its parseId throw — which leaves
+ // the client waiting forever, so a -32600 beats a silent hang.
+ JsonNode id = node.get("id");
+ return id != null && !id.isTextual() && !id.isNumber();
+ }
+
</file context>
97a7447 to
a831507
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java (1)
1450-1475: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftKeep future-only filters usable when receipt history is unavailable.
newFiltercreatesLogFilterWrapperwith the current head. The wrapper rejects this height when the receipt floor ishead + 1after LiteNode startup, orLong.MAX_VALUEwhen receipt persistence is disabled. A defaulteth_newFilterthen returns4444instead of subscribing to futureLogsFilterCapsuleevents.Apply receipt-history validation when
eth_getFilterLogsperforms a historical scan. Keep filter installation valid for future-only subscriptions. Add the corresponding4444interface mapping and regression tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java` around lines 1450 - 1475, Update newFilter and LogFilterWrapper creation so installing an eth_newFilter does not validate the current head against the receipt-history floor, allowing future-only subscriptions when history is unavailable. Move receipt-history validation to the historical scan performed by eth_getFilterLogs, preserving the 4444 error for requests that require unavailable receipts. Add the corresponding 4444 interface mapping and regression coverage for both future-only installation and historical rejection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.java`:
- Around line 644-647: Update the “earliest” resolution logic to always return
wallet.getLowestBlockNum(), removing the receiptFloor lookup and fallback so
LiteNode block-number methods use the retained block floor.
- Around line 729-734: Update the receipt-history validation around
wallet.getLowestReceiptBlockNum() so a Long.MAX_VALUE receiptFloor immediately
throws JsonRpcPrunedHistoryException with the non-persisted-history message,
including when blockNum equals the sentinel. Keep the existing floor comparison
and prunedMessage(receiptFloor) behavior for finite receipt floors.
In `@framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java`:
- Around line 300-303: Update the JSON-RPC request validation around the id
handling so an explicitly present null id is accepted alongside textual and
numeric ids, while a missing id remains a notification only when appropriate.
Preserve the null id through jsonrpc4j so it produces a normal response
containing "id": null, and add single-request and batch coverage for
explicit-null ids.
In `@framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java`:
- Around line 400-419: Update getBlockByNumOrTag in
framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java:400-419
so the earliest block tag resolves from wallet.getLowestBlockNum(), while
retaining receipt-floor checks only for receipt and log access; update
framework/src/test/java/org/tron/core/jsonrpc/JsonRpcPrunedHistoryTest.java:86-90
to expect LOWEST_BLOCK_NUM instead of RECEIPT_FLOOR_BLOCK_NUM.
In `@framework/src/main/java/org/tron/core/utils/ResultCodeUtil.java`:
- Line 46: Update ResultCodeUtil.resolve to recognize
Program.StaticCallModificationException and return the same result code used by
the consensus classifier before the existing contractResult.UNKNOWN fallback;
preserve the fallback for all other unrecognized exceptions.
In `@framework/src/test/java/org/tron/core/db/TransactionRetStoreTest.java`:
- Around line 69-75: Ensure the test cleanup for keys 3 and 7 in
TransactionRetStoreTest runs even when either assertion fails by moving the
deletion calls into a finally-protected cleanup block. Preserve the existing
assertions and delete all inserted test keys, including blockNum, regardless of
test outcome.
In
`@framework/src/test/java/org/tron/core/jsonrpc/JsonRpcCallAndEstimateGasTest.java`:
- Around line 78-81: Update the assertions in the panic and short-data eth_call
tests, plus the corresponding eth_estimateGas tests, to expect
JsonRpcExecutionRevertedException instead of the broader
JsonRpcInternalException. Keep the existing message and raw-data assertions
unchanged so every contractResult.REVERT path verifies the revert-specific
exception.
---
Outside diff comments:
In `@framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java`:
- Around line 1450-1475: Update newFilter and LogFilterWrapper creation so
installing an eth_newFilter does not validate the current head against the
receipt-history floor, allowing future-only subscriptions when history is
unavailable. Move receipt-history validation to the historical scan performed by
eth_getFilterLogs, preserving the 4444 error for requests that require
unavailable receipts. Add the corresponding 4444 interface mapping and
regression coverage for both future-only installation and historical rejection.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d4aca8f-7a46-4b9b-a42e-240123a4e7a1
📒 Files selected for processing (29)
chainbase/src/main/java/org/tron/core/ChainBaseManager.javachainbase/src/main/java/org/tron/core/capsule/TransactionResultCapsule.javachainbase/src/main/java/org/tron/core/store/TransactionRetStore.javacommon/src/main/java/org/tron/common/parameter/CommonParameter.javacommon/src/main/java/org/tron/core/config/args/NodeConfig.javacommon/src/main/java/org/tron/core/exception/jsonrpc/JsonRpcExecutionRevertedException.javacommon/src/main/java/org/tron/core/exception/jsonrpc/JsonRpcPrunedHistoryException.javacommon/src/main/resources/reference.confframework/src/main/java/org/tron/core/Wallet.javaframework/src/main/java/org/tron/core/config/args/Args.javaframework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.javaframework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.javaframework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpc.javaframework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.javaframework/src/main/java/org/tron/core/services/jsonrpc/filters/LogFilterAndResult.javaframework/src/main/java/org/tron/core/services/jsonrpc/filters/LogFilterWrapper.javaframework/src/main/java/org/tron/core/utils/ResultCodeUtil.javaframework/src/main/resources/config.confframework/src/test/java/org/tron/core/db/TransactionRetStoreTest.javaframework/src/test/java/org/tron/core/jsonrpc/HandleLogsFilterTest.javaframework/src/test/java/org/tron/core/jsonrpc/JsonRpcCallAndEstimateGasTest.javaframework/src/test/java/org/tron/core/jsonrpc/JsonRpcPrunedHistoryTest.javaframework/src/test/java/org/tron/core/jsonrpc/JsonRpcTest.javaframework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.javaframework/src/test/java/org/tron/core/jsonrpc/LogMatchOverLimitTest.javaframework/src/test/java/org/tron/core/jsonrpc/SectionBloomStoreTest.javaframework/src/test/java/org/tron/core/services/jsonrpc/JsonRpcErrorResolverTest.javaframework/src/test/java/org/tron/core/services/jsonrpc/JsonRpcServletTest.javaframework/src/test/java/org/tron/core/utils/ResultCodeUtilTest.java
| // "earliest" anchors to the receipt floor (first block with complete data); with | ||
| // receipt persistence off no such block exists — fall back to the body floor | ||
| long receiptFloor = wallet.getLowestReceiptBlockNum(); | ||
| return receiptFloor == Long.MAX_VALUE ? wallet.getLowestBlockNum() : receiptFloor; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve "earliest" from the block floor.
For a LiteNode, this code returns getLowestReceiptBlockNum(). The PR contract requires "earliest" to resolve to getLowestBlockNum(). When receipt retention starts later than block retention, block-number methods skip retained blocks.
Proposed fix
- // "earliest" anchors to the receipt floor (first block with complete data); with
- // receipt persistence off no such block exists — fall back to the body floor
- long receiptFloor = wallet.getLowestReceiptBlockNum();
- return receiptFloor == Long.MAX_VALUE ? wallet.getLowestBlockNum() : receiptFloor;
+ return wallet.getLowestBlockNum();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // "earliest" anchors to the receipt floor (first block with complete data); with | |
| // receipt persistence off no such block exists — fall back to the body floor | |
| long receiptFloor = wallet.getLowestReceiptBlockNum(); | |
| return receiptFloor == Long.MAX_VALUE ? wallet.getLowestBlockNum() : receiptFloor; | |
| return wallet.getLowestBlockNum(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.java`
around lines 644 - 647, Update the “earliest” resolution logic to always return
wallet.getLowestBlockNum(), removing the receiptFloor lookup and fallback so
LiteNode block-number methods use the retained block floor.
| long receiptFloor = wallet.getLowestReceiptBlockNum(); | ||
| if (wallet.isLiteNode() && blockNum < receiptFloor) { | ||
| throw new JsonRpcPrunedHistoryException(receiptFloor == Long.MAX_VALUE | ||
| ? PRUNED_HISTORY_ERROR + ": transaction history is not persisted on this node" | ||
| : prunedMessage(receiptFloor)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle disabled receipt persistence before comparing the floor.
When receipt persistence is disabled, receiptFloor is Long.MAX_VALUE. A request for 0x7fffffffffffffff passes the < receiptFloor check and does not return error 4444. Reject the sentinel state before the height comparison.
Proposed fix
long receiptFloor = wallet.getLowestReceiptBlockNum();
- if (wallet.isLiteNode() && blockNum < receiptFloor) {
- throw new JsonRpcPrunedHistoryException(receiptFloor == Long.MAX_VALUE
- ? PRUNED_HISTORY_ERROR + ": transaction history is not persisted on this node"
- : prunedMessage(receiptFloor));
+ if (wallet.isLiteNode() && receiptFloor == Long.MAX_VALUE) {
+ throw new JsonRpcPrunedHistoryException(
+ PRUNED_HISTORY_ERROR + ": transaction history is not persisted on this node");
+ }
+ if (wallet.isLiteNode() && blockNum < receiptFloor) {
+ throw new JsonRpcPrunedHistoryException(prunedMessage(receiptFloor));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| long receiptFloor = wallet.getLowestReceiptBlockNum(); | |
| if (wallet.isLiteNode() && blockNum < receiptFloor) { | |
| throw new JsonRpcPrunedHistoryException(receiptFloor == Long.MAX_VALUE | |
| ? PRUNED_HISTORY_ERROR + ": transaction history is not persisted on this node" | |
| : prunedMessage(receiptFloor)); | |
| } | |
| long receiptFloor = wallet.getLowestReceiptBlockNum(); | |
| if (wallet.isLiteNode() && receiptFloor == Long.MAX_VALUE) { | |
| throw new JsonRpcPrunedHistoryException( | |
| PRUNED_HISTORY_ERROR + ": transaction history is not persisted on this node"); | |
| } | |
| if (wallet.isLiteNode() && blockNum < receiptFloor) { | |
| throw new JsonRpcPrunedHistoryException(prunedMessage(receiptFloor)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.java`
around lines 729 - 734, Update the receipt-history validation around
wallet.getLowestReceiptBlockNum() so a Long.MAX_VALUE receiptFloor immediately
throws JsonRpcPrunedHistoryException with the non-persisted-history message,
including when blockNum equals the sentinel. Keep the existing floor comparison
and prunedMessage(receiptFloor) behavior for finite receipt floors.
| // Any non-scalar id is rejected. Spec deviation: null is a valid id per JSON-RPC 2.0, | ||
| // but jsonrpc4j treats it as a notification and answers nothing, so -32600 beats silence. | ||
| JsonNode id = node.get("id"); | ||
| return id != null && !id.isTextual() && !id.isNumber(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Accept an explicit null JSON-RPC ID.
id: null is a valid JSON-RPC 2.0 request ID. Strict mode currently returns -32600 for this request.
Preserve the explicit-null request through jsonrpc4j without converting it to a notification. Return a normal response with "id": null. Add single-request and batch tests for this case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java`
around lines 300 - 303, Update the JSON-RPC request validation around the id
handling so an explicitly present null id is accepted alongside textual and
numeric ids, while a missing id remains a notification only when appropriate.
Preserve the null id through jsonrpc4j so it produces a normal response
containing "id": null, and add single-request and batch coverage for
explicit-null ids.
| private Block getBlockByNumOrTag(String blockNumOrTag) | ||
| throws JsonRpcInvalidParamsException, JsonRpcPrunedHistoryException { | ||
| long blockNum; | ||
| if (JsonRpcApiUtil.isBlockTag(blockNumOrTag)) { | ||
| if (LATEST_STR.equalsIgnoreCase(blockNumOrTag)) { | ||
| // Return the head block directly from blockStore, bypassing blockIndexStore | ||
| // which may not yet be written when latestBlockHeaderNumber is already updated. | ||
| return wallet.getNowBlock(); | ||
| } | ||
| return wallet.getBlockByNum(JsonRpcApiUtil.parseBlockTag(blockNumOrTag, wallet)); | ||
| blockNum = JsonRpcApiUtil.parseBlockTag(blockNumOrTag, wallet); | ||
| } else { | ||
| blockNum = parseBlockNumber(blockNumOrTag); | ||
| } | ||
| // Reject a pruned height before touching any store, so a LiteNode pays no lookup for | ||
| // history it cannot serve. Genesis is exempt: a snapshot copies block 0 explicitly, and | ||
| // lowestBlockNum is computed from block 1 upwards, so block 0 is always retained. | ||
| if (blockNum > 0) { | ||
| JsonRpcApiUtil.checkPrunedHistory(blockNum, wallet); | ||
| } | ||
| return wallet.getBlockByNum(parseBlockNumber(blockNumOrTag)); | ||
| return wallet.getBlockByNum(blockNum); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Resolve LiteNode earliest from the block floor.
The supplied JsonRpcApiUtil.parseBlockTag implementation resolves LiteNode earliest to getLowestReceiptBlockNum(). This makes block selector methods skip retained blocks between the body floor and receipt floor. It conflicts with the PR objective that earliest resolves to lowestBlockNum.
framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java#L400-L419: resolveearliestblock selectors fromwallet.getLowestBlockNum(). Keep receipt-floor checks limited to receipt and log access.framework/src/test/java/org/tron/core/jsonrpc/JsonRpcPrunedHistoryTest.java#L86-L90: expectLOWEST_BLOCK_NUM, notRECEIPT_FLOOR_BLOCK_NUM.
📍 Affects 2 files
framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java#L400-L419(this comment)framework/src/test/java/org/tron/core/jsonrpc/JsonRpcPrunedHistoryTest.java#L86-L90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java`
around lines 400 - 419, Update getBlockByNumOrTag in
framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java:400-419
so the earliest block tag resolves from wallet.getLowestBlockNum(), while
retaining receipt-floor checks only for receipt and log access; update
framework/src/test/java/org/tron/core/jsonrpc/JsonRpcPrunedHistoryTest.java:86-90
to expect LOWEST_BLOCK_NUM instead of RECEIPT_FLOOR_BLOCK_NUM.
| if (exception instanceof Program.InvalidCodeException) { | ||
| return contractResult.INVALID_CODE; | ||
| } | ||
| return contractResult.UNKNOWN; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Map Program.StaticCallModificationException before the fallback.
ResultCodeUtil.resolve sends this exception to contractResult.UNKNOWN. The new parity test includes this exception. This can fail testStaysInSyncWithConsensusClassifier and report the wrong constant-call result code.
Proposed fix
if (exception instanceof Program.InvalidCodeException) {
return contractResult.INVALID_CODE;
}
+ if (exception instanceof Program.StaticCallModificationException) {
+ return contractResult.STATIC_CALL_MODIFICATION;
+ }
return contractResult.UNKNOWN;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@framework/src/main/java/org/tron/core/utils/ResultCodeUtil.java` at line 46,
Update ResultCodeUtil.resolve to recognize
Program.StaticCallModificationException and return the same result code used by
the consensus classifier before the existing contractResult.UNKNOWN fallback;
preserve the fallback for all other unrecognized exceptions.
- probe the lowest receipted block from transactionRetStore at startup - eth_getLogs / eth_newFilter / eth_getBlockReceipts answer 4444 below it - resolve "earliest" to the receipt floor on a LiteNode
a831507 to
7a7c797
Compare
What does this PR do?
Standardizes three JSON-RPC behaviors to match the Ethereum Execution API:
Contract revert gets its own error code (
3). NewJsonRpcExecutionRevertedException,mapped to
code: 3with the raw revert payload indata.Wallet.callConstantContractnowrecords
contractRet = REVERTon the revert branch, so the JSON-RPC layer classifies on theenum rather than on an error-message string, and
TronJsonRpcImplrouteseth_callandeth_estimateGasthrough onerequireExecutionSuccessguard so the two endpoints cannotdrift apart.
Pruned history on a LiteNode answers
4444 "Pruned history unavailable", matching theEthereum Execution API, geth (PR #31361) and Besu (PR #9643). Covers the six number/tag based
methods:
eth_getBlockByNumber,eth_getBlockTransactionCountByNumber,eth_getTransactionByBlockNumberAndIndex,eth_getBlockReceipts,eth_getLogs,eth_newFilter.earliestnow resolves tolowestBlockNum, sofromBlock: "earliest"keeps working. The cutoff is checked before any store lookup, so a LiteNode pays no I/O for
history it cannot serve.
Optional strict JSON-RPC 2.0 request validation. New config
node.jsonrpc.strictComplianceMode, off by default. When enabled, the servlet rejects —before dispatch — any request object malformed per spec §4:
jsonrpcmissing or not"2.0"-32600methodmissing or not a string-32601 method not found-32600paramspresent but not an array/object-32600idpresent but not a string/number-32600Batches are validated per member: a bad member gets its own
-32600while the compliantmembers still execute.
Why are these changes required?
Each divergence costs dapp / SDK / tooling authors real debugging time:
eth_call/eth_estimateGascollapse every TVM failure into-32000, so a contractrevert is indistinguishable from out-of-energy or a bad jump — clients cannot tell "your
input was rejected by the contract" from "the node failed to execute".
nullfor blocks it has pruned, which clients cannot tell apartfrom "this block does not exist"; worse, an
eth_getLogsrange crossing the pruning cutoffquietly returns incomplete results.
idor a non-structuredparams, sothose callers wait forever today; a missing
methodis blamed as-32601 "no such method"when the real problem is a malformed request object. All reproduced on a live node before and
after the change. Default-off means zero behaviour change for existing clients; operators can
opt in for stricter client diagnostics.
This PR has been tested by:
coverage for revert classification, the pruned-history guard (including the genesis and
future-block edges), and strict-mode validation in both switch states.
3withpayload;
4444on pruned heights while genesis and the cutoff block stay queryable;earliestresolving tolowestBlockNum; a future height answeringnullrather than4444; the strict-mode matrix with the switch off and on.Follow up
Extra details
Summary by cubic
Standardizes JSON-RPC error handling to match the Ethereum Execution API. Adds code 3 for contract reverts, code 4444 for pruned history with receipt-aware floors on LiteNode, and an optional strict JSON-RPC 2.0 validator.
New Features
eth_callandeth_estimateGas. Other execution failures remain-32000and include return data when present.eth_getBlockByNumber,eth_getBlockTransactionCountByNumber,eth_getTransactionByBlockNumberAndIndex,eth_getBlockReceipts,eth_getLogs,eth_newFilter. On LiteNode"earliest"resolves to the first block with receipts (falls back to the body floor when receipts are not persisted). Pruning checks run before store lookups.node.jsonrpc.strictComplianceMode(default off): enforces"jsonrpc":"2.0", stringmethod, array/object (non-null)paramswhen present, and scalarid; batches validate per member and reject invalid members instead of hanging on badid/params.Migration
node.jsonrpc.strictComplianceMode = true. Clients must send spec-compliant JSON-RPC 2.0 requests when strict mode is on.Written for commit 7a7c797. Summary will update on new commits.
Summary by CodeRabbit
New Features
earliesthistory boundaries and node APIs for available history ranges.Bug Fixes