Conversation
Time Submission Status
|
WalkthroughThe PR optimizes Changes
Sequence DiagramsequenceDiagram
participant Client
participant Indexer
participant RPC as RPC Node
Note over Client,RPC: ❌ Old Approach (N+1 calls)
Client->>Indexer: Get transactions (N blocks)
Indexer-->>Client: Transactions with block_height
loop Per Transaction
Client->>RPC: Get block timestamp
RPC-->>Client: timestamp
end
Note over Client,RPC: ✅ New Approach (Single query)
Client->>Indexer: Get transactions range query<br/>(includes stamp_ms)
Indexer-->>Client: All transactions with<br/>block_height, hash, sender, stamp_ms
Client->>Client: Map & normalize response
Note right of Client: Fallback handling for<br/>missing fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
examples/get_last_transactions/package.json (1)
1-11: Make the example self-contained and non-publishable.Without local deps,
npm run startcan fail; also guard against accidental publish. If the workspace doesn’t hoist these, add:{ + "private": true, "scripts": { "start": "tsx index.ts" }, "name": "get_last_transactions", "version": "1.0.0", "description": "", - "main": "index.ts", + "main": "index.ts", "author": "", - "license": "ISC" + "license": "ISC", + "type": "module", + "devDependencies": { + "tsx": "^4" + }, + "dependencies": { + "ethers": "^6" + } }If the monorepo already provides tsx/ethers, feel free to swap to workspace ranges instead (e.g., "workspace:^"). Please confirm the repo setup.
examples/get_last_transactions/index.ts (2)
4-14: Parametrize secrets and endpoints; avoid committing keys.Use env vars for the demo key/endpoint/chain to prevent accidental misuse and ease local testing.
-import { Wallet } from "ethers"; +import { Wallet } from "ethers"; -const wallet = new Wallet("0000000000000000000000000000000000000000000000000000000000000001"); +// Demo only. Prefer supplying DEMO_PRIV_KEY via env. Never commit real keys. +const PRIV_KEY = process.env.DEMO_PRIV_KEY ?? "0000000000000000000000000000000000000000000000000000000000000001"; +const wallet = new Wallet(PRIV_KEY); const client = new NodeTNClient({ - endpoint: "https://gateway.mainnet.truf.network", + endpoint: process.env.TRUF_GATEWAY ?? "https://gateway.mainnet.truf.network", signerInfo: { address: wallet.address, signer: wallet, }, - chainId: "tn-v2.1", + chainId: process.env.TRUF_CHAIN_ID ?? "tn-v2.1", timeout: 30000, });Please confirm ethers v6 is in use (the example uses ESM import style).
27-30: Type-safe error handling in catch.
errormay be unknown; guard before accessing.message.- } catch (error) { - console.error("❌ Error:", error.message); - console.error(error); - } + } catch (err) { + if (err instanceof Error) { + console.error("❌ Error:", err.message); + console.error(err.stack ?? err); + } else { + console.error("❌ Error:", err); + } + }src/client/getLastTransactions.ts (5)
5-5: Allow overriding the indexer base.Hardcoding makes staging/tests harder. Consider env/config override.
-const INDEXER_BASE = "https://indexer.infra.truf.network"; +const INDEXER_BASE = process.env.TRUF_INDEXER_BASE ?? "https://indexer.infra.truf.network";If config already exposes this elsewhere, wire it through instead.
24-37: Verifycreated_atis a block height, not a timestamp.You treat
created_atas a block height; the name suggests time. If it’s actually a timestamp, the range query becomes invalid.
Confirm the SQL action
main.get_last_transactionsreturns block heights increated_at. If not, switch to the correct field or adjust the query alias (e.g.,SELECT block_height AS created_at), or rename the local var to avoid confusion.Consider typing
rowswith an explicitblock_heightfield to align semantics and reduce casting.
39-51: Add timeout and ensure Node fetch availability.
- Node <18 lacks global
fetch. If you support Node 16, import a ponyfill (undici/cross-fetch) or surface a fetch dependency.- Add an AbortController timeout to prevent request hangs and to help meet the <100ms target in failure cases.
- const resp = await fetch(txUrl); + // Abort after 8s (tune or make configurable if needed) + const ac = new AbortController(); + const timeoutId = setTimeout(() => ac.abort(), 8000); + let resp: Response; + try { + resp = await fetch(txUrl, { signal: ac.signal }); + } finally { + clearTimeout(timeoutId); + } if (!resp.ok) { - throw new Error(`Indexer fetch failed: ${resp.status}`); + const text = await resp.text().catch(() => ""); + throw new Error(`Indexer fetch failed: ${resp.status} ${resp.statusText} ${text}`.trim()); }Please confirm the minimum Node version supported by this package.
39-46: Range query could be heavy for sparse heights.If
maxBlock - minBlockis large (e.g., multiple thousands), this may return many irrelevant txs and hurt latency. A guarded fallback (multi-query by specific block heights or an API that accepts a list) would keep performance predictable.Example guard:
- If
max - min > 2000, either:
- Split the range into chunks, or
- Query per unique height (still zero RPC calls) with small concurrency.
Does the indexer expose a “blocks=1,2,3” style filter? If yes, that’s ideal here.
66-82: First-transaction-per-block semantics: clarify intent.For timestamps this is fine (per-block), but
sender/hashwill be from an arbitrary tx in that block (first in returned order). If consumers assume these correspond 1:1 to the SQLmethodrow, this may change behavior.
Confirm no downstream relies on exact tx identity for a given
method.Consider documenting this in the return type or include a
notefield, or omitsender/hashwhen deduping by block (breaking change) to avoid misinterpretation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/get_last_transactions/index.ts(1 hunks)examples/get_last_transactions/package.json(1 hunks)src/client/getLastTransactions.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
examples/get_last_transactions/index.ts (1)
16-26: Example flow looks good.Clear timing and pretty-printed output; good for validating the latency objective.
src/client/getLastTransactions.ts (2)
84-106: Mapping and fallbacks look solid.Stable order preserved; sensible fallbacks for missing blocks;
stampMssourced fromstamp_msper acceptance criteria. Nice.
1-106: Based on the script execution results, I've verified the review comment's two key assertions:
"No RPC method is invoked for timestamps within
getLastTransactions"— Confirmed. The script output shows only the indexer endpoint (/v0/chain/transactions) is accessed viafetch()at line 43. No RPC timestamp patterns were detected."
stampMsis populated from indexer data" — Confirmed. The output showsstamp_msreferences only in the indexer response handling (lines 79 and 104:stampMs: tx.stamp_ms ?? 0).The implementation correctly eliminates RPC timestamp calls and uses the indexer's
stamp_msfield as intended. The review's suggestion to add lightweight tests to prevent regressions is a reasonable best practice, but the code itself is functionally correct and meets core objectives.
Verified: Implementation correctly uses indexer timestamps, not RPC calls.
The change successfully eliminates RPC timestamp calls and uses indexer
stamp_msdata as intended. To prevent regressions, consider adding a lightweight test that:
- Verifies the indexer endpoint is called (not RPC methods)
- Confirms
stampMsis populated from the response dataTests would help lock in these design decisions for future maintenance.
2 participants
resolves: #132
Summary by CodeRabbit
New Features
Performance Improvements