Conversation
There was a problem hiding this comment.
Bug: Render Phase Mutation Causes React Reconciliation Issues
Direct mutation of the transaction prop object occurs within the component's render phase, specifically modifying transaction.savedAction and transaction.assetAction. This violates React's immutability principles and render purity, causing unexpected behavior, potential performance issues, and reconciliation problems. Compounding this, the mutation condition uses reference equality for edgeTxActionSwapFromReports (a useMemo result), potentially causing unnecessary mutations on every render even when data is identical.
src/components/scenes/TransactionDetailsScene.tsx#L134-L143
edge-react-gui/src/components/scenes/TransactionDetailsScene.tsx
Lines 134 to 143 in 809d356
Bug: URL Handling Mismatch Causes Runtime Errors
The code exhibits inconsistent URL handling, mixing URLParse objects (from the url-parse library) with native URL constructor calls. This creates potential type mismatches, as functions like cleanFetch expect native URL objects. A critical instance of this is when new URL() is called with paymentRequestUrl, which is derived from a URL query parameter. Since query parameters can be relative paths, passing them to the native URL constructor (which requires an absolute URL) will cause a TypeError at runtime.
src/plugins/gui/providers/ioniaProvider.ts#L20-L421
edge-react-gui/src/plugins/gui/providers/ioniaProvider.ts
Lines 20 to 421 in 809d356
Was this report helpful? Give feedback by reacting with 👍 or 👎
In preparation for swapData to be available on receive transactions, we may not have the source wallet to pass to this component.
809d356 to
8cf2f7d
Compare
8cf2f7d to
21db217
Compare
| <EdgeText | ||
| ellipsizeMode="tail" | ||
| style={error ? styles.textHeaderError : styles.textHeader} | ||
| style={error != null ? styles.textHeaderError : styles.textHeader} |
There was a problem hiding this comment.
Incorrect conditional check breaks error styling logic
Medium Severity
The condition changed from error to error != null, which breaks the styling logic. Since error is a boolean prop, when error={false} is explicitly passed, false != null evaluates to true, causing the error style (styles.textHeaderError) to be incorrectly applied. The original truthy check error ? correctly handles all cases where error is true, false, or undefined.
| .map( | ||
| (target, index) => | ||
| `${target.publicAddress}${ | ||
| index + 1 !== spendTargets.length ? newline : '' |
There was a problem hiding this comment.
Accessing undefined wallet causes crash for tokens
Medium Severity
When computing destinationAssetName, the code accesses destinationWallet.currencyConfig without checking if destinationWallet is defined. The comment at line 155 acknowledges "The wallet may have been deleted" but this check is missing here. If swapData.payoutTokenId is not null (meaning a token swap) and the destination wallet was deleted, this will crash.
| transaction, | ||
| sourceWallet, | ||
| reportsTxInfo, | ||
| isReportsTxInfoLoading = false |
There was a problem hiding this comment.
Default value prevents intended status row hiding
Low Severity
The default value isReportsTxInfoLoading = false on line 76 contradicts the conditional check isReportsTxInfoLoading == null on line 342. The comment on lines 40-44 states "If not provided, the card will not show the status" but the default value ensures this condition is never met, as false == null is always false.
Additional Locations (1)
| plugin, | ||
| refundAddress | ||
| } = upgradeSwapData(wallet, swapData) | ||
| const swapData = upgradeSwapData(sourceWallet, props.swapData) |
There was a problem hiding this comment.
Wrong wallet passed to upgradeSwapData function
Medium Severity
The upgradeSwapData function expects a destinationWallet parameter (line 52) to resolve payoutTokenId from payoutCurrencyCode using the destination wallet's currency configuration. However, line 80 passes sourceWallet instead. This causes the function to look up the payout token in the wrong wallet's config, which will fail to find the correct token ID for token swaps.
| // Address information: | ||
| payoutAddress: txInfo.payout.address, | ||
| payoutCurrencyCode, | ||
| payoutNativeAmount: txInfo.payout.amount.toString(), |
There was a problem hiding this comment.
Inconsistent amount conversion between query and assignment
High Severity
The amount field from the API is handled inconsistently. At line 145, queryReportsTxInfo converts the amount to native units using mul(tx.payout.amount, denom.multiplier) for comparison. However, toEdgeTxSwap at line 183 and toEdgeTxActionSwap at lines 207/212 use the raw amount.toString() directly as nativeAmount/payoutNativeAmount without conversion. This results in incorrect swap amounts being stored.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have:
Note
Integrates reports server data into transaction details
util/reportsServerto query order status and amounts, map toEdgeTxSwap/EdgeTxActionSwap, and merge into receive tx’s without swap data; config viaENV.REPORTS_SERVERS.TransactionDetailsScene: usesreact-queryto fetch reports tx info, updatestransaction.savedAction, and passes status/loading toSwapDetailsCard.SwapDetailsCard: refactored to support optional source wallet, shows quote type and fetched exchange status, opens status page, and renders details via newDataSheetModal; adjusts support email body serialization.DataSheetModal(sectioned data display) andShimmerText(loading state);EdgeRowgainsshimmerprop. Snapshots updated for accessibility state and loading.Written by Cursor Bugbot for commit 21db217. This will update automatically on new commits. Configure here.