feat: add order submission block tracking and improve order ID retrieval logic#379
Merged
onahprosper merged 3 commits intomainfrom Feb 23, 2026
Merged
Conversation
…val logic in TransactionPreview component
Dprof-in-tech
requested review from
5ran6,
chibie and
onahprosper
as code owners
Contributor
📝 WalkthroughWalkthroughCapture the RPC block number at transaction submission and rework order polling to use that block as a lower bound. Polling runs every 2s, queries OrderCreated logs between computed bounds, resolves when a matching log is found, updates order state, and is invoked before all submission paths. Changes
Sequence DiagramsequenceDiagram
participant Component as TransactionPreview
participant RPC as Public Client (RPC)
participant Submit as Transaction Submitter
participant Poller as Poll Loop (2s)
participant Logs as Log Retrieval
participant State as State Updater
Component->>RPC: captureSubmissionBlock()
RPC-->>Component: blockNumber
Component->>Component: store orderSubmissionBlock.current
Component->>Submit: send transaction (approve/create/execute)
Submit-->>Component: tx hash / confirmation
loop every 2s
Poller->>Poller: determine fromBlock (orderSubmissionBlock || toBlock-10)
Poller->>Logs: query OrderCreated logs in [fromBlock, toBlock]
Logs-->>Poller: logs or empty
alt log found
Poller->>State: update orderId, createdAt, status, currentStep
State-->>Poller: resolved
Poller-->>Poller: stop polling
else not found
Poller-->>Poller: continue
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/pages/TransactionPreview.tsx (1)
624-627: NewcreatePublicClientallocated on every poll tickA fresh client is constructed every 2 seconds for the lifetime of polling. Hoist it once outside
pollto avoid unnecessary object allocation and potential connection overhead.♻️ Proposed refactor
const poll = async () => { if (found || !activeWallet?.address) return; try { - const publicClient = createPublicClient({ - chain: selectedNetwork.chain, - transport: http(getRpcUrl(selectedNetwork.chain.name)), - }); const toBlock = await publicClient.getBlockNumber();And before the
polldefinition:+ const publicClient = createPublicClient({ + chain: selectedNetwork.chain, + transport: http(getRpcUrl(selectedNetwork.chain.name)), + }); + const poll = async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/TransactionPreview.tsx` around lines 624 - 627, The code creates a new publicClient on every poll tick; move the createPublicClient(...) call out of the poll function and hoist it before the poll definition (e.g., create a single publicClient with the selectedNetwork info once) and then use that instance inside poll; if selectedNetwork can change during component lifetime, create the client with useMemo or recreate it in an effect keyed on selectedNetwork (refer to createPublicClient, poll, and selectedNetwork.chain) so you avoid allocating a new client every 2s.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pages/TransactionPreview.tsx`:
- Around line 617-618: The polling interval created inside the getOrderId
promise (the local intervalId variable used with setInterval) can continue after
the component unmounts; move the interval ID into a ref (e.g., intervalIdRef) so
the interval is accessible outside the closure, clear that ref in a useEffect
cleanup, and also clear the ref/interval when polling resolves or errors (inside
the getOrderId promise). Apply the same change for the other polling instance
referenced around the second setInterval (the one at the 677-678 region) so all
intervals are stored in refs and reliably cleared on unmount or completion.
- Around line 615-679: The getOrderId promise in getOrderId never rejects or
times out, causing the UI to stay stuck; add a clear failure path by introducing
a maxRetry counter or wall-clock timeout inside getOrderId (track attempts or
use setTimeout) that, when exceeded, clears the intervalId, sets found to true
(or otherwise stops polling), calls reject(new Error(...)) so the caller
(createOrder) can handle it, and optionally set isConfirming to false / update
state; ensure you reference and clear intervalId, handle the rejection where
getOrderId is awaited, and keep the existing success flow (decodeEventLog,
setOrderId, saveTransactionData) intact.
---
Nitpick comments:
In `@app/pages/TransactionPreview.tsx`:
- Around line 624-627: The code creates a new publicClient on every poll tick;
move the createPublicClient(...) call out of the poll function and hoist it
before the poll definition (e.g., create a single publicClient with the
selectedNetwork info once) and then use that instance inside poll; if
selectedNetwork can change during component lifetime, create the client with
useMemo or recreate it in an effect keyed on selectedNetwork (refer to
createPublicClient, poll, and selectedNetwork.chain) so you avoid allocating a
new client every 2s.
…nsactionPreview component
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/pages/TransactionPreview.tsx (2)
220-230:createPublicClientis re-instantiated on everypolltick — hoist it.
captureSubmissionBlock(line 222) and thepollclosure (line 642) each instantiate a fresh HTTP public client using the same chain + transport config.pollruns up to 60 times over the 120 s window, so this spawns up to 60 unnecessary client objects.Create the client once — either at the module level, in a
useMemo, or at minimum once at the top ofgetOrderId's Promise constructor — and pass or close over it:♻️ Proposed refactor
- const captureSubmissionBlock = async () => { + const getPublicClient = () => + createPublicClient({ + chain: selectedNetwork.chain, + transport: http(getRpcUrl(selectedNetwork.chain.name)), + }); + + const captureSubmissionBlock = async () => { try { - const publicClient = createPublicClient({ - chain: selectedNetwork.chain, - transport: http(getRpcUrl(selectedNetwork.chain.name)), - }); + const publicClient = getPublicClient(); orderSubmissionBlock.current = await publicClient.getBlockNumber(); } catch { orderSubmissionBlock.current = null; } };return new Promise<void>((resolve, reject) => { + const publicClient = getPublicClient(); let intervalId: NodeJS.Timeout; ... const poll = async () => { ... - const publicClient = createPublicClient({ - chain: selectedNetwork.chain, - transport: http(getRpcUrl(selectedNetwork.chain.name)), - }); const toBlock = await publicClient.getBlockNumber();Also applies to: 642-645
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/TransactionPreview.tsx` around lines 220 - 230, captureSubmissionBlock and the poll closure both call createPublicClient repeatedly (creating up to 60 clients); hoist the public client creation so a single instance is reused: create the client once (e.g., via useMemo or at the start of getOrderId's Promise) using selectedNetwork.chain and http(getRpcUrl(selectedNetwork.chain.name)), then have captureSubmissionBlock, poll, and orderSubmissionBlock.current reference that shared client instead of calling createPublicClient each time; remove the duplicate createPublicClient calls in the poll closure and ensure the shared client is in scope where getBlockNumber is invoked.
651-678:decodeEventLogre-decodes whatgetContractEventsalready decoded — uselogs[0].argsdirectly, and addstrict: true.
getContractEventswith a specificeventNamereturns logs with.argsalready decoded via the same ABI. CallingdecodeEventLogagain onlogs[0].data/logs[0].topicsis redundant. Additionally, withoutstrict: true,logs[0].args(and the duplicate decode) returns fields as optional —orderIdcould beundefined, silently breakingsetOrderIdandsaveTransactionData.♻️ Proposed refactor
const logs = await publicClient.getContractEvents({ address: getGatewayContractAddress( selectedNetwork.chain.name, ) as `0x${string}`, abi: gatewayAbi, eventName: "OrderCreated", args: { sender: activeWallet.address as `0x${string}`, token: tokenAddress, amount: parseUnits(amountSent.toString(), tokenDecimals ?? 18), }, fromBlock, toBlock, + strict: true, }); if (logs.length > 0 && !settled) { settled = true; - cleanup(); - - const decodedLog = decodeEventLog({ - abi: gatewayAbi, - eventName: "OrderCreated", - data: logs[0].data, - topics: logs[0].topics, - }); - - setIsOrderCreatedLogsFetched(true); - setOrderId(decodedLog.args.orderId); - - await saveTransactionData({ - orderId: decodedLog.args.orderId, - txHash: logs[0].transactionHash, - }); + try { + setIsOrderCreatedLogsFetched(true); + setOrderId(logs[0].args.orderId); + + await saveTransactionData({ + orderId: logs[0].args.orderId, + txHash: logs[0].transactionHash, + }); setCreatedAt(new Date().toISOString()); setTransactionStatus("pending"); setCurrentStep("status"); resolve(); + } catch (innerError) { + reject(innerError); + } finally { + cleanup(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/TransactionPreview.tsx` around lines 651 - 678, The code redundantly re-decodes events and risks undefined fields; update the publicClient.getContractEvents call to pass strict: true so args are non-optional, stop using decodeEventLog, and read the decoded values directly from logs[0].args (e.g. use logs[0].args.orderId) when setting state via setOrderId and setIsOrderCreatedLogsFetched; also add a simple presence check on logs[0].args.orderId before calling setOrderId to avoid silently setting undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pages/TransactionPreview.tsx`:
- Around line 666-689: The success branch currently calls cleanup() before
running decodeEventLog and other steps, so if decodeEventLog throws the promise
never settles; update the block around
decodeEventLog/saveTransactionData/resolve to ensure cleanup always runs and the
promise is settled on errors: either wrap the inner success logic
(decodeEventLog, setIsOrderCreatedLogsFetched, setOrderId, await
saveTransactionData, setCreatedAt, setTransactionStatus, setCurrentStep,
resolve) in a try/catch/finally where cleanup() is called in finally and any
exception calls reject(err) (or move cleanup() into a finally and keep resolve()
on success), ensuring decodeEventLog errors lead to reject() and the
timeout/interval are still cleared.
---
Nitpick comments:
In `@app/pages/TransactionPreview.tsx`:
- Around line 220-230: captureSubmissionBlock and the poll closure both call
createPublicClient repeatedly (creating up to 60 clients); hoist the public
client creation so a single instance is reused: create the client once (e.g.,
via useMemo or at the start of getOrderId's Promise) using selectedNetwork.chain
and http(getRpcUrl(selectedNetwork.chain.name)), then have
captureSubmissionBlock, poll, and orderSubmissionBlock.current reference that
shared client instead of calling createPublicClient each time; remove the
duplicate createPublicClient calls in the poll closure and ensure the shared
client is in scope where getBlockNumber is invoked.
- Around line 651-678: The code redundantly re-decodes events and risks
undefined fields; update the publicClient.getContractEvents call to pass strict:
true so args are non-optional, stop using decodeEventLog, and read the decoded
values directly from logs[0].args (e.g. use logs[0].args.orderId) when setting
state via setOrderId and setIsOrderCreatedLogsFetched; also add a simple
presence check on logs[0].args.orderId before calling setOrderId to avoid
silently setting undefined.
…nPreview component
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/pages/TransactionPreview.tsx (2)
220-230:createPublicClientduplicated acrosscaptureSubmissionBlockandpoll— consider sharing one instance
captureSubmissionBlockcreates a client and throws it away;pollthen creates a second client (on every tick) for the same chain and transport. Returning the client fromcaptureSubmissionBlockand threading it intogetOrderIdwould eliminate the per-tick allocation:♻️ Proposed refactor
- const captureSubmissionBlock = async () => { + const captureSubmissionBlock = async (): Promise<ReturnType<typeof createPublicClient> | null> => { try { const publicClient = createPublicClient({ chain: selectedNetwork.chain, transport: http(getRpcUrl(selectedNetwork.chain.name)), }); orderSubmissionBlock.current = await publicClient.getBlockNumber(); + return publicClient; } catch { orderSubmissionBlock.current = null; + return null; } };Then in each call site:
- await captureSubmissionBlock(); - // ... - await getOrderId(); + const submissionClient = await captureSubmissionBlock(); + // ... + await getOrderId(submissionClient);And inside
getOrderId, accept the optional client and skipcreatePublicClientwhen provided.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/TransactionPreview.tsx` around lines 220 - 230, captureSubmissionBlock creates and discards a new public client while poll creates its own client on every tick, causing redundant client allocations; refactor captureSubmissionBlock to create and return the public client (created via createPublicClient with selectedNetwork.chain and http(getRpcUrl(...))) and propagate that client into getOrderId and poll so they reuse the same instance; update getOrderId signature to accept an optional public client and skip calling createPublicClient when one is provided, and ensure orderSubmissionBlock.current is still set inside captureSubmissionBlock (or by the caller) so existing logic relying on orderSubmissionBlock.current remains intact.
638-645:createPublicClientre-instantiated on every 2-second poll tick — hoist it to thePromisebodyA fresh client is allocated on each of up to 60 poll invocations. Moving it above the
polldefinition reuses one instance for the entire polling lifetime:♻️ Proposed refactor
return new Promise<void>((resolve, reject) => { let intervalId: NodeJS.Timeout; let timeoutId: NodeJS.Timeout; let settled = false; + const publicClient = createPublicClient({ + chain: selectedNetwork.chain, + transport: http(getRpcUrl(selectedNetwork.chain.name)), + }); const cleanup = () => { ... }; timeoutId = setTimeout(...); const poll = async () => { if (settled || !activeWallet?.address) return; try { - const publicClient = createPublicClient({ - chain: selectedNetwork.chain, - transport: http(getRpcUrl(selectedNetwork.chain.name)), - }); const toBlock = await publicClient.getBlockNumber();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/TransactionPreview.tsx` around lines 638 - 645, The poll function currently calls createPublicClient on every tick, allocating a new client each time; hoist the client creation into the outer Promise body (before defining poll) so a single publicClient instance is reused for the whole polling lifecycle: createPublicClient({ chain: selectedNetwork.chain, transport: http(getRpcUrl(selectedNetwork.chain.name)) }) once in the Promise scope, keep a reference named publicClient, then remove its instantiation from inside poll and have poll use that outer publicClient; ensure selectedNetwork and getRpcUrl are available in the Promise scope and that any error handling around client creation remains intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pages/TransactionPreview.tsx`:
- Around line 647-649: In getOrderId, avoid using the latest toBlock as the
fallback floor (toBlock - BigInt(10)) because that narrows the search window;
instead capture a pollStartBlock once at the top of getOrderId by calling
publicClient.getBlockNumber() and use that as the floor when computing
fromBlock: set fromBlock = orderSubmissionBlock.current ?? (pollStartBlock -
BigInt(10)). Update references to publicClient.getBlockNumber() in getOrderId so
you compute pollStartBlock once, keep the later toBlock for the current head,
and use pollStartBlock as the fallback to ensure the scan window covers blocks
present at the start of the polling cycle.
- Around line 647-649: Compute and store a single "poll start" block number once
at the top of the Promise (e.g., read publicClient.getBlockNumber() into
pollStartBlock) and then use that stored pollStartBlock as the floor when
computing fromBlock (i.e., set fromBlock to orderSubmissionBlock.current ??
pollStartBlock - BigInt(10)) instead of re-reading toBlock each poll; update the
code around toBlock, fromBlock, and orderSubmissionBlock.current so toBlock
remains the latest block but the fallback lower bound is the initial
pollStartBlock to avoid a shrinking window.
- Around line 681-684: The call to saveTransactionData passes
logs[0].transactionHash which is typed nullable (Log.transactionHash | null);
update the code around saveTransactionData to guard or assert the value: check
that logs[0] exists and logs[0].transactionHash is non-null before calling
saveTransactionData (e.g., if (logs[0]?.transactionHash) { await
saveTransactionData({ orderId: decodedLog.args.orderId, txHash:
logs[0].transactionHash }); }), or if you can guarantee non-null, use a type
assertion on logs[0].transactionHash as 0x${string}; ensure you reference logs,
decodedLog.args.orderId and saveTransactionData in the fix.
---
Nitpick comments:
In `@app/pages/TransactionPreview.tsx`:
- Around line 220-230: captureSubmissionBlock creates and discards a new public
client while poll creates its own client on every tick, causing redundant client
allocations; refactor captureSubmissionBlock to create and return the public
client (created via createPublicClient with selectedNetwork.chain and
http(getRpcUrl(...))) and propagate that client into getOrderId and poll so they
reuse the same instance; update getOrderId signature to accept an optional
public client and skip calling createPublicClient when one is provided, and
ensure orderSubmissionBlock.current is still set inside captureSubmissionBlock
(or by the caller) so existing logic relying on orderSubmissionBlock.current
remains intact.
- Around line 638-645: The poll function currently calls createPublicClient on
every tick, allocating a new client each time; hoist the client creation into
the outer Promise body (before defining poll) so a single publicClient instance
is reused for the whole polling lifecycle: createPublicClient({ chain:
selectedNetwork.chain, transport: http(getRpcUrl(selectedNetwork.chain.name)) })
once in the Promise scope, keep a reference named publicClient, then remove its
instantiation from inside poll and have poll use that outer publicClient; ensure
selectedNetwork and getRpcUrl are available in the Promise scope and that any
error handling around client creation remains intact.
onahprosper
approved these changes
Feb 23, 2026
4 tasks
This was referenced Mar 27, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request enhances the transaction workflow in
TransactionPreview.tsxby improving the reliability of order log retrieval and ensuring more accurate block tracking during transaction submissions. The main changes revolve around capturing the blockchain block number at the moment of transaction submission and using it to optimize event log polling, which should reduce missed events and unnecessary polling.Key improvements include:
Block Tracking and Submission:
orderSubmissionBlockref to store the block number at the time of transaction submission, and added acaptureSubmissionBlockfunction to retrieve and set this block number before sending transactions. This ensures the system knows exactly from which block to start searching for relevant logs. [1] [2] [3] [4] [5] [6]Order Log Polling Improvements:
getOrderIdfunction to use the captured submission block as the starting point (fromBlock) for polling order creation logs, rather than using a fixed block range. This increases accuracy and efficiency in log retrieval. [1] [2]References
closes #355
Testing
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
Refactor
Bug Fixes
Improvements