Skip to content

Commit 6845892

Browse files
authored
Merge pull request #18 from crackedstudio/samuel1-ona/nim-payment-fails-without-warning
fix(buy): stop the NIM balance check blocking a player who can pay
2 parents 470cfee + 7d0e5e7 commit 6845892

3 files changed

Lines changed: 134 additions & 24 deletions

File tree

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { describe, it, expect } from 'vitest'
2+
3+
/**
4+
* The shortfall calculation, extracted as the pure decision it is.
5+
*
6+
* This exists because the first version of the check shipped a false negative
7+
* that stopped a real player buying: it read `listAccounts()[0]`, found an
8+
* empty account, and reported that somebody holding 4,937 NIM elsewhere had
9+
* nothing. The rules that prevent it coming back are pinned here.
10+
*/
11+
12+
/** Mirrors the reduction in `useNimPayment.checkBalance`. */
13+
function shortfallFrom(balances: (bigint | null)[], requiredLuna: bigint): bigint | null {
14+
const known = balances.filter((b): b is bigint => b !== null)
15+
if (known.length === 0) return null
16+
const richest = known.reduce((a, b) => (b > a ? b : a), 0n)
17+
return richest >= requiredLuna ? 0n : requiredLuna - richest
18+
}
19+
20+
/** 58.79 NIM — the quote from the report that exposed the bug. */
21+
const REQUIRED = 5_879_000n
22+
/** 4,936.65 NIM, the balance that was wrongly read as zero. */
23+
const FUNDED = 493_665_000n
24+
25+
describe('shortfall across a wallet with several accounts', () => {
26+
// The regression. The funded account is not first, which is precisely the
27+
// shape that produced "NOT ENOUGH NIM" for a wallet holding 84x the price.
28+
it('finds the money when the funded account is not the first one', () => {
29+
expect(shortfallFrom([0n, 0n, FUNDED], REQUIRED)).toBe(0n)
30+
})
31+
32+
it('is not fooled by the first account being empty', () => {
33+
expect(shortfallFrom([0n, FUNDED], REQUIRED)).toBe(0n)
34+
})
35+
36+
// A Nimiq transaction is funded by ONE address, so two accounts that each
37+
// hold half the price cannot pay it. Summing would wrongly say they can.
38+
it('does not add accounts together', () => {
39+
const half = REQUIRED / 2n
40+
expect(shortfallFrom([half, half], REQUIRED)).toBe(REQUIRED - half)
41+
})
42+
43+
it('reports the gap against the richest account, not the poorest', () => {
44+
expect(shortfallFrom([0n, 1_000_000n], REQUIRED)).toBe(REQUIRED - 1_000_000n)
45+
})
46+
47+
/* ---- "cannot tell" must never read as "has nothing" ------------------ */
48+
49+
it('returns null when every lookup failed', () => {
50+
expect(shortfallFrom([null, null], REQUIRED)).toBeNull()
51+
})
52+
53+
it('returns null when the wallet reported no accounts at all', () => {
54+
expect(shortfallFrom([], REQUIRED)).toBeNull()
55+
})
56+
57+
// A partial failure still answers from what is known, and the known account
58+
// covering the price is enough — the unknown one cannot make that false.
59+
it('answers from the accounts it could read', () => {
60+
expect(shortfallFrom([null, FUNDED], REQUIRED)).toBe(0n)
61+
})
62+
63+
// Control: the genuinely-broke wallet still reports a shortfall, so the
64+
// assertions above are the max working rather than the check being inert.
65+
it('control: a wallet that really is empty still reports the full amount', () => {
66+
expect(shortfallFrom([0n, 0n], REQUIRED)).toBe(REQUIRED)
67+
})
68+
69+
it('treats exactly enough as enough', () => {
70+
expect(shortfallFrom([REQUIRED], REQUIRED)).toBe(0n)
71+
})
72+
73+
it('reports one Luna short as one Luna short', () => {
74+
expect(shortfallFrom([REQUIRED - 1n], REQUIRED)).toBe(1n)
75+
})
76+
})

