Skip to content

Don't prevent detection when paying to self #625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chedieck merged 1 commit into from
Feb 20, 2026
Merged

Don't prevent detection when paying to self #625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions react/lib/tests/util/validate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ describe('Validate Util Tests', () => {
expect(result).toBe(false);
});

it('false when amount does not match received amount', async () => {
it('true when received amount is equal or greater than expected', async () => {
const transaction: Transaction = {
amount: '101.00',
paymentId: '123',
Expand All @@ -140,7 +140,52 @@ describe('Validate Util Tests', () => {
address: 'ecash:qrmm7edwuj4jf7tnvygjyztyy0a0qxvl7quss2vxek',
};
const expectedPaymentId = '123';
const expectedAmount = resolveNumber('100.00');
let expectedAmount = resolveNumber('100.00');
const expectedOpReturn = 'test opReturn message';
const price = 1;
const currency = 'XEC';

let result1 = shouldTriggerOnSuccess(
transaction,
currency,
price,
false,
false,
expectedPaymentId,
expectedAmount,
expectedOpReturn
);

expect(result1).toBe(true);

expectedAmount = resolveNumber('101.00');

let result2 = shouldTriggerOnSuccess(
transaction,
currency,
price,
false,
false,
expectedPaymentId,
expectedAmount,
expectedOpReturn
);

expect(result2).toBe(true);
});

it('false when received amount is less than expected', async () => {
const transaction: Transaction = {
amount: '100.00',
paymentId: '123',
message: 'test opReturn message',
rawMessage: 'test opReturn message',
hash: '',
timestamp: 0,
address: 'ecash:qrmm7edwuj4jf7tnvygjyztyy0a0qxvl7quss2vxek',
};
const expectedPaymentId = '123';
const expectedAmount = resolveNumber('100.01');
const expectedOpReturn = 'test opReturn message';
const price = 1;
const currency = 'XEC';
Expand Down
9 changes: 1 addition & 8 deletions react/lib/util/chronik.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ export async function satoshisToUnit(satoshis: bigint, networkFormat: string): P

const getTransactionAmountAndData = async (transaction: Tx, addressString: string): Promise<{amount: string, opReturn: string}> => {
let totalOutput = BigInt(0);
let totalInput = BigInt(0);
const addressFormat = xecaddr.detectAddressFormat(addressString)
const script = toHash160(addressString).hash160
let opReturn = ''
Expand All @@ -133,14 +132,8 @@ const getTransactionAmountAndData = async (transaction: Tx, addressString: stri
}
}
}
for (const input of transaction.inputs) {
if (input?.outputScript?.includes(script) === true) {
totalInput += input.sats
}
}
const satoshis = totalOutput - totalInput
return {
amount: await satoshisToUnit(satoshis, addressFormat),
amount: await satoshisToUnit(totalOutput, addressFormat),
opReturn
}
}
Expand Down
4 changes: 2 additions & 2 deletions react/lib/util/validate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ export const shouldTriggerOnSuccess = (
if (transactionCurrency !== currency) {
if (currencyObject){
const value = (currencyObject.float / price).toFixed(DECIMALS[transactionCurrency])
isAmountValid = resolveNumber(value).isEqualTo(amount)
isAmountValid = resolveNumber(value).isLessThanOrEqualTo(amount)
}else {
isAmountValid = false
}
} else {
isAmountValid = expectedAmount.isEqualTo(amount);
isAmountValid = expectedAmount.isLessThanOrEqualTo(amount);
Comment on lines +37 to +42
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

isLessThanOrEqualTo weakens the uniqueness guarantee when randomSatoshis is truthy.

The comparison direction (expectedAmount ≤ receivedAmount) is correct and logically consistent with the PR goal. However, a subtle regression:

When randomSatoshis is truthy and non-zero, the isPaymentIdValid check is entirely skipped (lines 48–53). Previously, isEqualTo enforced an exact match on the salt-modified amount, which implicitly gave each payment session uniqueness. With isLessThanOrEqualTo, any concurrent transaction to the monitored address with amount >= expectedAmount would trigger onSuccess.

In practice the WebSocket handler only processes TX_ADDED_TO_MEMPOOL events (new transactions), so a replay of an older larger transaction is not possible at the protocol level. The risk is therefore a coincidental same-block unrelated transaction to the same address that exceeds the threshold — low probability, but the uniqueness guarantee that randomSatoshis was intended to provide is no longer enforced by the amount check alone.

Consider adding a brief comment explaining that isLessThanOrEqualTo is intentional and that uniqueness for randomSatoshis sessions is instead provided by … (paymentId, or document the trade-off).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@react/lib/util/validate.ts` around lines 37 - 42, In the block where
randomSatoshis is truthy you replaced exact matching with isLessThanOrEqualTo
and also skip isPaymentIdValid, weakening the per-session uniqueness guarantee;
update validate.ts by adding a concise comment next to the randomSatoshis /
isLessThanOrEqualTo branch (and near the isPaymentIdValid skip) that states this
use of isLessThanOrEqualTo is intentional, documents that uniqueness is
preserved by the paymentId / isPaymentIdValid logic (or by the
TX_ADDED_TO_MEMPOOL event semantics) rather than exact amount matching
(isEqualTo), and notes the trade-off/risk of coincidental higher-value
transactions triggering onSuccess — or alternatively re-enable isPaymentIdValid
for randomSatoshis flows if you want strict uniqueness.

}
}
let isPaymentIdValid = true
Expand Down