feat(jsonrpc): return code 3 for revert and 4444 for pruned history - #22
0xbigapple wants to merge 1 commit into
Conversation
- eth_call / eth_estimateGas: contract revert changes from -32000 to 3 - data a node does not have (pruned on a LiteNode, or receipts with transHistory.switch off) returns 4444 "Pruned history unavailable"
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 23 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/utils/ResultCodeUtil.java">
<violation number="1" location="framework/src/main/java/org/tron/core/utils/ResultCodeUtil.java:12">
P3: This method duplicates the full exception-to-`contractResult` mapping in `RuntimeImpl.setResultCode`, so future TVM result additions can update one path while constant calls silently return `UNKNOWN`. Keep success/revert handling local, but share the exception classifier between both callers.</violation>
</file>
<file name="common/src/main/java/org/tron/core/exception/jsonrpc/JsonRpcPrunedHistoryException.java">
<violation number="1" location="common/src/main/java/org/tron/core/exception/jsonrpc/JsonRpcPrunedHistoryException.java:5">
P3: This javadoc says code 4444 is standardized by the Ethereum Execution API (EIP-4444), but it is a TRON-specific code — EIP-4444 specifies historical data expiry, not JSON-RPC error codes, and the PR description itself notes the 4444 extension. Reword to avoid claiming Ethereum standardization, e.g. 'returns JSON-RPC error code 4444 for pruned history (EIP-4444-inspired TRON extension)'.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/store/TransactionRetStore.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/store/TransactionRetStore.java:51">
P1: This limits the scan before filtering key shape, so an earlier non-block key can hide all valid receipt keys and make startup classify existing history as pruned. Scan past non-8-byte keys, or enforce and validate a block-number-only key invariant before applying the limit.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * layers, {@code getNext} does not merge deletions correctly. | ||
| */ | ||
| public OptionalLong getLowestBlockNum() { | ||
| Map<byte[], byte[]> entries = revokingDB.getNext(ByteArray.fromLong(0), 1); |
There was a problem hiding this comment.
P1: This limits the scan before filtering key shape, so an earlier non-block key can hide all valid receipt keys and make startup classify existing history as pruned. Scan past non-8-byte keys, or enforce and validate a block-number-only key invariant before applying the limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At chainbase/src/main/java/org/tron/core/store/TransactionRetStore.java, line 51:
<comment>This limits the scan before filtering key shape, so an earlier non-block key can hide all valid receipt keys and make startup classify existing history as pruned. Scan past non-8-byte keys, or enforce and validate a block-number-only key invariant before applying the limit.</comment>
<file context>
@@ -37,6 +40,23 @@ public void put(byte[] key, TransactionRetCapsule item) {
+ * layers, {@code getNext} does not merge deletions correctly.
+ */
+ public OptionalLong getLowestBlockNum() {
+ Map<byte[], byte[]> entries = revokingDB.getNext(ByteArray.fromLong(0), 1);
+ for (byte[] key : entries.keySet()) {
+ if (key.length == Long.BYTES) {
</file context>
| */ | ||
| public class ResultCodeUtil { | ||
|
|
||
| public static contractResult resolve(RuntimeException exception) { |
There was a problem hiding this comment.
P3: This method duplicates the full exception-to-contractResult mapping in RuntimeImpl.setResultCode, so future TVM result additions can update one path while constant calls silently return UNKNOWN. Keep success/revert handling local, but share the exception classifier between both callers.
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/utils/ResultCodeUtil.java, line 12:
<comment>This method duplicates the full exception-to-`contractResult` mapping in `RuntimeImpl.setResultCode`, so future TVM result additions can update one path while constant calls silently return `UNKNOWN`. Keep success/revert handling local, but share the exception classifier between both callers.</comment>
<file context>
@@ -0,0 +1,48 @@
+ */
+public class ResultCodeUtil {
+
+ public static contractResult resolve(RuntimeException exception) {
+ if (exception instanceof Program.IllegalOperationException) {
+ return contractResult.ILLEGAL_OPERATION;
</file context>
|
|
||
| /** | ||
| * Thrown when a request targets historical state that a LiteNode has pruned. | ||
| * Maps to JSON-RPC error code 4444 "Pruned history unavailable", as standardized |
There was a problem hiding this comment.
P3: This javadoc says code 4444 is standardized by the Ethereum Execution API (EIP-4444), but it is a TRON-specific code — EIP-4444 specifies historical data expiry, not JSON-RPC error codes, and the PR description itself notes the 4444 extension. Reword to avoid claiming Ethereum standardization, e.g. 'returns JSON-RPC error code 4444 for pruned history (EIP-4444-inspired TRON extension)'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At common/src/main/java/org/tron/core/exception/jsonrpc/JsonRpcPrunedHistoryException.java, line 5:
<comment>This javadoc says code 4444 is standardized by the Ethereum Execution API (EIP-4444), but it is a TRON-specific code — EIP-4444 specifies historical data expiry, not JSON-RPC error codes, and the PR description itself notes the 4444 extension. Reword to avoid claiming Ethereum standardization, e.g. 'returns JSON-RPC error code 4444 for pruned history (EIP-4444-inspired TRON extension)'.</comment>
<file context>
@@ -0,0 +1,17 @@
+
+/**
+ * Thrown when a request targets historical state that a LiteNode has pruned.
+ * Maps to JSON-RPC error code 4444 "Pruned history unavailable", as standardized
+ * by the Ethereum Execution API (EIP-4444).
+ */
</file context>
| * Maps to JSON-RPC error code 4444 "Pruned history unavailable", as standardized | |
| * Maps to JSON-RPC error code 4444 "Pruned history unavailable", a TRON extension | |
| * inspired by EIP-4444 historical data expiry. |
1 participant
First of two PRs for tronprotocol#6676; request validation is split out (see Follow up).
What does this PR do?
Aligns two JSON-RPC error behaviors with the Ethereum Execution API:
Contract revert returns code
3.eth_callandeth_estimateGasreturncode: 3instead of-32000; other execution failures stay-32000.Wallet.callConstantContractnow recordscontractReton the result, so the JSON-RPC layer classifies on the enum rather than on the error message.Pruned history returns
4444 "Pruned history unavailable".eth_getBlockByNumber,eth_getBlockTransactionCountByNumber,eth_getTransactionByBlockNumberAndIndex): on a LiteNode, heights below the lowest stored block. Genesis stays queryable.eth_getBlockReceipts,eth_getLogs,eth_newFilter), including when the block is selected by hash: on any node whentransHistory.switch = off; otherwise on a LiteNode, heights below the receipt floor (the first block intransactionRetStore, probed at startup). A FullNode has no floor check — receipts from before June 2019 may still be served fromTransactionHistoryStore, so the store's first key is not a floor — and receipt gaps on a FullNode keep the current behavior.datacarries the earliest available block as hex, the same valueearliestresolves to. With receipts not persisted there is no floor, anddatais the usual"{}"placeholder.earliestresolves to the lowest block with all persisted data: on a LiteNode the receipt floor (the body floor when receipts are not persisted), on a FullNode 0.Why are these changes required?
-32000, a contract revert cannot be told apart from out-of-energy or any other TVM failure.nullfor pruned blocks, which reads as "block does not exist", andeth_getLogsacross the cutoff returns incomplete results without an error.Breaking changes (release-note items):
eth_call/eth_estimateGas: a contract revert returns3instead of-32000.4444instead ofnullon block-body methods, and instead of-32000or incomplete results on receipt and log methods.transHistory.switch = off: receipt and log queries return4444.earlieston a LiteNode resolves to the pruning floor instead of 0.This PR has been tested by:
release_v4.8.3.Follow up
jsonrpcversion, non-stringmethod, explicitid: null) in a new PR.Extra details
datais a TRON extension; the Execution API, geth and Besu send onlycodeandmessage.triggerConstantContractresponses now carrycontractRet.eth_getLogsfrom0x0to a later block on a LiteNode returns4444, as geth does.null. Wheneth_getBlockReceiptsoreth_getLogsdoes find the block by hash, its number is known and the receipt rules above apply.eth_getBlockByHash,eth_getTransactionByHashandeth_getTransactionReceiptare unchanged.Summary by cubic
Aligns JSON-RPC error responses with the Ethereum Execution API by distinguishing contract reverts from other failures and by surfacing pruned history instead of returning empty results.
eth_callandeth_estimateGasreturn code3with the revert data for a contract revert; instead of-32000for all failures.eth0for non-revert failures remains on the same as before.4004and{height}so are handles with more data.Migrating
3; clients may continue to treat revert as-1.transactionetc.We'll produce a brief description with articles.
However, the expected output should be concise and not too long. Let's probably use 2 sections.
Given the PR is large, we have 2 sections permitted. We'll have "behaviour" and "breaking changes".
Thus, I'll produce:
We'll do that.
Let's ensure we mention all the changes.
We'll include the endpoints.
We'll also mention the
eth0.Now, craft final output.
We'll note "The
datafield now carries the earliest available block for outright but also important".Let's write.
But writing includes a bit more details.
Let's write a draft.
The PR description should be about 2-3 paragraphs. We'll include bulletsI'll present the PRD now.
No changes the requirement.
Written for commit c164330. Summary will update on new commits.