aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-12-13Merge bitcoin/bitcoin#26477: validation: fix broken maxtipage comparisonfanquake
e4be0e9b0661a8af49c4e6d5472804913f04b8fc test: add -maxtipage test for the maximum allowable value (James O'Beirne) a451e832b46bcb984dfcd9478ea8ebb8b3de0c62 fix: validation: cast now() to seconds for maxtipage comparison (James O'Beirne) Pull request description: Since https://github.com/bitcoin/bitcoin/commit/faf44876db555f7488c8df96db9fa88b793f897c, the maxtipage comparison in IsInitialBlockDownload() has been broken, since the NodeClock::now() time_point is in the system's native denomination (nanoseconds). Without this patch, specifying the maximum allowable -maxtipage (9223372036854775807) results in a SIGABRT crash: ``` % gdb --args ./src/bitcoind -maxtipage=9223372036854775207 -minimumchainwork=0x00 -stopatheight=30000 ... 2022-11-09T15:55:17Z [dnsseed] dnsseed thread exit [Thread 0x7fff937fe640 (LWP 69883) exited] Thread 29 "b-msghand" received signal SIGABRT, Aborted. [Switching to Thread 0x7fff91ffb640 (LWP 69886)] __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44 44 ./nptl/pthread_kill.c: No such file or directory. (gdb) bt #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44 #1 0x00007ffff768989f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78 #2 0x00007ffff763da52 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 #3 0x00007ffff7628469 in __GI_abort () at ./stdlib/abort.c:79 #4 0x00007ffff7cf79a4 in __mulvdi3 () from /lib/x86_64-linux-gnu/libgcc_s.so.1 #5 0x00005555558d13ab in std::chrono::__duration_cast_impl<std::chrono::duration<long, std::ratio<1l, 1000000000l> >, std::ratio<1000000000l, 1l>, long, false, true>::__cast<long, std::ratio<1l, 1l> > (__d=...) at /usr/include/c++/12/bits/chrono.h:521 #6 std::chrono::duration_cast<std::chrono::duration<long, std::ratio<1l, 1000000000l> >, long, std::ratio<1l, 1l> > (__d=...) at /usr/include/c++/12/bits/chrono.h:260 #7 std::chrono::duration<long, std::ratio<1l, 1000000000l> >::duration<long, std::ratio<1l, 1l>, void> (__d=..., this=<optimized out>) at /usr/include/c++/12/bits/chrono.h:514 #8 std::chrono::operator-<long, std::ratio<1l, 1000000000l>, long, std::ratio<1l, 1l> > (__rhs=..., __lhs=...) at /usr/include/c++/12/bits/chrono.h:650 #9 std::chrono::operator-<NodeClock, std::chrono::duration<long, std::ratio<1l, 1000000000l> >, long, std::ratio<1l, 1l> > (__rhs=..., __lhs=...) at /usr/include/c++/12/bits/chrono.h:1020 #10 Chainstate::IsInitialBlockDownload (this=0x555556071940) at ./src/validation.cpp:1545 #11 0x00005555556efd1e in operator() (__closure=<optimized out>) at ./src/net_processing.cpp:3369 #12 (anonymous namespace)::PeerManagerImpl::ProcessMessage (this=0x555556219be0, pfrom=..., msg_type=..., vRecv=..., time_received=..., interruptMsgProc=...) at ./src/net_processing.cpp:3369 #13 0x00005555556f75cc in (anonymous namespace)::PeerManagerImpl::ProcessMessages (this=0x555556219be0, pfrom=<optimized out>, interruptMsgProc=std::atomic<bool> = { false }) at ./src/net_processing.cpp:4985 #14 0x00005555556a83c9 in CConnman::ThreadMessageHandler (this=0x5555560ebc70) at ./src/net.cpp:2014 #15 0x0000555555c4d5d6 in std::function<void ()>::operator()() const (this=0x7fff91ffadb0) at /usr/include/c++/12/bits/std_function.h:591 #16 util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) ( thread_name="0\255\377\221\377\177\000\000\v\000\000\000\000\000\000\000TraceThread\000\000\000\000\000P\255\377\221\377\177\000\000\017\000\000\000\000\000\000\000util/thread.cpp\000\000\000\000\000\000\000\000\000\000ihB鵿6\000\000\000\000\000\000\000\000\260\255\377\221\377\177\000\000\277\211\321UUU\000\000p\324\304UUU\000\000\002\000\000\000\000\000\000\000\240xh\367\377\177\000\000\000\000\000\000\000\000\000\000]\340iUUU\000\000p\274\016VUU\000\000\000\000\000\000\000\000\000\000\300\303iUUU\000\000p\206jUUU", '\000' <repeats 11 times>, "ihB鵿6\200\251!VUU\000\000"..., thread_func=...) at util/thread.cpp:21 #17 0x000055555569e05d in std::__invoke_impl<void, void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > (__f=<optimized out>) at /usr/include/c++/12/bits/invoke.h:61 #18 std::__invoke<void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > (__fn=<optimized out>) at /usr/include/c++/12/bits/invoke.h:96 #19 std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > >::_M_invoke<0, 1, 2> (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:252 #20 std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > >::operator() (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:259 #21 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > > >::_M_run(void) (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:210 #22 0x00007ffff7ad43d3 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #23 0x00007ffff7687b27 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:435 #24 0x00007ffff770a78c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 (gdb) ``` ACKs for top commit: MarcoFalke: review ACK e4be0e9b0661a8af49c4e6d5472804913f04b8fc 🏽 Tree-SHA512: d892d6264a284d952a68a8631a6301277373b8df939dafd9e2652f2f22ab60712cde63b90c27c67ea2d05f02443452e3e4e1b9f25479bfaca00d4c4de13b9fbd
2022-12-12Merge bitcoin/bitcoin#26199: p2p: Don't self-advertise during version processingMarcoFalke
956c67059caf3807b3ceacdd5de1caaae538f009 refactor, doc: Improve SetupAddressRelay call in version processing (Martin Zumsande) 3c43d9db1e0f84279b08a93afd730caeece061a9 p2p: Don't self-advertise during VERSION processing (Gleb Naumenko) Pull request description: This picks up the last commit from #19843. Previously, we would prepare to self-announce to a new peer while parsing a `version` message from that peer. This is redundant, because we do something very similar in `MaybeSendAddr()`, which is called from `SendMessages()` after the version handshake is finished. There are a couple of differences: 1) `MaybeSendAddr()` self-advertises to all peers we do address relay with, not just outbound ones. 2) `GetLocalAddrForPeer()` called from `MaybeSendAddr()` makes a probabilistic decision to either advertise what they think we are or what we think we are, while `PushAddress()` on `version` deterministically only does the former if the address from the latter is unroutable. 3) During `version` processing, we haven't received a potential sendaddrv2 message from our peer yet, so self-advertisements with addresses from addrV2-only networks would always be dropped in `PushAddress()`. Since it's confusing to have two slightly different mechanisms for self-advertising, and the one in `MaybeSendAddr()` is better, remove the one in `version`. ACKs for top commit: stratospher: ACK 956c670 naumenkogs: ACK 956c67059caf3807b3ceacdd5de1caaae538f009 amitiuttarwar: reACK 956c67059c Tree-SHA512: 933d40615289f055c022170dde7bad0ac0a1d4be377538bfe9ba64375cfeb03bcd803901591f0739ac4850c880e8475a68fd1ab0330800030ab7f19e38c00274
2022-12-10Merge bitcoin/bitcoin#26666: refactor: Deleted unreachable code in ↵MarcoFalke
httpserver.cpp 8f5c560e11a9f3c7c5877f444caa605bdc296e81 refactor: Refactored RequestMethodString function to follow developer notes (JoaoAJMatos) 7fd3b9491b0e553aa7d60a68bda82968a79da8f5 refactor: Deleted unreachable code in httpserver.cpp (JoaoAJMatos) Pull request description: Some of the code in httpserver.cpp was unreachable, and didn't follow the developer notes. Continuation of [#26570 ](https://github.com/bitcoin/bitcoin/pull/26570) ACKs for top commit: stickies-v: re-ACK [8f5c560](https://github.com/bitcoin/bitcoin/commit/8f5c560e11a9f3c7c5877f444caa605bdc296e81) Tree-SHA512: ba8cf4c6dde9e2bb0ca9d63a0de86dfa37b070803dde71ac8384c261045835697a2335652cf5894511b3af8fd99f30e1cbda4e4234815b8b39538ade90fab3f9
2022-12-10Merge bitcoin/bitcoin#26672: build: Update libmultiprocess libraryfanquake
1986f129c6af7644d238c76f032713731d7afcac build: Update `libmultiprocess` library (Hennadii Stepanov) Pull request description: This update in particular includes: - https://github.com/chaincodelabs/libmultiprocess/pull/78 which is [required](https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-1246812573) for https://github.com/bitcoin/bitcoin/pull/25972 - https://github.com/chaincodelabs/libmultiprocess/pull/74 - https://github.com/chaincodelabs/libmultiprocess/pull/70 ACKs for top commit: ryanofsky: Code review ACK 1986f129c6af7644d238c76f032713731d7afcac Tree-SHA512: 2d9fa72df5de7d5be37d77d479702cba36c45e9fa9d9fc27e58aac0437c39e4a1054c0ac53b612cb43e830982e444d98c7d3e651d093ac68344e66f4734227bb
2022-12-10Merge bitcoin/bitcoin#26673: univalue: Remove confusing getBool methodfanquake
293849a26088a16f6187dfdc36bef1d4d83ebb9d univalue: Remove confusing getBool method (Ryan Ofsky) Pull request description: Drop `UniValue::getBool` method because it is easy to confuse with the `UniValue::get_bool` method, and could potentially cause bugs. Unlike `get_bool`, `getBool` doesn't ensure that the value is a boolean and returns false for all integer, string, array, and object values instead of throwing an exception. The `getBool` method is also redundant because it is an alias for `isTrue`. There were only 5 `getBool()` calls in the codebase, so this commit replaces them with `isTrue()` or `get_bool()` calls as appropriate. These changes were originally made by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/26213 but were dropped to limit the scope of that PR. ACKs for top commit: justinpickering: ACK https://github.com/bitcoin/bitcoin/pull/26673/commits/293849a26088a16f6187dfdc36bef1d4d83ebb9d sipa: utACK 293849a26088a16f6187dfdc36bef1d4d83ebb9d w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26673/commits/293849a26088a16f6187dfdc36bef1d4d83ebb9d hebasto: ACK 293849a26088a16f6187dfdc36bef1d4d83ebb9d, also verified that the removed `getBool` method is not mentioned in any docs: furszy: ACK 293849a2 Tree-SHA512: 9fbfe5e2083410f123b18703a0cc0161ecbbb4958f331c9ff808dcfcc6ad499b0e896abd16fb8ea200c53ba29878db9812ce141e59cc5e0fd174741b0bcb192d
2022-12-10Merge bitcoin/bitcoin#26213: rpc: Strict type checking for RPC boolean ↵fanquake
parameters fa0153e609caf61a59efb0779e754861edc1684d refactor: Replace isTrue with get_bool (MarcoFalke) fa2cc5d1d66aa00e828d1bb65b9923f76fbdf4e1 bugfix: Strict type checking for RPC boolean parameters (MarcoFalke) Pull request description: ACKs for top commit: ryanofsky: Code review ACK fa0153e609caf61a59efb0779e754861edc1684d furszy: Code ACK fa0153e6 Tree-SHA512: b221f823c69d90c94447fd491071ff3659cfd512872b495ebc3e711f50633351974102c9ef7e50fa4a393c4131d349adea8fd41cc9d66f1f31e1f5e7a5f78757
2022-12-09refactor: Refactored RequestMethodString function to follow developer notesJoaoAJMatos
Removed the default case in the switch statement in order to comply with the Developer Notes
2022-12-09refactor: Deleted unreachable code in httpserver.cppJoaoAJMatos
Removed all break statements from both RequestMethodString and GetRequestMethod functions as they were unreachable
2022-12-09Merge bitcoin/bitcoin#26601: test: Move wallet tests to wallet_*.pyMarcoFalke
fa7d71acccab4e5b416c9dd1e2080a9652107f84 test: Move rpc_fundrawtransaction.py to wallet_fundrawtransaction.py (MarcoFalke) fa933d6985830382c7e2da740bc363bf6a52c4ae test: Move feature_backwards_compatibility.py to wallet_backwards_compatibility.py (MarcoFalke) Pull request description: The tests only tests the wallet and it doesn't make sense to extend it for other stuff, so clarify that. ACKs for top commit: fanquake: ACK fa7d71acccab4e5b416c9dd1e2080a9652107f84 pablomartin4btc: re-ACK https://github.com/bitcoin/bitcoin/commit/fa7d71acccab4e5b416c9dd1e2080a9652107f84 Tree-SHA512: 9dc131ed8ff119bd6d43d04ecc5c96d02f2d6fdc3d0805492774a414c0fbe6a984da7631154122a080c54ddd25fc5a5bdd52b282c918fce4932057ba72c2bf71
2022-12-09build: Update `libmultiprocess` libraryHennadii Stepanov
Replacing `install` with `install-lib` and `install-bin` is not strictly necessary just to update the library, but it takes advantage of recent changes in the new version, and makes the build more minimal.
2022-12-09univalue: Remove confusing getBool methodRyan Ofsky
Drop UniValue::getBool method because it is easy to confuse with the UniValue::get_bool method, and could potentially cause bugs. Unlike get_bool, getBool doesn't ensure that the value is a boolean and returns false for all integer, string, array, and object values instead of throwing an exceptions. The getBool method is also redundant because it is an alias for isTrue. There were only 5 getBool() calls in the codebase, so this commit replaces them with isTrue() or get_bool() calls as appropriate. These changes were originally made by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/26213 but were dropped to limit the scope of that PR. Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2022-12-09test: Move rpc_fundrawtransaction.py to wallet_fundrawtransaction.pyMarcoFalke
2022-12-09test: Move feature_backwards_compatibility.py to ↵MarcoFalke
wallet_backwards_compatibility.py
2022-12-09Merge bitcoin/bitcoin#26660: test: Use last release in compatibility testsMarcoFalke
fabb24cbef6ccccf5e82ac52ca2aafd47c34455a test: Use last release in compatibility tests (MarcoFalke) Pull request description: In compatibility tests it makes sense to always use the last release without the new feature, as it is likely more in use than any even older previous release. ACKs for top commit: Sjors: utACK fabb24c Tree-SHA512: beb854f4d28ba313282e1e0303abb0e09377828b138bde5a3e209337210b6b4c24855ab90a68f8789387001e4ca33b15cc37dbc9b7809929f4e7d1b69833a527
2022-12-09Merge bitcoin/bitcoin#26658: test: Fix backwards compatibility intermittent ↵MarcoFalke
failure c29bff5b91061b565076093b7245dfb1717e97b7 test: Fix backwards compatibility intermittent failure (Aurèle Oulès) Pull request description: Fixes #24400. See https://github.com/bitcoin/bitcoin/issues/24400#issuecomment-1341531696 to reproduce the failure. As MarcoFalke suggested in https://github.com/bitcoin/bitcoin/issues/24400#issuecomment-1342282165, it can happen that the wallet is not fully flushed before being copied over to the other node. Fixed by unloading the wallet before copying the file. ACKs for top commit: Sjors: utACK c29bff5 Tree-SHA512: ea93b859878873d07fa44ccb1c5b4c1b1014be8568dc2b9bbef02bb7a9230fefa7a71b149eb553870d3f4e986263342ea4f996fa0221098a2244f38952100471
2022-12-08Merge bitcoin/bitcoin#24279: build: Make `$(package)_*_env` available to all ↵fanquake
`$(package)_*_cmds` affbf58a1e52a8e60c830be6a9e0347e0ff4c28e build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov) d44fcd3c976572883bbf7f386bc88e2610dc1a58 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov) Pull request description: On master (1e7564eca8a688f39c75540877ec3bdfdde766b1) the depends build system, which is based on pure GNU Make, works, but it lacks robustness, and in some corner cases it fails. For example, see bitcoin/bitcoin#22552. Another [bug](https://github.com/bitcoin/bitcoin/issues/22719) in the depends build system has already become a problem at least two times in the past (https://github.com/bitcoin/bitcoin/pull/16883#issuecomment-683817472 and https://github.com/bitcoin/bitcoin/pull/24134). Each time the problem was solved with other means. The initial [solution](https://github.com/bitcoin/bitcoin/pull/19882) had some discussion. Also it was discussed on the IRC meeting in #bitcoin-core-builds channel. This PR, actually, is a resurrection of it, as the bug silently struck pretty [recently](https://github.com/bitcoin/bitcoin/pull/24134). The bug is well described in bitcoin/bitcoin#22719. Here is another, a bit simpler description, which requires only basic shell (bash, dash etc) experience. After creating targets by this code:https://github.com/bitcoin/bitcoin/blob/1e7564eca8a688f39c75540877ec3bdfdde766b1/depends/funcs.mk#L280 a "draft" line of recipe like `$($(1)_config_env) $(call $(1)_config_cmds, $(1))` becomes a shell command sequence `VAR1=foo VAR2=bar command1 && command2` which is supposed to be executed in a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution). Please note that `VAR1=foo VAR2=bar` part is visible for the first `command1` only (for details see shell docs). Example: ```sh $ VAR1="foo" VAR2="bar" echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin $ echo $? 1 ``` Using the `export` command is a trivial solution: ```sh $ export VAR1="foo" VAR2="bar"; echo "begin" && printenv VAR1 && printenv VAR2 && echo "end" begin foo bar end $ echo $? 0 ``` As a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution) is invoked for each line of the recipe, there are no side effects of using `export`. It means this solution should not be considered invasive. Fixes bitcoin/bitcoin#22719. --- Also this PR removes no longer needed crutch from `qt.mk`. ACKs for top commit: fanquake: ACK affbf58a1e52a8e60c830be6a9e0347e0ff4c28e Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b
2022-12-08Merge bitcoin/bitcoin#26378: refactor: Pass reference to last header, not ↵MarcoFalke
pointer fa579f306363ed03c1138121415b67b9b36b4d53 refactor: Pass reference to last header, not pointer (MacroFake) Pull request description: It is never a nullptr, otherwise an assertion would fire in UpdatePeerStateForReceivedHeaders. Passing a reference makes the code easier to read and less brittle. ACKs for top commit: john-moffett: ACK fa579f3 aureleoules: ACK fa579f306363ed03c1138121415b67b9b36b4d53 Tree-SHA512: 9725195663a31df57ae46bb7b11211cc4963a8f3d100f60332bfd4a3f3327a73ac978b3172e3007793cfc508dfc7c3a81aab57a275a6963a5ab662ce85743fd0
2022-12-08Merge bitcoin/bitcoin#26513: Make static nLastFlush and nLastWrite ↵fanquake
Chainstate members 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 Make static nLastFlush and nLastWrite Chainstate members (Aurèle Oulès) Pull request description: Fixes #22189. The `static std::multimap<uint256, FlatFilePos> mapBlocksUnknownParent; ` referenced in the issue was already fixed by #25571. I don't believe Chainstate references any other static variables. ACKs for top commit: jamesob: ACK 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 ([`jamesob/ackr/26513.1.aureleoules.make_static_nlastflush_a`](https://github.com/jamesob/bitcoin/tree/ackr/26513.1.aureleoules.make_static_nlastflush_a)) theStack: Concept and code-review ACK 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 Tree-SHA512: 0f26463c079bbc5e0e62707d4ca4c8c9bbb99edfa3391d48d4915d24e2a1190873ecd4f9f11da25b44527671cdc82c41fd8234d56a4592a246989448d34406b0
2022-12-08test: Use last release in compatibility testsMarcoFalke
2022-12-08test: Fix backwards compatibility intermittent failureAurèle Oulès
2022-12-08Merge bitcoin/bitcoin#26308: rpc/rest/zmq: reduce LOCK(cs_main) scope: ~6 ↵MarcoFalke
times as many requests per second d7f61e7d5909da7227c9e34be06ce9eb872ba074 rpc: reduce LOCK(cs_main) scope in gettxoutproof (Andrew Toth) 4d92b5aabaf371225cdc37a7b114adc040bf4da5 rpc: reduce LOCK(cs_main) scope in GetUndoChecked and getblockstats (Andrew Toth) efd82aec8a2dd0fca8f2597c3f84cefe057d1243 rpc: reduce LOCK(cs_main) scope in blockToJSON (Andrew Toth) f00808e932c1f67bc76702a3f05778074b10025c rpc: reduce LOCK(cs_main) scope in GetBlockChecked and getblock (Andrew Toth) 7d253c943f44612431be894b198ffb49ff988fff zmq: remove LOCK(cs_main) from NotifyBlock (Andrew Toth) c75e3d2772b00acc3850f72a8cb733a0345aa773 rest: reduce LOCK(cs_main) scope in rest_block (Andrew Toth) Pull request description: Picking up from #21006. After commit https://github.com/bitcoin/bitcoin/commit/ccd8ef65f93ed82a87cee634660bed3ac17d9eb5 it is no longer required to hold `cs_main` when calling `ReadBlockFromDisk`. This can be verified in `master` at https://github.com/bitcoin/bitcoin/blob/master/src/node/blockstorage.cpp#L755. Same can be seen for `UndoReadFromDisk` https://github.com/bitcoin/bitcoin/blob/master/src/node/blockstorage.cpp#L485. The first commit moves `ReadBlockFromDisk` outside the lock scope in `rest_block`, where we can see a huge performance improvement when fetching blocks with multiple threads. My test setup, on an Intel i7 with 8 cores (16 threads): 1. Start a fully synced bitcoind, with this `bitcoin.conf`: ``` rest=1 rpcthreads=16 rpcworkqueue=64 rpcuser=user rpcpassword=password ``` 2. Run ApacheBench: 10000 requests, 16 parallel threads, fetching block nr. 750000 in binary: ``` ab -n 10000 -c 16 "http://127.0.0.1:8332/rest/block/0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e.bin" ``` Time per request (mean) 183 ms on master 30 ms this branch So this can process 6.1 times as many requests, and saturates all the cores instead of keeping them partly idle waiting in the lock. With 8 threads the mean times were 90 ms on master and 19 ms on this branch, a speedup of 4.7x. Big thanks to martinus for finding this and the original PR. The second commit is from a suggestion on the original PR by jonatack to remove the unnecessary `LOCK(cs_main)` in the zmq notifier's `NotifyBlock`. I also found that this approach could be applied to rpcs `getblock` (including `verbosity=3`), `getblockstats`, and `gettxoutproof` with similar very good results. The above benchmarks steps need to be modified slightly for RPC. Run the following ApacheBench command with different request data in a file named `data.json`: ``` ab -p data.json -n 10000 -c 16 -A user:password "http://127.0.0.1:8332/" ``` For `getblock`, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblock", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e"]} ``` master - 184 ms mean request time branch - 28 ms mean request time For `getblock` with verbosity level 3, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblock", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e", 3]} ``` This verbosity level fetches an undo file from disk, so it benefits from this approach as well. However, a lot of time is spent serializing to JSON so the performance gain is not as severe. master - 818 ms mean request time branch - 505 ms mean request time For `getblockstats`, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblockstats", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e", ["minfeerate","avgfeerate"]]} ``` This request used a lock on reading both a block and undo file, so the results are very good. master - 244 ms mean request time branch - 28 ms mean request time ACKs for top commit: MarcoFalke: re-ACK d7f61e7d5909da7227c9e34be06ce9eb872ba074 💫 hebasto: ACK d7f61e7d5909da7227c9e34be06ce9eb872ba074, I have reviewed the code and it looks OK. Did not make benchmarking though. Tree-SHA512: 305ac945b4571c5f47646d4f0e78180d7a3d40b2f70ee43e4b3e00c96a465f6d0b9c750b8e85c89ed833e557e2cdb5896743f07ef90e4e53d4ad85452b545886
2022-12-07Merge bitcoin/bitcoin#25934: wallet, rpc: add `label` to `listsinceblock`Andrew Chow
4e362c2b7269ae0426010850c605e5c1d0d58234 doc: add release note for 25934 (brunoerg) fe488b4c4b7aa07fb83d528e2942ef914fd188c0 test: add coverage for `label` in `listsinceblock` (brunoerg) 722e9a418d078ed34aedd1ca55c1ae104f29a7d3 wallet, rpc: add `label` to `listsinceblock` (brunoerg) 852891ff98cffd37a74b9cb96394f43b2e6ca30e refactor, wallet: use optional for `label` in `ListTransactions` (brunoerg) Pull request description: This PR adds `label` parameter to `listsinceblock` to be able to fetch all incoming transactions having the specified label since a specific block. It's possible to use it in `listtransactions`, however, it's only possible to set the number of transactions to return, not a specific block to fetch from. `getreceivedbylabel` only returns the total amount received, not the txs info. `listreceivedbylabel` doesn't list all the informations about the transactions and it's not possible to fetch since a block. ACKs for top commit: achow101: ACK 4e362c2b7269ae0426010850c605e5c1d0d58234 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25934/commits/4e362c2b7269ae0426010850c605e5c1d0d58234 aureleoules: ACK 4e362c2b7269ae0426010850c605e5c1d0d58234 Tree-SHA512: fbde5db8cebf7a27804154fa61997b5155ad512e978cebb78c17acab9efcb624ea5f39d649899d12e5e675f80d4d0064cae8132b864de0d93a8d1e6fbcb9a737
2022-12-07Merge bitcoin/bitcoin#24611: Add fish completionsAndrew Chow
ccba4fe7e3113f5d3bb2fbe54147b3a17e1bfa24 doc: Add completion subdir to contrib/README.md (willcl-ark) 7075848f9646a5b37044beb78c48e57e1f32b6d0 script: Add fish completions (willcl-ark) a27a445b71dbee41f81c7786e667eaa3d9813028 refactor: Sub-folder bash completions (willcl-ark) Pull request description: The completions are dynamically generated from the respective binary help pages. Completions should be sourced into the shell or added to `$XDG_CONFIG/fish/completions`. See [where to put completions](https://fishshell.com/docs/current/completions.html#where-to-put-completions) for more information. As the completions are auto-generated they should only require as much maintenance as the bash equivalents, which is to say very little! ACKs for top commit: achow101: ACK ccba4fe7e3113f5d3bb2fbe54147b3a17e1bfa24 josibake: ACK https://github.com/bitcoin/bitcoin/commit/ccba4fe7e3113f5d3bb2fbe54147b3a17e1bfa24 Tree-SHA512: fe6ed899ea1fe90f82970bde7739db11dd0c845ccd70b65f28ad5212f75b57d9105a3a7f70ccdff552d5b21fa3fe9c697d128fb10740bae31fe1854e716b4b8b
2022-12-07refactor: Replace isTrue with get_boolMarcoFalke
This makes the code more robust, see previous commit. In general replacing isTrue with get_bool is not equivalent because get_bool can throw exceptions, but in this case, exceptions won't happen because of RPCTypeCheck() and isNull() checks in the preceding code.
2022-12-07bugfix: Strict type checking for RPC boolean parametersMarcoFalke
2022-12-07build: Move environment variables into `$(package)_config_env`Hennadii Stepanov
2022-12-07build: Make $(package)_*_env available to all $(package)_*_cmdsHennadii Stepanov
2022-12-07Merge bitcoin/bitcoin#26517: test: Changed small_txpuzzle_randfee to return ↵MarcoFalke
the virtual size instead of the transaction hex for feerate calculation. 6fb102c9f361a7ba0a6aa0a9b41315f5e04559f7 test: Changed small_txpuzzle_randfee to return the virtual size instead of the transaction hex for feerate calculation. (Randall Naar) Pull request description: The fee rates used in feature_fee_estimation.py are calculated using the raw transaction size instead of the virtual transaction size (which is used in 'CBlockPolicyEstimator::processBlockTx' and 'CBlockPolicyEstimator::processBlock'). This leads to inconsistencies as the fee rates used in check_raw_estimates are incorrect and can cause assertions to fail. refs #25179 ACKs for top commit: MarcoFalke: ACK 6fb102c9f361a7ba0a6aa0a9b41315f5e04559f7 Tree-SHA512: b2bca85fffa605aeb17574f050736d4556506d782dd7fd969e165e48e108fd95ef4c4e2abbae83cce05ca777a00f4459cabfa0932694fa8bb93ddfba09d84357
2022-12-07Merge bitcoin/bitcoin#26298: refactor: Move src/interfaces/*.cpp files to ↵fanquake
libbitcoin_common.a b19c4124b3c9a15fe3baf8792c55eb26eca51c0f refactor: Rename ambiguous interfaces::MakeHandler functions (Ryan Ofsky) dd6e8bd71c6025a51d88000caf28121ec00499db build: remove BOOST_CPPFLAGS from libbitcoin_util (fanquake) 82e272a109281f750909d1feade784c778d8b592 refactor: Move src/interfaces/*.cpp files to libbitcoin_common.a (Ryan Ofsky) Pull request description: These belong in `libbitcoin_common.a`, not `libbitcoin_util.a`, because they aren't general-purpose utilities, they just contain some common glue code that is used by both the node and the wallet. Another reason not to include these in `libbitcoin_util.a` is to prevent them from being used by the kernel library. Also rename ambiguous `MakeHandler` functions to `MakeCleanupHandler` and `MakeSignalHandler`. Cleanup function handler was introduced after boost signals handler, so original naming didn't make much sense. This just contains a move-only commit, and a rename commit. There are no actual code or behavior changes. This PR is an alternative to #26293, and solves the same issue of removing a boost dependency from the _util_ library. The advantages of this PR compared to #26293 are that it keeps the source directory structure more flat, and it avoids having to change #includes all over the codebase. ACKs for top commit: hebasto: ACK b19c4124b3c9a15fe3baf8792c55eb26eca51c0f Tree-SHA512: b3a1d33eedceda7ad852c6d6f35700159d156d96071e59acae2bc325467fef81476f860a8855ea39cf3ea706a1df2a341f34fb2dcb032c31a3b0e9cf14103b6a
2022-12-07Merge bitcoin/bitcoin#26645: util: Include full version id in bug reportsMarcoFalke
fa825bd227b9d2bace896a2d29b5ce78bbd4e59c util: Include full version id in bug reports (MarcoFalke) Pull request description: This will show the unique id of the full source code when the bug occurred, which can help debugging ACKs for top commit: 1440000bytes: utACK https://github.com/bitcoin/bitcoin/commit/fa825bd227b9d2bace896a2d29b5ce78bbd4e59c theStack: ACK fa825bd227b9d2bace896a2d29b5ce78bbd4e59c john-moffett: ACK fa825bd227b9d2bace896a2d29b5ce78bbd4e59c Tree-SHA512: a7a775718f5f9796b5cffafbb3ace8adb5c163414ec584a57143157fc9dfb86f799e3b9c8365fcb831ee1e9eafc59d699d1653d772c68392de421b3de74dcd61
2022-12-06rpc: reduce LOCK(cs_main) scope in gettxoutproofAndrew Toth
2022-12-06rpc: reduce LOCK(cs_main) scope in GetUndoChecked and getblockstatsAndrew Toth
2022-12-06rpc: reduce LOCK(cs_main) scope in blockToJSON Andrew Toth
2022-12-06rpc: reduce LOCK(cs_main) scope in GetBlockChecked and getblockAndrew Toth
2022-12-06zmq: remove LOCK(cs_main) from NotifyBlockAndrew Toth
2022-12-06rest: reduce LOCK(cs_main) scope in rest_blockAndrew Toth
2022-12-06Merge bitcoin-core/gui#683: doc: Drop no longer relevant commentHennadii Stepanov
5d332da2cfdc67d69a165119c6ff598e29b28afa doc: Drop no longer relevant comment (Hennadii Stepanov) Pull request description: The comment was introduced in 4cf3411056f6a59fc5fe07784b6b6a512d76b046, and since 7e4bd19785ff9120b7242ff9f15231868aaf7db4 it has been no longer relevant. ACKs for top commit: jarolrod: ACK 5d332da2cfdc67d69a165119c6ff598e29b28afa Tree-SHA512: 6d32561336993b1ff7d7c524d090ac52aefb40078ed706ca4c6d5026cc3f63244c49c0e00e45ff192ba0e9f1527faf63249aa18bc8aa677b9e053d387e0f4027
2022-12-06doc: add release note for 25934brunoerg
2022-12-06test: add coverage for `label` in `listsinceblock`brunoerg
2022-12-06wallet, rpc: add `label` to `listsinceblock`brunoerg
2022-12-06refactor, wallet: use optional for `label` in `ListTransactions`brunoerg
2022-12-06Merge bitcoin/bitcoin#26648: doc: move release notes to 24.0.1 and add noticefanquake
896fca16a3818cc9102196863627e66d4d89307a doc: move release notes to 24.0.1 and add notice (fanquake) Pull request description: This mirrors what was done with 0.19.0.1. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/26648/commits/896fca16a3818cc9102196863627e66d4d89307a gruve-p: ACK https://github.com/bitcoin/bitcoin/pull/26648/commits/896fca16a3818cc9102196863627e66d4d89307a w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26648/commits/896fca16a3818cc9102196863627e66d4d89307a Tree-SHA512: 590462555422c0f96895152d1a2f9f9cf0ebf2c61dc342094d747f4d48b878e5d91840b9c2ac6825bb7214239f035789f1765a907fb614017205377ed89631fd
2022-12-06Merge bitcoin/bitcoin#26609: refactor: Move `txmempool_entry.h` --> ↵MarcoFalke
`kernel/mempool_entry.h` 38941a703e079709bb465ad1fcde50e11350f8f1 refactor: Move `txmempool_entry.h` --> `kernel/mempool_entry.h` (Hennadii Stepanov) Pull request description: This PR addresses the https://github.com/bitcoin/bitcoin/pull/17786#discussion_r1027818360: > why not move it to the right place, that is to `kernel/txmempool_entry.h`? ACKs for top commit: MarcoFalke: review ACK 38941a703e079709bb465ad1fcde50e11350f8f1 📊 Tree-SHA512: 0145974b63b67ca1d9d89af2dd9d4438beca480c16a563f330da05fec49b8394d7ba20ed83cf7d50b2e19454e006978ebed42b0e07887b98d00210f3201ce9ba
2022-12-06Merge bitcoin/bitcoin#25729: wallet: Check max transaction weight in ↵Andrew Chow
CoinSelection c7c7ee9d0b2d7b303b9300f941e37e09e7d8d8b6 test: Check max transaction weight in CoinSelection (Aurèle Oulès) 6b563cae92957dc30dc35103a7c321fdb0115ef3 wallet: Check max tx weight in coin selector (Aurèle Oulès) Pull request description: This PR is an attempt to fix #5782. I have added 4 test scenarios, 3 of them provided here https://github.com/bitcoin/bitcoin/issues/5782#issuecomment-73819058 (slightly modified to use a segwit wallet). Here are my benchmarks : ## PR | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 1,466,341.00 | 681.97 | 0.6% | 11,176,762.00 | 3,358,752.00 | 3.328 | 1,897,839.00 | 0.3% | 0.02 | `CoinSelection` ## Master | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 1,526,029.00 | 655.30 | 0.5% | 11,142,188.00 | 3,499,200.00 | 3.184 | 1,994,156.00 | 0.2% | 0.02 | `CoinSelection` ACKs for top commit: achow101: reACK c7c7ee9d0b2d7b303b9300f941e37e09e7d8d8b6 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25729/commits/c7c7ee9d0b2d7b303b9300f941e37e09e7d8d8b6 furszy: diff ACK c7c7ee9d Tree-SHA512: ef0b28576ff845174651ba494aa9adee234c96e6f886d0e032eceb7050296e45b099dda0039d1dfb9944469f067627b2101f3ff855c70353cf39d1fc7ee81828
2022-12-06doc: move release notes to 24.0.1 and add noticefanquake
This mirrors what was done with 0.19.0.1.
2022-12-06Merge bitcoin/bitcoin#26238: clang-tidy: fixup named argument commentsMarcoFalke
203886c443c4ad76f8a1dba740a286e383e55206 Fixup clang-tidy named argument comments (fanquake) Pull request description: Fix comments so they are checked/consistent. Fix incorrect comments. ACKs for top commit: hebasto: ACK 203886c443c4ad76f8a1dba740a286e383e55206, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: e1257840f91fe39842e2b19299c1633604697b8584fe44b1977ada33cdde5433c877ed0b669fa334e20b04971dc89cd47d58b2783b6f7004521f01d05a1245da
2022-12-06util: Include full version id in bug reportsMarcoFalke
2022-12-06Merge bitcoin/bitcoin#26611: wallet: Change coin selection fee assert to errorMarcoFalke
3eb041f014870954db564369a4be4bd0dea48fbe wallet: Change coin selection fee assert to error (Andrew Chow) c6e7f224c119f47af250a9e0c5b185cb98b30c4c util: Add StrFormatInternalBug and STR_INTERNAL_BUG (MarcoFalke) Pull request description: Returning an error instead of asserting for the low fee check will be better as it does not crash the node and instructs users to report the bug. ACKs for top commit: S3RK: ACK 3eb041f014870954db564369a4be4bd0dea48fbe aureleoules: ACK 3eb041f014870954db564369a4be4bd0dea48fbe furszy: ACK 3eb041f0 Tree-SHA512: 118c13d7cdfce492080edd4cb12e6d960695377b978c7573f9c58b6d918664afd0e8e591eed0605d08ac756fa8eceed456349de5f3a025174069abf369bb5a5f
2022-12-05Merge bitcoin/bitcoin#19888: rpc, test: Improve getblockstats for unspendablesAndrew Chow
d885bb2f6ea34cd21dacfebe763a07dbb389c1bd test: Test exclusion of OP_RETURN from getblockstats (Fabian Jahr) ba9d288b2468f047e5a8e4637dd8749e247ff547 test: Fix getblockstats test data generator (Fabian Jahr) 2ca5a496c2f3cbcc63ea15fa05c1658e7f527bbc rpc: Improve getblockstats (Fabian Jahr) cb94db119f4643f49da63520d64efc99fb0c0795 validation, index: Add unspendable coinbase helper functions (Fabian Jahr) Pull request description: Fixes #19885 The genesis block does not have undo data saved to disk so the RPC errored because of that. ACKs for top commit: achow101: ACK d885bb2f6ea34cd21dacfebe763a07dbb389c1bd aureleoules: ACK d885bb2f6ea34cd21dacfebe763a07dbb389c1bd stickies-v: ACK d885bb2f6 Tree-SHA512: f37bda736ed605b7a41a81eeb4bfbb5d2b8518f847819e5d6a090548a61caf1455623e15165d72589ab3f4478252b00e7b624f9313ad6708cac06dd5edb62e9a
2022-12-05Merge bitcoin/bitcoin#26462: wallet: fix crash on loading descriptor wallet ↵Andrew Chow
containing legacy key type entries 3198e4239e848bbb119e3638677aa9bcf8353ca6 test: check that loading descriptor wallet with legacy entries throws error (Sebastian Falbesoner) 349ed2a0eed3aaaf199ead93057c97730869c3a3 wallet: throw error if legacy entries are present on loading descriptor wallets (Sebastian Falbesoner) Pull request description: Loading a descriptor wallet currently leads to a segfault if a legacy key type entry is present that can be deserialized successfully and needs SPKman-interaction. To reproduce with a "cscript" entry (see second commit for details): ``` $ ./src/bitcoin-cli createwallet crashme $ ./src/bitcoin-cli unloadwallet crashme $ sqlite3 ~/.bitcoin/wallets/crashme/wallet.dat SQLite version 3.38.2 2022-03-26 13:51:10 Enter ".help" for usage hints. sqlite> INSERT INTO main VALUES(x'07637363726970740000000000000000000000000000000000000000', x'00'); $ ./src/bitcoin-cli loadwallet crashme --- bitcoind output: --- 2022-11-06T13:51:01Z Using SQLite Version 3.38.2 2022-11-06T13:51:01Z Using wallet /home/honey/.bitcoin/wallets/crashme 2022-11-06T13:51:01Z init message: Loading wallet… 2022-11-06T13:51:01Z [crashme] Wallet file version = 10500, last client version = 249900 Segmentation fault (core dumped) ``` Background: In the wallet key-value-loading routine, most legacy type entries require a `LegacyScriptPubKeyMan` instance after successful deserialization. On a descriptor wallet, creating that (via method `GetOrCreateLegacyScriptPubKeyMan`) fails and then leads to a null-pointer dereference crash. E.g. for CSCRIPT: https://github.com/bitcoin/bitcoin/blob/50422b770a40f5fa964201d1e99fd6b5dc1653ca/src/wallet/walletdb.cpp#L589-L594 ~~This PR fixes this by simply ignoring legacy entries if the wallet flags indicate that we have a descriptor wallet. The second commits adds a regression test to the descriptor wallet's functional test (fortunately Python includes sqlite3 support in the standard library).~~ ~~Probably it would be even better to throw a warning to the user if unexpected legacy entries are found in descriptor wallets, but I think as a first mitigation everything is obvisouly better than crashing. As far as I'm aware, descriptor wallets created/migrated by Bitcoin Core should never end up in a state containing legacy type entries though.~~ This PR fixes this by throwing an error if legacy entries are found in descriptor wallets on loading. ACKs for top commit: achow101: ACK 3198e4239e848bbb119e3638677aa9bcf8353ca6 aureleoules: ACK 3198e4239e848bbb119e3638677aa9bcf8353ca6 Tree-SHA512: ee43da3f61248e0fde55d9a705869202cb83df678ebf4816f0e77263f0beac0d7bae9490465d1753159efb093ee37182931d76b2e2b6e8c6f8761285700ace1c
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-09 08:58:30 +0000