Skip to content

Convert SimplifiedTransaction[] to Transaction[] type in getAddressDe… #627

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
Klakurka merged 3 commits into from
Feb 25, 2026
Merged

Convert SimplifiedTransaction[] to Transaction[] type in getAddressDe… #627

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
521d325
Convert SimplifiedTransaction[] to Transaction[] type in getAddressDe…
Fabcien Feb 23, 2026
e18dfeb
fix: remove any, add type
chedieck Feb 24, 2026
8bc9390
fix: wrong type
chedieck Feb 25, 2026
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
69 changes: 68 additions & 1 deletion react/lib/util/api-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,79 @@ import {
import { isFiat } from './currency';
import { CURRENCY_TYPES_MAP, DECIMALS } from './constants';

interface SimplifiedTransaction {
Copy link
Collaborator Author

@Fabcien Fabcien Feb 24, 2026

Choose a reason for hiding this comment

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

What's the point ? This is an API call so the type can't be enforced. This is guaranteed to go out of sync sooner or later. The whole point of this PR is to enforce the returned type when the input type is subject to change, so this type adds a false sense of type safety which is worst than no type imo

Copy link
Collaborator

@chedieck chedieck Feb 24, 2026

Choose a reason for hiding this comment

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

I added mostly because I saw (via code rabbit ai) that a type was being wrongly casted from Decimal to string: not a huge deal of course, but for me it's more about keeping the contract between client and server explicit.

I also don't think is a big deal keeping both in sync, logistically speaking, since we control both the client and the server. Besides that, changing the server api's response would most likely require a change in this file (or at least reviewing it), so having them tied together looks like a good thing to me, but maybe I am not seeing it right.

In any case, I don't think this is significant enough to be worth discussing it too much, so if you prefer, you can just revert my commit (but probably keep the amount.toString() part)

hash: string
amount: string
paymentId: string
confirmed?: boolean
message: string
timestamp: number
address: string
rawMessage: string
inputAddresses: Array<{
address: string
amount: string
}>
outputAddresses: Array<{
address: string
amount: string
}>
prices: Array<{
price: {
value: string
quoteId: number
}
}>
}

export const getAddressDetails = async (
address: string,
rootUrl = config.apiBaseUrl,
): Promise<Transaction[]> => {
const res = await fetch(`${rootUrl}/address/transactions/${address}`);
return res.json();

if (!res.ok) {
console.warn(`Received invalid response from ${rootUrl}/address/transactions/${address}`);
return [];
}

let apiTransactions;
try {
apiTransactions = await res.json();
} catch (error) {
console.warn(`Failed to fetch address transactions from ${rootUrl}/address/transactions/${address}`, error);
return [];
}

if (!Array.isArray(apiTransactions)) {
console.warn(`Received invalid data from ${rootUrl}/address/transactions/${address}`, apiTransactions);
return [];
}

const transactions: Transaction[] = [];
apiTransactions.forEach((apiTransaction: SimplifiedTransaction) => {
const opReturn = {
rawMessage: apiTransaction.rawMessage,
message: apiTransaction.message,
paymentId: apiTransaction.paymentId,
};
const transaction: Transaction = {
hash: apiTransaction.hash,
amount: apiTransaction.amount,
paymentId: apiTransaction.paymentId,
confirmed: apiTransaction.confirmed,
message: apiTransaction.message,
timestamp: apiTransaction.timestamp,
address: apiTransaction.address,
rawMessage: apiTransaction.rawMessage,
// Only keep the address string, drop the amount
inputAddresses: Array.isArray(apiTransaction.inputAddresses) ? apiTransaction.inputAddresses.map((input: { address: string, amount: string }) => input.address) : [],
opReturn: JSON.stringify(opReturn),
};
Comment on lines +66 to +83
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

rg -n --type=ts '\bopReturn\b' -A 3 -B 3

Repository: PayButton/paybutton

Length of output: 49620

🏁 Script executed:

rg -n 'getTransactions|SimplifiedTransaction' --type=ts -B 2 -A 5 | head -100

Repository: PayButton/paybutton

Length of output: 1035

🏁 Script executed:

rg -n 'getTransactions|from.*api-client' --type=ts -B 2 -A 8 | head -150

Repository: PayButton/paybutton

Length of output: 1538

🏁 Script executed:

rg -n 'getAddressDetails' --type=ts -B 2 -A 10 | head -200

Repository: PayButton/paybutton

Length of output: 3433

🏁 Script executed:

rg -n 'handleNewTransaction' --type=ts -B 5 -A 15

Repository: PayButton/paybutton

Length of output: 4476

🏁 Script executed:

rg -n 'const handlePayment|handlePayment.*=.*\(' --type=ts -B 2 -A 15 | head -200

Repository: PayButton/paybutton

Length of output: 1720

🏁 Script executed:

rg -n 'shouldTriggerOnSuccess' --type=ts -B 2 -A 12 | head -100

Repository: PayButton/paybutton

Length of output: 6200

🏁 Script executed:

cat -n react/lib/util/validate.ts | sed -n '7,80p'

Repository: PayButton/paybutton

Length of output: 2453

🏁 Script executed:

cat -n react/lib/util/api-client.ts | sed -n '60,90p'

Repository: PayButton/paybutton

Length of output: 1425

🏁 Script executed:

rg -n 'transaction\.opReturn|tx\.opReturn' --type=ts -B 2 -A 3

Repository: PayButton/paybutton

Length of output: 45

🏁 Script executed:

cat -n react/lib/util/chronik.ts | sed -n '116,160p'

Repository: PayButton/paybutton

Length of output: 2078

🏁 Script executed:

cat -n react/lib/util/validate.ts | sed -n '50,62p'

Repository: PayButton/paybutton

Length of output: 803

opReturn is always a truthy JSON string, masking "no OP_RETURN" cases and breaking payment validation.

opReturn is unconditionally set to JSON.stringify({ rawMessage, message, paymentId }). When there is no real OP_RETURN data, all three fields are empty strings, producing {"rawMessage":"","message":"","paymentId":""} — a truthy non-empty string. The shouldTriggerOnSuccess validation in validate.ts (line 56) explicitly checks for empty string or undefined to detect "no OP_RETURN," but receives this truthy JSON string instead. When a payment has no expected OP_RETURN (expectedOpReturn undefined), validation fails with the api-client transaction but passes with chronik transactions (which leave opReturn as empty string when no data exists). Transaction.opReturn is declared optional (opReturn?: string) to distinguish "present" from "absent," but the current implementation defeats this purpose.

🔧 Proposed fix — only set `opReturn` when meaningful data exists 
-    const opReturn = {
-      rawMessage: apiTransaction.rawMessage,
-      message: apiTransaction.message,
-      paymentId: apiTransaction.paymentId,
-    };
     const transaction: Transaction = {
       hash: apiTransaction.hash,
       amount: apiTransaction.amount,
       paymentId: apiTransaction.paymentId,
       confirmed: apiTransaction.confirmed,
       message: apiTransaction.message,
       timestamp: apiTransaction.timestamp,
       address: apiTransaction.address,
       rawMessage: apiTransaction.rawMessage,
       inputAddresses: Array.isArray(apiTransaction.inputAddresses)
         ? apiTransaction.inputAddresses.map((input: { address: string, amount: string }) => input.address)
         : [],
-      opReturn: JSON.stringify(opReturn),
+      ...(apiTransaction.rawMessage || apiTransaction.message || apiTransaction.paymentId
+        ? {
+            opReturn: JSON.stringify({
+              rawMessage: apiTransaction.rawMessage,
+              message: apiTransaction.message,
+              paymentId: apiTransaction.paymentId,
+            }),
+          }
+        : {}),
     };

transactions.push(transaction);
});

return transactions;
};

export const getAddressBalance = async (
Expand Down