apps/web/src/components/Overlays/NimPayPanel.tsx

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -155,21 +155,23 @@ export default function NimPayPanel({
155155
type="button"
156156
className="pixel-btn pixel-btn-sm"
157157
style={{ width: '100%', minHeight: 44, fontSize: 10, justifyContent: 'center' }}
158-
// `short` blocks only when the balance was actually read. A null
159-
// shortfall is "we could not tell", which must never stop a player
160-
// paying — the wallet is the authority on what they can afford.
161-
disabled={busy || !recipient || short}
158+
// Deliberately NOT disabled on a suspected shortfall. This check
159+
// reads the accounts the wallet chose to report, at one moment,
160+
// from one node — the wallet decides which account funds a payment
161+
// and is the only authority on whether it can. An earlier version
162+
// disabled the button here and stopped a player with 4,937 NIM from
163+
// paying. A warning costs a wasted tap when it is right; a block
164+
// costs a sale every time it is wrong.
165+
disabled={busy || !recipient}
162166
onClick={() => (quote ? void payAndSettle() : void getQuote(pixelIds))}
163167
>
164168
{busy
165169
? 'WORKING…'
166-
: short
167-
? 'NOT ENOUGH NIM'
168-
: quote
169-
? 'PAY WITH NIM'
170-
: recipient
171-
? 'GET NIM PRICE'
172-
: 'CONNECT WALLET FIRST'}
170+
: quote
171+
? 'PAY WITH NIM'
172+
: recipient
173+
? 'GET NIM PRICE'
174+
: 'CONNECT WALLET FIRST'}
173175
</button>
174176
</>
175177
)}
@@ -189,7 +191,7 @@ export default function NimPayPanel({
189191
{error ??
190192
progress ??
191193
(short
192-
? `You need ${formatNim(shortfall!)} more NIM. Top up, or pay with USDC instead.`
194+
? `Looks like you may be about ${formatNim(shortfall!)} NIM short — try anyway, or pay with USDC.`
193195
: quote
194196
? supportedHost === 'pay'
195197
? 'One confirmation in Nimiq Pay.'

apps/web/src/hooks/useNimPayment.ts

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ export interface NimQuote {
5252
expiresAt: number
5353
}
5454

55+
/**
56+
* Most accounts to price-check before giving up. A wallet with more than this
57+
* is unusual, and the check is advisory — a bound matters more than covering
58+
* every last account.
59+
*/
60+
const MAX_ACCOUNTS_CHECKED = 8
61+
5562
const POLL_MS = 4_000
5663
const MAX_POLLS = 45 // ~3 minutes
5764

@@ -80,23 +87,48 @@ export function useNimPayment(mapId: MapId, recipient: string | undefined) {
8087
/**
8188
* How much more NIM the player needs, or null when we cannot tell.
8289
*
83-
* Never throws and never changes `status`: this is decoration on a path that
84-
* has to keep working without it. Outside Nimiq Pay there is no provider to
85-
* ask, and a declined prompt resolves as an error envelope rather than a
86-
* rejection — both end here as "cannot tell", which renders as nothing.
90+
* Reads EVERY address the wallet reports, not the first one. A Nimiq Pay user
91+
* can hold several accounts, `listAccounts()` returns them all, and the order
92+
* says nothing about which one holds the money or which the wallet will spend
93+
* from. An earlier version of this took `[0]` and told a player with 4,937 NIM
94+
* across other accounts that they had none — the exact false negative this
95+
* comment exists to stop coming back.
96+
*
97+
* A Nimiq transaction is funded by ONE address, so the test is whether the
98+
* LARGEST single balance covers the amount. Summing would claim a player can
99+
* pay when no single account of theirs can.
100+
*
101+
* Even then the answer is advisory. The wallet chooses the sending account
102+
* and is the only authority on what a payment can do; this can be stale, can
103+
* miss an account the wallet knows about, and must therefore never be allowed
104+
* to stop somebody paying. It renders a warning and nothing more.
105+
*
106+
* Never throws and never changes `status`. Outside Nimiq Pay there is no
107+
* provider to ask, and a declined prompt resolves as an error envelope rather
108+
* than a rejection — both end here as "cannot tell", which renders as nothing.
87109
*/
88110
const checkBalance = useCallback(async (requiredLuna: bigint) => {
89111
setShortfall(null)
90112
if (!isNimiqPay()) return
91113
try {
92-
const [address] = await listNimiqAccounts()
93-
if (!address) return
94-
const res = await fetch(`/api/nim/balance?address=${encodeURIComponent(address)}`)
95-
if (!res.ok) return
96-
const { luna } = (await res.json()) as { luna?: string }
97-
if (typeof luna !== 'string' || !/^\d+$/.test(luna)) return
98-
const held = BigInt(luna)
99-
setShortfall(held >= requiredLuna ? 0n : requiredLuna - held)
114+
const addresses = (await listNimiqAccounts()).slice(0, MAX_ACCOUNTS_CHECKED)
115+
if (addresses.length === 0) return
116+
117+
const balances = await Promise.all(
118+
addresses.map(async (address) => {
119+
const res = await fetch(`/api/nim/balance?address=${encodeURIComponent(address)}`)
120+
if (!res.ok) return null
121+
const { luna } = (await res.json()) as { luna?: string }
122+
return typeof luna === 'string' && /^\d+$/.test(luna) ? BigInt(luna) : null
123+
}),
124+
)
125+
126+
const known = balances.filter((b): b is bigint => b !== null)
127+
// Every lookup failed: that is "cannot tell", not "has nothing".
128+
if (known.length === 0) return
129+
130+
const richest = known.reduce((a, b) => (b > a ? b : a), 0n)
131+
setShortfall(richest >= requiredLuna ? 0n : requiredLuna - richest)
100132
} catch {
101133
// Declined, offline, or a node that would not answer. Say nothing.
102134
}

0 commit comments

Comments
 (0)