Age | Commit message (Collapse) | Author |
|
|
|
|
|
|
|
|
|
time durations
3a998d2e37bf76667b08cd947807ada1305520d7 Use steady_clock in ConnectAndCallRPC and inline time call in loop conditional (Jon Atack)
3799d2dcdd736dd24850b192e1b264bee1cd5e5a Fix -rpcwait with -netinfo printing negative time durations (Jon Atack)
Pull request description:
- Fix `bitcoin-cli -rpcwait -netinfo 1` returning negative time durations on its first invocation after node startup in the "send", "recv", and "age" columns (potentially the "txn" and "blk" columns also). To reproduce, start bitcoind on mainnet (for a longer startup time) and run `bitcoin-cli -rpcwait -netinfo <n>` where n is 1 or larger. The negative time durations are larger with a slower CPU speed or e.g. higher `checkblocks`/`checklevel` config option settings.
Examples:
```
<-> type net mping ping send recv txn blk hb addrp addrl age id
out manual onion -126 -126 -2 0
ms ms sec sec min min min
```
```
<-> type net mping ping send recv txn blk hb addrp addrl age id
out manual cjdns -64 -64 -1 0
ms ms sec sec min min min
```
```
<-> type net mping ping send recv txn blk hb addrp addrl age id
out manual ipv4 -89 -89 * . -1 0
ms ms sec sec min min min
```
```
<-> type net mping ping send recv txn blk hb addrp addrl age id
out manual ipv6 -133 * . -2 0
ms ms sec sec min min min
```
- Use `steady_clock` in ConnectAndCallRPC and inline the time call in the loop conditional to avoid unnecessary invocations and an unneeded local variable allocation.
ACKs for top commit:
MarcoFalke:
cr ACK 3a998d2e37bf76667b08cd947807ada1305520d7
Tree-SHA512: 141430d47189ad9f646ce8e51cb31c21b395f6294bb27ba9f7ae4c1e1505a63209a4a19662a0b462806437a9cfd07f1ea114e775adc2872d87397fe823f8b8dc
|
|
from non-test/benchmarking code
a55db4ea1cf10e0ab4a6eb5cd1dd3bd95626fba0 Add more proper thread safety annotations (Hennadii Stepanov)
8cfe93e3fcf263bf059f738d5e7d9c94901a7c5a Add proper thread safety annotation to `CWallet::GetTxConflicts()` (Hennadii Stepanov)
ca446f2c59720c1575aeeab9c9d636d98ce8528c Add proper thread safety annotation to `CachedTxGetAvailableCredit()` (Hennadii Stepanov)
Pull request description:
In non-test/benchmarking code, there are three cases of the `NO_THREAD_SAFETY_ANALYSIS` annotation which are accompanied with `TODO` comments.
This PR adds proper thread safety annotations instead of `NO_THREAD_SAFETY_ANALYSIS`.
ACKs for top commit:
laanwj:
Code review ACK a55db4ea1cf10e0ab4a6eb5cd1dd3bd95626fba0
Tree-SHA512: 806d72eebc1edf088bfa435c8cd11465be0de6789798dd92abd008425516768acb864a73d834a49d412bb10f7fccfb47473f998cb72739dab6caeef6bcfaf191
|
|
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
|
|
6b9d53e1ff0099a8d9abb3c389df96fa75eac3f5 guix: native GCC 10 toolchain for Linux builds (fanquake)
88fd3f81ec626c363a5846089d99305a9a9b343d guix: use -fcommon when building glibc 2.24 (fanquake)
0e51913595c4fbe32ce9811e3ff5bcf1ed5f9ddc guix: fix glibc 2.27 multiple definition warnings with GCC 10 (fanquake)
508bd4d35720fd611a7bdfed559f46e7dbc70272 guix: adjust RISC-V __has_include() patch to work with GCC 10 (fanquake)
c9c5b3060d2edb47ebfa7974fdde3154036717c2 guix: compile glibc without -werror (fanquake)
Pull request description:
Completes the migration to using a native GCC 10 toolchain for all HOSTS. This change means we'll now use GCC 10 when compiling glibc and friends (currently we use GCC 7), which is the same as our release compiler, except for macOS (Clang 10). See each commit for more details.
Guix build (x86_64):
```bash
9f7ef2dc4421aded7f594c272c4feb1fe04f70b6c3f1ab85ed40242851cc6193 guix-build-6b9d53e1ff00/output/aarch64-linux-gnu/SHA256SUMS.part
216fde83c860a59d14a03c0a5f27c1d11ba40388da280dd42843d7c24b652a47 guix-build-6b9d53e1ff00/output/aarch64-linux-gnu/bitcoin-6b9d53e1ff00-aarch64-linux-gnu-debug.tar.gz
55b8bef29285dcd066156c2eaccd99f7d6956c3d9691363ac7482ad459856fdc guix-build-6b9d53e1ff00/output/aarch64-linux-gnu/bitcoin-6b9d53e1ff00-aarch64-linux-gnu.tar.gz
f190e12f5d2fe8bfd891421752c8f31f728c7db66736ca46f97c1c2f3a346583 guix-build-6b9d53e1ff00/output/arm-linux-gnueabihf/SHA256SUMS.part
ac4abd22b115896ba870a3f2149c66b3ce9bd25b401a75cf560681276bacc99d guix-build-6b9d53e1ff00/output/arm-linux-gnueabihf/bitcoin-6b9d53e1ff00-arm-linux-gnueabihf-debug.tar.gz
9a9a26f15b90ca5e22687272b7c9487863106f358f54b4a4cd9bcc844e96259f guix-build-6b9d53e1ff00/output/arm-linux-gnueabihf/bitcoin-6b9d53e1ff00-arm-linux-gnueabihf.tar.gz
96f73e9f17e19720e3517ebfed06a4b5295759906186770627fb8c0beb18508a guix-build-6b9d53e1ff00/output/arm64-apple-darwin/SHA256SUMS.part
4c9937e7221c56373808feacff38492f4530a4db986a07da3e56d1a293a59569 guix-build-6b9d53e1ff00/output/arm64-apple-darwin/bitcoin-6b9d53e1ff00-arm64-apple-darwin-unsigned.dmg
f7c447fb40fa0d1382db0ec6b20a45c6dbc8660c004d41fa7418e40cc684200f guix-build-6b9d53e1ff00/output/arm64-apple-darwin/bitcoin-6b9d53e1ff00-arm64-apple-darwin-unsigned.tar.gz
067bbc0f7a50be93e469af855ab8bbb9dd598ee970db9ccfc99621a93d725348 guix-build-6b9d53e1ff00/output/arm64-apple-darwin/bitcoin-6b9d53e1ff00-arm64-apple-darwin.tar.gz
d8ac116bec19dde955c5d19c85d41e3899a75def3e1c27bc047aa17906d094af guix-build-6b9d53e1ff00/output/dist-archive/bitcoin-6b9d53e1ff00.tar.gz
822085203ae9a64de3443141cd0a5f222a344451a7fdc69820efd0aeee8eca5e guix-build-6b9d53e1ff00/output/powerpc64-linux-gnu/SHA256SUMS.part
78a488dad3acf22d99f97d7874c5bf0fc0bd83bd33d27af8f1a723cd949df1d9 guix-build-6b9d53e1ff00/output/powerpc64-linux-gnu/bitcoin-6b9d53e1ff00-powerpc64-linux-gnu-debug.tar.gz
918477bee628771f3b927dba0e0e0ca0d0708cfe60a0cb47c10b98c403c9b266 guix-build-6b9d53e1ff00/output/powerpc64-linux-gnu/bitcoin-6b9d53e1ff00-powerpc64-linux-gnu.tar.gz
738e2771d4a6141cd69838bb65f54d853032075c077e428c6daf1eabc9046fc0 guix-build-6b9d53e1ff00/output/powerpc64le-linux-gnu/SHA256SUMS.part
5e7ce848931c790780154f276fae9d2b8dc03c0e995bb123d950e432a54803bb guix-build-6b9d53e1ff00/output/powerpc64le-linux-gnu/bitcoin-6b9d53e1ff00-powerpc64le-linux-gnu-debug.tar.gz
8caaf8bdc5e0a13a2e1c7242940d505c28a5fd2f2727c592b18f55530558f8c3 guix-build-6b9d53e1ff00/output/powerpc64le-linux-gnu/bitcoin-6b9d53e1ff00-powerpc64le-linux-gnu.tar.gz
8aeed7814f839aa0624752090ae75dc77f6098d2ac2b4d526945e42efeff16b8 guix-build-6b9d53e1ff00/output/riscv64-linux-gnu/SHA256SUMS.part
e694fa6a3ca56fd121afe3cbf26cf9c17d0b4bac424e1b9086a095d63fc6f0fb guix-build-6b9d53e1ff00/output/riscv64-linux-gnu/bitcoin-6b9d53e1ff00-riscv64-linux-gnu-debug.tar.gz
8241e6c1f1a669ca5144b90796235d2a4b9c08bf75d6d2bf1b2862df8da626b3 guix-build-6b9d53e1ff00/output/riscv64-linux-gnu/bitcoin-6b9d53e1ff00-riscv64-linux-gnu.tar.gz
84d0d1391e07ac55684e107492def79fa0a6e404f2ec10f3130bd0734d031ad9 guix-build-6b9d53e1ff00/output/x86_64-apple-darwin/SHA256SUMS.part
cde5bf4c3b1880b81b887b1b13293c1e91aac3f4ca9895aba515675e3dc69d1a guix-build-6b9d53e1ff00/output/x86_64-apple-darwin/bitcoin-6b9d53e1ff00-x86_64-apple-darwin-unsigned.dmg
906cebff955e514202a0d93fcf0782441f42c7592ade3205c908554086322440 guix-build-6b9d53e1ff00/output/x86_64-apple-darwin/bitcoin-6b9d53e1ff00-x86_64-apple-darwin-unsigned.tar.gz
c8a4522380d5ff22c800d73968b4758ed5edde346fc6720558285ce02251dec0 guix-build-6b9d53e1ff00/output/x86_64-apple-darwin/bitcoin-6b9d53e1ff00-x86_64-apple-darwin.tar.gz
de2e24dfe08f31bfeca03aff70f3e38b2671272f357b430ce8a3d3879020745f guix-build-6b9d53e1ff00/output/x86_64-linux-gnu/SHA256SUMS.part
32c01add177f3a1c7fb85f1c687b502105a98919a0fa9e8675917c68cb651dfb guix-build-6b9d53e1ff00/output/x86_64-linux-gnu/bitcoin-6b9d53e1ff00-x86_64-linux-gnu-debug.tar.gz
c2af2390d8232463c824756ba4d88194f6fc3bfd0f71286237f2d3067d67ff25 guix-build-6b9d53e1ff00/output/x86_64-linux-gnu/bitcoin-6b9d53e1ff00-x86_64-linux-gnu.tar.gz
b3fa5cc4dbe908274f84ca4ee4bd8fad7b0d0678b5e47ca8e0134b8872d1262c guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/SHA256SUMS.part
69cff7bfa42918434f7aee4a5c1f87c824bfe387d5a40bea502437e320703b68 guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/bitcoin-6b9d53e1ff00-win64-debug.zip
6c4ce699bdc9cd0fd5b3626b7c740b0d9f381f126a6581b38d481bddca74c25d guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/bitcoin-6b9d53e1ff00-win64-setup-unsigned.exe
f5ca2fc6988e9a90ed7c47bd05c120a8d5a2c2a0bc0abcc076739d27869779b7 guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/bitcoin-6b9d53e1ff00-win64-unsigned.tar.gz
73862bfc4c6a614e467b0b4f07a7264e4a758bacbf75c0ac49b76d253385dcec guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/bitcoin-6b9d53e1ff00-win64.zip
```
Guix build (arm64):
```bash
63fd172e3bf01fe47e845c7b5af76b56b40ecd26f77363c9e5782c12997a1f3b guix-build-6b9d53e1ff00/output/arm-linux-gnueabihf/SHA256SUMS.part
6007a3ab95315a9e7206f32f13b6fc574833afd3e3d1ea0ed905800016fbf786 guix-build-6b9d53e1ff00/output/arm-linux-gnueabihf/bitcoin-6b9d53e1ff00-arm-linux-gnueabihf-debug.tar.gz
a172a098403a29cc6c2fdb8fdd388fab10e0e2477f78cb8c7ee0d8112442c5f4 guix-build-6b9d53e1ff00/output/arm-linux-gnueabihf/bitcoin-6b9d53e1ff00-arm-linux-gnueabihf.tar.gz
e07565d39160db87a857edc8ea4dc4444476837fc9d85fb17245efaa68b4ec41 guix-build-6b9d53e1ff00/output/arm64-apple-darwin/SHA256SUMS.part
b9cc419a750afd5688c5f2dbfb717fccb5f177fb60b07bfede792336f7a4c563 guix-build-6b9d53e1ff00/output/arm64-apple-darwin/bitcoin-6b9d53e1ff00-arm64-apple-darwin-unsigned.dmg
d9105b702f6756645efba5a4e47fc1efbca178e0a76b30c46dd6333f2362c1a2 guix-build-6b9d53e1ff00/output/arm64-apple-darwin/bitcoin-6b9d53e1ff00-arm64-apple-darwin-unsigned.tar.gz
d46ecdc6f485d78d8afe4e6d679e97844dc6f817cd470226290843b9e0a5c677 guix-build-6b9d53e1ff00/output/arm64-apple-darwin/bitcoin-6b9d53e1ff00-arm64-apple-darwin.tar.gz
d8ac116bec19dde955c5d19c85d41e3899a75def3e1c27bc047aa17906d094af guix-build-6b9d53e1ff00/output/dist-archive/bitcoin-6b9d53e1ff00.tar.gz
8f35311efd75f1a2a27c5090a3134648a3e58d40692d363f1fe0afce08925bcb guix-build-6b9d53e1ff00/output/powerpc64-linux-gnu/SHA256SUMS.part
b012ce67aa18f54ea688961a01de30f9b4127fee7184d1292ac8f664449972f5 guix-build-6b9d53e1ff00/output/powerpc64-linux-gnu/bitcoin-6b9d53e1ff00-powerpc64-linux-gnu-debug.tar.gz
3ebd59090b02a295965923a1d74a2ce0aa23774d59116f62668e2a246343d971 guix-build-6b9d53e1ff00/output/powerpc64-linux-gnu/bitcoin-6b9d53e1ff00-powerpc64-linux-gnu.tar.gz
2017c04652d18a29107e06a1efee44d93d01607f12f7af626e687a72eca06a7c guix-build-6b9d53e1ff00/output/powerpc64le-linux-gnu/SHA256SUMS.part
4520225674157af40168813a2497a2f79df8131b0a73251a3d44dbe67cb002d2 guix-build-6b9d53e1ff00/output/powerpc64le-linux-gnu/bitcoin-6b9d53e1ff00-powerpc64le-linux-gnu-debug.tar.gz
9e7660470d56573c9c08470383d745910f2834586182a51b661eb10cc41fbf1d guix-build-6b9d53e1ff00/output/powerpc64le-linux-gnu/bitcoin-6b9d53e1ff00-powerpc64le-linux-gnu.tar.gz
2015b8d13798746e4d7e3a75250a2b83bb8cb9289410a1576f0e9892732e1931 guix-build-6b9d53e1ff00/output/riscv64-linux-gnu/SHA256SUMS.part
735c1a285933499406a4f6c396a12b13fcf79188100b541d532055c06092741c guix-build-6b9d53e1ff00/output/riscv64-linux-gnu/bitcoin-6b9d53e1ff00-riscv64-linux-gnu-debug.tar.gz
f541d5397e28558dea1976a79f6bfdb01f12d7b4031925247cb32ace2360782e guix-build-6b9d53e1ff00/output/riscv64-linux-gnu/bitcoin-6b9d53e1ff00-riscv64-linux-gnu.tar.gz
84d0d1391e07ac55684e107492def79fa0a6e404f2ec10f3130bd0734d031ad9 guix-build-6b9d53e1ff00/output/x86_64-apple-darwin/SHA256SUMS.part
cde5bf4c3b1880b81b887b1b13293c1e91aac3f4ca9895aba515675e3dc69d1a guix-build-6b9d53e1ff00/output/x86_64-apple-darwin/bitcoin-6b9d53e1ff00-x86_64-apple-darwin-unsigned.dmg
906cebff955e514202a0d93fcf0782441f42c7592ade3205c908554086322440 guix-build-6b9d53e1ff00/output/x86_64-apple-darwin/bitcoin-6b9d53e1ff00-x86_64-apple-darwin-unsigned.tar.gz
c8a4522380d5ff22c800d73968b4758ed5edde346fc6720558285ce02251dec0 guix-build-6b9d53e1ff00/output/x86_64-apple-darwin/bitcoin-6b9d53e1ff00-x86_64-apple-darwin.tar.gz
51c1c6f683896a23767af2a7c6afe07030b6d5ad5a136b65f31b7e1f685c0f28 guix-build-6b9d53e1ff00/output/x86_64-linux-gnu/SHA256SUMS.part
6276d1ee54575b16386866f4898033d4dce9ea5a4e4635f7ae65d5bddecba35b guix-build-6b9d53e1ff00/output/x86_64-linux-gnu/bitcoin-6b9d53e1ff00-x86_64-linux-gnu-debug.tar.gz
c553c0dfdd85f9af8e41741557c9eb5a6ee48a9f4a025df84eed21330927be89 guix-build-6b9d53e1ff00/output/x86_64-linux-gnu/bitcoin-6b9d53e1ff00-x86_64-linux-gnu.tar.gz
3dc586f08f454cfba3e8f66588a1017490fc6014bcd2aee0b0c5f557267aa961 guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/SHA256SUMS.part
bc5ed4302603fe2f574e83a0e5612cff306081f65c74d817bb0f977bef01fb7a guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/bitcoin-6b9d53e1ff00-win64-debug.zip
6c4ce699bdc9cd0fd5b3626b7c740b0d9f381f126a6581b38d481bddca74c25d guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/bitcoin-6b9d53e1ff00-win64-setup-unsigned.exe
f5ca2fc6988e9a90ed7c47bd05c120a8d5a2c2a0bc0abcc076739d27869779b7 guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/bitcoin-6b9d53e1ff00-win64-unsigned.tar.gz
761c97a37473f91dee8630a1409597ee400a8c8c99937f0a111e4572f084bdd9 guix-build-6b9d53e1ff00/output/x86_64-w64-mingw32/bitcoin-6b9d53e1ff00-win64.zip
```
Closes #24701.
ACKs for top commit:
hebasto:
ACK 6b9d53e1ff0099a8d9abb3c389df96fa75eac3f5
Tree-SHA512: 128981d6ee68a9824bf9f19f90502b26e9d0fc5d55bf70b44c49fc8bdd25d4c6adf6fe2a5f6e48b35eb6e1b6ba55db59528cd53e75ddc34fc74f5d0ab0a33cb1
|
|
c6122f560b5ed57f11441d2616108b43b10721bb test: use sendall in wallet_taproot.py tests (ishaanam)
Pull request description:
Fixes #25129 (subtractfeefromamount=true fails with insufficient
funds)
ACKs for top commit:
achow101:
ACK c6122f560b5ed57f11441d2616108b43b10721bb
Xekyo:
tACK c6122f560b5ed57f11441d2616108b43b10721bb
brunoerg:
ACK c6122f560b5ed57f11441d2616108b43b10721bb
Tree-SHA512: c73512852ced6216eab80f4079d6e3d5ba949fbc6bfea5f4034c7fa200b0048e97a1451274a142deb4f698de0702a8940957be8a00ebd2c19cf50604b21016d4
|
|
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
|
|
`make check`
4f31c21b7f384e50e0af8275e831085d1d69b386 bench: Make all arguments -kebab-case (laanwj)
652b54e53220fedf2c342e428617bcbd2022964d bench: Add `--sanity-check` flag, use it in `make check` (laanwj)
Pull request description:
The benchmarks are run as part of `make check` for a crash-sanity check. The actual results are being ignored. So only run them for one iteration.
This makes the `bench_bitcoin` part take 2m00 instead of 5m20 here. Which is still too long (imo), but this needs to be solved in the `WalletLoading*` benchmarks which take that long per iteration.
Also change all `bench_bitcoin` arguments to kebab-case to be consistent with the other tools (in a separate commit).
ACKs for top commit:
jonatack:
ACK 4f31c21b7f384e50e0af8275e831085d1d69b386 on the sanity-check version per `git diff c52a71e 4f31c28` (modulo s/--sanity check/--sanity-check/ in src/bench/bench.cpp::L61)
hebasto:
ACK 4f31c21b7f384e50e0af8275e831085d1d69b386, tested on Ubuntu 22.04.
Tree-SHA512: 2661d130fd82e57c9041755190997a4af588fadddcdd05e04fd024f75da1202480e9feab5764566e8dfe7930e8ae0ec71e93f40ac373274953d274072723980d
|
|
to avoid unnecessary invocations and an unneeded local variable allocation.
|
|
Fixes negative time duration values in the "send", "recv",
and "age" columns (potentially the "txn" and "blk" columns also)
for the first run of -rpcwait -netinfo after bitcoind startup.
To reproduce, start bitcoind on mainnet and run
`bitcoin-cli -rpcwait -netinfo <n>` where n is 1 or larger.
The negative times will be larger/more apparent with a slower
CPU speed or e.g. higher checkblocks/checklevel config option
settings.
|
|
CreateTransaction() as optional struct
4c5ceb040cf50d24201903a9200fb23be88d96fb wallet: CreateTransaction(): return out-params as (optional) struct (Sebastian Falbesoner)
c9fdaa5e3ae09b45be6a5c2d4ee6b1e8cef9d8a8 wallet: CreateTransactionInternal(): return out-params as (optional) struct (Sebastian Falbesoner)
Pull request description:
The method `CWallet::CreateTransaction` currently returns several values in the form of out-parameters:
* the actual newly created transaction (`CTransactionRef& tx`)
* its required fee (`CAmount& nFeeRate`)
* the position of the change output (`int& nChangePosInOut`) -- as the name suggests, this is both an in- and out-param
By returning these values in an optional structure (which returns no value a.k.a. `std::nullopt` if an error occured), the interfaces is shorter, cleaner (requested change position is now in-param and can be passed by value) and callers don't have to create dummy variables for results that they are not interested in.
Note that the names of the replaced out-variables were kept in `CreateTransactionInternal` to keep the diff minimal. Also, the fee calculation data (`FeeCalculation& fee_calc_out`) would be another candidate to put into the structure, but `FeeCalculation` is currently an opaque data type in the wallet interface and I think it should stay that way.
As a potential follow-up, I think it would make sense to also do the same refactoring for `CWallet::FundTransaction`, which has a very similar parameter structure.
Suggested by laanwj in https://github.com/bitcoin/bitcoin/pull/20588#issuecomment-739838428.
ACKs for top commit:
achow101:
re-ACK 4c5ceb040cf50d24201903a9200fb23be88d96fb
Xekyo:
ACK 4c5ceb040cf50d24201903a9200fb23be88d96fb
w0xlt:
crACK https://github.com/bitcoin/bitcoin/pull/20640/commits/4c5ceb040cf50d24201903a9200fb23be88d96fb
Tree-SHA512: 27e5348bbf4f698713002d40c834dcda59c711c93207113e14522fc6d9ae7f4d8edf1ef6d214c5dd62bb52943d342878960ca333728828bf39b645a27d55d524
|
|
This is customary for UNIX-style arguments, and more consistent with our
other tools
|
|
The benchmarks are run as part of `make check` for a minimum sanity
check. The actual results are being ignored. So only run them for one
iteration.
This makes the `bench_bitcoin` part take 2m00 instead of 5m20 here.
Which is still too long (imo), but this needs to be solved in the
`WalletLoading*` benchmarks which take that long per iteration.
|
|
`m_most_recent_block_mutex` with Mutex
83003ffe049a432f6fa4127e054f073127e70b90 refactor: replace RecursiveMutex `m_most_recent_block_mutex` with Mutex (Sebastian Falbesoner)
8edd0d31ac683378135a9839e5d4172b82f8f5b8 refactor: reduce scope of lock `m_most_recent_block_mutex` (Sebastian Falbesoner)
Pull request description:
This PR is related to #19303 and gets rid of the RecursiveMutex `m_most_recent_block_mutex`. All of the critical sections (5 in total) only directly access the guarded elements, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex:
https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L1650-L1655
https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L1861-L1865
https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L3149-L3152
https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L3201-L3206
https://github.com/bitcoin/bitcoin/blob/b019cdc036343a437fd7ced85467bd95f48d84c4/src/net_processing.cpp#L4763-L4769
The scope of the last critical section is reduced in the first commit, in order to avoid calling the non-trivial method `CConnman::PushMessage` while the lock is held.
ACKs for top commit:
furszy:
Code ACK 83003ffe with a small comment.
hebasto:
ACK 83003ffe049a432f6fa4127e054f073127e70b90
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/24062/commits/83003ffe049a432f6fa4127e054f073127e70b90
Tree-SHA512: 3df290cafd2f6c4d40afb9f14e822a77d9c1828e66f5e2233f3ac1deccc2b0a8290bc5fb8eb992f49d39e887b50bc0e9aad63e05db2d870791a8d409fb95695f
|
|
getblockchaininfo
a01b92ad8672dcf4369ee9cf36a6b0157d73786c doc: add release notes about removal of the `deprecatedrpc=softforks` flag (Sebastian Falbesoner)
8c5533c7a953e79b423b465905dbfaa45ad60a49 rpc: remove deprecated "softforks" field from getblockchaininfo (Sebastian Falbesoner)
Pull request description:
Information on soft fork status has been moved from the `getblockchaininfo` RPC to the `getdeploymentinfo` RPC in #23508. The "softfork" result in `getblockchaininfo` was still available for 23.0 with the `-deprecatedrpc=softforks` configuration option, but this can be fully removed now for the next release (24.0).
ACKs for top commit:
shaavan:
ACK a01b92ad8672dcf4369ee9cf36a6b0157d73786c
ajtowns:
ACK a01b92ad8672dcf4369ee9cf36a6b0157d73786c
Tree-SHA512: 692d9d02fdf0b3c18376644a85b24b57efebf612738084c01ef47d47e41861e773688613a808e81f10ab6eec340de00eef96987a1e34d612aaf7f0a0b134d89e
|
|
ExternalSigner::SignTransaction
2a22f034ca3298c9f86d1edd4283a0bea18dfbbe parsing external signer master fingerprint string as bytes instead of caring for lower/upper case in ExternalSigner::SignTransaction (avirgovi)
Pull request description:
Some external signers scripts may provide master fingerprint in uppercase format. In that case core will fail with `Signer fingerprint 00000000 does not match any of the inputs` as it only works with lowercase format. Even if the fingerprints match, yet one is lowercase the other uppercase.
ExternalSigner::SignTransaction is the only place where it is needed IMO, as changing it in other places may break the communication with the external signer (i.e. enumerating with lowercase may not find the device).
ACKs for top commit:
achow101:
ACK 2a22f034ca3298c9f86d1edd4283a0bea18dfbbe
theStack:
Code-review ACK 2a22f034ca3298c9f86d1edd4283a0bea18dfbbe
Sjors:
utACK 2a22f034ca3298c9f86d1edd4283a0bea18dfbbe
Tree-SHA512: f3d84b83fb0b5e06c405eaf9bf20a2fa864bf4172fd4de113b80b9b9a525a76f2f8cf63031b480358b3a7666023a2aef131fb89ff50448c66df3ed541da10f99
|
|
registering for signals
ba10b90915dae6a802ecb0f80f72a1a9ea5a4c67 Wallet: Ensure m_attaching_chain is set before registering for signals (Luke Dashjr)
Pull request description:
Avoids a race where chainStateFlushed could be called before rescanning began, yet rescan gets interrupted or fails
Followup for #24984 avoiding a race between registering and setting the flag.
ACKs for top commit:
mzumsande:
Code Review ACK ba10b90915dae6a802ecb0f80f72a1a9ea5a4c67
achow101:
ACK ba10b90915dae6a802ecb0f80f72a1a9ea5a4c67
Tree-SHA512: 1d2fa2db189d3e87f2d0863cf2ab62166094436483f0da16760b1083a4743bf08e476a3277e0d36564213d65dd6f0a1fc16a4bf68d3338c991a14d1de9fc0fee
|
|
|
|
|
|
|
|
performance
f336ff7f213564909cf5f9742618cc6ec87600fd rpc: avoid expensive `IsMine` calls in `GetReceived` tally (Sebastian Falbesoner)
a7b65af2a4450663d4a20a617c59dac87b34fb36 rpc: avoid scriptPubKey<->CTxDestination conversions in `GetReceived` tally (Sebastian Falbesoner)
Pull request description:
The RPC calls `getreceivedbyaddress`/`getreceivedbylabel` both use the internal helper function `GetReceived` which was introduced in PR #17579 to deduplicate tallying code. For every wallet-related transaction output, the following unnecessary operations are currently performed in the tally loop, leading to a quite bad performance (as reported in #23645):
- converting from CScript -> TxDestination (`ExtractDestination(...)`), converting from TxDestination -> CScript (`CWallet::IsMine(const CTxDestination& dest)`); this can be avoided by directly using output scripts in the search set instead of addresses (first commit)
- checking if the iterated output script belongs to the wallet by calling `IsMine`; this can be avoided by only adding addresses to the search set which fulfil `IsMine` in the first place (second commit)
### Benchmark results
The functional test [wallet_pr23662.py](https://github.com/theStack/bitcoin/blob/pr23662_benchmarks/test/functional/wallet_pr23662.py) (not part of this PR) creates transactions with 15000 different addresses:
- 5000 outputs (500 txs with 10 outputs each) with label set, IsMine set (received)
- 5000 outputs (500 txs with 10 outputs each) with label set, IsMine not set (sent)
- 5000 outputs (500 txs with 10 outputs each) without label set, IsMine not set (sent)
Then, the time is measured for calling `getreceivedbyaddress` and `getreceivedbylabel`, the latter with two variants. Results on my machine:
| branch | `getreceivedbyaddress` (single) | `getreceivedbylabel` (single) | `getreceivedbylabel` (10000) |
|--------------------|---------------------------------|-------------------------------|------------------------------|
| master | 406.13ms | 425.33ms | 446.58ms |
| PR (first commit) | 367.18ms | 365.81ms | 426.33ms |
| PR (second commit) | 3.96ms | 4.83ms | 339.69ms |
Fixes #23645.
ACKs for top commit:
achow101:
ACK f336ff7f213564909cf5f9742618cc6ec87600fd
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/23662/commits/f336ff7f213564909cf5f9742618cc6ec87600fd
furszy:
Code ACK f336ff7f
Tree-SHA512: 9cbf402b9e269713bd3feda9e31719d9dca8a0dfd526de12fd3d561711589195d0c50143432c65dae279c4eab90a4fc3f99e29fbc0452fefe05113e92d129b8f
|
|
|
|
|
|
11e79084845a78e2421ea3abafe0de5a54ca2bde prevector: only allow trivially copyable types (Martin Leitner-Ankerl)
Pull request description:
prevector uses `memmove` to move around data, that means it can only be used with types that are trivially copyable. That implies that the types are trivially destructible, thus the checks for `is_trivially_destructible` are not needed.
ACKs for top commit:
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/24962/commits/11e79084845a78e2421ea3abafe0de5a54ca2bde
MarcoFalke:
review ACK 11e79084845a78e2421ea3abafe0de5a54ca2bde 🏯
ajtowns:
ACK 11e79084845a78e2421ea3abafe0de5a54ca2bde -- code review only
Tree-SHA512: cbb4d8bfa095100677874b552d92c324c7d6354fcf7adab2ed52f57bd1793762871798b5288064ed1af2d2903a0ec9dbfec48d99955fc428f18cc28d6840dccc
|
|
fae3200bbf2a0f4482ec6f5f2ada9534f6c82709 test: Slim down versionbits_tests.cpp (MacroFake)
Pull request description:
Seems confusing to spin up a full chainman that isn't even used.
Fix that by only spinning up logging. Also, remove the chainman include and comment.
ACKs for top commit:
fanquake:
ACK fae3200bbf2a0f4482ec6f5f2ada9534f6c82709
Tree-SHA512: 35261e9116c0c276f807453db3d635d83916ec2ffd99cf5641f8732736a30a542213096dcec550ef4522d97b3cafe384fdc6068138bc0b577c66fa61256719f8
|
|
In each of the critical sections, only the the guarded variables are
accessed, without any chance that within one section another one is
called. Hence, we can use an ordinary Mutex instead of RecursiveMutex.
|
|
fa347a906685df1d44cafa3e6cc7fdd2ace68ff5 rpc: Fix implicit-integer-sign-change in gettxout (MacroFake)
Pull request description:
ACKs for top commit:
theStack:
Code-review ACK fa347a906685df1d44cafa3e6cc7fdd2ace68ff5
Tree-SHA512: 2a1128f714119b6b6cfeb20ee59c4f46404d5a83942253c73de64b0289a7d41e4437cf77d19b1cf623e2dd15fbaa1ec7acd03cc5d6dde41b3c7d06a082073ea1
|
|
This avoids calling the non-trivial method
`CConnman::PushMessage` within the critical section.
|
|
436ce0233c276e263dcb441255dc0b881cb39cfb sync.h: strengthen AssertLockNotHeld assertion (Anthony Towns)
7d73f58e9cea8f4b0bc16512983898fddde3d764 Increase threadsafety annotation coverage (Anthony Towns)
Pull request description:
This changes `AssertLockNotHeld` so that it is annotated with the negative capability for the mutex it refers to. clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions.
Note that this can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex. At present, the only global mutexes that use `AssertLockNotHeld` are `RecursiveMutex` so we treat that as an exception in order to avoid having to add an excessive number of negative annotations.
ACKs for top commit:
vasild:
ACK 436ce0233c276e263dcb441255dc0b881cb39cfb
MarcoFalke:
review ACK 436ce0233c276e263dcb441255dc0b881cb39cfb 🌺
Tree-SHA512: 5f16d098790a36b5277324d5ee89cdc87033c19b11c7943c2f630a41c2e3998eb39d356a763e857f4d8fefb6c0c02291f720bb6769bcbdf5e2cd765bf266ab8c
|
|
blocks
a50e34c367608fcec9697893981bfa294a7c08d1 [net processing] Remove redundant nodestate->m_sendcmpct check in MaybeSetPeerAsAnnouncingHeaderAndIDs() (John Newbery)
bb985a7b6abee503852c61eec74ca3a29f582815 [net processing] Only relay blocks by cmpctblock and cache for fast relay if segwit is enabled (John Newbery)
3b6bfbce386f61dcbb366f08cfff55c3882f429c [net processing] Rename CNodeState compact block members (John Newbery)
d0e97741748aaaad2a89ca42e4898e7f01308b35 [net processing] Tidy up `sendcmpct` processing (John Newbery)
30c3a01874cf51d987a0ae2bb699bf50d82768ff [net processing] fPreferHeaderAndIDs implies fProvidesHeaderAndIDs (John Newbery)
b486f721767d07c1e2eaf8deaf96b732b0a858dd [net processing] Remove fWantsCmpctWitness (John Newbery)
a45d53cab556505048c387429fd07188e4c40c3d [net processing] Remove fSupportsDesiredCmpctVersion (John Newbery)
25edb2b7bd4c41156fba09d0033a978e362435af [net processing] Simplify `sendcmpct` processing (John Newbery)
42882fc8fc2ef5c58eb963f7f1e852dd43de6c65 [net processing] Only accept `sendcmpct` with version=2 (John Newbery)
16730b64bb4a11995d792f626c8a63318e5eaeeb [net processing] Only advertise support for version 2 compact blocks (John Newbery)
cba909eaf938a775a9bd2dd994d06aba175e8713 [net] Stop testing version 1 compact blocks. (John Newbery)
Pull request description:
Compact blocks are used for efficient relay of blocks, either through High Bandwidth or Low Bandwidth mode. See [BIP 152](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki) for full details.
For compact block relay to work, the receiver must have a mempool containing transactions which are likely to be included in the block. The receiver uses these transactions to reconstruct the block from the short transaction ids included in the `cmpctblock` message. Compact blocks are therefore only useful for relaying blocks at or near the tip of the block chain. For older blocks, the recipient won't have the transactions in their mempool and so would need to request them using a `getblocktxn` message. In such cases, just requesting the full block is more efficient.
BIP 152 supports two versions: version 1 (without witnesses) and version 2 (with witnesses). Version 2 is required to reconstruct segwit blocks. Segwit was activated in August 2017, and providing non-witness blocks to peers is no longer useful. Since the witnesses are not included, the peer would not be able to fully validate all the consensus rules on the provided block.
Therefore, stop supporting version 1 compact blocks. Ignore `sendcmpct` messages with version=1, and don't advertise support by sending `sendcmpct` with version=1. Only send `sendcmpct` to peers with `NODE_WITNESS`. Respond to all requests for compact blocks or blocktxns with witness-serialized blocks and transactions.
ACKs for top commit:
dergoegge:
ACK a50e34c367608fcec9697893981bfa294a7c08d1 - Only changes since my last review were a rebase and some nits.
MarcoFalke:
re-ACK a50e34c367608fcec9697893981bfa294a7c08d after rebase 👓
Tree-SHA512: 8ad73acaa374d56ee9f28ca0a9d64b8630793d22fc8c942a1ee15551d4d80f76f3ba937682f85f22ec8ca82efae98a92de75a7e2f5a97499d8f9c7e4f2833c86
|
|
getblockchaininfo's pruneheight result
06822f86545a0e946fdc266c57955f98d163a8bc Bugfix: RPC/blockchain: Correct description of getblockchaininfo's pruneheight result (Luke Dashjr)
Pull request description:
It is possible that lower blocks are complete due to being stored in the same file as blocks not yet eligible for pruning.
Not really satisfied with this new description, so suggestions for better phasing welcome :)
(Split out of #24629)
ACKs for top commit:
theStack:
Code-review ACK 06822f86545a0e946fdc266c57955f98d163a8bc
Tree-SHA512: 755a5a40d065ad77f4ac2c19c0b3502eceb3162034823ee7ce1668100d97e8a2bfb822ac381feb7afd13e653cd08a81d5fa505575531757457d6d22c909a6510
|
|
a class, drop unused forward declarations
ca1ac1f0e0fbabbe97666aca94024afb8104da06 scripted-diff: Rename MainSignalsInstance() class to MainSignalsImpl() (Jon Atack)
2aaec2352d14753e1f2d4bf4b11bbe6ad42edf5c refactor: remove unused forward declarations in validationinterface.h (Jon Atack)
23854f84024c292164ce49b3fefee2a9b4fe0831 refactor: make MainSignalsInstance() a class (Jon Atack)
Pull request description:
Make MainSignalsInstance a class, rename it to MainSignalsImpl, use Doxygen documentation for it, and remove no longer used forward declarations in src/validationinterface.h.
----
MainSignalsInstance was created in 3a19fed9db5 and originally was a collection of boost::signals methods moved to validationinterface.cpp, in order to no longer need to include boost/signals in validationinterface.h.
MainSignalsInstance then evolved in d6815a23131 to become class-like:
- [C.8: Use class rather than struct if any member is non-public](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-class)
- [C.2: Use class if the class has an invariant; use struct if the data members can vary independently](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c2-use-class-if-the-class-has-an-invariant-use-struct-if-the-data-members-can-vary-independently)
- A class has the advantage of default private access, as opposed to public for a struct.
ACKs for top commit:
furszy:
Code ACK ca1ac1f
promag:
Code review ACK ca1ac1f0e0fbabbe97666aca94024afb8104da06.
danielabrozzoni:
Code review ACK ca1ac1f0e0fbabbe97666aca94024afb8104da06
Tree-SHA512: 25f85e2b582fe8c269e6cf384a4aa29350b97ea6477edf3c63591e4f68a97364f7fb2fc4ad2764f61ff86b27353e31d2f12eed7a23368a247e4cf271c7e133ea
|
|
654284209f30fd309c326a527cda1d2d7385cee8 Add clang lifetimebound section to developer notes (Jon Atack)
e66b321fd1ddfffd9bfc59d407ad8f03490b873c Add C++ functions and methods section to developer notes (Jon Atack)
5fca70f5b16fee4a732a1d7fd3fb1c7e775decdf Link in developer notes style to internal interface exception (Jon Atack)
fc4cb857ccfa622e76f0f8e7aa164ca4d8bd599a Prefer Python for scripts in developer notes (Jon Atack)
370120ec2ff4b5e7d5cd6678a7be7cfe651be509 Remove obsolete BDB ENABLE_WALLET section in developer notes (Jon Atack)
Pull request description:
A few updates noticed while working on a lifetimebound section.
- Remove obsolete BDB ENABLE_WALLET section (only one file, src/wallet/bdb.h, still has a `db_cxx.h` BDB header)
- Prefer Python for scripts in developer notes (and a few miscellaneous touch-ups)
- In the code style section, add a link to the internal interface exception so that people are aware of it
- Add a "C++ functions and methods" section
- Add a Clang `lifetimebound` attribute section
ACKs for top commit:
laanwj:
ACK 654284209f30fd309c326a527cda1d2d7385cee8
jarolrod:
code review ACK https://github.com/bitcoin/bitcoin/commit/654284209f30fd309c326a527cda1d2d7385cee8
Tree-SHA512: d2ae335cac899451d5c7bdc8e94fd82053a5f44ac1ba444bdde71abaaa24a519713c1078f3a8f07ec8466b181788a613fd3c68061e54b3fdc8cd6f3e3f9791ec
|
|
MaybeSetPeerAsAnnouncingHeaderAndIDs()
|
|
segwit is enabled
This introduces an early exit in PeerManagerImpl::NewPoWValidBlock() if
segwit has not been activated for the block. This means that we won't cache the
block/compact block for fast relay and won't relay the cmpctblock
immediately to peers that have requested hb compact blocks. This is fine
because any block where segwit is not yet activated is buried deep in
the chain, and so compact block relay will not be effective.
It's ok not to cache the block/compact block for fast relay for the same
reason - the block must be very deeply buried in the block chain.
ProcessBlockAvailability() also won't get called for all nodes. This is
also fine, since that function only updates hashLastUnknownBlock
and pindexBestKnownBlock, and is called early in every SendMessages()
call.
|
|
fPreferHeaderAndIDs -> m_requested_hb_cmpctblocks
fProvidesHeaderAndIDs -> m_provides_cmpctblocks
|
|
- use better local variable names
- drop unnecessary if statements
|
|
Remove all if(fProvidesHeaderAndIDs) conditionals inside
if(fPreferHeaderAndIDs) conditionals.
|
|
It is now completely redundant with fProvidesHeadersAndIDs.
|
|
It is now completely redundant with fProvidesHeadersAndIDs.
|
|
nCMPCTBLOCKVersion must always be 2 when processing.
|
|
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.
|