Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
github-actions
Bot
added
pr-not-ready-for-e2e
Jwhiles
force-pushed
the
MUSD-816
branch
2 times, most recently
from
9f4fafc to
70099e0
Compare
Jwhiles
changed the title
Jwhiles
added
team-earn
and removed
pr-not-ready-for-e2e
…ision Skip mid-compose (unapproved/approved/signed) and aborted (rejected/dropped/cancelled) rows in the new mUSD activity branches so a cancelled send to the money account no longer renders as a phantom "Received". Pass isRounding=false to fromTokenMinimalUnit so the manual BigInt calldata decode isn't undone by the default Number() cast. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (isMusdLike) { | ||
| const rateEntry = resolveCurrencyRateEntry(currencyRates, ETH_TICKER); | ||
| if (rateEntry !== undefined && rateEntry.usdConversionRate !== 0) { | ||
| const tokenToEthPricePeg = 1 / rateEntry.usdConversionRate; |
There was a problem hiding this comment.
Null usdConversionRate passes guard, causes Infinity
Medium Severity
The guard rateEntry.usdConversionRate !== 0 does not catch null, which is a valid value in CurrencyRateState. When usdConversionRate is null, 1 / null evaluates to Infinity, producing tokenToEthPricePeg = Infinity. This is then passed to balanceToFiatNumber, which would yield an infinite fiat value displayed to the user. Previously this code was in a fallback branch (only hit when market data was missing), but the reordering makes it the primary path for all mUSD transactions, significantly increasing the likelihood of hitting this edge case in production.
Reviewed by Cursor Bugbot for commit e6d58c3. Configure here.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Risk factors:
Tag selection rationale:
Performance tests: No performance impact expected - changes are UI display logic and transaction filtering within the Money feature, not affecting rendering performance of account lists, login, or other performance-sensitive flows. Performance Test Selection: |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 4 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7e966e7. Configure here.
| @@ -0,0 +1,96 @@ | |||
| import React from 'react'; | |||
| import { ScrollView } from 'react-native'; | |||
There was a problem hiding this comment.
ScrollView from react-native inside BottomSheet breaks scrolling
Medium Severity
ScrollView is imported from react-native in MoneyReceivedDetails, but this component is rendered inside a BottomSheet (in MoneyTransactionDetailsSheet). The gesture-handler version is required for scroll/gesture coordination inside bottom sheets — without it, scroll gestures conflict with the sheet's pan gesture, causing broken scrolling or accidental sheet dismissal. The import needs to come from react-native-gesture-handler instead.
Additional Locations (1)
Triggered by learned rule: ScrollView inside BottomSheet must be imported from react-native-gesture-handler
Reviewed by Cursor Bugbot for commit 7e966e7. Configure here.
| // `0x` + 8 hex chars selector + 64 hex chars (address) + 64 hex chars (uint256). | ||
| const ERC20_TRANSFER_CALLDATA_LENGTH = 138; | ||
| // `0x` + 8 hex chars selector + 3 × 64 hex chars (from, to, uint256). | ||
| const ERC20_TRANSFER_FROM_CALLDATA_LENGTH = 202; |
There was a problem hiding this comment.
Duplicate ERC20 calldata constants across two files
Low Severity
ERC20_TRANSFER_CALLDATA_LENGTH and ERC20_TRANSFER_FROM_CALLDATA_LENGTH are defined identically in both useMoneyAccountTransactions.ts and activityStyles.ts. Duplicating these magic-number constants increases maintenance burden — if the value changes in one file, the other risks becoming silently out of sync. These could be defined once and shared.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7e966e7. Configure here.
|
|
||
| it('renders the fiat hero when buildMoneyActivityFiatLine returns a value', () => { | ||
| const { queryByTestId } = render(); | ||
| expect(queryByTestId('money-received-hero')).toBeTruthy(); |
There was a problem hiding this comment.
Tests use toBeTruthy instead of toBeOnTheScreen
Low Severity
Several new test assertions use toBeTruthy() to verify element presence (e.g., expect(getByText('Token received')).toBeTruthy() and expect(getByTestId('money-received-details')).toBeTruthy()). The unit testing guidelines mandate toBeOnTheScreen() for element presence checks — toBeTruthy() is listed as a banned weak matcher for this purpose.
Additional Locations (1)
Triggered by project rule: Unit Testing Guidelines
Reviewed by Cursor Bugbot for commit 7e966e7. Configure here.
|
1 participant
Description
This PR updates the money activity list to show transfers into the money account as well as deposits.
Changelog
CHANGELOG entry: show transfers in the money account account activity list
Related issues
Fixes: MUSD-816
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Performance checks (if applicable)
trace()for usage andaddTokenfor an exampleFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist
Note
Medium Risk
Changes which transactions appear in Money activity and how mUSD amounts/fiat are computed; scoped to Monad and covered by broad tests, but incorrect filtering or peg logic could mislead users about balances.
Overview
Money Account activity now surfaces inbound mUSD on Monad—both polled
incomingtransfers to the Money address and locally signedtokenMethodTransfer/tokenMethodTransferFromwhen calldata’s recipient matches—while excluding the same token on Mainnet/Linea/BSC and non–Money-account recipients. List rows use a “Received” label/icon (vs “Deposited” formoneyAccountDeposit), and the details sheet uses a dedicatedMoneyReceivedDetailsview (fiat hero, From, token received) for those types.Classification & amounts: New helpers (
isMusdTokenOnChain,isMusdOnMoneyAccountChain,isMusdErc20Transfer) scope mUSD to Money Account chains;resolveMusdTransferMetafills amounts from ERC-20 calldata viaBigIntwhentransferInformationis missing. Fiat lines always use the 1:1 USD peg for mUSD, ignoring erroneous market prices. Phantom rows are suppressed by only showing confirmed / submitted / failed statuses for the new inbound paths.Reviewed by Cursor Bugbot for commit 7e966e7. Bugbot is set up for automated code reviews on this repo. Configure here.