Skip to content

refactor(jsonrpc): centralize block selector parsing into JsonRpcApiUtil#6668

Open
0xbigapple wants to merge 4 commits intotronprotocol:developfrom
0xbigapple:refactor/jsonrpc-block-selector
Open

refactor(jsonrpc): centralize block selector parsing into JsonRpcApiUtil#6668
0xbigapple wants to merge 4 commits intotronprotocol:developfrom
0xbigapple:refactor/jsonrpc-block-selector

Conversation

@0xbigapple
Copy link
Copy Markdown
Collaborator

What does this PR do?
This PR centralizes JSON-RPC block tag and block number resolution in JsonRpcApiUtil so block selectors are parsed consistently across the JSON-RPC layer.

It also removes Wallet's reverse dependency on JSON-RPC code, eliminates the -1 means latest sentinel, and simplifies the block range resolution logic in LogFilterWrapper.

  • Added JsonRpcApiUtil.isBlockTag, parseBlockTag, and parseBlockNumber, and moved block tag constants and related errors, including safe, into JsonRpcApiUtil.
  • Updated TronJsonRpcImpl to delegate block selector parsing to JsonRpcApiUtil; eth_getBlock* paths now use shared block resolution logic.
  • Centralized the existing latest-only validation for state-read APIs such as getTrxBalance, getStorageAt, getABIOfSmartContract, and eth_call.
  • Removed JSON-RPC block parsing helpers from Wallet, replaced the -1 sentinel behavior, and added getHeadBlockNum().
  • Simplified the branching logic in LogFilterWrapper by making block range handling strategy-based and easier to reason about.
  • Added tests for block tag parsing and log filter strategy behavior.

Why are these changes required?
Block selector parsing is currently duplicated across Wallet, JsonRpcApiUtil, and TronJsonRpcImpl, with slightly different responsibilities and control flow in each layer.

This creates several problems:

  • duplicated and inconsistent block tag resolution logic
  • an undesirable reverse dependency from core Wallet code to the JSON-RPC layer
  • ambiguous -1 sentinel handling that forces callers to reinterpret results differently
  • complex block range branching in LogFilterWrapper that is harder to maintain and verify

By centralizing parsing in JsonRpcApiUtil, this PR makes the behavior easier to reason about, reduces coupling, and preserves existing JSON-RPC semantics while removing fragile internal conventions.

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up
None.

Extra details

  • This PR is intended as a refactor and cleanup. All JSON-RPC endpoints return identical results.

Comment thread framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcApiUtil.java Outdated
Comment thread framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java Outdated
Comment thread framework/src/main/java/org/tron/core/services/jsonrpc/TronJsonRpcImpl.java Outdated
…ution, also use longValueExact in eth_getProof block number parsing and clarify the getBlockByNumOrTag comment
@waynercheung
Copy link
Copy Markdown
Collaborator

[NIT] A few comments in this PR use the Unicode arrow . This is readable, but I’d prefer plain ASCII -> in source comments/Javadoc for consistency with the rest of the codebase and fewer tooling/font portability issues.

@0xbigapple
Copy link
Copy Markdown
Collaborator Author

[NIT] A few comments in this PR use the Unicode arrow . This is readable, but I’d prefer plain ASCII -> in source comments/Javadoc for consistency with the rest of the codebase and fewer tooling/font portability issues.

Good catch, fixed.

…use ASCII arrows in comments, also add safe tag coverage in state-read endpoint tests
Copy link
Copy Markdown
Collaborator

@waynercheung waynercheung left a comment

Choose a reason for hiding this comment

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

[NIT] A few minor cleanups that can be done in a follow-up or squash:

  1. LogFilterWrapper.java:27-28 - there is still a double blank line between toBlock and the constructor; this should be a single blank line.
  2. LogFilterWrapperStrategyTest.java:164 - filter is unused in testStrategy4_LatestGreaterThanSmallBlock_Throws; the call can be just createFilter(...) since the exception is expected before assignment.
  3. LogFilterWrapperStrategyTest.java:19-22,126 - comments still use the Unicode arrow while LogFilterWrapper.java was already switched to ASCII ->; it would be better to align them for consistency.

* Verify LogFilterWrapper strategies match develop branch behavior.
*
* Four filter strategies based on parameter emptiness (develop branch semantics):
* - Strategy 1: Both fromBlock and toBlock are empty → (currentMaxBlockNum, Long.MAX_VALUE)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

replacing '→' with '->'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants