Release: develop -> main - #9
Merged
Merged
Conversation
Build Tor's upstream aggregate archive so new and conditional internal libraries remain complete.
* Evo: speed up early reindex merkle checks * Fix batching bug * Spark proof state caching simplified * Ai comment resolved --------- Co-authored-by: Navid Rahimi <rahimi.nv@gmail.com>
* Add memo size check * Ai comments resolved
* fix: use glibc assert hook with clang on linux * fix: clean up mechanical warning noise * fix: clean up stacktrace warning guards * fix: update spark mint documentation comments * fix: address compatibility warning cleanup * fix: initialize warning-prone values * build: keep project warnings off vendored targets * build: apply crash hook wraps at link time * fix: add peer logic validation destructor * fix: initialize warning-prone byte storage * fix: remove unreachable rpc post command * build: mark crypto boost includes as system * fix: clean up mingw source warnings * build: scope secp256k1 warning flags to C * fix: reserve spark ownership proof streams * fix: clean up automint lock and platform guards * build: package immer as dependency * build: report global compile properties as notices * fix: address remaining build warnings * build: set macOS deployment target for dependencies * test: remove flaky MTP nonce uniqueness check * build: avoid macOS byte-swap macro redefinitions * Build: add warning interface to bitcoin_crypto * Tests: fix spark name overflow captures * Build: fix bitcoin_crypto warning noise * Build: suppress prevector false positive
* fix: clean up mechanical warning noise * fix: initialize warning-prone byte storage * logging: add modern logger infrastructure * logging: wire logger options and categories * test: cover logging compatibility * logging: keep auxiliary builds compatible * logging: harden legacy compatibility * logging: finish v31 option wiring * test: cover production logging behavior * logging: close compatibility edge cases * test: cover logging edge cases * logging: preserve wildcard legacy categories * test: cover reopen and negated logging options * logging: honor all wildcard exclusions * logging: document callback restrictions * logging: honor printtoconsole * logging: defer debug log path resolution * Build: eliminate macOS warning noise * logging: add lock-aware logging * util: make thread names portable Store the logging name in bounded thread-local storage while retaining OS-level thread naming as best effort. Initialize the daemon and Qt entry threads and add upstream-style concurrency coverage.
* Qt: fix GUI freezes on new blocks and when sending The GUI thread was acquiring cs_main/cs_wallet/cs_spark_wallet with blocking locks on paths that run on every incoming block or transaction, while the validation thread holds those locks for long stretches (Spark batch proof verification in ConnectBlock/AcceptToMemoryPool and per-output trial decryption in UpdateMintStateFromBlock / UpdateSpendStateFromBlock). Any repaint or balance poll during that window stalled the whole GUI. Sending was worse: transaction creation, including multi-second Spark zero-knowledge proof generation, ran directly in the send button slot under LOCK2(cs_main, cs_wallet). Three changes: - AddressTableModel::labelForAddress() now uses TRY_LOCK with a label cache fallback. It is called from TransactionTableModel::data() for every visible row on every repaint, so it must never block. The cache is invalidated on address book updates and refreshed on every successful lookup. - WalletModel::checkBalanceChanged() try-locks cs_main and cs_wallet up front and, on contention, sets fForceCheckBalanceChanged so the next poll retries instead of silently dropping the update. Previously it called getSparkBalance() and the watch-only getters with blocking locks after the try-lock'd TryGetBalances(), which could still stall on cs_wallet/cs_spark_wallet mid block-connect. - SendCoinsDialog now runs prepare*/sendCoins/mintSparkCoins/ spendSparkCoins on a worker thread while a local event loop keeps the GUI repainting (user input excluded, wait cursor shown), so proof generation and mempool acceptance no longer freeze the UI. The post-commit checkBalanceChanged() calls in WalletModel are queued to the GUI thread accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UERaE4GaDoWGh4dfycVzvP * Qt: address review comments on GUI freeze fix - checkBalanceChanged(): also try-lock cs_spark_wallet before fetching the Spark balance. Holding cs_main excludes the block-connect path, but Spark wallet background tasks can hold cs_spark_wallet without cs_main, so the blocking LOCK inside getSparkBalance() could still stall the GUI poll. - runWalletOperation(): join the worker thread and restore the override cursor via an RAII guard, so an exception escaping the nested event loop cannot destroy a joinable std::thread (std::terminate). - on_sendButton_clicked(): reset fNewRecipientAllowed on the invalid-Spark-mint early return; previously hitting that branch left it stuck at false. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UERaE4GaDoWGh4dfycVzvP --------- Co-authored-by: Claude <noreply@anthropic.com>
Backport Bitcoin Core commit 45a5aaf147ba to skip no-op destruction loops for trivially destructible elements. Remove the obsolete GCC suppression now that union storage is initialized.
* Wallet: lock Spark address state * Wallet: guard Spark coin metadata
…rg#1886) * Wallet: cache Spark coin lookups, show wallet load progress CSparkWallet::isMine, getMyCoinV, getMintAmount, getMyCoinIsChange, getMyCoinAddress and getMintMeta(coin) ran a full EC trial decryption (coin.identify) on every call, and getMintMeta(nonce) linearly scanned coinMeta, even though every wallet-known coin is already recorded there. During wallet load and balance computation these run per output, so startup stalled at "Loading wallet..." with per-coin EC work plus an O(outputs x coins) scan. Add two lookup indexes into coinMeta, guarded by cs_spark_wallet and maintained at every mutation site (constructor bulk load, addOrUpdateMint, eraseMint, clearAllMints, updateMintInMemory): - coin hash -> lTagHash, consulted before trial decryption. A hit requires full coin equality plus an equal serial context, so it can only return what a successful identify already produced at recording time; unknown coins fall back to the existing identify path. - nonce hash -> lTagHash, replacing the linear scan in getMintMeta(nonce), with a nonce equality check on the hit. Also surface progress while the wallet loads: a transaction count is shown every 1000 transaction records read from the wallet database and a message marks the Spark wallet loading stage, using the splash screen's existing InitMessage support. No database format changes; the indexes are rebuilt from coinMeta at construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PRhMvsTAUDw51jHJV5pTyZ * Wallet: count only successfully loaded txs in load progress Gate the progress counter on ReadKeyValue's return value so corrupt transaction records are not reported as loaded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PRhMvsTAUDw51jHJV5pTyZ * Wallet: guard lookup erasure against colliding index entries Erase a coin or nonce index entry only when it still maps to the mint being removed, so a colliding entry pointing at a surviving mint is left intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PRhMvsTAUDw51jHJV5pTyZ * Tests: cover Spark wallet lookup indexes Exercise coinLookup and nonceLookup maintenance through the public CSparkWallet API. The test registers a mint meta whose coin was built from foreign keys, so trial decryption can never identify it and every positive isMine/getMintAmount/getMintMeta answer must come from the indexes — index maintenance bugs become test failures instead of being masked by the EC fallback. Covers: miss before registration, hits after addOrUpdateMint (by coin and by nonce), the serial-context mismatch guard, updateMintInMemory, eraseMint, and clearAllMints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UERaE4GaDoWGh4dfycVzvP * Wallet: verify cached Spark mint records against identification The lookup indexes answer ownership and value queries from recorded metadata without re-running identification, shifting trust from the cryptography to wallet.dat integrity. Restore verification without restoring its cost: - A background sweep posted from the constructor re-runs identify() on every indexed record (unspent first) and evicts the fast-path entries of any record the cryptography does not confirm, logging each eviction. Queries on evicted records fall back to full identification. The sweep stops early on shutdown and skips records that no longer hold an index entry. - -sparkcacheverify (default off) additionally cross-checks every index hit against identification at query time and rejects divergent hits. - validateLookupIndexes() checks the structural invariant between coinMeta and the indexes. The lookup-index unit test now asserts it after every mutation, and a new wallet_cache_verification test covers sweep eviction: a record built from foreign keys is evicted while a wallet-own record survives, and only the fast path is dropped, never the record itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UERaE4GaDoWGh4dfycVzvP --------- Co-authored-by: Claude <noreply@anthropic.com>
…T startup (firoorg#1885) * Wallet: avoid wallet-wide scans for Spark outputs CWallet::IsMine(const CTxOut&) and GetCredit(const CTxOut&, filter) recover a Spark output's serial context by linearly scanning every output of every transaction in mapWallet before trial-decrypting the coin. Callers that iterate wallet transactions (GUI model construction, balance computation, coin listing, wallet load) therefore did O(N^2) work in wallet size, which froze the Qt splash screen at "Starting network threads..." and delayed the main window for wallets with many Spark transactions. The serial context only depends on the transaction containing the output, which these callers already have in hand. Add parent-aware overloads IsMine(const CTxOut&, const CTransaction&) and GetCredit(const CTxOut&, const CTransaction&, const isminefilter&) that derive the context directly via spark::getSerialContext(tx), and switch all callers that know the containing transaction to them. The old signatures remain as a fallback that locates the containing transaction in the wallet and delegates, so behavior is unchanged for wallet-known transactions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PRhMvsTAUDw51jHJV5pTyZ * Wallet: drop redundant Spark scan in IsSpent, address review IsSpent already fetches the wallet transaction containing the output, so the wallet-wide scan to recover the Spark serial context always rediscovered that same transaction; derive the context from it directly. This removes the remaining O(N^2) path during wallet load and coin enumeration. Also take cs_wallet around the fallback wallet scan in GetCredit(const CTxOut&, filter), use prefix increment in the new loops, expand the Doxygen comments on the parent-aware overloads, and fix brace placement in GetJMintCredit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PRhMvsTAUDw51jHJV5pTyZ --------- Co-authored-by: Claude <noreply@anthropic.com>
* qt: sign and verify messages with Spark addresses signmessagewithsparkaddress and verifymessagewithsparkaddress were reachable only over RPC, so proving control of a Spark address from the GUI meant dropping into the debug console. Wire both into the existing Sign/Verify dialog instead. The dialog detects the address type rather than growing separate tabs. The address fields already accept Spark addresses and "@name" Spark name notation (BitcoinAddressCheckValidator), so the form itself is unchanged apart from four tooltips. "@name" input is now resolved to the address it points at, which it previously was not, despite the validator accepting it. The signing and verification crypto sat inline inside the two RPC bodies and so was not callable from the GUI. It moves to spark::VerifyMessage and CSparkWallet::SignMessage, and both RPCs are rewired to call them. Every existing RPC error code and message is preserved; spark::VerifyResult carries the distinctions the RPC needs rather than collapsing them into a bool. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * spark: require a canonical ownership proof when verifying Deserialization stops as soon as the OwnershipProof is complete, so a valid signature with arbitrary bytes appended verified successfully. Reject a proof that leaves unread bytes in the stream. This is the one place this change is not behaviour preserving: verifymessagewithsparkaddress previously returned true for such a signature and now raises "Malformed ownership proof". No signer Firo ships produces one. Also covers VerifyResult::WrongNetwork, which had no test. Reported by codeant-ai and coderabbitai on firoorg#1877. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * qt: drop cs_main from spark message signing, tidy two rough edges signSparkMessage() took LOCK2(cs_main, cs_wallet), but nothing on the signing path reads chain state: isAddressMine() walks the address map, generateSpendKey() takes pwalletMain->cs_wallet itself and GetKeyFromKeypath() touches no chain. Holding cs_main on the GUI thread would stall the dialog for the duration of a block connection for no benefit. Now takes cs_wallet only, matching WalletModel::generateSparkAddress() next door. Also: - Set the error status label as plain text. It carries a string from a lower layer, unlike the literals around it that are deliberately wrapped in <nobr>, and the label is Qt::AutoText. - Truncate the proof under test by exactly one byte. Halving the hex string assumed the serialized OwnershipProof has an even byte length; if that ever stopped holding, the input would be odd-length hex and the case would report NotHex instead of the MalformedProof it means to exercise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Qt: grow the signature output field with its content A Spark ownership proof is several hundred hex characters, which made signatureOut_SM (a single-line QLineEdit) scroll unusably; even the ~88 character base64 transparent signature overflowed one line. Convert the field to a read-only QPlainTextEdit sized to its content: one line tall while empty, so the dialog layout is unchanged until a signature appears, then up to four wrapped lines. Height follows documentSizeChanged, which also covers reflow on dialog resize. Keep the existing behaviors across the widget swap: click-to-select-all now also filters the viewport, where QPlainTextEdit delivers mouse events, and the copy handler uses toPlainText(). * Qt: reject wrong-network Spark addresses in signSparkMessage The network byte returned by address.decode() was discarded, so signSparkMessage enforced no network check, unlike the RPC signing path and spark::VerifyMessage. Through the dialog the case was already caught by validateSparkAddress, but the model API should hold the same invariant on its own. Addresses review feedback on PR firoorg#1877. * qt: let the sign tab's address book offer Spark addresses The Sign tab opened AddressBookPage with the default isReused=true, which is the one condition under which populateAddressTypes() hides the address type selector and offers Transparent only. So the dialog could sign with a Spark address but gave no way to pick one from the address book. The Verify tab was already fine, since SendingTab always populates the selector. Pass isReused=false to expose the selector, and pin the initial type to Transparent so the default selection is unchanged -- without that the combo would start on Spark, since it is the first item and initialAddressType defaults to -1. Same two-line pattern as receivecoinsdialog.cpp:537. isReused has no other effect; it is read only at addressbookpage.cpp:126. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * spark: enforce canonical message proofs --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Qt: make arbitrary funds private with Spark * Qt: improve Spark mint failure recovery * Qt: relock before Spark mint retry * Qt: Report partial Spark mint commit failures CreateSparkMintTransactions can split a Make Private request into several transactions (the Split option defaults to on and groups inputs per transparent address), and mintSparkCoins commits them sequentially, returning on the first failure after earlier ones have already been broadcast. The commit failure dialog asserted total failure, misinforming the user about funds already in flight. Say "No funds were moved" only for single-transaction transfers; for split transfers, point the user at the Transactions tab. Keep the wallet's rejection reason under Details. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TV7dpUCcTQrSSxnXaR7Xuq * Qt: Guide fee-inclusive amounts in Make Private Two dead ends remained in the amount dialog. Entering the full balance (or clicking Max) silently switched to fee-subtraction mode, so the user only discovered at review that less than the typed amount would be made private. And typing an amount just under the balance failed preparation with only "change the amount and try again", leaving the user to guess how much smaller to go. Show a note under the amount field when the network fee will be deducted from the entered amount, and offer a Use Maximum button on amount/fee-exceeds-balance failures that refills the field with the highest possible amount and reopens the dialog, so the change stays explicit and reviewable before anything is sent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TV7dpUCcTQrSSxnXaR7Xuq * Qt: Keep Make Private out of watch-only column Column 2 of the overview balance grid holds the watch-only amounts, so placing the Make Private button there made it sit under the watch-only totals whenever watch-only display was on, implying it acts on funds it cannot spend. Move it to column 3 (the spacer column) left-aligned: with watch-only hidden the empty column collapses and the button still sits directly beside the eligible balance, and with watch-only shown it stays clear of that column. Also hide the button, rather than merely disabling it, when the wallet has nothing eligible to make private, so watch-only wallets do not show a control that can never act. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TV7dpUCcTQrSSxnXaR7Xuq --------- Co-authored-by: Claude <noreply@anthropic.com>
* qt: add Rosen Bridge payment URI support * qt: support payment URI paste in recipient field * qt: avoid payment URI validator shadowing * qt: guard send warning validation without wallet model * qt: remove redundant Rosen bridge code
* Consensus: bind FiroPoW header height * test: assert malformed FiroPoW header is not indexed
* Minor fixes * Fix reject score in mempool spend check Mempool policy rejections should not carry a misbehavior score. Consensus-path scoring is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UcLaCvZ9xXovFUdi3bb8bd * CI: run on minor_fixes branch Allow manual runs via workflow_dispatch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UcLaCvZ9xXovFUdi3bb8bd * ci: trigger build Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UcLaCvZ9xXovFUdi3bb8bd * Merge branch 'master' into minor_fixes * Unittests fixed * Fix the Rosen fee update bug * Fix one more minor bug * Version upgrade * optimize the cache for verification * Making 5 to 10 * Single input logic for RPC added * Version upgrade * coderabbitai comments resolved * chatgpt-codex-connector comments resolved * Other comments resolved * Unittests fixed --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Navid Rahimi <rahimi.nv@gmail.com>
…ck timer (firoorg#1889) * Fix deadlock between walletpassphrase and the wallet relock timer RPCRunLater() erases any previously scheduled lockwallet timer, and destroying a libevent timer blocks until that timer's callback has finished executing. walletpassphrase() called it while still holding LOCK2(cs_main, pwallet->cs_wallet), so when the LockWallet() callback happened to be running at that moment it was itself blocked acquiring cs_wallet, and the two threads deadlocked: the RPC worker waiting on libevent, the HTTP event loop waiting on cs_wallet. Because the RPC worker also holds cs_main, every thread that needs cs_main then piles up behind it (net message handling, the scheduler, connection handling) and the node stops making progress entirely. Nothing is logged, and SIGTERM does not complete either, since the shutdown path needs the same locks. Scope the lock so it is released before scheduling the relock, and pass the expected nRelockTime into LockWallet() so an obsolete callback that fires after the wallet was re-unlocked or already locked does nothing. * Serialize walletpassphrase and guard the RPC timer map Moving the relock scheduling out of the cs_wallet scope also removed the serialization that cs_main used to provide for the two things walletpassphrase does after a successful unlock: publishing pwallet->nRelockTime and installing the lockwallet timer. RPCRunLater() can now be entered concurrently by up to -rpcthreads (default 4) HTTP worker threads, and both steps are racy: - deadlineTimers in rpc/server.cpp is a plain std::map that RPCRunLater() erases from and emplaces into, and that StopRPC() clears. Nothing serialized those accesses other than walletpassphrase happening to hold cs_main, so concurrent calls can now corrupt the map. - Two overlapping walletpassphrase calls can publish nRelockTime and schedule the timer in opposite orders. The slower call then installs the surviving timer while the faster call owns nRelockTime; when that timer fires, the generation check in LockWallet() correctly rejects it, no other timer is left, and the wallet stays unlocked past the requested timeout. Add a per-wallet cs_unlock that walletpassphrase holds across both the unlock and RPCRunLater(), so the two steps are atomic with respect to each other. It deliberately is not cs_wallet: the lock is held across RPCRunLater(), which blocks until a running relock callback returns, and that callback takes cs_wallet - which is exactly the deadlock fixed in the previous commit. Give deadlineTimers its own cs_deadlineTimers as well, so the map stays safe regardless of which RPC calls RPCRunLater() and independently of the fact that this build only ever has one wallet. It also closes the existing race between StopRPC() and an in-flight walletpassphrase, as StopRPC() runs before the HTTP worker threads have been joined. Neither new lock is acquired by any timer callback, so the resulting lock order (cs_unlock -> cs_main -> cs_wallet, and cs_unlock -> cs_deadlineTimers) has no cycle and cannot reintroduce the deadlock. * Validate walletpassphrase timeout before unlock Reject a negative timeout with RPC_INVALID_PARAMETER instead of accepting it and scheduling a relock in the past, and clamp the timeout to 100000000 seconds so GetTime() + nSleepTime cannot overflow nRelockTime. Both checks run before the wallet is unlocked, so a rejected call leaves the wallet state untouched. This mirrors the validation upstream Bitcoin Core performs in walletpassphrase (the negative-timeout error and MAX_SLEEP_TIME clamp). Also cover the negative-timeout rejection in wallet-encryption.py, including that the wallet stays locked afterwards. Follow-up to firoorg#1889. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTd2DsfD7pTdvxDbubqASZ --------- Co-authored-by: Claude <noreply@anthropic.com>
* llmq: limit signing sessions per peer * Trivial: Follow LLMQ coding conventions * Tests: Fix signing shares include order
* Qt: split Spark payments into single-input transactions Plan ordinary private payments as one transaction per Spark coin, commit the resulting batch sequentially, restrict name registration to one fundable coin, and disable the exchange-address flow that requires a single first-stage transaction ID. * Qt: harden single-input Spark batch spending Use a compiled and unit-tested single-input planner, preserve explicit Coin Control semantics, release global locks before proof generation, and revalidate each selected outpoint during construction. Make batch fees, linkability, limits, and partial-commit behavior explicit in the interface and error reporting. * Spark: add versioned Chaum proofs Preserve the deployed proof serialization as V1 and add a componentwise V2 representation with a distinct transcript domain. Use explicit versioned verifier names, enforce protocol-sized dimensions, and keep the Chaum fuzz harnesses bounded and internally consistent. * Consensus: activate versioned Spark spends Add an explicitly versioned transaction type and bounded payload parser for V2 spends. Gate consensus, mempool, mining, and wallet selection on a disabled-by-default deployment height while preserving historical V1 validation. Exercise cross-version rejection through the transaction-level parser selected by consensus. * Fuzz: cover canonical Spark V2 payloads Exercise bounded V2 payload parsing across version, dimension, truncation, and trailing-data failures. Require every accepted payload to consume the complete stream and serialize back to the exact canonical bytes. * Consensus: bind canonical Spark V2 context Commit proofs to canonical outputs, amounts, extension data, and the exact cover-set reference map. Enforce canonical Spark Name suffixes and keep transaction classification, relay, mining, wallet construction, RPC, and display paths consistent with the versioned type. * Wallet: bind Spark construction to one tip Hold the chain and wallet locks while selecting the spend format and coins. Prepare Spark Name metadata from the same chain snapshot, require the expected next-block height, and reject construction if the tip changes before completion. * Validation: verify Spark batches before state commit Finish per-block batch verification before committing chain state and clear all accumulated proofs on every exit path. Reject unknown active-format cover-set references while retaining the historical fallback, with non-punitive mempool policy handling. * Validation: isolate VerifyDB reconnection state Recheck Spark proofs during level-4 VerifyDB reconnects while advancing only temporary coin and Evo views. Suppress persistent Spark, masternode, quorum, notification, mining, and cache mutations and roll back transient spork state on every exit. * P2P: keep Spark Name policy failures non-punitive Classify malformed Spark Name payloads received through mempool admission as zero-score policy failures while retaining ordinary invalid-block scoring for authoritative validation. Cover both direct admission and the generic transaction-check path. * HF block set * Batch verifier changes reverted * more merge issues resolved * Unittests fixed * Tests: harden Spark review fallout and restore master proof cache * Drop Spark mint uniqueness checks, prefere PR1902 * one more unittest fix * Ai review comments resolved, rpc test fixed * More comments resolved * More minor comments resolved * Consensus: reject duplicate Spark mint coins (firoorg#1902) * Consensus: reject duplicate Spark mints Reject duplicate Spark mint coins within a block or against the active chain before mutating Spark state. Track mint reservations per transaction pool so duplicate transactions are rejected and conflicts are evicted symmetrically. * Consensus: harden duplicate mint reorgs Validate completed Spark mint sets before later ConnectBlock side effects, and use stored mint heights so historical verification can distinguish an earlier duplicate from its own replay.\n\nPreserve older mint occurrences when disconnecting legacy duplicate indexes, balance their metadata, use conflict removal notifications, and add full block, mempool, rebuild, same-group, cross-group, and same-block regressions. --------- Co-authored-by: levonpetrosyan93 <petrosyan.levon93@gmail.com> * revert removed check, comments added * Spark: synchronize minted coin state (firoorg#1910) * Wallet: Fix deadlock between walletpassphrase and the lockwallet relock timer (firoorg#1889) * Fix deadlock between walletpassphrase and the wallet relock timer RPCRunLater() erases any previously scheduled lockwallet timer, and destroying a libevent timer blocks until that timer's callback has finished executing. walletpassphrase() called it while still holding LOCK2(cs_main, pwallet->cs_wallet), so when the LockWallet() callback happened to be running at that moment it was itself blocked acquiring cs_wallet, and the two threads deadlocked: the RPC worker waiting on libevent, the HTTP event loop waiting on cs_wallet. Because the RPC worker also holds cs_main, every thread that needs cs_main then piles up behind it (net message handling, the scheduler, connection handling) and the node stops making progress entirely. Nothing is logged, and SIGTERM does not complete either, since the shutdown path needs the same locks. Scope the lock so it is released before scheduling the relock, and pass the expected nRelockTime into LockWallet() so an obsolete callback that fires after the wallet was re-unlocked or already locked does nothing. * Serialize walletpassphrase and guard the RPC timer map Moving the relock scheduling out of the cs_wallet scope also removed the serialization that cs_main used to provide for the two things walletpassphrase does after a successful unlock: publishing pwallet->nRelockTime and installing the lockwallet timer. RPCRunLater() can now be entered concurrently by up to -rpcthreads (default 4) HTTP worker threads, and both steps are racy: - deadlineTimers in rpc/server.cpp is a plain std::map that RPCRunLater() erases from and emplaces into, and that StopRPC() clears. Nothing serialized those accesses other than walletpassphrase happening to hold cs_main, so concurrent calls can now corrupt the map. - Two overlapping walletpassphrase calls can publish nRelockTime and schedule the timer in opposite orders. The slower call then installs the surviving timer while the faster call owns nRelockTime; when that timer fires, the generation check in LockWallet() correctly rejects it, no other timer is left, and the wallet stays unlocked past the requested timeout. Add a per-wallet cs_unlock that walletpassphrase holds across both the unlock and RPCRunLater(), so the two steps are atomic with respect to each other. It deliberately is not cs_wallet: the lock is held across RPCRunLater(), which blocks until a running relock callback returns, and that callback takes cs_wallet - which is exactly the deadlock fixed in the previous commit. Give deadlineTimers its own cs_deadlineTimers as well, so the map stays safe regardless of which RPC calls RPCRunLater() and independently of the fact that this build only ever has one wallet. It also closes the existing race between StopRPC() and an in-flight walletpassphrase, as StopRPC() runs before the HTTP worker threads have been joined. Neither new lock is acquired by any timer callback, so the resulting lock order (cs_unlock -> cs_main -> cs_wallet, and cs_unlock -> cs_deadlineTimers) has no cycle and cannot reintroduce the deadlock. * Validate walletpassphrase timeout before unlock Reject a negative timeout with RPC_INVALID_PARAMETER instead of accepting it and scheduling a relock in the past, and clamp the timeout to 100000000 seconds so GetTime() + nSleepTime cannot overflow nRelockTime. Both checks run before the wallet is unlocked, so a rejected call leaves the wallet state untouched. This mirrors the validation upstream Bitcoin Core performs in walletpassphrase (the negative-timeout error and MAX_SLEEP_TIME clamp). Also cover the negative-timeout rejection in wallet-encryption.py, including that the wallet stays locked afterwards. Follow-up to firoorg#1889. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTd2DsfD7pTdvxDbubqASZ --------- Co-authored-by: Claude <noreply@anthropic.com> * Spark: synchronize minted coin state Protect the active minted-coin map with a dedicated short-lived lock so asynchronous wallet scans cannot race block connection or disconnection. Return snapshots to callers instead of exposing map references. * Spark: preserve legacy duplicate mints on disconnect --------- Co-authored-by: Danswar <48102227+Danswar@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: levonpetrosyan93 <petrosyan.levon93@gmail.com> * Build fix * Review commets resolved * Consensus: validate Spark coin output types (firoorg#1901) * Consensus: validate Spark coin output types * Validation: contextualize Spark block checks Defer the preliminary body check for Spark type mismatches until AcceptBlock has validated the header and established its indexed height. This prevents unknown-parent and context-invalid blocks from reaching legacy Spark parsing while preserving historical block behavior. Add regressions for unknown parents, contextual difficulty, activated mismatches, stale unsolicited forks, and the INT_MAX caller sentinel. * Tests: qualify chain parameters * Consensus: enforce Spark coin types at Chaum V2 height Drop the unused nSparkCoinTypeFixStartBlock and reuse nSparkChaumV2StartBlock so opcode/coin-type pairs activate with the existing hard fork. --------- Co-authored-by: levonpetrosyan93 <petrosyan.levon93@gmail.com> * Build fix * Consensus: Fail closed on Spark batch verification (firoorg#1863) * Spark: fail closed on deferred batch verification * Consensus: address Spark batch feedback * Docs: document Spark batch APIs * Consensus: address remaining Spark batch feedback * Consensus: simplify Spark batch fail-closed boundary Review follow-up that keeps the fail-closed invariant while cutting the surface of the change: - Restore the connected-block SyncTransaction loop to its original position under cs_main in ActivateBestChain; moving it out of the lock changed notification ordering guarantees for wallet and LLMQ listeners and was unrelated to this PR. - Drop the nChainHeight plumbing and GetSparkBatchVerificationHeight. With ZC_MINT_CONFIRMATIONS == 1 the pinned-height cover set equals what GetCoinSet derives from the active tip at every guarded call site, so the parameter was a behavioral no-op. - Remove the now-unused BatchProofContainer::verify and make batch_spark private. - Share the batching predicate between ConnectBlock and ActivateBestChainStep via ShouldBatchSparkProofs so the collector and the guard cannot silently diverge. - Latch a failed batch (fBatchFailed) so the abort path does not re-run full proof verification at every subsequent boundary; the failed proofs are still retained until removed by disconnect. - Drop the Shutdown early-return: the final FlushStateToDisk already refuses to persist validation state under cs_main when the pending batch fails, and completing teardown avoids exiting with live network threads and an unflushed wallet. - Hold cs_main while verifying the pending batch before clearing the reindex flag in ThreadImport. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 * Tests: add Spark batch verification coverage Add a unit test that drives a real wallet-created Spark spend through the deferred-batch path: collection instead of inline verification, successful batch verification of a valid proof, fail-closed behavior with retained proofs when cover sets cannot be rebuilt, the failed batch latch, and recovery once the offending spend is removed. Add a spark_batching.py regtest that mines a Spark chain with timestamps older than one day, then reindexes with -batching=1 and asserts the node batch verifies the spends before completing the reindex (and reaches the same tip as -batching=0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 * Consensus: reset Spark batch failure latch on removal The fBatchFailed latch made batch_spark() fail fast after a failed verification, but remove() never cleared it. After a disconnect removed the offending spend, any remaining pending proofs kept failing fast forever, so recovery was only possible when removal emptied the batch. Clear the latch when remove() actually erases proofs: the pending batch changed, so the previous failure verdict no longer applies and the next boundary re-runs full verification. Extend spark_batch_tests with the partial-removal scenario: a batch holding a valid proof and an invalid one (a raw-parsed spend whose binding data is absent) fails and latches, and removing only the invalid spend lets the remaining valid proof verify again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 * Tests: wait for batch marker in spark_batching.py The tip can reach the target height while the final deferred Spark batch is still pending, so asserting on the debug.log success marker right after the height check could race and flake. For a batched reindex, wait until both the expected height and the batch verification success marker are present under the same deadline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 * Tests: make spark_batching.py executable The rpc-tests runner executes test scripts directly, so a script registered in rpc-tests.py without the executable bit fails the whole RPC test phase with PermissionError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 * Consensus: improve Spark batch failure recovery Address maintainer feedback that aborting on a failed Spark batch reproduced the old behavior's worst property: a plain restart redoes the reindex/sync and hits the same batched failure again, and the operator only learns which spend is invalid by manually reindexing with -batching=0. Two changes make the failure path actionable: - Identify offenders at failure time: the batch container now records the txid alongside each collected proof, and when the batch fails it re-verifies the retained proofs individually (cover sets are already built) and logs the exact invalid spend transactions to debug.log. - Auto-recover on plain restart: on failure the node persists a sparkbatchfailed flag in the block tree DB before aborting. On the next start LoadBlockIndexDB() reads and clears the flag and forces -batching=0 for that run, so the restart verifies Spark proofs block by block and rejects the invalid block through the normal consensus path instead of looping into the same batched abort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 * Tests: fix spark_batching.py spend setup The RPC test failed in CI with "Spark spend creation failed": CreateSparkSpendTransaction() refuses to build a spend unless the anonymity set contains at least two coins, and the test minted only one. Mint two coins before spending. Also truncate debug.log before each reindex so the batch verification success marker asserted by the test can only come from the reindex run itself, not from the live mining phase or the shutdown flush that precedes it. While at it, make verify_pending() discard the in-progress collection via init() so the temporary txid vector stays in lockstep with the temporary proof vector. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 * Consensus: survive -reindex restart in batch failure recovery The sparkbatchfailed recovery marker was stored as a block tree DB flag and read in LoadBlockIndexDB(). As Codex review pointed out, a node whose failed run was started with -reindex and which gets restarted with the same arguments (e.g. by a process manager) never performs the recovery: -reindex wipes the block tree database, destroying the flag, and LoadBlockIndex() skips LoadBlockIndexDB() entirely, so batching stayed enabled and the run looped into the same batched failure. Store the marker as a plain datadir file instead and consume it in AppInitMain right after fReindex is determined, before any database is opened. The marker now survives the reindex wipe and disables batching for the next run regardless of how the node is restarted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 * Tests: wait for wallet to settle after reindex in spark_batching.py The post-reindex balance assertions read getsparkbalance() immediately after the tip reached the target height, racing the wallet's asynchronous catch-up: a Debug CI run failed with fullBalance briefly equal to just the two spend change coins while coin metadata was still being rewritten. Poll for the expected balance with a deadline before asserting, on both the batched and block-by-block reindex paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ci8HSbWLZHxA1CXVPKdL44 --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: levonpetrosyan93 <45027856+levonpetrosyan93@users.noreply.github.com> * Merge conflicts and leftover bugs fixed * Unittest fixed * Consensus: Fix Spark group ID aliasing (firoorg#1907) * Consensus: Fix Spark group ID aliasing * Spark: avoid copying spend block hash map Bind the result of getBlockHashes() to a const reference instead of copying the std::map<uint64_t, uint256> on every Spark spend validation. The accessor returns a const reference into the spend object, which outlives all uses, and the map is only read afterwards, matching the reference binding already used for getCoinGroupIds(). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01848eNswZ82pAqdJcFA4Zi6 * Spark: avoid copy in batch cover-set caching Replace the count()-then-operator[] insertion in both batch_spark cover-set cache loops with a find() guard and emplace of the moved vector. This drops a default construction plus full copy of each retrieved cover set and follows the project guideline of using find() instead of operator[] on maps. GetCoinSet usage is unchanged. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01848eNswZ82pAqdJcFA4Zi6 * Consensus: Gate canonical Spark group IDs on Chaum V2 Drop the separate nSparkCanonicalGroupIdStartBlock height and enforce canonical 32-bit cover-set IDs at nSparkChaumV2StartBlock instead. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: levonpetrosyan93 <petrosyan.levon93@gmail.com> Co-authored-by: levonpetrosyan93 <45027856+levonpetrosyan93@users.noreply.github.com> * Spark: fix out-of-range vector writes in SpendKey derivation (firoorg#1894) * Tests: pin Spark spend key derivation of s1 and s2 The derivation of (s1, s2) from r in SpendKey is consensus-critical for wallets: any change alters every wallet's view keys and addresses, orphaning previously received funds. The deployed derivation is s1 = memberFromSeed(SHA256d("s1_generation" || ser(r))) s2 = memberFromSeed(SHA256d("s2_generation")) where the s2 seed intentionally commits only to its prefix, so s2 is identical for every spend key. PR firoorg#1767 accidentally changed this by keeping the serialized s1 in the hashed buffer, and was reverted in PR firoorg#1784; PR firoorg#1893 proposed the same change again. This test pins the deployed derivation so any future drift fails CI: - determinism of (s1, s2) in r - s1 depends on r while s2 does not - both scalars match an explicit re-derivation via CHash256 refs firoorg#1893 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HVp6JH7LvzRpZXU4VEjdDG * Spark: fix out-of-range vector writes in SpendKey SpendKey::SpendKey(params, r) cleared its hash input and output vectors and then kept writing through data()/&result[0], accessing storage outside the vectors' live element range. This is undefined behavior (and asserts in MSVC debug builds), although in practice the writes stayed inside the retained capacity of the same allocations, so it never corrupted memory in release builds. Keep both buffers at their required sizes for the whole derivation and wipe them with memory_cleanse() before scope exit instead of clear(), so sensitive intermediate material is not left behind in freed heap memory. Behavior is byte-for-byte identical to the deployed derivation: the cleared `data` vector previously contributed zero bytes to the s2 seed hash, so the s2 seed commits only to the "s2_generation" prefix. The dead s1.serialize() into that buffer is dropped and the invariant is documented in place; the spend_key_derivation unit test pins it. Unlike firoorg#1767 (reverted in firoorg#1784) and firoorg#1893, this does not change the derived s2, so existing wallets keep their view keys and addresses. refs firoorg#1893 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HVp6JH7LvzRpZXU4VEjdDG --------- Co-authored-by: Claude <noreply@anthropic.com> * Unittests fixed * Guix build fixed * Spark: bound cover-set reference work * Wallet: Remove Spark mints and restore spends on reorg (firoorg#1895) * Wallet: Roll back Spark coins on block disconnect DisconnectTip fed the wallet rollback with a stateless CheckTransaction re-run over the transactions of the disconnected block, which cannot produce usable data for the wallet: - Since firoorg#1873 CheckSparkSpendTransaction returns before the SMint outputs are collected when fStatefulSigmaCheck is false, so mints created by spend transactions (private change) never reach block.sparkTxInfo and RemoveSparkMints() is called with nothing. - The coins that path does collect are built by ParseSparkMintCoin() alone, and Coin::serial_context is not serialized. Spend-created coins are bound to the spent linking tags and pure mints to the serialized inputs, so without the serial context Coin::identify() fails the serial commitment check, throws, and RemoveSparkMints() skips the coin. The rollback of pure mints has therefore never worked either. - spentLTags is filled behind the same stateful gate, so RemoveSparkSpends() has always been given an empty set and the wallet side of a spend was never rolled back at all. Take the rollback data from the transactions of the block instead. spark::GetSparkMintCoins() parses the coins and binds them to the serial context of their transaction, covering spend-created SMints as well as pure mints, and spark::GetSparkUsedTags() returns the linking tags of a spend. Transactions that DisconnectTip resurrected into the pools are skipped: they still exist, only unconfirmed again, their coins are still the wallet's and their inputs are still spent, and pool acceptance has re-registered both. Only what did not make it back is rolled back. RemoveSparkSpends() now takes the linking tags directly. The group ids of its unordered_map parameter were never read and its only caller passed block.sparkTxInfo->spentLTags; AbandonSpends(), which had the same body, forwards to it. The regression test mints coins, spends part of one so that the change comes back as an SMint output of the spend, mines the spend and disconnects that block with the spend conflicting in the pools, the way a competing spend on the new chain would leave it. It asserts on wallet state, not on the collected coins, because Coin::operator== ignores serial_context. fixes firoorg#1887 * P2P: Set ban score on two Spark spend checks The mixed mint check and the private output limit check in CheckSparkSpendTransaction() returned a bare false without touching the validation state. Both only run in the stateful path today, where ConnectBlock's wrapper happens to re-wrap the failure in DoS(100) and mempool acceptance rejects the transaction with no reject reason and no penalty for the peer that relayed it. Set the score and a descriptive reject reason at the check itself, so the rejection is self-describing and the peer penalty stays correct if these checks ever run in the stateless path again. No block or transaction changes validity, only reject metadata and ban score. * Wallet: Hold cs_spark_wallet across RemoveSparkMints The erase of each mint took the lock through eraseMint(), but the loop around them did not, so a removal was not atomic against the wallet jobs running on the thread pool. Take the lock for the whole loop, the way RemoveSparkSpends() and AbandonSpends() already do. The lock is recursive, so the nested eraseMint() is unaffected, and the order stays the established cs_wallet -> cs_spark_wallet of AddToWallet() -> HandleSparkTransaction(). * Wallet: Open the wallet DB once in RemoveSparkSpends CWalletDB was constructed inside the loop, so a rollback opened and closed a handle once per linking tag. Hoist it above the loop, the way UpdateMintStateFromBlock() already does. This function only started receiving a non-empty set with the previous commit, so the per-tag construction had never run in the reorg path before. Read coinMeta through a single find() instead of count() followed by operator[], per the map access guideline in doc/developer-notes.md. The count() guard meant the read never inserted a default; this only drops the second lookup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UQdhRWx2azweZvrcHDk33y --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: levonpetrosyan93 <45027856+levonpetrosyan93@users.noreply.github.com> * Tests: avoid replacing global Spark state * Tests: use valid Spark group topology * Qt: restore Rosen Bridge OP_RETURN * Tests: preserve legacy Spark reference fixture * Include first mint block in cover-set hashes * Unittest fixed * Spark: H2 cover-set hash policy and cache clear - Clear spend-proof cache when HF-1 connects - Mempool-only ATMP gate (null info + INT_MAX) - Require 32-byte cover-set hash: mempool H2-10, consensus H2 - Wallet refuses unbound cover sets from H2-10 - Tests for rollover/cache/policy and wallet reject * Qt: report partial Spark mint commits * HF block set * Unittest fixed * Review comments resolved - Skip the txid-only spend-proof cache during VerifyDB - Relay only when -walletbroadcast=1; fCheckTransaction still accepts to mempool - Spark spend commits keep mempool checks without forcing relay - Show Spark fee-mismatch amounts in the GUI display unit * Unittest fixed * More review comments resolved - After a Spark batch failure, disable batching and force -reindex - Drop leftover Spark spends at H2-1 that fail H2 consensus - Record LTags on the admitting pool and evict conflicts from mempool and stem - Drop queued Spark wallet updates before disconnect resurrection - Fail mint commits on mempool rejection and keep one serialization per mint tx - Report a partial mint failure from commits that landed, not prepared tx count * Unittests fixed * Restore master batch-verify timing * Unittest fixed * Review comments resolved - Verify the terminal Spark batch before clearing the reindex flag - Switch and verify the pending batch under cs_main - Keep sparkbatchfailed until a successful -reindex -batching=0 run - Skip only dead Spark wallet updates instead of a global epoch * Making rpc test dip3-deterministicmns.py stable * Review comments resolved - Write sparkbatchfailed when batch collection starts; remove after success - Re-check mempool/chain under the wallet write locks - Run Spark mint/spend wallet jobs on one worker - Zap Spark mints and spends on -reindex from scratch * Spark: avoid wallet shutdown in state constructor --------- Co-authored-by: sneurlax <sneurlax@gmail.com> Co-authored-by: Reuben Yap <reuben@firo.org> Co-authored-by: Danswar <48102227+Danswar@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Sync the fork with upstream v0.14.18.0, the mandatory hard fork release that activates at block 1,371,000 and introduces the versioned Spark spend format. Nodes built from the previous state of this branch reject every block after the activation height with bad-txns-prevout-null, because the new Spark spend inputs carry null prevouts that a pre-fork CheckTransaction does not recognise as a Spark spend. The fork carries no source changes of its own: every path outside .github/ is taken verbatim from the upstream tag, and the branch delta is limited to the workflow files.
4a5b50eb - Merge upstream v0.14.18.0 (mandatory hard fork at block 1,371,000)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 31 new commit(s)
Checklist