Add electrum_libsparkmobile

[levoncrypto]

No description provided.

[codeant-ai]

🤖 CodeAnt AI — Review Status

Status	Commit	Started (UTC)	Finished (UTC)
✅ Reviewed your PR	d70d381	Sep 02, 2026 · 12:22	12:26

[codeant-ai]

---

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

[codeant-ai]

CodeAnt-AI Description

Add a native Spark wallet library for Electrum-Firo

What Changed

• Adds a shared native library that lets Electrum-Firo create and validate Spark addresses, recover coins, build mint and spend transactions, estimate fees, and create Spark Name scripts.
• Provides a C-compatible interface for loading the library from Python, including full-view-key serialization and controlled memory release.
• Rejects malformed inputs, invalid network addresses, unusable group elements, oversized lists, and invalid fixed-size data with safe error results instead of allowing failures to escape.
• Builds reproducibly on Linux, macOS, and Windows using pinned Spark Mobile, OpenSSL, and Boost dependencies with verified downloads and dependency checkouts.

Impact

✅ Spark wallet operations available to Electrum-Firo
✅ Clearer errors for invalid transaction and address data
✅ Reproducible native builds across desktop platforms

💡 Usage Guide

Checking Your Pull Request

Every 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 AI

Got 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You 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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To 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]

CodeAnt Nitpicks

2 code suggestions

1. These variable names do not match the consumer, which checks GMP_INCLUDE_DIR and GMP_LIBRARIES; GMP is therefore silently disabled even when installed.

Api mismatch · src/cmake/FindGMP.cmake:1-2

---

2. CTxOut constructors leave nRounds uninitialized, so newly created outputs contain indeterminate round data.

Logic error · src/transaction.h:142

[reubenyap]

Requesting changes: this PR is not ready for a post-H2, V2-only Electrum release.

Release blockers

1. [P0] The ABI creates only legacy V1 spends. The current wrapper exposes neither V2 nor the extension commitment and calls the pre-V2 builder. At H2 (height 1,371,000), V1 remains single-input; multi-input is enabled only by the separate V2 format. For this wallet's post-fork contract, expose no version selector: pin sparkmobile a9d078955312b3467f5bb14cbd64ab2ed8852256, hardcode SpendTransactionVersion::V2 in spend, fee, and Spark Name paths, and require exactly 32 commitment bytes. Plain spends use zero; Spark Name uses its computed commitment and appends the matching extension. The Electrum caller must emit and parse nVersion=3, nType=11, never silently fall back to type 9.

2. [P1] The pinned Spark revision contains reproduced memory-safety defects. ASan confirms:

   • every spend-key derivation writes 32 bytes through a vector after its size is cleared;
   • an authenticated one-byte memo claiming length 31 causes a 31-byte stack overread;
   • authenticated recipient data can declare and allocate a 32 MiB memo before EOF rejection.
     The same pin also suppresses coin-identification failure and returns partially uninitialized metadata. Updating to a9d0789 fixes these root causes, its first-use parameter race, and noncanonical address decoding.

3. [P1/P2] Outer serialized-coin parsing is still allocation-amplifiable, including on a9d0789. AEADEncryptedData generic-deserializes attacker-declared vector sizes before checking fixed sizes. A direct test allocated exactly 5,000,000 bytes before reporting EOF. The wrapper also accepts trailing bytes: a valid 244-byte coin with 1 MiB appended was accepted. Use fixed-size field parsing, a tight total-size bound, and require stream exhaustion in the shared decoder.

4. [P2 privacy] The server controls anonymity-set age. Core accepts a canonical historical reference within a group. Without an independent latest-mature-state check, an Electrum server can supply an older valid prefix containing the selected coin and reduce anonymity without invalidating the spend. Require the latest mature independently verified group reference and a minimum/expected set size.

Other confirmed findings

• Negative amounts/counts return successful fee estimates; UINT64_MAX mint values and negative vout wrapping are accepted. Core limits this to invalid-transaction/DoS behavior, not inflation.
• Returned serializedCoinContext is always empty. This breaks bookkeeping, although it does not invalidate the proof.
• The dylib exports 4,689 globals: 17 intended C functions and 4,672 dependency/C++ symbols. Hardening flags cover only the wrapper files.
• There are no registered tests, and the documented public Electrum binding/type-11 integration is absent.
• OpenSSL 1.1.1w is EOL, and referenced LGPL/Boost license texts are missing.

Minimum path to merge

• Bump the Spark pin and patch the remaining fixed-size AEAD parser.
• Make the ABI unconditionally V2 and update Electrum atomically for type 11 and extension commitments.
• Centralize strict coin/numeric/aggregate validation.
• Validate cover-set freshness.
• Add one post-H2 end-to-end check covering single- and multi-input V2, local V1 rejection, malformed coins, and stale cover sets.
• Restrict exports and resolve OpenSSL/license packaging.

