aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-03-30Merge bitcoin/bitcoin#24118: Add 'sendall' RPC née sweepMarcoFalke
bb84b7145b31dbfdcb4cf0b9b6e612a57e573993 add tests for no recipient and using send_max while inputs are specified (ishaanam) 49090ec4025152c847be8a5ab6aa6f379e345260 Add sendall RPC née sweep (Murch) 902793c7772e5bdd5aae5b0d20a32c02a1a6dc7c Extract FinishTransaction from send() (Murch) 6d2208a3f6849a3732af6ff010eeea629b9b10d0 Extract interpretation of fee estimation arguments (Murch) a31d75e5fb5c1304445d698595079e29f3cd3a3a Elaborate error messages for outdated options (Murch) 35ed094e4b0e0554e609709f6ca1f7d17096882c Extract prevention of outdated option names (Murch) Pull request description: Add sendall RPC née sweep _Motivation_ Currently, the wallet uses a fSubtractFeeAmount (SFFO) flag on the recipients objects for all forms of sending calls. According to the commit discussion, this flag was chiefly introduced to permit sweeping without manually calculating the fees of transactions. However, the flag leads to unintuitive behavior and makes it more complicated to test many wallet RPCs exhaustively. We proposed to introduce a dedicated `sendall` RPC with the intention to cover this functionality. Since the proposal, it was discovered in further discussion that our proposed `sendall` rpc and SFFO have subtly different scopes of operation. • sendall: Use _given UTXOs_ to pay a destination the remainder after fees. • SFFO: Use a _given budget_ to pay an address the remainder after fees. While `sendall` will simplify cases of spending a given set of UTXOs such as paying the value from one or more specific UTXOs, emptying a wallet, or burning dust, we realized that there are some cases in which SFFO is used to pay other parties from a limited budget, which can often lead to the creation of change outputs. This cannot be easily replicated using `sendall` as it would require manual computation of the appropriate change amount. As such, sendall cannot replace all uses of SFFO, but it still has a different use case and will aid in simplifying some wallet calls and numerous wallet tests. _Sendall call details_ The proposed sendall call builds a transaction from a specific subset of the wallet's UTXO pool (by default all of them) and assigns the funds to one or more receivers. Receivers can either be specified with a given amount or receive an equal share of the remaining unassigned funds. At least one recipient must be provided without assigned amount to collect the remainder. The `sendall` call will never create change. The call has a `send_max` option that changes the default behavior of spending all UTXOs ("no UTXO left behind"), to maximizing the output amount of the transaction by skipping uneconomic UTXOs. The `send_max` option is incompatible with providing a specific set of inputs. --- Edit: Replaced OP with latest commit message to reflect my updated motivation of the proposal. ACKs for top commit: achow101: re-ACK bb84b7145b31dbfdcb4cf0b9b6e612a57e573993 Tree-SHA512: 20aaf75d268cb4b144f5d6437d33ec7b5f989256b3daeeb768ae1e7f39dc6b962af8223c5cb42ecc72dc38cecd921c53c077bc0ec300b994e902412213dd2cc3
2022-03-30Merge bitcoin/bitcoin#24690: util: Add inotify_rm_watch to syscall sandbox ↵fanquake
(AllowFileSystem) f05a4cdf5a0363e1c12f00c034afb60e7ea0c775 util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem) (Hennadii Stepanov) Pull request description: This PR fixes the current master (3297f5c11c72dd83479ff8335e047555e3f8cb3b) when running `bitcoin-qt` on Ubuntu 22.04 and quitting: ``` $ ./src/qt/bitcoin-qt -signet -sandbox=log-and-abort Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway. ERROR: The syscall "inotify_rm_watch" (syscall number 255) is not allowed by the syscall sandbox in thread "main". Please report. terminate called without an active exception Aborted (core dumped) ``` Also see https://github.com/bitcoin/bitcoin/pull/24659#discussion_r835747166 ACKs for top commit: fanquake: ACK f05a4cdf5a0363e1c12f00c034afb60e7ea0c775 - checked that qt is using this in it's filesystem watcher code. Tree-SHA512: 9c7920a25422cd3a040bc1cbc487c12c3dc2b91358c3757f1030d6a1ff12c18c688a8e5b7466f683da88a5e4f5f15d442975660022d706e47021253c24c58f4a
2022-03-30Merge bitcoin/bitcoin#24704: compat: remove strnlen back-compat codeMarcoFalke
d4ba2b2cbc3d1ef381fbdbae88cb5b18ca53f678 compat: remove strnlen back-compat code (fanquake) Pull request description: This was needed for mingw (not mingw-w64), and some older versions of macOS, which we no-longer support. ACKs for top commit: hebasto: ACK d4ba2b2cbc3d1ef381fbdbae88cb5b18ca53f678 Tree-SHA512: d1beb9df58464feea3076091361d7d46e4a8901e347644a5fa6f24e052ca24ee0c7c0dd3f2a3d682b0204bf50430fa89eac62121691ea08af6dcf6b907bdec87
2022-03-30Merge bitcoin/bitcoin#24692: refactoring: [Net Processing] Follow-ups to #21160fanquake
a40978dcbd6608695bc7f5191c4d0a3e48cbca0b [fuzz] Assert that Peer.m_tx_relay.m_relay_txs has been set correctly (John Newbery) 0bca5f2b465616d7ac915ddcc1acd9a021e40abb [net processing] PushNodeVersion() takes a const Peer& (John Newbery) 21154ff9270e4bbae02d9012a4a73cce86a00334 net_processing: move CNode data access out of lock (John Newbery) Pull request description: #21160 ([net/net processing]: Move tx inventory into net_processing) had some unaddressed review comments when it was merged. This branch addresses those comments. ACKs for top commit: MarcoFalke: review ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b dergoegge: ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b ajtowns: ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b Tree-SHA512: 46624e275f918c5f32d0adab0766e9b3ef8ebdbc74a3c8886d8a2e2ff1079029dcc371b40ef0d787609e9c05219b7456f3e2dfe4fb0cb7bf23ef966769aef1a1
2022-03-29Add sendall RPC née sweepMurch
_Motivation_ Currently, the wallet uses a fSubtractFeeAmount (SFFO) flag on the recipients objects for all forms of sending calls. According to the commit discussion, this flag was chiefly introduced to permit sweeping without manually calculating the fees of transactions. However, the flag leads to unintuitive behavior and makes it more complicated to test many wallet RPCs exhaustively. We proposed to introduce a dedicated `sendall` RPC with the intention to cover this functionality. Since the proposal, it was discovered in further discussion that our proposed `sendall` rpc and SFFO have subtly different scopes of operation. • sendall: Use _specific UTXOs_ to pay a destination the remainder after fees. • SFFO: Use a _specific budget_ to pay an address the remainder after fees. While `sendall` will simplify cases of spending from specific UTXOs, emptying a wallet, or burning dust, we realized that there are some cases in which SFFO is used to pay other parties from a limited budget, which can often lead to the creation of change outputs. This cannot be easily replicated using `sendall` as it would require manual computation of the appropriate change amount. As such, sendall cannot replace all uses of SFFO, but it still has a different use case and will aid in simplifying some wallet calls and numerous wallet tests. _Sendall call details_ The proposed sendall call builds a transaction from a specific subset of the wallet's UTXO pool (by default all of them) and assigns the funds to one or more receivers. Receivers can either be specified with a specific amount or receive an equal share of the remaining unassigned funds. At least one recipient must be provided without assigned amount to collect the remainder. The `sendall` call will never create change. The call has a `send_max` option that changes the default behavior of spending all UTXOs ("no UTXO left behind"), to maximizing the output amount of the transaction by skipping uneconomic UTXOs. The `send_max` option is incompatible with providing a specific set of inputs.
2022-03-29[fuzz] Assert that Peer.m_tx_relay.m_relay_txs has been set correctlyJohn Newbery
2022-03-29[net processing] PushNodeVersion() takes a const Peer&John Newbery
The peer object is not mutated by PushNodeVersion, so pass a const reference
2022-03-29Merge bitcoin/bitcoin#24523: build: Fix Boost.Process test for Boost 1.78laanwj
532c64a7264dd3c7329e8839547837c57da7dbe8 build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov) Pull request description: Rebased #24415 with Luke's suggestion. Fixes #24413. ACKs for top commit: hebasto: ACK 532c64a7264dd3c7329e8839547837c57da7dbe8, tested on Mac mini (M1, 2020) + macOS Monterey 12.3 (21E230). Tree-SHA512: 74f779695f6bbc45a2b7341a1402f747cc0d433d74825c7196cb9f156db0c0299895365f01665bd0bff12a8ebb5ea33a29b9a52f5eac0007ec35d1dca6544705
2022-03-29compat: remove strnlen back-compat codefanquake
This was needed for mingw (not mingw-w64), and some older versions of macOS, which we no-longer support.
2022-03-28Merge bitcoin/bitcoin#24677: refactor: fix wallet and related named argsMarcoFalke
21db4eb3ff481334e0fdf4724f853556f6a0028f test: fix incorrect named args in wallet tests (fanquake) 8b0e776718f32cecfd2898d003510969276507a6 test: fix incorrect named args in coin_selection tests (fanquake) 6fc00f7331f2f926167de107a30b31e895fa26f5 bench: fix incorrect named args in coin_selection bench (fanquake) Pull request description: Should be one of the last changes split from #24661. Motivation: > Incorrect named args are source of bugs, like https://github.com/bitcoin/bitcoin/pull/22979. > To allow them being checked by clang-tidy, use a format it can understand. ACKs for top commit: MarcoFalke: Concept ACK 21db4eb3ff481334e0fdf4724f853556f6a0028f Tree-SHA512: c29743a70f6118cf73dc37b56b30f45da55b7d7b3b8ed36859ad59f602c3e6692eb755e05d9a4dd17f05085bcd6cb5b8c4007090a76e4fbfb053f925322cf985
2022-03-28Merge bitcoin/bitcoin#23083: rpc: Fail to return undocumented or ↵fanquake
misdocumented JSON fc892c3a80091fbeaa2b5a6ec5bdaa31359b42de rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke) f4bc4a705addea3e60c3b69437913e6571df275d rpc: Add m_skip_type_check to RPCResult (MarcoFalke) Pull request description: This avoids documentation shortcomings such as the ones fixed in commit e7b6272b305386a264adf2c04b7bebfb8499070f, 138d55e6a0241f126916fde6ac9177c7e2a119c4, 577bd51a4b8de066466a445192c1c653872657e2, f8c84e047c61200fae4cc1d85688e113bf270409, 0ee9a00f90e81a6978b30bdb250a37cbfa6da022, 13f41855c5fedf11d4a2525f2f0dd214533e5e62, or faecb2ee0a0ad3eb8303cfc84a87dc02ec553c43 ACKs for top commit: fanquake: ACK fc892c3a80091fbeaa2b5a6ec5bdaa31359b42de - tested that this catches issue, i.e #24691: Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84
2022-03-28doc: Fix getpeerinfo docMarcoFalke
2022-03-28net_processing: move CNode data access out of lockJohn Newbery
CNode::m_relays_tx and CNode::m_bloom_filter_loaded access don't require the Peer::TxRelay::m_bloom_filter_mutex lock, so move them out of the lock scope. See https://github.com/bitcoin/bitcoin/pull/21160#discussion_r736785417 and https://github.com/bitcoin/bitcoin/pull/21160#discussion_r736785662.
2022-03-28util: Add inotify_rm_watch to syscall sandbox (AllowFileSystem)Hennadii Stepanov
2022-03-28Merge bitcoin/bitcoin#24659: util: add linkat to syscall sandbox ↵MarcoFalke
(AllowFileSystem) 9809db3577f0fa618bea42635b1581e628a30395 util: add linkat to syscall sandbox (AllowFileSystem) (fanquake) Pull request description: Should fix #24536. ACKs for top commit: MarcoFalke: cr ACK 9809db3577f0fa618bea42635b1581e628a30395 Rspigler: Tested ACK (commit 9809db3577f0fa618bea42635b1581e628a30395) - this fixes https://github.com/bitcoin/bitcoin/issues/24536 Tree-SHA512: 2642f7dfa806e166fb32639a29b509b2edc8b919516c1f12430fc96f9887952395e157d71ef99fbaef8f7bcce1920530c24ecbce605b8a374b05d586f1f22a24
2022-03-28Merge bitcoin/bitcoin#24656: refactor: Move mempool RPCs to rpc/mempoolMarcoFalke
fac5a51c47fba21678fb35805e40d00fe7c891a0 Move mempool RPCs to rpc/mempool (MarcoFalke) fa0f666dd7b55a5a0250c5e35d2c3dfd565463b7 style: Add static keyword where possible in rpc/mempool (MarcoFalke) Pull request description: This moves the remaining mempool RPCs to `rpc/mempool`. Previously all mempool RPCs from the `blockchain` category have been moved. This patch moves the ones from the `rawtransactions` category. In the future, as a follow-up to this refactoring patch, it could be considered whether a new `mempool` category should be introduced. Beside a clearer code organization, this pull request should also reduce the compile time and space of the `rawtransactions.cpp` file. ACKs for top commit: promag: Code review ACK fac5a51c47fba21678fb35805e40d00fe7c891a0. Tree-SHA512: 5578b894b68d0595869a9b03ed8dceebe3366f73dec5f090ccc36ff4002b1bc4d58af77546c2d71537c1be03694d9a28c4b1bfbb3569560997879293c5c0301e
2022-03-25test: fix incorrect named args in wallet testsfanquake
2022-03-25test: fix incorrect named args in coin_selection testsfanquake
2022-03-25bench: fix incorrect named args in coin_selection benchfanquake
2022-03-25Merge bitcoin/bitcoin#24665: doc: document clang tidy named argsfanquake
7e22d80af333b202939bcb2631082006c097bf22 addrman: fix incorrect named args (fanquake) 67f654ef612c8dbefb969e6e67c286ea2c2e82d6 doc: Document clang-tidy in dev notes (MarcoFalke) Pull request description: The documentation, and a single commit extracted from #24661. Motivation: > Incorrect named args are source of bugs, like https://github.com/bitcoin/bitcoin/pull/22979. > To allow them being checked by clang-tidy, use a format it can understand. ACKs for top commit: glozow: ACK 7e22d80af333b202939bcb2631082006c097bf22 Tree-SHA512: 4037fcea59fdf583b171bce7ad350299fe5f9feb3c398413432168f3b9a185e51884d5b30e4b4ab9c6c5bb896c178cfaee1d78d5b4f0034cd70121c9ea4184b7
2022-03-25Merge bitcoin/bitcoin#24494: wallet: generate random change target for each ↵fanquake
tx for better privacy 9053f64fcbd26d87c26ae6b982d17756a6ea0896 [doc] release notes for random change target (glozow) 46f2fed6c5e0fa623bfeabf61ba4811d5cf8f47c [wallet] remove MIN_CHANGE (glozow) a44236addd01cff4e4d751e0f379d399fbfc8eae [wallet] randomly generate change targets (glozow) 1e52e6bd0a8888efb4ed247d74ec7ca9dfc2e002 refactor coin selection for parameterizable change target (glozow) Pull request description: Closes #24458 - the wallet always chooses 1 million sats as its change target, making it easier to fingerprint transactions created by the Core wallet. Instead of using a fixed value, choose one randomly each time (within a range). Using 50ksat (around $20) as the lower bound and `min(1 million sat, 2 * average payment value)` as the upper bound. RFC: If the payment is <25ksat, this doesn't work, so we're using the range (payment amount, 50ksat) instead. ACKs for top commit: achow101: ACK 9053f64fcbd26d87c26ae6b982d17756a6ea0896 Xekyo: reACK 9053f64fcbd26d87c26ae6b982d17756a6ea0896 Tree-SHA512: 45ce5d064697065549473347648e29935733f3deffc71a6ab995449431f60302d1f9911a0994dfdb960b48c48b5d8859f168b396ff2a62db67d535a7db041d35
2022-03-25Merge bitcoin/bitcoin#24502: wallet: don't create long chains by defaultMarcoFalke
da2bc865d644f6be748c305556bdd02f02d1b161 [wallet] don't create long chains by default (glozow) Pull request description: Default mempool policy doesn't let you have chains longer than 25 transactions. This is locally configurable of course, but it's not really safe to assume that a chain longer than 25 transactions will propagate. Thus, the wallet should probably avoid creating such transactions by default; set `DEFAULT_WALLET_REJECT_LONG_CHAINS` to true. Closes #9752 Closes #10004 ACKs for top commit: MarcoFalke: re-ACK da2bc865d644f6be748c305556bdd02f02d1b161 only change is fixing typos in tests 🎏 Tree-SHA512: 65d8e4ec437fe928adf554aa7e819a52e0599b403d5310895f4e371e99bbc838219b3097c4d2f775bc870ac617ef6b4227b94291f2b376f824f14e8f2b152f31
2022-03-25[wallet] don't create long chains by defaultglozow
2022-03-25Merge bitcoin/bitcoin#24674: refactor: remove unused boost include in ↵MarcoFalke
bitcoin-util.cpp 3bb96274632cc914e9fe7a97f5e029bd29187db5 refactor: remove unused boost header include in bitcoin-util.cpp (Sebastian Falbesoner) Pull request description: This header was included since the introduction of bitcoin-util in commit 13762bcc9618138dd28b53c2031defdc9d762d26, but boost was actually never used (see `git log -S boost ./src/bitcoin-util.cpp`). Cherry-picked out of #22953, which currently needs rebase. This commit could just be merged on its own. ACKs for top commit: MarcoFalke: review ACK 3bb96274632cc914e9fe7a97f5e029bd29187db5 Tree-SHA512: 201ee1aa4d49074056654203db73a473479c2b92c49df8dbf8e35979f85178013c66540a665f0f6dc0a2efef88eb091e2b088bebff85d840033dffd8ae719349
2022-03-25Merge bitcoin/bitcoin#24666: refactor: Fix coinselection.h include, Make ↵fanquake
COutput a struct fab287cedde85b21622d767d3ece65291e18b0bf Clarify that COutput is a struct, not a class (MarcoFalke) fa61cdf464381dddd9da076b1a1cab95ff5b3baf wallet: Fix coinselection include (MarcoFalke) Pull request description: * Fix include (see commit message) * `{}`-init, see https://github.com/bitcoin/bitcoin/pull/24091#discussion_r831193284 * `struct`, see https://github.com/bitcoin/bitcoin/pull/24091#discussion_r831192702 ACKs for top commit: theStack: Code-review ACK fab287cedde85b21622d767d3ece65291e18b0bf Tree-SHA512: dd2cfb9c06a92295dbd8fbb6d56afcf00ebda2a0440e301d392cd183d1b9cd87626311d539e302a9e6c6521d69d6183c74a51934e3fc16e64a5dcaba60c7e3ce
2022-03-25refactor: remove unused boost header include in bitcoin-util.cppSebastian Falbesoner
This header was included since the introduction of bitcoin-util in commit 13762bcc9618138dd28b53c2031defdc9d762d26, but boost was actually never used (see `git log -S boost ./src/bitcoin-util.cpp`).
2022-03-25Merge bitcoin/bitcoin#24672: init: add missing cs_main lockMarcoFalke
0346c26fcacca8abcf67f7320fd441e564aa97d1 init: add missing cs_main lock (Anthony Towns) Pull request description: `BlockManager::m_block_tree_db` is protected by `cs_main`, so take the `cs_main` lock while accessing it. ACKs for top commit: jonatack: Code review ACK 0346c26fcacca8abcf67f7320fd441e564aa97d1 Tree-SHA512: d6dff0b2d58871c7fbb281558b59fa9ad26fa75b3ceca9232277fc49ab795325e5ac3d266db49e7bda33da6de0b014b1bdebdf2c2c4347d43e50c0433a2cf06c
2022-03-25Extract FinishTransaction from send()Murch
The final step of send either produces a PSBT or the final transaction. We extract these steps to a new helper function `FinishTransaction()` to reuse them in `sendall`.
2022-03-25Extract interpretation of fee estimation argumentsMurch
This will be reused in `sendall`, so we extract a method to prevent duplication.
2022-03-25Elaborate error messages for outdated optionsMurch
2022-03-25Extract prevention of outdated option namesMurch
This will be reused in `sendall` so we extract it to avoid duplication.
2022-03-25Merge bitcoin/bitcoin#21160: net/net processing: Move tx inventory into ↵fanquake
net_processing 1066d10f71e6800c78012d789ff6ae19df0243fe scripted-diff: rename TxRelay members (John Newbery) 575bbd0dea6d12510fdf3220d0f0e47d969da6e9 [net processing] Move tx relay data to Peer (John Newbery) 785f55f7eeab0dedbeb8e0d0b459f3bdc538b621 [net processing] Move m_wtxid_relay to Peer (John Newbery) 36346703f8558d6781c079c29ddece5a97477beb [net] Add CNode.m_relays_txs and CNode.m_bloom_filter_loaded (John Newbery) Pull request description: This continues the work of moving application layer data into net_processing, by moving all tx data into the new Peer object added in #19607. For motivation, see #19398. ACKs for top commit: dergoegge: ACK 1066d10f71e6800c78012d789ff6ae19df0243fe - This is a good layer separation improvement with no behavior changes. glozow: utACK 1066d10f71e6800c78012d789ff6ae19df0243fe Tree-SHA512: 0c9d6b8a0a05e2d816b6d6588b7df133842ec960ae67667813422aa7bd8eb5308599c714f3822a98ddbdf364ffab9050b055079277ba4aff24092557ff99ebcc
2022-03-26init: add missing cs_main lockAnthony Towns
BlockManager::m_block_tree_db is protected by cs_main, so take the cs_main lock while accessing it.
2022-03-25Merge bitcoin/bitcoin#23536: Enforce Taproot script flags whenever WITNESS ↵laanwj
is set cccc1e70b8a14430cc94143da97936a60d6c83d3 Enforce Taproot script flags whenever WITNESS is set (MarcoFalke) fa422994116a7a053789304d56159760081479eb Remove nullptr check in GetBlockScriptFlags (MarcoFalke) faadc606c7644f2934de390e261d9d65a81a7592 refactor: Pass const reference instead of pointer to GetBlockScriptFlags (MarcoFalke) Pull request description: Now that Taproot is active, it makes sense to enforce its rules on all blocks, even historic ones, regardless of the deployment status. ### Benefits: (With "script flags" I mean "taproot script verification flags".) * Script flags are known ahead for all blocks (even blocks not yet created) and do not change. This may benefit static analysis, code review, and development of new script features that build on Taproot. * Any future bugs introduced in the deployment code won't have any effect on the script flags, as they are independent of deployment. * Enforcing the taproot rules regardless of the deployment status makes testing easier because invalid blocks after activation are also invalid before activation. So there is no need to differentiate the two cases. * It gives belt-and-suspenders protection against a practically expensive and theoretically impossible IBD reorg attack where the node is eclipsed. While `nMinimumChainWork` already protects against this, the cost for a few months worth of POW might be lowered until a major version release of Bitcoin Core reaches EOL. The needed work for the attack is the difference between `nMinimumChainWork` and the work at block 709632. For reference, previously the same was done for P2SH and WITNESS in commit 0a8b7b4b33c9d78574627fc606267e2d8955cd1c. ### Implementation: I found one block which fails verification with the flags applied, so I added a `TaprootException`, similar to the `BIP16Exception`. For reference, the debug log: ``` ERROR: ConnectBlock(): CheckInputScripts on b10c007c60e14f9d087e0291d4d0c7869697c6681d979c6639dbd960792b4d41 failed with non-mandatory-script-verify-flag (Witness program was passed an empty witness) BlockChecked: block hash=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad state=non-mandatory-script-verify-flag (Witness program was passed an empty witness) InvalidChainFound: invalid block=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad height=692261 log2_work=92.988459 date=2021-07-23T08:24:20Z InvalidChainFound: current best=0000000000000000000067b17a4c0ffd77c29941b15ad356ca8f980af137a25d height=692260 log2_work=92.988450 date=2021-07-23T07:47:31Z ERROR: ConnectTip: ConnectBlock 0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad failed, non-mandatory-script-verify-flag (Witness program was passed an empty witness) ``` Hint for testing, make sure to set `-noassumevalid`. ### Considerations Obviously this change can lead to consensus splits on the network in light of massive reorgs. Currently the last block before Taproot activation, that is the last block without the Taproot script flags set, is only buried by a few days of POW. However, when and if this patch is included in the next major release, it will be buried by a few months of POW. BIP90 considerations apply when looking at reorgs this large. ACKs for top commit: Sjors: tACK cccc1e70b8a14430cc94143da97936a60d6c83d3 achow101: ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 laanwj: Code review ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ajtowns: ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ; code review; wrote a "getblockscriptflags" rpc to quickly check that blocks just had bit 17 (taproot) added; review of earlier revisions had established non-exception blocks do validate with taproot rules enabled. jamesob: ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ([`jamesob/ackr/23536.1.MarcoFalke.enforce_taproot_script_f`](https://github.com/jamesob/bitcoin/tree/ackr/23536.1.MarcoFalke.enforce_taproot_script_f)) Tree-SHA512: 00044de68939caef6420ffd588c1291c041a8b397c80a3df1e3e3487fbeae1821d23975c51c95e44e774558db76f943b00b4e27cbd0213f64a9253116dc6edde
2022-03-25Merge bitcoin/bitcoin#19385: test: Change default test logging directoryMarcoFalke
f8cba0d9117fe9b9ac51d7044372b28270c7838b test: Change default test logging directory (Yancy Ribbens) Pull request description: This PR changes the default test log location request here: https://github.com/bitcoin/bitcoin/issues/17224. Instead of using the location of the makefile [automatic variable](https://www.gnu.org/software/make/manual/make.html#Automatic-Variables) `$<` I extract just the basename and then prepend a new location `./test`. This is done because `$<` represents the variable name AND location of the prerequisite here. Top commit has no ACKs. Tree-SHA512: f0fbc530cf0e14c284b4bbf6671c145b1d7a2e1f5561f5c5d09f0cbe88b98e620e763bbbf2dfa9aeeec3dcc9b0127939e105e14c7e4f6660c7c19663622a393d
2022-03-25[wallet] remove MIN_CHANGEglozow
2022-03-25[wallet] randomly generate change targetsglozow
If the wallet always chooses 1 million sats as its change target, it is easier to fingerprint transactions created by the Core wallet.
2022-03-25refactor coin selection for parameterizable change targetglozow
no behavior changes, since the target is always MIN_CHANGE
2022-03-25Clarify that COutput is a struct, not a classMarcoFalke
Also, use {}-initialization
2022-03-25wallet: Fix coinselection includeMarcoFalke
coinselection.h is not used by wallet.h but by qt/coincontroldialog.cpp
2022-03-25addrman: fix incorrect named argsfanquake
2022-03-25Merge bitcoin/bitcoin#24663: doc, init: add links to doc/cjdns.mdMarcoFalke
daae28885fd3b559cb9273f539d15fca5549cd36 doc, init: add links to doc/cjdns.md (Jon Atack) Pull request description: Follow-up to #24555. ACKs for top commit: jessebarton: ACK [daae288](https://github.com/bitcoin/bitcoin/pull/24663/commits/daae28885fd3b559cb9273f539d15fca5549cd36) Tree-SHA512: eb8f4324f182f7917ddafa9b88ad753fe8f890c1c883c1342768ed9eac998c422ecd9e998fc977e72e26bc87f6aed295940b522187190481889255c8b2e05311
2022-03-24Merge bitcoin/bitcoin#24091: wallet: Consolidate CInputCoin and COutputfanquake
049003fe68a4183f6f20da16f58f10079d1e02df coinselection: Remove COutput operators == and != (Andrew Chow) f6c39c6adb6cbf9c87f04d3d667701905ef5c0a0 coinselection: Remove CInputCoin (Andrew Chow) 70f31f1a81710aa59e95770de9a84bf58cbce1e8 coinselection: Use COutput instead of CInputCoin (Andrew Chow) 14fbb57b79c664090f6a4e60d7bdfc9759ff4307 coinselection: Add effective value and fees to COutput (Andrew Chow) f0821230b8de2eec21a869d1edf9e2b9f502de25 moveonly: move COutput to coinselection.h (Andrew Chow) 42e974e15c6deba1d9395a4da9341c9ebec6e8e5 wallet: Remove CWallet and CWalletTx from COutput's constructor (Andrew Chow) 14d04d5ad15ae56df56edee7ca9a202b52037889 wallet: Replace CWalletTx in COutput with COutPoint and CTxOut (Andrew Chow) 0ba4d1916e26e2a5d603edcdb7625463989d25b6 wallet: Provide input bytes to COutput (Andrew Chow) d51f27d3bb0d6e3ca55bcd23ce53e4fe413a9360 wallet: Store whether a COutput is from the wallet (Andrew Chow) b799814bbd53736b79495072f3c9e05989a465e8 wallet: Store tx time in COutput (Andrew Chow) 46022953ee2e8113167bafd1fd48a383a578b13c wallet: Remove use_max_sig default value (Andrew Chow) 10379f007fd2c18f4cd24d0a0783d6d929f45556 scripted-diff: Rename COutput member variables (Andrew Chow) c7c64db41e1718584aa2f30ff27f60ab0966de62 wallet: cleanup COutput constructor (Andrew Chow) Pull request description: While working on coin selection code, it occurred to me that `CInputCoin` is really a subset of `COutput` and the conversion of a `COutput` to a `CInputCoin` does not appear to be all that useful. So this PR adds fields that are present in `CInputCoin` to `COutput` and replaces the usage of `CInputCoin` with `COutput`. `COutput` is also moved to coinselection.h. As part of this move, the usage of `CWalletTx` is removed from `COutput`. It is instead replaced by storing a `COutPoint` and the `CTxOut` rather than the entire `CWalletTx` as coin selection does not really need the full `CWalletTx`. The `CWalletTx` was only used for figuring out whether the transaction containing the output was from the current wallet, and for the transaction's time. These are now parameters to `COutput`'s constructor. ACKs for top commit: ryanofsky: Code review ACK 049003fe68a4183f6f20da16f58f10079d1e02df, just adding comments and removing == operators since last review w0xlt: reACK 049003f Xekyo: reACK 049003fe68a4183f6f20da16f58f10079d1e02df Tree-SHA512: 048b4cd620a0415e1d9fe8597257ee4bc64656566e1d28a9bdd147d6d72dc87c3f34a3339fa9ab6acf42c388df7901fc4ee900ccaabc3de790ffad162b544c15
2022-03-24Merge bitcoin/bitcoin#24205: init, test: improve network reachability test ↵MarcoFalke
coverage and safety 58a14795b89a6bd812e0b71cb8b3088b8ab55c11 test: passing -onlynet=onion with -onion=0/-noonion raises expected init error (Jon Atack) 7000f66d367123d1de303fc15ce2ce60df379c11 test: passing -onlynet=onion without -proxy/-onion raises expected init error (Jon Atack) 8332e6e4cf45455fea0bf1f7527256cdb7bb1e6d test: passing invalid -onion raises expected init error (Jon Atack) d5edb087082a50e6f7d413c3b43fdf1e6a20d29b test: passing invalid -proxy raises expected init error (Jon Atack) bd57dcbaf2b5e5f50833912c894a1f1239ceb25b test: hoist proxy out of 2 network loops in feature_proxy.py (Jon Atack) afdf2de28296660fd0284453a241aece8494eea8 test: add CJDNS to LimitedAndReachable_Network unit tests (Jon Atack) 2b7a8180a94738c2fcb21232a2eca07a7b27656d net, init: assert each network reachability is true by default (Jon Atack) Pull request description: Adds missing network reachability test coverage and an assertion during init, noticed while reviewing #22834: - assert during init that each network reachability is true by default - add CJDNS to the `LimitedAndReachable_Network` unit tests - hoist proxy out of two network loops in feature_proxy.py - test that passing invalid `-proxy` raises expected init error - test that passing invalid `-onion` raises expected init error - test that passing `-onlynet=onion` without `-proxy` and `-onion` raises expected init error - test that passing `-onlynet=onion` with `-onion=0` and with `-noonion` raises expected init error ACKs for top commit: vasild: ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11 brunoerg: ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11 dongcarl: Code Review ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11 Tree-SHA512: bdee6dd0c12bb63591ce7c9321fe77b509ab1265123054e774adc38a187746dddafe1627cbe89e990bcc78b45e194bfef8dc782710d5b217e2e2106ab0158827
2022-03-24doc, init: add links to doc/cjdns.mdJon Atack
2022-03-24Merge bitcoin/bitcoin#24625: Replace struct update_fee_delta with lambdafanquake
fa84a49526fcf76e98b0b2f4d4b00b34a8dddf46 Use CAmount for fee delta and modified fee (MarcoFalke) fa8857c3f7863d5cfa9a9e62db7f95aad80ea48e Replace struct update_fee_delta with lambda (MarcoFalke) Pull request description: The same was done for another struct in e177fcab3831b6d259da5164cabedcc9e78f6957. Also, change type of feeDelta from int64_t to CAmount. ACKs for top commit: hebasto: re-ACK fa84a49526fcf76e98b0b2f4d4b00b34a8dddf46 promag: Code review ACK fa84a49526fcf76e98b0b2f4d4b00b34a8dddf46. Tree-SHA512: 2b9ee449d348b0f685793a35c6dd3c57ed97fdf707a89495a0518bb332f407303b48723e667351e96f2b698e0a2ade27095517a3accd926d4ec85e58d6fd441f
2022-03-24rpc: Fail to return undocumented or misdocumented JSONMarcoFalke
2022-03-24rpc: Add m_skip_type_check to RPCResultMarcoFalke
Used in the next commit.
2022-03-24Merge bitcoin/bitcoin#24626: init: disallow reindex-chainstate when pruningMarcoFalke
b2813980b81034ff9b40bd45080fa67dea475d39 init: disallow reindex-chainstate when pruning (Martin Zumsande) Pull request description: The combination of `-reindex-chainstate` and `-prune` currently makes the node stuck in an endless loop: - `LoadChainstate()` will wipe the existing chainstate (so we have no genesis block anymore). It won't clean up unusable block files by calling `CleanupBlockRevFiles()` as for full `-reindex`. - `ThreadImport()` has [logic](https://github.com/bitcoin/bitcoin/blob/91d12344b1e51809c1ef6b630b631a6da00267c3/src/node/blockstorage.cpp#L855) of reloading Genesis after reindexing. This is what makes full `-reindex` work with `-prune` but it's not executed for `-reindex-chainstate`. - Since we still don't have a genesis block, init will wait for it forever in an endless loop ([code](https://github.com/bitcoin/bitcoin/blob/91d12344b1e51809c1ef6b630b631a6da00267c3/src/init.cpp#L1630-L1640)). Fix this by disallowing `-reindex-chainstate` together with `-prune`. This is discouraged in the help for `-reindex-chainstate` anyway ("When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead.") but wasn't enforced. Fixes #24242 ACKs for top commit: MarcoFalke: cr ACK b2813980b81034ff9b40bd45080fa67dea475d39 Tree-SHA512: 7220842daaf9a4f972d82b13b81fdeac2833bf5e665c5b0f8eaf6a4bcd0725c8e97d19ec956ca4b730065a983475bb3a2732713d338f4caf8666ccbf63d4d988
2022-03-24Merge bitcoin/bitcoin#24169: build: Add --enable-c++20 optionfanquake
999982b06ce1d1280e5ce48f9253c6c536f41a12 build: Add --enable-c++20 option (MarcoFalke) fae679065e4ef0c6383bbdd1876aaed6c1e40104 Add CSerializedNetMsg::Copy() helper (MarcoFalke) fabb7c4ba629ecdea80a23674e2659d3d391565f Make fs.h C++20 compliant (MarcoFalke) fae2220f4e48934313389864d3d362f672627eb8 scheduler: Capture ‘this’ explicitly in lambda (MarcoFalke) Pull request description: This is for CI and devs only and doesn't change that C++17 is the standard we are currently using. The option `--enable-c++20` allows CI to check that the C++17 code in the repo is also valid C++20. (There are some cases where valid C++17 doesn't compile under C++20). Also, it allows developers to easily play with C++20 in the codebase. ACKs for top commit: ryanofsky: Code review ACK 999982b06ce1d1280e5ce48f9253c6c536f41a12. Since last review was rebased, and enum-conversion change was dropped, and CSerializedNetMsg copy workaround was added fanquake: utACK 999982b06ce1d1280e5ce48f9253c6c536f41a12 Tree-SHA512: afc95ba03ea2b937017fc8e2b1449379cd2b6f7093c430d2e344c665a00c51e402d6651cbcbd0be8118ea1e54c3a86e67d2021d19ba1d4da67168e9fcb6b6f83