Conversation
WalkthroughAdds refs and a 10s grace window to HomeScreen polling to delay failures when an on‑chain tx hash appears; adjusts ResourceLock step logic by isBuy in utils; updates Sell preview gas formatting and button state during gas estimation; minor UI styling and test/mocks updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as HomeScreen
participant Poller as StatusPoller
participant Chain as Blockchain
participant Refs as Local Refs/State
UI->>Poller: startPolling(txId)
loop every N seconds
Poller->>Chain: fetchStatus(txId)
Chain-->>Poller: { status, txHash? }
alt txHash present
Poller->>Refs: set blockchainTxHashRef, set failureGraceExpiryRef = now+10s
Poller->>UI: mirror txHash to state (first observation sets hasSeenHash)
end
alt status == success
Poller->>UI: dispatch Success (update state + hasSeenSuccess)
else status == failed and blockchainTxHashRef exists and now < failureGraceExpiryRef
Note right of Poller: defer final failure (within grace window)
Poller-->>UI: skip failure dispatch
else status == failed
Poller->>UI: dispatch Failure
end
end
sequenceDiagram
autonumber
participant Flow as Transaction Flow
participant Steps as Steps Engine (utils)
rect rgba(230,245,255,0.6)
note over Flow,Steps: On Success / Completed computation
Flow->>Steps: computeSteps(isBuy=true/false)
alt isBuy
Steps->>Steps: ResourceLock = completed
else
Steps->>Steps: ResourceLock = inactive
end
end
rect rgba(255,240,230,0.6)
note over Flow,Steps: On Failure
Flow->>Steps: computeSteps(isBuy=true/false, flags)
alt isBuy
alt isResourceLockFailed
Steps->>Steps: ResourceLock = failed (or inactive for completed step)
else
Steps->>Steps: ResourceLock = completed
end
else
Steps->>Steps: ResourceLock = inactive
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
Deploying x with
|
| Latest commit: |
d214c3e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d772d4f9.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3681-transaction-st.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/apps/pulse/components/Sell/PreviewSell.tsx (1)
363-367: Consider extracting the gas fee formatting logic.The gas fee formatting pattern
formatExponentialSmallNumber(limitDigitsNumber(parseFloat(gasCostNative)))is duplicated at lines 670-671. Extracting this into a helper function would improve maintainability.For example, add a helper function at the top of the component:
+const formatGasFee = (gasCostNative: string | undefined, nativeTokenSymbol: string | undefined): string => { + return gasCostNative && nativeTokenSymbol + ? `≈ ${formatExponentialSmallNumber(limitDigitsNumber(parseFloat(gasCostNative)))} ${nativeTokenSymbol}` + : '≈ 0.00'; +}; + const PreviewSell = (props: PreviewSellProps) => {Then simplify both usages:
- const gasFeeString = - gasCostNative && nativeTokenSymbol - ? `≈ ${formatExponentialSmallNumber(limitDigitsNumber(parseFloat(gasCostNative)))} ${nativeTokenSymbol}` - : '≈ 0.00'; + const gasFeeString = formatGasFee(gasCostNative, nativeTokenSymbol);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/apps/pulse/components/Sell/tests/__snapshots__/PreviewSell.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Transaction/tests/__snapshots__/TransactionDetails.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Transaction/tests/__snapshots__/TransactionErrorBox.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
src/apps/pulse/components/App/HomeScreen.tsx(1 hunks)src/apps/pulse/components/Sell/PreviewSell.tsx(4 hunks)src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx(3 hunks)src/apps/pulse/components/Transaction/TransactionDetails.tsx(2 hunks)src/apps/pulse/components/Transaction/TransactionErrorBox.tsx(2 hunks)src/apps/pulse/components/Transaction/TransactionInfo.tsx(1 hunks)src/apps/pulse/utils/utils.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/pulse/components/Sell/PreviewSell.tsx (1)
src/utils/number.tsx (2)
formatExponentialSmallNumber(119-149)limitDigitsNumber(42-69)
⏰ 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). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (4)
src/apps/pulse/components/Sell/PreviewSell.tsx (1)
8-11: LGTM!The import of
formatExponentialSmallNumberandlimitDigitsNumberutilities is appropriate for formatting gas fees with proper handling of exponential notation and digit limiting.src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx (3)
37-37: LGTM!The mock for
formatExponentialSmallNumbercorrectly returns the input as a string, aligning with the new gas formatting logic in the component.
52-63: LGTM!The extended mocks for
batch(withremovefunction) andsendBatches(with success response) correctly support the new transaction flow and batch cleanup logic in PreviewSell.
198-198: LGTM!The test assertion update from "≈ 1.000000 ETH" to "≈ 1 ETH" correctly reflects the new gas fee formatting behavior using
formatExponentialSmallNumberandlimitDigitsNumber.
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3681/transaction-status-pulse)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/apps/pulse/components/Sell/PreviewSell.tsx (2)
363-368: Extract duplicate gas fee formatting into a helper function.The gas fee formatting logic appears twice (lines 364–367 and lines 670–672) with identical computations. Extracting this into a helper function improves maintainability and reduces the risk of inconsistencies.
Add a helper function near the top of the component:
const formatGasFeeDisplay = ( gasCostNative: string | undefined, nativeTokenSymbol: string | undefined ): string => { return gasCostNative && nativeTokenSymbol ? `≈ ${formatExponentialSmallNumber(limitDigitsNumber(parseFloat(gasCostNative)))} ${nativeTokenSymbol}` : '≈ 0.00'; };Then replace lines 364–367 with:
- const gasFeeString = - gasCostNative && nativeTokenSymbol - ? `≈ ${formatExponentialSmallNumber(limitDigitsNumber(parseFloat(gasCostNative)))} ${nativeTokenSymbol}` - : '≈ 0.00'; + const gasFeeString = formatGasFeeDisplay(gasCostNative, nativeTokenSymbol);And replace lines 669–674 with:
'Gas fee', - (() => { - const gasFeeDisplay = gasCostNative - ? `≈ ${formatExponentialSmallNumber(limitDigitsNumber(parseFloat(gasCostNative)))} ${nativeTokenSymbol}` - : '≈ 0.00'; - return gasFeeDisplay; - })(), + formatGasFeeDisplay(gasCostNative, nativeTokenSymbol), false,Also applies to: 669-674
711-715: Consider showing "Executing..." instead of "Confirm" during transaction execution.When
isExecutingis true butisEstimatingGasis false, the button displays a spinner with the label "Confirm" (line 714). This may confuse users, as "Confirm" typically indicates a clickable action rather than an in-progress operation. Changing the label to "Executing..." or similar would improve clarity.Apply this diff:
{isExecuting || isEstimatingGas ? ( <div className="flex items-center justify-center gap-2"> <TailSpin color="#FFFFFF" height={20} width={20} /> - <span>{isEstimatingGas ? 'Estimating Gas...' : 'Confirm'}</span> + <span>{isEstimatingGas ? 'Estimating Gas...' : 'Executing...'}</span> </div> ) : ( <>Confirm</> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/LeftColumnTokenMarketDataRow.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensWithMarketDataTile/test/__snapshots__/TokensWithMarketDataTile.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/apps/pulse/components/App/HomeScreen.tsx(3 hunks)src/apps/pulse/components/Sell/PreviewSell.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/pulse/components/Sell/PreviewSell.tsx (1)
src/utils/number.tsx (2)
formatExponentialSmallNumber(119-149)limitDigitsNumber(42-69)
⏰ 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). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3681/transaction-status-pulse)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/apps/pulse/components/App/HomeScreen.tsx (1)
657-673: Verify grace window interaction with failure timeouts.When a blockchain hash arrives after a failure timeout has already been scheduled (e.g., status reported as 'Reverted' before hash appears), the grace window prevents new timeouts from being scheduled but doesn't cancel the existing one. This means:
- t=0s: Status 'Reverted', no hash →
updateStatusWithDelayschedules failure timeout at t=10s- t=2s: Hash arrives → grace initialized (expires at t=12s), early return prevents new timeout
- t=10s: Original timeout fires → marks transaction as failed (if
hasSeenSuccessRefstill false)- t=12s: Grace window expires, but transaction already marked as failed
Is this intended? If the grace window should give the transaction 10s from when the hash is observed (not from the first failure), consider canceling any pending
failureTimeoutIdwhen initializing the grace window at line 653:if (firstObservation && failureGraceExpiryRef.current == null) { failureGraceExpiryRef.current = Date.now() + 10000; // start grace on first hash + if (failureTimeoutId) { + clearTimeout(failureTimeoutId); + setFailureTimeoutId(null); + } }Additionally, the comment at line 667 ("skip once immediately when hash first observed") is misleading because:
- This block only executes when status is failed (line 660), not when hash first observed
- The skip is not once—it's for the entire 10s grace period (line 669-670)
- Grace is typically initialized at line 653 when hash first observed, making lines 665-667 a defensive fallback
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/components/App/HomeScreen.tsx(3 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). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (6)
src/apps/pulse/components/App/HomeScreen.tsx (6)
160-161: LGTM: Refs properly avoid stale closures.The use of refs for
blockchainTxHashRefandfailureGraceExpiryRefcorrectly addresses the stale closure issue mentioned in previous reviews, ensuring the polling effect always reads fresh values without requiring them in the dependency array.
291-292: LGTM: Refs properly reset for each transaction.Resetting both
blockchainTxHashRefandfailureGraceExpiryRefalongside other transaction state ensures each transaction starts with a clean slate.
635-635: LGTM: Ref mirrors state for timeout callbacks.Setting
hasSeenSuccessRef.current = trueensures that failure timeout callbacks (which check this ref at line 416) see the fresh success status and abort correctly.
639-645: LGTM: Defensive dual-check pattern.Checking both
hasSeenSuccessstate andhasSeenSuccessRef.currentprovides defense-in-depth against edge cases and ensures failures are suppressed once success is observed, regardless of state/ref sync timing.
647-655: LGTM: Hash extraction now happens before failure check.Moving the hash extraction block before the failure guard (lines 657-673) correctly addresses the race condition identified in previous reviews, ensuring the ref is populated before the grace-window logic evaluates it.
675-676: LGTM: Status update happens after all guards.Calling
updateStatusWithDelayafter the grace window logic ensures that status updates only occur when appropriate, maintaining the existing delayed display behavior.
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
Bug Fixes
Style
Refactor