3472e2f5ec Merge sipa/minisketch#81: Avoid overflowing shift by special casing inverse of 1
653d8b2e26 Avoid overflowing shift by special casing inverse of 1
33b7c200b9 Merge sipa/minisketch#80: Add c++20 version of CountBits
4a48f31a37 Merge sipa/minisketch#83: ci: Fix "s390x (big-endian)" task
82b6488acb Add c++20 version of CountBits
0498084d31 ci: Fix "s390x (big-endian)" task
71709dca9e Merge sipa/minisketch#82: ci: Fix `x86_64-w64-mingw32` task
9e6127fa98 Merge sipa/minisketch#74: Avoid >> above type width in BitWriter
ed420bc170 ci: Fix `x86_64-w64-mingw32` task
fe1040f227 Drop -Wno-shift-count-overflow compile flag
154bcd43bd Avoid >> above type width in BitWriter
67b87acdb6 Merge sipa/minisketch#78: ci: Update macOS image for CI
7de7250416 ci: Update macOS image for CI
83d812ea9f Merge sipa/minisketch#73: ci: Use correct variable to designate C++ compiler
e051a7d690 ci: Install wine32 package for Windows tests
2d2c695d78 build: Drop unused `CC` variable
1810fcbd11 ci: Use correct variable to designate C++ compiler
022b959049 Merge sipa/minisketch#77: Add missing include
08443c4892 Add missing include
git-subtree-dir: src/minisketch
git-subtree-split: 3472e2f5ec75ace39ce9243af6b3fee233a67492
4ba1d0b553 fuzz: Add coverage for client_maxfeerate (Greg Sanders)
91d7d8f22a AcceptMultipleTransactions: Fix workspace client_maxfeerate (Greg Sanders)
f3aa5bd5eb fill_mempool: assertions and docsctring update (Greg Sanders)
a3da63e8fe Move fill_mempool to util function (Greg Sanders)
73b68bd8b4 fill_mempool: remove subtest-specific comment (Greg Sanders)
Pull request description:
Bug causes an `Assume()` failure due to the expectation that the individual result should be invalid when done over `submitpackage` via rpc.
Bug introduced by https://github.com/bitcoin/bitcoin/pull/28950 , and I discovered it rebasing https://github.com/bitcoin/bitcoin/pull/28984 since it's easier to hit in that test scenario.
Tests in place were only checking `AcceptSingleTransaction`-level checks due to package evaluation only triggering when minfee is too high for the parent transaction.
Added test along with fix, moving the fill_mempool utility into a common area for re-use.
ACKs for top commit:
glozow:
reACK 4ba1d0b553
theStack:
ACK 4ba1d0b553
ismaelsadeeq:
re-ACK 4ba1d0b553 via [diff](4fe7d150eb..4ba1d0b553)
Tree-SHA512: 3729bdf7f25d04e232f173ccee04ddbb2afdaafa3d04292a01cecf58fb11b3b2bc133e8490277f1a67622b62d17929c242dc980f9bb647896beea4332ee35306
Limit number of IPs learned from a single DNS seed to 32, to prevent the results from
one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is
bounded to 33 already, but it is possible for it to use TCP where a potentially enormous
number of results can be returned.
Closes #16070.
This helper class is an alternative to CMedianFilter, but without a
lot of the special logic and exceptions that we needed while it was
still used for consensus.
d5a715536e build: remove boost::process dependency for building external signer support (Sebastian Falbesoner)
70434b1c44 external_signer: replace boost::process with cpp-subprocess (Sebastian Falbesoner)
cc8b9875b1 Add `cpp-subprocess` header-only library (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/24907.
This PR is based on **theStack**'s [work](https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1466087049).
The `subprocess.hpp` header has been sourced from the [upstream repo](https://github.com/arun11299/cpp-subprocess) with the only modification being the removal of convenience functions, which are not utilized in our codebase.
Windows-related changes will be addressed in subsequent follow-ups.
ACKs for top commit:
achow101:
reACK d5a715536e
Sjors:
re-tACK d5a715536e
theStack:
Light re-ACK d5a715536e
fanquake:
ACK d5a715536e - with the expectation that this code is going to be maintained as our own. Next PRs should:
Tree-SHA512: d7fb6fecc3f5792496204190afb7d85b3e207b858fb1a75efe483c05260843b81b27d14b299323bb667c990e87a07197059afea3796cf218ed8b614086bd3611
a71eadf66b Change MAC_OSX macro to __APPLE__ in crypto package (Lőrinc)
Pull request description:
Split out from https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2044405345 to avoid the uncertainties and simplify review.
ACKs for top commit:
theuni:
ACK a71eadf66b
fanquake:
ACK a71eadf66b
Tree-SHA512: b6a7bd7ca95585dd9110cefe7c1213f4a1a72bdfc88670abf4a0d9a8bbc12e093544524adce46aa9ca714c472d417f74ca4a678af682ed2488053059434eaa02
a8203e9412 refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair<Wtxid, CTransactionRef> (AngusP)
c3c18433ae refactor: Use typesafe Wtxid in compact block encoding message, instead of ambiguous uint256. (AngusP)
Pull request description:
The first commit replaces `uint256` with typesafe `Wtxid` (or `Txid`) types introduced in #28107.
The second commit then simplifies the extra tx `vector` to just be of `CTransactionRef`s instead of a `std::pair<Wtxid, CTransactionRef>`, as it's easy to get a `Wtxid` from a transaction ref.
ACKs for top commit:
glozow:
ACK a8203e9412
dergoegge:
ACK a8203e9412
Tree-SHA512: b4ba1423a8059f9fc118782bd8a668213d229c822f22b01267de74d6ea97fe4f2aad46b5da7c0178ecc9905293e9a0eafba1d75330297c055e27fd53c8c8ebfd
03b87a3e64 Drop Windows Socket dependency for `randomenv.cpp` (Hennadii Stepanov)
Pull request description:
This change drops a dependency on the ws2_32 library for our libbitcoinkernel by switching to [`GetComputerName`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getcomputernamew) function.
ACKs for top commit:
sipsorcery:
utACK 03b87a3e64.
laanwj:
Code review ACK 03b87a3e64.
fanquake:
ACK 03b87a3e64
Tree-SHA512: a4abd5499176634d5f3fbf4e794a7504c40232fb73bd7f41955fbfb2cc7c44bc7ea4518c5203836e52f552c30414c6c3e1b24f0922641dbf1c8377981c0ffaf0
4f273ab436 Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us (Luke Dashjr)
Pull request description:
To avoid issues with DNS blacklisting, I've setup a separate domain for my DNS seed.
(This time, without a potentially alarming name)
ACKs for top commit:
kevkevinpal:
Concept ACK [4f273ab](4f273ab436), name looks good to me
petertodd:
ACK 4f273ab436
mzumsande:
ACK 4f273ab436
fanquake:
ACK 4f273ab436
Tree-SHA512: 689698e3c735df3ed0c2756a9d4adb5644bb9d8a6954e23d66bfa9d94ee10954f77fb241d9593f750054d731aa1532368a0fc8277884f6c2a98ac47cd0bdeeb7
2d1819455c crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc's (Cory Fields)
Pull request description:
Looking at libc sources, apple and openbsd implementations match our naive fallback. Only FreeBSD (and only x86_64) seems to [implement an optimized version](https://github.com/freebsd/freebsd-src/blob/main/lib/libc/amd64/string/timingsafe_bcmp.S).
It's not worth the hassle of using a platform-specific function for such little gain.
Additionally, as mentioned below, this is the only case outside of sha2 that requires an autoconf check, and I have upcoming PRs to remove the sha2 ones.
Apple's [impl is unoptimized](https://opensource.apple.com/source/Libc/Libc-1244.1.7/string/FreeBSD/timingsafe_bcmp.c.auto.html).
As-is [OpenBSD's impl](https://github.com/openbsd/src/blob/master/lib/libc/string/timingsafe_bcmp.c).
Relevant IRC conversation with sipa:
> \<cfields\> sipa: chacha20poly1305.cpp uses libc's timingsafe_bcmp when possible. But looking around at apple/freebsd/openbsd, I don't see any impl that doesn't use the naive implementation that matches our fallback...
> \<cfields\> is there any reason to belive there's an optimized impl somewhere that we're actually hitting?
> \<cfields\> asking because after cleaning up sha2, timingsafe_bcmp is the last autoconf check that remains in all of crypto. It'd make life easy if we could just always use our internal one.
> \<cfields\> *all of crypto/
> \<sipa\> cfields: let's get rid of the dependency then
> \<sipa\> it's a trivial function
> \<sipa\> and if we need it for some platforms, no real reason not to use it on all
After the above discusstion, I did end up finding the x86_64-optimized FreeBSD impl, but I don't think that's all that significant.
ACKs for top commit:
sipa:
utACK 2d1819455c
fanquake:
ACK 2d1819455c
TheCharlatan:
ACK 2d1819455c
theStack:
ACK 2d1819455c
Tree-SHA512: b9583e19ac2f77c5d572aa5b95bc4b53669d5717e5708babef930644980de7c5d06a9c7decd5c2b559d70b8597328ecfe513375e3d8c3ef523db80012dfe9266
All `CTransactionRef` have `.GetWitnessHash()` that returns a cached `const Wtxid` (since fac1223a56),
so we don't need to pass transaction refs around with their IDs as they're easy to get from a ref.
561a650e0f test: Fix debug recommendation in argsman_tests (Fabian Jahr)
Pull request description:
There are recommendations in the `argsman_tests` comments on how to re-run and debug a test failure to see if it reflects an expected or unexpected change. The command tries to run a test in `util_tests` but this is in `argsman_tests` so the command doesn't work with just copy+paste. I didn't investigate further but I suspect that these tests were moved between files.
ACKs for top commit:
fanquake:
ACK 561a650e0f
Tree-SHA512: b3bb94ba1635c9455149b455f2b30ee37a8067a6242339531ab54d428177a288da29a4a10702652305eb34aa7638f51dad35fa6b0e7b74617e445327b8c4c053
fa9f36baba build: Remove HAVE_GMTIME_R (MarcoFalke)
fa72dcbfa5 refactor: FormatISO8601* without gmtime* (MarcoFalke)
fa2c486afc Revert "time: add runtime sanity check" (MarcoFalke)
Pull request description:
Now that the `ChronoSanityCheck` has passed for everyone with C++17 and is guaranteed by C++20 to always pass, remove it.
Also, remove `gmtime_r` and `gmtime_s` and replace them with `year_month_day`+`hh_mm_ss` from C++20.
ACKs for top commit:
sipa:
utACK fa9f36baba
fanquake:
ACK fa9f36baba - more std lib & even less stuff to port.
Tree-SHA512: a9e7e805b757b7dade0bcc3f95273a7dc4f68622630d74838339789dd203ad7542d36b2e090a93b2bc5a7ecc383207dd7ec82c68147108bdac7ce44f088c8c9a
Looking at apple/freebsd/openbsd sources, their implementations match our naive
fallback. It's not worth the hassle of using a platform-specific function for
no gain.
bbe82c116e Fix #29767, set m_synced = true after Commit() (nanlour)
Pull request description:
I think this problem https://github.com/bitcoin/bitcoin/issues/29767#issue-2216373048 is because of
in BaseIndex::Sync
61de64df67/src/index/base.cpp (L163-L168)
Setup m_synced = true; before Commit();
So this may cause a race condition window to BaseIndex::BlockConnected
61de64df67/src/index/base.cpp (L271-L274)
So i try to fix it with move m_synced = true after Commit().
Also see comment of Sync():
61de64df67/src/index/base.h (L151-L156)
I am a newcomer interested in Bitcoin, trying to become a member of the Bitcoin Core development team. Please give me some feedback if you could, as I may be doing something wrong. Thank you!
ACKs for top commit:
fjahr:
Code review ACK bbe82c116e
ryanofsky:
Code review ACK bbe82c116e
Tree-SHA512: 89a09498a232c87ef1e083d4cc4ed9bb15f045ad0624d5d150a87187b2b8a48a41137974dbc7ea5c37f73da90742c43259f5aa7f84b4179eb8d62033e44fa479
b0344c219a logging: remove unused BCLog::UTIL (Vasil Dimov)
d3b3af9034 log: deduplicate category names and improve logging.cpp (Vasil Dimov)
Pull request description:
The code in `logging.cpp` needs to:
* Get the category name given the flag (e.g. `BCLog::PRUNE` -> `"prune"`)
* Get the flag given the category name (e.g. `"prune"` -> `BCLog::PRUNE`)
* Get the list of category names sorted in alphabetical order
Achieve this by using the proper std containers. The result is
* less code (the diff of the first commit is +62 / -129)
* faster code (to linear search and no copy+sort)
* more maintainable code (the categories are no longer duplicated in `LogCategories[]` and `LogCategoryToStr()`)
This behavior is preserved:
`BCLog::NONE` -> `""` (lookup by `LogCategoryToStr()`)
`""` -> `BCLog::ALL` (lookup by `GetLogCategory("")`)
---
Also remove unused `BCLog::UTIL`.
---
These changes (modulo the `BCLog::UTIL` removal) are part of https://github.com/bitcoin/bitcoin/pull/29415 but they make sense on their own and would be good to have them, regardless of the fate of https://github.com/bitcoin/bitcoin/pull/29415. Also, if this is merged, that would reduce the size of https://github.com/bitcoin/bitcoin/pull/29415, thus the current standalone PR.
ACKs for top commit:
davidgumberg:
crACK b0344c219a
pinheadmz:
ACK b0344c219a
ryanofsky:
Code review ACK b0344c219a. Nice cleanup! Having to maintain multiple copies of the same mapping seemed messy and a like a possible footgun. I checked old and new mappings in both directions and confirmed no behavior should be changing.
Tree-SHA512: 57f87a090932f9b33dc8e075d1855dba9b71a3243a0758511745483dec2d9c46d3b532eadab297e78164c9b7caba370986ee380696a45f0778a841082f8e21a7
80f8b92f4f remove libbitcoinconsensus (fanquake)
Pull request description:
This was deprecated in `v27.0`, for removal in `v28.0`. See discussion in PR #29189.
ACKs for top commit:
theuni:
Concept ACK and light review ACK 80f8b92f4f. My only hesitation here is that (afaics?) there's now nothing keeping undesired features like threading or globals from working their way into the interpreter in future commits.
m3dwards:
Concept ACK 80f8b92f4f
TheCharlatan:
ACK 80f8b92f4f
hebasto:
ACK 80f8b92f4f, I have reviewed the code and it looks OK.
Tree-SHA512: 17a62118aeb088f2695c892bb32794dfea3061e3cb7d9e8e9f1c06c3ff6f63a7587fa532e37edbb91fbc5a19b12c9a0f8e05fa9e8864aa07f92665375d847e80
601edd8ee8 ci: use codespell 2.2.6 (fanquake)
52fa0d285f doc: fix some typos (crazeteam)
b5ed13a240 doc: Fix typos (RoboSchmied)
Pull request description:
Combines the recent PRs to fix typos so they can be merged.
ACKs for top commit:
brunoerg:
crACK 601edd8ee8
tdb3:
crACK 601edd8ee8
kristapsk:
cr utACK 601edd8ee8
Tree-SHA512: d054b1dad1336d6b9291cc5d5252d4debf6424a993d4edd6a97d7c15055a7fc48a333d30967f72e7dc9c6c1d9a9038ca8bb5e219c529f4c2365ea48404a508d0
ee1b9b231a CalculateFeerateDiagramsForRBF: update misleading description of old diagram contents (Greg Sanders)
a9d42b9aa5 CompareFeerateDiagram: short-circuit comparison when detected as incomparable (Greg Sanders)
cebcced65e remove erroneous CompareFeerateDiagram comment about slope (Greg Sanders)
a0376e1061 unit test: clarify unstated assumption for calc_feerate_diagram_rbf chunking (Greg Sanders)
890cb015f3 s/effected/affected/ (Greg Sanders)
d9391ec095 CalculateFeerateDiagramsForRBF: remove size tie-breaking from chunking conflicts (Greg Sanders)
b684d82d7e fuzz: Add more invariant checks for package_rbf (Greg Sanders)
2a3ada8b21 fuzz: finer grained ImprovesFeerateDiagram check on error result (Greg Sanders)
c377ae9ba0 unit test: improve ImprovesFeerateDiagram coverage with one less vb case (Greg Sanders)
d2bf923eb1 unit test: make calc_feerate_diagram_rbf less brittle (Greg Sanders)
defe023f6e fuzz: add PrioritiseTransaction coverage in diagram checks (Greg Sanders)
216d5ff162 unit test: add coverage showing priority affects diagram check results (Greg Sanders)
a80d80936a unit test: add CheckConflictTopology case for not the only child (Greg Sanders)
69bd18ca80 unit test: check tx4 conflict error message (Greg Sanders)
c0c37f07eb unit test: have CompareFeerateDiagram tested with diagrams both ways (Greg Sanders)
b62e2c0fa5 ImprovesFeerateDiagram: Spelling fix and removal of unused diagram vectors (Greg Sanders)
bb42402945 doc: fix comment about non-existing CompareFeeFrac (Greg Sanders)
Pull request description:
Follow-ups to https://github.com/bitcoin/bitcoin/pull/29242
ACKs for top commit:
glozow:
ACK ee1b9b231a, reviewed the changes and package_rbf fuzzer seems to run fine
murchandamus:
crACK ee1b9b231a
ismaelsadeeq:
Code review ACK ee1b9b231a
willcl-ark:
ACK ee1b9b231a
Tree-SHA512: 8399fe12064fb49b0e4c73258968b57be1d9c2e35701b2d3b0bb67e2e4052e44216358238f92508e4697d0fb6176518d5b885474054d3deda242f669e99262a7
746b6d8839 test: Add test for createwalletdescriptor (Ava Chow)
2402b63062 wallet: Test upgrade of pre-taproot wallet to have tr() descriptors (Ava Chow)
460ae1bf67 wallet, rpc: Add createwalletdescriptor RPC (Ava Chow)
8e1a475062 wallet: Be able to retrieve single key from descriptors (Ava Chow)
85b1fb19dd wallet: Add GetActiveHDPubKeys to retrieve xpubs from active descriptors (Ava Chow)
73926f2d31 wallet, descspkm: Refactor wallet descriptor generation to standalone func (Andrew Chow)
54e74f46ea wallet: Refactor function for single DescSPKM setup (Andrew Chow)
3b09d0eb7f tests: Test for gethdkeys (Ava Chow)
5febe28c9e wallet, rpc: Add gethdkeys RPC (Ava Chow)
66632e5c24 wallet: Add IsActiveScriptPubKeyMan (Ava Chow)
fa6a259985 desc spkm: Add functions to retrieve specific private keys (Ava Chow)
fe67841464 descriptor: Be able to get the pubkeys involved in a descriptor (Ava Chow)
ef6745879d key: Add constructor for CExtKey that takes CExtPubKey and CKey (Ava Chow)
Pull request description:
This PR adds a `createwalletdescriptor` RPC which allows users to add new automatically generated descriptors to their wallet, e.g. to upgrade a 0.21.x wallet to contain a taproot descriptor. This RPC takes 3 arguments: the output type to create a descriptor for, whether the descriptor will be internal or external, and the HD key to use if the user wishes to use a specific key. The HD key is an optional parameter. If it is not specified, the wallet will use the key shared by the active descriptors, if they are all single key. For most users in the expected upgrade scenario, this should be sufficient. In more advanced cases, the user must specify the HD key to use.
Currently, specified HD keys must already exist in the wallet. To make it easier for the user to know, `gethdkeys` is also added to list out the HD keys in use by all of the descriptors in the wallet. This will include all HD keys, whether we have the private key, for it, which descriptors use it and their activeness, and optionally the extended private key. In this way, users with more complex wallets will be still be able to get HD keys from their wallet for use in other scenarios, and if they want to use `createwalletdescriptor`, they can easily get the keys that they can specify to it.
See also https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866961865
ACKs for top commit:
Sjors:
re-utACK 746b6d8839
furszy:
ACK 746b6d8
ryanofsky:
Code review ACK 746b6d8839, and this looks ready to merge. There were various suggested changes since last review where main change seems to be switching `gethdkeys` output to use normalized descriptors (removing hardened path components).
Tree-SHA512: f2849101e6fbf1f59cb031eaaaee97af5b1ae92aaab54c5716940d210f08ab4fc952df2725b636596cd5747b8f5beb1a7a533425bc10d09da02659473516fbda
4d5b55735b log: renamed disk to file so wording was more accurate (kevkevin)
b9f04be870 mempool: Log added for dumping mempool transactions to disk (kevkevin)
Pull request description:
Sometimes when shutting off bitcoind it can take a while to dump the mempool transaction onto the disk so
this change adds additional logging to the `DumpMempool` method in `kernel/mempool_persist.cpp`
Motivated by https://github.com/bitcoin/bitcoin/pull/29227 this change
- adds a single new line for the amount of transactions being dumped and the amount of memory being dumped to file
This is in response to https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1893375082
The logs will now look like this
```
2024-02-09T23:41:52Z DumpAnchors: Flush 2 outbound block-relay-only peer addresses to anchors.dat completed (0.02s)
2024-02-09T23:41:52Z scheduler thread exit
2024-02-09T23:41:52Z Writing 29 mempool transactions to file...
2024-02-09T23:41:52Z Writing 0 unbroadcast transactions to file.
2024-02-09T23:41:52Z Dumped mempool: 0.000s to copy, 0.022s to dump, 0.015 MB dumped to file
2024-02-09T23:41:52Z Flushed fee estimates to fee_estimates.dat.
2024-02-09T23:41:53Z Shutdown: done
```
ACKs for top commit:
maflcko:
cr-ACK 4d5b55735b
glozow:
reACK 4d5b557
Tree-SHA512: 049191e140d00c1ea57debe0138f1c9eb0f9bb0ef8138e2568e6d89e64f45a5d5853ce3b9cc0b28566aab97555b47ddfb0f9199fc8cea6b81e53f50592d5ae6a
5952292133 wallet, rpc: show mempool conflicts in `gettransaction` result (ishaanam)
54e07ee22f wallet: track mempool conflicts (ishaanam)
d64922b590 wallet refactor: use CWalletTx member functions to determine tx state (ishaanam)
ffe5ff1fb6 scripted-diff: wallet: s/TxStateConflicted/TxStateBlockConflicted (ishaanam)
180973a941 test: Add tests for wallet mempool conflicts (ishaanam)
Pull request description:
The `mempool_conflicts` variable is added to `CWalletTx`, it is a set of txids of txs in the mempool conflicting with the wallet tx or a wallet tx's parent. This PR only changes how mempool-conflicted txs are dealt with in memory.
`IsSpent` now returns false for an output being spent by a mempool conflicted transaction where it previously returned true.
A txid is added to `mempool_conflicts` during `transactionAddedToMempool`. A txid is removed from `mempool_conflicts` during `transactionRemovedFromMempool`.
This PR also adds a `mempoolconflicts` field to the `gettransaction` wallet RPC result.
Builds on #27145
Second attempt at #18600
ACKs for top commit:
achow101:
ACK 5952292133
ryanofsky:
Code review ACK 5952292133. Just small suggested changes since last review
furszy:
ACK 59522921
Tree-SHA512: 615779606723dbb6c2e302681d8e58ae2052ffee52d721ee0389746ddbbcf4b4c4afacf01ddf42b6405bc6f883520524186a955bf6b628fe9b3ae54cffc56a29
Upstream repo: https://github.com/arun11299/cpp-subprocess
Commit: 4025693decacaceb9420efedbf4967a04cb028e7
The "Convenience Functions" section is unused in our codebase, so it has
been removed.
