[WIP] Merging bitcoin upstream changes from 27.x #172
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When two successive UTXOs differ in weight but match in effective value, we can skip the second if the first is not selected, because all input sets we can generate by swapping out a lighter UTXOs with a heavier UTXO of matching effective value would be strictly worse.
In situations where we have UTXO groups of various weight, we can CUT rather than SHIFT when we exceeded the max_weight or the best selection’s weight while the last step was equal to the minimum weight in the lookahead.
Initialize `best_selection_weight` as `max_weight` allows us to skip the separate `max_weight` check on every loop.
Given a lot of small amount UTXOs it is possible that the lookahead indicates sufficient funds, but any combination of them would push us beyond the current best_weight. We can estimate a lower bound for the minimal necessary weight to reach target from the maximal amount and minimal weight in the tail of the UTXO pool: if adding a number of hypothetical UTXOs of this maximum amount and minimum weight would not be able to beat `best_weight`, we can SHIFT to the omission branch, and CUT if the last selected UTXO is not heavier than the minimum weight of the remainder.
The goal of the function is to erase the wallet transactions that match the inputted hashes. There is no need to traverse the database, reading record by record, to then perform single entry removals for each of them. To ensure consistency and improve performance, this change-set removes all tx records within a single atomic db batch operation, as well as it cleans up code, improves error handling and simplifies the transactions removal process entirely. This optimizes the removal of watch-only transactions during the wallet migration process and the 'removeprunedfunds' RPC command.
-BEGIN VERIFY SCRIPT- sed -i 's/ZapSelectTx/RemoveTxs/g' $(git grep -l 'ZapSelectTx' ./src/wallet) -END VERIFY SCRIPT-
13161ec opt: Skip over barren combinations of tiny UTXOs (Murch) b7672c7 opt: Skip checking max_weight separately (Murch) 1edd2ba opt: Cut if last addition was minimal weight (Murch) 5248e2a opt: Skip heavier UTXOs with same effective value (Murch) 9124c73 opt: Tiebreak UTXOs by weight for CoinGrinder (Murch) 451be19 opt: Skip evaluation of equivalent input sets (Murch) 407b1e3 opt: Track remaining effective_value in lookahead (Murch) 5f84f3c opt: Skip branches with worse weight (Murch) d68bc74 fuzz: Test optimality of CoinGrinder (Murch) 67df6c6 fuzz: Add CoinGrinder fuzz target (Murch) 1502231 coinselection: Track whether CG completed (Murch) 7488acc test: Add coin_grinder_tests (Murch) 6cc9a46 coinselection: Add CoinGrinder algorithm (Murch) 89d0956 opt: Tie-break UTXO sort by waste for BnB (Murch) aaee658 doc: Document max_weight on BnB (Murch) Pull request description: ***Please refer to the [topic on Delving Bitcoin](https://delvingbitcoin.org/t/gutterguard-and-coingrinder-simulation-results/279) discussing Gutter Guard/Coingrinder simulation results.*** Adds a coin selection algorithm that minimizes the weight of the input set while creating change. Motivations --- - At high feerates, using unnecessary inputs can significantly increase the fees - Users are upset when fees are relatively large compared to the amount sent - Some users struggle to maintain a sufficient count of UTXOs in their wallet Approach --- So far, Bitcoin Core has used a balanced approach to coin selection, where it will generate multiple input set candidates using various coin selection algorithms and pick the least wasteful among their results, but not explicitly minimize the input set weight. Under some circumstances, we _do_ want to minimize the weight of the input set. Sometimes changeless solutions require many or heavy inputs, and there is not always a changeless solution for Branch and Bound to find in the first place. This can cause expensive transactions unnecessarily. Given a wallet with sufficient funds, `CoinGrinder` will pick the minimal-waste input set for a transaction with a change output. The current implementation only runs `CoinGrinder` at feerates over 3×long-term-feerate-estimate (by default 30 ṩ/vB), which may be a decent compromise between our goal to reduce costs for the users, but still permit transactions at lower feerates to naturally reduce the wallet’s UTXO pool to curb bloat. Trade-offs --- Simulations for my thesis on coin selection ([see Section 6.3.2.1 [PDF]](https://murch.one/erhardt2016coinselection.pdf)) suggest that minimizing the input set for all transactions tends to grind a wallet’s UTXO pool to dust (pun intended): an approach selecting inputs per coin-age-priority (in effect similar to “largest first selection”) on average produced a UTXO pool with 15× the UTXO count as Bitcoin Core’s Knapsack-based Coin Selection then (in 2016). Therefore, I do not recommend running `CoinGrinder` under all circumstances, but only at extreme feerates or when we have another good reason to minimize the input set for other reasons. In the long-term, we should introduce additional metrics to score different input set candidates, e.g. on basis of their privacy and wallet health impact, to pick from all our coin selection results, but until then, we may want to limit use of `CoinGrinder` in other ways. ACKs for top commit: achow101: ACK 13161ec sr-gi: ACK [13161ec](bitcoin@13161ec) sipa: ACK 13161ec Tree-SHA512: 895b08b2ebfd0b71127949b7dba27146a6d10700bf8590402b14f261e7b937f4e2e1b24ca46de440c35f19349043ed2eba4159dc2aa3edae57721384186dae40
29029df [doc] v3 signaling in mempool-replacements.md (glozow) e643ea7 [fuzz] v3 transactions and sigop-adjusted vsize (glozow) 1fd16b5 [functional test] v3 transaction submission (glozow) 27c8786 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke) 9a1fea5 [policy/validation] allow v3 transactions with certain restrictions (glozow) eb8d5a2 [policy] add v3 policy rules (glozow) 9a29d47 [rpc] return full string for package_msg and package-error (glozow) 158623b [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow) Pull request description: See bitcoin#27463 for overall package relay tracking. Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340 Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418 Rationale: - There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see bitcoin#28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2] - Switching to a cluster-based mempool (see bitcoin#27677 and bitcoin#28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution. V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2. Immediate benefits: - You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later. - Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction. This also enables some other cool things (again see bitcoin#27463 for overall roadmap): - Ephemeral Anchors - Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees. - We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use. - We can switch to a cluster-based mempool [5] (bitcoin#27677 bitcoin#28676), which removes CPFP carve out [6]. [1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html [2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward. [3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html [4]: Original PR bitcoin#25038 also contains a lot of the discussion [5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7 [6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12 ACKs for top commit: sdaftuar: ACK 29029df achow101: ACK 29029df instagibbs: ACK 29029df modulo that Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
…for BIP21 URIs ede5014 Modify command line help to show support for BIP21 URIs (Hernan Marino) Pull request description: While reviewing a different PR (see bitcoin-core/gui#742 ) **hebasto** suggested that the help for bitcoin-qt should be updated to reflect the fact that bitcoin-qt supports an optional BIP21 URI parameter. Since this reflects actual behaviour of bitcoin-qt and is independent of whether or not the other PR gets merged, I created this simple PR to fix the help message. ACKs for top commit: kristapsk: utACK ede5014 pablomartin4btc: lgtm, re ACK ede5014 hebasto: ACK ede5014. Tree-SHA512: c456297c486bc5cc65e0e092e7ba9d51b0bd7a584d4fabca7f7ca1f8e58cbcc66e96226539c689ed0f5e7f40da220bbc4ea30b90e31e1aeeb8867a385a90209c
…sabled if no wallet is selected b2e531e qt: update widgets availability on wallet selection (pablomartin4btc) Pull request description: This PR addresses an issue where, with no wallet selected, ticking on "Settings -> Mask values" checkbox twice enables the transaction tab when the checkbox is unticked. <details> <summary>Current behavior display on master</summary>  </details> <details> <summary>Correction display from this branch</summary>  </details> Note for maintaners: this PR should be backported to both 25.x and 26.x. --- Originally this PR was disabling the "Mask Values" checkbox when no wallet was selected but since a reviewer pointed out that a user might want to open a wallet already on "privacy mode" I rolled that change out. <details> <summary>Original correction display disabling "Mask Values" </summary>  </details> ACKs for top commit: alfonsoromanz: Tested ACK bitcoin-core/gui@b2e531e hebasto: ACK b2e531e, tested on Ubuntu 22.04. Tree-SHA512: 6be77ab4d5ec86267a9b0a289a4d8600bb67d279f7e0be65e47b608ec392fe705cf026e32f3c082d2f27449b697d1d9e6a1d110035900d7a804ba823c9f5dfd4
fa0ceae test: Fix utxo set hash serialisation signedness (MarcoFalke) Pull request description: It is unsigned in Bitcoin Core, so the tests should match it: https://github.com/bitcoin/bitcoin/blob/5b8990a1f3c49b0b02b7383c69e95320acbda13e/src/kernel/coinstats.cpp#L54 Large positive values for the block height are too difficult to hit in tests, but it still seems fine to fix this. The bug was introduced when the code was written in 6ccc8fc. (Lowercase `i` means signed, see https://docs.python.org/3/library/struct.html#format-characters) ACKs for top commit: epiccurious: Tested ACK fa0ceae. fjahr: utACK fa0ceae Tree-SHA512: ab4405c74fb191fff8520b456d3a800cd084d616bb9ddca27d56b8e5c8969bd537490f6e204c1870dbb09a3e130b03b22a27b6644252a024059c200bbd9004e7
…ter the user has touched it bee0ffb GUI/Intro: Never change the prune checkbox after the user has touched it (Luke Dashjr) 420a983 Bugfix: GUI/Intro: Disable GUI prune option if -prune is set, regardless of set value (Luke Dashjr) Pull request description: Re-PR from bitcoin#18729 Now includes a bugfix too (`-prune=2+` disabled the checkbox, but `-prune=0/1` did not; this behaviour is necessary since `-prune` overrides GUI settings) ACKs for top commit: hebasto: ACK bee0ffb, both commits are improvements of the current behaviour. Tested on Ubuntu 23.10. Tree-SHA512: 8eb7d90af37deb30fe226179db3bc9df8ab59e4f3218c8e447ed31fc9ddc81ac1a1629da63347518587a56a4c8558b05cf7ec474024c5f5dfc6d49d6ff0eb0cc
9d37886 gui: Update Node window title with chain type (pablomartin4btc) Pull request description: It fixes bitcoin#544. Enhance the Node window title by appending the chain type to it, except for the `mainnet`, mirroring the behavior in the main window.  There was also some [interest](bitcoin-core/gui#78 (comment)) on this while discussing network switching. ACKs for top commit: MarnixCroes: tACK 9d37886 hernanmarino: tACK 9d37886 BrandonOdiwuor: tested ACK 9d37886 alfonsoromanz: Tested ACK bitcoin-core/gui@9d37886 kristapsk: ACK 9d37886 hebasto: ACK 9d37886, tested on Ubuntu 23.10. Tree-SHA512: 8c34c4586bd59b1c522662e8aa0726dccc8f12e020f7a6a1af5200a29e5817e1c51e0f467c7923041fc41535ea093c3e0dd787befbbcc84d6b9f7ff0d969db04
Co-authored-by: ismaelsadeeq <[email protected]>
This changes the default behavior, individual tests can overwrite this option. As a result, it is possible to run the entire test suite with --v2transport, and all connections to the python p2p will then use it. Also adjust several tests that are already running with --v2transport in the test runner (although they actually made v1 connection before this change). This is done in the same commit so that there isn't an intermediate commit in which the CI fails.
…tempting unlock 517c7f9 gui: Check for private keys disabled before attempting unlock (Andrew Chow) Pull request description: Before trying to unlock a wallet, first check if it has private keys disabled. If so, there is no need to unlock. Note that such wallets are not expected to occur in typical usage. However bugs in previous versions allowed such wallets to be created, and so we need to handle them. Fixes bitcoin#772 For some additional context, see bitcoin#631 ACKs for top commit: hebasto: ACK 517c7f9, I have reviewed the code and it looks OK. BrandonOdiwuor: ACK 517c7f9 Tree-SHA512: c92aa34344d04667b70b059d2aa0a1da999cb7239cd1413f3009781aa82379f309ff9808d7dc91d385e2c8afe2abda3564568e2091ef833b1536ebfcf80f7c3c
9a3c5c8 scripted-diff: rename ZapSelectTx to RemoveTxs (furszy) 83b7628 wallet: batch and simplify ZapSelectTx process (furszy) 595d50a wallet: migration, remove extra NotifyTransactionChanged call (furszy) a2b071f wallet: ZapSelectTx, remove db rewrite code (furszy) Pull request description: Work decoupled from bitcoin#28574. Brother of bitcoin#28894. Includes two different, yet interconnected, performance and code improvements to the zap wallet transactions process. 1) As the goal of the `ZapSelectTx` function is to erase tx records that match any of the inputted hashes. There is no need to traverse the whole database record by record. We could just check if the tx exist, and remove it directly by calling `EraseTx()`. 2) Instead of performing single write operations per removed tx record, this PR batches them all within a single atomic db txn. Moreover, these changes will enable us to consolidate all individual write operations that take place during the wallet migration process into a single db txn in the future. ACKs for top commit: achow101: ACK 9a3c5c8 josibake: ACK bitcoin@9a3c5c8 Tree-SHA512: fb2ecc48224c400ab3b1fbb32e174b5b13bf03794717727f80f01f55fb183883b067a68c0a127b2de8885564da15425d021a96541953bf38a72becc2e9929ccf
'RunWithinTxn()' provides a way to execute db operations within a transactional context. It avoids writing repetitive boilerplate code for starting and committing the database transaction.
Transactions are intended to be started on upper layers rather than internally by the bdb batch object. This enables us to consolidate different write operations within a procedure in the same db txn, improving consistency due to the atomic property of the transaction, as well as its performance due to the reduction of disk write operations. Important Note: This approach also ensures that the BerkeleyBatch::ErasePrefix function behaves exactly as the SQLiteBatch::ErasePrefix function, which does not create a db txn internally. Furthermore, since the `BerkeleyBatch::ErasePrefix' implementation erases records one by one (by traversing the db), this change ensures that the function is always called within an active txn context. Without this measure, there's a potential risk to consistency; certain records may be removed while others could persist due to an internal failure during the procedure.
...by adding a missing sync_blocks call. There was a race at node2 between connecting the block produced by node 0, and using -generate to create new blocks itself. In the failed run, the latter happened first, resulting in a final block height that was smaller by 1 than expected.
The diff is produced by running `make -C src translate`.
…store.py 44d1153 test: fix intermittent failure in wallet_reorgrestore.py (Martin Zumsande) Pull request description: By adding a missing `sync_blocks` call. There was a race at `node2` between connecting the block produced by `node0`, and using `-generate` to create new blocks itself. In the failed run, block generation started before connecting the block, resulting in a final block height that was smaller by 1 than expected. See bitcoin#29392 (comment) for a more detailed analysis of the failed run. Can be reproduced by adding a sleep to [this spot](https://github.com/bitcoin/bitcoin/blob/6ff0aa089c01ff3e610ecb47814ed739d685a14c/src/validation.cpp#L4217) in `ChainstateManager::ProcessNewBlock()`: ``` if (util::ThreadGetInternalName() == "msghand") { std::this_thread::sleep_for(0.2s); } ``` which fails for me on master and succeeds with the fix. Fixes bitcoin#29392 ACKs for top commit: maflcko: lgtm ACK 44d1153 Tree-SHA512: c08699e5ae348d4c0626022b519449d052f511d3f44601bcd8dac836a130a3f67fca149532e1e3690367ebfdcbcdd32e527170d039209c1f599ce861136ae29f
6b161cb [test] second child of a v3 tx can be replaced individually (glozow) 5c998a6 [refactor] use MAX_PUBKEYS_PER_MULTISIG instead of magic numbers in test (glozow) a934642 [test] PackageV3Checks with inheritance violation in mempool ancestor (glozow) 63b62e1 [doc] fix docs and comments from v3 (glozow) Pull request description: Addresses final comments from bitcoin#28948: - thread at bitcoin#28948 (comment), using ismaelsadeeq@87fc7f0 with some modifications - bitcoin#28948 (comment) - bitcoin#28948 (comment) - bitcoin#28948 (comment) - bitcoin#28948 (comment) - bitcoin#28948 (comment) - bitcoin#28948 (comment) - bitcoin#28948 (comment) - bitcoin#28948 (comment) ACKs for top commit: instagibbs: ACK 6b161cb sdaftuar: utACK 6b161cb Tree-SHA512: 584fce7810f4d704ee6ab51fdc7d42bab342140cae3d076f89b5e1966dd1dd8293cb25b3121e41a4dcd65f9d4a735102b9ab2e90f98aa770b84e21f4d35d63d3
…n mempool not empty 8d20602 test, assumeutxo: Add test to ensure failure when mempool not empty (Hernan Marino) Pull request description: Add a test to ensure that loadtxoutset fails when the node's mempool is not empty, as suggested by maflcko here: bitcoin#27596 (comment) ACKs for top commit: maflcko: re-ACK 8d20602 BrandonOdiwuor: ACK 8d20602 Tree-SHA512: 97c9668c0f38897934bf0d326515d898d4e682ff219deba9d751b35125b5cf33d51c9df116a74117ecf0394f28995a3d0cae1266b1e5acb4365ff4f309ce3f6c
…telist{bind}Permissions::TryParse`
864e2e9 fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse` (brunoerg)
Pull request description:
The string `s` represents the value from `-whitelist`/`-whitebind` (e.g. "bloom,forcerelay,[email protected]:32") and it is used in `NetWhitelistPermissions::TryParse` and `NetWhitebindPermissions::TryParse`. However, a max length of 32 is not enough to cover a lot of cases. Even disconsidering the permissions, 32 would not be enough to cover a lot of addresses. This PR fixes it.
ACKs for top commit:
maflcko:
lgtm ACK 864e2e9
epiccurious:
utACK 864e2e9.
vasild:
ACK 864e2e9
Tree-SHA512: 2b89031b9f2ea92d636f05fd167b1e5ac726742a7e7c1af8ddaeaf90236e659731aaa6b7c23f65ec16ce52ac1b9e68e7b16e23c59e355312d057e001976d172a
Github-Pull: bitcoin#30299 Rebased-From: 518b06c
See: miniupnp/miniupnp@c0a50ce The return value of 2 now indicates: "A valid connected IGD has been found but its IP address is reserved (non routable)" We continue to ignore any return value other than 1. Github-Pull: bitcoin#30283 Rebased-From: 8acdf66
b3093eb doc: Update rel notes for 27.x (fanquake) 6338f92 upnp: add compatibility for miniupnpc 2.2.8 (Cory Fields) f34e446 ci: remove unused bcc variable from workflow (Max Edwards) 0d524b1 ci: move Asan / LSan / USDT job to Github Actions (Max Edwards) 43c40dd ci: add IPV6 network to ci container (Max Edwards) Pull request description: Backports: * bitcoin#30193 * bitcoin#30283 * bitcoin#30299 ACKs for top commit: willcl-ark: ACK b3093eb stickies-v: ACK b3093eb Tree-SHA512: 325149f2b388072276e10fae2ebb7d8f3f5138d75f237c0182a09c631334fc2af9c2fe500db31bf41e94d4f154771e3cd386f8eb0d09d7a1ad656f637b71e735
A common issue that our fuzzers keep finding is that outpoints don't exist in the non witness utxos. Instead of trying to track this down and checking in various individual places, do the check early during deserialization. Github-Pull: bitcoin#29855 Rebased-From: 9e13ccc
Initiating an outbound network connection currently involves the
following steps after the socket connection is established (see
`CConnman::OpenNetworkConnection` method):
1. set up node state
2. queue VERSION message
3. add new node to vector `m_nodes`
If we connect to ourself, it can happen that the sent VERSION message
(step 2) is received and processed locally *before* the node object
is added to the connection manager's `m_nodes` vector (step 3). In this
case, the self-connect remains undiscovered, as the detection doesn't
find the outbound peer in `m_nodes` yet (see `CConnman::CheckIncomingNonce`).
Fix this by swapping the order of 2. and 3., by taking the `PushNodeVersion`
call out of `InitializeNode` and doing that in the `SendMessages` method
instead, which is only called for `CNode` instances in `m_nodes`.
Thanks go to vasild, mzumsande, dergoegge and sipa for suggestions on
how to fix this.
Github-Pull: bitcoin#30394
Rebased-From: 66673f1
Now that the queueing of the VERSION messages has been moved out of
`InitializeNode`, there is no need to pass a mutable `CNode` reference any
more. With a const reference, trying to send messages in this method would
lead to a compile-time error, e.g.:
----------------------------------------------------------------------------------------------------------------------------------
...
net_processing.cpp: In member function ‘virtual void {anonymous}::PeerManagerImpl::InitializeNode(const CNode&, ServiceFlags)’:
net_processing.cpp:1683:21: error: binding reference of type ‘CNode&’ to ‘const CNode’ discards qualifiers
1683 | PushNodeVersion(node, *peer);
...
----------------------------------------------------------------------------------------------------------------------------------
Github-Pull: bitcoin#30394
Rebased-From: 0dbcd4c
…ect" This reverts commit 9ec2c53 with a tiny change included (identation of the wait_until call). Github-Pull: bitcoin#30394 Rebased-From: 16bd283
This avoids situations during a reindex in which shutdown doesn't finish since SyncWithValidationInterfaceQueue is called by the load block thread when the scheduler is already stopped. Github-Pull: bitcoin#30435 Rebased-From: 5fd4836
Fix cases of calls to `FillPSBT` returning `complete=true` when it's not the case. This can happen when some inputs have been signed but the transaction is subsequently modified, e.g. in the context of PayJoins. Also fixes a related bug where a finalized hex string is attempted to be added during `walletprocesspsbt` but a CHECK_NONFATAL causes an abort. Reported in bitcoin#30077. Github-Pull: bitcoin#30357 Rebased-From: 39cea21
This test checks that we can successfully process PSBTs and opt out of finalization. Previously trying to call `walletprocesspsbt` would attempt to auto-finalize (as a convenience), and would not permit opt-out of finalization, instead aborting via `CHECK_NONFATAL`. Github-Pull: bitcoin#30357 Rebased-From: 7e36dca
4f23c86 [WIP] doc: update release notes for 27.x (fanquake) 54bb9b0 test: add test for modififed walletprocesspsbt calls (willcl-ark) f22b9ca wallet: fix FillPSBT errantly showing as complete (willcl-ark) 05192ba init: change shutdown order of load block thread and scheduler (Martin Zumsande) ab42206 Reapply "test: p2p: check that connecting to ourself leads to disconnect" (Sebastian Falbesoner) 064f214 net: prevent sending messages in `NetEventsInterface::InitializeNode` (Sebastian Falbesoner) 0933cf5 net: fix race condition in self-connect detection (Sebastian Falbesoner) fa90989 psbt: Check non witness utxo outpoint early (Ava Chow) Pull request description: Backports: * bitcoin#29855 * bitcoin#30357 * bitcoin#30394 (modified test commit) * bitcoin#30435 ACKs for top commit: stickies-v: ACK 4f23c86 willcl-ark: ACK 4f23c86 Tree-SHA512: 5c26445f0855f9d14890369ce19873b0686804eeb659e7d6da36a6f404f64d019436e1e6471579eaa60a96ebf8f64311883b4aef9d0ed528a95bd610c101c079
In python, if the default value is a mutable object (here: a class) its shared over all instances, so that one instance being changed would affect others to be changed as well. This was likely the source of various intermittent bugs in the functional tests. Github-Pull: bitcoin#30552 Rebased-From: ec5e294
Github-Pull: bitcoin#30633 Rebased-From: 138f867
In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/format:48,
from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/chrono_io.h:39,
from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/chrono:3362,
from ./util/time.h:9,
from ./primitives/block.h:12,
from ./blockencodings.h:8,
from blockencodings.cpp:5:
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits: In substitution of 'template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<CFeeRate>::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]':
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1140:25: required by substitution of 'template<class _Tp, class ... _Args> using std::__is_constructible_impl = std::__bool_constant<__is_constructible(_Tp, _Args ...)> [with _Tp = CFeeRate; _Args = {std::optional<CFeeRate>&}]'
1140 | = __bool_constant<__is_constructible(_Tp, _Args...)>;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1145:12: required from 'struct std::is_constructible<CFeeRate, std::optional<CFeeRate>&>'
1145 | struct is_constructible
| ^~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:178:35: required by substitution of 'template<class ... _Bn> std::__detail::__first_t<std::integral_constant<bool, false>, typename std::enable_if<(!(bool)(_Bn::value)), void>::type ...> std::__detail::__or_fn(int) [with _Bn = {std::is_constructible<CFeeRate, std::optional<CFeeRate>&>, std::is_convertible<std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, std::optional<CFeeRate> >, std::is_convertible<std::optional<CFeeRate>, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate>&>, std::is_convertible<const std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate> >, std::is_convertible<const std::optional<CFeeRate>, CFeeRate>}]'
178 | __enable_if_t<!bool(_Bn::value)>...>;
| ^~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:196:41: required from 'struct std::__or_<std::is_constructible<CFeeRate, std::optional<CFeeRate>&>, std::is_convertible<std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, std::optional<CFeeRate> >, std::is_convertible<std::optional<CFeeRate>, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate>&>, std::is_convertible<const std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate> >, std::is_convertible<const std::optional<CFeeRate>, CFeeRate> >'
196 | : decltype(__detail::__or_fn<_Bn...>(0))
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:824:45: required from 'constexpr const bool std::optional<CFeeRate>::__construct_from_contained_value<CFeeRate, CFeeRate>'
824 | = !__converts_from_optional<_Tp, _From>::value;
| ^~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:7: required by substitution of 'template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<CFeeRate>::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]'
884 | && __construct_from_contained_value<_Up>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./validation.h:164:41: required from here
164 | return MempoolAcceptResult(state);
| ^
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:886:2: required by the constraints of 'template<class _Tp> template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<_Tp>::optional(const std::optional<_From>&)'
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:14: error: satisfaction of atomic constraint '__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type> [with _Tp = _Tp; _Up = _Up]' depends on itself
884 | && __construct_from_contained_value<_Up>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Github-Pull: bitcoin#30633
Rebased-From: 055bc05
b06c4c6 [WIP] doc: update release notes for 27.x (fanquake) 57de0f5 policy/feerate.h: avoid constraint self-dependency (Matt Whitlock) ccff378 add missing #include <cstdint> for GCC 15 (Matt Whitlock) 500bba0 test: fix constructor of msg_tx (Martin Zumsande) Pull request description: Backports: * bitcoin#30552 * bitcoin#30633 ACKs for top commit: stickies-v: ACK b06c4c6 Tree-SHA512: 1b669d1c7e0c6c2c2a1b123970c2b5b59a417a423ee1133296ebad2ecb50e5c3889a6ae8dc640f8ae464a969b1b0287a8005a3317ee7d7252b61d96e59c131a4
0cdfb7e doc: update for 27.2rc1 (fanquake) 693403b doc: update manual pages for 27.2rc1 (fanquake) c338e43 build: bump version to 27.2rc1 (fanquake) Pull request description: Prepare for 27.2. ACKs for top commit: stickies-v: ACK 0cdfb7e Tree-SHA512: 2ccf7d9eb02450e254649d50188c15c754ce74fb94e5101324a1cde5839de5451b0dc7567eed135c84bf83fefcd1a6d37cb757094f27563cbff43969b9852b1b
Having `@par title` followed by an empty line renders improperly in Doxygen - it results in a paragraph with a title but without a body. https://www.doxygen.nl/manual/commands.html#cmdpar This also results in a compiler warning (or error) with Clang 19: ``` ./txmempool.h:368:34: error: empty paragraph passed to '@Par' command [-Werror,-Wdocumentation] 368 | * @Par Consistency guarantees | ~~~~~~~~~~~~~~~~~~~~~~~~~~^ 1 error generated. ``` Github-Pull: bitcoin#30504 Rebased-From: 6a5e9e4
dd1bf8b doc: update manual pages for 27.2 (fanquake) f42fcf6 build: bump version to v27.2 final (fanquake) 6c09325 doc: finalise release notes for 27.2 (fanquake) c838ce5 doc: use proper doxygen formatting for CTxMemPool::cs (Vasil Dimov) Pull request description: This backports one other change (that doesn't warrant an rc), which fixes noisey output from newer versions of Clang (19+). Also makes the changes for 27.2 final. Bins for rc1 are available here: https://bitcoincore.org/bin/bitcoin-core-27.2/test.rc1/. ACKs for top commit: stickies-v: ACK dd1bf8b Tree-SHA512: 10599443bb8862dc5f238246e21ff817d572fc23174efc0fe27960e490a4e82501555bc859a1a84f465ea211b00c54a3e9125612ca2d98be6e1e8684d5c61a4b
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.