Repository navigation
Conversation
WalkthroughAdds an Escrow app plus supporting infrastructure: new Escrow UI, iframe host, RPC handler with tests, escrow client and provider, navigation wiring, env/type updates, Chatwoot support, backup refactors, JS capability checks, many UI/type refinements, e2e tests, Docker/infra and styling updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as AppEscrow
participant Host as ArkadeIframeHost
participant Frame as Iframe (Escrow App)
participant RPC as RpcHandler
participant Wallet as Wallet/Flow Contexts
participant Server as EscrowServer
User->>App: navigate to Escrow
App->>Host: mount iframe (src, allowedChildOrigins, messageHandler)
Host->>Frame: create iframe element
Frame->>Host: onload -> initial handshake / ARKADE_KEEP_ALIVE
Host->>Frame: send keep-alive pings periodically
Frame->>Host: postMessage(ARKADE_RPC_REQUEST {method, params})
Host->>Host: validate origin && source === iframe.contentWindow
Host->>RPC: forward inbound message to messageHandler
RPC->>Wallet: call requested wallet APIs (get key, sign, fund, balance)
Wallet-->>RPC: respond with result or throw error
RPC-->>Host: return ARKADE_RPC_RESPONSE or Failure envelope
Host-->>Frame: postMessage(response)
Frame->>Host: postMessage(ARKADE_KEEP_ALIVE)
Host->>Host: respond locally (no echo) and mark isAlive
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
| } | ||
| }, | ||
| fundAddress: async (address, amount) => { | ||
| const result = await handlers.fundAddress(address, amount) |
There was a problem hiding this comment.
| const result = await handlers.fundAddress(address, amount) | |
| await handlers.fundAddress(address, amount) |
There was a problem hiding this comment.
| const signed = await svcWallet.sign(tx, inputIndexes) | ||
| return signed |
There was a problem hiding this comment.
nit
| const signed = await svcWallet.sign(tx, inputIndexes) | |
| return signed | |
| return svcWallet.sign(tx, inputIndexes) |
There was a problem hiding this comment.
bordalix
left a comment
bordalix
left a comment
There was a problem hiding this comment.
Please remove draft mode from PR so that @coderabbitai could make a review.
| <App name='Ark Invaders' icon={<InvadersIcon />} desc='The classic arcade game' link='' /> | ||
| <App name='Coinflip' icon={<CoinflipIcon />} desc='Head or Tails? Place your bet!' link='' /> | ||
| <App name='Fuji Money' icon={<FujiMoneyIcon />} desc='Synthetic Assets on the Bitcoin network' link='' /> | ||
| <App name='Escrow' icon={<EscrowIcon />} desc='Escrow system on Ark' link='' page={Pages.AppEscrow} /> |
There was a problem hiding this comment.
Let's put active Apps on top of the list and Coming Soon on the bottom.
There was a problem hiding this comment.
| @@ -0,0 +1,87 @@ | |||
| import { useContext, useMemo, useState } from 'react' | |||
There was a problem hiding this comment.
| import { useContext, useMemo, useState } from 'react' | |
| import { useContext, useMemo } from 'react' |
| import { IframeContext } from '../../../providers/iframe' | ||
|
|
||
| export default function AppEscrow() { | ||
| const { navigate, screen, tab } = useContext(NavigationContext) |
There was a problem hiding this comment.
| const { navigate, screen, tab } = useContext(NavigationContext) | |
| const { navigate } = useContext(NavigationContext) |
There was a problem hiding this comment.
|
|
||
| export default function AppEscrow() { | ||
| const { navigate, screen, tab } = useContext(NavigationContext) | ||
| const { iframeUrl } = useContext(IframeContext) |
There was a problem hiding this comment.
| const { iframeUrl } = useContext(IframeContext) |
| const { iframeUrl } = useContext(IframeContext) | ||
|
|
||
| const { svcWallet } = useContext(WalletContext) | ||
| const { setRecvInfo, setSendInfo } = useContext(FlowContext) |
There was a problem hiding this comment.
| const { setRecvInfo, setSendInfo } = useContext(FlowContext) | |
| const { setSendInfo } = useContext(FlowContext) |
There was a problem hiding this comment.
e9a752c to
8002e1c
Compare
| @@ -0,0 +1,3 @@ | |||
| export default function EscrowIcon() { | |||
| return <img height='55' width='55' src='/escrow.png' /> | |||
There was a problem hiding this comment.
nit
| return <img height='55' width='55' src='/escrow.png' /> | |
| return <img height='55' width='55' src='/escrow.png' style={{ borderRadius: '50%' }} /> |
after
8002e1c to
25c4b25
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
src/lib/privateKey.ts (1)
48-48: Do not log private keys.Printing raw key material to the console is a critical secret leakage risk. Remove this line.
- console.log('privateKey', privateKey) + // never log private keys
🧹 Nitpick comments (8)
src/lib/privateKey.ts (1)
100-111: Make derived AES key non-extractable.Set extractable to false to reduce key exfiltration risk. No behavior change.
- { name: 'AES-GCM', length: 256 }, - true, + { name: 'AES-GCM', length: 256 }, + false,src/screens/Apps/Index.tsx (1)
100-101: Hide link label when no link is provided.Avoid rendering an empty line for Escrow’s blank link.
- <Text color='dark80' small thin wrap> - {link} - </Text> + {link ? ( + <Text color='dark80' small thin wrap>{link}</Text> + ) : null}src/icons/Escrow.tsx (1)
1-3: Add alt text for accessibility.
Provide a short alt or mark decorative.-export default function EscrowIcon() { - return <img height='55' width='55' src='/escrow.png' style={{ borderRadius: '50%' }} /> -} +export default function EscrowIcon() { + return <img alt='Escrow' height='55' width='55' src='/escrow.png' style={{ borderRadius: '50%' }} /> +}src/screens/Apps/Escrow/Index.tsx (1)
69-80: Fix stale closures in handlers.
Empty deps freeze initial svcWallet/navigate. Depend on the functions instead.- const handlers: ArkadeIdentityHandlers = useMemo( - () => ({ - sign, - signerSession, - signin, - signout, - getXOnlyPublicKey, - getArkWalletAddress, - fundAddress, - }), - [], - ) + const handlers: ArkadeIdentityHandlers = useMemo( + () => ({ + sign, + signerSession, + signin, + signout, + getXOnlyPublicKey, + getArkWalletAddress, + fundAddress, + }), + [sign, signerSession, signin, signout, getXOnlyPublicKey, getArkWalletAddress, fundAddress], + )src/screens/Apps/Escrow/RpcHandler.ts (1)
116-118: Strengthen TS narrowing by switching onmessage.method.Switch on
message.method(not an alias) somessage.payloadnarrows per discriminant. Also reduce risk of alias drift.- const { id, method } = message - switch (method) { + const { id } = message + switch (message.method) { case 'get-x-public-key': { const xOnlyPublicKey = await props.getXOnlyPublicKey().then((k) => (k ? bytesToHex(k) : null)) return { tag: 'success', result: { kind: 'ARKADE_RPC_RESPONSE', id, - method, + method: 'get-x-public-key', payload: { xOnlyPublicKey }, }, } } case 'get-ark-wallet-address': { const arkAddress = (await props.getArkWalletAddress()) ?? null return { tag: 'success', result: { kind: 'ARKADE_RPC_RESPONSE', id, - method, + method: 'get-ark-wallet-address', payload: { arkAddress }, }, } } case 'sign-login-challenge': try { const signedChallenge = await props.signLoginChallenge(message.payload.challenge) return { tag: 'success', - result: { kind: 'ARKADE_RPC_RESPONSE', id, method, payload: { signedChallenge } }, + result: { kind: 'ARKADE_RPC_RESPONSE', id, method: 'sign-login-challenge', payload: { signedChallenge } }, }Also applies to: 141-147, 158-165, 175-177
src/screens/Apps/Escrow/ArkadeIframeHost.tsx (3)
100-105: Hook deps: includehandleMessage.Effect uses
handleMessagebut it’s not in deps; add it to avoid stale closures.- }, [allowed, handlers, isAlive]) + }, [allowed, handlers, isAlive, handleMessage])
59-64: Optional: usesetIntervalwith cleanup for polling.Simplifies timer management and ensures cleanup on unmount.
- const poll = useCallback(() => { - if (iframeRef.current?.contentWindow) { - iframeRef.current.contentWindow.postMessage({ kind: 'ARKADE_KEEP_ALIVE', timestamp: Date.now() }, childOrigin) - setTimeout(() => poll(), 5000) - } - }, [isAlive, iframeRef, childOrigin]) + const poll = useCallback(() => { + const id = window.setInterval(() => { + if (iframeRef.current?.contentWindow) { + iframeRef.current.contentWindow.postMessage( + { kind: 'ARKADE_KEEP_ALIVE', timestamp: Date.now() }, + childOrigin, + ) + } + }, 5000) + return () => window.clearInterval(id) + }, [childOrigin])And in the effect:
- useEffect(() => { - poll() - }, [poll]) + useEffect(() => poll(), [poll])
22-26: Docs nit: clarifyallowedChildOriginsexpects origins.Add a short JSDoc to prevent passing full URLs.
type Props = { - src: string - allowedChildOrigins: string[] + /** Full iframe URL */ + src: string + /** List of allowed origins (e.g., "https://example.com"), not full URLs */ + allowedChildOrigins: string[] handlers: ArkadeIdentityHandlers }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/escrow.pngis excluded by!**/*.png
📒 Files selected for processing (10)
README.md(1 hunks)src/icons/Escrow.tsx(1 hunks)src/lib/privateKey.ts(1 hunks)src/providers/navigation.tsx(3 hunks)src/screens/Apps/Escrow/ArkadeIframeHost.tsx(1 hunks)src/screens/Apps/Escrow/Index.tsx(1 hunks)src/screens/Apps/Escrow/RpcHandler.ts(1 hunks)src/screens/Apps/Index.tsx(2 hunks)src/screens/Wallet/Send/Form.tsx(1 hunks)src/vite-env.d.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-06T09:53:28.428Z
Learnt from: bordalix
PR: arkade-os/wallet#134
File: src/screens/Settings/Password.tsx:34-39
Timestamp: 2025-08-06T09:53:28.428Z
Learning: In src/screens/Settings/Password.tsx, the saveNewPassword function doesn't need error handling for getPrivateKey because it's only called after authentication is verified. The useEffect validates the oldPassword using isValidPassword() which internally calls getPrivateKey(), and only sets authenticated=true if successful. The saveNewPassword function has a guard clause that returns early if not authenticated, so getPrivateKey(oldPassword) is guaranteed to work when called.
Applied to files:
src/lib/privateKey.ts
🧬 Code graph analysis (3)
src/screens/Apps/Index.tsx (1)
src/icons/Escrow.tsx (1)
EscrowIcon(1-3)
src/screens/Apps/Escrow/ArkadeIframeHost.tsx (1)
src/screens/Apps/Escrow/RpcHandler.ts (1)
makeMessageHandler(110-183)
src/screens/Apps/Escrow/Index.tsx (5)
src/providers/navigation.tsx (1)
NavigationContext(159-163)src/providers/wallet.tsx (1)
WalletContext(39-53)src/providers/flow.tsx (1)
FlowContext(82-95)src/providers/asp.tsx (1)
AspContext(15-20)src/screens/Apps/Escrow/ArkadeIframeHost.tsx (2)
ArkadeIdentityHandlers(6-14)ArkadeIframeHost(22-121)
🪛 Biome (2.1.2)
src/screens/Apps/Escrow/RpcHandler.ts
[error] 119-119: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 130-130: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 170-173: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (4)
src/providers/navigation.tsx (1)
27-34: Escrow page wiring looks good.
Routes, tab mapping, and component render are consistent.Also applies to: 69-70, 102-104
README.md (1)
25-31: Env var addition LGTM.
Docs align with new VITE_ARK_ESCROW_URL.src/vite-env.d.ts (1)
9-9: Type surface updated correctly.
Optional VITE_ARK_ESCROW_URL matches README.src/screens/Apps/Escrow/Index.tsx (1)
2-5: Review comment is based on incorrect assumptions about v1.x APIs.The import path
'@noble/hashes/sha2.js'is correct for @noble/hashes v1.x (aliases like'@noble/hashes/sha256'also work, but both are valid). More importantly, the override patternsecp.hashes.sha256 = sha256is still supported and recommended in @noble/secp256k1 v1.x for synchronous signing scenarios, and the code's implementation matches this guidance exactly.The current approach is correct and follows v1.x best practices.
Likely an incorrect or invalid review comment.
| export type ArkadeIdentityHandlers = { | ||
| getXOnlyPublicKey: () => Promise<Uint8Array | null> | ||
| sign(tx: Transaction, inputIndexes?: number[]): Promise<Transaction> | ||
| signerSession: () => Promise<unknown> | ||
| signin: (challenge: string) => Promise<string> | ||
| signout: () => Promise<unknown> | ||
| getArkWalletAddress: () => Promise<string | undefined> | ||
| fundAddress: (address: string, amount: number) => Promise<unknown> | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Propagate actual funding result and type it.
You return fundedAmount: amount unconditionally, which can be wrong (fees/rounding). Type the handler to return the structured result and forward it.
export type ArkadeIdentityHandlers = {
getXOnlyPublicKey: () => Promise<Uint8Array | null>
sign(tx: Transaction, inputIndexes?: number[]): Promise<Transaction>
signerSession: () => Promise<unknown>
signin: (challenge: string) => Promise<string>
signout: () => Promise<unknown>
getArkWalletAddress: () => Promise<string | undefined>
- fundAddress: (address: string, amount: number) => Promise<unknown>
+ fundAddress: (
+ address: string,
+ amount: number,
+ ) => Promise<{ address: string; requestedAmount: number; fundedAmount: number }>
}
@@
- fundAddress: async (address, amount) => {
- await handlers.fundAddress(address, amount)
- return {
- address,
- requestedAmount: amount,
- fundedAmount: amount,
- }
- },
+ fundAddress: async (address, amount) => {
+ return handlers.fundAddress(address, amount)
+ },If the underlying wallet can partially fund or apply fees, ensure its fundAddress implementation returns the exact amounts so the iframe gets accurate data.
Also applies to: 47-54
I'll verify the concerns in the review comment by examining the implementation and how fundAddress is used.
🏁 Script executed:
#!/bin/bash
# Get the full file to see implementation context
cat src/screens/Apps/Escrow/ArkadeIframeHost.tsxLength of output: 4068
🏁 Script executed:
#!/bin/bash
# Search for fundAddress implementations and usages
rg -n "fundAddress" --type ts --type tsx -C 5Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for wallet or handler implementations that might provide fundAddress
rg -n "handlers\.fundAddress|fundAddress.*Promise" --type ts --type tsx -C 3Length of output: 27
I need to search without the tsx type filtering and find where the handlers are implemented.
🏁 Script executed:
#!/bin/bash
# Search for fundAddress without type restrictions
rg -n "fundAddress" -C 5Length of output: 5051
🏁 Script executed:
#!/bin/bash
# Find where ArkadeIframeHost handlers are provided
rg -n "handlers.*=|fundAddress.*async" -C 3Length of output: 2658
🏁 Script executed:
#!/bin/bash
# Search for any wallet or funding related implementations
rg -n "walletHandlers|ArkadeIdentityHandlers" -C 3Length of output: 2166
Update type and implementation to forward actual funding result.
The type at line 13 is Promise<unknown> but should match the structured return expected by RpcHandler (lines 170-173 in RpcHandler.ts). The wrapper at lines 47-53 ignores handlers.fundAddress return value and unconditionally returns fundedAmount: amount, losing any information about actual amounts funded (after fees/rounding).
- fundAddress: (address: string, amount: number) => Promise<unknown>
+ fundAddress: (
+ address: string,
+ amount: number,
+ ) => Promise<{ address: string; requestedAmount: number; fundedAmount: number }>Then update the implementation to forward the result:
fundAddress: async (address, amount) => {
- await handlers.fundAddress(address, amount)
- return {
- address,
- requestedAmount: amount,
- fundedAmount: amount,
- }
+ return handlers.fundAddress(address, amount)
},Ensure the handler in Index.tsx (line 64) returns the structured result with actual amounts.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/screens/Apps/Escrow/ArkadeIframeHost.tsx around lines 6–14 (and adjust
usages at lines 47–53) the ArkadeIdentityHandlers.fundAddress is typed as
Promise<unknown> and the wrapper discards the actual result, returning a
synthetic fundedAmount: amount; update the fundAddress type to return the
structured result expected by RpcHandler (see RpcHandler.ts lines 170–173),
change the wrapper to await and forward the entire returned object (not
overwrite fundedAmount), and then update the handler in Index.tsx (around line
64) to return that same structured result with actual amounts from the
underlying implementation.
| } | ||
| } | ||
| } | ||
| return { tag: 'failure', error: new Error('Unknown message') } | ||
| } |
There was a problem hiding this comment.
Return a specific error for unknown RPC methods.
Unknown methods currently surface as a generic “Unknown message”. Return a precise error and keep the generic fallback for unknown kinds.
- }
- }
- }
- return { tag: 'failure', error: new Error('Unknown message') }
+ default: {
+ return { tag: 'failure', error: new Error(`Unknown RPC method: ${String((message as any).method)}`) }
+ }
+ }
+ }
+ }
+ return { tag: 'failure', error: new Error('Unknown message kind') }🤖 Prompt for AI Agents
In src/screens/Apps/Escrow/RpcHandler.ts around lines 178 to 182, the function
currently returns a generic Error('Unknown message') for unrecognized inputs;
change this to return a precise error for unknown RPC methods (e.g.,
Error(`Unknown RPC method: ${methodName}`)) by detecting the incoming message's
method field (or equivalent property used in this file) and including it in the
error message, while keeping the existing generic fallback for unknown kinds
intact.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/screens/Apps/Escrow/Index.tsx (1)
33-38: Add missing dependency to useEffect.The effect references
svcWalleton line 34 but doesn't include it in the dependency array. This violates the exhaustive-deps rule and can lead to stale closures.Apply this diff:
useEffect(() => { if (!aspInfo.network || !svcWallet) return const baseUrl = BASE_URLS[aspInfo.network as Network] if (!baseUrl) return // No escrow app for this network setEscrowAppUrl(baseUrl) - }, [aspInfo]) + }, [aspInfo, svcWallet])
🧹 Nitpick comments (2)
src/screens/Apps/Escrow/Index.tsx (1)
93-95: Add error handling for URL parsing.If
escrowAppUrlis malformed,new URL(escrowAppUrl)will throw and crash the component. While fixing the BASE_URLS configuration (lines 18-23) prevents known bad URLs, defensive error handling is still prudent.Consider wrapping the URL parsing:
<Header text='Escrow on Ark' back={() => navigate(Pages.Apps)} /> {escrowAppUrl !== null && ( - <ArkadeIframeHost src={escrowAppUrl} allowedChildOrigins={[new URL(escrowAppUrl).origin]} handlers={handlers} /> + <ArkadeIframeHost + src={escrowAppUrl} + allowedChildOrigins={(() => { + try { + return [new URL(escrowAppUrl).origin] + } catch { + console.error('Invalid escrow URL:', escrowAppUrl) + return [] + } + })()} + handlers={handlers} + /> )}Or validate the URL before setting
escrowAppUrlin the useEffect (lines 33-38).src/screens/Apps/Escrow/RpcHandler.ts (1)
111-177: Add a default case for unknown RPC methods.The method switch (lines 111-175) has no default case. If an unknown method is received, execution falls through to the outer error handler at line 179, which reports "Unknown message kind" instead of the more specific "Unknown RPC method".
Add a default case:
case 'fund-address': { const { address, requestedAmount, fundedAmount } = await props.fundAddress( message.payload.address, message.payload.amount, ) return { tag: 'success', result: { kind: 'ARKADE_RPC_RESPONSE', id, method, payload: { address, requestedAmount, fundedAmount } }, } } + default: { + return { tag: 'failure', error: new Error(`Unknown RPC method: ${method}`) } + } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/icons/Escrow.tsx(1 hunks)src/screens/Apps/Escrow/ArkadeIframeHost.tsx(1 hunks)src/screens/Apps/Escrow/Index.tsx(1 hunks)src/screens/Apps/Escrow/RpcHandler.ts(1 hunks)vite.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- vite.config.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/icons/Escrow.tsx
- src/screens/Apps/Escrow/ArkadeIframeHost.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/screens/Apps/Escrow/Index.tsx (5)
src/providers/navigation.tsx (1)
NavigationContext(159-163)src/providers/wallet.tsx (1)
WalletContext(39-53)src/providers/flow.tsx (1)
FlowContext(82-95)src/providers/asp.tsx (1)
AspContext(15-20)src/screens/Apps/Escrow/ArkadeIframeHost.tsx (2)
ArkadeIdentityHandlers(6-14)ArkadeIframeHost(22-128)
🔇 Additional comments (3)
src/screens/Apps/Escrow/Index.tsx (2)
40-63: Identity handler implementations look correct.The wallet identity integration is sound. The
signerSessionplaceholder at lines 53-55 is acceptable if this functionality isn't required yet.
64-75: LGTM! Full state clearing implemented.The explicit clearing of mutually exclusive fields prevents stale send state from leaking into the escrow funding flow.
src/screens/Apps/Escrow/RpcHandler.ts (1)
104-181: RPC handler structure is solid.The message handler is well-structured with proper type definitions, block-scoped case statements, and error handling for critical operations.
| case 'fund-address': { | ||
| await props.fundAddress(message.payload.address, message.payload.amount) | ||
| return { | ||
| tag: 'success', | ||
| result: { kind: 'ARKADE_RPC_RESPONSE', id, method, payload: {} }, | ||
| } |
There was a problem hiding this comment.
Use the returned funding details in the response.
The props.fundAddress call returns { address, requestedAmount, fundedAmount } (per the Props type at lines 98-101), but the result is discarded and an empty payload is returned. The escrow client likely expects these values to confirm the funding operation.
Apply this diff:
case 'fund-address': {
- await props.fundAddress(message.payload.address, message.payload.amount)
+ const { address, requestedAmount, fundedAmount } = await props.fundAddress(
+ message.payload.address,
+ message.payload.amount,
+ )
return {
tag: 'success',
- result: { kind: 'ARKADE_RPC_RESPONSE', id, method, payload: {} },
+ result: { kind: 'ARKADE_RPC_RESPONSE', id, method, payload: { address, requestedAmount, fundedAmount } },
}
}Also update the response type to match:
type RpcFundAddressResponse = {
method: 'fund-address'
- payload: {}
+ payload: {
+ address: string
+ requestedAmount: number
+ fundedAmount: number
+ }
}🤖 Prompt for AI Agents
In src/screens/Apps/Escrow/RpcHandler.ts around lines 169 to 174, the code calls
props.fundAddress but discards its returned { address, requestedAmount,
fundedAmount } and returns an empty payload; capture the returned funding
details, include them in the response payload instead of {}, and update the
response type to reflect { address, requestedAmount, fundedAmount } (or the
proper interface used elsewhere) so the escrow client receives the funding
confirmation.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/screens/Apps/Escrow/RpcHandler.ts (1)
106-115: Return a precise error for unknown RPC methods.Unknown methods currently fall through and return “Unknown message kind”. Add an inner default to report the method name.
switch (method) { @@ - case 'fund-address': { + case 'fund-address': { ... } + default: { + return { + tag: 'failure', + error: new Error(`Unknown RPC method: ${String((message as any).method)}`), + } + } } } } - return { tag: 'failure', error: new Error('Unknown message kind') } + return { tag: 'failure', error: new Error('Unknown message kind') }Also applies to: 173-176
src/screens/Apps/Escrow/Index.tsx (1)
33-39: Fix stale dependency: include svcWallet in useEffect deps.Effect guards on
svcWalletbut doesn’t list it; escrow URL may never set after wallet init.- }, [aspInfo]) + }, [aspInfo, svcWallet])
🧹 Nitpick comments (2)
src/screens/Apps/Escrow/RpcHandler.test.ts (1)
189-189: Add a test for unknown RPC methods.Covers the new inner default path.
@@ }) + it('returns failure for unknown RPC method', async () => { + const handler = makeMessageHandler(props) + const res = await handler({ + kind: 'ARKADE_RPC_REQUEST', + id, + // @ts-expect-error testing unknown method + method: 'non-existent-method', + }) + expect(res.tag).toBe('failure') + if (res.tag === 'failure') { + expect(res.error.message).toContain('Unknown RPC method') + } + })src/screens/Apps/Escrow/Index.tsx (1)
77-88: Optional: stabilize handler identities with useCallback or full deps.To satisfy exhaustive-deps and avoid subtle staleness, either wrap each handler in
useCallbackwith proper deps or include all referenced functions in the array.If you want, I can push a small refactor that adds
useCallbackforsign,signin,signout,getXOnlyPublicKey,getArkWalletAddress, andfundAddress.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/screens/Apps/Escrow/Index.tsx(1 hunks)src/screens/Apps/Escrow/RpcHandler.test.ts(1 hunks)src/screens/Apps/Escrow/RpcHandler.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/screens/Apps/Escrow/RpcHandler.test.ts (1)
src/screens/Apps/Escrow/RpcHandler.ts (1)
makeMessageHandler(101-178)
src/screens/Apps/Escrow/Index.tsx (5)
src/providers/navigation.tsx (1)
NavigationContext(159-163)src/providers/wallet.tsx (1)
WalletContext(39-53)src/providers/flow.tsx (1)
FlowContext(82-95)src/providers/asp.tsx (1)
AspContext(15-20)src/screens/Apps/Escrow/ArkadeIframeHost.tsx (2)
ArkadeIdentityHandlers(6-14)ArkadeIframeHost(22-128)
| it('handles fund-address', async () => { | ||
| ;(props.fundAddress as any).mockResolvedValue({ address: 'ark1qq', requestedAmount: 1, fundedAmount: 1 }) | ||
| const handler = makeMessageHandler(props) | ||
| const res = await handler({ | ||
| kind: 'ARKADE_RPC_REQUEST', | ||
| id, | ||
| method: 'fund-address', | ||
| payload: { address: 'ark1qq', amount: 123 }, | ||
| }) | ||
| expect(props.fundAddress).toHaveBeenCalledWith('ark1qq', 123) | ||
| expect(res.tag).toBe('success') | ||
| if (res.tag === 'success') { | ||
| const out: any = res.result | ||
| expect(out.method).toBe('fund-address') | ||
| expect(out.payload).toEqual({}) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Update fund-address test to expect returned details.
After propagating funding details, assert the payload matches what the host returns.
it('handles fund-address', async () => {
- ;(props.fundAddress as any).mockResolvedValue({ address: 'ark1qq', requestedAmount: 1, fundedAmount: 1 })
+ ;(props.fundAddress as any).mockResolvedValue({ address: 'ark1qq', requestedAmount: 1, fundedAmount: 1 })
const handler = makeMessageHandler(props)
const res = await handler({
kind: 'ARKADE_RPC_REQUEST',
id,
method: 'fund-address',
payload: { address: 'ark1qq', amount: 123 },
})
expect(props.fundAddress).toHaveBeenCalledWith('ark1qq', 123)
expect(res.tag).toBe('success')
if (res.tag === 'success') {
const out: any = res.result
expect(out.method).toBe('fund-address')
- expect(out.payload).toEqual({})
+ expect(out.payload).toEqual({ address: 'ark1qq', requestedAmount: 123, fundedAmount: 123 })
}
})🤖 Prompt for AI Agents
In src/screens/Apps/Escrow/RpcHandler.test.ts around lines 162 to 177, the test
currently asserts the handler returns an empty payload for 'fund-address' but
the host now returns funding details; update the expectation to assert that
res.result.payload equals the object resolved by props.fundAddress (e.g., {
address: 'ark1qq', requestedAmount: 1, fundedAmount: 1 }) and keep the existing
call assertion (expect(props.fundAddress).toHaveBeenCalledWith('ark1qq', 123)).
| type RpcFundAddressResponse = { | ||
| method: 'fund-address' | ||
| payload: {} | ||
| } |
There was a problem hiding this comment.
Propagate funding details; fix type contract to match host wrapper.
The handler discards { address, requestedAmount, fundedAmount } and returns {}. ArkadeIframeHost passes a fundAddress that returns these details, so they're lost here and the client likely needs them. Update types and include them in the response payload.
Apply:
@@
type RpcFundAddressResponse = {
method: 'fund-address'
- payload: {}
+ payload: {
+ address: string
+ requestedAmount: number
+ fundedAmount: number
+ }
}
@@
type Props = {
getXOnlyPublicKey: () => Promise<Uint8Array | null>
signLoginChallenge: (challenge: string) => Promise<string>
getArkWalletAddress: () => Promise<string | undefined>
signArkTransaction: (tx: string, checkpoints: string[]) => Promise<{ signedTx: string; signedCheckpoints: string[] }>
- fundAddress: (address: string, amount: number) => Promise<void>
+ fundAddress: (
+ address: string,
+ amount: number,
+ ) => Promise<{ address: string; requestedAmount: number; fundedAmount: number }>
}
@@
- case 'fund-address': {
- await props.fundAddress(message.payload.address, message.payload.amount)
- return {
- tag: 'success',
- result: { kind: 'ARKADE_RPC_RESPONSE', id, method, payload: {} },
- }
- }
+ case 'fund-address': {
+ const { address, requestedAmount, fundedAmount } = await props.fundAddress(
+ message.payload.address,
+ message.payload.amount,
+ )
+ return {
+ tag: 'success',
+ result: {
+ kind: 'ARKADE_RPC_RESPONSE',
+ id,
+ method,
+ payload: { address, requestedAmount, fundedAmount },
+ },
+ }
+ }Also applies to: 93-99, 166-171
🤖 Prompt for AI Agents
In src/screens/Apps/Escrow/RpcHandler.ts around lines 66-69 (and also at 93-99
and 166-171), the RpcFundAddressResponse type and corresponding handler return
an empty payload but the host wrapper (ArkadeIframeHost) actually returns {
address, requestedAmount, fundedAmount }; update the RpcFundAddressResponse type
payload to include these three fields (address: string, requestedAmount: number
| string as appropriate, fundedAmount: number | string) and modify the handler
code at each occurrence to propagate the returned object instead of {} (return
the actual { address, requestedAmount, fundedAmount } from the host call),
keeping types aligned and preserving null/undefined handling if the host may
return partial data.
790c85e to
ec39bb5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/screens/Apps/Escrow/Index.tsx (1)
33-38: AddsvcWalletto the useEffect dependency array.The effect checks
svcWalleton line 34 but doesn't include it in the dependency array. This could prevent the escrow URL from being set when the wallet initializes.Apply this diff:
useEffect(() => { if (!aspInfo.network || !svcWallet) return const baseUrl = BASE_URLS[aspInfo.network as Network] if (!baseUrl) return // No escrow app for this network setEscrowAppUrl(baseUrl) - }, [aspInfo]) + }, [aspInfo, svcWallet])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/escrow.pngis excluded by!**/*.png
📒 Files selected for processing (10)
README.md(1 hunks)src/icons/Escrow.tsx(1 hunks)src/providers/navigation.tsx(3 hunks)src/screens/Apps/Escrow/ArkadeIframeHost.tsx(1 hunks)src/screens/Apps/Escrow/Index.tsx(1 hunks)src/screens/Apps/Escrow/RpcHandler.test.ts(1 hunks)src/screens/Apps/Escrow/RpcHandler.ts(1 hunks)src/screens/Apps/Index.tsx(2 hunks)src/vite-env.d.ts(1 hunks)vite.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/icons/Escrow.tsx
- src/screens/Apps/Escrow/RpcHandler.test.ts
- src/providers/navigation.tsx
- src/vite-env.d.ts
- src/screens/Apps/Escrow/ArkadeIframeHost.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-30T18:33:29.839Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.
Applied to files:
src/screens/Apps/Escrow/Index.tsxsrc/screens/Apps/Escrow/RpcHandler.ts
🧬 Code graph analysis (2)
src/screens/Apps/Index.tsx (1)
src/icons/Escrow.tsx (1)
EscrowIcon(1-3)
src/screens/Apps/Escrow/Index.tsx (5)
src/providers/navigation.tsx (1)
NavigationContext(169-173)src/providers/wallet.tsx (1)
WalletContext(39-53)src/providers/flow.tsx (1)
FlowContext(82-95)src/providers/asp.tsx (1)
AspContext(15-20)src/screens/Apps/Escrow/ArkadeIframeHost.tsx (2)
ArkadeIdentityHandlers(6-14)ArkadeIframeHost(22-121)
🔇 Additional comments (7)
src/screens/Apps/Index.tsx (1)
118-118: LGTM!The Escrow app is properly integrated into the Apps grid with the correct live status and navigation wiring.
README.md (1)
35-35: LGTM!The new
VITE_ARK_ESCROW_URLenvironment variable is properly documented.src/screens/Apps/Escrow/Index.tsx (1)
77-88: LGTM!The handlers are properly memoized with correct dependencies to avoid stale closures.
src/screens/Apps/Escrow/RpcHandler.ts (4)
66-69: Verify that empty payload is acceptable for fund-address response.The
fund-addressRPC response returns an empty payload, which means the escrow client has no confirmation of the funding request (address, requested amount, or funded amount). If the client needs to verify the funding operation, consider returning these details.If the current void return is intentional (perhaps the client polls or receives updates another way), this is fine.
Do you want me to help implement a version that returns funding confirmation details?
109-132: LGTM!The public key and wallet address RPC handlers are correctly implemented with proper null handling.
134-144: LGTM!Login challenge signing is properly implemented with error handling.
146-164: LGTM!Transaction signing handles both the transaction and checkpoints correctly with appropriate error handling.
| ], | ||
| server: { | ||
| port: 3002, | ||
| port: 3003, |
There was a problem hiding this comment.
🧩 Analysis chain
Update README to reflect the new port.
The dev server port changed to 3003, but README.md line 57 still references port 3002.
🏁 Script executed:
#!/bin/bash
# Verify README references to the old port
rg -n "3002" README.mdLength of output: 81
---
🏁 Script executed:
```shell
#!/bin/bash
# Verify vite.config.ts shows port 3003 at line 16 and check for other port 3002 references
echo "=== vite.config.ts around line 16 ==="
head -20 vite.config.ts | tail -10
echo ""
echo "=== All references to port 3002 in repo ==="
rg -n "3002" --type-list | head -1
rg -n "3002"
Length of output: 3198
---
🏁 Script executed:
```shell
#!/bin/bash
# Find vite.config.ts and verify port 3003, then check context of other port references
echo "=== Finding vite.config.ts ==="
fd "vite.config" git/
echo ""
echo "=== Content of vite.config.ts ==="
find git/ -name "vite.config.ts" -exec cat {} \;
echo ""
echo "=== Context around port 3002 in playwright.config.ts ==="
rg -B2 -A2 "port: 3002" git/playwright.config.ts
echo ""
echo "=== Context around port 3002 in package.json ==="
rg -B2 -A2 "3002" git/package.json
Length of output: 427
🏁 Script executed:
#!/bin/bash
# Find vite.config.ts and verify port 3003, then check context of other port references
echo "=== Finding vite.config.ts ==="
fd "vite.config"
echo ""
echo "=== Content of vite.config.ts ==="
cat vite.config.ts
echo ""
echo "=== Context around port 3002 in playwright.config.ts ==="
rg -B2 -A2 "port: 3002" playwright.config.ts
echo ""
echo "=== Context around port 3002 in package.json ==="
rg -B2 -A2 "3002" package.jsonLength of output: 1004
Update all port references from 3002 to 3003 across the codebase.
The review comment correctly identifies that README.md needs updating, but verification reveals the fix is incomplete. Multiple other files reference the old port 3002 and will break or fail to connect:
README.mdline 57playwright.config.tsline 21 (webServer port config)package.jsonline 37 (test:codegen script)docs/swaps.regtest.mdlines 152, 174, 198
Update all references to reflect the new dev server port 3003.
🤖 Prompt for AI Agents
In vite.config.ts around line 16 and across the repo (README.md line 57,
playwright.config.ts line 21, package.json line 37, and docs/swaps.regtest.md
lines 152, 174, 198) change any hard-coded port value 3002 to 3003 so all dev
server and test references match the new port; update the webServer.port in
playwright.config.ts, the test:codegen script in package.json, the README and
docs lines, then run the test suite and a local dev start to verify no remaining
3002 references remain.
* Fix showing password when server is unreachable * new server down e2e test
* add swap e2e tests * fix lint * CI: use_ln: true * Fix showing password when server is unreachable (arkade-os#234) * Fix showing password when server is unreachable * new server down e2e test * use docker images --------- Co-authored-by: João Bordalo <bordalix@users.noreply.github.com>
* Add ChatWoot integration for customer support - Added Support settings page under Settings menu - Implemented ChatWoot widget integration with secure chat functionality - Users can now report bugs and track support conversations from the wallet - Added useChatwoot hook for managing widget lifecycle - Support page displays when ChatWoot is configured via environment variables - Added VITE_CHATWOOT_WEBSITE_TOKEN and VITE_CHATWOOT_BASE_URL to configuration - Integration includes user identification and custom attributes (wallet pubkey, network) * Include wallet addresses in ChatWoot support integration - Added ARK address (offchainAddr) to ChatWoot custom attributes - Added BTC boarding address (boardingAddr) to custom attributes - Included explorer URL for the current network - Support operators can now search user addresses in the blockchain explorer - Addresses are automatically fetched when support page is opened * Add comprehensive service URLs and addresses to ChatWoot - Added location origin (window.location.origin) for deployment context - Added ARK server URL and indexer URL (both use aspInfo.url) - Added wallet ARK address (offchainAddr) and BTC boarding address - Added blockchain explorer URL for the user's network - Added Boltz Lightning swap provider URL - Added LendaSat and LendaSwap application URLs - Support operators can now see all service endpoints and addresses - Enables quick troubleshooting by showing user's complete configuration * add chatwoot support * deal with error loading chatwoot * Add safety check before using non-null assertion * Fix the timeout logic bug * console.log to debug * console.log for debug * increase menu row height * remove debug console logs * increase height of menus --------- Co-authored-by: Claude <noreply@anthropic.com>
* refactor nostr backups to use chunks * Add error handling around JSON parsing * fix enabling Nostr backup doesn’t persist config change * use consoleError instead of console.error * new regtest service: nak * update docker images * adds nostr test * fixes * tidy code * Chatwoot (arkade-os#236) * Add ChatWoot integration for customer support - Added Support settings page under Settings menu - Implemented ChatWoot widget integration with secure chat functionality - Users can now report bugs and track support conversations from the wallet - Added useChatwoot hook for managing widget lifecycle - Support page displays when ChatWoot is configured via environment variables - Added VITE_CHATWOOT_WEBSITE_TOKEN and VITE_CHATWOOT_BASE_URL to configuration - Integration includes user identification and custom attributes (wallet pubkey, network) * Include wallet addresses in ChatWoot support integration - Added ARK address (offchainAddr) to ChatWoot custom attributes - Added BTC boarding address (boardingAddr) to custom attributes - Included explorer URL for the current network - Support operators can now search user addresses in the blockchain explorer - Addresses are automatically fetched when support page is opened * Add comprehensive service URLs and addresses to ChatWoot - Added location origin (window.location.origin) for deployment context - Added ARK server URL and indexer URL (both use aspInfo.url) - Added wallet ARK address (offchainAddr) and BTC boarding address - Added blockchain explorer URL for the user's network - Added Boltz Lightning swap provider URL - Added LendaSat and LendaSwap application URLs - Support operators can now see all service endpoints and addresses - Enables quick troubleshooting by showing user's complete configuration * add chatwoot support * deal with error loading chatwoot * Add safety check before using non-null assertion * Fix the timeout logic bug * console.log to debug * console.log for debug * increase menu row height * remove debug console logs * increase height of menus --------- Co-authored-by: Claude <noreply@anthropic.com> * Fix UI incoherence on Boltz app page --------- Co-authored-by: Claude <noreply@anthropic.com>
…rkade-os#237) * Initial plan * Add currency toggle feature to mobile keyboard input Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * Format code with prettier Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * Address code review feedback: improve accessibility and precision Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * Add Playwright e2e test for currency toggle feature Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * Improve Playwright test with clearAmount helper function Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * Refactor e2e tests with shared helper functions Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * general fixes to input amount in mobile using fiat * fix swap icons * fix tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> Co-authored-by: João Bordalo <bordalix@users.noreply.github.com>
Signed-off-by: Marco Argentieri <3596602+tiero@users.noreply.github.com>
* more unit tests * fixes
* Initial plan * Add CHF as fiat currency option Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * Apply suggestion from @tiero Signed-off-by: Marco Argentieri <3596602+tiero@users.noreply.github.com> --------- Signed-off-by: Marco Argentieri <3596602+tiero@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> Co-authored-by: João Bordalo <bordalix@users.noreply.github.com>
* fix input amount in fiat only * fixes * more fixes
* fix threshold computation when all VTXOs are swept * remove console.log * revert * get all vtxos from svcWallet * worker update
* refactor nsec backup * wip * new modal component * wip * authenticate with biometrics * fix tests * fix tests * add testid to tabs * more testid * fix orange
* improve onboard images * remove not used css classes
…s in restricted environments (arkade-os#192) * Initial plan * Add JavaScript/JIT capability detection with informative error messages Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * Format code with prettier Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> * rebuild service worker * refactor how blocking errors are shown * fix tests --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tiero <3596602+tiero@users.noreply.github.com> Co-authored-by: João Bordalo <bordalix@users.noreply.github.com>
af4f3f9 to
c36a8e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/nostr.ts (1)
46-75: Add pool cleanup in thesave()method.The
save()method callsthis.pool.publish()but never closes the pool connection. According to nostr-tools documentation, SimplePool requires explicitclose()calls to prevent resource leaks. This is inconsistent with theload()method (line 103), which properly closes its subscription. Either callthis.pool.close(this.relays)after the publish operation completes, or implement a cleanup method for the NostrStorage instance.src/screens/Apps/Index.tsx (2)
57-73: Remove unuseddeepLinkInfoand normalize test IDs
deepLinkInfois imported fromFlowContexton line 96 but unused since the conditional logic is hardcoded and commented out (line 97). Remove it from the destructuring.Also,
data-testid={app-${name}}produces IDs with spaces (e.g.,app-Fuji Money). Normalize to lowercase with dashes replaced:app-${name.toLowerCase().replace(/\s+/g, '-')}for cleaner test selectors.
23-37:React.CSSPropertiesusage requires proper type importThe file uses
React.CSSPropertieson lines 24 and 60 (in bothTagandAppcomponents), but does not importReactas a namespace. This causes a TypeScript error: "Cannot find name 'React'".Import
CSSPropertiesas a named type instead:-import { ReactElement, useContext } from 'react' +import type { CSSProperties, ReactElement } from 'react' +import { useContext } from 'react'Then replace both usages:
- const style: React.CSSProperties = { + const style: CSSProperties = {This applies to the
Tagcomponent (line 24) andAppcomponent (line 60).
♻️ Duplicate comments (2)
src/screens/Apps/Escrow/RpcHandler.ts (2)
208-211: Return a specific error for unknown RPC methods.When an unknown method is received within
ARKADE_RPC_REQUEST, the code falls through to line 211 returning a generic "Unknown message kind" error. Adding adefaultcase in the inner switch would provide a more specific error message.case 'fund-address': { // ... } + default: { + const _exhaustive: never = method + return { tag: 'failure', error: new Error(`Unknown RPC method: ${method}`) } + } } } } - return { tag: 'failure', error: new Error('Unknown message kind') } + return { tag: 'failure', error: new Error(`Unknown message kind: ${(message as any).kind}`) }This also enables exhaustiveness checking via TypeScript.
76-79: Type mismatch:RpcFundAddressResponsepayload is empty but handler returns{ txid }.The response type declares
payload: {}but the handler at line 199 returnspayload: { txid }. This causes a type inconsistency.Apply this diff:
type RpcFundAddressResponse = { method: 'fund-address' - payload: {} + payload: { + txid: string + } }
🟡 Minor comments (13)
src/ionic.css-3-3 (1)
3-3: Misleading variable name: --cream is purple.The
--creamvariable is set to#391998, which is a purple color, not cream. This could cause confusion for future maintainers.Consider renaming to match the actual color, or update the color value to an actual cream color:
- --cream: #391998; + --cream: #f5f5dc; /* actual cream color */Or rename the variable:
- --cream: #391998; + --purple-alt: #391998;Committable suggestion skipped: line range outside the PR's diff.
src/screens/Settings/Sign.tsx-1-1 (1)
1-1: Fix Prettier formatting issues.The pipeline detected formatting issues. Run
prettier --write src/screens/Settings/Sign.tsxto fix them.src/screens/Settings/Sign.tsx-70-70 (1)
70-70: Handle potential undefined pubkey.The
pubkeycould be undefined (line 70), but the copy button on line 96 uses it without a null check. While line 95 handles display withpubkey ? ... : '-', the clipboard operation should also be guarded.Apply this diff to fix the issue:
- <Button label="copy" onClick={() => navigator.clipboard.writeText(pubkey ? hex.encode(pubkey) : '')} /> + <Button label="copy" onClick={() => pubkey && navigator.clipboard.writeText(hex.encode(pubkey))} />Also applies to: 95-96
src/test/screens/settings/fiat.test.tsx-8-19 (1)
8-19: Incomplete test coverage for CHF option.The test validates only EUR (select-option-0) and USD (select-option-1), but the Fiat component now includes CHF as a third option. Add verification for
select-option-2to ensure complete coverage.Add this assertion after line 18:
expect(screen.getByTestId('select-option-1').querySelector('p')?.textContent).toBe('USD') + expect(screen.getByTestId('select-option-2')).toBeInTheDocument() + expect(screen.getByTestId('select-option-2').querySelector('p')?.textContent).toBe('CHF') })src/screens/Apps.tsx-1-92 (1)
1-92: Fix Prettier formatting issues.The CI pipeline reports formatting issues in this file. Run
prettier --write src/screens/Apps.tsxto resolve the code style violations.src/providers/escrow.tsx-1-1 (1)
1-1: Address Prettier formatting issues.The pipeline detected Prettier formatting issues. Run
prettier --write src/providers/escrow.tsxto fix.src/screens/Settings/Support.tsx-62-85 (1)
62-85: Remove invalid dependency from useEffect.Line 85 includes
window.$chatwootin the dependency array, but this is not a valid React dependency. Global objects should not be in dependency arrays. Consider using a state flag to track when Chatwoot is ready instead.Apply this diff:
useEffect(() => { if (!addresses || !supportChatLoaded || !window.$chatwoot || !wallet.pubkey) return // Set user identifier (using wallet pubkey) const userIdentifier = wallet.pubkey.substring(0, 16) window.$chatwoot.setUser(userIdentifier, { name: `User ${userIdentifier}` }) // Set custom attributes including addresses and service URLs window.$chatwoot.setCustomAttributes({ wallet_pubkey: wallet.pubkey, network: wallet.network || 'not available', location_origin: window.location.origin, ark_address: addresses.offchainAddr || 'not available', boltz_url: swapProvider?.getApiUrl() || 'not available', indexer_url: aspInfo.url || config.aspUrl || 'not available', btc_boarding_address: addresses.boardingAddr || 'not available', ark_server_url: aspInfo.url || config.aspUrl || 'not available', app_version: import.meta.env.VITE_APP_VERSION || 'not available', lendasat_url: import.meta.env.VITE_LENDASAT_IFRAME_URL || 'not available', lendaswap_url: import.meta.env.VITE_LENDASWAP_IFRAME_URL || 'not available', explorer_url: wallet.network ? getWebExplorerURL(wallet.network as NetworkName) : 'not available', }) - }, [addresses, supportChatLoaded, wallet.pubkey, window.$chatwoot]) + }, [addresses, supportChatLoaded, wallet.pubkey])src/lib/escrow.ts-1-7 (1)
1-7: Prettier formatting issues flagged by pipeline.The pipeline indicates formatting issues. Also,
getPrivateKeyis imported but not used in this file.Run
prettier --write src/lib/escrow.tsand remove the unused import:import { ArkInfo, ServiceWorkerWallet } from '@arkade-os/sdk' import { bytesToHex, hexToBytes } from '@noble/hashes/utils' -import { getPrivateKey } from './privateKey' import { schnorr } from '@noble/curves/secp256k1' -const fetcher = <T>(p: Promise<Response>): Promise<T> => - p.then(r => r.ok ? r.json() as T : Promise.reject(r)) +const fetcher = <T>(p: Promise<Response>): Promise<T> => + p.then((r) => (r.ok ? (r.json() as T) : Promise.reject(r)))src/screens/Apps/Escrow/RpcHandler.test.ts-150-166 (1)
150-166: Test expectation does not match handler implementation.The mock returns
undefined, but according toRpcHandler.ts(lines 194-199), the handler returns{ txid }fromprops.fundAddress. The test should mock a txid value and assert the payload contains it.it('handles fund-address', async () => { - ;(props.fundAddress as any).mockResolvedValue(undefined) + ;(props.fundAddress as any).mockResolvedValue('txid-abc123') const handler = makeMessageHandler(props) const res = await handler({ kind: 'ARKADE_RPC_REQUEST', id, method: 'fund-address', payload: { address: 'ark1qq', amount: 123 }, }) expect(props.fundAddress).toHaveBeenCalledWith('ark1qq', 123) expect(res.tag).toBe('success') if (res.tag === 'success') { const out: any = res.result expect(out.method).toBe('fund-address') - expect(out.payload).toEqual({}) + expect(out.payload).toEqual({ txid: 'txid-abc123' }) } })src/screens/Apps/Escrow/ArkadeIframeHost.tsx-67-71 (1)
67-71: Missing cleanup for load event listener.The load event listener is never removed, causing a potential memory leak. Also, if the iframe ref changes or reloads, multiple polling chains could start.
useEffect(() => { - iframeRef.current?.addEventListener('load', () => { - poll() - }) + const iframe = iframeRef.current + const handleLoad = () => poll() + iframe?.addEventListener('load', handleLoad) + return () => { + iframe?.removeEventListener('load', handleLoad) + } }, [poll])src/providers/config.tsx-50-55 (1)
50-55: Silent error swallowing inbackupConfig.The
.catch()logs the error but the Promise resolves successfully, so callers cannot detect failures. If callers need to handle backup errors (e.g., show UI feedback), they won't be able to.Consider re-throwing after logging if callers need to handle failures:
const backupConfig = async (config: Config) => { const backupProvider = new BackupProvider({ pubkey: config.pubkey }) - await backupProvider.backupConfig(config).catch((error) => { - consoleError(error, 'Backup to Nostr failed') - }) + try { + await backupProvider.backupConfig(config) + } catch (error) { + consoleError(error, 'Backup to Nostr failed') + throw error + } }Alternatively, document that this function intentionally suppresses errors.
src/components/Keyboard.tsx-35-39 (1)
35-39: Missing dependencies in useEffect.This effect depends on
inputMode,toFiat, andgetMaxDecimalsbut only includesvaluein the dependency array. If the user toggles currency mode whilevalueremains unchanged, thetextValuewon't update correctly.useEffect(() => { if (!value) return setTextValue('') const amount = inputMode === 'fiat' ? toFiat(value) : value setTextValue(prettyNumber(amount, getMaxDecimals())) - }, [value]) + }, [value, inputMode, toFiat])Committable suggestion skipped: line range outside the PR's diff.
src/screens/Settings/Backup.tsx-63-75 (1)
63-75: Error value fromauthenticateUseris set but not displayed in the biometric flow.At line 66,
.catch(setError)stores the error, but the Dialog showsErrorMessageonly for the password input path (line 117), not the biometric path. Users won't see authentication failures when using biometrics.Consider displaying the error in the biometric section as well:
wallet.lockedByBiometrics ? ( <FlexCol centered gap='0.5rem'> <FingerprintIcon /> <Text centered>Unlock with your passkey</Text> + <ErrorMessage error={Boolean(error)} text={error} /> </FlexCol> ) : (Committable suggestion skipped: line range outside the PR's diff.
🧹 Nitpick comments (33)
src/components/Button.tsx (1)
42-42: Consider moving the hard‑coded margin into a CSS utility/classInline
style={{ margin: '4px 0' }}works but makes spacing harder to customize or theme globally. Consider shifting this to a reusable CSS/utility class (or layout container) so spacing can be adjusted without touching the component code.src/components/Header.tsx (1)
47-47: Consider conditionally rendering the aria-label.When
auxAriaLabelis undefined, this results inaria-label={undefined}. While React handles this gracefully by not rendering the attribute, it's cleaner to conditionally include it only when defined.Apply this diff to conditionally render the aria-label:
- <div style={style} onClick={auxFunc} aria-label={auxAriaLabel}> + <div style={style} onClick={auxFunc} {...(auxAriaLabel && { 'aria-label': auxAriaLabel })}>src/components/Modal.tsx (2)
15-23: Prefer CSS classes or memoized style objects over inline literals
innerStyleis recreated on every render and hard-codes stylistic concerns in JS. For consistency with the rest of the app and easier theming, consider moving these rules into a CSS/SCSS module or a styled component, or at least hoisting the style objects outside the component so they’re not reallocated per render.
24-30: Tighten markup and add basic modal accessibility hooksThe fragment wrapping the single outer
<div>is unnecessary, and the modal lacks semantic/a11y hints (e.g.,role="dialog"andaria-modal="true").You could simplify and improve semantics:
- return ( - <> - <div style={outerStyle}> - <div style={innerStyle}>{children}</div> - </div> - </> - ) + return ( + <div style={outerStyle} role="dialog" aria-modal="true"> + <div style={innerStyle}>{children}</div> + </div> + )If this component is intended as a general modal, you may also want to handle focus trapping and closing (ESC / backdrop click) higher up in the stack.
nak.Dockerfile (1)
5-5: Consider pinning the nak version for reproducible builds.Using
@latestfor the nak installation can lead to non-reproducible builds and potential breakage from upstream changes.Consider pinning to a specific version:
-RUN go install github.com/fiatjaf/nak@latest +RUN go install github.com/fiatjaf/nak@v0.x.xOr document that latest is intentional if dynamic updates are desired.
src/screens/Apps/Boltz/Swap.tsx (1)
46-62: Consider caching decoded invoice to avoid redundant calls.The
decodeInvoice(invoice)function is called twice (lines 46 and 54). For efficiency, consider decoding once and reusing the result.Apply this diff to optimize:
const isReverse = swapInfo.type === 'reverse' const refunded = !isReverse && swapInfo.refunded const kind = isReverse ? 'Reverse Swap' : 'Submarine Swap' const direction = isReverse ? 'Lightning to Arkade' : 'Arkade to Lightning' const invoice = isReverse ? swapInfo.response.invoice : swapInfo.request.invoice const address = isReverse ? swapInfo.response.lockupAddress : swapInfo.response.address const total = isReverse ? swapInfo.request.invoiceAmount : swapInfo.response.expectedAmount -const amount = isReverse ? swapInfo.response.onchainAmount : decodeInvoice(invoice).amountSats +const decodedInvoice = decodeInvoice(invoice) +const amount = isReverse ? swapInfo.response.onchainAmount : decodedInvoice.amountSats const formatAmount = (amt: number) => (config.showBalance ? prettyAmount(amt) : prettyHide(amt)) const data = [ ['When', prettyAgo(swapInfo.createdAt)], ['Kind', kind], ['Swap ID', swapInfo.response.id], - ['Description', decodeInvoice(invoice).note || 'N/A'], + ['Description', decodedInvoice.note || 'N/A'], ['Direction', direction], ['Date', prettyDate(swapInfo.createdAt)], ['Invoice', invoice], ['Preimage', swapInfo.preimage || 'N/A'], ['Address', address || 'N/A'], ['Status', swapInfo.status], ['Amount', formatAmount(amount)], ['Fees', formatAmount(total - amount)], ['Total', formatAmount(total)], ]src/screens/Settings/Sign.tsx (3)
19-47: Remove unused LogsTable component.The
LogsTablecomponent is defined but never used in this file. This appears to be leftover code that should be removed or moved to a separate file if it's intended for future use.Apply this diff to remove the unused component:
-function LogsTable({ logs }: { logs: LogLine[] }) { - const color = (level: string): string => { - if (level === 'info') return '' - if (level === 'warn') return 'orange' - if (level === 'error') return 'red' - return '' - } - - const numChars = (v: string) => Math.floor((36 - v.length) / 2) - - if (logs.length === 0) { - return <EmptyList text='No logs available' secondaryText='Start using the app to generate logs.' /> - } - - return ( - <div style={{ margin: '1rem' }}> - <FlexCol gap='0.5rem'> - {[...logs].reverse().map(({ time, msg, level }) => ( - <FlexRow between key={`${time}${msg}`}> - <Text color={color(level)}>{prettyAgo(time)}</Text> - <Text color='dark50' copy={msg}> - {prettyLongText(msg.replace('...', ''), numChars(prettyAgo(time)))} - </Text> - </FlexRow> - ))} - </FlexCol> - </div> - ) -} -
76-86: Consider replacing prompt() with a proper password input component.Using
prompt()provides poor UX (blocks the UI thread, displays a native dialog) and lacks proper password masking in some browsers. Consider using a password Input component with proper validation.
49-66: Complete or remove the incomplete transaction signing feature.The
PendingTransactiontype,toPendingTransactionparser, andtransactionToSignstate are defined, but the transaction signing functionality is not implemented. The "Sign" button at the bottom has no onClick handler.Do you want me to help implement the transaction signing feature, or should this incomplete code be removed for now?
Also applies to: 72-74, 110-118
src/components/Select.tsx (1)
34-40: Consider removing redundant key prop.The
FlexRowhas akeyprop (line 37), but its parentdivalready has the samekey(line 33). React only requires the key on the outermost element in the map iteration. The inner key is redundant.Apply this diff to remove the redundant key:
<FlexRow between testId={`select-option-${index}`} - key={option} onClick={() => onChange(option)} padding='0.8rem 0' >src/components/Bullet.tsx (1)
2-10: Redundant React.CSSProperties typing onstyleYou’re both annotating the variable type and casting the object to
React.CSSProperties. One is enough; consider dropping theas React.CSSPropertiesor the explicit variable type to keep things cleaner.src/lib/fiat.ts (1)
6-6: CHF feed integration matches existing EUR/USD patternExtending
FiatPriceswithchfand wiring it tojson.CHF?.lastkeepsgetPriceFeedconsistent with the existing EUR/USD handling. Just ensure downstream code treats CHF the same way (and tolerates missing data if the API ever omits CHF).Also applies to: 16-17
src/test/screens/settings/display.test.tsx (1)
7-32: Display tests are clear; consider DRYing repeated render setupThe two tests correctly validate labels and default selection. You could slightly reduce duplication by extracting a small helper to render
<Display />with the mockedConfigContext.Provider, but that’s purely cosmetic.src/test/screens/settings/about.test.tsx (1)
7-28: About screen test reads well; copy coupling is acceptable hereThe test verifies the expected texts under the mocked
AspContextand should catch regressions in the About layout. It is quite copy‑sensitive, but that seems intentional for this settings screen.src/screens/Settings/Theme.tsx (1)
10-24: Theme change flow with backup looks consistentCreating
newConfig, conditionally awaitingbackupConfigwhennostrBackupis enabled, then callingupdateConfigmatches the backup pattern used elsewhere; wiring intoSelectviahandleChangeis straightforward.src/screens/Wallet/Unavailable.tsx (1)
16-27: Add aspInfo to the useEffect dependency array.The
useEffectreadsaspInfo.unreachablebut doesn't includeaspInfoin the dependency array. IfaspInfochanges after the component mounts, the error message won't update accordingly.Apply this diff:
detectJSCapabilities() .then((result) => { if (result.isSupported) return // Use specific error message or fallback to iOS/generic message setError(result.errorMessage || getRestrictedEnvironmentMessage(isIOS())) }) .catch(() => { setError(getRestrictedEnvironmentMessage(isIOS())) }) - }, []) + }, [aspInfo])src/providers/escrow.tsx (2)
31-32: Refactor to handle dependency changes.The
escrowClientis initialized inuseStatewith dependencies (aspInfo.network,aspInfo,svcWallet) that may change over time, but there's no effect to update it when these dependencies change. This could lead to stale client instances.Consider adding a
useEffectto update the client when dependencies change:const [escrowClient, setEscrowClient] = useState<EscrowClient>(new EscrowClient(BASE_URLS[aspInfo.network], aspInfo, svcWallet!)) const [ isSignedIn, setIsSignedIn ] = useState(false) + + useEffect(() => { + if (svcWallet) { + setEscrowClient(new EscrowClient(BASE_URLS[aspInfo.network], aspInfo, svcWallet)) + } + }, [aspInfo.network, aspInfo, svcWallet])
6-7: Remove unused imports.
BoltzUrlandconsoleErrorare imported but never used in this file.Apply this diff:
-import { BoltzUrl } from '../lib/constants' -import { consoleError } from '../lib/logs'src/screens/Settings/Support.tsx (1)
67-68: Add safety check for short pubkeys.Line 67 uses
substring(0, 16)without verifying thatwallet.pubkeyhas sufficient length. While unlikely, if the pubkey is shorter than 16 characters, this could produce an unexpectedly short identifier.Consider adding a minimum length check or using the full pubkey if it's short:
- const userIdentifier = wallet.pubkey.substring(0, 16) + const userIdentifier = wallet.pubkey.length >= 16 ? wallet.pubkey.substring(0, 16) : wallet.pubkeysrc/providers/fees.tsx (1)
22-52: Address TODO comments.All four fee calculation functions have TODO comments (lines 27, 35, 43, 51), suggesting the implementation may be incomplete or requires future work. Consider if additional logic is needed beyond reading from
aspInfo.fees.intentFee.Do these functions need additional vtxo-specific logic, or is reading from
aspInfo.fees.intentFeethe final implementation? If additional logic is planned, consider opening an issue to track this work.Would you like me to create a tracking issue for completing these fee calculation implementations?
src/lib/escrow.ts (1)
62-65: Consider implementing proper server-side signout.The TODO indicates server-side signout is not implemented. This could leave sessions active on the server after client-side signout.
Would you like me to open an issue to track implementing proper server-side signout, or help draft the implementation?
src/lib/chatwoot.ts (1)
28-33:getChatwootVarsmay return undefined values.
import.meta.envvalues could beundefinedif not set, but the return type declares them asstring. This could cause runtime issues when constructing the script URL.export const getChatwootVars = (): ChatwootVars => { + const websiteToken = import.meta.env.VITE_CHATWOOT_WEBSITE_TOKEN ?? '' + const baseUrl = import.meta.env.VITE_CHATWOOT_BASE_URL ?? '' return { - websiteToken: import.meta.env.VITE_CHATWOOT_WEBSITE_TOKEN, - baseUrl: import.meta.env.VITE_CHATWOOT_BASE_URL, + websiteToken, + baseUrl, } }src/lib/lightning.ts (1)
40-57: Clarify intent of swap re-fetch before backup.The logic fetches pending swaps and overwrites the passed swap with an existing one if found. This adds API calls on every backup. If the intent is to ensure the latest swap state is backed up, a brief comment would help future maintainers understand this design choice.
private backup = async (swap: PendingReverseSwap | PendingSubmarineSwap) => { if (!this.config.nostrBackup) return try { if (isPendingSubmarineSwap(swap)) { + // Fetch latest swap state to ensure backup includes any updates const pendingSwaps = await this.provider.getPendingSubmarineSwaps() const existingSwap = pendingSwaps.find((s) => s.id === swap.id) if (existingSwap) swap = existingSwap await this.backupProvider.backupSubmarineSwap(swap) } else if (isPendingReverseSwap(swap)) { + // Fetch latest swap state to ensure backup includes any updates const pendingSwaps = await this.provider.getPendingReverseSwaps() const existingSwap = pendingSwaps.find((s) => s.id === swap.id) if (existingSwap) swap = existingSwap await this.backupProvider.backupReverseSwap(swap) }src/test/e2e/nostr.test.ts (3)
78-78: Avoid hardcoded timeout; prefer explicit wait conditions.
waitForTimeout(1000)is a test smell that can cause flakiness. Prefer waiting for a specific condition like navigation or element visibility.await page.getByRole('contentinfo').getByText('Reset wallet').click() - await page.waitForTimeout(1000) + await page.waitForURL('**/') // or wait for specific element indicating reset complete
139-139: Consider handling the payment command result.Using
exec()without awaiting means payment failures won't be caught. While the test continues to wait for "Payment received!", an early failure indication would improve debugging.- exec(`docker exec lnd lncli --network=regtest payinvoice ${receiveInvoice} --force`) + execAsync(`docker exec lnd lncli --network=regtest payinvoice ${receiveInvoice} --force`) + .catch(err => console.error('Payment command failed:', err))
20-31: Consider extracting common test helpers.Both tests share wallet creation and restore flows. Extracting helpers like
createWallet(page),resetWallet(page),restoreWallet(page, nsec)would reduce duplication and improve maintainability.src/screens/Settings/Backup.tsx (2)
97-140: MoveDialogoutside the component to avoid re-creation on each render.Defining
Dialogas a nested function component causes it to be recreated on every render ofBackup, which can cause unnecessary re-mounts and loss of input focus. Extract it as a separate component or memoize it.- const Dialog = () => ( + const dialogContent = ( <FlexCol gap='1.5rem'> ... </FlexCol> )Then use
{dialogContent}inside the Modal instead of<Dialog />.
44-46: Missing dependency in useEffect.The
useEffectdepends onverifyPasswordbut doesn't include it in the dependency array. SinceverifyPasswordis stable (uses no external state), this is safe but triggers linter warnings. Consider adding an ESLint disable comment or movingverifyPasswordoutside the component.src/components/Keyboard.tsx (1)
101-105: Remove debugconsole.logbefore merging.The
console.logat line 102 appears to be debug output that should be removed for production.const handleSave = () => { - console.log('Keyboard - saving amountInSats:', amountInSats) onSats(amountInSats) back() }src/providers/config.tsx (1)
62-71:updateConfigis markedasyncbut performs no async operations.The function signature implies it returns a Promise, but the implementation is synchronous. This can mislead callers into awaiting unnecessarily or expecting Promise-based error handling.
Either remove
asyncor add a comment explaining the signature is for future compatibility:- const updateConfig = async (config: Config) => { + const updateConfig = (config: Config) => {src/lib/backup.ts (3)
85-86: Redundant null check:configis always defined.At line 86,
if (config)is unnecessary sinceconfigis a required parameter offullBackup(config: Config). This doesn't cause a bug but is misleading.if (dataSize > 65000) { - if (config) await this.backupConfig(config) + await this.backupConfig(config)
104-118:restoreassumesupdateConfignever throws.If
updateConfigthrows, the subsequent swap restoration loops won't execute. Consider wrapping in try/catch or documenting the expected behavior.Additionally, the console.log statements at lines 110 and 115 are useful for debugging but consider using a structured logger for consistency with
consoleErrorused elsewhere.
76-98: Consider batching swaps into groups rather than creating individual Nostr events.Each
backupReverseSwaporbackupSubmarineSwapcall creates a separate Nostr event. When the payload exceeds 65KB and the code loops through many swaps (lines 88-94), this results in numerous individual events being published, increasing relay round-trips and network overhead. Batching swaps into groups that fit under the size limit would reduce the number of events created and improve performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/escrow.pngis excluded by!**/*.png
📒 Files selected for processing (107)
.github/workflows/playwright.yml(1 hunks)LICENSE(1 hunks)README.md(1 hunks)cors.nginx.conf(1 hunks)index.html(1 hunks)nak.Dockerfile(1 hunks)package.json(2 hunks)playwright.config.ts(3 hunks)scripts/setupRegtestEnv.sh(0 hunks)src/App.tsx(6 hunks)src/components/AlertBox.tsx(1 hunks)src/components/Balance.tsx(1 hunks)src/components/Bullet.tsx(1 hunks)src/components/Button.tsx(2 hunks)src/components/CenterScreen.tsx(1 hunks)src/components/ChatWoot.tsx(1 hunks)src/components/Checkbox.tsx(1 hunks)src/components/FlexCol.tsx(3 hunks)src/components/FlexRow.tsx(3 hunks)src/components/Header.tsx(2 hunks)src/components/Info.tsx(2 hunks)src/components/Input.tsx(2 hunks)src/components/InputAddress.tsx(3 hunks)src/components/InputAmount.tsx(4 hunks)src/components/InputContainer.tsx(2 hunks)src/components/InputFake.tsx(1 hunks)src/components/InputWithScanner.tsx(3 hunks)src/components/Keyboard.tsx(2 hunks)src/components/Menu.tsx(1 hunks)src/components/Modal.tsx(1 hunks)src/components/Padded.tsx(1 hunks)src/components/Select.tsx(1 hunks)src/components/Shadow.tsx(2 hunks)src/components/StepBars.tsx(1 hunks)src/components/Strength.tsx(1 hunks)src/components/Title.tsx(1 hunks)src/components/Toggle.tsx(1 hunks)src/components/Warning.tsx(2 hunks)src/icons/Dont.tsx(1 hunks)src/icons/Escrow.tsx(1 hunks)src/icons/Logo.tsx(1 hunks)src/icons/Megaphone.tsx(1 hunks)src/icons/Ok.tsx(1 hunks)src/icons/Onboard2.tsx(2 hunks)src/icons/Safe.tsx(1 hunks)src/icons/Support.tsx(1 hunks)src/icons/Swap.tsx(3 hunks)src/icons/X.tsx(1 hunks)src/index.css(0 hunks)src/index.tsx(2 hunks)src/ionic.css(4 hunks)src/lib/asp.ts(4 hunks)src/lib/backup.ts(1 hunks)src/lib/chatwoot.ts(1 hunks)src/lib/escrow.ts(1 hunks)src/lib/fiat.ts(2 hunks)src/lib/format.ts(1 hunks)src/lib/jsCapabilities.ts(1 hunks)src/lib/lightning.ts(5 hunks)src/lib/nostr.ts(5 hunks)src/lib/types.ts(3 hunks)src/providers/config.tsx(5 hunks)src/providers/escrow.tsx(1 hunks)src/providers/fees.tsx(1 hunks)src/providers/fiat.tsx(2 hunks)src/providers/lightning.tsx(2 hunks)src/providers/navigation.tsx(6 hunks)src/providers/options.tsx(3 hunks)src/providers/wallet.tsx(3 hunks)src/screens/Apps.tsx(1 hunks)src/screens/Apps/Boltz/Index.tsx(1 hunks)src/screens/Apps/Boltz/Swap.tsx(1 hunks)src/screens/Apps/Escrow/ArkadeIframeHost.tsx(1 hunks)src/screens/Apps/Escrow/Index.tsx(1 hunks)src/screens/Apps/Escrow/RpcHandler.test.ts(1 hunks)src/screens/Apps/Escrow/RpcHandler.ts(1 hunks)src/screens/Apps/Index.tsx(6 hunks)src/screens/Init/Restore.tsx(3 hunks)src/screens/Init/Success.tsx(1 hunks)src/screens/Settings/Backup.tsx(2 hunks)src/screens/Settings/Display.tsx(1 hunks)src/screens/Settings/Fiat.tsx(1 hunks)src/screens/Settings/General.tsx(1 hunks)src/screens/Settings/Index.tsx(2 hunks)src/screens/Settings/Notifications.tsx(2 hunks)src/screens/Settings/Server.tsx(2 hunks)src/screens/Settings/Sign.tsx(1 hunks)src/screens/Settings/Support.tsx(1 hunks)src/screens/Settings/Theme.tsx(1 hunks)src/screens/Settings/Vtxos.tsx(2 hunks)src/screens/Wallet/Receive/Amount.tsx(7 hunks)src/screens/Wallet/Receive/QrCode.tsx(2 hunks)src/screens/Wallet/Send/Details.tsx(4 hunks)src/screens/Wallet/Send/Form.tsx(1 hunks)src/screens/Wallet/Unavailable.tsx(1 hunks)src/test/e2e/keyboard.test.ts(1 hunks)src/test/e2e/nostr.test.ts(1 hunks)src/test/e2e/send.test.ts(3 hunks)src/test/e2e/serverdown.test.ts(1 hunks)src/test/e2e/swap.test.ts(1 hunks)src/test/lib/fiat.test.ts(1 hunks)src/test/lib/jsCapabilities.test.ts(1 hunks)src/test/screens/mocks.ts(2 hunks)src/test/screens/settings/about.test.tsx(1 hunks)src/test/screens/settings/backup.test.tsx(1 hunks)src/test/screens/settings/display.test.tsx(1 hunks)src/test/screens/settings/fiat.test.tsx(1 hunks)
⛔ Files not processed due to max files limit (6)
- src/test/screens/settings/notifications.test.tsx
- src/test/screens/settings/theme.test.tsx
- src/test/setup-arkd.js
- src/test/setup.mjs
- src/vite-env.d.ts
- test.docker-compose.yml
💤 Files with no reviewable changes (2)
- scripts/setupRegtestEnv.sh
- src/index.css
✅ Files skipped from review due to trivial changes (2)
- LICENSE
- src/screens/Settings/General.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/icons/Escrow.tsx
- src/screens/Apps/Escrow/Index.tsx
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-07-21T18:25:36.391Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.
Applied to files:
src/components/InputFake.tsxsrc/components/InputAddress.tsxsrc/components/InputWithScanner.tsxsrc/test/screens/settings/backup.test.tsxsrc/screens/Settings/Backup.tsxsrc/components/InputAmount.tsxsrc/screens/Init/Restore.tsxsrc/components/Keyboard.tsxsrc/screens/Wallet/Send/Form.tsxsrc/components/Input.tsxsrc/screens/Wallet/Receive/Amount.tsxsrc/screens/Init/Success.tsxsrc/components/InputContainer.tsx
📚 Learning: 2025-06-30T18:33:29.839Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.
Applied to files:
src/screens/Wallet/Unavailable.tsxsrc/lib/jsCapabilities.tssrc/screens/Apps/Escrow/ArkadeIframeHost.tsxsrc/test/screens/mocks.tssrc/screens/Apps/Escrow/RpcHandler.tssrc/lib/escrow.tssrc/screens/Wallet/Send/Details.tsxsrc/lib/asp.tssrc/providers/escrow.tsxsrc/providers/wallet.tsxsrc/screens/Wallet/Receive/Amount.tsxsrc/test/e2e/send.test.tssrc/App.tsx
📚 Learning: 2025-07-07T14:41:35.991Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 106
File: src/lib/bolt11.ts:153-155
Timestamp: 2025-07-07T14:41:35.991Z
Learning: In src/lib/bolt11.ts, the code is copied from the original bitcoinjs/bolt11 library (https://github.com/bitcoinjs/bolt11/blob/master/payreq.js) and should be kept unchanged to maintain compatibility with the upstream source, even if there are potential improvements or syntax issues.
Applied to files:
src/screens/Apps/Boltz/Swap.tsxsrc/screens/Wallet/Receive/QrCode.tsxsrc/test/e2e/swap.test.tssrc/screens/Apps.tsxsrc/screens/Wallet/Receive/Amount.tsx
📚 Learning: 2025-06-30T18:23:34.163Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 114
File: public/wallet-service-worker.mjs:6350-6353
Timestamp: 2025-06-30T18:23:34.163Z
Learning: Do not review `public/wallet-service-worker.mjs` as it is a built file generated by Vite.
Applied to files:
src/lib/jsCapabilities.tssrc/providers/wallet.tsx
📚 Learning: 2025-08-06T09:53:28.428Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 134
File: src/screens/Settings/Password.tsx:34-39
Timestamp: 2025-08-06T09:53:28.428Z
Learning: In src/screens/Settings/Password.tsx, the saveNewPassword function doesn't need error handling for getPrivateKey because it's only called after authentication is verified. The useEffect validates the oldPassword using isValidPassword() which internally calls getPrivateKey(), and only sets authenticated=true if successful. The saveNewPassword function has a guard clause that returns early if not authenticated, so getPrivateKey(oldPassword) is guaranteed to work when called.
Applied to files:
src/screens/Settings/Sign.tsxsrc/test/screens/settings/backup.test.tsxsrc/screens/Settings/Backup.tsxsrc/screens/Init/Restore.tsx
📚 Learning: 2025-11-20T12:08:19.542Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 230
File: test.docker-compose.yml:46-46
Timestamp: 2025-11-20T12:08:19.542Z
Learning: The Docker image `ghcr.io/arkade-os/arkd-wallet:v0.8.7` exists and is available in the GitHub Container Registry for the arkade-os/wallet repository.
Applied to files:
src/screens/Apps/Escrow/ArkadeIframeHost.tsx
📚 Learning: 2025-11-20T12:08:19.542Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 230
File: test.docker-compose.yml:46-46
Timestamp: 2025-11-20T12:08:19.542Z
Learning: The Docker image `ghcr.io/arkade-os/arkd:v0.8.7` exists and is available in the GitHub Container Registry for the arkade-os/wallet repository.
Applied to files:
src/screens/Apps/Escrow/ArkadeIframeHost.tsx
📚 Learning: 2025-11-20T12:08:19.542Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 230
File: test.docker-compose.yml:46-46
Timestamp: 2025-11-20T12:08:19.542Z
Learning: The Docker image `boltz/boltz:ark` exists and is available for the arkade-os/wallet repository.
Applied to files:
src/screens/Apps.tsx
🧬 Code graph analysis (46)
src/components/InputFake.tsx (1)
src/components/Text.tsx (1)
TextSecondary(91-97)
src/test/lib/jsCapabilities.test.ts (1)
src/lib/jsCapabilities.ts (1)
getRestrictedEnvironmentMessage(118-123)
src/screens/Wallet/Unavailable.tsx (7)
src/providers/asp.tsx (1)
AspContext(15-20)src/lib/jsCapabilities.ts (2)
detectJSCapabilities(20-70)getRestrictedEnvironmentMessage(118-123)src/lib/browser.ts (1)
isIOS(3-6)src/components/CenterScreen.tsx (1)
CenterScreen(9-28)src/icons/WalletNew.tsx (1)
WalletNewIcon(1-3)src/components/Text.tsx (1)
Text(25-79)src/components/Error.tsx (1)
ErrorMessage(10-21)
src/screens/Apps/Boltz/Swap.tsx (2)
src/lib/bolt11.ts (1)
decodeInvoice(19-29)src/lib/format.ts (4)
prettyAmount(27-34)prettyHide(69-74)prettyAgo(12-25)prettyDate(57-67)
src/screens/Wallet/Receive/QrCode.tsx (2)
src/test/setup.mjs (1)
invoice(243-243)src/components/Info.tsx (1)
InfoLine(32-43)
src/screens/Settings/Support.tsx (5)
src/providers/wallet.tsx (1)
WalletContext(42-56)src/lib/types.ts (1)
Addresses(3-6)src/lib/asp.ts (1)
getReceivingAddresses(201-207)src/lib/chatwoot.ts (1)
hasChatwootVars(35-38)src/lib/explorers.ts (1)
getWebExplorerURL(38-40)
src/test/e2e/swap.test.ts (2)
src/test/setup.mjs (1)
invoice(243-243)src/test/e2e/utils.ts (1)
readClipboard(3-16)
src/lib/jsCapabilities.ts (1)
src/lib/browser.ts (1)
isIOS(3-6)
src/components/ChatWoot.tsx (1)
src/lib/chatwoot.ts (3)
getChatwootVars(28-33)getChatwootSettings(47-54)injectAndRunChatwootScript(60-89)
src/screens/Settings/Sign.tsx (7)
src/lib/logs.ts (1)
LogLine(8-12)src/components/FlexCol.tsx (1)
FlexCol(16-46)src/components/FlexRow.tsx (1)
FlexRow(18-50)src/lib/format.ts (2)
prettyAgo(12-25)prettyLongText(76-83)src/providers/wallet.tsx (1)
WalletContext(42-56)src/lib/privateKey.ts (1)
getPrivateKey(40-44)src/components/Input.tsx (1)
Input(13-40)
src/providers/fiat.tsx (2)
src/lib/fiat.ts (1)
FiatPrices(3-7)src/lib/format.ts (2)
toSatoshis(8-10)fromSatoshis(4-6)
src/components/Select.tsx (1)
src/components/FlexRow.tsx (1)
FlexRow(18-50)
src/test/screens/settings/backup.test.tsx (3)
src/providers/config.tsx (1)
ConfigContext(33-43)src/test/screens/mocks.ts (1)
mockConfigContextValue(37-46)src/screens/Settings/Backup.tsx (1)
Backup(31-187)
src/screens/Settings/Display.tsx (2)
src/providers/config.tsx (1)
ConfigContext(33-43)src/components/Select.tsx (1)
Select(12-49)
src/icons/Support.tsx (1)
scripts/git-commit-info.js (1)
path(2-2)
src/screens/Apps/Escrow/ArkadeIframeHost.tsx (1)
src/screens/Apps/Escrow/RpcHandler.ts (1)
MessageHandler(105-105)
src/test/screens/mocks.ts (2)
src/providers/asp.tsx (1)
AspInfo(6-6)src/lib/asp.ts (1)
emptyAspInfo(21-49)
src/components/Warning.tsx (3)
src/icons/Forbid.tsx (1)
ForbidIcon(1-29)src/icons/Info.tsx (1)
InfoIconDark(16-27)src/components/FlexRow.tsx (1)
FlexRow(18-50)
src/screens/Wallet/Send/Details.tsx (2)
src/providers/fees.tsx (1)
FeesContext(12-17)src/lib/asp.ts (2)
sendOffChain(231-233)collaborativeExitWithFees(87-130)
src/providers/escrow.tsx (5)
src/providers/asp.tsx (1)
AspContext(15-20)src/providers/wallet.tsx (1)
WalletContext(42-56)src/providers/config.tsx (1)
ConfigContext(33-43)src/lib/escrow.ts (4)
EscrowClient(10-70)isSignedIn(67-69)signin(19-60)signout(62-65)src/lib/privateKey.ts (1)
getPrivateKey(40-44)
src/providers/fees.tsx (2)
src/lib/types.ts (1)
Vtxo(98-98)src/providers/asp.tsx (1)
AspContext(15-20)
src/components/InputAmount.tsx (1)
src/lib/format.ts (1)
prettyNumber(85-88)
src/components/Toggle.tsx (3)
src/components/FlexCol.tsx (1)
FlexCol(16-46)src/components/FlexRow.tsx (1)
FlexRow(18-50)src/components/Text.tsx (1)
Text(25-79)
src/screens/Init/Restore.tsx (4)
src/lib/backup.ts (1)
BackupProvider(17-168)src/lib/logs.ts (1)
consoleError(40-44)src/components/Loading.tsx (1)
Loading(5-18)src/components/Input.tsx (1)
Input(13-40)
src/providers/lightning.tsx (1)
src/providers/config.tsx (1)
ConfigContext(33-43)
src/test/screens/settings/display.test.tsx (3)
src/providers/config.tsx (1)
ConfigContext(33-43)src/test/screens/mocks.ts (1)
mockConfigContextValue(37-46)src/screens/Settings/Display.tsx (1)
Display(9-32)
src/components/Header.tsx (2)
src/screens/Settings/Header.tsx (1)
Header(13-24)src/icons/Back.tsx (1)
BackIcon(1-13)
src/components/Keyboard.tsx (6)
src/providers/fiat.tsx (1)
FiatContext(16-20)src/providers/wallet.tsx (1)
WalletContext(42-56)src/lib/format.ts (2)
prettyNumber(85-88)prettyAmount(27-34)src/lib/constants.ts (1)
defaultFee(2-2)src/screens/Settings/Header.tsx (1)
Header(13-24)src/icons/Swap.tsx (1)
SwapIcon(1-10)
src/screens/Settings/Index.tsx (1)
src/screens/Settings/Support.tsx (1)
Support(21-136)
src/screens/Apps/Escrow/RpcHandler.test.ts (1)
src/screens/Apps/Escrow/RpcHandler.ts (1)
makeMessageHandler(116-213)
src/lib/lightning.ts (3)
src/lib/types.ts (1)
Config(8-23)src/lib/backup.ts (1)
BackupProvider(17-168)src/lib/logs.ts (1)
consoleError(40-44)
src/test/e2e/nostr.test.ts (2)
src/test/e2e/utils.ts (1)
readClipboard(3-16)src/test/setup.mjs (1)
invoice(243-243)
src/lib/chatwoot.ts (1)
src/lib/logs.ts (1)
consoleError(40-44)
src/screens/Apps.tsx (2)
src/providers/navigation.tsx (1)
NavigationContext(174-178)src/icons/FujiMoney.tsx (1)
FujiMoneyIcon(1-3)
src/providers/options.tsx (2)
src/icons/Logs.tsx (1)
LogsIcon(1-10)src/icons/Support.tsx (1)
SupportIcon(1-10)
src/screens/Settings/Server.tsx (1)
src/providers/config.tsx (1)
ConfigContext(33-43)
src/providers/navigation.tsx (1)
src/screens/Wallet/Unavailable.tsx (1)
Unavailable(10-36)
src/lib/backup.ts (3)
src/lib/types.ts (1)
Config(8-23)src/lib/nostr.ts (1)
NostrStorage(9-141)src/lib/logs.ts (1)
consoleError(40-44)
src/screens/Wallet/Receive/Amount.tsx (4)
src/providers/fiat.tsx (1)
FiatContext(16-20)src/lib/format.ts (2)
prettyNumber(85-88)prettyAmount(27-34)src/components/Keyboard.tsx (1)
Keyboard(24-174)src/components/Info.tsx (1)
InfoLine(32-43)
src/components/InputContainer.tsx (1)
src/components/FlexRow.tsx (1)
FlexRow(18-50)
src/screens/Apps/Index.tsx (3)
src/providers/flow.tsx (1)
FlowContext(89-104)src/App.tsx (1)
App(36-164)src/icons/Escrow.tsx (1)
EscrowIcon(1-3)
src/index.tsx (4)
src/providers/fees.tsx (1)
FeesProvider(19-66)src/providers/options.tsx (1)
OptionsProvider(138-173)src/providers/nudge.tsx (1)
NudgeProvider(18-50)src/App.tsx (1)
App(36-164)
src/components/Info.tsx (4)
src/components/FlexCol.tsx (1)
FlexCol(16-46)src/components/FlexRow.tsx (1)
FlexRow(18-50)src/icons/Info.tsx (1)
InfoIcon(1-14)src/components/Text.tsx (1)
Text(25-79)
src/lib/nostr.ts (2)
src/test/setup.mjs (1)
options(13-13)src/lib/logs.ts (1)
consoleError(40-44)
src/screens/Settings/Notifications.tsx (3)
src/providers/config.tsx (1)
ConfigContext(33-43)src/lib/notifications.ts (3)
notificationApiSupport(1-2)requestPermission(4-7)sendTestNotification(29-33)src/components/Toggle.tsx (1)
Toggle(14-28)
src/screens/Settings/Fiat.tsx (4)
src/providers/config.tsx (1)
ConfigContext(33-43)src/components/Content.tsx (1)
Content(9-16)src/components/Padded.tsx (1)
Padded(7-14)src/components/Select.tsx (1)
Select(12-49)
🪛 GitHub Actions: CI
src/screens/Settings/Sign.tsx
[error] 1-1: Prettier formatting issues detected in this file. Run 'prettier --write' to fix code style.
src/lib/escrow.ts
[error] 1-1: Prettier formatting issues detected in this file. Run 'prettier --write' to fix code style.
src/providers/escrow.tsx
[error] 1-1: Prettier formatting issues detected in this file. Run 'prettier --write' to fix code style.
src/screens/Apps.tsx
[error] 1-1: Prettier formatting issues detected in this file. Run 'prettier --write' to fix code style.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
| with: | ||
| use_liquid: false | ||
| use_ln: false | ||
| use_ln: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if e2e tests reference Lightning Network or Escrow functionality
# Search for LN-related test patterns in e2e tests
echo "=== Searching for Lightning/Escrow references in test files ==="
rg -nP --type=ts -C3 'lightning|escrow|use_ln' 'src/test/' 2>/dev/null || echo "No matches found in src/test/"
# Also search in broader test directories
echo -e "\n=== Searching in test directories ==="
find . -type d -name "*test*" -o -name "*e2e*" -o -name "*spec*" 2>/dev/null | head -20Repository: arkade-os/wallet
Length of output: 3050
🏁 Script executed:
#!/bin/bash
# Check the workflow file structure and context around line 24
echo "=== Checking playwright.yml around the change ==="
cat -n .github/workflows/playwright.yml | head -40Repository: arkade-os/wallet
Length of output: 1081
🏁 Script executed:
#!/bin/bash
# Search for any escrow or lightning test files
echo "=== Finding Escrow-related test files ==="
find . -type f \( -name "*.ts" -o -name "*.js" \) | xargs grep -l -i "escrow" 2>/dev/null | head -20Repository: arkade-os/wallet
Length of output: 141
🏁 Script executed:
#!/bin/bash
# Check e2e test files for escrow coverage
echo "=== E2E test files ==="
find ./src/test/e2e -type f -name "*.ts" | xargs ls -la
echo -e "\n=== Search for Escrow references in e2e tests ==="
grep -r -i "escrow" ./src/test/e2e/ 2>/dev/null || echo "No escrow references found in e2e tests"
echo -e "\n=== Check nostr.test.ts for full coverage ==="
wc -l ./src/test/e2e/nostr.test.tsRepository: arkade-os/wallet
Length of output: 955
🏁 Script executed:
#!/bin/bash
# Check what use_ln flag does and if there are other e2e tests
echo "=== All e2e test files content overview ==="
ls -la ./src/test/e2e/
echo -e "\n=== Check for escrow-related e2e test files ==="
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) -path "*e2e*" | xargs grep -l "escrow" 2>/dev/null || echo "No escrow e2e tests found"
echo -e "\n=== Check escrow.ts library implementation ==="
head -50 ./src/lib/escrow.tsRepository: arkade-os/wallet
Length of output: 2554
Add e2e tests for Escrow functionality with Lightning Network before enabling use_ln: true.
While Lightning Network tests already exist in the e2e suite (nostr.test.ts), there are no e2e tests covering Escrow functionality. The escrow.ts library is implemented but lacks e2e test coverage. Before relying on use_ln: true for Escrow integration, create dedicated e2e tests that exercise the Escrow client's authentication and Lightning Network operations.
| </div> | ||
| ) : ( | ||
| <p> </p> | ||
| '\u00A0' |
There was a problem hiding this comment.
Fix JSX syntax error.
String literals in JSX content positions must be wrapped in braces. The bare string '\u00A0' is invalid syntax and will cause a compilation error.
Apply this diff to fix the syntax:
- '\u00A0'
+ {'\u00A0'}📝 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.
| '\u00A0' | |
| {'\u00A0'} |
🤖 Prompt for AI Agents
In src/components/Header.tsx around line 43 the JSX contains a bare string
literal '\u00A0' which is invalid in JSX; wrap the string in expression braces
so it becomes {'\u00A0'} (or replace with if preferred) to fix the JSX
syntax error and ensure the non-breaking space renders correctly.
| @@ -0,0 +1,31 @@ | |||
| export default function Modal({ children }: { children: React.ReactNode }) { | |||
There was a problem hiding this comment.
Import React types for React.ReactNode
React.ReactNode is referenced in the props type but React is not imported, which will usually cause a TS compile error in module scope.
You can fix it by importing the type explicitly:
+import type React from 'react'
+
export default function Modal({ children }: { children: React.ReactNode }) {📝 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.
| export default function Modal({ children }: { children: React.ReactNode }) { | |
| import type React from 'react' | |
| export default function Modal({ children }: { children: React.ReactNode }) { |
🤖 Prompt for AI Agents
In src/components/Modal.tsx around line 1, the component references
React.ReactNode but React is not imported; add an import for the type (either
import React from 'react' or preferably import type { ReactNode } from 'react')
and update the props signature to use the imported type (e.g., { children }: {
children: ReactNode }) so TypeScript recognizes the type and the module
compiles.
| const outerStyle = { | ||
| top: '0', | ||
| left: '0', | ||
| zIndex: 21, | ||
| opacity: 0.95, | ||
| width: '100%', | ||
| height: '100%', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| position: 'absolute' as 'absolute', | ||
| backgroundColor: 'var(--ion-background-color)', | ||
| } |
There was a problem hiding this comment.
Use position: fixed and avoid opacity on the container
For a full-screen modal overlay, position: 'absolute' tied to an ancestor can fail to cover the viewport or scroll unexpectedly. Also, setting opacity on the outer container affects all descendants (the inner modal stays semi-transparent even with opacity: 1).
Consider:
- const outerStyle = {
- top: '0',
- left: '0',
- zIndex: 21,
- opacity: 0.95,
- width: '100%',
- height: '100%',
- display: 'flex',
- alignItems: 'center',
- justifyContent: 'center',
- position: 'absolute' as 'absolute',
- backgroundColor: 'var(--ion-background-color)',
- }
+ const outerStyle = {
+ top: 0,
+ left: 0,
+ zIndex: 21,
+ width: '100%',
+ height: '100%',
+ display: 'flex',
+ alignItems: 'center',
+ justifyContent: 'center',
+ position: 'fixed' as const,
+ // Semi-opaque backdrop without dimming modal content:
+ backgroundColor: 'rgba(0, 0, 0, 0.5)',
+ }📝 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.
| const outerStyle = { | |
| top: '0', | |
| left: '0', | |
| zIndex: 21, | |
| opacity: 0.95, | |
| width: '100%', | |
| height: '100%', | |
| display: 'flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| position: 'absolute' as 'absolute', | |
| backgroundColor: 'var(--ion-background-color)', | |
| } | |
| const outerStyle = { | |
| top: 0, | |
| left: 0, | |
| zIndex: 21, | |
| width: '100%', | |
| height: '100%', | |
| display: 'flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| position: 'fixed' as const, | |
| // Semi-opaque backdrop without dimming modal content: | |
| backgroundColor: 'rgba(0, 0, 0, 0.5)', | |
| } |
🤖 Prompt for AI Agents
In src/components/Modal.tsx around lines 2 to 14, the overlay uses position:
'absolute' and a global opacity which can fail to cover the viewport and makes
all children semi-transparent; change the container to position: 'fixed'
(top:0,left:0,width:100%,height:100%) so it always covers the viewport, remove
the opacity key from the outer container and instead apply a semi-transparent
backgroundColor (e.g. rgba or CSS variable with alpha) for the overlay, and if
needed set explicit opacity on inner modal content to keep it fully opaque.
| borderRadius: '0.5rem', | ||
| color: 'white', | ||
| padding: '1rem', | ||
| color: 'var(--orange)', |
There was a problem hiding this comment.
Verify hardcoded text color is intentional.
The text color is now always var(--orange) regardless of the warning state, creating a visual inconsistency:
- Red warnings have red background/border but orange text
- Green warnings have green background/border but orange text
Consider using borderColor for the text color to maintain consistency across all warning states.
Apply this diff if consistent coloring is desired:
- color: 'var(--orange)',
+ color: borderColor,📝 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.
| color: 'var(--orange)', | |
| color: borderColor, |
🤖 Prompt for AI Agents
In src/components/Warning.tsx around line 20, the text color is hardcoded to
'var(--orange)' which causes inconsistent text coloring across warning states;
change the style so the text color uses the same value as the component's border
color (or the mapped color for the current warning state) instead of the fixed
orange; implement this by replacing the hardcoded color with the computed
borderColor (or the state-to-color mapping) and provide a sensible fallback
(e.g., a default CSS variable) so red/green/other warnings render text that
matches their border/background.
| interface FeesContextProps { | ||
| calcOffchainInputFee: (vtxo: Vtxo) => number | ||
| calcOffchainOutputFee: (vtxo: Vtxo) => number | ||
| calcOnchainInputFee: (vtxo: Vtxo) => number | ||
| calcOnchainOutputFee: () => number | ||
| } |
There was a problem hiding this comment.
Fix interface to match implementation.
The interface declares that three functions accept a vtxo: Vtxo parameter, but the implementations (lines 26, 34, 42) don't use this parameter. Additionally, line 2 imports Vtxo type but it's unused if the parameter is removed.
Apply this diff to align the interface with the implementation:
interface FeesContextProps {
- calcOffchainInputFee: (vtxo: Vtxo) => number
- calcOffchainOutputFee: (vtxo: Vtxo) => number
- calcOnchainInputFee: (vtxo: Vtxo) => number
+ calcOffchainInputFee: () => number
+ calcOffchainOutputFee: () => number
+ calcOnchainInputFee: () => number
calcOnchainOutputFee: () => number
}And remove the unused import:
-import { Vtxo } from '../lib/types'📝 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.
| interface FeesContextProps { | |
| calcOffchainInputFee: (vtxo: Vtxo) => number | |
| calcOffchainOutputFee: (vtxo: Vtxo) => number | |
| calcOnchainInputFee: (vtxo: Vtxo) => number | |
| calcOnchainOutputFee: () => number | |
| } | |
| interface FeesContextProps { | |
| calcOffchainInputFee: () => number | |
| calcOffchainOutputFee: () => number | |
| calcOnchainInputFee: () => number | |
| calcOnchainOutputFee: () => number | |
| } |
🤖 Prompt for AI Agents
In src/providers/fees.tsx around lines 5 to 10 (implementations at ~26, 34, 42)
the FeesContextProps interface declares calcOffchainInputFee,
calcOffchainOutputFee and calcOnchainInputFee with a vtxo: Vtxo parameter even
though their implementations do not use that parameter; also Vtxo is imported at
the top but becomes unused. Update the interface so those three functions take
no arguments (match implementation) and adjust calcOnchainOutputFee signature if
necessary to remain consistent, then remove the unused Vtxo import from the top
of the file to eliminate the stale type import.
| if (!allowedChildOrigins.some((_) => _.startsWith(event.origin))) { | ||
| console.error(`[wallet]: ignoring message from ${event.origin}`) | ||
| return | ||
| } |
There was a problem hiding this comment.
Security: Origin check using startsWith is too permissive.
Using startsWith for origin validation can be bypassed. An attacker could register https://allowed-origin.attacker.com which would match https://allowed-origin. Use exact equality instead.
- if (!allowedChildOrigins.some((_) => _.startsWith(event.origin))) {
+ if (!allowedChildOrigins.includes(event.origin)) {📝 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.
| if (!allowedChildOrigins.some((_) => _.startsWith(event.origin))) { | |
| console.error(`[wallet]: ignoring message from ${event.origin}`) | |
| return | |
| } | |
| if (!allowedChildOrigins.includes(event.origin)) { | |
| console.error(`[wallet]: ignoring message from ${event.origin}`) | |
| return | |
| } |
🤖 Prompt for AI Agents
In src/screens/Apps/Escrow/ArkadeIframeHost.tsx around lines 24 to 27, the
origin check currently uses startsWith which is too permissive; update the check
to require exact equality against allowedChildOrigins (e.g., use includes or a
Set lookup) and also guard against missing/empty event.origin before checking,
so only an exact match will be accepted and messages from unknown origins are
rejected.
| const toggleNostrBackup = async () => { | ||
| const newConfig = { ...config, nostrBackup: !config.nostrBackup } | ||
| updateConfig(newConfig) | ||
| if (newConfig.nostrBackup) { | ||
| const backupProvider = new BackupProvider({ pubkey: config.pubkey }) | ||
| await backupProvider.fullBackup(newConfig).catch((error) => { | ||
| consoleError(error, 'Backup to Nostr failed') | ||
| setError('Backup to Nostr failed') | ||
| return | ||
| }) | ||
| } else { | ||
| backupConfig(newConfig) | ||
| } | ||
| present(backupToNostr) | ||
| } |
There was a problem hiding this comment.
Toast shown even when backup fails.
The present(backupToNostr) toast is called unconditionally at line 94, even when the backup fails at lines 86-90. The early return inside the .catch() callback doesn't prevent line 94 from executing.
Apply this diff to fix the control flow:
const toggleNostrBackup = async () => {
const newConfig = { ...config, nostrBackup: !config.nostrBackup }
updateConfig(newConfig)
if (newConfig.nostrBackup) {
const backupProvider = new BackupProvider({ pubkey: config.pubkey })
- await backupProvider.fullBackup(newConfig).catch((error) => {
+ try {
+ await backupProvider.fullBackup(newConfig)
+ } catch (error) {
consoleError(error, 'Backup to Nostr failed')
setError('Backup to Nostr failed')
return
- })
- } else {
- backupConfig(newConfig)
+ }
+ } else {
+ backupConfig(newConfig)
}
present(backupToNostr)
}📝 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.
| const toggleNostrBackup = async () => { | |
| const newConfig = { ...config, nostrBackup: !config.nostrBackup } | |
| updateConfig(newConfig) | |
| if (newConfig.nostrBackup) { | |
| const backupProvider = new BackupProvider({ pubkey: config.pubkey }) | |
| await backupProvider.fullBackup(newConfig).catch((error) => { | |
| consoleError(error, 'Backup to Nostr failed') | |
| setError('Backup to Nostr failed') | |
| return | |
| }) | |
| } else { | |
| backupConfig(newConfig) | |
| } | |
| present(backupToNostr) | |
| } | |
| const toggleNostrBackup = async () => { | |
| const newConfig = { ...config, nostrBackup: !config.nostrBackup } | |
| updateConfig(newConfig) | |
| if (newConfig.nostrBackup) { | |
| const backupProvider = new BackupProvider({ pubkey: config.pubkey }) | |
| try { | |
| await backupProvider.fullBackup(newConfig) | |
| } catch (error) { | |
| consoleError(error, 'Backup to Nostr failed') | |
| setError('Backup to Nostr failed') | |
| return | |
| } | |
| } else { | |
| backupConfig(newConfig) | |
| } | |
| present(backupToNostr) | |
| } |
🤖 Prompt for AI Agents
In src/screens/Settings/Backup.tsx around lines 81 to 95, the toast
present(backupToNostr) is invoked unconditionally even when the async fullBackup
fails because the .catch callback's return doesn't stop outer flow; change the
control flow to await the backup inside a try/catch (or capture success boolean)
and only call present(backupToNostr) when the backup completed successfully —
i.e., updateConfig(newConfig) first, then if newConfig.nostrBackup perform: try
{ await backupProvider.fullBackup(newConfig); present(backupToNostr); } catch
(error) { consoleError(error, 'Backup to Nostr failed'); setError('Backup to
Nostr failed'); } else call backupConfig(newConfig).
| const signChallenge = async () => { | ||
| if (!hashToSign) return | ||
| const password = prompt('Password') | ||
| if (!password) return | ||
| const privkey = await getPrivateKey(password) | ||
| const signatureBytes = schnorr.sign( | ||
| hexToBytes(hashToSign), | ||
| privkey, | ||
| ); | ||
| setSignatureHex(bytesToHex(signatureBytes)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for signing operations.
The signChallenge function should handle errors from getPrivateKey() and schnorr.sign() to provide user feedback when signing fails.
Apply this diff to add error handling:
const signChallenge = async () => {
if (!hashToSign) return
const password = prompt('Password')
if (!password) return
- const privkey = await getPrivateKey(password)
- const signatureBytes = schnorr.sign(
- hexToBytes(hashToSign),
- privkey,
- );
- setSignatureHex(bytesToHex(signatureBytes));
+ try {
+ const privkey = await getPrivateKey(password)
+ const signatureBytes = schnorr.sign(
+ hexToBytes(hashToSign),
+ privkey,
+ )
+ setSignatureHex(bytesToHex(signatureBytes))
+ } catch (error) {
+ console.error('Failed to sign:', error)
+ alert('Failed to sign. Please check your password and try again.')
+ }
}📝 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.
| const signChallenge = async () => { | |
| if (!hashToSign) return | |
| const password = prompt('Password') | |
| if (!password) return | |
| const privkey = await getPrivateKey(password) | |
| const signatureBytes = schnorr.sign( | |
| hexToBytes(hashToSign), | |
| privkey, | |
| ); | |
| setSignatureHex(bytesToHex(signatureBytes)); | |
| } | |
| const signChallenge = async () => { | |
| if (!hashToSign) return | |
| const password = prompt('Password') | |
| if (!password) return | |
| try { | |
| const privkey = await getPrivateKey(password) | |
| const signatureBytes = schnorr.sign( | |
| hexToBytes(hashToSign), | |
| privkey, | |
| ) | |
| setSignatureHex(bytesToHex(signatureBytes)) | |
| } catch (error) { | |
| console.error('Failed to sign:', error) | |
| alert('Failed to sign. Please check your password and try again.') | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/screens/Settings/Sign.tsx around lines 76-86, the signChallenge function
lacks error handling for getPrivateKey() and schnorr.sign(), so wrap the
password retrieval, getPrivateKey call and schnorr.sign call in a try/catch
block; on success setSignatureHex as before, on failure catch the error, log it
(console.error or a logger) and surface a user-friendly message (e.g., alert or
set an error state) so the UI reflects the failure; also ensure sensitive values
(password, privkey) are not retained on error by clearing any local variables or
state before exiting.
| test('should show server unreachable', async ({ page }) => { | ||
| await page.goto('/') | ||
| await page.context().setOffline(true) | ||
| await expect(page.getByText('Arkade server unreachable')).toBeVisible() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -nP -C3 '\b(unreachable|offline|navigator\.onLine)\b' --type=ts --type=tsx -g '!**/node_modules/**' -g '!**/dist/**'Repository: arkade-os/wallet
Length of output: 87
🏁 Script executed:
rg -nP -C3 'unreachable|setOffline|navigator\.onLine|offline' --type=ts -g '!node_modules' -g '!dist'Repository: arkade-os/wallet
Length of output: 12115
🏁 Script executed:
cat -n src/test/e2e/serverdown.test.tsRepository: arkade-os/wallet
Length of output: 358
🏁 Script executed:
cat -n src/lib/asp.ts | head -150Repository: arkade-os/wallet
Length of output: 5393
🏁 Script executed:
cat -n src/providers/asp.tsxRepository: arkade-os/wallet
Length of output: 3584
🏁 Script executed:
cat -n src/App.tsx | head -130Repository: arkade-os/wallet
Length of output: 5826
🏁 Script executed:
rg -n 'setOffline|beforeEach|beforeAll|test\.describe' src/test/e2e/serverdown.test.ts -B5 -A5Repository: arkade-os/wallet
Length of output: 323
🏁 Script executed:
cat -n src/screens/Wallet/Unavailable.tsx | head -40Repository: arkade-os/wallet
Length of output: 1538
🏁 Script executed:
rg -n "Arkade server unreachable" src/ -C2Repository: arkade-os/wallet
Length of output: 720
Fix text mismatch and timing race condition.
The test expects 'Arkade server unreachable' but the Unavailable screen displays 'Arkade server unreachable.' (with a period). Update the test to match the actual error message text. Additionally, the test has a timing race condition: setOffline(true) is called after page.goto('/') completes, but the HTTP request for server info may already be inflight. Consider setting the offline context before navigation to ensure the request fails reliably, or add an explicit wait for the error state before the assertion.
🤖 Prompt for AI Agents
In src/test/e2e/serverdown.test.ts around lines 3 to 6, the assertion text and
timing are fragile: update the expected string to include the trailing period
("Arkade server unreachable.") and ensure the page is offline before the page
load to avoid the inflight request race; specifically call
page.context().setOffline(true) before page.goto('/') (or alternatively add an
explicit wait for the Unavailable state before asserting) so the server-info
request fails reliably and the assertion matches the actual text.
|
Will be replaced by https://github.com/arkade-os/wallet/tree/integrate-escrow-app |
Summary by CodeRabbit
New Features
UI
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.