aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-07-25test: Avoid intermittent issues due to async events in validationinterface_testsMarcoFalke
2023-07-24[net processing] Move -capturemessages to PeerManager::Optionsdergoegge
2023-07-24[net processing] Move -blockreconstructionextratxn to PeerManager::Optionsdergoegge
2023-07-24[net processing] Move -maxorphantx to PeerManager::Optionsdergoegge
2023-07-24[net processing] Move -txreconciliation to PeerManager::Optionsdergoegge
2023-07-24[net processing] Use ignore_incoming_txs from m_optsdergoegge
2023-07-24[net processing] Introduce PeerManager optionsdergoegge
2023-07-20Merge bitcoin/bitcoin#28067: descriptors: do not return top-level only funcs ↵Andrew Chow
as sub descriptors dd9633b516d6936ac4e23a40f9b0bea120117d35 test: wallet, add coverage for watch-only raw sh script migration (furszy) cc781a21800a6ce13875feefd0cb14ab0a84524c descriptor: InferScript, do not return top-level only func as sub descriptor (furszy) 286e0c7d5e9538198b28b792c5168b8fafa1534f wallet: loading, log descriptor parsing error details (furszy) Pull request description: Linked to #28057. Currently, the `InferScript` function returns an invalid descriptor when it tries to infer a p2sh-p2pkh script whose pubkey is not known by the wallet. This behavior occurs because the inference process bypasses the `pkh` subscript when the pubkey is not contained by the wallet (no pubkey provider), interpreting it as a `sh(addr(ADDR))` descriptor. Then, the failure arises because the `addr()` function is restricted to being used only at the top level. For reviewers, would recommend to start by examining the functional test to understand the context and the circumstances on which this can result in a fatal error (e.g. during the migration process). ACKs for top commit: achow101: ACK dd9633b516d6936ac4e23a40f9b0bea120117d35 darosior: utACK dd9633b516d6936ac4e23a40f9b0bea120117d35 Tree-SHA512: 61e763206c604c372019d2c36e31684f3dddf81f8b154eb9aba5cd66d8d61bda457ed4e591613eb6ce6c76cf7c3f11764abc6cd727a7c2b6414f1065783be032
2023-07-20descriptor: InferScript, do not return top-level only func as sub descriptorfurszy
e.g. sh(addr(ADDR)) or sh(raw(HEX)) are invalid descriptors. Making sh and wsh top level functions to return addr/raw descriptors when the subscript inference fails.
2023-07-20Merge bitcoin/bitcoin#26654: util: Show descriptive error messages when ↵fanquake
FileCommit fails 5408a55fc87350baeae3a32f1003f956d5533a79 Consolidate Win32-specific error formatting (John Moffett) c95a4432d7c9d0e5829cd802908700ba2e2c25dc Show descriptive error messages when FileCommit fails (John Moffett) Pull request description: Only raw [`errno`](https://en.cppreference.com/w/cpp/error/errno) int values are logged if `FileCommit` fails. These values are implementation-specific, so it makes it harder to debug based on user reports. For instance, https://github.com/bitcoin/bitcoin/issues/26455#issue-1436654238 and [another](https://bitcointalk.org/index.php?topic=5182526.0#:~:text=FileCommit%3A%20FlushFileBuffers%20failed%3A%205). Instead, use `SysErrorString` (or the refactored Windows equivalent `Win32ErrorString`) to display both the raw int value and the descriptive message. All other instances in the code I could find where `errno` or (Windows-only) `GetLastError()`/`WSAGetLastError()` are logged use the full descriptive string. For example: https://github.com/bitcoin/bitcoin/blob/1b680948d43b1d39645b9d839a6fa7c6c1786b51/src/util/sock.cpp#L390 https://github.com/bitcoin/bitcoin/blob/1b680948d43b1d39645b9d839a6fa7c6c1786b51/src/util/sock.cpp#L272 https://github.com/bitcoin/bitcoin/blob/7e1007a3c6c9a921c2b60919b84a60eaabfe1c5d/src/netbase.cpp#L515-L516 https://github.com/bitcoin/bitcoin/blob/8ccab65f289e3cce392cbe01d5fc0e7437f51f1e/src/init.cpp#L164 I refactored the Windows formatting code to put it in `syserror.cpp`, as it's applicable to all Win32 API system errors, not just networking errors. To be clear, the Windows API functions `WSAGetLastError()` and `GetLastError()` are currently [equivalent](https://stackoverflow.com/questions/15586224/is-wsagetlasterror-just-an-alias-for-getlasterror). ACKs for top commit: MarcoFalke: lgtm ACK 5408a55fc87350baeae3a32f1003f956d5533a79 💡 Tree-SHA512: 3921cbac98bd9edaf84d3dd7a43896c7921f144c8ca2cde9bc96d5fb05281f7c55e7cc99db8debf6203b5f916f053025e4fa741f51458fe2c53bb57b0a781027
2023-07-20Merge bitcoin/bitcoin#28066: fuzz: Generate process_message targets individuallyfanquake
fa6245da6061050eb77ad07cd4caf8c596d89dc6 fuzz: Generate process_message targets individually (MarcoFalke) fa1471e5754484f997ddf9db70888679dcd1d64a refactor: Remove duplicate allNetMessageTypesVec (MarcoFalke) Pull request description: Now that `LIMIT_TO_MESSAGE_TYPE` is a runtime setting after commit 927b001502a74a7224f04cfe6ffddc9a59409ba1, it shouldn't hurt to also generate each message type individually. Something similar was done for the `rpc` target in commit cf4da5ec29f9e8cd6cc6577e5ecbd87174edba62. ACKs for top commit: stickies-v: re-crACK fa6245da6061050eb77ad07cd4caf8c596d89dc6 brunoerg: reACK fa6245da6061050eb77ad07cd4caf8c596d89dc6 Tree-SHA512: 8f3ec71bab89781f10820a0e027fcde8949f3333eb19a30315aaad6f90f5167028113cea255b2d60b700da817c7eaac20b7b4c92f931052d7f5c2f148d33aa5a
2023-07-20Merge bitcoin/bitcoin#26467: bumpfee: Allow the user to choose which output ↵fanquake
is change e8c31f135c6e9a5f57325dbf4feceafd384f7762 tests: Test for bumping single output transaction (Andrew Chow) 4f4d4407e3d2cc5ac784524c0cb0602837dc7860 test: Test bumpfee reduce_output (Andrew Chow) 7d83502d3d52218e7b0b0634cff2a9aba9cc77ef bumpfee: Allow original change position to be specified (Andrew Chow) Pull request description: When bumping the transaction fee, we will try to find the change output of the transaction in order to have an output whose value we can modify so that we can meet the target feerate. However we do not always find the change output which can cause us to unnecessarily add an additional output to the transaction. We can avoid this issue by outsourcing the determination of change to the user if they so desire. This PR adds a `orig_change_pos` option to bumpfee which the user can use to specify the index of the change output. Fixes #11233 Fixes #20795 ACKs for top commit: ismaelsadeeq: re ACK e8c31f135c6e9a5f57325dbf4feceafd384f7762 pinheadmz: re-ACK e8c31f135c6e9a5f57325dbf4feceafd384f7762 furszy: Code review ACK e8c31f13 Tree-SHA512: 3a230655934af17f7c1a5953fafb5ef0d687c21355cf284d5e98fece411f589cd69ea505f06d6bdcf82836b08d268c366ad2dd30ae3d71541c9cdf94d1f698ee
2023-07-19Merge bitcoin/bitcoin#27928: test: Add more tests for the BIP21 implementationRyan Ofsky
f1d807e383942ae20e080174d33ac17afd649351 Add more tests for the BIP21 implementation (Kiminuo) Pull request description: This PR is an attempt to make it clear how the current BIP21 implementation behaves in Bitcoin Core. Especially, I'm interested whether one can specify multiple `amount` (`message`, etc.) parameters. My primary end goal is to answer [this question of mine](https://bitcoin.stackexchange.com/questions/118654/how-to-interpret-bip21-uri-with-amount-specified-twice/) but I figured that maybe it's worth a PR. If not, I'll close the PR. ACKs for top commit: MarcoFalke: lgtm ACK f1d807e383942ae20e080174d33ac17afd649351 kevkevinpal: ACK [f1d807e](https://github.com/bitcoin/bitcoin/pull/27928/commits/f1d807e383942ae20e080174d33ac17afd649351) Tree-SHA512: d287809d47c5cfc667f850927bfd969bd345a996d3d53a4c26ef0ffd29eb75ef53358692a15f9a0493ec9e1c101123b6584572e25f87bcb98ff67f6b6c166de4
2023-07-19Merge bitcoin/bitcoin#28085: refactor: use Span for SipHash::WriteAndrew Chow
7d92b1430a6fd42c4438810640576830d0ff8d13 refactor: use Span for SipHash::Write (Sebastian Falbesoner) Pull request description: This simple refactoring PR changes the interface for the `SipHash` arbitrary-data `Write` method to take a `Span<unsigned char>` instead of having to pass data and length. (`Span<std::byte>` seems to be more modern, but vectors of `unsigned char` are still used prety much everywhere where SipHash is called, and I didn't find it very appealing having to clutter the code with `Make(Writable)ByteSpan` helpers). ACKs for top commit: sipa: utACK 7d92b1430a6fd42c4438810640576830d0ff8d13 MarcoFalke: lgtm ACK 7d92b1430a6fd42c4438810640576830d0ff8d13 achow101: ACK 7d92b1430a6fd42c4438810640576830d0ff8d13 Tree-SHA512: f17a27013c942aead4b09f5a64e0c3ff8dbc7e83fe63eb9a2e3ace8be9921c9cbba3ec67e3e83fbe3332ca941c42370efd059e702c060f9b508307e9657c66f2
2023-07-19Merge bitcoin/bitcoin#28056: rpc: doc: Added `longpollid` and `data` params ↵fanquake
to `template_request` f6a26196cfb7e2c90e25f82b0e2f569a05013cae Added `longpollid` and `data` params to `template_request` #27998 (Rhythm Garg) Pull request description: This PR will add the optional parameters `longpollid` and `data` to `template_request` as they were missing when calling `help getblocktemplate` in RPCHelpMan. I request the maintainers to review this and let me know about any mistakes in the descriptions of the parameters. This PR refers to the issue #27998 ACKs for top commit: ItIsOHM: > tACK [f6a2619](https://github.com/bitcoin/bitcoin/commit/f6a26196cfb7e2c90e25f82b0e2f569a05013cae) russeree: tACK https://github.com/bitcoin/bitcoin/commit/f6a26196cfb7e2c90e25f82b0e2f569a05013cae stickies-v: tACK f6a26196cfb7e2c90e25f82b0e2f569a05013cae Tree-SHA512: 6c592db59cb11b2d031ce5265c547fa296266278f6c25f96afe18a420e0d547f4d483e0f66de75d52c0c319ac1585f3558b9f70c12ef208c96ec96a51f786c6a
2023-07-19Merge bitcoin/bitcoin#28093: subtree: update libsecp256k1 to latest masterfanquake
5080c9c25f44ae9d16a69d41f0da1d1e06483bf7 build: adapt Windows builds for libsecp256k1 build changes (fanquake) ff061fde183f68d3b77db24b5742f54e58952de6 Squashed 'src/secp256k1/' changes from 705ce7ed8c..c545fdc374 (fanquake) Pull request description: Includes https://github.com/bitcoin-core/secp256k1/pull/1378. Which fixes #28079. Adapts Windows build for https://github.com/bitcoin-core/secp256k1/pull/1367. ACKs for top commit: hebasto: ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7, I've made the `src/secp256k1` subtree update locally and got zero diff with this PR branch. jonasnick: ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7 Tree-SHA512: 37915d420ebacefc6bc82c2511bff3d6884e01d5c92795f19cd61862f96b30aa1fe768aeabec128c9d25c1d8bc62b46b46969626067266074b39566ad9e2f5ba
2023-07-19Merge bitcoin/bitcoin#27425: test: move remaining rand code from ↵fanquake
util/setup_common to util/random 1cd45d4e08c3dfd1d6423620c79169f1404ac12b test: move random.h include header from setup_common.h to cpp (Jon Atack) 1b246fdd145a95f5da479159f5e8eaf5a76bdc3a test: move remaining random test util code from setup_common to random (jonatack) Pull request description: and drop the `util/random` dependency on `util/setup_common`. This improves code separation and allows `util/setup_common` to call `util/random` functions without creating a circular dependency, thereby addressing https://github.com/bitcoin/bitcoin/pull/26940#issuecomment-1497266140 by glozow (thanks!) ACKs for top commit: MarcoFalke: lgtm ACK 1cd45d4e08c3dfd1d6423620c79169f1404ac12b 🌂 Tree-SHA512: 6ce63d9103ba9b04eebbd8ad02fe9aa79e356296533404034a1ae88e9b7ca0bc9a5c51fd754b71cf4e7b55b18bcd4d5474b2d588edee3851e3b3ce0e4d309a93
2023-07-18Update secp256k1 subtree to latest upstream masterfanquake
2023-07-18Squashed 'src/secp256k1/' changes from 705ce7ed8c..c545fdc374fanquake
c545fdc374 Merge bitcoin-core/secp256k1#1298: Remove randomness tests b40e2d30b7 Merge bitcoin-core/secp256k1#1378: ellswift: fix probabilistic test failure when swapping sides c424e2fb43 ellswift: fix probabilistic test failure when swapping sides 907a67212e Merge bitcoin-core/secp256k1#1313: ci: Test on development snapshots of GCC and Clang 0f7657d59c Merge bitcoin-core/secp256k1#1366: field: Use `restrict` consistently in fe_sqrt cc55757552 Merge bitcoin-core/secp256k1#1340: clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3) 600c5adcd5 clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3) 981e5be38c ci: Fix typo in comment e9e9648219 ci: Reduce number of macOS tasks from 28 to 8 609093b387 ci: Add x86_64 Linux tasks for gcc and clang snapshots 1deecaaf3b ci: Install development snapshots of gcc and clang b79ba8aa4c field: Use `restrict` consistently in fe_sqrt c9ebca95f9 Merge bitcoin-core/secp256k1#1363: doc: minor ellswift.md updates afd7eb4a55 Merge bitcoin-core/secp256k1#1371: Add exhaustive tests for ellswift (with create+decode roundtrip) 2792119278 Add exhaustive test for ellswift (create+decode roundtrip) c7d900ffd1 doc: minor ellswift.md updates 332af315fc Merge bitcoin-core/secp256k1#1344: group: save normalize_weak calls in `secp256k1_ge_is_valid_var`/`secp256k1_gej_eq_x_var` 9e6d1b0e9b Merge bitcoin-core/secp256k1#1367: build: Improvements to symbol visibility logic on Windows (attempt 3) 0aacf64352 Merge bitcoin-core/secp256k1#1370: Corrected some typos b6b9834e8d small fixes 07c0e8b82e group: remove unneeded normalize_weak in `secp256k1_gej_eq_x_var` 3fc1de5c55 Merge bitcoin-core/secp256k1#1364: Avoid `-Wmaybe-uninitialized` when compiling with `gcc -O1` fb758fe8d6 Merge bitcoin-core/secp256k1#1323: tweak_add: fix API doc for tweak=0 c6cd2b15a0 ci: Add task for static library on Windows + CMake 020bf69a44 build: Add extensive docs on visibility issues 0196e8ade1 build: Introduce `SECP256k1_DLL_EXPORT` macro 9f1b1904a3 refactor: Replace `SECP256K1_API_VAR` with `SECP256K1_API` ae9db95cea build: Introduce `SECP256K1_STATIC` macro for Windows users 7966aee31d Merge bitcoin-core/secp256k1#1369: ci: Print commit in Windows container a7bec34231 ci: Print commit in Windows container 249c81eaa3 Merge bitcoin-core/secp256k1#1368: ci: Drop manual checkout of merge commit 98579e297b ci: Drop manual checkout of merge commit 5b9f37f136 ci: Add `CFLAGS: -O1` to task matrix a6ca76cdf2 Avoid `-Wmaybe-uninitialized` when compiling with `gcc -O1` 0fa84f869d Merge bitcoin-core/secp256k1#1358: tests: introduce helper for non-zero `random_fe_test()` results 5a95a268b9 tests: introduce helper for non-zero `random_fe_test` results 304421d57b tests: refactor: remove duplicate function `random_field_element_test` 3aef6ab8e1 Merge bitcoin-core/secp256k1#1345: field: Static-assert that int args affecting magnitude are constant 4494a369b6 Merge bitcoin-core/secp256k1#1357: tests: refactor: take use of `secp256k1_ge_x_on_curve_var` 799f4eec27 Merge bitcoin-core/secp256k1#1356: ci: Adjust Docker image to Debian 12 "bookworm" c862a9fb49 ci: Adjust Docker image to Debian 12 "bookworm" a1782098a9 ci: Force DWARF v4 for Clang when Valgrind tests are expected 7d8d5c86df tests: refactor: take use of `secp256k1_ge_x_on_curve_var` 8a7273465b Help the compiler prove that a loop is entered fd491ea1bb Merge bitcoin-core/secp256k1#1355: Fix a typo in the error message ac43613d25 Merge bitcoin-core/secp256k1#1354: Add ellswift to CHANGELOG 67887ae65c Fix a typo in the error message 926dd3e962 Merge bitcoin-core/secp256k1#1295: abi: Use dllexport for mingw builds 10836832e7 Merge bitcoin-core/secp256k1#1336: Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC 7c7467ab7f Refer to ellswift.md in API docs c32ffd8d8c Add ellswift to CHANGELOG 3c1a0fd37f Merge bitcoin-core/secp256k1#1347: field: Document return value of fe_sqrt() 5779137457 field: Document return value of fe_sqrt() be8ff3a02a field: Static-assert that int args affecting magnitude are constant efa76c4bf7 group: remove unneeded normalize_weak in `secp256k1_ge_is_valid_var` 5b7bf2e9d4 Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC 05873bb6b1 tweak_add: fix API doc for tweak=0 6ec3731e8c Simplify test PRNG implementation fb5bfa4eed Add static test vector for Xoshiro256++ 723e8ca8f7 Remove randomness tests bc7c8db179 abi: Use dllexport for mingw builds git-subtree-dir: src/secp256k1 git-subtree-split: c545fdc374964424683d9dac31a828adedabe860
2023-07-18Merge bitcoin/bitcoin#28090: validation: use noexcept instead of deprecated ↵fanquake
throw() 047daad4f59942488163c6be8516a69291646294 clang-tidy: turn on modernize-use-noexcept (fanquake) 85e9e1f80236b7f3768bb69415ad35c80460e120 validation: use noexcept instead of deprecated throw() (fanquake) Pull request description: We fixed this once before in https://github.com/bitcoin/bitcoin/pull/10965. Turn on https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-noexcept.html#modernize-use-noexcept. ACKs for top commit: MarcoFalke: lgtm ACK 047daad4f59942488163c6be8516a69291646294 sipa: utACK 047daad4f59942488163c6be8516a69291646294 Empact: utACK https://github.com/bitcoin/bitcoin/commit/047daad4f59942488163c6be8516a69291646294 stickies-v: ACK 047daad4f59942488163c6be8516a69291646294 Tree-SHA512: 949b0fe598d66583747853094db13f196b402000e601f8634e5a708b55454d29c5aa18eaf1f2420d3ccf10e3e524b7414ff3a6fe4cb431420bf749c22b2b8bab
2023-07-17Merge bitcoin/bitcoin#27997: Descriptors: rule out unspendable miniscript ↵Andrew Chow
descriptors c7db88af71b3204171f33399aa4f33b40a4f7cd9 descriptor: assert we never parse a sane miniscript with no pubkey (Antoine Poinsot) a49402a9ec7431c286139b76f8759719a99a8551 qa: make sure we don't let unspendable Miniscript descriptors be imported (Antoine Poinsot) 639e3b6c9759a7a582c5c86fdbfa5ea99cb7bb16 descriptor: refuse to parse unspendable miniscript descriptors (Antoine Poinsot) e3280eae1b53006d74d11f3cf9d7a9dc7ff2c39e miniscript: make GetStackSize() and GetOps() return optionals (Antoine Poinsot) Pull request description: `IsSane()` in Miniscript does not ensure a Script is actually spendable. This is an issue as we would accept any sane Miniscript when parsing a descriptor. Fix this by explicitly checking a Miniscript descriptor is both sane and spendable when parsing it. This bug was exposed due to a check added in #22838 (https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1226859880) that triggered a fuzz crash (https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057). ACKs for top commit: sipa: utACK c7db88af71b3204171f33399aa4f33b40a4f7cd9 achow101: ACK c7db88af71b3204171f33399aa4f33b40a4f7cd9 Tree-SHA512: e79bc9f7842e98a4e8f358f05811fca51b15b4b80a171c0d2b17cf4bb1f578a18e4397bc2ece9817d392e0de0196ee6a054b7318441fd3566dd22e1f03eb64a5
2023-07-17Merge bitcoin/bitcoin#27993: Make poly1305 support incremental computation + ↵Andrew Chow
modernize 4e5c933f6a40c07d1c68115f7979b89a5b2ba580 Switch all callers from poly1305_auth to Poly1305 class (Pieter Wuille) 8871f7d1ae096839abcbf25a548319185acc01a2 tests: add more Poly1305 test vectors (Pieter Wuille) 40e6c5b9fce92ffe64e91c2aba38bb2ed57bfbfb crypto: add Poly1305 class with std::byte Span interface (Pieter Wuille) 50269b391fa18556bad72dc8c2fb4e2493a6a054 crypto: switch poly1305 to incremental implementation (Pieter Wuille) Pull request description: Our current Poly1305 code (src/crypto/poly1305.*) only supports computing the entire tag in one go (the `poly1305_auth` function takes a key and message, and outputs the tag). However, the RFC8439 authenticated encryption (as used in BIP324, see #27634) scheme makes use of Poly1305 in a way where the message consists of 3 different pieces: * The additionally authenticated data (AAD), padded to 16 bytes. * The ciphertext, padded to 16 bytes. * The length of the AAD and the length of the ciphertext, together another 16 bytes. Implementing RFC8439 using the existing `poly1305_auth` function requires creating a temporary copy with all these pieces of data concatenated just for the purpose of computing the tag (the approach used in #25361). This PR replaces the poly1305 code with new code from https://github.com/floodyberry/poly1305-donna (with minor adjustments to make it match our coding style and use our utility functions, documented in the commit) which supports incremental operation, and then adds a C++ wrapper interface using std::byte Spans around it, and adds tests that incremental and all-at-once computation match. ACKs for top commit: achow101: ACK 4e5c933f6a40c07d1c68115f7979b89a5b2ba580 theStack: ACK 4e5c933f6a40c07d1c68115f7979b89a5b2ba580 stratospher: tested ACK 4e5c933. Tree-SHA512: df6e9a2a4a38a480f9e4360d3e3def5311673a727a4a85b008a084cf6843b260dc82cec7c73e1cecaaccbf10f3521a0ae7dba388b65d0b086770f7fbc5223e2a
2023-07-17clang-tidy: turn on modernize-use-noexceptfanquake
2023-07-17validation: use noexcept instead of deprecated throw()fanquake
```bash CXX libbitcoin_node_a-validation.o validation.cpp:5164:30: warning: dynamic exception specifications are deprecated [-Wdeprecated-dynamic-exception-spec] const char* what() const throw() override ^~~~~~~ validation.cpp:5164:30: note: use 'noexcept' instead const char* what() const throw() override ^~~~~~~ noexcept ```
2023-07-17Merge bitcoin/bitcoin#28065: fuzz: Flatten all FUZZ_TARGET macros into onefanquake
fa6dfaaf45bde465969fa7d8fa6b6574a497a6ca scripted-diff: Use new FUZZ_TARGET macro everywhere (MarcoFalke) fa36ad8b091c70190491280dcf0794e94e34a9ed fuzz: Accept options in FUZZ_TARGET macro (MarcoFalke) Pull request description: The `FUZZ_TARGET` macros have many issues: * The developer will have to pick the right macro to pass the wanted option. * Adding a new option requires doubling the number of existing macros in the worst case. Fix all issues by using only a single macro. This refactor does not change behavior. ACKs for top commit: dergoegge: ACK fa6dfaaf45bde465969fa7d8fa6b6574a497a6ca Tree-SHA512: 49a34553867a1734ce89e616b2d7c29b784a67cd8990db6573f0c7b18957636ef0c81d3d0d444a04c12cdc98bc4c4aa7a2ec94e6232dc363620a746e28416444
2023-07-17Merge bitcoin/bitcoin#28086: fuzz: Bump FuzzedDataProvider.hfanquake
fa367422efa3c00f27dab2b58f2080303ed18b91 fuzz: Bump FuzzedDataProvider.h (MarcoFalke) Pull request description: Also, remove suppression. ACKs for top commit: dergoegge: utACK fa367422efa3c00f27dab2b58f2080303ed18b91 Tree-SHA512: 1d960cbedc4f516ef3dcec05b158164eb9673bcb02793c39d4b345be6d767aded1569289175701bc7382afd00ca41a2409831877f100ab9324969de9045ab6fc
2023-07-17refactor: use Span for SipHash::WriteSebastian Falbesoner
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2023-07-17fuzz: Bump FuzzedDataProvider.hMarcoFalke
From https://github.com/llvm/llvm-project/blob/fa8401f9bfe81f4853bf9b67bff42a2cebffc10f/compiler-rt/include/fuzzer/FuzzedDataProvider.h
2023-07-16Merge bitcoin-core/gui#740: Show own outputs on PSBT signing windowHennadii Stepanov
4da243ba023f2987e97fc62886c6ebc70d6ee50a qt: show own outputs on PSBT signing window (Hernan Marino) Pull request description: This fixes https://github.com/bitcoin-core/gui/issues/732 . It allows you to identify your own addresses in the outputs of a transaction in the PSBT signing window. This enables easy identification of change outputs, and prevents certain attacks where someone (co-signers of a multisig, or others ) might trick you into signing a transaction while they are stealing the change, since prior to this modification there was no easy way of knowing this. The identification of the output is similar to the way this is done in the transaction details window. A sample output is : ![image](https://github.com/bitcoin-core/gui/assets/87907936/48b8a652-7570-466b-9a34-cc0303c86d8c) ACKs for top commit: achow101: ACK 4da243ba023f2987e97fc62886c6ebc70d6ee50a jarolrod: ACK 4da243ba023f2987e97fc62886c6ebc70d6ee50a Tree-SHA512: fa9901d2acc84472c11afcd0a59a859db598cdf5cea755b492178d3e7434b70d9bd8f554928938a2ff9920c8f397fef814ce14b416556c30fba0c3c1f62cd722
2023-07-14Merge bitcoin/bitcoin#28048: kernel: Remove StartShutdown calls from ↵Andrew Chow
validation code 31eca93a9eb8e54f856d3f558aa3c831ef181d37 kernel: Remove StartShutdown calls from validation code (Ryan Ofsky) Pull request description: This change drops the last kernel dependency on shutdown.cpp. It also adds new hooks for libbitcoinkernel applications to be able to interrupt kernel operations when the chain tip changes. This change is mostly a refactoring, but does slightly change `-stopatheight` behavior (see release note and commit message) ACKs for top commit: TheCharlatan: ACK 31eca93a9eb8e54f856d3f558aa3c831ef181d37 furszy: Concept and light review ACK 31eca93a hebasto: ACK 31eca93a9eb8e54f856d3f558aa3c831ef181d37, I have reviewed the code and it looks OK. MarcoFalke: lgtm ACK 31eca93a9eb8e54f856d3f558aa3c831ef181d37 🕷 Tree-SHA512: e26928436bcde658e842b1f92e9c24b1ce91031fb63b41aafccf3130bfff532b75338a269a2bb7558bff2973913f17b97a00fec3e7e0588e2ce44de097142047
2023-07-14Added `longpollid` and `data` params to `template_request` #27998Rhythm Garg
Added `longpollid` and `data` params to `template_request` in `getblocktemplate` #27998
2023-07-13Merge bitcoin/bitcoin#27549: fuzz: addrman, add coverage for `network` field ↵Andrew Chow
in `Select()`, `Size()` and `GetAddr()` 35a2175ad8bec92b409ae2202c124e39b2f3f838 fuzz: addrman, add coverage for `network` field in `Select()`, `Size()` and `GetAddr()` (brunoerg) Pull request description: This PR adds fuzz coverage for `network` field in `Select()`, `Size()` and `GetAddr()`, there was only call to them without passing a network. https://marcofalke.github.io/b-c-cov/fuzz.coverage/src/addrman.cpp.gcov.html ACKs for top commit: amitiuttarwar: for the record, ACK 35a2175ad8bec92b409ae2202c124e39b2f3f838 - only small changes from the version (previously) proposed in 27213 achow101: ACK 35a2175ad8bec92b409ae2202c124e39b2f3f838 mzumsande: Code Review ACK 35a2175ad8bec92b409ae2202c124e39b2f3f838, haven't tested this yet, but I will let the fuzzer run for a while now. Tree-SHA512: dddb8322298d6c373c8e68d57538470b11825a9a310a355828c351d5c0b19ff6779d024a800e3ea90126d0c050e86f71fd22cd23d1a306c784cef0f82c45e3ca
2023-07-13scripted-diff: Use new FUZZ_TARGET macro everywhereMarcoFalke
-BEGIN VERIFY SCRIPT- ren() { sed --regexp-extended -i "s|$1|$2|g" $(git grep -l --extended-regexp "$1"); } # Replace FUZZ_TARGET_INIT ren 'FUZZ_TARGET_INIT\((.+), (.+)\)' 'FUZZ_TARGET(\1, .init = \2)' # Delete unused FUZZ_TARGET_INIT sed -i -e '37,39d' src/test/fuzz/fuzz.h -END VERIFY SCRIPT-
2023-07-13fuzz: Accept options in FUZZ_TARGET macroMarcoFalke
* This allows to reduce the number of total macros. * Also, adding a new option no longer requires doubling the number of macros in the worst case.
2023-07-13Merge bitcoin/bitcoin#27411: p2p: Restrict self-advertisements with privacy ↵Andrew Chow
networks to avoid fingerprinting e7cf8657e1165ea5da3911a9e543837cd8938f97 test: add unit test for local address advertising (Martin Zumsande) f4754b9dfb84859166843fb2a1888fb3cfebf73c net: restrict self-advertisements with privacy networks (Martin Zumsande) e4d541c7cfa65da77e80e6786fdcb197ab50d04b net, refactor: pass reference for peer address in GetReachabilityFrom (Martin Zumsande) 62d73f5370415f910c95a67b3d9f97bc85487bbe net, refactor: pass CNode instead of CNetAddr to GetLocalAddress (Martin Zumsande) Pull request description: The current logic for self-advertisements works such that we detect as many local addresses as we can, and then, using the scoring matrix from `CNetAddr::GetReachabilityFrom()`, self-advertise with the address that fits best to our peer. It is in general not hard for our peers to distinguish our self-advertisements from other addrs we send them, because we self-advertise every ~24h and because the first addr we send over a connection is likely our self-advertisement. `GetReachabilityFrom()` currently only takes into account actual reachability, but not whether we'd _want_ to announce our identity for one network to peers from other networks, which is not straightforward in connection with privacy networks. While the general approach is to prefer self-advertising with the address for the network our peer is on, there are several special situations in which we don't have one, and as a result could allow self-advertise other local addresses, for example: A) We run i2p and clearnet, use `-i2pacceptincoming=0` (so we have no local i2p address), and we have a local ipv4 address. In this case, we'd advertise the ipv4 address to our outbound i2p peers. B) Our `-discover` logic cannot detect any local clearnet addresses in our network environment, but we are actually reachable over clearnet. If we ran bitcoind clearnet-only, we'd always advertise the address our peer sees us with instead, and could get inbound peers this way. Now, if we also have an onion service running (but aren't using tor as a proxy for clearnet connections), we could advertise our onion address to clearnet peers, so that they would be able to connect our clearnet and onion identities. This PR tries to avoid these situations by 1.) never advertising our local Tor or I2P address to peers from other networks. 2.) never advertising local addresses from non-anonymity networks to peers from Tor or I2P Note that this affects only our own self-advertisements, the rules to forward other people's addrs are not changed. [Edit] after Initial [discussion](https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1497176155): CJDNS is not being treated like Tor and I2P at least for now, because it has different privacy properties and for the practical reason that it has still very few bitcoin nodes. ACKs for top commit: achow101: ACK e7cf8657e1165ea5da3911a9e543837cd8938f97 vasild: ACK e7cf8657e1165ea5da3911a9e543837cd8938f97 luke-jr: utACK e7cf8657e1165ea5da3911a9e543837cd8938f97 Tree-SHA512: 3db8415dea6f82223d11a23bd6cbb3b8cf68831321280e926034a1f110cbe22562570013925f6fa20d8f08e41d0202fd69c733d9f16217318a660d2a1a21b795
2023-07-12Switch all callers from poly1305_auth to Poly1305 classPieter Wuille
This also removes the old poly1305_auth interface, as it no longer serves any function. The new Poly1305 class based interface is more modern and safe.
2023-07-12tests: add more Poly1305 test vectorsPieter Wuille
2023-07-12crypto: add Poly1305 class with std::byte Span interfacePieter Wuille
2023-07-12crypto: switch poly1305 to incremental implementationPieter Wuille
This code is taken from poly1305-donna-32.h, poly1305-donna.h, poly1305-donna.c from https://github.com/floodyberry/poly1305-donna, commit e6ad6e091d30d7f4ec2d4f978be1fcfcbce72781, with the following modifications: * Coding style (braces around one-line indented if/for loops). * Rename unsigned long (long) to uint32_t and uint64_t. * Rename poly1305_block_size to POLY1305_BLOCK_SIZE. * Adding noexcept to functions. * Merging poly1305_state_internal_t and poly1305_context types. * Merging code from multiple files. * Place all imported code in the poly1305_donna namespace.
2023-07-12Merge bitcoin/bitcoin#27985: Add support for RFC8439 variant of ChaCha20Andrew Chow
0bf87476f55dceb106563156c7c8d6bfb8162e29 test: add ChaCha20 test triggering 32-bit block counter overflow (Sebastian Falbesoner) 7f2a985147ef541123c65d5db1c3fc3e533fd4ce tests: improve ChaCha20 unit tests (Pieter Wuille) 511a8d406e3115b97c6d35e2c603af53b3f9da13 crypto: Implement RFC8439-compatible variant of ChaCha20 (Pieter Wuille) Pull request description: Based on and replaces part of #25361, part of the BIP324 project (#27634). See also #19225 for background. There are two variants of ChaCha20 in use. The currently implemented one uses a 64-bit nonce and a 64-bit block counter, while the one used in RFC8439 (and thus BIP324) uses a 96-bit nonce and 32-bit block counter. This PR changes the logic to use the 96-bit nonce variant, though in a way that's compatible with >256 GiB output (by automatically incrementing the first 32-bit part of the nonce when the block counter overflows). For those who reviewed the original PR, the biggest change is here that the 96-bit nonce is passed as a Nonce96 type (pair of 32-bit + 64-bit integer) rather than a 12-byte array. ACKs for top commit: achow101: ACK 0bf87476f55dceb106563156c7c8d6bfb8162e29 theStack: Code-review ACK 0bf87476f55dceb106563156c7c8d6bfb8162e29 Tree-SHA512: 62e4cbd5388b8d50ef1a0dc99b6f4ad36c7b4419032035f8e622dda63a62311dd923032217e20054bcd836865d4be5c074f9e5538ca158f94f08eab75c5519c1
2023-07-11refactor: Remove duplicate allNetMessageTypesVecMarcoFalke
With C++11 (and later), the duplicate variable is no longer needed. Also, run clang-format on the namespace, as the script in the next commit relies on a specific format. This prevents a clang-format run in the future from breaking the script. (Review hint: --ignore-all-space).
2023-07-11kernel: Remove StartShutdown calls from validation codeRyan Ofsky
This change drops the last kernel dependency on shutdown.cpp. It also adds new hooks for libbitcoinkernel applications to be able to interrupt kernel operations when the chain tip changes. This is a refactoring that does not affect behavior. (Looking at the code it can appear like the new break statement in the ActivateBestChain function is a change in behavior, but actually the previous StartShutdown call was indirectly triggering a break before, because it was causing m_chainman.m_interrupt to be true. The new code just makes the break more obvious.)
2023-07-11Merge bitcoin/bitcoin#28044: test: indexes, fix on error infinite loopRyan Ofsky
89ba8905f5c68ae29412f9c4010314c5a113c234 test: indexes, fix on error infinite loop (furszy) Pull request description: Coming from https://github.com/bitcoin/bitcoin/pull/28036#issuecomment-1623813703, I thought that we were going to fix it there but seems that got merged without it for some reason. As index sync failures trigger a shutdown request without notifying `BaseIndex::BlockUntilSyncedToCurrentChain` in any way, we also need to check whether a shutdown was requested or not inside 'IndexWaitSynced'. Otherwise, any error inside the index sync process will hang the test forever. ACKs for top commit: MarcoFalke: lgtm ACK 89ba8905f5c68ae29412f9c4010314c5a113c234 jamesob: ACK 89ba890 ryanofsky: Code review ACK 89ba8905f5c68ae29412f9c4010314c5a113c234. Just comment update since last review Tree-SHA512: 1f6daf34e51d3fbc802799bfa4ac0ef0d8f774db5f9e2f5d35df18a77679778475c94efc3da1fb723ebaf3583e4075e4a5cbe4a5104ad0c50e2b32076e247b29
2023-07-11wallet: loading, log descriptor parsing error detailsfurszy
The `UNKNOWN_DESCRIPTOR` error comes from the `WalletDescriptor::DeserializeDescriptor` std::ios_base exception, which contains further information about the parsing error.
2023-07-11refactor: Move stopafterblockimport handling out of blockstorageTheCharlatan
This has the benefit of moving the StartShutdown call out of the blockstorage file and thus out of the kernel's responsibility. The user can now decide if he wants to start shutdown / interrupt after a block import or not.
2023-07-10test: indexes, fix on error infinite loopfurszy
As index sync failures trigger a shutdown request without notifying BaseIndex::BlockUntilSyncedToCurrentChain in any way, we also need to check whether a shutdown was requested or not inside 'IndexWaitSynced'. Otherwise, any error inside the index sync process will hang the test forever.
2023-07-10Merge bitcoin/bitcoin#27607: index: make startup more efficientRyan Ofsky
ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550 index: verify blocks data existence only once (furszy) fcbdaeef4d5a63e3e5b479c6fcad730eb86fb923 init: don't start indexes sync thread prematurely (furszy) 2ec89f1970935d27631bcd17b7923a79cdb1edbb refactor: simplify pruning violation check (furszy) c82ef91eae38f385d408b095ebbc8a180ce52ebb make GetFirstStoredBlock assert that 'start_block' always has data (furszy) 430e7027a18870a296abb0bbd9332cbe40d8fdc0 refactor: index, decouple 'Init' from 'Start' (furszy) 225e213110602b4fd1d345167f5f92d26557f6c1 refactor: init indexes, decouple 'Start()' from the creation step (furszy) 2ebc7e68cc9d347807b646f601b27940c9590c89 doc: describe 'init load' thread actions (Martin Zumsande) 04575106b2529f495ce8110ddf7ed2247d4bc339 scripted-diff: rename 'loadblk' thread name to 'initload' (furszy) ed4462cc78afd2065bbf5bd79728852b65b9ad7f init: start indexes sync earlier (furszy) Pull request description: Simplifies index startup code, eliminating the `g_indexes_ready_to_sync` variable, deduplicating code and moving the prune violation check out of the `BaseIndex` class. Also makes startup more efficient by running the prune violation check once for all indexes instead of once for each index, and by delaying the prune violation check and moving it off of the main thread so the node can start up faster and perform the block data availability verification even when the '-reindex" or the "-reindex-chainstate" flags are enabled (which hasn't being possible so far). ACKs for top commit: ryanofsky: Code review ACK ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550. Just rebase and suggested changes since last review (Start return check, and code simplification) TheCharlatan: re-ACK ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550 Tree-SHA512: e9c98ce89aeb29e8d0f505f17b34aa54fe44efefbf017f4746e3b446ab4de25ade4f707254a0bbe4b99b69731b04a4067ce529eb7aa834ced196784b694cf7ce
2023-07-10index: verify blocks data existence only oncefurszy
At present, during init, we traverse the chain (once per index) to confirm that all necessary blocks to sync each index up to the current tip are present. To make the process more efficient, we can fetch the oldest block from the indexers and perform the chain data existence check from that point only once. This also moves the pruning violation check to the end of the 'loadinit' thread, which is where the reindex, block loading and chain activation processes happen. Making the node's startup process faster, allowing us to remove the global g_indexes_ready_to_sync flag, and enabling the execution of the pruning violation verification even when the reindex or reindex-chainstate flags are enabled (which has being skipped so far).
2023-07-10init: don't start indexes sync thread prematurelyfurszy
By moving the 'StartIndexes()' call into the 'initload' thread, we can remove the threads active wait. Optimizing the available resources. The only difference with the current state is that now the indexes threads will only be started when they can process work and not before it.
2023-07-10refactor: simplify pruning violation checkfurszy
By generalizing 'GetFirstStoredBlock' and implementing 'CheckBlockDataAvailability' we can dedup code and avoid repeating work when multiple indexes are enabled. E.g. get the oldest block across all indexes and perform the pruning violation check from that point up to the tip only once (this feature is being introduced in a follow-up commit). This commit shouldn't change behavior in any way. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>