refactor(jsonrpc): centralize block selector parsing into JsonRpcApiUtil#6668
refactor(jsonrpc): centralize block selector parsing into JsonRpcApiUtil#66680xbigapple wants to merge 3 commits intotronprotocol:developfrom
Conversation
…ution, also use longValueExact in eth_getProof block number parsing and clarify the getBlockByNumOrTag comment
| } | ||
| fromBlockSrc = min(toBlockSrc, currentMaxBlockNum); | ||
|
|
||
| } else if (!fromEmpty && toEmpty) { |
There was a problem hiding this comment.
[NIT] The !toEmpty / !fromEmpty checks are redundant here because the previous branches already exclude those cases, so static analysis reports them as always true when reached. This can be simplified to else if (fromEmpty) and else if (toEmpty).
|
[NIT] A few comments in this PR use the Unicode arrow |
| Assert.fail("Expected to be thrown"); | ||
| } catch (Exception e) { | ||
| Assert.assertEquals("TAG [earliest | pending | finalized] not supported", | ||
| Assert.assertEquals(TAG_NOT_SUPPORT_ERROR, |
There was a problem hiding this comment.
The parseBlockTag("safe") test at line 458 is a nice addition — safe was previously unhandled!
nit: Since safe is now part of isBlockTag and the error message was updated to include it, would it be worth adding at least one test case exercising safe through a state-read endpoint like getTrxBalance? The existing tests cover earliest, pending, and finalized through each endpoint but not safe:
try {
tronJsonRpc.getTrxBalance("", "safe");
Assert.fail("Expected to be thrown");
} catch (Exception e) {
Assert.assertEquals(TAG_NOT_SUPPORT_ERROR, e.getMessage());
}| * Reject any block selector that is not "latest". | ||
| * Accepts "latest" silently; throws for other tags, numeric blocks, or invalid input. | ||
| */ | ||
| private void requireLatestBlockTag(String blockNumOrTag) |
There was a problem hiding this comment.
Really clean extraction — the old code had the same tag-check + hex-parse + quantity-reject sequence copy-pasted across getTrxBalance, getStorageAt, getABIOfSmartContract, and getCall. Pulling it into requireLatestBlockTag with clear error stratification (tag error → number error → quantity error) is a significant readability win. 👏
5 participants
What does this PR do?
This PR centralizes JSON-RPC block tag and block number resolution in
JsonRpcApiUtilso 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 latestsentinel, and simplifies the block range resolution logic inLogFilterWrapper.JsonRpcApiUtil.isBlockTag,parseBlockTag, andparseBlockNumber, and moved block tag constants and related errors, includingsafe, intoJsonRpcApiUtil.TronJsonRpcImplto delegate block selector parsing toJsonRpcApiUtil;eth_getBlock*paths now use shared block resolution logic.latest-only validation for state-read APIs such asgetTrxBalance,getStorageAt,getABIOfSmartContract, andeth_call.Wallet, replaced the-1sentinel behavior, and addedgetHeadBlockNum().LogFilterWrapperby making block range handling strategy-based and easier to reason about.Why are these changes required?
Block selector parsing is currently duplicated across
Wallet,JsonRpcApiUtil, andTronJsonRpcImpl, with slightly different responsibilities and control flow in each layer.This creates several problems:
Walletcode to the JSON-RPC layer-1sentinel handling that forces callers to reinterpret results differentlyLogFilterWrapperthat is harder to maintain and verifyBy 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:
Follow up
None.
Extra details