Age | Commit message (Collapse) | Author |
|
warning for expected cases
|
|
last pruned block
From 0.14 (2017 Mar) until before 0.19 (2019 Nov), the height of the last
block pruned was returned, subject to a bug if there were blocks left unpruned
due to sharing files with later blocks.
In #15991, this was "fixed" to the current implementation, introducing a new
bug: now, it returns the first *unpruned* block.
Since the user provides the parameter as a block to include in pruning, it
makes more sense to fix the behaviour to match the documentation.
|
|
during IBD
48262a00f58489d705314ee3c31136133040bb0e Add functional test for block sync from inbound peers (Suhas Daftuar)
0569b5c4bbf8f725e3969d76f7cb081cdf1e4195 Sync chain more readily from inbound peers during IBD (Suhas Daftuar)
Pull request description:
When in IBD, if the honest chain is only known by inbound peers, then we must
eventually sync from them in order to learn it. This change allows us to
perform initial headers sync and fetch blocks from inbound peers, if we have no
blocks in flight.
The restriction on having no blocks in flight means that we will naturally
throttle our block downloads to any such inbound peers that we may be
downloading from, until we leave IBD. This is a tradeoff between preferring
outbound peers for most of our block download, versus making sure we always
eventually will get blocks we need that are only known by inbound peers even
during IBD, as otherwise we may be stuck in IBD indefinitely (which could have
cascading failure on the network, if a large fraction of the network managed to
get stuck in IBD).
Note that the test in the second commit fails on master, without the first commit.
ACKs for top commit:
ajtowns:
ACK 48262a00f58489d705314ee3c31136133040bb0e
sipa:
ACK 48262a00f58489d705314ee3c31136133040bb0e
Tree-SHA512: ffad3a05fa9a32a92226843c9128f52c275e8d51930fde7368badc340227f2ed680561c4c9f2937b4e3bd722474464849ec9b624f912f5e380ce98d71b55764d
|
|
|
|
This testcase exercises rule 5 of BIP-125 (no more than 100 evictions
due to replacement) without having to test under non-default mempool
parametmers.
|
|
|
|
|
|
1da5e45725a49a867f7ce16fb37b138ad329d132 test: use MiniWallet for feature_dbcrash.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (feature_dbcrash.py) to be run even with the Bitcoin Code wallet by using the MiniWallet instead, as proposed in https://github.com/bitcoin/bitcoin/issues/20078.
ACKs for top commit:
laanwj:
Code review ACK 1da5e45725a49a867f7ce16fb37b138ad329d132
brunoerg:
crACK 1da5e45725a49a867f7ce16fb37b138ad329d132
Tree-SHA512: 75ee9a32fd1451254004797d695d18032bd0fcb66ebd88cf737e147e43812525f6e884ec05fcc4f76f566dc71174c8ed7347bcdce16567db6511746ae64cead0
|
|
|
|
codespell in comments
f565b2836d5efeb6f7c16d0fac813b06fa4d41e4 Fixup option name in bench message (Ben Woosley)
bf209ac7a732394c3a54d6d1e3fb43f180ac1bb8 doc: Fix spelling errors identified by codespell in coments (Ben Woosley)
Pull request description:
From the output [here](https://cirrus-ci.com/task/5275612980969472?logs=lint#L849):
```
src/qt/test/addressbooktests.cpp:185: wilcard ==> wildcard
src/qt/test/addressbooktests.cpp:191: wilcard ==> wildcard
src/test/miniscript_tests.cpp:227: nd ==> and, 2nd
src/test/versionbits_tests.cpp:260: everytime ==> every time
src/util/time.h:89: precicion ==> precision
src/util/time.h:90: precicion ==> precision
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
~~I left the 'nd' in miniscript_tests as-is, as it's valid miniscript,
and I'm wary of whitelisting it.~~
ACKs for top commit:
dunxen:
ACK f565b28
Tree-SHA512: 501a426c5f6f9761e2c8f980d5d955611428a827321888f53e0ae9526b0fecd43f9d1fa845fc70ae2489d77be6dc0b5b371dff55c5146f4b39ed874f4a1ea917
|
|
|
|
sufficient chainwork
a35f963edf1a14ee572abea106fb0147575e4694 Add test for getheaders behavior (Suhas Daftuar)
ef6dbe6863d92710fd2da7781e5b2aac87578751 Respond to getheaders if we have sufficient chainwork (Suhas Daftuar)
Pull request description:
Previously, we would check to see if we were in IBD and ignore getheaders requests accordingly. However, the IBD criteria -- an optimization mostly targeted at behavior when we have peers serving us many blocks we need to download -- is difficult to reason about in edge-case scenarios, such as if the network were to go a long time without any blocks found and nodes are getting restarted during that time.
To make things simpler to reason about, just use `nMinimumChainWork` as our anti-DoS threshold for responding to a getheaders request; as long as our chain has that much work, it should be fine to respond to a peer asking for our headers (and this should allow such a peer to request blocks from us if needed).
ACKs for top commit:
klementtan:
crACK a35f963edf1a14ee572abea106fb0147575e4694
naumenkogs:
ACK a35f963edf1a14ee572abea106fb0147575e4694
MarcoFalke:
review ACK a35f963edf1a14ee572abea106fb0147575e4694 🗯
Tree-SHA512: 131e3872e7fe80382ea9c1ec202d6c2dc59c006355c69000aa3f4ce6bccd02a6c689c8cb8f3542b5d9bc48bfa61edcbd1a78535c0b79018971d02bed2655d284
|
|
e8959000b63db4f2a21579fd4be27618c5fbd5b9 test: Use MiniWallet in rpc_rawtransaction.py (Daniela Brozzoni)
e93046c10b4d4e139cd7b41791ad1bfe925351e2 MOVEONLY: Move signrawtransactionwithwallet test (Daniela Brozzoni)
Pull request description:
This PR allows `rpc_rawtransaction.py` to be run even without the Core wallet by using the MiniWallet instead, as proposed in https://github.com/bitcoin/bitcoin/issues/20078.
This test was previously run twice, once with `--legacy-wallet` and once with
`--descriptors`. Since this would have meant running the same test twice
if the wallet wasn't compiled, now we run it just once with the legacy
wallet.
ACKs for top commit:
jonatack:
ACK e8959000b63db4f2a21579fd4be27618c5fbd5b9
Tree-SHA512: d1580570a54dad8e30a5df1ab7d03ecb3f824efe6843323e1f3aef63592045d823c7d54fc86321dc7c1d414854a253431a01a7baa9f30426ea9a09ef11ae3a04
|
|
This test was previously run twice, once with `--legacy-wallet` and once with
`--descriptors`.
Now we run it only with `--legacy-wallet`, as all the tests has been
ported to the MiniWallet but `raw_multisig_transaction_legacy_tests`,
which can be run only with the legacy wallet.
We also decrease the number of nodes used from 4 to 3, making the test
run slightly faster.
|
|
Put signrawtransactionwithwallet_tests in rpc_signrawtransaction.py,
as the test is mainly testing the signrawtransaction RPC.
Review with `git show --color-moved=dimmed-zebra`
|
|
And some little type annotation additions.
|
|
|
|
specific prevouts
418557034055f740951294e7677ae9fd5149ea9b Add RPC to get mempool txs spending outputs (t-bast)
Pull request description:
We add an RPC to fetch mempool transactions spending any of the given outpoints.
Without this RPC, application developers need to first call `getrawmempool` which returns a long list of `txid`, then fetch each of these transactions individually (`getrawtransaction`) to check whether they spend the given outpoints, which wastes a lot of bandwidth (in the worst case we need to transfer the whole mempool).
For example in lightning, when we discover that one of our channel funding transactions has been spent, we need to find the spending transaction to claim our outputs from it. We are currently forced to fetch the whole mempool to do the analysis ourselves, which is quite costly.
I believe that this RPC is also generally useful when doing some introspection on your mempool after one of your transactions failed to broadcast, for example when you implement RBF at the application level. Fetching and analyzing the conflicting transaction gives you more information to successfully replace it.
ACKs for top commit:
darosior:
re-utACK 418557034055f740951294e7677ae9fd5149ea9b
vincenzopalazzo:
re-ACK https://github.com/bitcoin/bitcoin/pull/24408/commits/418557034055f740951294e7677ae9fd5149ea9b
danielabrozzoni:
re-tACK 418557034055f740951294e7677ae9fd5149ea9b
w0xlt:
reACK https://github.com/bitcoin/bitcoin/pull/24408/commits/418557034055f740951294e7677ae9fd5149ea9b
Tree-SHA512: 206687efb720308b7e0b6cf16dd0a994006c0b5a290c8eb386917a80130973a6356d0d5cae1c63a01bb29e066dd721594969db106cba7249214fcac90d2c3dbc
|
|
and add WalletLogPrintf() (already detected) to the lint-logs.py suggestion
Co-authored-by: laanwj <126646+laanwj@users.noreply.github.com>
|
|
-blockfilterindex
295ff61934f8adea04dabb47695070e2cfd0548e test: add coverage for unknown -blockfilterindex (brunoerg)
Pull request description:
This PR adds test coverage for the following init error:
https://github.com/bitcoin/bitcoin/blob/44037a29129a830fd9c9580f0818387756cfd7d3/src/init.cpp#L844
Passing an unknown value to -blockfilterindex should throw an error.
ACKs for top commit:
dunxen:
cr-ACK 295ff61
Tree-SHA512: 1444903cf0696406c485ce0575f951d527fe7d699094d5845622c0b57c954d6d7dcf1e78ef0c4e8b9b26f53b79583f07fec0e8d8e7f04aa744d2a8cd98329db9
|
|
From the output here:
src/qt/test/addressbooktests.cpp:185: wilcard ==> wildcard
src/qt/test/addressbooktests.cpp:191: wilcard ==> wildcard
src/test/miniscript_tests.cpp:227: nd ==> and, 2nd
src/test/versionbits_tests.cpp:260: everytime ==> every time
src/util/time.h:89: precicion ==> precision
src/util/time.h:90: precicion ==> precision
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
https://cirrus-ci.com/task/5275612980969472?logs=lint#L849
I added 'nd' to the spelling.ignored-words.txt, as it's valid miniscript.
|
|
`GetUTXOStats` codepaths, decouple from `coinstatsindex`
664a14ba7ccb40aa82d35a59831acd35db1897a6 coinstats: Move GetUTXOStats to rpc/blockchain (Carl Dong)
f1006875665ffe8ff5da8185effe25b860743b4e kernel: Use ComputeUTXOStats in validation (Carl Dong)
faa52387e8e4856445b1cfc9b5e072ce8f690f36 style-only: Rearrange using decls after scripted-diff (Carl Dong)
f329a9298c06ffe74b9e9fbc07bfe6d282fef9cb scripted-diff: Move src/kernel/coinstats to kernel:: (Carl Dong)
0e54456f0498e52131f8ae0c76b4dfe25f45b076 Use only kernel/coinstats.h in index/coinstatsindex.h (Carl Dong)
80970985c965f79b8c376c8a922497e385445dd8 coinstats: Split node/coinstats.h to kernel/coinstats.h (Carl Dong)
35f73ce4b2efd7341fe55f77b334f27ad8aad090 coinstats: Move hasher codepath to kernel/coinstats (Carl Dong)
b7634fe02b6b030f5d62502c73db84ba9a276640 Move logic from LookupUTXOStatsWithIndex to CoinStatsIndex::LookUpStats (Carl Dong)
1352e410a5b84070279ff28399083cb3ab278593 coinstats: Separate hasher/index lookup codepaths (Carl Dong)
524463daf6a10b20a4e20116a68101a684929eda coinstats: Return purely out-param CCoinsStats (Carl Dong)
46eb9fc56a296a2acea10ec7e5bf7b1827f73c45 coinstats: Extract index_requested in-member to in-param (Carl Dong)
a789f3f2b878e1236f8e043a8bb1ffb1afc1b673 coinstats: Extract hash_type in-member to in-param (Carl Dong)
102294898d708b7adc0150aba8e500a4aa19bc1c includes: Remove rpc/util.h -> node/coinstats.h (Carl Dong)
0848db9c35d9eae4d68cbdbef68c337656f3c906 fuzz: Remove useless GetUTXOStats fuzz case (Carl Dong)
52b1939993771d0a8a718ca1667241872de8241a kernel: Remove unnecessary blockfilter{index,}.cpp (Carl Dong)
Pull request description:
Part of: #24303
Depends on: #24322
The `GetUTXOStats` function has 2 codepaths:
- One which queries the `CoinStatsIndex` for the UTXO hash
- One which actually performs the hashing
For `libbitcoinkernel`, the only place where we call `GetUTXOStats` is in `PopulateAndValidateSnapshots`, which uses the `SHA256D` hash, and is therefore unable to use the `CoinStatsIndex` since that only provides `MuHash` hashes. Not that I think indices necessarily belong in `libbitcoinkernel` anyway.
This PR separates these 2 aforementioned codepaths of `GetUTXOStats`, uses the hashing codepath in `PopulateAndValidateSnapshots`, and removes the need to link in `index/coinstatsindex.cpp` and `node/coinstats.cpp`.
-----
Logistically, this PR:
- Extracts out the `index_requested` and `hash_type` members of `CoinStats`, which served as "in-params" to `GetUTXOStats` embedded within the `CoinStats` struct. This allows `CoinStats` to only consist of "out-param" members, and be returned by `GetUTXOStats` without needing to be an "in-out" param
- Introduce the purely virtual `UTXOHashers` class, with 3 implementations: `SHA256DHasher`, `MuHashHasher`, and `NullHasher`. These replace the existing template-based polymorphism.
- Split `GetUTXOStats` into:
- `CalculateUTXOStatsWithHasher(UTXOHasher&, ...)`, and
- `LookupUTXOStatsWithIndex(CoinStatsIndex&, ...)`
- Use `CalculateUTXOStatsWithHasher` directly where appropriate (`src/validation.cpp` and `src/fuzz`)
- Move `GetUTXOStats` to `rpc/blockchain`, which is the only place that depends on `GetUTXOStats`'s weird fallback behaviour
- Move `LookupUTXOStatsWithIndex` to `index/coinstatsindex`
Code organization:
- `src/`
- `kernel/` → only contains the hashing codepath
- `coinstats.cpp` → hashing codepath implementations
- `coinstats.h` → header for `kernel/coinstats.cpp`
- `index/` → only contains the index codepath
- `coinstatsindex.cpp` → index codepath implementations
- `coinstatsindex.h`
- `validation.cpp` → only uses the hashing codepath
- `rpc/blockchain.cpp` → uses both the hashing and index codepath, old `GetUTXOStats` fallback logic moved here as static
- `test/fuzz/coins_view.cpp` → only uses the hashing codepath
TODOs:
- [x] Commit messages could be fleshed out more
Would love any feedback!
ACKs for top commit:
laanwj:
Code review ACK 664a14ba7ccb40aa82d35a59831acd35db1897a6
Tree-SHA512: 18722c7bd279174d2d1881fec33ea04a9b261aae1c12e998cf434ef297d8ded47de69c526c8033a2ba7abc93ba3d2ff5faf4ce05e8888c725c31cf885ce3ef73
|
|
|
|
This is the "fruit of our labor" for this patchset.
ChainstateManager::PopulateAndValidateSnapshot can now directly call
ComputeUTXOStats(...).
Our consensus engine is now fully decoupled from all indices.
See the src/Makefile.am for some satisfying removals.
|
|
Removes a circular dependency, horray!
|
|
908fb7e2ec37fe68675d38dbfee4df9f861bb2b5 test: Use permissions from git in `lint-files.py` (laanwj)
48d2e80a7479a44b0ab09e87542c8cb7a8f72223 test: Don't use shell=True in `lint-files.py` (laanwj)
Pull request description:
Improvements to the `lint-files.py` script:
- Avoid use of `shell=True`.
- Check the permissions in git's metadata instead of in the filesystem. This stops the umask or filesystem from interfering. It's also more efficient as it only needs a single call to `git ls-files`.
(what triggered this change was `File "..." contains a shebang line, but has the file permission 775 instead of the expected executable permission 755.` errors running the script locally).
ACKs for top commit:
vincenzopalazzo:
re-tACK https://github.com/bitcoin/bitcoin/commit/908fb7e2ec37fe68675d38dbfee4df9f861bb2b5
Tree-SHA512: 2eaf868c55a9c3508b12658a5b3ac429963fd0551e645332d0ac54be56fefccee95115e4667386df24b46b545593cb0d0bf8c6cecab73f9cb19d37ddf704c614
|
|
addresses were found in the address book
baa3ddc49c46d00e3e0de06e494656f0f00b0ee8 doc: add release notes about `getreceivedbylabel` returning an error if the label is not in the address book. (furszy)
8897a21658ad93f7b628eb2a3411fec2265d73fb rpc: getreceivedbylabel, don't loop over the entire wallet txs map if no destinations were found for the input label. (furszy)
Pull request description:
Built on top of #23662, coming from comment https://github.com/bitcoin/bitcoin/pull/23662#pullrequestreview-971407999.
If `wallet.GetLabelAddresses()` returns an empty vector (the wallet does not have stored destinations with that label in the addressbook) or if none of the returned destinations are from the wallet, we can return the function right away.
Otherwise, we are walking through all the wallet txs + outputs for no reason (`output_scripts` is empty).
ACKs for top commit:
achow101:
ACK baa3ddc49c46d00e3e0de06e494656f0f00b0ee8
theStack:
re-ACK baa3ddc49c46d00e3e0de06e494656f0f00b0ee8
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/25122/commits/baa3ddc49c46d00e3e0de06e494656f0f00b0ee8
Tree-SHA512: 00e10365b179bf008da2f3ef8fbb3ee04a330426374020e3f2d0151b16991baba4ef2b944e4659452f3e4d6cb20f128d0918ddf0453933a25a4d9fd8414a1911
|
|
Instead of using permissions from the local file system, which might
depend on the umask, directly check the permissions from git's metadata.
|
|
destinations were found for the input label.
If wallet.GetLabelAddresses() returns an empty vector (the wallet does not have addresses with that label in the addressbook) or if none of the returned destinations are from the wallet, we can return the function right away.
|
|
`-deprecatedrpc=exclude_coinbase` logic
a4703ce9d79855ac0bd7dc07b71a51245f9aa5f8 doc: add release notes about removal of the `deprecatedrpc=exclude_coinbase` (Sebastian Falbesoner)
ef0aa74836c4339aa7f14fc1c9583d86dd5c388a rpc: wallet: remove `-deprecatedrpc=exclude_coinbase` logic (Sebastian Falbesoner)
Pull request description:
Including coinbase transactions in `receivedby` RPCs and adding the `-deprecatedrpc=exclude_coinbase` was done in PR #14707 (released in v23.0). For the next release v24.0, this configuration option can be removed.
ACKs for top commit:
fanquake:
ACK a4703ce9d79855ac0bd7dc07b71a51245f9aa5f8
Tree-SHA512: 97cd4e78501e64f678c78d2ebb5be5376688c023e34fced71dd24e432d27aa31a74b5483545f49ba0bdf48656d8b8b7bee74e3db26cf6daf112613f1caa4dfa4
|
|
1d4122dfefcb0a33c3d5bf7bbe2c7cd7e09d3764 init: Allow -proxy="" setting values (Ryan Ofsky)
Pull request description:
This drops the `No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>` error when a empty `-proxy=` command line argument, `bitcoin.conf` value, or `settings.json` value is specified, and just makes bitcoin connect and listen normally in these cases.
The error was originally added in https://github.com/bitcoin/bitcoin/pull/20003 to prevent a bare `-proxy` command line argument with no assignment from clearing proxy settings. But it was implemented in an overbroad way breaking empty `-proxy=` assignments as well.
The motivation for this change is to prevent a GUI bug that happens with https://github.com/bitcoin/bitcoin/pull/15936, reported in https://github.com/bitcoin/bitcoin/pull/15936#pullrequestreview-937685759 by vasild, that happens after a proxy setting is enabled and disabled in the GUI. But this change also makes sense on its own to remove a potentially confusing error message.
ACKs for top commit:
hebasto:
re-ACK 1d4122dfefcb0a33c3d5bf7bbe2c7cd7e09d3764, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/24830#pullrequestreview-941255672).
Tree-SHA512: 753adfce199ed078a6cd9e0ea78e76c0b14070f8fcfe2a4632cd0c6dfe6b4e135ddffbe11a97e5e30520ea9e5bda00bad1493cbaef74cf425aa8613249167f53
|
|
055d94d1ab4937ed0080459fbe63568dc47b6786 test: add coverage for unknown network in -onlynet (brunoerg)
Pull request description:
This PR adds test coverage for the following init error by passing an unknown network in -onlynet
https://github.com/bitcoin/bitcoin/blob/0de36941eca1bff91420dd878eb097db2b1a596c/src/init.cpp#L1311
ACKs for top commit:
MarcoFalke:
rACK 055d94d1ab4937ed0080459fbe63568dc47b6786
Tree-SHA512: 01bbb297afff371f6345889fa04117ff195b68f0bbf934878ba446049791fdbd7d2ce119ee4f9b3616cc0a81330d7055507dc81151acf68532c077f3575258e9
|
|
|
|
This test can now be run even with the Bitcoin Core wallet disabled.
|
|
|
|
support for v1 compact blocks)
bf6526f4a0ab30f77952ecdee90cd77dd9ba06f6 [test] Remove segwit argument from build_block_on_tip() (John Newbery)
c65bf50b44a38bc55224d8967e0df7af60ea4f1b Remove fUseWTXID parameter from CBlockHeaderAndShortTxIDs constructor (John Newbery)
Pull request description:
This implements two of the suggestions from code reviews of PR 20799:
- Remove fUseWTXID parameter from CBlockHeaderAndShortTxIDs constructor
- Remove segwit argument from build_block_on_tip()
ACKs for top commit:
dergoegge:
Code review ACK bf6526f4a0ab30f77952ecdee90cd77dd9ba06f6
naumenkogs:
ACK bf6526f4a0ab30f77952ecdee90cd77dd9ba06f6
Tree-SHA512: d553791d1364b9e655183755e829b195c9b47f59c62371dbae49d9c0f8d84fec58cf18f4dde89591672ef5658e18c9cf0206c2efd70606980f87e506bc3bd4e5
|
|
a runtime flag
b953ea6cc691ba61bf08eb186e76e7e8b7ba8120 rpc: Put undocumented JSON failure mode behind a runtime flag (Suhail Saqan)
Pull request description:
Fixes #24695 (Put undocumented JSON failure mode behind a runtime flag)
ACKs for top commit:
luke-jr:
utACK b953ea6cc691ba61bf08eb186e76e7e8b7ba8120
vincenzopalazzo:
ACK https://github.com/bitcoin/bitcoin/pull/25161/commits/b953ea6cc691ba61bf08eb186e76e7e8b7ba8120
Tree-SHA512: 2005ee1b1f3b637918390b2ecd4166f2fd8c86e3c59fba3da8a0cbd5b1dffd03190c92f6dca3c489ecce4276eaf3108b2edcf9cd6224b713adb52f5bb848163b
|
|
rpc: Put undocumented JSON failure mode behind a runtime flag
|
|
MESSAGEMAP)
5dc6d9207778c51c10c16fac4b3663bc7905bafc test: make BIP157 messages default-constructible (MESSAGEMAP compatibility) (Sebastian Falbesoner)
71e4cfefe765c58937b3fd3125782ca8407315d2 test: p2p: add missing BIP157 message types to MESSAGEMAP (Sebastian Falbesoner)
Pull request description:
The script [message-capture-parser.py](https://github.com/bitcoin/bitcoin/blob/master/contrib/message-capture/message-capture-parser.py) currently doesn't support parsing the BIP157 messages `getcfilters`, `getcfheaders` and `getcfcheckpt`, e.g.
```
$ ./contrib/message-capture/message-capture-parser.py msgs_recv.dat
...
WARNING - Unrecognized message type b'getcfcheckpt' in /home/thestack/bitcoin/msgs_recv.dat
...
```
This PR fixes this by adding the missing message type mappings to the [`MESSAGEMAP`](https://github.com/bitcoin/bitcoin/blob/225e5b57b2ee2bc1acd7f09c89ccccc15ef8c85f/test/functional/test_framework/p2p.py#L95-L127) in the test framework and add default-constructors for the corresponding `msg_`... classes.
Without the second commit, the following error message would occur:
```
File "/home/thestack/bitcoin/./contrib/message-capture/message-capture-parser.py", line 141, in process_file
msg = MESSAGEMAP[msgtype]()
TypeError: __init__() missing 2 required positional arguments: 'filter_type' and 'stop_hash'
```
ACKs for top commit:
dunxen:
tACK [5dc6d92](https://github.com/bitcoin/bitcoin/pull/25126/commits/5dc6d9207778c51c10c16fac4b3663bc7905bafc)
Tree-SHA512: d656c4d38a856373f01d7c293ae7d2b27378a9fc248048ebf2a64725ef8b498b3ddf4f420704abdb20d0c68ca548f1777602c5e73b66821a20c97ae618f1d63f
|
|
p2p_unrequested_blocks.py
faac67cab02a755b0ce67716c5e5889432b13b83 test: Fix intermittent race in p2p_unrequested_blocks.py (MacroFake)
Pull request description:
Disconnect may also result in an `OSError`, not only an `AssertionError`. Instead of maintaining a dead code path and enumerating disconnect reasons, just assume disconnection happens every time.
ACKs for top commit:
jamesob:
Code review ACK https://github.com/bitcoin/bitcoin/pull/25124/commits/faac67cab02a755b0ce67716c5e5889432b13b83
Tree-SHA512: d2cec003168e421a5faed275cb2e1ef9fc63f9e8514f41d21da17e8964c79e5b453ccd72cd7ec62805f45293cf877be5bc8124ae98a515c0aa42d6e053409653
|
|
The only place that segwit=True is for a block that contains only the
coinbase transaction. Since the witness commitment is optional if none
of the transactions have a witness, we can leave it out. This doesn't
change the test coverage, which is testing p2p compact block logic.
Suggested in https://github.com/bitcoin/bitcoin/pull/20799#discussion_r867782119
|
|
|
|
Fixes #25129 (subtractfeefromamount=true fails with insufficient
funds)
|
|
ada8358ef54aaa04c9182afe115d8046c801bdde Sanitize port in `addpeeraddress()` (amadeuszpawlik)
Pull request description:
In connection to #22087, it has been [pointed out](https://github.com/bitcoin/bitcoin/pull/22087#pullrequestreview-674786285) that `addpeeraddress` needs to get its port-value sanitized.
ACKs for top commit:
fanquake:
ACK ada8358ef54aaa04c9182afe115d8046c801bdde
Tree-SHA512: 48771cd4f6940aa7840fa23488565c09dea86bd5ec5a5a1fc0374afb4857aebcd2a1f51e2d4cb7348460e0ad9793dc5d2962df457084ed2b8d8142cae650003f
|
|
|
|
Subsequent commits will remove support for other versions of compact blocks.
Add a test that a received `sendcmpct` message with version = 1 is
ignored.
|
|
Subsequent commits will remove support.
|
|
Support for version 1 is removed in the following commits.
|
|
4faa5500724e3b1423ce1f927236b1ab1ac07943 test: Fix race condition in index pruning test (Fabian Jahr)
Pull request description:
Fixes #25031
The `feature_index_prune.py` test seems to be racy because connections are reestablished after restarts and the blocks are synced via the `sync_blocks` function. The `sync_blocks` function has a sanity check at the beginning to check that all nodes in the set have at least one established connection and that is not always the case.
As a solution nodes are not connected via the `-connect` parameter on start but instead via the `connect_nodes` helper.
Top commit has no ACKs.
Tree-SHA512: f88377715f455f1620725fe8ebd6b486fa0209660b193bf68d1ce1452e2086ac5d169d8ca4c2b61443566232e96fb9c6386ee482bc546cce38078d72e7c3c29f
|
|
Nodes are restarted and reconnected as part of the test. Afterwards
`sync_blocks` is called immediately on the nodes. `sync_blocks`
first checks that all the included nodes have at least one
connection. Since adding a connection is usually happening in a
thread, sometimes nodes could run into this check before the
connection was fully established so that it would fail the entire
test.
This fix uses the `connect_nodes` helper to make the connection the
nodes. `connect_nodes` has a wait for the connection built into it.
|