feat: replace @buildonspark/spark-sdk with @breeztech/breez-sdk-spark#986
feat: replace @buildonspark/spark-sdk with @breeztech/breez-sdk-spark#986ditto-agent wants to merge 27 commits intomasterfrom
Conversation
Phase C validation complete — all tests passed. These docs provide context for the Phase A production replacement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rk path Instead of keeping @buildonspark/spark-sdk for Lightning Address, throw not-implemented error and leave TODO to fork Breez SDK to expose receiverIdentityPubkey. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Changes: - WASM init in _protected.tsx clientLoader (not entry.client.tsx) - Static imports (Breez package exports handle SSR/client) - No app/lib/breez-spark/ folder — everything in app/lib/spark/ - No getBreezSdk accessor or type re-exports - Event-driven balance, send status, and receive status (no polling) - Cached prepareSendPayment response (no double-prepare) - Single balance value (ownedBalance = availableBalance = balanceSats) - Simplified updateSparkAccountBalance cache method - Remove all dead code (workarounds, polling, SparkProto, etc.) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- connectBreezWallet takes { mnemonic, network, apiKey } object param
- Remove createEventListener wrapper (inline addEventListener is simple enough)
- updateSparkAccountBalance takes single balance param (no owned/available split)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
No init.ts file needed. shared/spark.ts calls connect() + defaultConfig() from @breeztech/breez-sdk-spark inline. Tasks renumbered (7 total). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…per account - getSparkIdentityPublicKeyFromMnemonic uses static import (not dynamic) - SparkAccount.balance replaces ownedBalance/availableBalance - updateSparkAccountBalance works with single balance - Event listeners: one per account, matches against all pending quotes - Clarify addEventListener does NOT replay past events (initial check needed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace @buildonspark/spark-sdk polling with Breez SDK event listeners for receive status tracking. Simplify SparkReceiveLightningQuote type, use receivePayment() + parseBolt11Invoice instead of createLightningInvoice, and match completions by paymentHash from paymentSucceeded events. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…k-sdk Breez SDK does not expose receiverIdentityPubkey for delegated invoices, so the Spark Lightning Address path now throws with a clear TODO. The old @buildonspark/spark-sdk dependency, its patch file, and all its imports are fully removed. ReadUserDefaultAccountRepository returns a stub wallet for the server-side spark branch (never called in practice now). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Narrow PENDING quotes type for sparkId access in send hooks - Add Money type cast for moneyFromSats in send service - Fix empty catch block lint errors in spark.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| return 'mainnet'; | ||
| case 'REGTEST': | ||
| case 'LOCAL': | ||
| return 'regtest'; |
There was a problem hiding this comment.
did you play with this? Also, are these the only network options? the buildonspark sdk had signet last I looked
There was a problem hiding this comment.
i didn't. breez network type only has mainnet and regtest while spark sdk has this:
declare enum Network {
MAINNET = 0,
TESTNET = 1,
SIGNET = 2,
REGTEST = 3,
LOCAL = 4
}
There was a problem hiding this comment.
yeah it only has mainnet and regtest in breez sdk
| await this.queryClient.invalidateQueries({ | ||
| queryKey: sparkBalanceQueryKey(quotes.destinationAccount.id), | ||
| }); | ||
| // Balance is updated event-driven via useTrackAndUpdateSparkAccountBalances |
There was a problem hiding this comment.
do we need this comment? Assuming this is claude because it always adds comments like this in place of code that is removed
There was a problem hiding this comment.
yeah it's claude. I will cleanup all the code now
| () => wallet.getLightningReceiveRequest(quote.sparkId), | ||
| { receiveRequestId: quote.sparkId }, | ||
| ); | ||
| const handlePayment = (payment: { |
There was a problem hiding this comment.
does the Breez SDK have types we can use instead of writing our own inline?
| if (settled) return; | ||
| const details = payment.details; | ||
| if (details?.type !== 'lightning') return; | ||
| if (details.htlcDetails?.paymentHash !== quote.paymentHash) return; |
There was a problem hiding this comment.
can the type or paymentHash ever not be set or is this just to make TS show these as defined?
| }) | ||
| .then((id) => { | ||
| listenerId = id; | ||
| // If already settled before listener registered, clean up immediately |
There was a problem hiding this comment.
how does this happen? If the timeout fires and sets settled to true? or can it happen too from the initial check below?
| ): error is SparkError => { | ||
| /** | ||
| * Checks if an error is an insufficient balance error from the Breez SDK. | ||
| * Phase C validation confirmed: message contains "insufficient funds". |
There was a problem hiding this comment.
i think this line should be removed
There was a problem hiding this comment.
it should. as a matter of fact I'd remove the entire tsdoc here. doesn't really add a lot of value imo
|
|
||
| /** | ||
| * Checks if an error indicates the invoice was already paid. | ||
| * Phase C validation confirmed: message contains "preimage request already exists". |
There was a problem hiding this comment.
yeah entire tsdoc here should be removed
app/features/shared/spark.ts
Outdated
| connect({ | ||
| config, | ||
| seed: { type: 'mnemonic', mnemonic }, | ||
| storageDir: `spark-${breezNetwork}`, |
There was a problem hiding this comment.
where is the storage? And do you know what's in it?
| sparkBalanceQueryKey(updatedQuote.accountId), | ||
| ), | ||
| }); | ||
| // Balance is updated event-driven via useTrackAndUpdateSparkAccountBalances |
| typename: response.typename, | ||
| paymentPreimage: response.paymentPreimage ?? undefined, | ||
| receiverIdentityPublicKey: response.receiverIdentityPublicKey ?? undefined, | ||
| fee: moneyFromSats(0), |
There was a problem hiding this comment.
does this fee ever get set to something else? If not, then why is fee added to this type. I don't think receiver pays fees
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The package's web entry exports only the WASM init function as default, so `import pkg from` + destructure loses the named exports in the browser. The patch adds all named exports to the default via Object.assign, making `import pkg from` work in both Node (CJS) and browser (ESM) environments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add nodejs/index.mjs that re-exports the CJS entry as named ESM
bindings, and update the package exports map to use it for ESM
imports. This lets `import { connect } from '@breeztech/breez-sdk-spark'`
work in Vite SSR without dynamic imports or Object.assign hacks.
Reverts app code to clean static named imports.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
clientMiddleware runs before clientLoader, so WASM must be initialized in the middleware where defaultExternalSigner is first called. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rData Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace @breeztech/breez-sdk-spark with @agicash/breez-sdk-spark@0.12.2-1 (fork with ESM Node entry and receiverIdentityPubkey support) - Remove the CJS-to-ESM bun patch (fork ships native ESM) - Restore Spark Lightning Address flow: server wallet via LNURL_SERVER_SPARK_MNEMONIC creates delegated invoices with receiverIdentityPubkey, matching the master branch behavior - Restore handleSparkLnurlpVerify using BreezSdk.getPayment() - ReadUserDefaultAccountRepository accepts optional mnemonic to create real Spark wallets on the server Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| * Hex-encoded identity public key of the receiver. | ||
| * Used for delegated invoices (Lightning Address). | ||
| */ | ||
| receiverIdentityPubkey?: string; |
There was a problem hiding this comment.
why was this added? it was removed in RepositoryCreateQuoteParams and added here. I'd revert that change
| paymentHash, | ||
| expiresAt, | ||
| sparkId, | ||
| receiverIdentityPubkey, |
| * If provided, the incoming payment can only be claimed by the Spark wallet that controls the specified public key. | ||
| * If not provided, the invoice will be created for the user that owns the Spark wallet. | ||
| */ | ||
| receiverIdentityPubkey?: string; |
There was a problem hiding this comment.
this was removed form here and added below. no need to do such unnecessary changes. also not sure there was a need to chagne the comment
identityPublicKey() returns { bytes: number[] }, not ArrayBuffer.
The incorrect cast produced garbage hex, causing "malformed public key"
errors for delegated invoices and potentially Spark offline on Vercel.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
We use our own lightning address system, not Breez's. Setting lnurlDomain to undefined prevents the 404 recovery call to breez.tips on every SDK connect. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| ## Integration Notes | ||
|
|
||
| - Import from `@breeztech/breez-sdk-spark` (root path, not `/bundler`) — root has conditional exports: Node.js gets CJS entry, browser gets ESM/WASM entry | ||
| - WASM init must happen in `entry.client.tsx` (client-only) via dynamic import |
There was a problem hiding this comment.
i changed this to happen in protected route middleware so we don't init it for users that are not logged in. it is only needed for logged in users
|
|
||
| - Import from `@breeztech/breez-sdk-spark` (root path, not `/bundler`) — root has conditional exports: Node.js gets CJS entry, browser gets ESM/WASM entry | ||
| - WASM init must happen in `entry.client.tsx` (client-only) via dynamic import | ||
| - All SDK usage in app code must use dynamic `import()` to avoid SSR module graph issues |
There was a problem hiding this comment.
I solved this in the fork of the lib by adding an option to use esm instead of commonjs for node
|
|
||
| **Result: SAME** — Fees are identical between Breez SDK and current Spark SDK. Both use the same Spark protocol. | ||
|
|
||
| ## C6: Init Performance |
There was a problem hiding this comment.
This is kind of comparing apples to oranges due to have sdks are bundling and loading wasm. They have different bundle sizes and are triggering things at different times so to make the comparison that is relevant for us we'd need to consider the speed to show the balance on the home screen in different scenarios:
a) no cache hit time to display the balance on the home page
b) cache hit time to display the balance on the home page
also possibly try both on slow network, etc.
| | Insufficient balance | `Error` | `insufficient funds` | | ||
| | Already paid invoice | `Error` | `preimage request already exists` | | ||
|
|
||
| **Key difference from current Spark SDK:** Spark SDK's `SparkError` has `getContext()` returning structured data (e.g., `{ expected, value, field }` for insufficient balance). Breez SDK errors are plain `Error` with flat message strings — no typed subclasses, no structured context. |
There was a problem hiding this comment.
One corollary of this is that we can't have those specific error messages for cases where owned balance hasn't been decreased yet and another send is attempted. This is probably fine because Breez sdk seems much better at handling that. Owned and available balances are abstracted away and balance does seem to decrease relatively fast after initiating the send even while it is still pending.
| @@ -1,2 +1,6 @@ | |||
| export * from './errors'; | |||
There was a problem hiding this comment.
we need to revert this change. this was absolutely unnecessary
| }); | ||
| } catch (error) { | ||
| if (isInsufficentBalanceError(error)) { | ||
| throw new DomainError(`Insufficient balance. ${error.message}`); |
There was a problem hiding this comment.
before this change the message was Insufficient balance. Total cost of send is ${value} sats but the available balance is ${availableSats} sats.
we should see if there is a way to keep it the same (if we can get the total send cost and available balance values)
| private readonly db: AgicashDb, | ||
| private readonly queryClient: QueryClient, | ||
| private readonly getSparkWalletMnemonic: () => Promise<string>, | ||
| private readonly getSparkWalletMnemonic?: () => Promise<string>, |
There was a problem hiding this comment.
why was this made optional? it should be reverted
|
|
||
| private async getInitializedSparkWallet(network: NetworkType) { | ||
| private async getInitializedSparkWalletForNetwork(network: SparkNetwork) { | ||
| if (!this.getSparkWalletMnemonic) { |
There was a problem hiding this comment.
when getSparkWalletMnemonic is made mandatory again this whole change can also be reverted
| } | ||
|
|
||
| private async getInitializedSparkWallet(network: NetworkType) { | ||
| private async getInitializedSparkWalletForNetwork(network: SparkNetwork) { |
There was a problem hiding this comment.
this name change was unnecessary and should be reverted
| * Network of the Spark account. | ||
| * Based on the NetworkType enum from the spark-sdk. | ||
| */ | ||
| network: z.enum(['MAINNET', 'TESTNET', 'SIGNET', 'REGTEST', 'LOCAL']), |
There was a problem hiding this comment.
we should change this to only have values 'MAINNET' and 'REGTEST' and change the comment to say it is based on breez sdk type. then when storing the value in the db we need to convert it to uppercase (because breez type is lowercase) and when we need to convert db type to breez type we need to convert to uppercase
we can change the type like that because in all our dbs all storated accoutns atm are 'MAINNET'
|
|
||
| export type SparkNetwork = SparkAccountDetailsDbData['network']; | ||
|
|
||
| export function toBreezNetwork(network: SparkNetwork): 'mainnet' | 'regtest' { |
There was a problem hiding this comment.
when we do this here we can just call toLower.
also this fn doesn't even belong here. we can just call tolower where needed
| optimizationOptions: { | ||
| auto: true, | ||
| const breezNetwork = toBreezNetwork(network); | ||
| const apiKey = import.meta.env.VITE_BREEZ_API_KEY; |
There was a problem hiding this comment.
lets load this env var into variable on the top of the module and throw if not set (same pattern like in database.server.ts file for example)
| const apiKey = import.meta.env.VITE_BREEZ_API_KEY; | ||
|
|
||
| // initLogging is idempotent — safe to call multiple times | ||
| try { |
There was a problem hiding this comment.
lets extract this logger init into small helper function and also log a warning to the console in catch clause
| } | ||
|
|
||
| export function useTrackAndUpdateSparkAccountBalances() { | ||
| const { data: sparkAccounts } = useAccounts({ type: 'spark' }); |
There was a problem hiding this comment.
just filter online accounts here so we don't need to do if (!account.isOnline) continue; below
| if ( | ||
| event.type === 'paymentSucceeded' || | ||
| event.type === 'paymentPending' || | ||
| event.type === 'synced' |
There was a problem hiding this comment.
this should also include paymentFailed and claimedDeposits I think. those are also updating the balance
|
|
||
| const sdk = account.wallet; | ||
|
|
||
| sdk |
There was a problem hiding this comment.
I think we should do something like this instead:
useEffect(() => {
const registrations = sparkOnlineAccounts
.map((account) => {
const sdk = account.wallet;
const listenerPromise = sdk.addEventListener({
onEvent(event: SdkEvent) {
sparkDebugLog('Breez event', {
accountId: account.id,
type: event.type,
});
if (
event.type === 'paymentSucceeded' ||
...
) {
sdk.getInfo({}).then((info) => {
...
accountCache.updateSparkAccountBalance({
accountId: account.id,
balance,
});
});
}
},
});
return { sdk, listenerPromise };
});
return () => {
for (const { sdk, listenerPromise } of registrations) {
listenerPromise.then((id) => sdk.removeEventListener(id)).catch(() => {});
}
};
}, [sparkAccounts, accountCache]);
| /** | ||
| * Optional public key of the wallet receiving the lightning invoice. | ||
| */ | ||
| receiverIdentityPubkey?: string; |
There was a problem hiding this comment.
this should not be removed
Summary
@buildonspark/spark-sdkwith@breeztech/breez-sdk-spark@0.12.2across all Spark operations (wallet init, send, receive, balance tracking)sdk.addEventListener— removes all polling (useQueries/refetchInterval/setInterval) and the zero-balance workaroundownedBalance/availableBalanceto singlebalance: Money | nullprepareSendPayment+sendPaymentwithidempotencyKey(replacespayLightningInvoice+ manual dedup viafindExistingLightningSendRequest)receivePayment+ bolt11 parsing, event-driven completion detectionreceiverIdentityPubkeyfor delegated invoices yet (cashu path unaffected)@buildonspark/spark-sdkand its patch fileTest plan
_protectedclientLoader[Spark]/[Breez]logs)waitForSparkReceiveToComplete)/lnurl-test)bun run fix:allpasses with no errors@buildonspark/spark-sdkimports remain (rg "@buildonspark/spark-sdk" app/)🤖 Generated with Claude Code