feat: prop tokenId #595
feat: prop tokenId #595
Conversation
📝 WalkthroughWalkthroughThis PR adds token support to the PayButton library by introducing an optional Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant PayButton
participant Widget
participant Chronik
participant ChronikWS
User->>PayButton: Pass tokenId prop
PayButton->>PayButton: Destructure tokenId
PayButton->>Widget: Pass tokenId to WidgetContainer
Widget->>Widget: Store tokenId in props
alt tokenId provided
Widget->>Chronik: getTokenInfo(tokenId, address)
Chronik-->>Widget: Return token metadata
Widget->>Widget: Set tokenName state
Widget->>Widget: resolveUrl with tokenId param
else no tokenId
Widget->>Widget: Use currency code
Widget->>Widget: resolveUrl without tokenId
end
Widget->>ChronikWS: setupChronikWebSocket with tokenId
ChronikWS->>ChronikWS: initializeChronikWebsocket with tokenId
alt Token transaction received
ChronikWS->>Chronik: parseWebsocketMessage with tokenId
Chronik->>Chronik: getTokenAmount for matching outputs
Chronik-->>ChronikWS: Return token-aware transaction
else Regular transaction
ChronikWS->>Chronik: parseWebsocketMessage standard flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
react/lib/util/chronik.ts (1)
166-184: Add error handling for token amount calculation.The call to
getTokenAmount(line 171) lacks error handling. If token amount calculation fails, it will throw an unhandled error.🔎 Recommended fix
const getTransactionFromChronikTransaction = async (transaction: Tx, address: string, tokenId?: string): Promise<Transaction> => { const { amount, opReturn } = await getTransactionAmountAndData(transaction, address) const parsedOpReturn = resolveOpReturn(opReturn) const networkSlug = getAddressPrefix(address) const inputAddresses = getSortedInputAddresses(networkSlug, transaction) - const tokenAmount = tokenId ? await getTokenAmount(transaction, tokenId, address, networkSlug) : undefined; + let tokenAmount: string | undefined; + if (tokenId) { + try { + // Fetch token decimals from token info + const tokenInfo = await getTokenInfo(tokenId, address); + const decimals = tokenInfo.genesisInfo.decimals; + tokenAmount = await getTokenAmount(transaction, tokenId, address, networkSlug, decimals); + } catch (err) { + console.error('Failed to calculate token amount:', err); + tokenAmount = '0'; + } + } return { hash: transaction.txid, amount: tokenId ? tokenAmount! : amount, address, timestamp: transaction.block !== undefined ? transaction.block.timestamp : transaction.timeFirstSeen, confirmed: transaction.block !== undefined, opReturn, paymentId: parsedOpReturn?.paymentId ?? '', message: parsedOpReturn?.message ?? '', rawMessage: parsedOpReturn?.rawMessage ?? '', inputAddresses, } }
🧹 Nitpick comments (3)
react/lib/components/Widget/Widget.tsx (2)
501-503: Consider adding validation for the tokenId parameter.The helper constructs an icon URL without validating the tokenId. While image loading failures are typically handled by the browser, consider adding basic validation (non-empty, expected format) to avoid constructing invalid URLs.
🔎 Optional: Add basic validation
const getTokenIconUrl = useCallback((tokenId: string): string => { + if (!tokenId || typeof tokenId !== 'string' || tokenId.trim() === '') { + throw new Error('Invalid tokenId provided'); + } return `https://icons.etokens.cash/128/${tokenId}.png` }, [])
845-845: Handle null tokenName in display text.If
tokenNameis null (e.g., token info fetch failed or tokenTicker is undefined), the display text will show "null" to users. Consider providing a fallback.🔎 Proposed fix
-setText(`Send ${amountToDisplay} ${tokenId ? tokenName : cur}`) +setText(`Send ${amountToDisplay} ${tokenId ? (tokenName || 'token') : cur}`)-setText(`Send any amount of ${tokenId ? tokenName : thisAddressType}`) +setText(`Send any amount of ${tokenId ? (tokenName || 'token') : thisAddressType}`)Also applies to: 849-850
react/lib/util/chronik.ts (1)
258-269: Consider documenting error handling expectations.The
getTokenInfofunction lacks error handling and will propagate errors to callers. While this is acceptable, ensure all call sites properly handle potential failures (network errors, invalid tokenId, etc.).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
paybutton/src/index.tsxreact/lib/components/PayButton/PayButton.tsxreact/lib/components/PaymentDialog/PaymentDialog.tsxreact/lib/components/Widget/Widget.tsxreact/lib/util/chronik.tsreact/lib/util/socket.ts
🧰 Additional context used
🧬 Code graph analysis (2)
react/lib/util/socket.ts (2)
react/lib/util/chronik.ts (1)
initializeChronikWebsocket(296-323)react/lib/util/types.ts (1)
Transaction(10-21)
react/lib/components/Widget/Widget.tsx (3)
react/lib/util/chronik.ts (1)
getTokenInfo(258-269)react/lib/util/format.ts (1)
amount(5-12)react/lib/util/address.ts (1)
isValidCashAddress(4-13)
🔇 Additional comments (10)
react/lib/util/socket.ts (1)
125-125: LGTM! Clean parameter propagation.The tokenId parameter is properly threaded through the interface and passed to initializeChronikWebsocket, consistent with the token-aware flow.
Also applies to: 149-155
paybutton/src/index.tsx (1)
109-109: LGTM! Properly enables tokenId as a recognized prop.The addition to allowedProps enables DOM attribute parsing and forwarding of tokenId to PayButton/Widget components.
react/lib/components/PaymentDialog/PaymentDialog.tsx (1)
70-70: LGTM! Clean prop forwarding.The tokenId is properly threaded from PaymentDialogProps through to WidgetContainer, enabling token support in the dialog flow.
Also applies to: 134-134, 264-264
react/lib/components/PayButton/PayButton.tsx (1)
62-62: LGTM! Proper tokenId integration.The tokenId is correctly added to PayButtonProps and forwarded to both the Chronik WebSocket setup (for transaction monitoring) and PaymentDialog (for UI rendering).
Also applies to: 97-97, 298-298, 456-456
react/lib/components/Widget/Widget.tsx (5)
1070-1070: LGTM! Proper QR code icon selection for tokens.The image selection correctly prioritizes token icons when tokenId is present, falling back to the appropriate BCH or XEC logo based on the address format.
854-854: LGTM! Effect dependencies are complete.The effect dependencies properly include both
tokenNameandtokenId, which are used within the effect for URL generation and display text.
546-546: LGTM! TokenId properly propagated to WebSocket setup.The tokenId is correctly passed to the Chronik WebSocket setup for transaction monitoring in standalone widget mode.
55-55: LGTM! Proper setup for token support.The imports, prop additions, and state initialization properly establish the foundation for token functionality in the Widget component.
Also applies to: 118-118, 179-179, 330-330
976-1021: Verify token payment URL parameter names with Cashtab protocol.The code confirms that donation functionality is intentionally disabled for token payments (lines 993-1004 do not append donation address/amount parameters when
tokenIdis present), while donations work normally for XEC/BCH payments.Please verify that
token_decimalized_qtyandtoken_idare the correct parameter names for the Cashtab wallet's token payment protocol.react/lib/util/chronik.ts (1)
271-277: LGTM! Proper tokenId propagation through WebSocket flow.The tokenId parameter is consistently threaded through the WebSocket initialization and message parsing functions, enabling token-aware transaction detection.
Also applies to: 287-287, 296-299, 314-315
| useEffect(() => { | ||
| ;(async (): Promise<void> => { | ||
| if (tokenId && tokenId !== null && tokenId !== '' ) { | ||
| const tokenInfo = await getTokenInfo(tokenId, to) | ||
| const name = tokenInfo.genesisInfo.tokenTicker ?? null | ||
| setTokenName(name) | ||
|
|
||
| return | ||
| } | ||
| setLoading(false) | ||
| })() | ||
| }, [tokenId, to]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for token info fetching.
The effect lacks error handling if getTokenInfo fails or returns unexpected data. This could leave the widget in a loading state or display incorrect information.
🔎 Recommended fix with error handling
useEffect(() => {
;(async (): Promise<void> => {
- if (tokenId && tokenId !== null && tokenId !== '' ) {
+ if (tokenId && tokenId !== '') {
+ try {
const tokenInfo = await getTokenInfo(tokenId, to)
const name = tokenInfo.genesisInfo.tokenTicker ?? null
setTokenName(name)
-
- return
+ } catch (err) {
+ console.error('Failed to fetch token info:', err)
+ setTokenName(null)
+ setErrorMsg('Unable to load token information')
+ } finally {
+ setLoading(false)
+ }
+ return
}
setLoading(false)
})()
}, [tokenId, to])Note: The redundant tokenId !== null check can be removed since the truthiness check already covers it.
📝 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.
| useEffect(() => { | |
| ;(async (): Promise<void> => { | |
| if (tokenId && tokenId !== null && tokenId !== '' ) { | |
| const tokenInfo = await getTokenInfo(tokenId, to) | |
| const name = tokenInfo.genesisInfo.tokenTicker ?? null | |
| setTokenName(name) | |
| return | |
| } | |
| setLoading(false) | |
| })() | |
| }, [tokenId, to]) | |
| useEffect(() => { | |
| ;(async (): Promise<void> => { | |
| if (tokenId && tokenId !== '') { | |
| try { | |
| const tokenInfo = await getTokenInfo(tokenId, to) | |
| const name = tokenInfo.genesisInfo.tokenTicker ?? null | |
| setTokenName(name) | |
| } catch (err) { | |
| console.error('Failed to fetch token info:', err) | |
| setTokenName(null) | |
| setErrorMsg('Unable to load token information') | |
| } finally { | |
| setLoading(false) | |
| } | |
| return | |
| } | |
| setLoading(false) | |
| })() | |
| }, [tokenId, to]) |
🤖 Prompt for AI Agents
In react/lib/components/Widget/Widget.tsx around lines 595 to 606, the useEffect
that calls getTokenInfo lacks error handling and contains a redundant tokenId
!== null check; update it to remove the redundant null check, wrap the async
call in try/catch/finally, handle cases where tokenInfo or tokenInfo.genesisInfo
is missing (setTokenName to null or a safe default), log or surface the error as
appropriate, and ensure setLoading(false) is called in finally so the widget
never stays stuck in loading on failure.
| const getTokenAmount = async (transaction: Tx, tokenId: string, address: string, networkSlug: string): Promise<string> => { | ||
| let totalTokenOutput = BigInt(0); | ||
|
|
||
| for (const output of transaction.outputs) { | ||
| if (output.token?.tokenId === tokenId) { | ||
| const outputAddress = outputScriptToAddress(networkSlug, output.outputScript) | ||
|
|
||
| if(outputAddress === address) { | ||
| const atoms = BigInt(output.token.atoms); | ||
|
|
||
| totalTokenOutput += atoms / BigInt(100); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return totalTokenOutput.toString(); | ||
| } |
There was a problem hiding this comment.
Critical: Hardcoded token decimals assumption.
Line 126 divides by BigInt(100), assuming all tokens have 2 decimal places. However, eCash tokens support 0-9 decimal places, specified in the token's genesis information. This will cause incorrect amount calculations for tokens with different decimal counts.
🔎 Recommended fix
-const getTokenAmount = async (transaction: Tx, tokenId: string, address: string, networkSlug: string): Promise<string> => {
+const getTokenAmount = async (transaction: Tx, tokenId: string, address: string, networkSlug: string, decimals: number): Promise<string> => {
let totalTokenOutput = BigInt(0);
for (const output of transaction.outputs) {
if (output.token?.tokenId === tokenId) {
const outputAddress = outputScriptToAddress(networkSlug, output.outputScript)
if(outputAddress === address) {
const atoms = BigInt(output.token.atoms);
- totalTokenOutput += atoms / BigInt(100);
+ totalTokenOutput += atoms;
}
}
}
- return totalTokenOutput.toString();
+ // Convert from atoms to decimalized amount
+ const divisor = BigInt(10 ** decimals);
+ return (totalTokenOutput / divisor).toString();
}Then update the caller at line 171 to fetch and pass the decimals from token info.
🤖 Prompt for AI Agents
In react/lib/util/chronik.ts around lines 116 to 132, the code incorrectly
divides token atom amounts by BigInt(100) (line 126), hardcoding 2 decimals;
instead, read the token's decimals from the token genesis info and use that to
scale atoms (divide by BigInt(10) ** BigInt(decimals)) before summing. Update
the function signature to accept a decimals parameter (or retrieve decimals from
a passed token info object) and change the caller at line 171 to fetch and pass
the token's decimals from token info so amounts are computed using the correct
decimal precision.
Related to #44
Description
Description
This PR adds full support for token-based payments and updates the payment flow accordingly.
Changes included:
Accepts a
token-idparameter to enable token-specific paymentsDetects and validates payments for the specified token
Fetches the token icon and displays it in the QR code
Updates the dialog text to display the token ticker (e.g., Send 10 XECX)
Updates the payment URL to include
token_idandtoken_decimalized_qty, removing unused parametersWill continue the following remaining steps in another PR:
Test plan
Create a button with
token-idparam check dialog text, qrcode icon. Copy the payment url check if it contains thetoken-id, test the payment url in cashtab, make a payment with the specifications of the button and make sure the payment is detected.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.