fix(tron): the receipt for unsolidified block should not be set to ni…#1583
fix(tron): the receipt for unsolidified block should not be set to ni…#1583cranycrane wants to merge 7 commits into
Conversation
…l so all token transfers are parsed and indexed correctly
…block to speed up syncing
…en sequential sync path
… reuse net/http only returns a connection to the idle pool when the response body is read to EOF and then closed. Request() closed the body without draining: the >=300 early return reads nothing, and json.Decoder can leave trailing bytes unconsumed on larger responses, so connections were torn down instead of pooled — defeating the MaxIdleConns/MaxIdleConnsPerHost tuning this branch added to speed up syncing. Drain the body in the deferred close. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
requestBlockHashByNum is only called from GetBlockHash's fast path, which is guarded by isBlockSolidified() && solidityNodeHTTP \!= nil, so the sole caller always passed true and the /wallet/getblock (full-node) branch was unreachable. Hardcode the solidity-node client and /walletsolidity/getblock path so the function's actual single-purpose role is explicit and no untested branch lingers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pragmaxim
left a comment
There was a problem hiding this comment.
Reviewed the branch. I pushed two fixes directly:
- Drain the HTTP response body before close (
tronhttp.go) —net/httponly pools a keep-alive connection when the body is read to EOF and then closed.Request()closed without draining (the>=300path reads nothing;json.Decodercan leave trailing bytes), so connections were torn down instead of reused — defeating theMaxIdleConns*tuning this branch added for sync speed. - Drop the dead
isSolidifiedparam fromrequestBlockHashByNum— the only caller (theGetBlockHashfast path) is guarded byisBlockSolidified() && solidityNodeHTTP != niland always passedtrue, so the/wallet/getblockbranch was unreachable.
The three inline comments below are design calls I left for you rather than changing unilaterally.
| rebuiltTx, err := b.buildTxFromHTTPData(txByID, txInfo, bbh.Time, confirmations, txInternalData, isSolidified) | ||
| // Keep receipts for block indexing even before solidity. If logs are dropped | ||
| // here, token-transfer participants never reach the address/contract indexes. | ||
| rebuiltTx, err := b.buildTxFromHTTPData(txByID, txInfo, bbh.Time, confirmations, txInternalData, true) |
There was a problem hiding this comment.
This fixes the indexing path, but the serving path stays asymmetric: GetTransaction (line 1044) and GetTransactionForMempool (line 1074) still pass isSolidified/false, so for a TRC-20 transfer in a mined-but-not-yet-solidified block (~19-block window) the transfer is indexed with its receipt here, yet a live tx-detail fetch of the same tx returns Receipt=nil, pending status, and zero token transfers. The address page would then list the transfer while the transaction page shows none — the same index-vs-detail disparity #1576 targets, on the serving side. The PR description says the GetTransaction/mempool paths are intentionally left pending until solid; flagging to confirm that user-visible inconsistency is acceptable, or whether GetTransaction should also keep the receipt once the tx is in a block.
| return b.bestHeader, nil | ||
| } | ||
|
|
||
| b.bestHeaderLock.Lock() |
There was a problem hiding this comment.
Heads-up on the removed if b.mq == nil { refreshBestHeaderFromChain() } block: getBestHeader now serves a cached tip for up to tronBestHeaderMaxAge (30s), and the watchdog that refreshes it only starts in InitializeMempool, which runs after the initial ResyncIndex. So during initial bulk sync (and in polling-only mode before mempool init) the target tip and computeBlockConfirmations can lag the chain by up to 30s where they were previously always fresh. This looks like the deliberate trade-off from the "reduce requests per block" commit and is benign for far-behind sync — just confirming it's intended and that the syncer can't briefly treat the stale tip as fully-synced.
| // monotonic guard exists to prevent). That is acceptable for the common single-node | ||
| // java-tron backend; if Tron is ever fronted by a load balancer, port the EVM pattern | ||
| // here (monotonic hot path + on-advance liveness + allowRegress watchdog poll). | ||
| func (b *TronRPC) setBestHeader(h bchain.EVMHeader) bool { |
There was a problem hiding this comment.
This branch deleted the doc comment that explained setBestHeader is intentionally non-monotonic — it accepts a lower height so a genuine rollback surfaces immediately to resyncIndex, unlike EthereumRPC.setBestHeader's monotonic guard, plus the load-balancer caveat. The body is unchanged, so that rationale is still accurate. Consider restoring a trimmed version: without it, a future maintainer "harmonizing" Tron with the EVM path could re-add a monotonic guard and silently reintroduce frozen-tip masking on rollbacks.
Closes #1576.
Problem
Token transfers on Tron were missing from address/contract indexes for transactions in not-yet-solidified blocks. During indexing,
GetBlockdropped the receipt (and its event logs) for unsolidified blocks. Since Tron derives token transfers from receipt logs, the transfer participants never reached the indexes until the block solidified and was re-processed.Fix
buildTxFromHTTPData'sisSolidifiedparam →keepReceiptto name what it controls.GetBlocknow always keeps the receipt, so logs are retained for indexing regardless of solidity.GetTransaction/mempool paths are behavior-unchanged (still pending until solid).Tests
TestTronBuildTxFromHTTPData_KeepReceiptControlsTokenLogs:keepReceipt=false→ nil receipt, no transfers;keepReceipt=true→ receipt/logs retained and TRC-20 transfer decoded correctly.