Age | Commit message (Collapse) | Author |
|
|
|
A "version" message in the V1 protocol starts with a fixed 16 bytes:
* The 4-byte network magic
* The 12-byte zero-padded command "version" plus 5 0x00 bytes
The current code detects incoming V1 connections by just looking at the
first 12 bytes (matching an earlier version of BIP324), but 16 bytes is
more precise. This isn't an observable difference right now, as a 12 byte
prefix ought to be negligible already, but it may become observable with
future extensions to the protocol, so make the code match the
specification.
|
|
benchmarks
ce6df7df9bab2405cfe7d6e382f5682cf30de476 bench: Add SHA256 implementation specific benchmarks (Hennadii Stepanov)
5f72417176cfffece9a5aa11e97d5a6599c51e7a Add ability to specify SHA256 implementation for benchmark purposes (Hennadii Stepanov)
Pull request description:
On the master branch, only the best available `SHA256` implementation is being benchmarked. This PR makes `bench_bitcoin` benchmark all `SHA256` implementations that are available on the system.
For example:
- on Linux:
```
$ ./src/bench/bench_bitcoin -filter=SHA.*
Using the 'x86_shani(1way,2way)' SHA256 implementation
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1.00 | 1,002,545,462.93 | 0.4% | 0.01 | `SHA1`
| 2.91 | 344,117,991.18 | 0.1% | 0.03 | `SHA256 using the 'standard' SHA256 implementation`
| 2.21 | 453,081,794.40 | 0.1% | 0.02 | `SHA256 using the 'sse4(1way),sse41(4way)' SHA256 implementation`
| 2.21 | 453,396,506.58 | 0.1% | 0.02 | `SHA256 using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`
| 0.53 | 1,870,520,687.49 | 0.1% | 0.01 | `SHA256 using the 'x86_shani(1way,2way)' SHA256 implementation`
| 7.90 | 126,627,134.33 | 0.0% | 0.01 | `SHA256D64_1024 using the 'standard' SHA256 implementation`
| 3.94 | 253,850,206.07 | 0.0% | 0.01 | `SHA256D64_1024 using the 'sse4(1way),sse41(4way)' SHA256 implementation`
| 1.40 | 716,247,553.38 | 0.4% | 0.01 | `SHA256D64_1024 using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`
| 1.26 | 792,706,270.13 | 0.9% | 0.01 | `SHA256D64_1024 using the 'x86_shani(1way,2way)' SHA256 implementation`
| 6.75 | 148,172,097.64 | 0.2% | 0.01 | `SHA256_32b using the 'standard' SHA256 implementation`
| 4.90 | 204,156,289.96 | 0.1% | 0.01 | `SHA256_32b using the 'sse4(1way),sse41(4way)' SHA256 implementation`
| 4.90 | 204,101,274.22 | 0.1% | 0.01 | `SHA256_32b using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`
| 1.70 | 589,052,595.35 | 0.4% | 0.01 | `SHA256_32b using the 'x86_shani(1way,2way)' SHA256 implementation`
| 2.21 | 453,441,736.14 | 1.0% | 0.02 | `SHA3_256_1M`
| 1.92 | 521,807,101.48 | 1.0% | 0.02 | `SHA512`
```
- on macOS (M1):
```
% ./src/bench/bench_bitcoin -filter=SHA.\*
Using the 'arm_shani(1way,2way)' SHA256 implementation
| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1.36 | 737,644,274.00 | 0.6% | 0.02 | `SHA1`
| 3.08 | 324,556,777.15 | 0.2% | 0.03 | `SHA256 using the 'standard' SHA256 implementation`
| 0.45 | 2,198,104,135.18 | 0.3% | 0.01 | `SHA256 using the 'arm_shani(1way,2way)' SHA256 implementation`
| 8.84 | 113,131,299.18 | 0.0% | 0.01 | `SHA256D64_1024 using the 'standard' SHA256 implementation`
| 0.94 | 1,059,406,239.36 | 0.0% | 0.01 | `SHA256D64_1024 using the 'arm_shani(1way,2way)' SHA256 implementation`
| 6.17 | 162,050,659.51 | 0.2% | 0.01 | `SHA256_32b using the 'standard' SHA256 implementation`
| 1.15 | 866,637,155.98 | 0.0% | 0.01 | `SHA256_32b using the 'arm_shani(1way,2way)' SHA256 implementation`
| 1.69 | 592,636,491.59 | 0.2% | 0.02 | `SHA3_256_1M`
| 1.89 | 528,785,775.66 | 0.0% | 0.02 | `SHA512`
```
Found it useful, while working on https://github.com/bitcoin/bitcoin/pull/24773.
ACKs for top commit:
martinus:
ACK ce6df7df9bab2405cfe7d6e382f5682cf30de476. I would have created a helper function in the test to avoid the code duplication for each test, but that's just me nitpicking. Here are results from my Ryzen 7950X, with `./src/bench/bench_bitcoin -filter="SHA256.*" -min-time=1000`:
MarcoFalke:
review ACK ce6df7df9bab2405cfe7d6e382f5682cf30de476 🏵
sipa:
ACK ce6df7df9bab2405cfe7d6e382f5682cf30de476
Tree-SHA512: e3de50e11b9a3a0d1e05583786041d4dc9afa2022e2115d75d6d1f63b11f62f6336f093001e53a631431d558c4dae29c596755c9e2d6aa78c382270116cc1f7f
|
|
|
|
|
|
|
|
|
|
|
|
remote client disconnection
68f23f57d77bc172ed39ecdd4d2d5cd5e13cf483 http: bugfix: track closed connection (stickies-v)
084d0372311e658a486622f720d2b827d8416591 http: log connection instead of request count (stickies-v)
41f9027813f849a9fd6a1479bbb74b9037990c3c http: refactor: use encapsulated HTTPRequestTracker (stickies-v)
Pull request description:
#26742 significantly increased the http server shutdown speed, but also introduced a bug (#27722 - see https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1559453982 for steps to reproduce on master) that causes http server shutdown to halt in case of a remote client disconnection. This happens because `evhttp_request_set_on_complete_cb` is never called and thus the request is never removed from `g_requests`.
This PR fixes that bug, and improves robustness of the code by encapsulating the request tracking logic. Earlier approaches (#27909, #27245, #19434) attempted to resolve this but [imo are fundamentally unsafe](https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1265614783) because of differences in lifetime between an `evhttp_request` and `evhttp_connection`.
We don't need to keep track of open requests or connections, we just [need to ensure](https://github.com/bitcoin/bitcoin/pull/19420#issue-648067169) that there are no active requests on server shutdown. Because a connection can have multiple requests, and a request can be completed in various ways (the request actually being handled, or the client performing a remote disconnect), keeping a counter per connection seems like the approach with the least overhead to me.
Fixes #27722
ACKs for top commit:
vasild:
ACK 68f23f57d77bc172ed39ecdd4d2d5cd5e13cf483
theStack:
ACK 68f23f57d77bc172ed39ecdd4d2d5cd5e13cf483
Tree-SHA512: dfa711ff55ec75ba44d73e9e6fac16b0be25cf3c20868c2145a844a7878ad9fc6998d9ff62d72f3a210bfa411ef03d3757b73d68a7c22926e874c421e51444d6
|
|
bdee8589644fac121320e95f53457c3ddfc71e1b typo: in packages.md (Erik McKelvey)
Pull request description:
Removed extra word `the` in packages.md
ACKs for top commit:
fanquake:
ACK bdee8589644fac121320e95f53457c3ddfc71e1b
Tree-SHA512: 14a745e5f8ad97f38c21c7b80e88592f84f50d87bc71930c1212fb9ba5a46b129ffec0aa1dd53780f872c0067b58bd2a65ed9def4d9f5f50dc8c7d8e34a429d4
|
|
|
|
|
|
- make `getaddrmaninfo` RPC public since it's not for development
purposes only and regular users might find it useful
- add missing `all_networks` key to RPC help
- use clang format spacing
|
|
|
|
fac054d24c4b6fe3370e458ad6b626f98bd018d6 ci: Print Linux kernel info (MarcoFalke)
Pull request description:
Required to debug issues like https://github.com/bitcoin/bitcoin/pull/28487#issuecomment-1729717923. For example:
```
FATAL: ThreadSanitizer: unexpected memory mapping 0x57cf8f031000-0x57cf8f173000
ACKs for top commit:
hebasto:
ACK fac054d24c4b6fe3370e458ad6b626f98bd018d6
Tree-SHA512: 7eb158e52daffe5cbcdfa3ed1efb45e1930b80a2672558fe400c8d72ce59a8cbeb02296dfc2032721d511410885a1f057672fe8086ba1c72a494aef541bf7eb4
|
|
addrman table entries
352d5eb2a9e89cff4a2815d94a9d81fcc20c4b2c test: getrawaddrman RPC (0xb10c)
da384a286bd84a97e7ebe7a64654c5be20ab2df1 rpc: getrawaddrman for addrman entries (0xb10c)
Pull request description:
Inspired by `getaddrmaninfo` (#27511), this adds a hidden/test-only `getrawaddrman` RPC. The RPC returns information on all addresses in the address manager new and tried tables. Addrman table contents can be used in tests and during development.
The RPC result encodes the `bucket` and `position`, the internal location of addresses in the tables, in the address object's string key. This allows users to choose to consume or to ignore the location information. If the internals of the address manager implementation change, the location encoding might change too.
```
getrawaddrman
EXPERIMENTAL warning: this call may be changed in future releases.
Returns information on all address manager entries for the new and tried tables.
Result:
{ (json object)
"table" : { (json object) buckets with addresses in the address manager table ( new, tried )
"bucket/position" : { (json object) the location in the address manager table (<bucket>/<position>)
"address" : "str", (string) The address of the node
"port" : n, (numeric) The port number of the node
"network" : "str", (string) The network (ipv4, ipv6, onion, i2p, cjdns) of the address
"services" : n, (numeric) The services offered by the node
"time" : xxx, (numeric) The UNIX epoch time when the node was last seen
"source" : "str", (string) The address that relayed the address to us
"source_network" : "str" (string) The network (ipv4, ipv6, onion, i2p, cjdns) of the source address
},
...
},
...
}
Examples:
> bitcoin-cli getrawaddrman
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getrawaddrman", "params": []}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
```
ACKs for top commit:
willcl-ark:
reACK 352d5eb2a9
amitiuttarwar:
reACK 352d5eb2a9e
stratospher:
reACK 352d5eb.
achow101:
ACK 352d5eb2a9e89cff4a2815d94a9d81fcc20c4b2c
Tree-SHA512: cc462666b5c709617c66b0e3e9a17c4c81e9e295f91bdd9572492d1cb6466fc9b6d48ee805ebe82f9f16010798370effe5c8f4db15065b8c7c0d8637675d615e
|
|
qt build
848eec09363d1ba8198376eb9654b1a69e3541aa depends: fix unusable memory_resource in macos qt build (fanquake)
Pull request description:
See https://codereview.qt-project.org/c/qt/qtbase/+/482392.
Fixes #28566.
ACKs for top commit:
hebasto:
ACK 848eec09363d1ba8198376eb9654b1a69e3541aa.
Tree-SHA512: dd902f7abb09bda3800d78fe58937b4426d974c24ba321b979eba0d6da30fa0c661b4ed629afab827df8f9ab599efc7a288e9f381ec2b3c69d1063d4d4f73f9e
|
|
|
|
7df450836969b81e98322c9a09c08b35d1095a25 test: improve sock_tests/move_assignment (Vasil Dimov)
5086a99b84367a45706af7197da1016dd966e6d9 net: remove Sock default constructor, it's not necessary (Vasil Dimov)
7829272f7826511241defd34954e6040ea963f07 net: remove now unnecessary Sock::Get() (Vasil Dimov)
944b21b70ae490a5a746bcc1810a5074d74e9d34 net: don't check if the socket is valid in ConnectSocketDirectly() (Vasil Dimov)
aeac68d036e3cff57ce155f1a904d77f98b357d4 net: don't check if the socket is valid in GetBindAddress() (Vasil Dimov)
5ac1a51ee5a57da59f1ff1986b7d9054484d3c80 i2p: avoid using Sock::Get() for checking for a valid socket (Vasil Dimov)
Pull request description:
_This is a piece of #21878, chopped off to ease review._
Peeking at the underlying socket file descriptor of `Sock` and checkig if it is `INVALID_SOCKET` is bad encapsulation and stands in the way of testing/mocking/fuzzing.
Instead use an empty `unique_ptr` to denote that there is no valid socket where appropriate or outright remove such checks where they are not necessary.
The default constructor `Sock::Sock()` is unnecessary now after recent changes, thus remove it.
ACKs for top commit:
ajtowns:
ACK 7df450836969b81e98322c9a09c08b35d1095a25
jonatack:
ACK 7df450836969b81e98322c9a09c08b35d1095a25
Tree-SHA512: 9742aeeeabe8690530bf74caa6ba296787028c52f4a3342afd193b05dbbb1f6645935c33ba0a5230199a09af01c666bd3c7fb16b48692a0d185356ea59a8ddbf
|
|
bae209e3879fa099302d3b211362c49bbbfbdd14 gui: macOS, make appMenuBar part of the main app window (furszy)
e14cc8fc69cb3e3a98076fbb23a94eba7873368a gui: macOS, do not process dock icon actions during shutdown (furszy)
Pull request description:
As the 'QMenuBar' is created without a parent window in MacOS, the app crashes when the user presses the shutdown button and, right after it, triggers any action in the menu bar.
This happens because the QMenuBar is manually deleted in the BitcoinGUI destructor but the events attached to it children actions are not disconnected, so QActions events such us the 'QMenu::aboutToShow' could try to access null pointers.
Instead of guarding every single QAction pointer inside the QMenu::aboutToShow slot, or manually disconnecting all registered events in the destructor, we can check if a shutdown was requested and discard the event.
The 'node' field is a ref whose memory is held by the main application class, so it is safe to use here. Events are disconnected prior destructing the main application object.
Furthermore, the 'MacDockIconHandler::dockIconClicked' signal can make the app crash during shutdown for the very same reason. The 'show()' call triggers the 'QApplication::focusWindowChanged' event, which is connected to the 'minimize_action' QAction, which is also part of the app menu bar, which could no longer exist.
Another cause of crashes stems from the shortcuts provided by the `appMenuBar` submenus during shutdown. For instance, executing actions like opening the information dialog (command + I) or the console dialog (command + T) lead to access null pointers. The second commit addresses and resolves these issues.
Basically, in the present setup, we create a parentless `appMenuBar` whose submenus `QActions` are connected to `qApp` events (the app's global instance). However, at the `BitcoinGUI` destructor, we manually destruct this object without properly disconnecting the events. This leaves `qApp` events, such as `focusWindowChanged`, tied to submenus' `QAction` pointers, which causes the application to crash when it attempts to access them.
Important Note:
This happened to me few times. The worst consequence was an inconsistent chain state during IBD. Which triggered a full "replay blocks" process on the next startup. Which was painfully slow.
ACKs for top commit:
RandyMcMillan:
utACK bae209e
hebasto:
ACK bae209e3879fa099302d3b211362c49bbbfbdd14.
Tree-SHA512: 432e19c5f7e02c3165b7e7bd7f96f2a902bae5b5e439c2594db1c69d74ab6e0d4509d90f02db8c076f616e567e6a07492ede416ef651b5f749637398291b92fd
|
|
It is possible that the client disconnects before the request is
handled. In those cases, evhttp_request_set_on_complete_cb is never
called, which means that on shutdown the server we'll keep waiting
endlessly.
By adding evhttp_connection_set_closecb, libevent automatically
cleans up those dead connections at latest when we shutdown, and
depending on the libevent version already at the moment of remote
client disconnect. In both cases, the bug is fixed.
|
|
There is no significant benefit in logging the request count instead
of the connection count. Reduces amount of code and computational
complexity.
|
|
Introduces and uses a HTTPRequestTracker class to keep track of
how many HTTP requests are currently active, so we don't stop the
server before they're all handled.
This has two purposes:
1. In a next commit, allows us to untrack all requests associated
with a connection without running into lifetime issues of the
connection living longer than the request
(see https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1265614783)
2. Improve encapsulation by making the mutex and cv internal members,
and exposing just the WaitUntilEmpty() method that can be safely
used.
|
|
See https://codereview.qt-project.org/c/qt/qtbase/+/482392.
|
|
|
|
|
|
|
|
conventions
360b917674e63c1e95119040463b3f50976bf331 contrib/bash-completions: use package naming conventions (Erik Arvstedt)
Pull request description:
#### Copy of commit msg
This naming scheme supports auto-detection and on-demand loading of completions.
See
https://github.com/scop/bash-completion/blob/ba109693ee2284f6a82f8f0e1563baf071252df9/README.md#faq,
section "Where should I put it to be sure that interactive bash shells will find it and source it", keyword `foo.bash`.
Previously, distro package maintainers had to rename these files manually.
ACKs for top commit:
willcl-ark:
ACK 360b917674e63c1e95119040463b3f50976bf331
Tree-SHA512: 6dd1f62309e877402fac2f7aec785d3053a1fd3fdeae38abc961c3f61269eb664a2489b6fa8294bd706d8ac387c7af3ac66cd7a852810f69371fa8ae991afacb
|
|
fa6e6a3f03a38f8b431bf694268ed344d1815b3b doc: Remove confusing assert linter (MarcoFalke)
Pull request description:
The `assert()` documentation and linter are redundant and confusing:
* The source code already refuses to compile with `assert()` disabled.
* They violate the assumptions about `Assert()`, which *requires* side effects.
* The existing linter doesn't enforce the guideline, only checking for `++` and `--` side effects.
Fix all issues by removing the docs and the linter. See also https://github.com/bitcoin/bitcoin/pull/26684#discussion_r1287370102
Going forward everyone is free to use whatever code in this regard they think is the easiest to read. Also, everyone is still free to share style-nits, if they think it is a good use of their time and of the pull request author. Finally, the author is still free to dismiss or ignore this style-nit, or any other style-nit.
ACKs for top commit:
hebasto:
ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b, I have reviewed the code and it looks OK.
theStack:
ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b
Tree-SHA512: 686738d71e1316cc95e5d3f71869b55a02bfb137c795cc0875057f4410e564bc8eff03c985a2087b007fb08fc84551c7da1e8b30c7a9c3f2b14e5e44a5970236
|
|
Xcode 15 linker
79ef528511f0cbbe0a7097ef031f2964aaccfe5c build, macos: Fix `qt` package build with new Xcode 15 linker (Hennadii Stepanov)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/28541 by backporting an upstream [patch](https://github.com/qt/qtbase/commit/cdf64b0e47115cc473e1afd1472b4b09e130b2a5).
Guix build:
```
x86_64
b37713bc8a526662eac3d9535924f4a4d2893c58f9c12d3c7599e761e6ff677c guix-build-79ef528511f0/output/arm64-apple-darwin/SHA256SUMS.part
0befb524181aa10e1635a2616a8bed53f51beafa4f0d495d3bf52a64cbd2d977 guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin-unsigned.tar.gz
9cba170f2ffe542c33fdd1ac52b7684dd6301e91d32aa45af7b4ce8769d88d4a guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin-unsigned.zip
04556309266c791ae4d7409359222c88cd7aeb569566f7ef4d29816148a5b7e4 guix-build-79ef528511f0/output/arm64-apple-darwin/bitcoin-79ef528511f0-arm64-apple-darwin.tar.gz
51229df8e104a2ffcd5c5b3f81f7585e1258ef10461d136948ea2a2d690a920d guix-build-79ef528511f0/output/dist-archive/bitcoin-79ef528511f0.tar.gz
3fe216a05561f2fe7229ddf186ff495b29a5cc31b6f35f407187573d072c5743 guix-build-79ef528511f0/output/x86_64-apple-darwin/SHA256SUMS.part
961d71104e61a2baf727576eb2da630697bb4f109f66e73be5c96add25378d12 guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin-unsigned.tar.gz
5598f514d065756ac376e2f3c4f8e758bfba53a43ddef778f106456de1536073 guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin-unsigned.zip
5360ae1f1b7d96a44a33b2c87708b466e4a7bf3f9de0fc58bccbbcdb21ee254e guix-build-79ef528511f0/output/x86_64-apple-darwin/bitcoin-79ef528511f0-x86_64-apple-darwin.tar.gz
```
Top commit has no ACKs.
Tree-SHA512: e3a0f7a578b30a216cc84c8ac6a0eeac3f59b02525e1eb5a9f5512bc9a049a1b17d3feb140259ffe5d2197279c74594126b85112aa596df9013f74bb1047c298
|
|
|
|
75a329103505736acb9036224da2dfa8ab038c43 doc: mention BIP324 support in bips.md (Pieter Wuille)
64ca7210f05c4003228f4cb0b160d869e15f47d2 test: enable v2 transport between nodes in some functional tests (Pieter Wuille)
05d19fbcc10f26c7f1e3a9afc660eb7fa71b1d8c test: Functional test for opportunistic encryption (dhruv)
b815cce50e4bfa0efea8ea02659b7042c8fb18be net: expose transport types/session IDs of connections in RPC and logs (Pieter Wuille)
432a62c4dce908729c62edcfaebc3da6387c3afe net: reconnect with V1Transport under certain conditions (Pieter Wuille)
4d265d0342ae7e92df07ba51e8355db57c44f811 sync: modernize CSemaphore / CSemaphoreGrant (Pieter Wuille)
c73cd423636e06df46742f573640ca773b281ffc rpc: addnode arg to use BIP324 v2 p2p (dhruv)
62d21ee0974b582a6a32aa97ee35ef51c977ea4b net: use V2Transport when NODE_P2P_V2 service flag is present (Pieter Wuille)
a4706bc877504057e8522c929cc0704d3eaa7302 rpc: don't report v2 handshake bytes in the per-type sent byte statistics (Sebastian Falbesoner)
abf343b32026c3f8246f98c416e2c6cf5b66aa38 net: advertise NODE_P2P_V2 if CLI arg -v2transport is on (Pieter Wuille)
Pull request description:
Part of #27634.
This makes BIP324 support feature complete, through a (default off) `-v2transport` option for enabling V2 connections. If it is enabled:
* The `NODE_P2P_V2` service flag (*1 << 11*) is advertized.
* Inbound connections can use V1 or V2 (automatically detected based on the protocol used by the peer)
* V2 connections are used on outbound when the `NODE_P2P_V2` service is available (or the new `use_v2` parameter is set on the `addnode` RPC).
* V2 outbound connections that instantly fail get retried as V1.
There are two new RPC fields, `"transport_protocol_type"` and `"session_id"`, in `getpeerinfo`.
ACKs for top commit:
mzumsande:
re-ACK 75a329103505736acb9036224da2dfa8ab038c43
theStack:
re-ACK 75a329103505736acb9036224da2dfa8ab038c43
Tree-SHA512: 90ea1cd37f3dce410a59ff5de1c2405891e8aa62318d0e06dcb68b21603fb0c061631526633f3d4fb630e63d2b8db407eed48e246befcbef3503bea893a4ff15
|
|
|
|
|
|
Co-authored-by: Pieter Wuille <bitcoin-dev@wuille.net>
|
|
Co-authored-by: Dhruv Mehta <856960+dhruv@users.noreply.github.com>
|
|
When an outbound v2 connection is disconnected without receiving anything, but at
least 24 bytes of our pubkey were sent out (enough to constitute an invalid v1
header), add them to a queue of reconnections to be tried.
The reconnections are in a queue rather than performed immediately, because we should
not block the socket handler thread with connection creation (a blocking operation
that can take multiple seconds).
|
|
|
|
Co-authored-by: Pieter Wuille <bitcoin-dev@wuille.net>
|
|
Co-authored-by: Dhruv Mehta <856960+dhruv@users.noreply.github.com>
|
|
This matches the behavior for per-type received bytes.
|
|
Co-authored-by: Dhruv Mehta <856960+dhruv@users.noreply.github.com>
|
|
edbed31066e3674ba52b8c093ab235625527f383 chainparams: add signet assumeutxo param at height 160_000 (Sjors Provoost)
b8cafe38713cbf10d15459042f7f911bcc1b1e4e chainparams: add testnet assumeutxo param at height 2_500_000 (Sjors Provoost)
99839bbfa7110c7abf22e587ae2f72c9c57d3c85 doc: add note about confusing HaveTxsDownloaded name (James O'Beirne)
7ee46a755f1d57ce9d51975d3b54dc9ac3d08d52 contrib: add script to demo/test assumeutxo (James O'Beirne)
42cae39356fd20d521aaf99aff1ed85856f3c9f3 test: add feature_assumeutxo functional test (James O'Beirne)
0f64bac6030334d798ae205cd7af4bf248feddd9 rpc: add getchainstates (James O'Beirne)
bb0585779472962f40d9cdd9c6532132850d371c refuse to activate a UTXO snapshot if mempool not empty (James O'Beirne)
ce585a9a158476b0ad3296477b922e79f308e795 rpc: add loadtxoutset (James O'Beirne)
62ac519e718eb7a31dca1102a96ba219fbc7f95d validation: do not activate snapshot if behind active chain (James O'Beirne)
9511fb3616b7bbe1d0d2f54a45ea0a650ba0367b validation: assumeutxo: swap m_mempool on snapshot activation (James O'Beirne)
7fcd21544a333ffdf1910b65c573579860be6a36 blockstorage: segment normal/assumedvalid blockfiles (James O'Beirne)
4c3b8ca35c2e4a441264749bb312df2bd054b5b8 validation: populate nChainTx value for assumedvalid chainstates (James O'Beirne)
49ef778158c43859946a592e11ec34fe1b93a5b6 test: adjust chainstate tests to use recognized snapshot base (James O'Beirne)
1019c399825b0d512c1fd751c376d46fed4992b9 validation: pruning for multiple chainstates (James O'Beirne)
373cf91531b84bfdd06fdf8abf4dca228029ce6b validation: indexing changes for assumeutxo (James O'Beirne)
1fffdd76a1bca908f55d73b64983655b14cf7432 net_processing: validationinterface: ignore some events for bg chain (James O'Beirne)
fbe0a7d7ca680358237b6c2369b3fd2b43221113 wallet: validationinterface: only handle active chain notifications (James O'Beirne)
f073917a9e7ba423643dcae0339776470b628f65 validationinterface: only send zmq notifications for active (James O'Beirne)
4d8f4dcb450d31e4847804e62bf91545b949fa14 validation: pass ChainstateRole for validationinterface calls (James O'Beirne)
1e59acdf17309f567c370885f0cf02605e2baa58 validation: only call UpdatedBlockTip for active chainstate (James O'Beirne)
c6af23c5179cc383f8e6c275373af8d11e6a989f validation: add ChainstateRole (James O'Beirne)
9f2318c76cc6986d48e13831cf5bd8dab194fdf4 validation: MaybeRebalanceCaches when chain leaves IBD (James O'Beirne)
434495a8c1496ca23fe35b84499f3daf668d76b8 chainparams: add blockhash to AssumeutxoData (James O'Beirne)
c711ca186f8d8a28810be0beedcb615ddcf93163 assumeutxo: remove snapshot during -reindex{-chainstate} (James O'Beirne)
c93ef43e4fd4fbc1263cdc9e98ae5856830fe89e bugfix: correct is_snapshot_cs in VerifyDB (James O'Beirne)
b73d3bbd23220857bf17cbb6401275bf58013b72 net_processing: Request assumeutxo background chain blocks (Suhas Daftuar)
Pull request description:
- Background and FAQ: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
- Prior progress/project: https://github.com/bitcoin/bitcoin/projects/11
- Replaces https://github.com/bitcoin/bitcoin/pull/15606, which was closed due to Github slowness. Original description and commentary can be found there.
---
This changeset finishes the first phase of the assumeutxo project. It makes UTXO snapshots loadable via RPC (`loadtxoutset`) and adds `assumeutxo` parameters to chainparams. It contains all the remaining changes necessary to both use an assumedvalid snapshot chainstate and do a full validation sync in the background.
This may look like a lot to review, but note that
- ~200 lines are a (non-essential) demo shell script
- Many lines are functional test, documentation, and relatively dilute RPC code.
So it shouldn't be as burdensome to review as the linecount might suggest.
- **P2P**: minor changes are made to `init.cpp` and `net_processing.cpp` to make simultaneous IBD across multiple chainstates work.
- **Pruning**: implement correct pruning behavior when using a background chainstate
- **Blockfile separation**: to prevent "fragmentation" in blockfile storage, have background chainstates use separate blockfiles from active snapshot chainstates to avoid interleaving heights and impairing pruning.
- **Indexing**: some `CValidationInterface` events are given with an additional parameter, ChainstateRole, and all indexers ignore events from ChainstateRole::ASSUMEDVALID so that indexation only happens sequentially.
- Have `-reindex` properly wipe snapshot chainstates.
- **RPC**: introduce RPC commands `loadtxoutset` and (hidden) `getchainstates`.
- **Release docs & first assumeutxo commitment**: add notes and a particular assumeutxo hash value for first AU-enabled release.
- This will complete the project and allow use of UTXO snapshots for faster node bootstrap.
The next phase, if it were to be pursued, would be coming up with a way to distribute the UTXO snapshots over the P2P network.
---
### UTXO snapshots
Create your own with `./contrib/devtools/utxo_snapshot.sh`, e.g.
```shell
./contrib/devtools/utxo_snapshot.sh 788000 utxo.dat ./src/bitcoin-cli -datadir=$(pwd)/testdata`)
```
or use the pre-generated ones listed below.
- Testnet: **2'500'000** (Sjors):
- torrent: `magnet:?xt=urn:btih:511e09f4bf853aefab00de5c070b1e031f0ecbe9&dn=utxo-testnet-2500000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
- sha256: `79db4b025448cc0ac388d8589a28eab02de53055d181e34eb47391717aa16388`
- Signet: **160'000** (Sjors):
- torrent: `magnet:?xt=urn:btih:9da986cb27b3980ea7fd06b21e199b148d486880&dn=utxo-signet-160000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
- sha256: `eeeca845385ba91e84ef58c09d38f98f246a24feadaad57fe1e5874f3f92ef8c`
- Mainnet: **800'000** (Sjors):
- Note: this needs the following commit cherry-picked in: https://github.com/Sjors/bitcoin/commit/24deb2022b822f22fba9fcbee201e37a83225eb2
- torrent: `magnet:?xt=urn:btih:50ee955bef37f5ec3e5b0df4cf0288af3d715a2e&dn=utxo-800000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
### Testing
#### For fun (~5min)
If you want to do a quick test, you can run `./contrib/devtools/test_utxo_snapshots.sh` and follow the instructions. This is mostly obviated by the functional tests, though.
#### For real (longer)
If you'd like to experience a real usage of assumeutxo, you can do that too.
I've cut a new snapshot at height 788'000 (http://img.jameso.be/utxo-788000.dat - but you can do it yourself with `./contrib/devtools/utxo_snapshot.sh` if you want). Download that, and then create a datadir for testing:
```sh
$ cd ~/src/bitcoin # or whatever
# get the snapshot
$ curl http://img.jameso.be/utxo-788000.dat > utxo-788000.dat
# you'll want to do this if you like copy/pasting
$ export AU_DATADIR=/home/${USER}/au-test # or wherever
$ mkdir ${AU_DATADIR}
$ vim ${AU_DATADIR}/bitcoin.conf
dbcache=8000 # or, you know, something high
blockfilterindex=1
coinstatsindex=1
prune=3000
logthreadnames=1
```
Obtain this branch, build it, and then start bitcoind:
```sh
$ git remote add jamesob https://github.com/jamesob/bitcoin
$ git fetch jamesob assumeutxo
$ git checkout jamesob/assumeutxo
$ ./configure $conf_args && make # (whatever you like to do here)
# start 'er up and watch the logs
$ ./src/bitcoind -datadir=${AU_DATADIR}
```
Then, in some other window, load the snapshot
```sh
$ ./src/bitcoin-cli -datadir=${AU_DATADIR} loadtxoutset $(pwd)/utxo-788000.dat
```
You'll see some log messages about headers retrieval and waiting to see the snapshot in the headers chain. Once you get the full headers chain, you'll spend a decent amount of time (~10min) loading the snapshot, checking it, and flushing it to disk. After all that happens, you should be syncing to tip in pretty short order, and you'll see the occasional `[background validation]` log message go by.
In yet another window, you can check out chainstate status with
```sh
$ ./src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates
```
as well as usual favorites like `getblockchaininfo`.
ACKs for top commit:
achow101:
ACK edbed31066e3674ba52b8c093ab235625527f383
Tree-SHA512: 6086fb9a38dc7df85fedc76b30084dd8154617a2a91e89a84fb41326d34ef8e7d7ea593107afba01369093bf8cc91770621d98f0ea42a5b3b99db868d2f14dc2
|
|
|
|
|
|
d67aa25eb2f781b9edcfcf164a08401f9937a0c1 bench: drop NO_THREAD_SAFETY_ANALYSIS from disconnected_txs (fanquake)
Pull request description:
Followup to https://github.com/bitcoin/bitcoin/pull/28385#discussion_r1339964480.
ACKs for top commit:
MarcoFalke:
lgtm ACK d67aa25eb2f781b9edcfcf164a08401f9937a0c1
hebasto:
ACK d67aa25eb2f781b9edcfcf164a08401f9937a0c1, tested on Ubuntu 22.04 with clang 18.0.
glozow:
utACK d67aa25eb2
Tree-SHA512: a9a9a8cc70a50d4fbd51779a3203bbc2a29d354b557e8a99cfd649d5998b71ff1087f5bae7170471bed9a917a93c8f3351ae90c9a6e87d88928c35912d007b64
|
|
Test that the getrawaddrman returns the addresses in the new and tried
tables. We can't check the buckets and positions as these are not
deterministic (yet).
|
|
Exposing address manager table entries in a hidden RPC allows to introspect
addrman tables in tests and during development.
As response JSON object the following FORMAT1 is choosen:
{
"table": {
"<bucket>/<position>": { "address": "..", "port": .., ... },
"<bucket>/<position>": { "address": "..", "port": .., ... },
"<bucket>/<position>": { "address": "..", "port": .., ... },
...
}
}
An alternative would be FORMAT2
{
"table": {
"bucket": {
"position": { "address": "..", "port": .., ... },
"position": { "address": "..", "port": .., ... },
..
},
"bucket": {
"position": { "address": "..", "port": .., ... },
..
},
}
}
FORMAT1 and FORMAT2 have different encodings for the location of the
address in the address manager. While FORMAT2 might be easier to process
for downstream tools, it also mimics internal addrman mappings, which
might change at some point. Users not interested in the address location
can ignore the location key. They don't have to adapt to a new RPC
response format, when the internal addrman layout changes. Additionally,
FORMAT1 is also slightly easier to to iterate in downstream tools. The
RPC response-building implemenation complexcity is lower with FORMAT1
as we can more easily build a "<bucket>/<position>" key than a multiple
"bucket" objects with multiple "position" objects (FORMAT2).
|
|
|
|
380130d9d70f8f8d395949a51f43806f6e600efa test: add coverage to feature_addrman.py (kevkevin)
Pull request description:
I added two new tests that will cover the nNew and nTried tests which add coverage to the if block by checking values larger than our range since we only check for negative values now
adding coverage to these lines
https://github.com/bitcoin/bitcoin/blob/master/src/addrman.cpp#L273
https://github.com/bitcoin/bitcoin/blob/master/src/addrman.cpp#L280
our test seem to only cover the `nTried < 0` and `nNew < 0` scenarios
ACKs for top commit:
ismaelsadeeq:
ACK 380130d9d70f8f8d395949a51f43806f6e600efa, code looks good to me 🍃 .
0xB10C:
Re-ACK 380130d9d70f8f8d395949a51f43806f6e600efa
Tree-SHA512: a063bd9ca4d2d536a27c8c22a28fb13759a96f19cd8ba6cb8879cf7f65046d4ff6e8f70df17feaffd0d0d08ef914cb18a11258d313a4841c811a7e7ae4df6d5b
|