Repository navigation
Spark Integration - #88
levoncrypto wants to merge 13 commits into
Conversation
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
codeant-ai
Bot
added
the
size:XXL
CodeAnt-AI DescriptionAdd Spark private payments and wallet synchronization What Changed
Impact
💡 Usage GuideChecking Your Pull RequestEvery time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later. Talking to CodeAnt AIGot a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask: This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code. ExamplePreserve Org Learnings with CodeAntYou can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input: This helps CodeAnt AI learn and adapt to your team's coding style and standards. ExampleRetrigger reviewAsk CodeAnt AI to review the PR again, by typing: Check Your Repository HealthTo analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health. |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| serialized = coin.get('serialized_coin') or coin.get('serializedCoin') | ||
| context = (coin.get('serialized_coin_context') | ||
| or coin.get('context') | ||
| or coin.get('serial_context')) | ||
| if isinstance(serialized, str): | ||
| serialized = base64.b64decode(''.join(serialized.splitlines())) | ||
| if isinstance(context, str): | ||
| context = base64.b64decode(''.join(context.splitlines())) | ||
| coin_stream = _make_data_stream(serialized, keep) | ||
| ctx_stream = _make_data_stream(context, keep) | ||
| keep.extend((coin_stream, ctx_stream)) | ||
| arr[i].serializedCoin = ctypes.pointer(coin_stream) | ||
| arr[i].serializedCoinContext = ctypes.pointer(ctx_stream) |
There was a problem hiding this comment.
Suggestion: Malformed or incomplete coin data reaches from_buffer_copy, causing fee estimation or spending to fail with an unhandled low-level TypeError. [type error]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/libsparkmobile.py
**Line:** 411:423
**Comment:**
*Type Error: Malformed or incomplete coin data reaches `from_buffer_copy`, causing fee estimation or spending to fail with an unhandled low-level `TypeError`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| scripts = [] | ||
| for i in range(res.contents.outputScriptsLength): | ||
| s = res.contents.outputScripts[i] | ||
| scripts.append(ctypes.string_at(s.bytes, s.length)) | ||
| lib.native_free(s.bytes) | ||
| used = [] | ||
| for i in range(res.contents.usedCoinsLength): | ||
| u = res.contents.usedCoins[i] | ||
| used.append({ | ||
| 'serialized_coin': ctypes.string_at( | ||
| u.serializedCoin.contents.data, | ||
| u.serializedCoin.contents.length), | ||
| 'serialized_coin_context': ctypes.string_at( | ||
| u.serializedCoinContext.contents.data, | ||
| u.serializedCoinContext.contents.length), | ||
| 'group_id': int(u.groupId), | ||
| 'height': int(u.height), | ||
| }) | ||
| lib.native_free(u.serializedCoin.contents.data) | ||
| lib.native_free(u.serializedCoin) | ||
| lib.native_free(u.serializedCoinContext.contents.data) | ||
| lib.native_free(u.serializedCoinContext) | ||
| if res.contents.outputScripts: | ||
| lib.native_free(res.contents.outputScripts) | ||
| if res.contents.usedCoins: | ||
| lib.native_free(res.contents.usedCoins) |
There was a problem hiding this comment.
Suggestion: If native result processing raises, outputScripts, usedCoins, and already-owned child buffers are leaked because the finally block frees only data and res. [resource leak]
Assessment: 🟠 Major · 🔁 Occurrence: Rarely
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/libsparkmobile.py
**Line:** 559:584
**Comment:**
*Resource Leak: If native result processing raises, `outputScripts`, `usedCoins`, and already-owned child buffers are leaked because the `finally` block frees only `data` and `res`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| LIBSPARKMOBILE_DEFAULT_REPO_URL="https://github.com/firoorg/electrum_libsparkmobile.git" | ||
| LIBSPARKMOBILE_DEFAULT_REPO_REF="initial-import" | ||
| LIBSPARKMOBILE_DEFAULT_REPO_COMMIT="" | ||
|
|
||
| LIBSPARKMOBILE_REPO_URL="${LIBSPARKMOBILE_REPO_URL:-$LIBSPARKMOBILE_DEFAULT_REPO_URL}" | ||
| LIBSPARKMOBILE_REPO_REF="${LIBSPARKMOBILE_REPO_REF:-$LIBSPARKMOBILE_DEFAULT_REPO_REF}" | ||
| LIBSPARKMOBILE_REPO_COMMIT="${LIBSPARKMOBILE_REPO_COMMIT:-$LIBSPARKMOBILE_DEFAULT_REPO_COMMIT}" | ||
|
|
||
| fetch_target="${LIBSPARKMOBILE_REPO_COMMIT:-$LIBSPARKMOBILE_REPO_REF}" |
There was a problem hiding this comment.
Suggestion: The default commit is empty, so builds reset to the moving initial-import branch tip and silently install changed native wallet code. [security]
Assessment: 🔴 Critical · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** contrib/make_libsparkmobile.sh
**Line:** 15:23
**Comment:**
*Security: The default commit is empty, so builds reset to the moving `initial-import` branch tip and silently install changed native wallet code.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| self.broadcast_or_show(tx, pr) | ||
| for extra in sparkMints: | ||
| self.broadcast_or_show(extra, None) |
There was a problem hiding this comment.
Suggestion: Mint transactions are broadcast independently without checking earlier results, so a failed primary or extra broadcast can leave a partial mint sequence and no recovery handling. [incomplete implementation]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/gui/qt/main_window.py
**Line:** 2386:2388
**Comment:**
*Incomplete Implementation: Mint transactions are broadcast independently without checking earlier results, so a failed primary or extra broadcast can leave a partial mint sequence and no recovery handling.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| txid = cls._decode_b64(txid_b64)[::-1].hex() | ||
| _logger.error( | ||
| f'Error identifying spark coin in tx {txid} ' | ||
| f'(this is not expected): {e!r}') | ||
| continue |
There was a problem hiding this comment.
Suggestion: If a malformed server row causes recovery to fail and its transaction identifier is also malformed, decoding that identifier escapes the handler and aborts the entire refresh. [possible bug]
Assessment: 🟠 Major · 🔁 Occurrence: Rarely
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/spark_interface.py
**Line:** 487:491
**Comment:**
*Possible Bug: If a malformed server row causes recovery to fail and its transaction identifier is also malformed, decoding that identifier escapes the handler and aborts the entire refresh.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| numberOfCoinsToFetch = size - prevSize | ||
| if numberOfCoinsToFetch > 0 and not self.syncing: | ||
| self.syncing = True | ||
| util.trigger_callback('wallet_updated', self.wallet) | ||
|
|
||
| rows = [] | ||
| if numberOfCoinsToFetch > 0: | ||
| rows = await self._fetch_sectors( | ||
| groupId, blockHash, numberOfCoinsToFetch) |
There was a problem hiding this comment.
Suggestion: When a set grows, this fetch starts at index zero instead of the previous size, so it stores old coins and misses the newly appended anonymity-set coins. [logic error]
Assessment: 🟠 Major · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/spark_interface.py
**Line:** 636:644
**Comment:**
*Logic Error: When a set grows, this fetch starts at index zero instead of the previous size, so it stores old coins and misses the newly appended anonymity-set coins.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if local_height < state.get('chain_height', 0): | ||
| state.pop('firo_spark_cache_set_block_hash_cache', None) | ||
| for c in coins.values(): | ||
| c['height'] = None | ||
| c['is_used'] = False | ||
| c.pop('spent_txid', None) |
There was a problem hiding this comment.
Suggestion: After a chain rollback, only the wallet cursor is reset; the shared anonymity-set cache remains, so old-chain sets can prevent rescanning the replacement chain. [stale reference]
Assessment: 🟠 Major · 🔁 Occurrence: Rarely
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/spark_interface.py
**Line:** 801:806
**Comment:**
*Stale Reference: After a chain rollback, only the wallet cursor is reset; the shared anonymity-set cache remains, so old-chain sets can prevent rescanning the replacement chain.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| self._merge_used_flags(coins) | ||
| shared.update(anon_sets=anon_sets, used_tags=used_tags, | ||
| groups=groups) |
There was a problem hiding this comment.
Suggestion: Rollback resets copied coins as unused, but this later restores every previously used flag from the old database, leaving reverted coins unavailable and retaining old spend records. [stale reference]
Assessment: 🟠 Major · 🔁 Occurrence: Rarely
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/spark_interface.py
**Line:** 921:923
**Comment:**
*Stale Reference: Rollback resets copied coins as unused, but this later restores every previously used flag from the old database, leaving reverted coins unavailable and retaining old spend records.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| transparentSumOut = sum(int(a) for _, a in transparentRecipients) | ||
| sparkSumOut = sum(int(a) for _, a, _ in privateSparkRecipients) | ||
| txAmount = transparentSumOut + sparkSumOut | ||
|
|
||
| if transparentSumOut > 50_000 * COIN: | ||
| raise ValueError(_('Spend to transparent address limit exceeded ' | ||
| '(50,000 Firo per transaction).')) | ||
|
|
||
| available = sum(int(c['value']) for c in coins) | ||
| if txAmount > available: | ||
| raise NotEnoughFunds(_('Insufficient Spark balance')) | ||
|
|
||
| send_all_flag = (amount == '!' or any(a == '!' for _, a in transparentRecipients) | ||
| or any(a == '!' for _, a, _ in privateSparkRecipients)) |
There was a problem hiding this comment.
Suggestion: Direct callers passing '!' in recipients or sparkRecipients reach int(a) before send-all handling, causing a ValueError instead of creating a send-all transaction. [type error]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/spark_interface.py
**Line:** 1512:1525
**Comment:**
*Type Error: Direct callers passing `'!'` in `recipients` or `sparkRecipients` reach `int(a)` before send-all handling, causing a `ValueError` instead of creating a send-all transaction.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| tx_type_name = SPEC_TX_NAMES[PSTxTypes.SPARK_MINT] | ||
| height = coin.get('height') or 0 | ||
| timestamp = coin.get('timestamp') | ||
| transactions_tmp[txid] = { | ||
| 'txid': txid, 'fee_sat': None, 'height': height, | ||
| 'confirmations': max(self.get_local_height() - height + 1, 0), |
There was a problem hiding this comment.
Suggestion: Unconfirmed coins use height 0, but this formula assigns them local_height + 1 confirmations, making pending Spark mints appear confirmed. [logic error]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/wallet.py
**Line:** 1008:1013
**Comment:**
*Logic Error: Unconfirmed coins use height `0`, but this formula assigns them `local_height + 1` confirmations, making pending Spark mints appear confirmed.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| def disable_spark(self) -> None: | ||
| self.spark_enabled = False | ||
| self.spark_password = None | ||
| self.spark_key_data = None | ||
| self.spark_view_key_hex = None |
There was a problem hiding this comment.
Suggestion: Clearing spark_key_data does not stop an active synchronizer; an in-progress refresh later reads None and can fail native recovery or leave partial state. [state/lifecycle]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/wallet.py
**Line:** 1495:1499
**Comment:**
*State Lifecycle: Clearing `spark_key_data` does not stop an active synchronizer; an in-progress refresh later reads `None` and can fail native recovery or leave partial state.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| try: | ||
| self.add_transaction(tx, allow_unrelated=True) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Suggestion: If adding the broadcast transaction fails, the exception is discarded and pending state is still recorded, leaving wallet history inconsistent without reporting the failure. [error handling]
Assessment: 🟠 Major · 🔁 Occurrence: Rarely
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electrum_dash/wallet.py
**Line:** 1526:1529
**Comment:**
*Error Handling: If adding the broadcast transaction fails, the exception is discarded and pending state is still recorded, leaving wallet history inconsistent without reporting the failure.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
CodeAnt Nitpicks1 code suggestion1. Spark credentials are cleared before Spark synchronization and wallet worker shutdown, so an in-progress refresh can access missing key data during window cleanup.State/lifecycle · |
1e7b30a to
6525818
Compare
# Conflicts: # electrum_dash/dash_tx.py
42dc04e to
f1b1864
Compare
reubenyap
left a comment
reubenyap
left a comment
There was a problem hiding this comment.
Reviewed exact head 45bf26cf6ea8e1721bf6c3edad32a55295a1d51f against base cfcbdada779f9b892cee380f2e462720a27b0047. I am requesting changes because the current implementation can accept server-fabricated confirmed Spark funds, weakens wallet custody controls, ships native libraries that cannot load on intended targets, and has reorg and accounting failures. The inline comments give concrete triggers and suggested fixes.
Validation performed:
git diff --check cfcbdada..45bf26c: passed.- Parsed all eight changed Python modules with the Python AST: passed.
- Inspected the exact Windows and Linux artifacts from successful workflow run 35356682348. The builds completed, but the packaged native libraries have the dependency failures called out inline.
python -m unittest -v electrum_dash.tests.test_libsparkmobilecould not start in this environment becauseaiohttp_socksis missing. Independently, 18 of the 20 new tests skip when the native library is unavailable; the two remaining tests check constants/registration rather than constructing or serializing a Spark transaction.- The repository's
run_testsworkflow is disabled, and the only current status is CodeRabbit success after it skipped this draft PR, so there is no automated runtime or regression-test evidence on this head.
| for coin in coinsToCheck: | ||
| if coin.get('height') is None: | ||
| tx = coinsToCheckTransactions.get(coin['txid']) | ||
| if isinstance(tx, dict) and isinstance(tx.get('height'), int): |
There was a problem hiding this comment.
[P1] Verify recovered mints through the normal SPV path before crediting them. A sector row is accepted when it decrypts, and this block then copies height and blocktime from the same server's verbose transaction response. It never fetches/parses the raw transaction, verifies that the requested txid matches it, checks that the exact serialized coin and context occur in an OP_SPARKMINT output, or obtains a Merkle proof against locally validated headers. Anyone who knows the public Spark address can construct a recoverable mint offline; a malicious selected server can associate that row with an existing confirmed txid and height, causing the wallet to display confirmed, spendable-looking funds that do not exist on-chain. Fetch and validate the raw transaction and mint inclusion first, then credit only after the existing verifier has established the Merkle proof.
| if not ks or not hasattr(ks, 'get_private_key'): | ||
| raise RuntimeError(_('This wallet does not support Spark')) | ||
| from .libsparkmobile import BIP44_SPARK_CHAIN, SPARK_KEY_INDEX | ||
| self.spark_key_data = ks.get_private_key( |
There was a problem hiding this comment.
[P1] Restrict Spark to an explicitly supported single-key deterministic wallet before deriving this key. Multisig_Wallet.get_keystore() returns only x1/, so an m-of-n wallet that mints into Spark creates funds controlled by cosigner 1 alone and silently loses its threshold/recovery policy. Imported private-key wallets also pass the GUI capability test, but their get_private_key() expects a pubkey string, so [6, 1] raises TypeError and encrypted wallets loop back to the password prompt. Gate both the UI and wallet API to a standard deterministic single-sig software keystore until there is an explicit multisig/imported-wallet design.
| from .libsparkmobile import BIP44_SPARK_CHAIN, SPARK_KEY_INDEX | ||
| self.spark_key_data = ks.get_private_key( | ||
| [BIP44_SPARK_CHAIN, SPARK_KEY_INDEX], password)[0] | ||
| self.spark_password = password |
There was a problem hiding this comment.
[P1] Do not retain the raw wallet password and Spark spending key for the whole GUI session. The mint and spend dialogs later prefill this cached password, set password_required = False, and hide the authentication controls, while spark_key_data is passed directly to fee estimation and proof creation. This weakens an encrypted wallet's normal per-transaction authorization boundary, and a session-memory disclosure yields both the wallet password and Spark spending authority. The FullViewKey is already persisted and is sufficient for scanning. Keep only that view key resident, prompt for each transaction, derive the SpendKey inside the post-confirmation signing/proof operation, and clear both password and SpendKey immediately afterward.
| binaries += [('C:/x11_hash/libx11hash-0.dll', '.')] | ||
| binaries += [('C:/libsecp256k1/libsecp256k1-0.dll', '.')] | ||
| binaries += [('C:/zbarw/libzbar-0.dll', '.')] | ||
| binaries += [('C:/libsparkmobile/electrum_libsparkmobile.dll', '.')] |
There was a problem hiding this comment.
[P1] Bundle the runtime DLLs imported by this library, or link them statically. In the exact artifacts from workflow run 35356682348, the win64 DLL imports libgcc_s_seh-1.dll, libssp-0.dll, and libstdc++-6.dll, but the installer and portable package contain none of them. The win32 DLL imports libgcc_s_sjlj-1.dll, libssp-0.dll, and libstdc++-6.dll; only the GCC runtime is packaged. Loading the shipped win64 DLL fails with WinError 126, after which libsparkmobile.py silently marks Spark unavailable. Add every imported runtime or use static runtime linkage, and smoke-test ctypes loading from the finished installer/portable directory in CI.
|
|
||
|
|
||
| info "building libsparkmobile." | ||
| CC=gcc-9 CXX=g++-9 "$PROJECT_ROOT/contrib/make_libsparkmobile.sh" |
There was a problem hiding this comment.
[P1] Preserve the AppImage's declared oldest-host compatibility when adding this GCC 9 library. The exact AppImage's libelectrum_libsparkmobile.so needs libstdc++.so.6, libgcc_s.so.1, and GLIBCXX_3.4.26, but the AppImage bundles neither runtime. The build image deliberately targets Ubuntu 16.04, whose system libstdc++ does not provide GLIBCXX_3.4.26, so the new library cannot load there and Spark silently disappears. Link the C++/GCC runtimes statically or bundle compatible copies, then load the library from the finished AppImage on the oldest supported host.
| c['is_used'] = False | ||
| c.pop('spent_txid', None) | ||
|
|
||
| latestGroupId = int(await self._request( |
There was a problem hiding this comment.
[P2] Bound server-provided group IDs and anonymity-set sizes before materializing ranges or issuing requests. latestGroupId drives two full Python ranges and sequential group fetches, while meta['size'] drives one sector request per 1,500 entries. A tiny malicious response can request billions of iterations or allocations and freeze/OOM the client. Validate nonnegative, monotonic protocol bounds, cap work per refresh, and disconnect a server that exceeds them. Core limits a group to about 32,000 coins, and the pinned native wrapper rejects cover sets above 32,768, which provides a concrete upper bound.
| amount = tx.output_value() | ||
| else: | ||
| amount = self.output_value | ||
| if (getattr(tx, '_spark_fee', None) is not None |
There was a problem hiding this comment.
[P2] Keep Spark spends inside the normal fee-safety checks. This early return unconditionally enables Send and bypasses wallet.get_tx_fee_warning(), including its below-relay rejection and high absolute/rate warnings. At this point the final transaction, fee, amount, and estimated size are available, so the standard policy can evaluate the Spark spend too. Removing the bypass prevents an abnormal native fee result from being signed without the warning users receive for every other transaction.
| if framer is None: | ||
| yield | ||
| return | ||
| previous = framer.max_size |
There was a problem hiding this comment.
[P2] Do not mutate the shared session framer across awaited requests. Multiple wallet synchronizers share the Network session. If A saves 2 MB and sets 64 MB, B then saves 64 MB, and A exits while B is awaiting its response, A restores 2 MB and B can fail on a valid large reply; B then restores 64 MB permanently. This also relies on a private transport field. Configure one supported session-wide limit through NotificationSession.default_framer(), or coordinate temporary changes with session-level locking/reference counting.
| f'could not write spark shared cache {self.path}: {e!r}') | ||
|
|
||
| @staticmethod | ||
| def _fingerprint(data: dict): |
There was a problem hiding this comment.
[P2] Persist cache value changes even when its shape is unchanged. This fingerprint records only anonymity-set sizes, used-tag count, and group keys. update() can replace an equal-size set entry or change values under existing tag/group keys and then skip the disk write because the fingerprint is unchanged. The valid replacement is lost on restart and may be overwritten on the next reload. Compare complete pre/post values, or have the merge helpers report whether any stored value changed and flush on that result.
| group_data = (group_delta_sat, group_balance_sat, group_txids) | ||
| tx_item['group_data'] = group_data | ||
| transactions_tmp[txid] = tx_item | ||
| if self.spark_enabled: |
There was a problem hiding this comment.
[P2] Put Spark reconciliation in the history source used by CLI, summaries, and exports. This augmentation exists only in get_full_history(). get_detailed_history() still iterates get_onchain_history(), so private-only receipts/spends can be absent and self-mints keep their full transparent debit in the history command, Qt summary, and CSV/JSON export even when the visible history has been adjusted. Factor the Spark-aware stream into shared code and cover a Spark-only receive, spend, and self-mint through both full and detailed history.
- Verify recovered Spark coins against the raw transaction and SPV before crediting them - Anchor anonymity sets to local headers and detect forks by header hash - Release reservations of dropped Spark spends - Restrict Spark to single-key BIP32 software wallets; keep only the view key in memory and ask for the password per spend - Reconcile self-mints and Spark-only rows in full and detailed history - Reject exchange addresses, bound server-provided Spark data, keep fee checks for Spark spends - Broadcast split mints sequentially; disable preview for Spark transactions - Add Wallet > Rescan Spark - Link C++ runtimes statically for AppImage and Windows, smoke-test the DLL - Add Spark tests workflow and wallet-level Spark tests
No description provided.