d3b8a54a81 Refactor CFeeRate to use FeeFrac internally (Pol Espinasa)
Pull request description:
The `FeeFrac` type represents a fraction, intended to be used for `sats/vbyte` or `sats/WU`. It was added to improve accuracy when evaluating fee rates in cluster mempool. [1]
But it can also be used to fix the precision issues that the current `CFeeRate` class has now.
At the moment, `CFeeRate` handles the fee rate as satoshis per kilovirtualbyte: `CAmount / kvB` using an integer.
This PR fix `CFeeRate` precision issues by encapsulating `FeeFrac` internally keeping backwards compatibility.
This PR can also be used as a based to use multiple units on RPC calls as detailed in this issue [2].
Some previous discussions:
[1] https://github.com/bitcoin/bitcoin/pull/30535
[2] https://github.com/bitcoin/bitcoin/issues/32093
ACKs for top commit:
achow101:
ACK d3b8a54a81
murchandamus:
code review, lightly tested ACK d3b8a54a81
ismaelsadeeq:
re-ACK d3b8a54a81📦
theStack:
Code-review ACK d3b8a54a81
Tree-SHA512: 5a8149d81e82ad4e60a0e76ff6a82a5b1c4e212cf5156c1cdd16bf9acbb351e7be458eac3f0a2ae89107f331062b299c1d9ca649d3b820ad0b68e6d1a14292e5
27aefac425 validation: detect witness stripping without re-running Script checks (Antoine Poinsot)
2907b58834 policy: introduce a helper to detect whether a transaction spends Segwit outputs (Antoine Poinsot)
eb073209db qa: test witness stripping in p2p_segwit (Antoine Poinsot)
Pull request description:
Since it was introduced in 4eb515574e (#18044), the detection of a stripped witness relies on running the Script checks 3 times. In the worst case, this consists in running Script validation for every single input 3 times.
Detection of a stripped witness is necessary because in this case wtxid==txid, and the transaction's wtxid must not be added to the reject filter or it could allow a malicious peer to interfere with txid-based orphan resolution as used in 1p1c package relay.
However it is not necessary to run Script validation to detect a stripped witness (much less so doing it 3 times in a row). There are 3 types of witness program: defined program types (Taproot, P2WPKH and P2WSH), undefined types, and the Pay-to-anchor carve-out.
For defined program types, Script validation with an empty witness will always fail (by consensus). For undefined program types, Script validation is always going to fail regardless of the witness (by standardness). For P2A, an empty witness is never going to lead to a failure.
Therefore it holds that we can always detect a stripped witness without re-running Script validation. However this might lead to more "false positives" (cases where we return witness stripping for an otherwise invalid transaction) than the existing implementation. For instance a transaction with one P2PKH input with an invalid signature and one P2WPKH input with its witness stripped. The existing implementation would treat it as consensus invalid while the implementation in this PR would always consider it witness stripped.
h/t AJ: this essentially implements a variant of https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3135258539.
ACKs for top commit:
sipa:
re-ACK 27aefac425
Crypt-iQ:
re-ACK 27aefac425
glozow:
reACK 27aefac425
Tree-SHA512: 70cf76b655b52bc8fa2759133315a3f11140844b6b80d9de3c95f592050978cc01a87bd2446e3a9c25cc872efea7659d6da3337b1a709511771fece206e9f149
656e16aa5e qt: Update the `src/qt/locale/bitcoin_en.xlf` translation source file (Hennadii Stepanov)
a0eaa44925 Fix typos (Hennadii Stepanov)
8d4aaaec49 Update Transifex slug for 30.x (Hennadii Stepanov)
Pull request description:
This PR follows our [Release Process](53a996f122/doc/release-process.md).
It is required to open Transifex translations for v30.0, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.
For reference, see the previous similar PR: https://github.com/bitcoin/bitcoin/pull/31809.
**Note for reviewers:**
To reproduce the diff in the last commit, run:
```
cmake --preset dev-mode
cmake --build build_dev_mode --target translate
```
ACKs for top commit:
laanwj:
Code review ACK 656e16aa5e
stickies-v:
ACK 656e16aa5e , was able to reproduce
Tree-SHA512: 403b534329755079584fcdf98b696e3e75952dfc8d069f305843dbfa85de95f6816ee1d5dfc9b553c7c7f52cc296cb8d3cb03207051d26e0e76ff30d377f49e4
Since it was introduced in 4eb515574e (#18044), the detection of a
stripped witness relies on running the Script checks 3 times. In the worst case, this consists in
running Script validation 3 times for every single input.
Detection of a stripped witness is necessary because in this case wtxid==txid, and the transaction's
wtxid must not be added to the reject filter or it could allow a malicious peer to interfere with
txid-based orphan resolution as used in 1p1c package relay.
However it is not necessary to run Script validation to detect a stripped witness (much less so
doing it 3 times in a row). There are 3 types of witness program: defined program types (Taproot,
P2WPKH, P2WSH), undefined types, and the Pay-to-anchor carve-out.
For defined program types, Script validation with an empty witness will always fail (by consensus).
For undefined program types, Script validation is always going to fail regardless of the witness (by
standardness). For P2A, an empty witness is never going to lead to a failure.
Therefore it holds that we can always detect a stripped witness without re-running Script validation.
However this might lead to more "false positives" (cases where we return witness stripping for an
otherwise invalid transaction) than the existing implementation. For instance a transaction with one
P2PKH input with an invalid signature and one P2WPKH input with its witness stripped. The existing
implementation would treat it as consensus invalid while the implementation in this commit would
always consider it witness stripped.
f49840dd90 doc: Fix typo in files.md (Ryan Ofsky)
f5cf0b1ccc bitcoin wrapper: improve help output (Ryan Ofsky)
c810b168b8 doc: Add description of installed files to files.md (Ryan Ofsky)
94ffd01a02 doc: Add release notes describing libexec/ binaries (Ryan Ofsky)
cd97905ebc cmake: Move internal binaries from bin/ to libexec/ (Ryan Ofsky)
Pull request description:
This change moves binaries that are not typically invoked directly by users from the `bin/` directory to the `libexec/` directory in CMake installs and binary releases. The goal of the PR is to introduce a distinction between internal and external binaries so starting with #31802, we can use IPC to implement features in new binaries without adding those binaries to the CLI. The change also helps reduce clutter in `bin/`, making it easier for users to identify useful tools to run. Summary of changes:
- For **source builds** (i.e. developer builds) — There are no changes.
- For **source installs** (i.e. `cmake --install` result) — `test_bitcoin`, `test_bitcoin-qt`, and `bench_bitcoin` are installed in `${CMAKE_PREFIX_PATH}/libexec` instead of `${CMAKE_PREFIX_PATH}/bin`, so they are no longer on the system `PATH`. However, they can still be invoked from the `libexec/` directory, or from the CLI as `bitcoin test`, `bitcoin test-gui`, and `bitcoin bench`, respectively.
- For **binary releases** — Since `test_bitcoin` is the only test binary enabled in releases, the only change is moving `test_bitcoin` from `bin/` to `libexec/`.
<details><summary>Details</summary>
<p>
The table below shows the install location of each binary after this change, and the availability of each binary.
| Binary | Location | Availability | Change |
|----------------------|--------------|----------------------|-------------------------------|
| `bitcoin` | `bin/` | 📦 Binary release (since #31375) | Unchanged |
| `bitcoin-cli` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoind` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-qt` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-tx` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-util` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-wallet` | `bin/` | 📦 Binary release | Unchanged |
| `bench_bitcoin` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
| `bitcoin-chainstate` | `libexec/` | 🛠 Source build only | Newly installed (was built) |
| `bitcoin-gui` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `bitcoin-node` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `test_bitcoin` | `libexec/` | 📦 Binary release | Moved from `bin/` |
| `test_bitcoin-qt` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
</p>
</details>
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
l0rinc:
re-ACK f49840dd90
Sjors:
re-ACK f49840dd90
achow101:
ACK f49840dd90
janb84:
re ACK f49840dd90
BrandonOdiwuor:
Tested ACK f49840dd90
hodlinator:
re-ACK f49840dd90
willcl-ark:
utACK f49840dd90
Tree-SHA512: 858a2e1a53db11ee3c5c759bfdeea566f242b9ce5e8a898fa435222e41662b8184577c0dc2c4c058294b4de41d8cb3ba3e5d24c748c280efa4a3f84e3ec4344d
9a5d29711a Squashed 'src/crc32c/' changes from b60d2b7334..efb8ea04e4 (fanquake)
Pull request description:
Sync the subtree with latest upstream. The changes here are a no-op, but pull them to fix the drive-by-typo-fixing: #33057.
Includes https://github.com/bitcoin-core/crc32c-subtree/pull/8.
ACKs for top commit:
maflcko:
lgtm ACK 8ef8dd6871
janb84:
ACK 8ef8dd6871
Tree-SHA512: b20a47514218206b934c4aa27ec667fb9b3ec7f7388a78725c52fc6e916358d2b9a2075a37808dbc2430b4c7816511ecf20e36bfe2fbd2d8a26bc8882a46d5e7
86e3a0a8cb refactor: standardize obfuscation memory alignment (Lőrinc)
13f00345c0 refactor: write `Obfuscation` object when new key is generated in dbwrapper (Lőrinc)
e5b1b7c557 refactor: rename `OBFUSCATION_KEY_KEY` (Lőrinc)
298bf95105 refactor: simplify `Obfuscation::HexKey` (Lőrinc)
2dea045425 test: make `obfuscation_serialize` more thorough (Lőrinc)
a17d8202c3 test: merge xor_roundtrip_random_chunks and xor_bytes_reference (Lőrinc)
Pull request description:
Follow up for https://github.com/bitcoin/bitcoin/pull/31144
Applied the remaining comments in separate commits - except for the last one where I could group them.
Please see the commit messages for more context.
ACKs for top commit:
achow101:
ACK 86e3a0a8cb
ryanofsky:
Code review ACK 86e3a0a8cb, just tweaking key write assert as suggested
hodlinator:
ACK 86e3a0a8cb
Tree-SHA512: 967510a141fbb57bf9d088d92b554cf2fffc2f6aa0eab756cbae3230f53e9b04ceebcc6fea5f3383c01ad41985ecde5b5686c64a771ca9deae3497b9b88c1c8b
fdbade6f8d kernel: create monolithic kernel static library (Cory Fields)
Pull request description:
Currently, consuming `libbitcoinkernel.a` requires all its dependency static libraries to be available. A switch to a monolithic variant, which contains object files from its dependencies, was discussed in the Kernel WG. The necessary preparations in the libsecp256k1 build scripts were completed in https://github.com/bitcoin-core/secp256k1/pull/1678, which are now available in this repository since https://github.com/bitcoin/bitcoin/pull/33036.
The changes in this PR were picked from https://github.com/theuni/bitcoin/commits/static_kernel/, with an additional adjustment in `libbitcoinkernel.pc.in`.
This PR can be tested as described in https://github.com/bitcoin/bitcoin/pull/30814#issue-2505698234.
ACKs for top commit:
TheCharlatan:
ACK fdbade6f8d
stickies-v:
tACK fdbade6f8d
Tree-SHA512: bd9e9dbb0b765bdcb162fb3f4ad3c4e01fe5fa0b7061f97d0bad64442b21db036cbe0e4341fd45d43a8862df76d62c9532ca8945f76423aca753c6b528f70873
b093a19ae2 cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED` (Hennadii Stepanov)
eb59a192d9 cmake, refactor: Encapsulate adding secp256k1 subtree in function (Hennadii Stepanov)
Pull request description:
The `SECP256K1_DISABLE_SHARED` CMake variable has been [removed](https://github.com/bitcoin-core/secp256k1/pull/1688) upstream.
This PR removes its usage ahead of the next `secp256k1` subtree update to prevent breakage and simplify integration.
ACKs for top commit:
fanquake:
ACK b093a19ae2
Tree-SHA512: a87cee71cf356f458f68d3163253ca5c4f86e56d268006b6b8e1d4b2c009ba436148a07a6b67b89ddbb2d0e3c1113ab4b4906c5fc5624cb3082b20e916e0e82b
3543bfdfec test: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify 'spend_vin' is the correct field (Chris Stewart)
Pull request description:
Fixes bug in `getdescriptoractivity` RPC help manual.
Here is the line that pushes `spend_vin` field, there is no `spend_vout` json field.
https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2757
ACKs for top commit:
nervana21:
tACK 3543bfd
luke-jr:
utACK 3543bfdfec
jonatack:
ACK 3543bfdfec
Tree-SHA512: 2cd543569a87261d8d804d9afe36f8e8ead55839c01da9c4831aea3ced7d1251e6885621e628898105700aae4d76cbb8a682f518f33c1c52163e66f75ec87a61
a26fbee38f qt: Translations update (Hennadii Stepanov)
ca04eebd72 cmake: Switch to generated `ts_files.cmake` file (Hennadii Stepanov)
95341de6ca cmake, refactor: Move handling of Qt TS files into `locale` directory (Hennadii Stepanov)
Pull request description:
This PR:
1. Moves handling of Qt TS files into the `locale` directory.
2. Switches from inferior globbing to the explicit file list generated by the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) script.
Closes#32653.
ACKs for top commit:
fanquake:
ACK a26fbee38f
Tree-SHA512: 10596768c120d9da21a2340b693c5f39e9e1e02976805e14284cf9785780756f953d73d3d2b4a7246ada37acafe3e5d0e8927a8f51bf9fce3bc93a3544231489
ad132761fc [allocators] Apply manual ASan poisoning to PoolResource (dergoegge)
Pull request description:
Currently ASan will not detect use-after-free issues for memory allocated by a `PoolResource`. This is because ASan is only aware of the memory chunks allocated by `PoolResource` but not the individual "sub-chunks" within.
E.g. this test will not produce an ASan error even though the referenced coin has been deallocated:
```c++
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index c46144b34b..aa6ca15ce1 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -508,6 +508,17 @@ BOOST_FIXTURE_TEST_CASE(updatecoins_simulation_test, UpdateTest)
BOOST_CHECK(spent_a_duplicate_coinbase);
}
+BOOST_AUTO_TEST_CASE(asan_uaf)
+{
+ CCoinsMapMemoryResource cache_coins_memory_resource{};
+ CCoinsMap map(0, SaltedOutpointHasher(/*deterministic=*/true), CCoinsMap::key_equal{}, &cache_coins_memory_resource);
+ COutPoint outpoint{};
+ map.emplace(outpoint, Coin{});
+ auto& coin = map.at(outpoint);
+ map.erase(outpoint);
+ coin.coin.nHeight = 1;
+}
+
BOOST_AUTO_TEST_CASE(ccoins_serialization)
{
// Good example
```
Fix this by applying [manual ASan poisoning](https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning) for memory allocated by `PoolResource`:
* Newly allocated chunks are poisoned as a whole
* "Sub-chunks" are unpoisoned/re-poisoned during allocation/deallocation
With the poisoning applied, ASan catches the issue in the test above:
```
$ ./build_unit/bin/test_bitcoin --run_test="coins_tests/asan_uaf"
Running 1 test case...
=================================================================
==366064==ERROR: AddressSanitizer: use-after-poison on address 0x7f99c3204870 at pc 0x55569dab6f8a bp 0x7ffe0210e4d0 sp 0x7ffe0210e4c8
READ of size 4 at 0x7f99c3204870 thread T0 (b-test)
```
ACKs for top commit:
achow101:
ACK ad132761fc
marcofleon:
code review ACK ad132761fc
Tree-SHA512: eb5e80bfa9509225e784151807bd8aa21fb0826ca1781dfe81b1d60bd3766019384ea3f9cb8e53398fde2f4e994a9c201b5a9962b4d279d7e52bb60e8961be11
1252eeb997 rpc: fix getpeerinfo ping duration unit docs (0xb10c)
Pull request description:
The docs have been incorrect since a3789c700b (released in v25; master since Sept. 2022). Noticed while setting up monitoring using getpeerinfo.
0cb1ed2b7c/src/rpc/net.cpp (L249-L257)
ACKs for top commit:
luke-jr:
utACK 1252eeb997
maflcko:
lgtm ACK 1252eeb997
jonatack:
ACK 1252eeb997
theStack:
ACK 1252eeb997
janb84:
ACK 1252eeb997
Tree-SHA512: 33f576336b2a4d9533f51f4641d564ee59ef692c5fa9a3cad239fc31465883d5da534bfd0e069be1e1d688e5f0dea3fe6850be19bf35335041b8f414d08f7f09
When the detailed peers list is requested, return the shortened services in the
-netinfo header in the same format as the "serv" column, instead of the full names
list in the report.
The getpeerinfo docs incorrectly specified the ping durations as
milliseconds. This was incorrectly changed in a3789c700b
(released in v25; master since Sept. 2022). The correct duration unit
is seconds.
Also, remove the documentation of the getpeerinfo RPC response from the
ping RPC since it's incomplete. Better to just reference the getpeerinfo
RPC and it's documenation for this.
e017ef3c7e init: make `-blockmaxweight` startup option debug-only (ismaelsadeeq)
Pull request description:
This PR updates `-blockmaxweight` startup option to be debug-only so that it will be hidden from help text.
The option is currently unlikely to be used on mainnet, after the addition of the new `blockreservedweight` option. however it can be useful for test and signet network see https://github.com/bitcoin/bitcoin/pull/32654#issuecomment-2925674473
ACKs for top commit:
Sjors:
tACK e017ef3c7e
fjahr:
ACK e017ef3c7e
polespinasa:
tACK e017ef3c7e
Tree-SHA512: 6c18781826b2f96b13b70b7f1624481f5971746a613079d0d9528366f274ba657a02611f134d7a64f35ecb7e5faf2e3cd025458b04574ac68f804372f6eb715f
a3cf623364 test: Test max_selection_weight edge cases (Murch)
57fe8acc8a test: Check max_weight_exceeded error (Murch)
Pull request description:
I tested all of the reported surviving mutants that @brunoerg reported in https://gist.github.com/brunoerg/834063398d5002f738506d741513e310.
I found that all Mutants except for 12, 14, 17, 37, and 39 were now being caught by one of the existing tests. This fixes Mutants 14, 37, and 39.
Mutant 17 is not fixed, because I consider it acceptable that running BnB for 100,001 instead of 100,000 comparisons doesn’t cause an issue, and Mutant 12 is not yet fixed, because at `fee` = `long_term_fee`, the waste of inputs is 0 and only excess matters, and I haven’t evaluated yet, whether it needs to be fixed.
ACKs for top commit:
achow101:
ACK a3cf623364
jlest01:
ACK a3cf623364
brunoerg:
code review ACK a3cf623364
Tree-SHA512: db67c52127ed98f809f64a903c6b3a012e56cf665a0cd851457af7c85c37ec3af8bb72035d7ad370dd883f99cf3014464e3576559899e37c1d6ee01230511754
cc33e45789 test: improve assertion for SRD max weight test (yancy)
Pull request description:
Replace generic assertion with a result specific assertion showing the correctness of the solution found. If the max weight parameter is exceeded, the least valuable `UTXOs` are removed from the result. Therefore, only the most valued _encountered_ `UTXO's` are selected. While the smallest set would include all the most valued `UTXO's`, in the case of the test there is one high value `UTXO` that is never found before the target value is reached.
Correct the test comment to be more specific about why the assertion is a good result.
ACKs for top commit:
murchandamus:
ACK cc33e45789
furszy:
ACK cc33e45789
Tree-SHA512: bad224063ba830c27fba1b7b80e411ac7cd6c3edcb60bade4e6e3010f3b5d360a921de742c7c20efea8fa839d7939f338270658f66bbcebedebe5c5c8a3e8f9b
c0642e558a [fuzz] fix latency score check in txorphan_protected (glozow)
3d4d4f0d92 scripted-diff: rename "ann" variables to "latency_score" (monlovesmango)
3b92448923 [doc] comment fixups for orphanage changes (glozow)
1384dbaf6d [config] emit warning for -maxorphantx, but allow it to be set (glozow)
b10c55b298 fix up TxOrphanage lower_bound sanity checks (glozow)
cfd71c6704 scripted-diff: rename TxOrphanage outpoints index (glozow)
edb97bb3f1 [logging] add logs for inner loop of LimitOrphans (glozow)
8a58d0e87d scripted-diff: rename OrphanTxBase to OrphanInfo (glozow)
cc50f2f0df [cleanup] replace TxOrphanage::Size() with CountUniqueOrphans (glozow)
ed24e01696 [optimization] Maintain at most 1 reconsiderable announcement per wtxid (Pieter Wuille)
af7402ccfa [refactor] make TxOrphanage keep itself trimmed (glozow)
d1fac25ff3 [doc] 31829 release note (glozow)
Pull request description:
Followup to #31829:
- Release notes
- Have the orphanage auto-trim itself whenever necessary (and test changes) https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2169508690
- Reduce duplicate reconsiderations by keeping track of which txns are already reconsiderable so we only mark it for reconsideration for 1 peer at a time https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3001627814
- Rename `OrphanTxBase` to `OrphanInfo`
- Get rid of `Size()` method by replacing all calls with `CountUniqueOrphans`
- Rename outpoints index since they point to wtxids, not iterators https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205557613
- Add more logging in the `LimitOrphans` inner loop to make it easy to see which peers are being trimmed https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3074385460
ACKs for top commit:
sipa:
utACK c0642e558a
marcofleon:
Nice, ACK c0642e558a
Tree-SHA512: f298eae92cf906ed5e4f15a24eeffa7b9e620bcff457772cd77522dd9f0b3b183ffc976871b1b0e6fe93009e64877d518e53d4b9e186e0df58fc16d17f6de90a
The translations for the following languages, which appear to be the
result of a mistake or an act of vandalism, have been discarded:
- Greek (el)
- Vietnamese (vi)
In the `txgraph` fuzz test, the `CommitStaging` step updates the
`SimTxGraph` levels simply by erasing the front (=main) one in the
`sims` vector, i.e. the staging level instance takes the place of the
main level instance. This also includes the `real_is_optimal` flag
(reflecting whether the corresponding real graph is known to be
optimally linearized), without taking into account that this flag
should only be set if _both_ levels before the commiting are optimal.
E.g. in case of #33097, the main level is not optimally linearized,
while the staging level is, and due to the incorrect propagation of the
latter to the simulation incorrectly assumes that the main level is
optimal, leading to the assertion fail. Fix this by setting the flag
in the resulting main level explicitly.
Resolves the fuzzing assertion fail in issue #33097.
-BEGIN VERIFY SCRIPT-
sed -i 's/max_global_ann/max_global_latency_score/g' src/node/txorphanage.cpp
sed -i 's/max_global_ann/max_global_latency_score/g' src/node/txorphanage.h
sed -i 's/max_global_ann/max_global_latency_score/g' src/test/orphanage_tests.cpp
sed -i 's/max_global_ann/max_global_latency_score/g' src/test/fuzz/txorphan.cpp
sed -i 's/max_global_ann/max_global_latency_score/g' src/bench/txorphanage.cpp
sed -i 's/max_ann/max_lat/g' src/node/txorphanage.cpp
-END VERIFY SCRIPT-
This introduces an invariant that TxOrphanageImpl never holds more than one
announcement with m_reconsider=true for a given wtxid. This avoids duplicate
work, both in the caller might otherwise reconsider the same transaction multiple
times before it is ready, and internally in AddChildrenToWorkSet, which might
otherwise iterate over all announcements multiple times.
This change offers a few advantages, such as:
- a more readable and cleaner `ts_files.cmake` (see the next commit);
- a scoped `ts_files` variable;
- improved code locality;
- no need to adjust the location of the resulting `*.qm` files.
ea17a9423f [doc] release note for relaxing requirement of all unconfirmed parents present (glozow)
12f48d5ed3 test: add chained 1p1c propagation test (Greg Sanders)
525be56741 [unit test] package submission 2p1c with 1 parent missing (glozow)
f24771af05 relax child-with-unconfirmed-parents rule (glozow)
Pull request description:
Broadens the package validation interface, see #27463 for wider context.
On master, package rules include that (1) the package topology must be child-wth-parents (2) all of the child's unconfirmed parents must be present. This PR relaxes the second rule, leaving the first rule untouched (there are plans to change that as well, but not here).
Original motivation for this rule was based on the idea that we would have a child-with-unconfirmed-parents package relay protocol, and this would verify that the peer provided the "correct" package. For various reasons, we're not planning on doing this. We could potentially do this for ancestor packages (with a similar definition that all UTXOs to make the tx valid are available in this package), but it's also questionable whether it's useful to enforce this.
This rule gets in the way of certain usage of 1p1c package relay currently. If a transaction has multiple parents, of which only 1 requires a package CPFP, this rule blocks the package from relaying. Even if all the non-low-feerate parents are already in mempool, when the p2p logic submits the 1p1c package, it gets rejected for not meeting this rule.
ACKs for top commit:
ishaanam:
re-utACK ea17a9423f
instagibbs:
ACK ea17a9423f
Tree-SHA512: c2231761ae7b2acea10a96735e7a36c646f517964d0acb59bacbae1c5a1950e0223458b84c6d5ce008f0c1d53c1605df0fb3cd0064ee535ead006eb7c0fa625b
fac90e5261 test: Check that the GUI interactive reindex works (MarcoFalke)
faaaddaaf8 init: [gui] Avoid UB/crash in InitAndLoadChainstate (MarcoFalke)
Pull request description:
`InitAndLoadChainstate` is problematic, when called twice in the GUI. This can happen when it returns a failure and the user selects an interactive reindex.
There are several bugs that have been introduced since the last time this was working correctly:
* The first one is a crash (assertion failure), which happens due to a cached tip block in the notifiications from the previous run. See https://github.com/bitcoin/bitcoin/pull/31346#discussion_r2207914726
* The second one is UB (use-after-free), which happens because the block index db in the blockmanager is not reset. See https://github.com/bitcoin/bitcoin/pull/30965#discussion_r2207822121
Fix both bugs by resetting any dirty state in `InitAndLoadChainstate`.
Also, add a test, because I don't really want to keep testing this manually every time. (A failing test run can be seen in https://github.com/bitcoin/bitcoin/pull/32979/checks)
ACKs for top commit:
achow101:
ACK fac90e5261
TheCharlatan:
ACK fac90e5261
mzumsande:
Tested ACK fac90e5261
Tree-SHA512: 9f744d36e7cdd3f5871764386ec5a5cca1ae144f1bacc26c07e60313c2bdacdc5fca351aa185cb51359540eea4534dda17e4fb6073ad90f91ba0a6936faeead8