I built the exact head twice, built it again with ASan/UBSan, and independently reproduced the high-severity paths. I did not find a reachable value-inflation or key-extraction flaw in a9d0789's V2 proof construction.

[reubenyap]

[P2 privacy] The server supplies the cover set and historical block reference. A canonical but stale prefix can still yield a valid spend with reduced anonymity. The integration must require the latest mature independently verified group reference and a minimum/expected set size.

[reubenyap]

Second-pass review of c5cc9bb: the important fixes are real. Spend construction and fee estimation are now V2-only, the Spark pin contains the prior memory-safety fixes, a clean macOS build and ASan/UBSan build pass, GMP is gone, and the dylib exports only the intended 16 C symbols.

I am still requesting changes for the dependency-verification bypass, malformed-input allocation amplification, late V2 resource checks, incomplete redistribution notices, and the missing executable caller/CI gate.

Release requirements outside this repository also remain: the companion Electrum change must use nVersion=3/nType=11; testnet and devnet must disable Spark sends because Core keeps V2 activation at INT_MAX; and Electrum must independently require the latest mature cover-set reference because consensus accepts valid historical prefixes.

Correction to my earlier parser comment: do not require blanket stream exhaustion. A valid first-mint payload is a 244-byte coin followed by a 66-byte Schnorr proof. The remaining parser defect is nested length allocation before fixed-size validation, described inline below.

[reubenyap]

[P2] The 64 KiB outer cap does not stop nested CompactSize allocation amplification. A reproduced 108-byte ABI input declared a 5,000,000-byte AEAD ciphertext and directly caused a 5,000,000-byte allocation before rejection. Validate the fixed ciphertext/tag/commitment sizes before resizing in sparkmobile, add the malformed-coin regression, and repin. Do not require stream.empty() here: the first mint output legitimately carries a trailing 66-byte proof.

[reubenyap]

[P1] This referenced binding/test file is absent from public Electrum-Firo, this repository's ctest reports No tests were found, and the PR head has no GitHub status checks. Link the companion type-11 caller PR and make its V2/ABI regression test a required CI gate before merging this native library.

[reubenyap]

Current-head triage for 03cae944.

Address in this library PR

• P1: dependency pin verification must include Git-ignored files; an ignored OpenSSL header was compiled into the supposedly pinned archive.
• P2: fix the reproduced nested CompactSize allocation amplification in sparkmobile, add the malformed-input regression, and repin. The existing line-64 thread is the canonical report.
• P2: raise the declared CMake minimum to match the commands actually used.
• P3: correct the bundled sparkmobile copyright notice.
• P3: reject negative key indexes and diversifiers. The existing negative-diversifier thread is valid but low severity, not “Major.”

Required in the companion Electrum integration before release

• Add type-11 parsing/construction, the ctypes caller, and required end-to-end/ABI CI.
• Enforce independently verified latest-mature cover-set references and a minimum anonymity set.
• Disable Spark sends on testnet/devnet; do not add a V1 fallback.
• Ensure the packaging script selects a valid target platform; an unset platform currently produces a dylib with an extra @rpath/libsparkmobile.dylib dependency.

Not actionable for this PR

The unresolved Boost-wrapper comments are either disproved by explicit exact-head builds or concern unused MPI, Locale, and iOS infrastructure. The shipped target links only Boost.Thread and Boost.Chrono. The UsedCoin and null-plus-zero comments are also false positives. V2 selection, the zero plain-spend commitment, V2 limits, amount validation, and the broad license-text issue are resolved.

[reubenyap]

Final review triage (03cae944)

The unresolved threads now represent only work that should be acted on:

Fix in this library PR

1. Include Git-ignored files in dependency pin verification.
2. Fix the bounded nested CompactSize allocation amplification upstream, add the regression, and repin.
3. Raise the CMake minimum to 3.13.
4. Correct the pinned sparkmobile copyright to Copyright (c) 2022 Firo.

Complete in the companion Electrum work before release

1. Add type-11 parsing/construction, the ctypes binding, and executable ABI/end-to-end CI.
2. Enforce latest-mature cover-set references and a minimum anonymity set in the wallet layer.
3. Disable Spark sends on testnet/devnet without adding V1 fallback.
4. Ensure packaging always selects a supported target platform.

Resolved as false positive, not applicable, or non-blocking

• The Boost test/serialization/system/wave/fiber/coroutine/filesystem reports are false positives for this build.
• MPI, Locale, and the iOS toolchain are unused by this desktop target.
• UsedCoin is already validly declared.
• Null-plus-zero pointer arithmetic is defined in C++17.
• Negative index/diversifier behavior is optional caller-contract hardening, not a demonstrated defect.
• The older parser thread was superseded by the precise line-64 allocation report.
• V2-only selection, zero plain-spend commitment, V2 limits, amount checks, and bundled license texts are resolved.

The 5 MB parser case is allocation amplification, not memory corruption. Zero in-repo ctest tests is not independently a defect; the release issue is that the claimed caller-side tests and CI are not publicly present.