fix: execution price RUNE outbound introspection #10427
Conversation
📝 WalkthroughWalkthroughReplaced transaction lookup by ID/index with a filter-based lookup using selectTxByFilter in useSwapActionSubscriber. Added early returns when swap or buyTxHash/buyAccountId are missing. Existing logic for computing actualBuyAmountCryptoPrecision remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant AC as SwapActionSubscriber
participant Store as Redux Store
participant Selector as selectTxByFilter
rect rgb(235, 245, 255)
note over AC: New gating
AC->>AC: Validate swap, buyTxHash, buyAccountId
alt Missing data
AC-->>AC: Early return
end
end
rect rgb(240, 255, 240)
AC->>Store: getState()
AC->>Selector: selectTxByFilter(state, {accountId, txHash})
Selector-->>AC: tx | undefined
alt tx found
AC->>AC: Compute actualBuyAmountCryptoPrecision
AC->>Store: Dispatch updates as before
else tx missing
AC-->>AC: Skip computation
end
end
note over AC: Comments mention special cases (e.g., RUNE outbounds)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
Outdated
Show resolved
Hide resolved
![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)
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx (2)
244-261: RUNE outbound fix via accountId + buyTxHash filter — LGTM; tiny cleanup.Logic is sound and the comment clearly explains the OUT: memo pitfall. Nit: the
!swapguard is redundant sinceswapis a required param; consider trimming and simplifying the IIFE.- const tx = (() => { - if (!swap || !buyTxHash) return - const { buyAccountId } = swap - if (!buyAccountId) return - // Use selectTxByFilter to find tx by hash instead of leveraging serialized index - // This handles the special case of RUNE outbounds i.e our serialized Tx would be - // cosmos:thorchain-1:thorAddy*txHash*:thorAddy but the Tx in store is - // cosmos:thorchain-1:thorAddy*txHash*thorAddy*OUT:txHash (note memo part) - const maybeTx = selectTxByFilter(store.getState(), { - accountId: buyAccountId, - txHash: buyTxHash, - }) - return maybeTx - })() + const tx = + buyTxHash && swap.buyAccountId + ? selectTxByFilter(store.getState(), { + accountId: swap.buyAccountId, + txHash: buyTxHash, + }) + : undefined
286-294: Potential race: success may commit without actualBuyAmount if tx isn’t indexed yet.Since polling stops when status is Success, a slow indexer could leave
actualBuyAmountCryptoPrecisionundefined and the execution price absent. Consider a short-lived post-success retry (separate query keyed by buyTxHash) until the receive tx is found or a max backoff is hit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
**/use*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/use*.{ts,tsx}: ALWAYS use use prefix for custom hooks
ALWAYS use descriptive names that indicate the hook's purpose
ALWAYS use camelCase after the use prefix for custom hooks
Files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.
Learnt from: NeOMakinG
PR: shapeshift/web#10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T15:36:25.122Z
Learning: In swap transaction handling, buy transaction hashes should always use the swapper's explorer (stepSource) because they are known by the swapper immediately upon swap execution. The conditional logic for using default explorers applies primarily to sell transactions which need to be detected/indexed by external systems like Thorchain or ViewBlock.
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/config.ts:127-128
Timestamp: 2025-08-07T11:20:44.614Z
Learning: gomesalexandre prefers required environment variables without default values in the config file (src/config.ts). They want explicit configuration and fail-fast behavior when environment variables are missing, rather than having fallback defaults.
Learnt from: gomesalexandre
PR: shapeshift/web#10249
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:447-503
Timestamp: 2025-08-13T17:07:10.763Z
Learning: gomesalexandre prefers relying on TypeScript's type system for validation rather than adding defensive runtime null checks when types are properly defined. They favor a TypeScript-first approach over defensive programming with runtime validations.
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpDepositActionSubscriber.tsx:61-66
Timestamp: 2025-08-14T17:51:47.556Z
Learning: gomesalexandre is not concerned about structured logging and prefers to keep console.error usage as-is rather than implementing structured logging patterns, even when project guidelines suggest otherwise.
Learnt from: gomesalexandre
PR: shapeshift/web#10413
File: src/components/Modals/FiatRamps/fiatRampProviders/onramper/utils.ts:29-55
Timestamp: 2025-09-02T14:26:19.028Z
Learning: gomesalexandre prefers to keep preparatory/reference code simple until it's actively consumed, rather than implementing comprehensive error handling, validation, and robustness improvements upfront. They prefer to add these improvements when the code is actually being used in production.
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:396-402
Timestamp: 2025-08-14T17:55:57.490Z
Learning: gomesalexandre is comfortable with functions/variables that return undefined or true (tri-state) when only the truthy case matters, preferring to rely on JavaScript's truthy/falsy behavior rather than explicitly returning boolean values.
Learnt from: gomesalexandre
PR: shapeshift/web#10206
File: src/lib/moralis.ts:47-85
Timestamp: 2025-08-07T11:22:16.983Z
Learning: gomesalexandre prefers console.error over structured logging for Moralis API integration debugging, as they find it more conventional and prefer to examine XHR requests directly rather than rely on structured logs for troubleshooting.
Learnt from: gomesalexandre
PR: shapeshift/web#10418
File: src/Routes/RoutesCommon.tsx:231-267
Timestamp: 2025-09-03T21:17:27.681Z
Learning: gomesalexandre prefers to keep PR diffs focused and reasonable in size, deferring tangential improvements (like Mixpanel privacy enhancements) to separate efforts rather than expanding the scope of feature PRs.
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/hooks/useActionCenterSubscribers/useThorchainLpActionSubscriber.tsx:46-53
Timestamp: 2025-08-14T19:21:45.426Z
Learning: gomesalexandre does not like code patterns being labeled as "technical debt" and prefers neutral language when discussing existing code patterns that were intentionally moved/maintained for consistency.
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Avoid side effects in swap logic within swapper implementation files.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-22T15:07:18.021Z
Learnt from: kaladinlight
PR: shapeshift/web#10326
File: src/hooks/useActionCenterSubscribers/useThorchainLpActionSubscriber.tsx:37-41
Timestamp: 2025-08-22T15:07:18.021Z
Learning: In src/hooks/useActionCenterSubscribers/useThorchainLpActionSubscriber.tsx, kaladinlight prefers not to await the upsertBasePortfolio call in the Base chain handling block, indicating intentional fire-and-forget behavior for Base portfolio upserts in the THORChain LP completion flow.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-08T11:40:55.734Z
Learnt from: NeOMakinG
PR: shapeshift/web#10234
File: src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx:41-41
Timestamp: 2025-08-08T11:40:55.734Z
Learning: In MultiHopTrade confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx and related hooks), there is only one active trade per flow. Because of this, persistent (module/Redux) dedupe for QuotesReceived in useTrackTradeQuotes is not necessary; the existing ref-based dedupe is acceptable.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Leverage shared utilities (e.g., executeEvmTransaction, checkEvmSwapStatus) in swapper implementations when applicable.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-04T16:02:27.360Z
Learnt from: NeOMakinG
PR: shapeshift/web#10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T16:02:27.360Z
Learning: In multi-hop swap transactions, last hop sell transactions might not be detected by the swapper (unlike buy transactions which are always known immediately). The conditional stepSource logic for last hop buy transactions (`isLastHopSellTxSeen ? stepSource : undefined`) serves as defensive programming for future multi-hop support with intermediate chains, even though multi-hop functionality is not currently supported in production.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-04T15:36:25.122Z
Learnt from: NeOMakinG
PR: shapeshift/web#10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T15:36:25.122Z
Learning: In swap transaction handling, buy transaction hashes should always use the swapper's explorer (stepSource) because they are known by the swapper immediately upon swap execution. The conditional logic for using default explorers applies primarily to sell transactions which need to be detected/indexed by external systems like Thorchain or ViewBlock.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
🧬 Code graph analysis (1)
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx (2)
src/state/slices/txHistorySlice/selectors.ts (1)
selectTxByFilter(293-297)src/state/store.ts (1)
store(137-137)
⏰ 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: Call / Static
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
https://jam.dev/c/e5849c95-4e1a-40b1-82ce-287711c404d7
Get this in 🐐
Description
See inline comment - the current execution price logic (or more generally, Tx selection from serialized Tx) doesn't work for RUNE Txs with a memo, since a RUNE swap with an outbound consists of:
However, when we serialize it, we do miss the OUT: part, which is not part of the original memo, but an on-chain THOR memo for outbounds.
This fixes it by introspecting by txHash + buy accountId. Note the latter is important as both sell and receive Tx share the same Txid.
Issue (if applicable)
closes #10417
Risk
Low, though risk of borked Txs for other chains/pairs
Testing
Note: This won't work for previous Txs, make sure to initiate a new swap
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit
New Features
Bug Fixes
Refactor