aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-10-26wallet: encapsulate pre-selected-inputs lookup into its own functionfurszy
First step towards decoupling the pre-selected-inputs fetching functionality from `SelectCoins`. Which, will let us not waste resources calculating the available coins if one of the pre-set inputs has an error. (right now, if one of the pre-set inputs is invalid, we first walk through the entire wallet txes map just to end up failing right after it finish)
2022-10-26wallet: skip manually selected coins from 'AvailableCoins' resultfurszy
No need to walk through the entire wallet's txes map just to get coins that we could have gotten by just doing a simple map.find(out.hash). (Which is what we are doing inside `SelectCoins` anyway)
2022-10-26wallet: skip available coins fetch if "other inputs" are disallowedfurszy
no need to waste resources calculating the wallet available coins if they are not going to be used. The 'm_allow_other_inputs=true` default value change is to correct an ugly misleading behavior: The tx creation process was having a workaround patch to automatically fall back to select coins from the wallet if `m_allow_other_inputs=false` (previous default value) and no manual inputs were selected. This could be seen in master in flows like `sendtoaddress`, `sendmany` and even the GUI, where the `m_allow_other_inputs` value isn't customized and the wallet still selects and adds coins to the tx internally.
2022-10-20Merge bitcoin/bitcoin#26158: bench: add "priority level" to the benchmark ↵Andrew Chow
framework 3e9d0bea8deb61596c91ead997e9db83f5b0ff68 build: only run high priority benchmarks in 'make check' (furszy) 466b54bd4ab8227ff8c066a027a92791366a81c1 bench: surround main() execution with try/catch (furszy) 3da7cd2a762077fa81dc40832d556d8a3fd53674 bench: explicitly make all current benchmarks "high" priority (furszy) 05b8c76232dedf938740e8034c725ac16d32974a bench: add "priority level" to the benchmark framework (furszy) f1593780b8e3b6adefee08b10d270c5c329f91fe bench: place benchmark implementation inside benchmark namespace (furszy) Pull request description: This is from today's meeting, a simple "priority level" for the benchmark framework. Will allow us to run certain benchmarks while skip non-prioritized ones in `make check`. By default, `bench_bitcoin` will run all the benchmarks. `make check`will only run the high priority ones, and have marked all the existent benchmarks as "high priority" to retain the current behavior. Could test it by modifying any benchmark priority to something different from "high", and run `bench_bitcoin -priority-level=high` and/or `bench_bitcoin -priority-level=medium,low` (the first command will skip the modified bench while the second one will include it). Note: the second commit could be avoided by having a default arg value for the priority level but.. an explicit set in every `BENCHMARK` macro call makes it less error-prone. ACKs for top commit: kouloumos: re-ACK 3e9d0bea8deb61596c91ead997e9db83f5b0ff68 achow101: ACK 3e9d0bea8deb61596c91ead997e9db83f5b0ff68 theStack: re-ACK 3e9d0bea8deb61596c91ead997e9db83f5b0ff68 stickies-v: re-ACK https://github.com/bitcoin/bitcoin/commit/3e9d0bea8deb61596c91ead997e9db83f5b0ff68 Tree-SHA512: ece59bf424c5fc1db335f84caa507476fb8ad8c6151880f1f8289562e17023aae5b5e7de03e8cbba6337bf09215f9be331e9ef51c791c43bce43f7446813b054
2022-10-20build: only run high priority benchmarks in 'make check'furszy
2022-10-20bench: surround main() execution with try/catchfurszy
so we have a cleaner exit on internal runtime errors. e.g. an unknown priority level.
2022-10-20bench: explicitly make all current benchmarks "high" priorityfurszy
no-functional changes. Only have set the priority level explicitly on every BENCHMARK macro call.
2022-10-20bench: add "priority level" to the benchmark frameworkfurszy
Will allow us to run certain benchmarks while skip non-prioritized ones in 'make check'.
2022-10-20Merge bitcoin/bitcoin#25595: Verify PSBT inputs rather than check for fields ↵fanquake
being empty e133264c5b1f72e94dcb9cebd85cdb523fcf8070 Add test for PSBT input verification (Greg Sanders) d25699280af1ea45bebc884f63a10da7ea275ef9 Verify PSBT inputs rather than check for fields being empty (Greg Sanders) Pull request description: In a few keys spots, PSBT finality is checked by looking for non-empty witness data. This complicates a couple things: 1) Empty data can be valid in certain cases 2) User may be passed bogus final data by a counterparty during PSBT work happening, and end up with incorrect signatures that they may not be able to check in other contexts if the UTXO doesn't exist yet in chain/mempool, timelocks, etc. On the whole I think these heavier checks are worth it in case someone is actually assuming the signatures are correct if our API is saying so. ACKs for top commit: achow101: ACK e133264c5b1f72e94dcb9cebd85cdb523fcf8070 Tree-SHA512: 9de4fbb0be1257b081781f5df908fd55666e3acd5c4e36beb3b3f2f5a6aed69ff77068c44cde6127e159e773293fd9ced4c0bb47e693969f337e74dc8af030da
2022-10-19Merge bitcoin/bitcoin#25830: refactor: Replace m_params with ↵MacroFake
chainman.GetParams() 5d3f98d27879cd6d84b8590e947336e8d09613ed refactor: Replace m_params with chainman.GetParams() (Aurèle Oulès) Pull request description: Fixes a TODO introduced in #24595. Removes `m_params` from `CChainState` class and replaces it with `m_chainman.GetParams()`. ACKs for top commit: MarcoFalke: review ACK 5d3f98d27879cd6d84b8590e947336e8d09613ed 🌎 Tree-SHA512: de0fe31450d281cc7307c0d820495e86c93c7998e77a148db2c703da66cff1059e6560c041f1864913c42075aa24d259c2623d45e929ca0a8056ed330a9f9978
2022-10-19Merge bitcoin/bitcoin#26286: test: Remove unused txmempool include from testsMacroFake
1c48dae76fc808bf7154aca976e7303364c56509 test: Use C++11 member initializers for TestMemPoolEntryHelper (MacroFake) fad7f2239c74a4db31b3023f2bcaf1f0852453f8 test: Remove unused txmempool include from tests (MacroFake) Pull request description: Seems odd to include this heavy header in all tests despite it only being used in a few tests. Can be reviewed with `--color-moved=dimmed-zebra --ignore-all-space` ACKs for top commit: aureleoules: reACK 1c48dae76fc808bf7154aca976e7303364c56509 hebasto: ACK 1c48dae76fc808bf7154aca976e7303364c56509, I have reviewed the code and it looks OK, I agree it can be merged. w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26286/commits/1c48dae76fc808bf7154aca976e7303364c56509 Tree-SHA512: 31f2808d04ec33bfc2409832b8e59e6c870eaa98fbcf879e1c786492c7d07134711b30f8290bdb34e1b8f7b8f2f11dae8e10c64e7eb31f584b2f5c58fcc7743b
2022-10-19Merge bitcoin/bitcoin#26142: Use `PACKAGE_NAME` in messages rather than ↵MacroFake
hardcoding "Bitcoin Core" b147322a7a387423164de5a7d91a33eacad51689 Use `PACKAGE_NAME` in messages rather than hardcoding "Bitcoin Core" (Hennadii Stepanov) Pull request description: Usually, we do not hardcode "Bitcoin Core" in the user-faced messages. See: - bitcoin/bitcoin#18646 - bitcoin/bitcoin#19282 Also grammar has been improved -- singular instead of plural. ACKs for top commit: jarolrod: ACK b147322a7a387423164de5a7d91a33eacad51689 Tree-SHA512: b135c18703dfdd7b63d4cb27d1ac48f6a9dbf69382142ae381f33bf561cbf57477a11d1c73263aa834f705206d7dd5716df2523d38ed0d4cfec8babc38bb017a
2022-10-19Merge bitcoin/bitcoin#26179: bench: Add missed `ECCVerifyHandle` instanceMacroFake
f09d47b263459e27faf5416287255eb3aed369a2 bench: Add missed `ECCVerifyHandle` instance (Hennadii Stepanov) Pull request description: To clearly observe the lack of an `ECCVerifyHandle` instance, - apply the following diff: ```diff --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -19,11 +19,9 @@ bench_bench_bitcoin_SOURCES = \ bench/bench.h \ bench/bench_bitcoin.cpp \ bench/block_assemble.cpp \ - bench/ccoins_caching.cpp \ bench/chacha20.cpp \ bench/chacha_poly_aead.cpp \ bench/checkblock.cpp \ - bench/checkqueue.cpp \ bench/crypto_hash.cpp \ bench/data.cpp \ bench/data.h \ @@ -46,8 +44,7 @@ bench_bench_bitcoin_SOURCES = \ bench/rpc_blockchain.cpp \ bench/rpc_mempool.cpp \ bench/strencodings.cpp \ - bench/util_time.cpp \ - bench/verify_script.cpp + bench/util_time.cpp nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES) ``` - then ``` $ ./autogen $ ./configure $ make clean $ make ``` - then ``` $ ./src/bench/bench_bitcoin -filter=ExpandDescriptor bench_bitcoin: pubkey.cpp:296: bool CPubKey::IsFullyValid() const: Assertion `secp256k1_context_verify && "secp256k1_context_verify must be initialized to use CPubKey."' failed. Aborted (core dumped) ``` ACKs for top commit: achow101: ACK f09d47b263459e27faf5416287255eb3aed369a2 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26179/commits/f09d47b263459e27faf5416287255eb3aed369a2 Tree-SHA512: e1f33f88d427c57fe31d5810d12e9f46fed2911f5736208ebf7d4a968de0dd8c1f6b73a0d1093316da117dd3bcfda5dde6e41d6c95fcdb99bdea62e19df5ad20
2022-10-19Merge bitcoin/bitcoin#26206: test: check importing wallets when blocks are ↵MacroFake
pruned throw an error 4aff7a48a4e0f1075306f181a276b8a74c857022 test: check importing wallets when blocks are pruned throw an error (brunoerg) Pull request description: This PR adds test coverage for the following error: https://github.com/bitcoin/bitcoin/blob/437b608df289c97fd88f1dd79bdc8359e1b1c5b1/src/wallet/rpc/backup.cpp#L513-L518 ACKs for top commit: andrewtoth: ACK 4aff7a48a4e0f1075306f181a276b8a74c857022 Tree-SHA512: fbbf6056cb3759f726b8a5ff25fca51bf47e973e5d655ec164e2bec88e2dbd3b243677869d2cf33af268ea635ca0f2e9f737c4734077fc5a936ac3a24ad4b88b
2022-10-18test: Use C++11 member initializers for TestMemPoolEntryHelperMacroFake
Co-authored-by: Aurèle Oulès <aurele@oules.com>
2022-10-18test: Remove unused txmempool include from testsMacroFake
2022-10-18Merge bitcoin/bitcoin#26321: Adjust `.tx/config` for new Transifex CLIfanquake
d6adbb7ee1de661ad89879609eecd11129322405 Adjust `.tx/config` for new Transifex CLI (Hennadii Stepanov) Pull request description: The old Transifex Command-Line Tool is considered deprecated (as of January 2022) and will sunset on Nov 30, 2022. See: https://github.com/transifex/cli/blob/devel/README.md#migrating-from-older-versions-of-the-client An accompanying PR: https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/142 ACKs for top commit: jarolrod: ACK d6adbb7ee1de661ad89879609eecd1112932240 Tree-SHA512: aa63158e02dc136be1669f3ff9ca28a990da215227520a197968ebe1a0a6fa73526cbd6d8fd7eed608eb06ad242c66598826c58e124c1fc53beaa28ae6b53c37
2022-10-18Merge bitcoin/bitcoin#26313: doc: consolidate library documentation to ↵MacroFake
libraries.md af781bf4b2998eb17e89b6b24d26a2590e548259 doc: fix typo in doc/libraries.md (fanquake) 9e9ae6101ff505b97a514148e86a5699acba20df doc: remove library commentary from src/Makefile.am (fanquake) Pull request description: Deduplicate the makefile comments, in favour of doc/libraries.md. I think a single, more comprehensive source of truth is preferable. Diagrams are also useful. Came up in https://github.com/bitcoin/bitcoin/pull/26292#issuecomment-1275094478. ACKs for top commit: ryanofsky: Code review ACK af781bf4b2998eb17e89b6b24d26a2590e548259, nice cleanups hebasto: ACK af781bf4b2998eb17e89b6b24d26a2590e548259, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: df61ed1394102221701ae2dfa42886dfabe9d9fd7f601b794e2195f93d8f7c2a1cd1c000a77d0a969b42328e8ebc0387755c57291837b283fdf376dbd98fdda1
2022-10-17Merge bitcoin/bitcoin#23443: p2p: Erlay support signalingglozow
e56d1d2afdd477be6dd462d838617d385bac5d7b test: Add functional tests for sendtxrcncl message from outbound (Gleb Naumenko) cfcef60779e62bcb81243e0bb8ffc8abb5b5baf5 test: Add functional tests for sendtxrcncl from inbound (Gleb Naumenko) b99ee9d22d53777f501d0926d7ca6b0b45b237a1 test: Add unit tests for reconciliation negotiation (Gleb Naumenko) f63f1d3f4bdef2923a8395473ca59e4763769bd7 p2p: clear txreconciliation state for non-wtxid peers (Gleb Naumenko) 88d326c8e3796b9685c141fa50628615d3e43a38 p2p: Finish negotiating reconciliation support (Gleb Naumenko) 36cf6bf2168f9f154a652c91bbb96480c2e1d044 Add helper to see if a peer is registered for reconciliations (Gleb Naumenko) 4470acf076289831ac60bcbafb6b3afa0e98e99d p2p: Forget peer's reconciliation state on disconnect (Gleb Naumenko) 3fcf78ee6ab59d1a539fdf54bbae21b5d29caea9 p2p: Announce reconciliation support (Gleb Naumenko) 24e36fac0a86aa371046470dc4f8dfed7a868fb2 log: Add tx reconciliation log category (Gleb Naumenko) Pull request description: This is a part of the Erlay project: - [parent PR](https://github.com/bitcoin/bitcoin/pull/21515) - [associated BIP-330](https://github.com/bitcoin/bips/pull/1376). ------- This PR adds a new p2p message `sendtxrcncl` signaling for reconciliation support. Before sending that message, a node is supposed to "pre-register" the peer by generating and storing an associated reconciliation salt component. Once the salts are exchanged within this new message, nodes "register" each other for future reconciliations by computing and storing the aggregate salt, along with the reconciliation parameters based on the connection direction. ACKs for top commit: dergoegge: re-ACK e56d1d2afdd477be6dd462d838617d385bac5d7b sipa: re-ACK e56d1d2afdd477be6dd462d838617d385bac5d7b. No differences with a rebase of previously reviewed e91690e67dad180c7fb9bed0409a9c4567d3e5df. mzumsande: re-ACK e56d1d2afdd477be6dd462d838617d385bac5d7b vasild: ACK e56d1d2afdd477be6dd462d838617d385bac5d7b Tree-SHA512: 0db953b7347364e2496ebca3bfe6a27ac336307eec698242523a18336fcfc7a1ab87e3b09ce8b2bdf800ebbb1c9d33736ffdb8f5672f93735318789aa4a45f39
2022-10-17Add test for PSBT input verificationGreg Sanders
2022-10-17Verify PSBT inputs rather than check for fields being emptyGreg Sanders
2022-10-17Merge bitcoin/bitcoin#25985: Revert "build: Use Homebrew's sqlite package if ↵Andrew Chow
it is available" d216d714aae36e6f1c95f82aef81a0be74dee2f3 Revert "build: Use Homebrew's sqlite package if it is available" (fanquake) Pull request description: This reverts ee7b84e63cbeadd5e680d69ff0548275581e9241 from #20527. That change was made without any rationale, maybe other than, a brew installed version might be newer, and that's "better". However when building from source on macOS, it just results in drastically worse performance, and issues / confusion like #25724. The difference in performance can be observed using the example from https://github.com/bitcoin/bitcoin/issues/25724#issuecomment-1213554922, but minified i.e: ```bash time src/bitcoin-cli createwallet speedy true time src/bitcoin-cli importdescriptors '[ {"desc":"raw(00145846369f3d6ba366d6f5a903fb5cf4dca3763c0e)#k9wh6v62","timestamp":"now"}, {"desc":"raw(001420800aabf13f3a4c4ce3ce4c66cecf1d17f21a6e)#6m0hlfh4","timestamp":"now"}, {"desc":"raw(0014c6bf9715e06d73ebf9b3b02d5cc48d24d8bbabc1)#wyavh36r","timestamp":"now"}, {"desc":"raw(00141ba7807b3f46af113beaea5c698428ce7138cd8a)#jctdsups","timestamp":"now"}, {"desc":"raw(00140c1bd27f10fff01b36ddf3c1febaa1acff19b080)#9s6nc3pk","timestamp":"now"}, {"desc":"raw(00141226e31987e4bc2e63c0ee12908f675e40464b20)#9pp7qm39","timestamp":"now"}, {"desc":"raw(0014f73f149f7503960a5e849c6ee7a8a8c336f631cb)#qtkxv9fc","timestamp":"now"}, {"desc":"raw(0014c8ccb4d81ffc769fc5fdd8d7eed69b0e0cae5749)#hn39qayv","timestamp":"now"}, {"desc":"raw(001498565aead2d67a22a6021d55210f2a917fc22169)#6ar3vwsx","timestamp":"now"}, {"desc":"raw(001403013248ac0cd9eabe176cad162cda2a19f771e1)#4m47mukd","timestamp":"now"} ]' ``` Running master, when building from souce and using brew installed sqlite, this takes ~3.4s. With this PR, the same operation takes ~0.3s. Resolves the "build from source" portion of #25724. Building from depends is still not ideal, however I have some other changes that might help improve things in that case. Related performance issue reports: * https://github.com/bitcoindevkit/bdk/issues/749 * https://bitcoin.stackexchange.com/questions/113898/bitcoin-v23-is-10-times-slower-than-v22-on-macos-for-basic-regtest-tests * https://github.com/bitcoin/bitcoin/issues/25724 * https://github.com/bitcoin/bitcoin/pull/25985#issuecomment-1245942400 ACKs for top commit: achow101: ACK d216d714aae36e6f1c95f82aef81a0be74dee2f3 jarolrod: ACK d216d714aae36e6f1c95f82aef81a0be74dee2f3 hebasto: ACK d216d714aae36e6f1c95f82aef81a0be74dee2f3, I have reviewed the code and it looks OK, I agree it can be merged. No conflicts with our build [docs](https://github.com/bitcoin/bitcoin/blob/d216d714aae36e6f1c95f82aef81a0be74dee2f3/doc/build-osx.md#descriptor-wallet-support). Tree-SHA512: 1bb4b44385b11fa9fe66edd7449278f9e47a6cc679b7111f9adf17db94c34e29c9cceafc917454e134420db40b24b56da29226af6f43e6dbeff822b79b77ed60
2022-10-17test: Add functional tests for sendtxrcncl message from outboundGleb Naumenko
2022-10-17test: Add functional tests for sendtxrcncl from inboundGleb Naumenko
2022-10-17test: Add unit tests for reconciliation negotiationGleb Naumenko
2022-10-17p2p: clear txreconciliation state for non-wtxid peersGleb Naumenko
We optimistically pre-register a peer for txreconciliations upon sending txreconciliation support announcement. But if, at VERACK, we realize that the peer never sent WTXIDRELAY message, we should unregister the peer from txreconciliations, because txreconciliations rely on wtxids.
2022-10-17p2p: Finish negotiating reconciliation supportGleb Naumenko
Once we received a reconciliation announcement support message from a peer and it doesn't violate our protocol, we store the negotiated parameters which will be used for future reconciliations.
2022-10-17Add helper to see if a peer is registered for reconciliationsGleb Naumenko
2022-10-17p2p: Forget peer's reconciliation state on disconnectGleb Naumenko
2022-10-17p2p: Announce reconciliation supportGleb Naumenko
If we're connecting to the peer which might support transaction reconciliation, we announce we want to reconcile with them. We store the reconciliation salt so that when the peer responds with their salt, we are able to compute the full reconciliation salt. This behavior is enabled with a CLI flag.
2022-10-17log: Add tx reconciliation log categoryGleb Naumenko
2022-10-15Merge bitcoin/bitcoin#26319: add 0xb10c builder keyAndrew Chow
5165ae1405b71106ce5f5efca99904ba7d2a9771 add 0xb10c builder key (0xb10c) Pull request description: I've been asked to add my key given my [activity as GUIX builder](https://github.com/bitcoin-core/guix.sigs/commits?author=0xB10C). ACKs for top commit: achow101: ACK 5165ae1405b71106ce5f5efca99904ba7d2a9771 1440000bytes: ACK https://github.com/bitcoin/bitcoin/pull/26319/commits/5165ae1405b71106ce5f5efca99904ba7d2a9771 hebasto: ACK 5165ae1405b71106ce5f5efca99904ba7d2a9771, the added fingerprint is the same as one in my local gpg output. Tree-SHA512: 794b01c87dec5139cd9dd3a1ec7ca4dd21351b16b46a4ea64c3be0e569ff20a301cdfa45873663f446e0a59d6319950f77c32f776260cff63e176b81ed262be3
2022-10-15Adjust `.tx/config` for new Transifex CLIHennadii Stepanov
The old Transifex Command-Line Tool is considered deprecated (as of January 2022) and will sunset on Nov 30, 2022. See: https://github.com/transifex/cli/blob/devel/README.md#migrating-from-older-versions-of-the-client An accompanying PR: https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/142
2022-10-15add 0xb10c builder key0xb10c
2022-10-14doc: fix typo in doc/libraries.mdfanquake
2022-10-14doc: remove library commentary from src/Makefile.amfanquake
This duplicates and is less explanatory than doc/libraries.md.
2022-10-14Merge bitcoin/bitcoin#26307: test: use MiniWallet for rpc_scanblocks.pyMacroFake
ae3626ea52deb22010c0d57d9f992e1e1bc31ba8 test: use MiniWallet for rpc_scanblocks.py (Sebastian Falbesoner) Pull request description: This is a small follow-up for #23549 which introduced `scanblocks`. Since that RPC doesn't need the wallet, we can switch the functional test to use MiniWallet. ACKs for top commit: MarcoFalke: review ACK ae3626ea52deb22010c0d57d9f992e1e1bc31ba8 Tree-SHA512: e0b0088103e059b29719299c58fd5173b1cff58cb73025a9a33ad493cd0ac50ba25a5f790e471c00a09b4dca80f3c011174ca4e0c8f34746c39831f5823dc8ba
2022-10-14Merge bitcoin/bitcoin#26297: ci: Use all available CPUs for functional tests ↵fanquake
in "Win64 native" task 6fbd173d8a4519967d75d2707b2285d62faa4424 ci: Use all available CPUs for functional tests in "Win64 native" task (Hennadii Stepanov) Pull request description: On the [master](https://cirrus-ci.com/task/5422842484359168) branch: ![Screenshot from 2022-10-12 09-45-58](https://user-images.githubusercontent.com/32963518/195296883-3852ea09-7345-4166-b855-7704dcd87202.png) This [PR](https://cirrus-ci.com/task/6392972617973760) branch: ![Screenshot from 2022-10-12 11-11-15](https://user-images.githubusercontent.com/32963518/195315902-f667874a-8aeb-4f2f-bdc3-5ba432ae9353.png) Also consider "CPU Usage" charts provided by CI. Overlooked in cda62657e95a90a5fd61ba43e2acbd407e3a4135 (bitcoin/bitcoin#25929). ACKs for top commit: hebasto: Indeed. Reverted back to 6fbd173d8a4519967d75d2707b2285d62faa4424 ([pr26297.01](https://github.com/hebasto/bitcoin/commits/pr26297.01)), which was already [ACKed](https://github.com/bitcoin/bitcoin/pull/26297#pullrequestreview-1138724890) by @aureleoules. aureleoules: ACK 6fbd173d8a4519967d75d2707b2285d62faa4424 jarolrod: ACK 6fbd173d8a4519967d75d2707b2285d62faa4424 shaavan: ACK 6fbd173d8a4519967d75d2707b2285d62faa4424 Tree-SHA512: ddd4b41af95bd735f881a3b2c64ee308de2725381f770e313e66555f929d88c8848c98cc5fcd15dfa6845b5dd84ca6c8764ef5d01602b0a62041820856af2b98
2022-10-13test: use MiniWallet for rpc_scanblocks.pySebastian Falbesoner
2022-10-13Merge bitcoin/bitcoin#26306: add lock annotation for FeeFilterRounder::round()Andrew Chow
cbb2da8fcf0c81453695c4ec8ce6c467e6a5c8ba add lock annotation for FeeFilterRounder::round() (glozow) Pull request description: CI failure from #24407: https://github.com/bitcoin/bitcoin/runs/8876014446 Calling `WITH_LOCK()` on a non-recursive mutex requires not holding it beforehand. ACKs for top commit: achow101: ACK cbb2da8fcf0c81453695c4ec8ce6c467e6a5c8ba dergoegge: ACK cbb2da8fcf0c81453695c4ec8ce6c467e6a5c8ba hebasto: ACK cbb2da8fcf0c81453695c4ec8ce6c467e6a5c8ba, tested on Ubuntu 22.04 with clang 14.0. Tree-SHA512: d6782ee48442b9d64d58a54c1ec7c53822ab051bf9728b44d6a0e05f1953e90f16420d349379345845db203fbad4e1f5750d9070adcb7daa18f12359a29488ca
2022-10-13add lock annotation for FeeFilterRounder::round()glozow
Calling WITH_LOCK() on a non-recursive mutex requires not holding it beforehand. Co-authored-by: Niklas Gögge <n.goeggi@gmail.com>
2022-10-13Merge bitcoin/bitcoin#24851: init: ignore BIP-30 verification in ↵Andrew Chow
DisconnectBlock for problematic blocks e899d4ca6f44785b6e5481e200a6a9a8d2448612 init: limit bip30 exceptions to coinbase txs (Chris Geihsler) 511eb7fdeac36da9d6576c878a1c8d390b38f1bd Ignore problematic blocks in DisconnectBlock (Chris Geihsler) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/22596 When using checklevel=4, block verification fails because of duplicate coinbase transactions involving blocks 91812 and 91722. There was already a check in place within `ConnectBlock` to ignore the problematic blocks, but `DisconnectBlock` did not contain a similar check to ignore these blocks when called from `VerifyDB`. By ignoring these two blocks in `DisconnectBlock`, the block verification process succeeds at checklevel=4. (Note to reviewers: this is my first contribution to Bitcoin Core, so any feedback is most welcome. Thanks in advance for reviewing!) ## Steps to reproduce: Use the following bitcoin.conf file and start bitcoind. I only used block data through block ~100000 so that the verification process was much faster. ``` assumevalid=0 checkblocks=0 checklevel=4 ``` Without this change, you will see the following error when the blocks are verified: ``` 2022-04-14T02:56:44Z init message: Verifying blocks… 2022-04-14T02:56:44Z Verifying last 101881 blocks at level 4 2022-04-14T02:56:44Z [0%]...[10%]...[20%]...[30%]...[40%]...ERROR: VerifyDB(): *** coin database inconsistencies found (last 10160 blocks, 142571 good transactions before that) 2022-04-14T02:57:01Z : Corrupted block database detected. Please restart with -reindex or -reindex-chainstate to recover. : Corrupted block database detected. Please restart with -reindex or -reindex-chainstate to recover. ``` With this change, you will see this instead: ``` 2022-04-14T02:32:29Z init message: Verifying blocks… 2022-04-14T02:32:29Z Verifying last 101746 blocks at level 4 2022-04-14T02:32:29Z [0%]...[10%]...[20%]...[30%]...[40%]...[50%]...[60%]...[70%]...[80%]...[90%]...[DONE]. 2022-04-14T02:32:48Z No coin database inconsistencies in last 101746 blocks (226126 transactions) ``` ACKs for top commit: laanwj: Code review ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 achow101: ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 jamesob: (Biased) ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 ([`jamesob/ackr/24851.2.seejee.init_ignore_bip_30_verif`](https://github.com/jamesob/bitcoin/tree/ackr/24851.2.seejee.init_ignore_bip_30_verif)) Tree-SHA512: d2f6d25e9619aee32c1a73fe846b1b587698eaa5a4994fa6424f1038f45654f9fd52b74a69843cc84d90168d74827130ccf8e9201502f5d52281acdb20429291
2022-10-13Merge bitcoin/bitcoin#25412: rest: add `/deploymentinfo` endpointAndrew Chow
a8250e30f16f2919ea5aa122b2880b076bd398a3 doc: add release note about `/rest/deploymentinfo` (brunoerg) 5c960200242d237f2cf74309b8fd29e8162682ed doc: add `/deploymentinfo` in REST-interface (brunoerg) 3e44bee08eb93e086179b92007649d47652aa439 test: add coverage for `/rest/deploymentinfo` (brunoerg) 91497031cbd74a0665b7fc31eb6b73bfb7bd0d40 rest: add `/deploymentinfo` (brunoerg) Pull request description: #23508 added a new RPC named `getdeploymentinfo`, it moved the softfork section from `getblockchaininfo` into this new one. In the REST interface, we have an endpoint named`/rest/chaininfo.json` (which refers to `getblockchaininfo`), so, this PR adds a new REST endpoint named `/deploymentinfo` which refers to `getdeploymentinfo`. You can use it by passing a block hash, e.g: '/rest/deploymentinfo/<BLOCKHASH>.json' or you can use it without passing a block hash to get the 'deploymentinfo' for the last block. ACKs for top commit: jonatack: re-ACK a8250e30f16f2919ea5aa122b2880b076bd398a3 rebase-only since my last review at c65f82bb achow101: ACK a8250e30f16f2919ea5aa122b2880b076bd398a3 stickies-v: re-ACK https://github.com/bitcoin/bitcoin/commit/a8250e30f16f2919ea5aa122b2880b076bd398a3 Tree-SHA512: 0735183b6828d51a72ed0e2be5a09b314ac4693f548982c6e9adaa0ef07a55aa428d3b2d1b1de70b83169811a663a8624b686166e5797f624dcc00178b9796e6
2022-10-13Merge bitcoin/bitcoin#24407: fees: make the class FeeFilterRounder thread-safeAndrew Chow
8173f160e085186c9bcc7f3506205c309ee66af6 style: rename variables to match coding style (Vasil Dimov) 8b4ad203d06c5ded6ecebbd7277b29a442d88bcf fees: make FeeFilterRounder::feeset const (Vasil Dimov) e7a5bf6be79e341e037305a4c2d8a1a510a8d709 fees: make the class FeeFilterRounder thread-safe (Vasil Dimov) Pull request description: Make the class `FeeFilterRounder` thread-safe so that its methods can be called concurrently by different threads on the same object. Currently it has just one method (`round()`). The second commit is optional, but it improves readability, showing that the `feeset` member will never be changed, thus does not need protection from concurrent access. ACKs for top commit: jonatack: re-ACK 8173f160e085186c9bcc7f3506205c309ee66af6 laanwj: Code review ACK 8173f160e085186c9bcc7f3506205c309ee66af6 promag: Code review ACK 8173f160e085186c9bcc7f3506205c309ee66af6 Tree-SHA512: 94b809997c485c0d114fa702d0406b980be8eaaebcfefa56808ed670aa943959c2f16cfd0ef72b4752fe2a409a23af1b4b7f2f236e51212957759569e3bbbefd
2022-10-13Merge bitcoin/bitcoin#24269: test: add functional test for `-discover`Andrew Chow
bff05bd7456d3634b0c83539293a753db6d76376 test: add functional test for -discover (brunoerg) Pull request description: This PR adds a functional test for `-discover`. It tests different scenarios where `localaddresses` should be empty or may contain the addresses. Obs: `localaddresses` is not always accurate, so it's not possible to ensure (100%) it will contain any addresses. https://github.com/bitcoin/bitcoin/blob/515200298b555845696a07ae2bc0a84a5dd02ae4/src/init.cpp#L449 Obs: See #24258 - It adds test coverage for this field but for nodes with proxy. ACKs for top commit: mzumsande: Code review ACK bff05bd7456d3634b0c83539293a753db6d76376 achow101: ACK bff05bd7456d3634b0c83539293a753db6d76376 rajarshimaitra: tACK bff05bd7456d3634b0c83539293a753db6d76376 Tree-SHA512: 8782497c146bce1ba86fda6146f3847465d7069f2cb6b84f2afc8f3b43efa813442bffe7447e9ce02adee304100b60365409bf0e5d875dfb880038442feec2a6
2022-10-13Merge bitcoin/bitcoin#25526: wallet: avoid double keypool TopUp() call on ↵Andrew Chow
descriptor wallets bfb9b94ebefdb95ac7656836975b3d5afc428744 wallet: remove duplicate descriptor type check in GetNewDestination (furszy) 76b982a4a5328c1357dbc5361317f682db160876 wallet: remove unused `nAccountingEntryNumber` field (furszy) 599ff5adfc7e1227c6d97d861d0715aee57611dd wallet: avoid double TopUp() calls on descriptor wallets (furszy) Pull request description: Found it while was digging over a `getnewaddress` timeout on the functional test suite. ### Context: We are calling `TopUp()` twice in the following flows for descriptor wallets: A) `CWallet::GetNewDestination`: 1) Calls spk_man->TopUp() 2) Calls spk_man->GetNewDestination() --> which, after the basic script checks, calls TopUp() again. B) `CWallet::GetReservedDestination`: 1) Calls spk_man->TopUp() 2) Calls spk_man->GetReservedDestination() --> which calls to GetNewDestination (which calls to TopUp again). ### Changes: Move `TopUp()` responsibility from the wallet class to each scriptpubkeyman. So each spkm can decide to call it or not after perform the basic checks for the new destination request. Aside from that, remove the unused `nAccountingEntryNumber` wallet field. And a duplicated descriptor type check in `GetNewDestination` ACKs for top commit: aureleoules: re-ACK bfb9b94ebefdb95ac7656836975b3d5afc428744. achow101: ACK bfb9b94ebefdb95ac7656836975b3d5afc428744 theStack: Code-review ACK bfb9b94ebefdb95ac7656836975b3d5afc428744 Tree-SHA512: 3ab73f37729e50d6c6a4434f676855bc1fb404619d63c03e5b06ce61c292c09c59d64cb1aa3bd9277b06f26988956991d62c90f9d835884f41ed500b43a12058
2022-10-13Merge bitcoin/bitcoin#26109: rpc, doc: getpeerinfo updatesAndrew Chow
a3789c700b5a43efd4b366b4241ae840d63f2349 Improve getpeerinfo pingtime, minping, and pingwait help docs (Jon Atack) df660ddb1cce1ee330346fe1728d868f41ad0256 Update getpeerinfo/-netinfo/TxRelay#m_relay_txs relaytxes docs (for v24 backport) (Jon Atack) 1f448542e79452a48f93f53ebbcb3b6df45aeef0 Always return getpeerinfo "minfeefilter" field (for v24 backport) (Jon Atack) 9cd6682545e845277d2207654250d1ed78d0c695 Make getpeerinfo field order consistent with its help (for v24 backport) (Jon Atack) Pull request description: Various updates and fixups, mostly targeting v24. Please refer to the commit messages for details. ACKs for top commit: achow101: ACK a3789c700b5a43efd4b366b4241ae840d63f2349 brunoerg: ACK a3789c700b5a43efd4b366b4241ae840d63f2349 vasild: ACK a3789c700b5a43efd4b366b4241ae840d63f2349 Tree-SHA512: b8586a9b83c1b18786b5ac1fc1dba91573c13225fc2cfc8d078f4220967c95056354f6be13327f33b4fcf3e9d5310fa4e1bdc93102cbd6574f956698993a54bf
2022-10-13Merge bitcoin/bitcoin#23549: Add scanblocks RPC call (attempt 2)Andrew Chow
626b7c8493ea1063f30ae4f62e1b36eb87adf685 fuzz: add scanblocks as safe for fuzzing (James O'Beirne) 94fe5453c7f8a86136dc9a6e0b370afd32564209 test: rpc: add scanblocks functional test (Jonas Schnelli) 6ef2566b68cb8570220c13df11c5cb5a5f4f7a4d rpc: add scanblocks - scan for relevant blocks with descriptors (Jonas Schnelli) a4258f6e81a476ce1687e2d58f7d2bf16162a172 rpc: move-only: consolidate blockchain scan args (James O'Beirne) Pull request description: Revives #20664. All feedback from the previous PR has either been responded to inline or incorporated here. --- Major changes from Jonas' PR: - consolidated arguments for scantxoutset/scanblocks - substantial cleanup of the functional test Here's the range-diff (`git range-diff master jonasschnelli/2020/12/filterblocks_rpc jamesob/2021-11-scanblocks`): https://gist.github.com/jamesob/aa4a975344209f0316444b8de2ec1d18 ### Original PR description > The `scanblocks` RPC call allows one to get relevant blockhashes from a set of descriptors by scanning all blockfilters in a given range. > > **Example:** > > `scanblocks start '["addr(<bitcoin_address>)"]' 661000` (returns relevant blockhashes for `<bitcoin_address>` from blockrange 661000->tip) > > ## Why is this useful? > **Fast wallet rescans**: get the relevant blocks and only rescan those via `rescanblockchain getblockheader(<hash>)[height] getblockheader(<hash>)[height])`. A future PR may add an option to allow to provide an array of blockhashes to `rescanblockchain`. > > **prune wallet rescans**: (_needs additional changes_): together with a call to fetch blocks from the p2p network if they have been pruned, it would allow to rescan wallets back to the genesis block in pruned mode (relevant #15946). > > **SPV mode** (_needs additional changes_): it would be possible to build the blockfilterindex from the p2p network (rather then deriving them from the blocks) and thus allow some sort of hybrid-SPV mode with moderate bandwidth consumption (related #9483) ACKs for top commit: furszy: diff re-ACK 626b7c8 Tree-SHA512: f84e4dcb851b122b39e9700c58fbc31e899cdcf9b587df9505eaf1f45578cc4253e89ce2a45d1ff21bd213e31ddeedbbcad2c80810f46755b30acc17b07e2873
2022-10-13Merge bitcoin/bitcoin#25667: assumeutxo: snapshot initializationAndrew Chow
bf9597606166323158bbf631137b82d41f39334f doc: add note about snapshot chainstate init (James O'Beirne) e4d799528696c5ede38c257afaffd367917e0de8 test: add testcases for snapshot initialization (James O'Beirne) cced4e7336d93a2dc88e4a61c49941887766bd72 test: move-only-ish: factor out LoadVerifyActivateChainstate() (James O'Beirne) 51fc9241c08a00f1f407f1534853a5cddbbc0a23 test: allow on-disk coins and block tree dbs in tests (James O'Beirne) 3c361391b8f5971eb3c7b620aa7ad9b437cc515e test: add reset_chainstate parameter for snapshot unittests (James O'Beirne) 00b357c215ed900145bd770525a341ba0ed9c027 validation: add ResetChainstates() (James O'Beirne) 3a29dfbfb2c16a50d854f6f81428a68aa9180509 move-only: test: make snapshot chainstate setup reusable (James O'Beirne) 8153bd9247dad3982d54488bcdb3960470315290 blockmanager: avoid undefined behavior during FlushBlockFile (James O'Beirne) ad67ff377c2b271cb4683da2fb25fd295557f731 validation: remove snapshot datadirs upon validation failure (James O'Beirne) 34d159033106cc595cfa852695610bfe419c989c add utilities for deleting on-disk leveldb data (James O'Beirne) 252abd1e8bc5cdf4368ad55e827a873240535b28 init: add utxo snapshot detection (James O'Beirne) f9f1735f139b6a1f1c7fea50717ff90dc4ba2bce validation: rename snapshot chainstate dir (James O'Beirne) d14bebf100aaaa25c7558eeed8b5c536da99885f db: add StoragePath to CDBWrapper/CCoinsViewDB (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606) --- Half of the replacement for #24232. The original PR grew larger than expected throughout the review process. This change adds the ability to initialize a snapshot-based chainstate during init if one is detected on disk. This is of course unused as of now (aside from in unittests) given that we haven't yet enabled actually loading snapshots. Don't be scared! There are some big move-only commits in here. Accompanying changes include: - moving the snapshot coinsdb directory from being called `chainstate_[base blockhash]` to `chainstate_snapshot`, since we only support one snapshot in use at a time. This simplifies some logic, but it necessitates writing that base blockhash out to a file within the coinsdb dir. See [discussion here](https://github.com/bitcoin/bitcoin/pull/24232#discussion_r832762880). - adding a simple fix in `FlushBlockFile()` that avoids a crash when attemping to flush to disk before `LoadBlockIndexDB()` is called, which happens when calling `MaybeRebalanceCaches()` during multiple chainstate init. - improving the unittest to allow testing with on-disk chainstates - necessary to test a simulated restart and re-initialization. ACKs for top commit: naumenkogs: utACK bf9597606166323158bbf631137b82d41f39334f ariard: Code Review ACK bf9597606 ryanofsky: Code review ACK bf9597606166323158bbf631137b82d41f39334f. Changes since last review: rebasing, switching from CAutoFile to AutoFile, adding comments, switching from BOOST_CHECK to Assert in test util, using chainman.GetMutex() in tests, destroying one ChainstateManager before creating a new one in tests fjahr: utACK bf9597606166323158bbf631137b82d41f39334f aureleoules: ACK bf9597606166323158bbf631137b82d41f39334f Tree-SHA512: 15ae75caf19f8d12a12d2647c52897904d27b265a7af6b4ae7b858592eeadb8f9da6c2394b6baebec90adc28742c053e3eb506119577dae7c1e722ebb3b7bcc0
2022-10-13Merge bitcoin/bitcoin#25858: psbt: Only include PSBT_OUT_TAP_TREE when the ↵glozow
output has a script path 9e386afb67bf8fa71b72f730da1695eeb11828cd tests: Test that PSBT_OUT_TAP_TREE is included correctly (Andrew Chow) 30ff25cf37eec4b09ab40424eb5d6a4a80410955 psbt: Only include m_tap_tree if it has scripts (Andrew Chow) 0577d423adda8e719d7611d03355680c8fbacab8 psbt: Change m_tap_tree to store just the tuples (Andrew Chow) 22c051ca70bae73e0430b05fb9d879591df27699 tests: Test that PSBT_OUT_TAP_TREE is combined correctly (Andrew Chow) 7df6e1bb77a96eac4fbcba424bbe780636b86650 psbt: Fix merging of m_tap_tree (Andrew Chow) 0652dc53b291bd295caff4093ec2854fd4b34645 [BugFix]: Do not allow deserializing PSBT with empty PSBT_OUT_TAP_TREE (Jeremy Rubin) Pull request description: PSBT_OUT_TAP_TREE should not be included for outputs that do not have such a tree. This should be disallowed during parsing, as well as prior to serialization when the field is populated during updating. Also added some test cases. Alternative to #25856 ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/25858/commits/9e386afb67bf8fa71b72f730da1695eeb11828cd darosior: ACK 9e386afb67bf8fa71b72f730da1695eeb11828cd Tree-SHA512: ce5c02a69752d176dbd967c1e8d30129b1905c8f186aeeef034576c1de82059271a1ee846bd040f5be4e66bb77ba711dcf14ac1e597c5707d7e7e2293f6cfefb
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-06 10:57:09 +0000