[Tron Support] Do not panic on Invalid state roots#409
[Tron Support] Do not panic on Invalid state roots#409Wizdave97 wants to merge 7 commits intosubquery:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Tron blockchain network support to the Ethereum API client by introducing Tron-specific JSON-RPC providers that transform method parameters for compatibility, integrating these providers into the main EthereumApi class for automatic use on Tron chains, and implementing utilities for parameter sanitization and block number handling specific to Tron's limitations. Changes
Sequence DiagramsequenceDiagram
participant Client
participant EthereumApi
participant ProviderFactory
participant TronProvider as TronProvider<br/>(JSON-RPC/WS)
participant TronUtils
participant TronNetwork
Client->>EthereumApi: init(chainId)
EthereumApi->>EthereumApi: Check if chainId in TRON_CHAIN_IDS
alt Tron Chain Detected
EthereumApi->>ProviderFactory: Create TronJsonRpcProvider<br/>or TronWsProvider
ProviderFactory->>TronProvider: instantiate(url, network)
else Non-Tron Chain
EthereumApi->>ProviderFactory: Create standard provider
end
Client->>EthereumApi: send(method, params)
EthereumApi->>TronProvider: send(method, params)
TronProvider->>TronUtils: applyTronParamTransforms(method,<br/>params, chainId)
TronUtils->>TronUtils: cleanParamsForTron()
TronUtils->>TronUtils: replaceBlockNumberForTron()
TronUtils-->>TronProvider: transformedParams
TronProvider->>TronNetwork: send(transformedParams)
TronNetwork-->>TronProvider: result
TronProvider-->>EthereumApi: result
EthereumApi-->>Client: result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Thanks, will look into this today. |
Thank you |
|
@ianhe8x any updates here? |
sorry for the late reply.
|
|
@ianhe8x attended to your comments |
|
@ianhe8x Added some new fixes for tron support, please take a look. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/node/src/ethereum/ethers/tron-utils.ts (1)
7-17:eth_callreceives both param-cleaning transforms — worth documenting
eth_callappears in bothTRON_TRANSACTION_METHODSandTRON_BLOCK_NUMBER_METHODS, so at every call site bothcleanParamsForTronandreplaceBlockNumberForTronare applied to it sequentially. A brief comment here (or at the call sites) would make the dual-pass intent explicit for future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/node/src/ethereum/ethers/tron-utils.ts` around lines 7 - 17, Update the module-level exports TRON_TRANSACTION_METHODS and TRON_BLOCK_NUMBER_METHODS to include a short clarifying comment that eth_call intentionally appears in both lists so that both cleanParamsForTron and replaceBlockNumberForTron are applied in sequence at call sites; mention the dual-pass behavior and reference the transforming functions cleanParamsForTron and replaceBlockNumberForTron so future maintainers understand this is intentional and not a duplication bug.packages/node/src/ethereum/ethers/json-rpc-provider.ts (1)
35-47: Extract duplicated Tron parameter transformation logicThe Tron processing block (lines 35–47) is copy-pasted identically into
json-rpc-batch-provider.tslines 58–70. Extract this into a utility function intron-utils.tsto eliminate duplication and improve maintainability:Proposed extraction
In
tron-utils.ts:+/** + * Apply all Tron-specific parameter transformations for the given method. + */ +export function applyTronParamTransforms( + method: string, + params: Array<any>, + chainId: number, +): Array<any> { + let result = params; + if (TRON_TRANSACTION_METHODS.includes(method)) { + result = cleanParamsForTron(result, chainId); + } + if (TRON_BLOCK_NUMBER_METHODS[method] !== undefined) { + result = replaceBlockNumberForTron(method, result, chainId); + } + return result; +}Then in both providers'
send:- let cleanedParams = params; - if (TRON_TRANSACTION_METHODS.includes(method)) { - cleanedParams = cleanParamsForTron(cleanedParams, chainId); - } - if (TRON_BLOCK_NUMBER_METHODS[method] !== undefined) { - cleanedParams = replaceBlockNumberForTron(method, cleanedParams, chainId); - } + const cleanedParams = applyTronParamTransforms(method, params, chainId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/node/src/ethereum/ethers/json-rpc-provider.ts` around lines 35 - 47, Extract the duplicated Tron param transformation into a new utility function (e.g., normalizeTronParams) in tron-utils.ts and call it from both providers' send paths; specifically, move the logic that checks TRON_TRANSACTION_METHODS and TRON_BLOCK_NUMBER_METHODS and that calls cleanParamsForTron(cleanedParams, chainId) and replaceBlockNumberForTron(method, cleanedParams, chainId) into normalizeTronParams(method, params, network) (or similar) and replace the existing inline blocks in json-rpc-provider.ts and json-rpc-batch-provider.ts with a single call to that function, ensuring you preserve the chainId derivation from this.network and the original method/params contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/node/src/ethereum/ethers/tron-utils.ts`:
- Around line 44-61: Add a prominent comment inside replaceBlockNumberForTron
(and near TRON_BLOCK_NUMBER_METHODS if helpful) explaining that Tron RPC rejects
numeric block tags and we forcibly replace numeric block parameters with
'latest', and explicitly state the limitation: all state-querying methods
(eth_call, eth_getBalance, eth_getCode, eth_getTransactionCount,
eth_getStorageAt) will return tip/current state rather than point-in-time state
for historical indexing, so SubQL/indexers cannot obtain historical state via
these calls; include the RPC error text ("QUANTITY not supported, just support
TAG as latest") and the JSON-RPC code (-32602) in the comment to make the
limitation and its operational impact clear.
---
Duplicate comments:
In `@packages/node/src/ethereum/ethers/json-rpc-batch-provider.ts`:
- Around line 58-74: The Tron-specific param transformations in
json-rpc-batch-provider.ts duplicate logic from json-rpc-provider.ts and use the
same confusing guard; extract the shared logic into a new function
applyTronParamTransforms(method, params, chainId) in tron-utils.ts that
internally calls TRON_TRANSACTION_METHODS, cleanParamsForTron,
TRON_BLOCK_NUMBER_METHODS, and replaceBlockNumberForTron, then replace the
copied block in json-rpc-batch-provider.ts with a call to that helper; also
compute chainId with a clear safe expression (e.g. this.network?.chainId ?? 0)
before calling the helper to avoid the misleading this.network ? guard.
---
Nitpick comments:
In `@packages/node/src/ethereum/ethers/json-rpc-provider.ts`:
- Around line 35-47: Extract the duplicated Tron param transformation into a new
utility function (e.g., normalizeTronParams) in tron-utils.ts and call it from
both providers' send paths; specifically, move the logic that checks
TRON_TRANSACTION_METHODS and TRON_BLOCK_NUMBER_METHODS and that calls
cleanParamsForTron(cleanedParams, chainId) and replaceBlockNumberForTron(method,
cleanedParams, chainId) into normalizeTronParams(method, params, network) (or
similar) and replace the existing inline blocks in json-rpc-provider.ts and
json-rpc-batch-provider.ts with a single call to that function, ensuring you
preserve the chainId derivation from this.network and the original method/params
contract.
In `@packages/node/src/ethereum/ethers/tron-utils.ts`:
- Around line 7-17: Update the module-level exports TRON_TRANSACTION_METHODS and
TRON_BLOCK_NUMBER_METHODS to include a short clarifying comment that eth_call
intentionally appears in both lists so that both cleanParamsForTron and
replaceBlockNumberForTron are applied in sequence at call sites; mention the
dual-pass behavior and reference the transforming functions cleanParamsForTron
and replaceBlockNumberForTron so future maintainers understand this is
intentional and not a duplication bug.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/node/src/ethereum/ethers/tron-utils.ts (1)
7-8:eth_estimateGasis absent from bothTRON_TRANSACTION_METHODSandTRON_BLOCK_NUMBER_METHODS
eth_estimateGas(transaction, blockTag)has the same shape aseth_call: its first parameter is a transaction object (susceptible to the sametype/accessListrejection) and its second parameter is an optional block tag. If any caller invokeseth_estimateGason a Tron chain with EIP-2930/EIP-1559 transaction fields, the params will not be cleaned, likely producing a Tron RPC error.♻️ Suggested addition
// Methods that accept transaction objects as parameters -export const TRON_TRANSACTION_METHODS = ['eth_call']; +export const TRON_TRANSACTION_METHODS = ['eth_call', 'eth_estimateGas'];export const TRON_BLOCK_NUMBER_METHODS: Record<string, number> = { eth_call: 1, + eth_estimateGas: 1, eth_getStorageAt: 2, eth_getBalance: 1, eth_getCode: 1, eth_getTransactionCount: 1, };Also applies to: 27-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/node/src/ethereum/ethers/tron-utils.ts` around lines 7 - 8, Add eth_estimateGas to the arrays that treat RPC calls with a transaction object like eth_call so its params are cleaned before sending to Tron: update TRON_TRANSACTION_METHODS to include 'eth_estimateGas' and also include 'eth_estimateGas' in TRON_BLOCK_NUMBER_METHODS where transaction+optional blockTag methods are listed; ensure any logic that strips EIP-2930/EIP-1559 fields (type/accessList/maxFeePerGas/etc.) for eth_call also runs for eth_estimateGas (refer to TRON_TRANSACTION_METHODS and TRON_BLOCK_NUMBER_METHODS to find where the cleaning is applied).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/node/src/ethereum/ethers/tron-utils.ts`:
- Around line 7-8: Add eth_estimateGas to the arrays that treat RPC calls with a
transaction object like eth_call so its params are cleaned before sending to
Tron: update TRON_TRANSACTION_METHODS to include 'eth_estimateGas' and also
include 'eth_estimateGas' in TRON_BLOCK_NUMBER_METHODS where
transaction+optional blockTag methods are listed; ensure any logic that strips
EIP-2930/EIP-1559 fields (type/accessList/maxFeePerGas/etc.) for eth_call also
runs for eth_estimateGas (refer to TRON_TRANSACTION_METHODS and
TRON_BLOCK_NUMBER_METHODS to find where the cleaning is applied).
Wizdave97
changed the title
|
@Wizdave97 If you need to customise rpc provider for Tron, can you do the same way we did for Celo. check it out here, packages/node/src/ethereum/ethers/celo/celo-provider.ts |
|
@ianhe8x I've updated the implementation |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/node/src/ethereum/ethers/tron/tron-utils.ts (1)
7-8: Consider addingeth_estimateGastoTRON_TRANSACTION_METHODS.
eth_estimateGasalso receives a transaction object as its first parameter (params:[txObj, blockTag?]). If ethers.js ever calls it with an EIP-1559/EIP-2930 transaction (which would includetypeandaccessList), Tron would reject it with the same error. Adding it here and toTRON_BLOCK_NUMBER_METHODSat index1(block tag is optional but at index 1 when present) would make the handling symmetric with the documented intent.♻️ Proposed addition
-export const TRON_TRANSACTION_METHODS = ['eth_call']; +export const TRON_TRANSACTION_METHODS = ['eth_call', 'eth_estimateGas'];export const TRON_BLOCK_NUMBER_METHODS: Record<string, number> = { eth_call: 1, + eth_estimateGas: 1, eth_getStorageAt: 2,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/node/src/ethereum/ethers/tron/tron-utils.ts` around lines 7 - 8, The TRON handling currently treats only 'eth_call' as a method that accepts a transaction object, which misses 'eth_estimateGas' and can allow EIP-1559/EIP-2930 fields to reach Tron and fail; update the TRON_TRANSACTION_METHODS array to include 'eth_estimateGas' and also add 'eth_estimateGas' at index 1 of TRON_BLOCK_NUMBER_METHODS (so the optional blockTag param is handled symmetrically) to ensure transaction objects passed to estimateGas are sanitized like eth_call.packages/node/src/ethereum/ethers/tron/tron-provider.ts (1)
16-18: Redundant no-op constructors can be removed.All three constructors simply delegate to
super(url, network)with no additional logic, which is already TypeScript's default behavior when no constructor is declared. Removing them reduces noise.♻️ Proposed simplification (shown for TronJsonRpcProvider; apply identically to TronJsonRpcBatchProvider and TronWsProvider)
export class TronJsonRpcProvider extends JsonRpcProvider { - constructor(url: string | ConnectionInfo, network?: Networkish) { - super(url, network); - } - async send(method: string, params: Array<any>): Promise<any> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/node/src/ethereum/ethers/tron/tron-provider.ts` around lines 16 - 18, Remove the redundant no-op constructors that only call super(url, network) in the Tron provider classes: TronJsonRpcProvider, TronJsonRpcBatchProvider, and TronWsProvider in tron-provider.ts; delete each empty constructor method so the classes inherit the default constructor behavior from their base class (i.e., remove the constructor(url: string | ConnectionInfo, network?: Networkish) { super(url, network); } declarations).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/node/src/ethereum/ethers/tron/tron-provider.ts`:
- Around line 20-24: The Tron provider constructors are not being passed the
already-resolved network from const network = await this.client.getNetwork(),
which causes send() (in send(method, params)) to see chainId = 0 until the
provider resolves itself; update the instantiation sites to pass the resolved
network into TronWsProvider, TronJsonRpcBatchProvider and TronJsonRpcProvider
(and apply the same change to the analogous Celo provider constructors) so the
provider instances are constructed with the known network and
applyTronParamTransforms receives the correct chainId immediately.
---
Nitpick comments:
In `@packages/node/src/ethereum/ethers/tron/tron-provider.ts`:
- Around line 16-18: Remove the redundant no-op constructors that only call
super(url, network) in the Tron provider classes: TronJsonRpcProvider,
TronJsonRpcBatchProvider, and TronWsProvider in tron-provider.ts; delete each
empty constructor method so the classes inherit the default constructor behavior
from their base class (i.e., remove the constructor(url: string |
ConnectionInfo, network?: Networkish) { super(url, network); } declarations).
In `@packages/node/src/ethereum/ethers/tron/tron-utils.ts`:
- Around line 7-8: The TRON handling currently treats only 'eth_call' as a
method that accepts a transaction object, which misses 'eth_estimateGas' and can
allow EIP-1559/EIP-2930 fields to reach Tron and fail; update the
TRON_TRANSACTION_METHODS array to include 'eth_estimateGas' and also add
'eth_estimateGas' at index 1 of TRON_BLOCK_NUMBER_METHODS (so the optional
blockTag param is handled symmetrically) to ensure transaction objects passed to
estimateGas are sanitized like eth_call.
|
@Wizdave97 if tron doesn't support query a historical states, instead of replacing it to latest, maybe the better way is throwing error? This may lead to unexpected behaviour |
|
Throwing an error will break indexing and means the user has to modify these methods manually if they need to use them. How about we just add the behaviour of these calls to the docs? Anyone indexing Tron knows historical queries are not supported. I don't think they would want to go through the hassle of using manual RPC calls to use these methods instead of relying on the api provided by the indexer |
|
I can't think of a reason why people want to query latest states in a history block in a subquery project. so let people be aware of this is a correct way. |
But Tron only supports latest state queries so they have no choice, without this fix, it's not possible to index Tron with the subquery project. |
|
in a indexer project, the data flows with the blocks, should not query latest states and utilize latest states in the mapping. |
2 participants
Description
This PR prevents panics when the state root of the fetched Ethereum block is missing or malformed. This fix allows indexing of Events on the Tron network.
The Tron Network Ethereum compatibility RPC layer sometimes returns invalid state roots in block headers.
Also added some fixes for json rpc calls that accept a block number parameter, tron only allows the "latest" tag, this pr ensures that's the case for tron
We also strip unsupported parameters from transaction objects to the tron rpc, fields like type and accessList which are sometimes added by ethers are deleted if the chain id is tron.
With this fix i was able to index events from contracts on the tron network without any errors.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist
Summary by CodeRabbit