Repository navigation
feat(jsonrpc): return code 3 for revert and 4444 for pruned history - #22
0xbigapple wants to merge 12 commits 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 configuration
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.
2 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="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>
|
|
||
| /** | ||
| * 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. |
- 4444 no longer carries the earliest available block in data, as geth - remove ResultCodeUtil; constant calls record only REVERT - test that a reverted constant call sets contractRet to REVERT
d8616e8 to
c3fccfc
Compare
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ove init methods to the top - rename probeLowestBlockNumOfReceiptStore to initLowestBlockNumOfReceiptStore - move init and initLowestBlockNumOfReceiptStore to the top of ChainBaseManager methods
…sonrpc-revert-and-pruned-history # Conflicts: # framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java # framework/src/test/java/org/tron/core/jsonrpc/JsonrpcServiceTest.java # framework/src/test/java/org/tron/core/services/jsonrpc/JsonRpcErrorResolverTest.java
- rename TransactionRetStore.getLowestBlockNum to getLowestReceiptBlockNum - note that transHistory.switch off disables receipt and log queries
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
- run the constant revert test with debug on to skip the cpu time limit - clarify transHistory.switch comment in reference.conf and config.conf - drop unused throws from getFilterChanges
…sonrpc-revert-and-pruned-history
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 lowest receipt block (the first block intransactionRetStore, read at startup). A FullNode has no such check — receipts from before June 2019 may still be served fromTransactionHistoryStore, so the store's first key is not a lower bound — and receipt gaps on a FullNode keep the current behavior.datais the"{}"placeholder every java-tron JSON-RPC error has.earliestresolves to the lowest block with all persisted data: on a LiteNode the lowest receipt block (the lowest stored block 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, including for receipts already on disk.earlieston a LiteNode resolves to the lowest available block instead of 0.This PR has been tested by:
Follow up
jsonrpcversion, non-stringmethod, explicitid: null) in a new PR.Extra details
triggerConstantContractresponses now carrycontractRet: REVERTwhen the call reverts.3, including one with empty revert data; geth returns-32000in that case.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.