aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-09-30doc: add note about confusing HaveTxsDownloaded nameJames O'Beirne
2023-09-30contrib: add script to demo/test assumeutxoJames O'Beirne
Add the script to the shellcheck exception list since the quoted variables rule needs to be violated in order to get bitcoind to pick up on $CHAIN_HACK_FLAGS.
2023-09-30test: add feature_assumeutxo functional testJames O'Beirne
Most ideas for test improvements (TODOs) provided by Russ Yanofsky.
2023-09-30rpc: add getchainstatesJames O'Beirne
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-09-30refuse to activate a UTXO snapshot if mempool not emptyJames O'Beirne
This ensures that we avoid any unexpected conditions inherent in transferring non-empty mempools across chainstates. Note that this should never happen in practice given that snapshot activation will not occur outside of IBD, based upon the height checks in `loadtxoutset`.
2023-09-30rpc: add loadtxoutsetJames O'Beirne
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2023-09-30validation: do not activate snapshot if behind active chainJames O'Beirne
Most easily reviewed with git show --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-09-30validation: assumeutxo: swap m_mempool on snapshot activationJames O'Beirne
Otherwise we will not receive transactions during background sync until restart.
2023-09-30blockstorage: segment normal/assumedvalid blockfilesJames O'Beirne
When using an assumedvalid (snapshot) chainstate along with a background chainstate, we are syncing two very different regions of the chain simultaneously. If we use the same blockfile space for both of these syncs, wildly different height blocks will be stored alongside one another, making pruning ineffective. This change implements a separate blockfile cursor for the assumedvalid chainstate when one is in use.
2023-09-30validation: populate nChainTx value for assumedvalid chainstatesJames O'Beirne
Use the expected AssumeutxoData in order to bootstrap nChainTx values for assumedvalid blockindex entries in the snapshot chainstate. This is necessary because nChainTx is normally built up from nTx values, which are populated using blockdata which the snapshot chainstate does not yet have.
2023-09-30test: adjust chainstate tests to use recognized snapshot baseJames O'Beirne
In future commits, loading the block index while making use of a snapshot is contingent on the snapshot being recognized by chainparams. Ensure all existing unittests that use snapshots use a recognized snapshot (at height 110). Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-09-30validation: pruning for multiple chainstatesJames O'Beirne
Introduces ChainstateManager::GetPruneRange(). The prune budget is split evenly between the number of chainstates, however the prune budget may be exceeded if the resulting shares are beneath `MIN_DISK_SPACE_FOR_BLOCK_FILES`.
2023-09-30validation: indexing changes for assumeutxoJames O'Beirne
When using an assumedvalid chainstate, only process validationinterface callbacks from the background chainstate within indexes. This ensures that all indexes are built in-order. Later, we can possibly designate indexes which can be built out of order and continue their operation during snapshot use. Once the background sync has completed, restart the indexes so that they continue to index the now-validated snapshot chainstate.
2023-09-30net_processing: validationinterface: ignore some events for bg chainJames O'Beirne
2023-09-30wallet: validationinterface: only handle active chain notificationsJames O'Beirne
2023-09-30validationinterface: only send zmq notifications for activeJames O'Beirne
2023-09-30validation: pass ChainstateRole for validationinterface callsJames O'Beirne
This allows consumers to decide how to handle events from background or assumedvalid chainstates.
2023-09-30validation: only call UpdatedBlockTip for active chainstateJames O'Beirne
This notification isn't needed for background chainstates. `kernel::Notifications::blockTip` are also skipped.
2023-09-30validation: add ChainstateRoleJames O'Beirne
2023-09-30validation: MaybeRebalanceCaches when chain leaves IBDJames O'Beirne
Check to see if we need to rebalance caches across chainstates when a chain leaves IBD.
2023-09-30chainparams: add blockhash to AssumeutxoDataJames O'Beirne
This allows us to reference assumeutxo configuration by blockhash as well as height; this is helpful in future changes when we want to reference assumeutxo configurations before the block index is loaded.
2023-09-30assumeutxo: remove snapshot during -reindex{-chainstate}James O'Beirne
Removing a snapshot chainstate from disk (and memory) is consistent with existing reindex operations.
2023-09-30bugfix: correct is_snapshot_cs in VerifyDBJames O'Beirne
2023-09-30net_processing: Request assumeutxo background chain blocksSuhas Daftuar
Add new PeerManagerImpl::TryDownloadingHistoricalBlocks method and use it to request background chain blocks in addition to blocks normally requested by FindNextBlocksToDownload. Co-authored-by: Ryan Ofsky <ryan@ofsky.org> Co-authored-by: James O'Beirne <james.obeirne@gmail.com>
2023-09-29Merge bitcoin/bitcoin#27653: test: add unit test coverage for Python ECDSA ↵Andrew Chow
implementation 96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983 test: add unit test coverage for Python ECDSA implementation (Sebastian Falbesoner) Pull request description: This PR adds missing unit test coverage for the Python ECDSA implementation, which should be useful for detecting potential problems early whenever changes in the test framework's Python implementation of secp256k1 are made (e.g. #26222). Note that right now we don't call `ECPubKey.verify_ecdsa` anywhere in our tests, so we wouldn't notice if it is broken at some point. To keep it simple, the already existing unit test for Schnorr signatures is extended to also check ECDSA signatures. For that purpose, the dictionary storing private-key/public-key entries use their legacy types `ECKey/ECPubKey` instead of bare byte-arrays, and for Schnorr signing/verification the necessary conversions (ECKey -> bare private key, ECPubKey -> x-only pubkey) is done later when needed. To avoid code duplication, a helper function `random_bitflip` for damaging signatures is introduced. The unit test can be run by either calling it for this single module: `$ python3 -m unittest ./test/functional/test_framework/key.py` or simply running `$ ./test/functional/test_runner.py` which calls all test framework module's unit tests at the start (see TEST_FRAMEWORK_MODULES list). ACKs for top commit: achow101: ACK 96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983 sipa: utACK 96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983 stratospher: tested ACK 96b3f2d. Tree-SHA512: b993f25b843fa047376addda4ce4b0f15750ffba926528b5cca4c5f99b9af456206f4e8af885d25a017dddddf382ddebf38765819b3d16a3f28810d03b010808
2023-09-29Merge bitcoin/bitcoin#27866: blockstorage: Return on fatal flush errorsRyan Ofsky
d8041d4e042957660827313951b18c8dd9a99a16 blockstorage: Return on fatal undo file flush error (TheCharlatan) f0207e00303a1030eca795ede231e3c0d94df061 blockstorage: Return on fatal block file flush error (TheCharlatan) 5671c15f4520c6dc20e0805fd0b06157ff94bcd7 blockstorage: Mark FindBlockPos as nodiscard (TheCharlatan) Pull request description: The goal of this PR is to establish that fatal blockstorage flush errors should be treated as errors at their call site. Prior to this patch `FlushBlockFile` may have failed without returning in `Chainstate::FlushStateToDisk`, leading to a potential write from `WriteBlockIndexDB` that may refer to a block that is not fully flushed to disk yet. By returning if either `FlushUndoFile` or `FlushBlockFile` fail, we ensure that no further write operations take place that may lead to an inconsistent database when crashing. Add `[[nodiscard]]` annotations to them such that they are not ignored in future. Functions that call either `FlushUndoFile` or `FlushBlockFile`, need to handle these extra abort cases properly. Since `Chainstate::FlushStateToDisk` already produces an abort error in case of `WriteBlockIndexDB` failing, no extra logic for functions calling `Chainstate::FlushStateToDisk` is required. Besides `Chainstate::FlushStateToDisk`, `FlushBlockFile` is also called by `FindBlockPos`, while `FlushUndoFile` is only called by `FlushBlockFile` and `WriteUndoDataForBlock`. For both these cases, the flush error is not further bubbled up. Instead, the error is logged and a comment is provided why bubbling up an error would be less desirable in these cases. --- This pull request is part of a larger effort towards improving the shutdown / abort / fatal error handling in validation code. It is a first step towards implementing proper fatal error return type enforcement similar as proposed by theuni in this pull request [comment](https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1563561502). For ease of review of these critical changes, a first step would be checking that `AbortNode` leads to early and error-conveying returns at its call site. Further work for enforcing returns when `AbortNode` is called is done in https://github.com/bitcoin/bitcoin/pull/27862. ACKs for top commit: stickies-v: re-ACK d8041d4 ryanofsky: Code review ACK d8041d4e042957660827313951b18c8dd9a99a16 Tree-SHA512: 47ade9b873b15e567c8f60ca538d5a0daf32163e1031be3212a3a45eb492b866664b225f2787c9e40f3e0c089140157d8fd1039abc00c7bdfeec1b52ecd7e219
2023-09-29Merge bitcoin/bitcoin#28525: net: Drop v2 garbage authentication packetAndrew Chow
e3720bca398820038b3e97f467adb2c45ef9ef5f net: Simplify v2 recv logic by decoupling AAD from state machine (Tim Ruffing) b0f5175c044df956c0f07f540706d457c4912856 net: Drop v2 garbage authentication packet (Tim Ruffing) Pull request description: Note that this is a breaking change, see also https://github.com/bitcoin/bips/pull/1498 The benefit is a simpler implementation: - The protocol state machine does not need separate states for garbage authentication and version phases. - The special case of "ignoring the ignore bit" is removed. - The freedom to choose the contents of the garbage authentication packet is removed. This simplifies testing. ACKs for top commit: naumenkogs: ACK e3720bca398820038b3e97f467adb2c45ef9ef5f sipa: ACK e3720bca398820038b3e97f467adb2c45ef9ef5f. Re-ran the v2 transport fuzzer overnight. ajtowns: ACK e3720bca398820038b3e97f467adb2c45ef9ef5f - simpler and more flexible, nice achow101: ACK e3720bca398820038b3e97f467adb2c45ef9ef5f Sjors: utACK e3720bca398820038b3e97f467adb2c45ef9ef5f theStack: Code-review ACK e3720bca398820038b3e97f467adb2c45ef9ef5f Tree-SHA512: 16600ed868c8a346828de075c4072e37cf86440751d08ab099fe8b58ff71d8b371a90397d6a4247096796db68794275e7e0403f218859567d04838e0411dadd6
2023-09-28Add instructions for headerssync-params.py to release-process.mdPieter Wuille
2023-09-28Update parameters in headerssync.cppPieter Wuille
2023-09-28Add headerssync-params.py script to the repositoryPieter Wuille
2023-09-28Merge bitcoin/bitcoin#28540: tests: Fix wallet_resendwallettransactions.py ↵Andrew Chow
intermittent failure by using manual bumps instead of bumpfee b5a962564eb15075e4e2a7bc0c235a56fa998ac3 tests: Use manual bumps instead of bumpfee for resendwallettransactions (Andrew Chow) Pull request description: Bumpfee will try to increase the entire package to the target feerate, which causes repeated bumpfees to quickly shoot up in fees, causing intermittent failures when the fee is too large. We don't care about this property, just that the child is continuously replaced until we observe it's position in mapWallet is before its parent. Instead of using bumpfee, we can create raw transactions which have only pay (just above) the additional incremental relay fee, thus avoiding this problem. Fixes #28491 ACKs for top commit: kevkevinpal: ACK [b5a9625](https://github.com/bitcoin/bitcoin/pull/28540/commits/b5a962564eb15075e4e2a7bc0c235a56fa998ac3) mzumsande: Code review ACK b5a962564eb15075e4e2a7bc0c235a56fa998ac3 pablomartin4btc: ACK b5a962564eb15075e4e2a7bc0c235a56fa998ac3 -> adding the `try_rpc` to avoid (skip) any possible failure around the manual bump fee (if we ever reach it as [explained](https://github.com/bitcoin/bitcoin/pull/28540#issuecomment-1737648048)) makes a lot of sense as the spirit of the test is the tx (child before parent) sort in the `mapWallet` (as also [explained](https://github.com/bitcoin/bitcoin/issues/28491#issuecomment-1736161363)). MarcoFalke: lgtm ACK b5a962564eb15075e4e2a7bc0c235a56fa998ac3 Tree-SHA512: f184f11c73be0c30753181901f51a3b4b9c4135e0c4681e9f4ca94692c49bac15c91683c85266a2124333c8593e9919bfd9102724616faab299740f2eb98741f
2023-09-28lint: fix custom mypy cache dir settingFabian Jahr
2023-09-28Merge bitcoin/bitcoin#28450: Add package evaluation fuzzerglozow
262ab8ef7860d43cebc9d04721e3a075b4edf06e Add package evaluation fuzzer (Greg Sanders) Pull request description: This fuzzer target caught the issue in https://github.com/bitcoin/bitcoin/pull/28251 within 5 minutes on master branch, and an additional issue which I've applied a preliminary patch to cover. Fuzzer target does the following: 1) Picks mempool confgs, including max package size, count, mempool size, etc 2) Generates 1 to 26 transactions with arbitrary coins/fees, the first N-1 spending only confirmed outpoints 3) Nth transaction, if >1, sweeps all unconfirmed outpoints in mempool 4) If N==1, it may submit it through single-tx submission path, to allow for more interesting topologies 5) Otherwise submits through package submission interface 6) Repeat 1-5 a few hundred times per mempool instance In other words, it ends up building chains of txns in the mempool using parents-and-children packages, which is currently the topology supported on master. The test itself is a direct rip of tx_pool.cpp, with a number of assertions removed because they were failing for unknown reasons, likely due to the notification changes of single tx submission to package, which is used to track addition/removal of transactions in the test. I'll continue working on re-adding these assertions for further invariant testing. ACKs for top commit: murchandamus: ACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e glozow: reACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e dergoegge: tACK 262ab8ef7860d43cebc9d04721e3a075b4edf06e Tree-SHA512: 190784777d0f2361b051b3271db8f79b7927e3cab88596d2c30e556da721510bd17f6cc96f6bb03403bbf0589ad3f799fa54e63c1b2bd92a2084485b5e3e96a5
2023-09-27Merge bitcoin/bitcoin#28505: rpc: bumpfee, improve doc for 'reduce_output' argAndrew Chow
b3db8c9d5ccfe5c31341169fa7ac044427122921 rpc: bumpfee, improve doc for 'reduce_output' arg (furszy) Pull request description: Fixes #28180. Resulted from discussions with S3RK, achow101, and Murch. The current argument name and description are dangerous as it don't describe the case where the user selects the recipient output as the change address. This one could end up been increased by the inputs minus outputs remainder. Which, when `bumpfee` adds new inputs to the transaction, leads the process to send more coins to the recipient. Which is not what the user would expect from a 'reduce_output' param naming. ACKs for top commit: S3RK: ACK b3db8c9d5ccfe5c31341169fa7ac044427122921 achow101: ACK b3db8c9d5ccfe5c31341169fa7ac044427122921 murchandamus: ACK b3db8c9d5ccfe5c31341169fa7ac044427122921 Tree-SHA512: 91f607e2f5849041d7c099afdddae11af8bed5b1ac90c9d22921267f272e21b44e107d6968e037f05f958a61fe29e94e5fb44b224fb3606f197f83ec4ba3b1e7
2023-09-27Add package evaluation fuzzerGreg Sanders
2023-09-27key: don't allocate secure mem for null (invalid) keyPieter Wuille
Instead of storing the key material as an std::vector (with secure allocator), use a secure_unique_ptr to a 32-byte array, and use nullptr for invalid keys. This means a smaller CKey type, and no secure/dynamic memory usage for invalid keys.
2023-09-27Add make_secure_unique helperAnthony Towns
Co-authored-by: Pieter Wuille <bitcoin-dev@wuille.net>
2023-09-27wallet: Add TxStateString function for debugging and loggingRyan Ofsky
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2023-09-27tests: Use manual bumps instead of bumpfee for resendwallettransactionsAndrew Chow
Bumpfee will try to increase the entire package to the target feerate, which causes repeated bumpfees to quickly shoot up in fees, causing intermittent failures when the fee is too large. We don't care about this property, just that the child is continuously replaced until we observe it's position in mapWallet is before its parent. Instead of using bumpfee, we can create raw transactions which have only pay the additional incremental relay fee, thus avoiding this problem.
2023-09-27test: Avoid test failure on Linux root without cap-add LINUX_IMMUTABLEMarcoFalke
2023-09-27build, macos: Fix `qt` package build with new Xcode 15 linkerHennadii Stepanov
2023-09-27net: Simplify v2 recv logic by decoupling AAD from state machineTim Ruffing
2023-09-27net: Drop v2 garbage authentication packetTim Ruffing
See also https://github.com/bitcoin/bips/pull/1498 The benefit is a simpler implementation: - The protocol state machine does not need separate states for garbage authentication and version phases. - The special case of "ignoring the ignore bit" is removed. - The freedom to choose the contents of the garbage authentication packet is removed. This simplifies testing.
2023-09-26test: Test loading wallets with conflicts without a chainAndrew Chow
Loading a wallet with conflicts without a chain (e.g. wallet tool and migration) would previously result in an assertion due to -1 being both a valid number of conflict confirmations, and the indicator that that member has not been set yet.
2023-09-26wallet: Check last block and conflict height are valid in MarkConflictedAndrew Chow
MarkConflicted calculates conflict confirmations incorrectly when both the last block processed height and the conflicting height are negative (i.e. uninitialized). If either are negative, we should not be marking conflicts and should exit early.
2023-09-26rpc: bumpfee, improve doc for 'reduce_output' argfurszy
The current argument name and description are dangerous as it don't describe the case where the user selects the recipient output as the change address. This one could end up been increased by the inputs minus outputs remainder. Which, when bumpfee adds new inputs to the transaction, leads the process to send more coins to the recipient. Which is not what the user would expect from a 'reduce_output' param naming. Co-authored-by: Murch <murch@murch.one>
2023-09-26Merge bitcoin/bitcoin#28483: refactor: Return CAutoFile from ↵fanquake
BlockManager::Open*File() fa56c421be04af846f479c30749b17e6663ab418 Return CAutoFile from BlockManager::Open*File() (MarcoFalke) 9999b89cd37fb2a23c5ebcd91d9cb31d69375933 Make BufferedFile to be a CAutoFile wrapper (MarcoFalke) fa389d902fbf5fac19fba8c5d0c5e0a25f15ca63 refactor: Drop unused fclose() from BufferedFile (MarcoFalke) Pull request description: This is required for https://github.com/bitcoin/bitcoin/pull/28052, but makes sense on its own, because offloading logic to `CAutoFile` instead of re-implementing it allows to delete code and complexity. ACKs for top commit: TheCharlatan: Re-ACK fa56c421be04af846f479c30749b17e6663ab418 willcl-ark: tACK fa56c421be Tree-SHA512: fe4638f3a6bd3f9d968cfb9ae3259c9d6cd278fe2912cbc90289851311c8c781099db4c160e775960975c4739098d9af801a8d2d12603f371f8edfe134d8f85a
2023-09-24Merge bitcoin/bitcoin#28512: doc: Be vague instead of wrong about ↵fanquake
MALLOC_ARENA_MAX 12f7257b8f6ba0de1fb05b598e916c8b14bcab8c doc: Be vague instead of wrong about MALLOC_ARENA_MAX (Tim Ruffing) Pull request description: Before this commit, we claim that glibc's malloc implementation uses 2 arenas by default. But that's true only on 32-bit systems, and even there, it uses *up* to 2 arenas. This commit fixes the wrong statement. The new statement is intentionally vague to reduce our maintenance burden. For details, see: https://www.gnu.org/software/libc/manual/html_node/Memory-Allocation-Tunables.html#index-glibc_002emalloc_002earena_005fmax Noticed in: https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1728103427 ACKs for top commit: fanquake: ACK 12f7257b8f6ba0de1fb05b598e916c8b14bcab8c Tree-SHA512: c0ff1e35b682a841e366a1cad26e18ff79a93d97103529be35a972c7dcbb95f5354e7a7b98a86731f491434d64685bb58cc3cc9100f0577d8f75db05e951b09a
2023-09-23Merge bitcoin/bitcoin#28385: [refactor] rewrite ↵fanquake
DisconnectedBlockTransactions to not use boost 4313c77400eb8eaa8586db39a7e29a861772ea80 make DisconnectedBlockTransactions responsible for its own memory management (glozow) cf5f1faa037e9a40a5029cc7dd4ee61454b62466 MOVEONLY: DisconnectedBlockTransactions to its own file (glozow) 2765d6f3434c101fe2d46e9313e540aa680fbd77 rewrite DisconnectedBlockTransactions as a list + map (glozow) 79ce9f0aa46de8ff742be83fd6f68eab40e073ec add std::list to memusage (glozow) 59a35a7398f5bcb3e3805d1e4f363e4c2fb336b3 [bench] DisconnectedBlockTransactions (glozow) 925bb723ca71aa76380b769d8926c7c2ad9bbb7b [refactor] batch-add transactions to DisconnectedBlockTransactions (glozow) Pull request description: Motivation - I think it's preferable to use stdlib data structures instead of depending on boost if we can achieve the same thing. - Also see #28335 for further context/motivation. This PR simplifies that one. Things done in this PR: - Add a bench for `DisconnectedBlockTransactions` where we reorg and the new chain has {100%, 90%, 10%} of the same transactions. AFAIU in practice, it's usually close to 100%. - Rewrite `DisconnectedBlockTransactions` as a `std::list` + `unordered_map` instead of a boost multi index container. - On my machine, the bench suggests the performance is very similar. - Move `DisconnectedBlockTransactions` from txmempool.h to its own kernel/disconnected_transactions.h. This struct isn't used by txmempool and doesn't have much to do with txmempool. My guess is that it's been living there for convenience since the boost includes are there. ACKs for top commit: ismaelsadeeq: Tested ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 stickies-v: ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 TheCharlatan: ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 Tree-SHA512: 273c80866bf3acd39b2a039dc082b7719d2d82e0940e1eb6c402f1c0992e997256722b85c7e310c9811238a770cfbdeb122ea4babbc23835d17128f214a1ef9e
2023-09-23Merge bitcoin/bitcoin#28492: RPC: `descriptorprocesspsbt` returns hex ↵Andrew Chow
encoded tx if complete a99e9e655a58b2364a74aec5cafb827a73c6b0c4 doc: add release note (ismaelsadeeq) 2b4edf889a4b555c8c7f6793fa5d820e5513ecac test: check `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq) c405207a18fdee75a4dea470bb0d13e59e15ce45 rpc: `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq) Pull request description: Coming from [#28414 comment](https://github.com/bitcoin/bitcoin/pull/28414#pullrequestreview-1618684391) Same thing also for `descriptorprocesspsbt`. Before this PR `descriptorprocesspsbt` returns a boolean `complete` which indicates that the psbt is final, users then have to call `finalizepsbt` to get the hex encoded network transaction. In this PR if the psbt is complete the return object also has the hex encoded network transaction ready for broadcast with `sendrawtransaction`. This save users calling `finalizepsbt` with the descriptor, if it is already complete. ACKs for top commit: achow101: ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4 pinheadmz: ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4 ishaanam: ACK a99e9e655a58b2364a74aec5cafb827a73c6b0c4 Tree-SHA512: c3f1b1391d4df05216c463127cd593f8703840430a99febb54890bc66fadabf9d9530860605f347ec54c1694019173247a0e7a9eb879d3cbb420f9e8d9839b75
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-15 20:26:35 +0000