Repository navigation
fix(send): populate amount field for fixed-amount LNURL - #671
Merged
Merged
Conversation
When an LNURL-pay endpoint resolves to a fixed amount (minSendable === maxSendable), route the auto-filled value through setState so the amount input (bound to amountTextValue) reflects it. Previously the field was set via setSendInfo/setAmount, which never synced the text value and left the locked field blank, producing an unpayable invoice. Adds a regression test covering a fixed-amount LNURL (21000 msat -> 21 sats) that asserts the read-only amount input is populated.
Contributor
WalkthroughThe PR fixes LNURL fixed-amount handling in ChangesLNURL Fixed-Amount Synchronization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 wallet-bitcoin with
|
| Latest commit: |
613b960
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e702eaaa.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://fix-lnurl-fixed-amount-blank.wallet-bitcoin.pages.dev |
Deploying tmp-boltz-upstream-mainnet-arkade-wallet with
|
| Latest commit: |
613b960
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://aed950ab.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
| Branch Preview URL: | https://fix-lnurl-fixed-amount-blank.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
613b960
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2341e11b.arkade-wallet.pages.dev |
| Branch Preview URL: | https://fix-lnurl-fixed-amount-blank.arkade-wallet.pages.dev |
ghost
approved these changes
Jun 13, 2026
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Review: fix(send): populate amount field for fixed-amount LNURL
Verdict: APPROVE ✅
Analysis
Root cause is correctly identified. Two effects manage fixed-amount LNURL state:
- "check lnurl conditions" (Form.tsx ~L410): fetches LNURL pay endpoint, and if
min === max, setssendInfo.satoshis - "check lnurl limits" (Form.tsx ~L393): fires when
lnUrlResponsechanges, setsamountIsReadOnly
The old code called setSendInfo() directly, which updates context state but never touches amountTextValue — the string bound to the <input>. Only the local setState() helper (L170–183) syncs both. Classic React "two sources of truth" bug.
The fix routes through setState() instead of setSendInfo(). This is correct — setState calls setSendInfo internally AND sets amountTextValue via the info.satoshis branch (L179).
What I checked
setAmount(min)removal (L402): Safe. Theamountstate is only read at L675 for keyboard focus heuristics (handleEnter). For fixed-amount LNURL the field is readonly so this path is irrelevant.setStateside effects:setStatealso callssetScan(false)(L171). Harmless — scan is already false when LNURL conditions resolve.- Stale closure on
sendInfo: The.then()callback at L419 capturessendInfofrom render time. If the user mutates state while the fetch is in-flight, the spread could overwrite changes. This is a pre-existing issue — the oldsetSendInfo({...sendInfo, satoshis: min})had the identical closure problem. Not introduced by this PR. - Zero-amount edge case: If an LNURL returns
minSendable === maxSendable === 0,setStatewon't updateamountTextValuebecauseif (info.satoshis)is falsy for 0. Pre-existing, and 0-sat LNURL is invalid per spec (LUD-06 requiresminSendable >= 1). - Effect ordering: Conditions effect calls
setStatethensetLnUrlResponse. React batches these. The limits effect fires next render, seesmin === max, setsamountIsReadOnly(true). Correct sequence. - No protocol-critical paths touched: This is pure UI state management — no VTXO handling, signing, forfeit, or exit paths.
Test
The regression test is well-constructed:
- Mocks
fetchwith a fixed-amount LNURL response (minSendable === maxSendable === 21000 msat) - Pre-populates
sendInfo.lnUrlandsendInfo.recipientto trigger the conditions effect - Asserts
getByDisplayValue('21')+readonlyattribute - Properly cleans up with
fetchMocker.disableMocks()
Clean fix. No issues found.
🤖 Reviewed by Arkana
ghost
approved these changes
Jun 13, 2026
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Review: fix(send): populate amount field for fixed-amount LNURL
Verdict: APPROVE ✅
Analysis
Root cause is correctly identified. Two effects manage fixed-amount LNURL state:
- "check lnurl conditions" (Form.tsx ~L410): fetches LNURL pay endpoint, and if
min === max, setssendInfo.satoshis - "check lnurl limits" (Form.tsx ~L393): fires when
lnUrlResponsechanges, setsamountIsReadOnly
The old code called setSendInfo() directly, which updates context state but never touches amountTextValue — the string bound to the <input>. Only the local setState() helper (L170–183) syncs both. Classic React "two sources of truth" bug.
The fix routes through setState() instead of setSendInfo(). This is correct — setState calls setSendInfo internally AND sets amountTextValue via the info.satoshis branch (L179).
What I checked
setAmount(min)removal (L402): Safe. Theamountstate is only read at L675 for keyboard focus heuristics (handleEnter). For fixed-amount LNURL the field is readonly so this path is irrelevant.setStateside effects:setStatealso callssetScan(false)(L171). Harmless — scan is already false when LNURL conditions resolve.- Stale closure on
sendInfo: The.then()callback at L419 capturessendInfofrom render time. If the user mutates state while the fetch is in-flight, the spread could overwrite changes. This is a pre-existing issue — the oldsetSendInfo({...sendInfo, satoshis: min})had the identical closure problem. Not introduced by this PR. - Zero-amount edge case: If an LNURL returns
minSendable === maxSendable === 0,setStatewon't updateamountTextValuebecauseif (info.satoshis)is falsy for 0. Pre-existing, and 0-sat LNURL is invalid per spec (LUD-06 requiresminSendable >= 1). - Effect ordering: Conditions effect calls
setStatethensetLnUrlResponse. React batches these. The limits effect fires next render, seesmin === max, setsamountIsReadOnly(true). Correct sequence. - No protocol-critical paths touched: This is pure UI state management — no VTXO handling, signing, forfeit, or exit paths.
Test
The regression test is well-constructed:
- Mocks
fetchwith a fixed-amount LNURL response (minSendable === maxSendable === 21000 msat) - Pre-populates
sendInfo.lnUrlandsendInfo.recipientto trigger the conditions effect - Asserts
getByDisplayValue('21')+readonlyattribute - Properly cleans up with
fetchMocker.disableMocks()
Clean fix. No issues found.
🤖 Reviewed by Arkana
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/Wallet/Send/Form.tsx (1)
366-380:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard async LNURL resolution before applying fixed-amount state.
This effect can apply a stale
sendInfosnapshot if an earlier LNURL request resolves late, and Line 373 then overwrites newer form state viasetState({ ...sendInfo, satoshis: min }).Suggested fix
useEffect(() => { if (!sendInfo.lnUrl) return if (sendInfo.arkAddress) return if (sendInfo.lnUrl && sendInfo.invoice) return - checkLnUrlConditions(sendInfo.lnUrl) + let cancelled = false + const requestedLnUrl = sendInfo.lnUrl + checkLnUrlConditions(requestedLnUrl) .then((conditions) => { + if (cancelled) return if (!conditions) return setError('Unable to fetch LNURL conditions') const min = Math.floor(conditions.minSendable / 1000) // from millisatoshis to satoshis const max = Math.floor(conditions.maxSendable / 1000) // from millisatoshis to satoshis // when the LNURL resolves to a fixed amount, set it via setState so the // amount input (bound to amountTextValue) reflects it instead of staying blank if (min === max) setState({ ...sendInfo, satoshis: min }) return setLnUrlResponse({ ...conditions, minSendable: min, maxSendable: max }) }) .catch((e) => { + if (cancelled) return consoleError(e, 'Error checking LNURL conditions') setError(extractError(e)) }) - }, [sendInfo.arkAddress, sendInfo.lnUrl]) + return () => { + cancelled = true + } + }, [sendInfo.arkAddress, sendInfo.lnUrl, sendInfo.invoice])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/Wallet/Send/Form.tsx` around lines 366 - 380, The effect may apply a stale sendInfo when an earlier checkLnUrlConditions resolves; capture the current lnUrl into a local constant before calling checkLnUrlConditions (e.g., const requestedLnUrl = sendInfo.lnUrl) and in the .then() handler verify that the component's current lnUrl still equals requestedLnUrl before calling setState({ ...sendInfo, satoshis: min }) or setLnUrlResponse; additionally, replace setState({ ...sendInfo, satoshis: min }) with the functional updater form setState(prev => ({ ...prev, satoshis: min })) to avoid using a stale sendInfo snapshot if you prefer not to compare strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/screens/wallet/send.test.tsx`:
- Around line 86-134: The test sets up fetch mocks with createFetchMock and
calls fetchMocker.enableMocks()/mockResponseOnce but calls
fetchMocker.disableMocks() only at the end, which can leak if an assertion
throws; wrap the portion between fetchMocker.enableMocks() and
fetchMocker.disableMocks() in a try/finally so fetchMocker.disableMocks() always
runs (i.e., call fetchMocker.enableMocks(), then try { ...render and assertions
involving SendForm and await waitFor(...) } finally { fetchMocker.disableMocks()
}) to ensure mocks are restored even on failure.
---
Outside diff comments:
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 366-380: The effect may apply a stale sendInfo when an earlier
checkLnUrlConditions resolves; capture the current lnUrl into a local constant
before calling checkLnUrlConditions (e.g., const requestedLnUrl =
sendInfo.lnUrl) and in the .then() handler verify that the component's current
lnUrl still equals requestedLnUrl before calling setState({ ...sendInfo,
satoshis: min }) or setLnUrlResponse; additionally, replace setState({
...sendInfo, satoshis: min }) with the functional updater form setState(prev =>
({ ...prev, satoshis: min })) to avoid using a stale sendInfo snapshot if you
prefer not to compare strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d144435-be5b-4a50-b79d-01981ee517a9
📒 Files selected for processing (2)
src/screens/Wallet/Send/Form.tsxsrc/test/screens/wallet/send.test.tsx
Comment on lines
+86
to
+134
| const fetchMocker = createFetchMock(vi) | ||
| fetchMocker.enableMocks() | ||
| fetchMocker.mockResponseOnce( | ||
| JSON.stringify({ | ||
| callback: 'https://pay.staging.galoy.io/.well-known/lnurlp/testing', | ||
| minSendable: 21000, // millisatoshis -> 21 sats | ||
| maxSendable: 21000, | ||
| metadata: 'mock-metadata', | ||
| }), | ||
| ) | ||
| const lnUrl = 'lnurl1dp68gurn8ghj7urp0yh8xarpva5kueewvaskcmme9e5k7tewwajkcmpdddhx7amw9akxuatjd3cz7ar9wd6xjmn8h9qlv7' | ||
| const flowValue = { ...mockFlowContextValue, sendInfo: { ...emptySendInfo, lnUrl, recipient: lnUrl } } | ||
| const walletValue = { | ||
| ...mockWalletContextValue, | ||
| balance: 1_000_000, | ||
| svcWallet: { | ||
| ...mockSvcWallet, | ||
| getAddress: () => 'tark1mockoffchain', | ||
| getBoardingAddress: () => Promise.resolve('bcrt1mockboarding'), | ||
| getBalance: () => Promise.resolve({ available: 1_000_000 }), | ||
| } as any, | ||
| } | ||
| render( | ||
| <NavigationContext.Provider value={mockNavigationContextValue}> | ||
| <AspContext.Provider value={mockAspContextValue}> | ||
| <ConfigContext.Provider value={mockConfigContextValue as any}> | ||
| <FiatContext.Provider value={mockFiatContextValue as any}> | ||
| <SwapsContext.Provider value={mockSwapsContextValue as any}> | ||
| <OptionsContext.Provider value={mockOptionsContextValue as any}> | ||
| <FlowContext.Provider value={flowValue as any}> | ||
| <WalletContext.Provider value={walletValue}> | ||
| <LimitsContext.Provider value={mockLimitsContextValue}> | ||
| <SendForm /> | ||
| </LimitsContext.Provider> | ||
| </WalletContext.Provider> | ||
| </FlowContext.Provider> | ||
| </OptionsContext.Provider> | ||
| </SwapsContext.Provider> | ||
| </FiatContext.Provider> | ||
| </ConfigContext.Provider> | ||
| </AspContext.Provider> | ||
| </NavigationContext.Provider>, | ||
| ) | ||
| // amount input is bound to amountTextValue; before the fix it stayed empty | ||
| const amountInput = await waitFor(() => screen.getByDisplayValue('21')) | ||
| expect(amountInput).toHaveAttribute('name', 'send-amount') | ||
| expect(amountInput).toHaveAttribute('readonly') | ||
| fetchMocker.disableMocks() | ||
| }) |
Contributor
There was a problem hiding this comment.
Always restore fetch mocks with try/finally in this test.
If an assertion throws before Line 133, mocked fetch can leak into later tests and create cascading failures.
Suggested fix
const fetchMocker = createFetchMock(vi)
fetchMocker.enableMocks()
- fetchMocker.mockResponseOnce(
- JSON.stringify({
- callback: 'https://pay.staging.galoy.io/.well-known/lnurlp/testing',
- minSendable: 21000, // millisatoshis -> 21 sats
- maxSendable: 21000,
- metadata: 'mock-metadata',
- }),
- )
- const lnUrl = 'lnurl1dp68gurn8ghj7urp0yh8xarpva5kueewvaskcmme9e5k7tewwajkcmpdddhx7amw9akxuatjd3cz7ar9wd6xjmn8h9qlv7'
- const flowValue = { ...mockFlowContextValue, sendInfo: { ...emptySendInfo, lnUrl, recipient: lnUrl } }
- const walletValue = {
- ...mockWalletContextValue,
- balance: 1_000_000,
- svcWallet: {
- ...mockSvcWallet,
- getAddress: () => 'tark1mockoffchain',
- getBoardingAddress: () => Promise.resolve('bcrt1mockboarding'),
- getBalance: () => Promise.resolve({ available: 1_000_000 }),
- } as any,
- }
- render(
- <NavigationContext.Provider value={mockNavigationContextValue}>
- <AspContext.Provider value={mockAspContextValue}>
- <ConfigContext.Provider value={mockConfigContextValue as any}>
- <FiatContext.Provider value={mockFiatContextValue as any}>
- <SwapsContext.Provider value={mockSwapsContextValue as any}>
- <OptionsContext.Provider value={mockOptionsContextValue as any}>
- <FlowContext.Provider value={flowValue as any}>
- <WalletContext.Provider value={walletValue}>
- <LimitsContext.Provider value={mockLimitsContextValue}>
- <SendForm />
- </LimitsContext.Provider>
- </WalletContext.Provider>
- </FlowContext.Provider>
- </OptionsContext.Provider>
- </SwapsContext.Provider>
- </FiatContext.Provider>
- </ConfigContext.Provider>
- </AspContext.Provider>
- </NavigationContext.Provider>,
- )
- // amount input is bound to amountTextValue; before the fix it stayed empty
- const amountInput = await waitFor(() => screen.getByDisplayValue('21'))
- expect(amountInput).toHaveAttribute('name', 'send-amount')
- expect(amountInput).toHaveAttribute('readonly')
- fetchMocker.disableMocks()
+ try {
+ fetchMocker.mockResponseOnce(
+ JSON.stringify({
+ callback: 'https://pay.staging.galoy.io/.well-known/lnurlp/testing',
+ minSendable: 21000, // millisatoshis -> 21 sats
+ maxSendable: 21000,
+ metadata: 'mock-metadata',
+ }),
+ )
+ const lnUrl =
+ 'lnurl1dp68gurn8ghj7urp0yh8xarpva5kueewvaskcmme9e5k7tewwajkcmpdddhx7amw9akxuatjd3cz7ar9wd6xjmn8h9qlv7'
+ const flowValue = { ...mockFlowContextValue, sendInfo: { ...emptySendInfo, lnUrl, recipient: lnUrl } }
+ const walletValue = {
+ ...mockWalletContextValue,
+ balance: 1_000_000,
+ svcWallet: {
+ ...mockSvcWallet,
+ getAddress: () => 'tark1mockoffchain',
+ getBoardingAddress: () => Promise.resolve('bcrt1mockboarding'),
+ getBalance: () => Promise.resolve({ available: 1_000_000 }),
+ } as any,
+ }
+ render(
+ <NavigationContext.Provider value={mockNavigationContextValue}>
+ <AspContext.Provider value={mockAspContextValue}>
+ <ConfigContext.Provider value={mockConfigContextValue as any}>
+ <FiatContext.Provider value={mockFiatContextValue as any}>
+ <SwapsContext.Provider value={mockSwapsContextValue as any}>
+ <OptionsContext.Provider value={mockOptionsContextValue as any}>
+ <FlowContext.Provider value={flowValue as any}>
+ <WalletContext.Provider value={walletValue}>
+ <LimitsContext.Provider value={mockLimitsContextValue}>
+ <SendForm />
+ </LimitsContext.Provider>
+ </WalletContext.Provider>
+ </FlowContext.Provider>
+ </OptionsContext.Provider>
+ </SwapsContext.Provider>
+ </FiatContext.Provider>
+ </ConfigContext.Provider>
+ </AspContext.Provider>
+ </NavigationContext.Provider>,
+ )
+ const amountInput = await waitFor(() => screen.getByDisplayValue('21'))
+ expect(amountInput).toHaveAttribute('name', 'send-amount')
+ expect(amountInput).toHaveAttribute('readonly')
+ } finally {
+ fetchMocker.disableMocks()
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const fetchMocker = createFetchMock(vi) | |
| fetchMocker.enableMocks() | |
| fetchMocker.mockResponseOnce( | |
| JSON.stringify({ | |
| callback: 'https://pay.staging.galoy.io/.well-known/lnurlp/testing', | |
| minSendable: 21000, // millisatoshis -> 21 sats | |
| maxSendable: 21000, | |
| metadata: 'mock-metadata', | |
| }), | |
| ) | |
| const lnUrl = 'lnurl1dp68gurn8ghj7urp0yh8xarpva5kueewvaskcmme9e5k7tewwajkcmpdddhx7amw9akxuatjd3cz7ar9wd6xjmn8h9qlv7' | |
| const flowValue = { ...mockFlowContextValue, sendInfo: { ...emptySendInfo, lnUrl, recipient: lnUrl } } | |
| const walletValue = { | |
| ...mockWalletContextValue, | |
| balance: 1_000_000, | |
| svcWallet: { | |
| ...mockSvcWallet, | |
| getAddress: () => 'tark1mockoffchain', | |
| getBoardingAddress: () => Promise.resolve('bcrt1mockboarding'), | |
| getBalance: () => Promise.resolve({ available: 1_000_000 }), | |
| } as any, | |
| } | |
| render( | |
| <NavigationContext.Provider value={mockNavigationContextValue}> | |
| <AspContext.Provider value={mockAspContextValue}> | |
| <ConfigContext.Provider value={mockConfigContextValue as any}> | |
| <FiatContext.Provider value={mockFiatContextValue as any}> | |
| <SwapsContext.Provider value={mockSwapsContextValue as any}> | |
| <OptionsContext.Provider value={mockOptionsContextValue as any}> | |
| <FlowContext.Provider value={flowValue as any}> | |
| <WalletContext.Provider value={walletValue}> | |
| <LimitsContext.Provider value={mockLimitsContextValue}> | |
| <SendForm /> | |
| </LimitsContext.Provider> | |
| </WalletContext.Provider> | |
| </FlowContext.Provider> | |
| </OptionsContext.Provider> | |
| </SwapsContext.Provider> | |
| </FiatContext.Provider> | |
| </ConfigContext.Provider> | |
| </AspContext.Provider> | |
| </NavigationContext.Provider>, | |
| ) | |
| // amount input is bound to amountTextValue; before the fix it stayed empty | |
| const amountInput = await waitFor(() => screen.getByDisplayValue('21')) | |
| expect(amountInput).toHaveAttribute('name', 'send-amount') | |
| expect(amountInput).toHaveAttribute('readonly') | |
| fetchMocker.disableMocks() | |
| }) | |
| const fetchMocker = createFetchMock(vi) | |
| fetchMocker.enableMocks() | |
| try { | |
| fetchMocker.mockResponseOnce( | |
| JSON.stringify({ | |
| callback: 'https://pay.staging.galoy.io/.well-known/lnurlp/testing', | |
| minSendable: 21000, // millisatoshis -> 21 sats | |
| maxSendable: 21000, | |
| metadata: 'mock-metadata', | |
| }), | |
| ) | |
| const lnUrl = | |
| 'lnurl1dp68gurn8ghj7urp0yh8xarpva5kueewvaskcmme9e5k7tewwajkcmpdddhx7amw9akxuatjd3cz7ar9wd6xjmn8h9qlv7' | |
| const flowValue = { ...mockFlowContextValue, sendInfo: { ...emptySendInfo, lnUrl, recipient: lnUrl } } | |
| const walletValue = { | |
| ...mockWalletContextValue, | |
| balance: 1_000_000, | |
| svcWallet: { | |
| ...mockSvcWallet, | |
| getAddress: () => 'tark1mockoffchain', | |
| getBoardingAddress: () => Promise.resolve('bcrt1mockboarding'), | |
| getBalance: () => Promise.resolve({ available: 1_000_000 }), | |
| } as any, | |
| } | |
| render( | |
| <NavigationContext.Provider value={mockNavigationContextValue}> | |
| <AspContext.Provider value={mockAspContextValue}> | |
| <ConfigContext.Provider value={mockConfigContextValue as any}> | |
| <FiatContext.Provider value={mockFiatContextValue as any}> | |
| <SwapsContext.Provider value={mockSwapsContextValue as any}> | |
| <OptionsContext.Provider value={mockOptionsContextValue as any}> | |
| <FlowContext.Provider value={flowValue as any}> | |
| <WalletContext.Provider value={walletValue}> | |
| <LimitsContext.Provider value={mockLimitsContextValue}> | |
| <SendForm /> | |
| </LimitsContext.Provider> | |
| </WalletContext.Provider> | |
| </FlowContext.Provider> | |
| </OptionsContext.Provider> | |
| </SwapsContext.Provider> | |
| </FiatContext.Provider> | |
| </ConfigContext.Provider> | |
| </AspContext.Provider> | |
| </NavigationContext.Provider>, | |
| ) | |
| // amount input is bound to amountTextValue; before the fix it stayed empty | |
| const amountInput = await waitFor(() => screen.getByDisplayValue('21')) | |
| expect(amountInput).toHaveAttribute('name', 'send-amount') | |
| expect(amountInput).toHaveAttribute('readonly') | |
| } finally { | |
| fetchMocker.disableMocks() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/screens/wallet/send.test.tsx` around lines 86 - 134, The test sets
up fetch mocks with createFetchMock and calls
fetchMocker.enableMocks()/mockResponseOnce but calls fetchMocker.disableMocks()
only at the end, which can leak if an assertion throws; wrap the portion between
fetchMocker.enableMocks() and fetchMocker.disableMocks() in a try/finally so
fetchMocker.disableMocks() always runs (i.e., call fetchMocker.enableMocks(),
then try { ...render and assertions involving SendForm and await waitFor(...) }
finally { fetchMocker.disableMocks() }) to ensure mocks are restored even on
failure.
ghost
mentioned this pull request
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
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.
Problem
When an LNURL-pay endpoint resolves to a fixed amount (
minSendable === maxSendable), the Send form locked the amount input but left it blank. The user saw an empty, read-only field and any attempt to continue produced an unpayable invoice.Cause
The fixed amount was applied via
setSendInfo/setAmount, neither of which syncsamountTextValue— the state the amount<input>is actually bound to. So the value never reached the field.Fix
Route the auto-filled fixed amount through
setState({ ...sendInfo, satoshis: min }), which updatesamountTextValueand makes the locked field display the correct value. Removed the deadsetAmount(min)call in the read-only branch.Test
Adds a regression test that mocks a fixed-amount LNURL response (
minSendable === maxSendable === 21000msat → 21 sats) and asserts the amount input is populated with21and read-only.pnpm test— 268 passed / 4 skippedpnpm lint,pnpm format:check— cleanSummary by CodeRabbit
Bug Fixes
Tests