Age | Commit message (Collapse) | Author |
|
reconstruction
|
|
|
|
|
|
process_message(s) targets
58c2bbdb55b97e350baf51cfd7f4692b681a8acb [fuzz] Enable erlay in process_message(s) targets (dergoegge)
Pull request description:
The process_message(s) targets can't exercise the Erlay logic at the moment as the config setting is off by default and not switched on in the fuzz targets.
This PR enables the `-txreconciliation` setting in both targets.
ACKs for top commit:
fanquake:
ACK 58c2bbdb55b97e350baf51cfd7f4692b681a8acb
Tree-SHA512: a2754fd04549bdcac94d8225244c5c83fe4c26114c0c2fdf316257480625e05e4e6b1b791974e1f1021451d3f81cb59a109261fb73178ad03911f0a3db963077
|
|
version 0.2.0
202291722300b86f36e97de7960d40a32544c2d1 Add secp256k1_selftest call (Pieter Wuille)
3bfca788b0dae879bfc745cc52c2cb6edc49fd70 Remove explicit enabling of default modules (Pieter Wuille)
4462cb04986d77eddcfc6e8f75e04dc278a8147a Adapt to libsecp256k1 API changes (Pieter Wuille)
9d47e7b71b2805430e8c7b43816efd225a6ccd8c Squashed 'src/secp256k1/' changes from 44c2452fd3..21ffe4b22a (Pieter Wuille)
Pull request description:
Now that libsecp256k1 has a release (https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-December/021271.html), update the subtree to match it.
The changes themselves are not very impactful for Bitcoin Core, but include:
* It's no longer needed to specify whether contexts are for signing or verification or both (all contexts support everything), so make use of that in this PR.
* Verification operations can use the static context now, removing the need for some infrastructure in pubkey.cpp to make sure a context exists.
* Most modules are now enabled by default, so we can drop explicit enabling for them.
* CI improvements (in particular, MSVC and more recent MacOS)
* Introduction of an internal int128 type, which has no effect for GCC/Clang builds, but enables 128-bit multiplication in MSVC, giving a ~20% speedup there (but still slower than GCC/Clang).
* Release process changes (process documentation, changelog, ...).
ACKs for top commit:
Sjors:
ACK 202291722300b86f36e97de7960d40a32544c2d1, but 4462cb04986d77eddcfc6e8f75e04dc278a8147a could use more eyes on it.
achow101:
ACK 202291722300b86f36e97de7960d40a32544c2d1
jonasnick:
utACK 202291722300b86f36e97de7960d40a32544c2d1
Tree-SHA512: 8a9fe28852abe74abd6f96fef16a94d5a427b1d99bff4caab1699014d24698aab9b966a5364a46ed1001c07a7c1d825154ed4e6557c7decce952b77330a8616b
|
|
clang-tidy check
9567bfeab95cc0932073641dd162903850987d43 clang-tidy: Add `performance-no-automatic-move` check (Hennadii Stepanov)
Pull request description:
Split from bitcoin/bitcoin#26642 as [requested](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1054673201).
For the problem description see https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html.
The following types are affected:
- `std::pair<CAddress, NodeSeconds>`
- `std::vector<CAddress>`
- `UniValue`, also see bitcoin/bitcoin#25429
- `QColor`
- `CBlock`
- `MempoolAcceptResult`
- `std::shared_ptr<CWallet>`
- `std::optional<SelectionResult>`
- `CTransactionRef`, which is `std::shared_ptr<const CTransaction>`
ACKs for top commit:
andrewtoth:
ACK 9567bfeab95cc0932073641dd162903850987d43
aureleoules:
ACK 9567bfeab95cc0932073641dd162903850987d43
Tree-SHA512: 9b6a5d539205b41d2c86402d384318ed2e1d89e66333ebd200a48fd7df3ce6f6c60a3e989eda5cc503fb34b8d82526f95e56776e1af51e63b49e3a1fef72dbcb
|
|
|
|
https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
36c201feb74bbb87d22bd956373dbbb9c47fb7e7 remove CBlockIndex copy construction (James O'Beirne)
Pull request description:
Copy construction of CBlockIndex objects is a footgun because of the
wide use of equality-by-pointer comparison in the code base. There are
also potential lifetime confusions of using copied instances, since
there are recursive pointer members (e.g. pprev).
(See also https://github.com/bitcoin/bitcoin/pull/24008#discussion_r891949166)
We can't just delete the copy constructors because they are used for
derived classes (CDiskBlockIndex), so we mark them protected.
ACKs for top commit:
ajtowns:
ACK 36c201feb74bbb87d22bd956373dbbb9c47fb7e7 - code review only
MarcoFalke:
re-ACK 36c201feb74bbb87d22bd956373dbbb9c47fb7e7 🏻
Tree-SHA512: b1cf9a1cb992464a4377dad609713eea63cc099435df374e4553bfe62d362a4eb5e3c6c6649177832f38c0905b23841caf9d62196cef8e3084bfea0bfc26374b
|
|
Copy construction of CBlockIndex objects is a footgun because of the
wide use of equality-by-pointer comparison in the code base. There are
also potential lifetime confusions of using copied instances, since
there are recursive pointer references (e.g. pprev).
We can't just delete the copy constructors because they are used for
derived classes (CDiskBlockIndex), so we mark them protected.
Delete move constructors and declare the destructor to satisfy the
"rule of 5."
|
|
* Use SECP256K1_CONTEXT_NONE when creating signing context, as
SECP256K1_CONTEXT_SIGN is deprecated and unnecessary.
* Use secp256k1_static_context where applicable.
|
|
|
|
|
|
It is used only internally in `CService::ToStringAddrPort()`.
|
|
Both methods do the same thing, so simplify to having just one.
`ToString()` is too generic in this case and it is unclear what it does,
given that there are similar methods:
`ToStringAddr()` (inherited from `CNetAddr`),
`ToStringPort()` and
`ToStringAddrPort()`.
|
|
Both methods do the same thing, so simplify to having just one.
Further, `CService` inherits `CNetAddr` and `CService::ToString()`
overrides `CNetAddr::ToString()` but the latter is not virtual which
may be confusing. Avoid such a confusion by not having non-virtual
methods with the same names in inheritance.
|
|
"IP" stands for "Internet Protocol".
"IP address" is sometimes shortened to just "IP" or "address".
However, Tor or I2P addresses are not "IP addresses", nor "IPs".
Thus, use "Addr" instead of "IP" for addresses that could be IP, Tor or
I2P addresses:
`CService::ToStringIPPort()` -> `CService::ToStringAddrPort()`
`CNetAddr::ToStringIP()` -> `CNetAddr::ToStringAddr()`
-BEGIN VERIFY SCRIPT-
sed -i 's/ToStringIPPort/ToStringAddrPort/g' -- $(git grep -l ToStringIPPort src)
sed -i 's/ToStringIP/ToStringAddr/g' -- $(git grep -l ToStringIP src)
-END VERIFY SCRIPT-
|
|
|
|
|
|
7082ce3e88d77456d60a5a66bd38807fbd66f311 scripted-diff: rename and de-globalise g_cs_orphans (Anthony Towns)
733d85f79cde353d8c9b54370f296b1031fa33d9 Move all g_cs_orphans locking to txorphanage (Anthony Towns)
a936f41a5d5f7bb97425f82ec64dfae62e840a56 txorphanage: make m_peer_work_set private (Anthony Towns)
3614819864a84ac32f6d53c6ace79b5e71bc174a txorphange: move orphan workset to txorphanage (Anthony Towns)
6f8e442ba61378489a6e2e2ab5bcfbccca1a29ec net_processing: Localise orphan_work_set handling to ProcessOrphanTx (Anthony Towns)
0027174b396cacbaac5fd52f13be3dca9fcbbfb8 net_processing: move ProcessOrphanTx docs to declaration (Anthony Towns)
9910ed755c3dfd7669707b44d993a20030dd7477 net_processing: Pass a Peer& to ProcessOrphanTx (Anthony Towns)
89e2e0da0bcd0b0757d7b42907e2d2214da9f68b net_processing: move extra transactions to msgproc mutex (Anthony Towns)
ff8d44d1967d8ff9fb9b9ea0b87c0470d8cc2550 Remove unnecessary includes of txorphange.h (Anthony Towns)
Pull request description:
Moves extra transactions to be under the `m_msgproc_mutex` lock rather than `g_cs_orphans` and refactors orphan handling so that the lock can be internal to the `TxOrphange` class.
ACKs for top commit:
dergoegge:
Code review ACK 7082ce3e88d77456d60a5a66bd38807fbd66f311
glozow:
ACK 7082ce3e88d77456d60a5a66bd38807fbd66f311 via code review and some [basic testing](https://github.com/glozow/bitcoin/blob/review-26295/src/test/orphanage_tests.cpp#L150). I think putting txorphanage in charge of handling peer work sets is the right direction.
Tree-SHA512: 1ec454c3a69ebd45ff652770d6a55c6b183db71aba4d12639ed70f525f0035e069a81d06e9b65b66e87929c607080a1c5e5dcd2ca91eaa2cf202dc6c02aa6818
|
|
|
|
b89530483d39f6a6a777df590b87ba2fad8c8b60 util: move threadinterrupt into util (fanquake)
Pull request description:
Alongside thread and threadnames. It's part of libbitcoin_util.
ACKs for top commit:
ryanofsky:
Code review ACK b89530483d39f6a6a777df590b87ba2fad8c8b60. No changes since last review other than rebase
theuni:
ACK b89530483d39f6a6a777df590b87ba2fad8c8b60.
Tree-SHA512: 0421f4d1881ec295272446804b27d16bf63e6b62b272f8bb52bfecde9ae6605e8109ed16294690d3e3ce4b15cc5e7c4046f99442df73adb10bdf069d3fb165aa
|
|
onion addresses
0eeb9b0442fb2f2da33c04704eefe6a84d28e981 [fuzz] Move ConsumeNetAddr to fuzz/util/net.h (dergoegge)
291c8697d4be0f38635b67880107e39d3ec585ad [fuzz] Make ConsumeNetAddr produce valid onion addresses (dergoegge)
c9ba3f836e1646875d2f96f1f466f8a83634a6f7 [netaddress] Make OnionToString public (dergoegge)
Pull request description:
The chance that the fuzzer is able to guess a valid onion address is probably slim, as they are Base32 encoded and include a checksum. Right now, any target using `ConsumeNetAddr` would have a hard time uncovering bugs that require valid onion addresses as input.
This PR makes `ConsumeNetAddr` produce valid onion addresses by using the 32 bytes given by the fuzzer as the pubkey for the onion address and forming a valid address according to the torv3 spec.
ACKs for top commit:
vasild:
ACK 0eeb9b0442fb2f2da33c04704eefe6a84d28e981
brunoerg:
ACK 0eeb9b0442fb2f2da33c04704eefe6a84d28e981
Tree-SHA512: 7c687a4d12f9659559be8f0c3cd4265167d1261d419cfd3d503fd7c7f207cc0db745220f02fb1737e4a5700ea7429311cfc0b42e6c15968ce6a85f8813c7e1d8
|
|
dependencies
c8dc0e3eaa9e0f956c5177bcb69632beb0d51770 refactor: Inline `CTxMemPoolEntry` class's functions (Hennadii Stepanov)
75bbe594e54402ed248ecf2bdfe54e8283d20fff refactor: Move `CTxMemPoolEntry` class to its own module (Hennadii Stepanov)
Pull request description:
This PR:
- gets rid of the `policy/fees` -> `txmempool` -> `policy/fees` circular dependency
- is an alternative to #13949, which nukes only one circular dependency
ACKs for top commit:
ryanofsky:
Code review ACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770. Just include and whitespace changes since last review, and there's a moveonly commit now so it's very easy to review
theStack:
Code-review ACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770
glozow:
utACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770, agree these changes are an improvement.
Tree-SHA512: 36ece824e6ed3ab1a1e198b30a906c8ac12de24545f840eb046958a17315ac9260c7de26e11e2fbab7208adc3d74918db7a7e389444130f8810548ca2e81af41
|
|
|
|
|
|
This change nukes the policy/fees->mempool circular dependency.
Easy to review using `diff --color-moved=dimmed-zebra`.
|
|
|
|
|
|
|
|
If we're connecting to the peer which might support
transaction reconciliation, we announce we want to reconcile
with them.
We store the reconciliation salt so that when the peer
responds with their salt, we are able to compute the
full reconciliation salt.
This behavior is enabled with a CLI flag.
|
|
626b7c8493ea1063f30ae4f62e1b36eb87adf685 fuzz: add scanblocks as safe for fuzzing (James O'Beirne)
94fe5453c7f8a86136dc9a6e0b370afd32564209 test: rpc: add scanblocks functional test (Jonas Schnelli)
6ef2566b68cb8570220c13df11c5cb5a5f4f7a4d rpc: add scanblocks - scan for relevant blocks with descriptors (Jonas Schnelli)
a4258f6e81a476ce1687e2d58f7d2bf16162a172 rpc: move-only: consolidate blockchain scan args (James O'Beirne)
Pull request description:
Revives #20664. All feedback from the previous PR has either been responded to inline or incorporated here.
---
Major changes from Jonas' PR:
- consolidated arguments for scantxoutset/scanblocks
- substantial cleanup of the functional test
Here's the range-diff (`git range-diff master jonasschnelli/2020/12/filterblocks_rpc jamesob/2021-11-scanblocks`): https://gist.github.com/jamesob/aa4a975344209f0316444b8de2ec1d18
### Original PR description
> The `scanblocks` RPC call allows one to get relevant blockhashes from a set of descriptors by scanning all blockfilters in a given range.
>
> **Example:**
>
> `scanblocks start '["addr(<bitcoin_address>)"]' 661000` (returns relevant blockhashes for `<bitcoin_address>` from blockrange 661000->tip)
>
> ## Why is this useful?
> **Fast wallet rescans**: get the relevant blocks and only rescan those via `rescanblockchain getblockheader(<hash>)[height] getblockheader(<hash>)[height])`. A future PR may add an option to allow to provide an array of blockhashes to `rescanblockchain`.
>
> **prune wallet rescans**: (_needs additional changes_): together with a call to fetch blocks from the p2p network if they have been pruned, it would allow to rescan wallets back to the genesis block in pruned mode (relevant #15946).
>
> **SPV mode** (_needs additional changes_): it would be possible to build the blockfilterindex from the p2p network (rather then deriving them from the blocks) and thus allow some sort of hybrid-SPV mode with moderate bandwidth consumption (related #9483)
ACKs for top commit:
furszy:
diff re-ACK 626b7c8
Tree-SHA512: f84e4dcb851b122b39e9700c58fbc31e899cdcf9b587df9505eaf1f45578cc4253e89ce2a45d1ff21bd213e31ddeedbbcad2c80810f46755b30acc17b07e2873
|
|
and SetSocketNonBlocking() to Sock methods
b527b549504672704a61f70d2565b9489aaaba91 net: convert standalone SetSocketNonBlocking() to Sock::SetNonBlocking() (Vasil Dimov)
29f66f76826056f53d971ac734b7ed49b39848d3 moveonly: move SetSocketNonBlocking() from netbase to util/sock (Vasil Dimov)
b4bac556791b5bb8aa118d4c1fed42c3fe45550c net: convert standalone IsSelectableSocket() to Sock::IsSelectable() (Vasil Dimov)
5db7d2ca0aa51ff25f97bf21ce0cbc9e6b741cbd moveonly: move IsSelectableSocket() from compat.h to sock.{h,cpp} (Vasil Dimov)
Pull request description:
_This is a piece of #21878, chopped off to ease review._
* convert standalone `IsSelectableSocket()` to `Sock::IsSelectable()`
* convert standalone `SetSocketNonBlocking()` to `Sock::SetNonBlocking()`
This further encapsulates syscalls inside the `Sock` class and makes the callers mockable.
ACKs for top commit:
jonatack:
ACK b527b549504672704a61f70d2565b9489aaaba91 review/debug build/unit tests at each commit, cross-referenced the changes with `man select` and `man errno`, ran a signet node on the last commit with ip4/ip6//tor/i2p/cjdns and network connections were nominal
dergoegge:
Code review ACK b527b549504672704a61f70d2565b9489aaaba91
Tree-SHA512: af783ce558c7a89e173f7ab323fb3517103d765c19b5d14de29f64706b4e1fea3653492e8ea73ae972699986aaddf2ae72c7cfaa7dad7614254283083b7d2632
|
|
|
|
|
|
|
|
|
|
|
|
Moving the mempool code (Boost) out of util.h, results in a ~10% speedup
(for me) when compiling the fuzz tests.
|
|
|
|
079cf88c0df6de038b8f8933d55c0d17af007b43 refactor: move Boost datetime usage to wallet (fanquake)
Pull request description:
This means we don't need Boost Datetime in a `--disable-wallet` build, and it isn't included in the kernel (via time.h/cpp). Split from a larger boost removal branch/effort.
ACKs for top commit:
hebasto:
re-ACK 079cf88c0df6de038b8f8933d55c0d17af007b43
aureleoules:
re-ACK 079cf88c0df6de038b8f8933d55c0d17af007b43 - rebased and two additional unit tests since my last review.
jarolrod:
crACK 079cf88c0df6de038b8f8933d55c0d17af007b43
Tree-SHA512: c84f47158a4f21902f211c059d8c4bd55ffe95a256835deee723653be08cca49eeddfc33a2316b0cd31805e81cf77eaa39c6c9dcff4cda11a26ba4c1c143974e
|
|
to avoid OOM
fa5752da6a58fadd3f79f47ff98b796d9768872a fuzz: Limit outpoints.size in txorphan target to avoid OOM (MacroFake)
Pull request description:
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=52008
ACKs for top commit:
fanquake:
ACK fa5752da6a58fadd3f79f47ff98b796d9768872a
Tree-SHA512: f010c0eabb72ad4bbf428954f6f978e88d6d15ec3ee77536334b11c0ca605377bdaa40ecf1984f027a430d62f05e9201775f5a6b047ffa38563aeefc04958a1f
|
|
This means we don't need datetime in a --disable-wallet build, and it
isn't included in the kernel.
|
|
|
|
fa4ba04c157b83b827f7541fa007710bd6211fe7 fuzz: Remove no-op call to get() (MacroFake)
fa642286b83f29cb0ac0c8d4c7d8eba10600402c fuzz: Avoid timeout in bitdeque fuzz target (MacroFake)
Pull request description:
I'd guess that any bug should be discoverable within `10` ops. However, `900` seems also better than no limit at all, which causes timeouts such as https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=50892
ACKs for top commit:
sipa:
ACK fa4ba04c157b83b827f7541fa007710bd6211fe7
Tree-SHA512: f6bd25e78d5f04c6f88e9300c2fa3d0993a0911cb0fd1b414077adc0edde1a06ad72af5e2f50f0ab1324f91999ae57d879686c545b2e6c19ae7f637a8804bd48
|
|
d575a675cc884b1bebdb6197f2ef45c51788d4a3 net_processing: add thread safety annotation for m_highest_fast_announce (Anthony Towns)
0ae7987f68211729d87c9255889f4d9d1aa6a37f net_processing: add thread safety annotations for PeerManagerImpl members accessed only via the msgproc thread (Anthony Towns)
a66a7ccb822f0f1f554d27d04735b7fb585d3b71 net_processing: add thread safety annotations for Peer members accessed only via the msgproc thread (Anthony Towns)
bf12abe4542f2cf658516ea7e7fbbff6864c2208 net: drop cs_sendProcessing (Anthony Towns)
1e78f566d575a047a6f0b762bc79601e0208d103 net: add NetEventsInterface::g_msgproc_mutex (Anthony Towns)
Pull request description:
There are many cases where we assume message processing is single-threaded in order for how we access node-related memory to be safe. Add an explicit mutex that we can use to document this, which allows the compiler to catch any cases where we try to access that memory from other threads and break that assumption.
ACKs for top commit:
MarcoFalke:
review ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3 📽
dergoegge:
Code review ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/26036/commits/d575a675cc884b1bebdb6197f2ef45c51788d4a3
vasild:
ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3 modulo the missing runtime checks
Tree-SHA512: b886d1aa4adf318ae64e32ccaf3d508dbb79d6eed3f1fa9d8b2ed96f3c72a3d38cd0f12e05826c9832a2a1302988adfd2b43ea9691aa844f37d8f5c37ff20e05
|
|
SendMessages() is now protected g_msgproc_mutex; so this additional
per-node mutex is redundant.
|
|
There are many cases where we assume message processing is
single-threaded in order for how we access node-related memory to be
safe. Add an explicit mutex that we can use to document this, which allows
the compiler to catch any cases where we try to access that memory from
other threads and break that assumption.
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*')
-END VERIFY SCRIPT-
Co-authored-by: MacroFake <falke.marco@gmail.com>
|