aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-09-07crypto: Spanify EllSwiftPubKey constructorPieter Wuille
2023-09-07net: remove unused Transport::SetReceiveVersionPieter Wuille
2023-09-07net: add have_next_message argument to Transport::GetBytesToSend()Pieter Wuille
Before this commit, there are only two possibly outcomes for the "more" prediction in Transport::GetBytesToSend(): * true: the transport itself has more to send, so the answer is certainly yes. * false: the transport has nothing further to send, but if vSendMsg has more message(s) left, that still will result in more wire bytes after the next SetMessageToSend(). For the BIP324 v2 transport, there will arguably be a third state: * definitely not: the transport has nothing further to send, but even if vSendMsg has more messages left, they can't be sent (right now). This happens before the handshake is complete. To implement this, we move the entire decision logic to the Transport, by adding a boolean to GetBytesToSend(), called have_next_message, which informs the transport whether more messages are available. The return values are still true and false, but they mean "definitely yes" and "definitely no", rather than "yes" and "maybe".
2023-09-07Merge bitcoin/bitcoin#25284: net: Use serialization parameters for CAddress ↵fanquake
serialization fa626af3edbe8d98b2de91dd71729ceef90389fb Remove unused legacy CHashVerifier (MarcoFalke) fafa3fc5a62702da72991497e3270034eb9159c0 test: add tests that exercise WithParams() (MarcoFalke) fac81affb527132945773a5315bd27fec61ec52f Use serialization parameters for CAddress serialization (MarcoFalke) faec591d64e40ba7ec7656cbfdda1a05953bde13 Support for serialization parameters (MarcoFalke) fac42e9d35f6ba046999b2e3a757ab720c51b6bb Rename CSerAction* to Action* (MarcoFalke) aaaa3fa9477eef9ea72e4a501d130c57b47b470a Replace READWRITEAS macro with AsBase wrapping function (MarcoFalke) Pull request description: It seems confusing that picking a wrong value for `ADDRV2_FORMAT` could have effects on consensus. (See the docstring of `ADDRV2_FORMAT`). Fix this by implementing https://github.com/bitcoin/bitcoin/issues/19477#issuecomment-1147421608 . This may also help with libbitcoinkernel, see https://github.com/bitcoin/bitcoin/pull/28327 ACKs for top commit: TheCharlatan: ACK fa626af3edbe8d98b2de91dd71729ceef90389fb ajtowns: ACK fa626af3edbe8d98b2de91dd71729ceef90389fb Tree-SHA512: 229d379da27308890de212b1fd2b85dac13f3f768413cb56a4b0c2da708f28344d04356ffd75bfcbaa4cabf0b6cc363c4f812a8f1648cff9e436811498278318
2023-09-07Merge bitcoin/bitcoin#28419: fuzz: introduce and use `ConsumePrivateKey` helperfanquake
583af18fd1d0bda5a6a1d0403ffc498a512a546d fuzz: introduce and use `ConsumePrivateKey` helper (Sebastian Falbesoner) Pull request description: In the course of reviewing BIP324 related PRs I noticed a frequent pattern of creating private keys (`CKey` instances) with data consumed from the fuzz data provider: ``` auto key_data = provider.ConsumeBytes<unsigned char>(32); key_data.resize(32); CKey key; key.Set(key_data.begin(), key_data.end(), /*fCompressedIn=*/true); ``` This PR introduces a corresponding helper `ConsumePrivateKey` in order to deduplicate code. The compressed flag can either be set to a fixed value, or, if `std::nullopt` is passed (=default), is also consumed from the fuzz data provider via `.ConsumeBool()`. Note that this is not a pure refactor, as some of the replaced call-sites previously consumed a random length (`ConsumeRandomLengthByteVector`) instead of a fixed size of 32 bytes for key data. As far as I can see, there is not much value in using a random size, as in all those cases we can only proceed or do something useful with a valid private key, and key data with sizes other than 32 bytes always lead to invalid keys. ACKs for top commit: sipa: utACK 583af18fd1d0bda5a6a1d0403ffc498a512a546d brunoerg: crACK 583af18fd1d0bda5a6a1d0403ffc498a512a546d Tree-SHA512: 58a178432ba1eb0a2f7597b6700e96477e8b10f429ef642445a52db12e74d81aec307888315b772bfda9610f90df3e1d556cf024c2bef1d520303b12584feaaa
2023-09-06Merge bitcoin/bitcoin#26567: Wallet: estimate the size of signed inputs ↵Andrew Chow
using descriptors 10546a569c6c96a5ec1b9708abf9ff5c8644f669 wallet: accurately account for the size of the witness stack (Antoine Poinsot) 9b7ec393b82ca9d7ada77d06e0835df0386a8b85 wallet: use descriptor satisfaction size to estimate inputs size (Antoine Poinsot) 8d870a98731e8db5ecc614bb5f7c064cbf30c7f4 script/signingprovider: introduce a MultiSigningProvider (Antoine Poinsot) fa7c46b503f0b69630f55dc43021d2099e3515ba descriptor: introduce a method to get the satisfaction size (Antoine Poinsot) bdba7667d2d65f31484760a8e8420c488fc5f801 miniscript: introduce a helper to get the maximum witness size (Antoine Poinsot) 4ab382c2cdb09fb4056711b4336807845cbe1ad5 miniscript: make GetStackSize independent of P2WSH context (Antoine Poinsot) Pull request description: The wallet currently estimates the size of a signed input by doing a dry run of the signing logic. This is unnecessary since all outputs we can sign for can be represented by a descriptor, and we can derive the size of a satisfaction ("signature") directly from the descriptor itself. In addition, the current approach does not generalize well: dry runs of the signing logic are only possible for the most basic scripts. See for instance the discussion in #24149 around that. This introduces a method to get the maximum size of a satisfaction from a descriptor, and makes the wallet use that instead of the dry-run. ACKs for top commit: sipa: utACK 10546a569c6c96a5ec1b9708abf9ff5c8644f669 achow101: re-ACK 10546a569c6c96a5ec1b9708abf9ff5c8644f669 Tree-SHA512: 43ed1529fbd30af709d903c8c5063235e8c6a03b500bc8f144273d6184e23a53edf0fea9ef898ed57d8a40d73208b5d935cc73b94a24fad3ad3c63b3b2027174
2023-09-06fuzz: introduce and use `ConsumePrivateKey` helperSebastian Falbesoner
2023-09-05Merge bitcoin/bitcoin#28195: blockstorage: Drop legacy -txindex checkfanquake
fae405556d56f6f13ce57f69a06b9ec1e825422b scripted-diff: Rename CBlockTreeDB -> BlockTreeDB (MarcoFalke) faf63039cce40f5cf8dea5a1d24945773c3433a1 Fixup style of moved code (MarcoFalke) fa65111b99627289fd47dcfaa5197e0f09b8a50e move-only: Move CBlockTreeDB to node/blockstorage (MarcoFalke) fa8685597e7302fc136f21b6dd3a4b187fa8e251 index: Drop legacy -txindex check (MarcoFalke) fa69148a0a26c5054dbccdceeac8e117bf449275 scripted-diff: Use blocks_path where possible (MarcoFalke) Pull request description: The only reason for the check was to print a warning about an increase in storage use. Now that 22.x is EOL and everyone should have migrated (or decided to not care about storage use), remove the check. Also, a move-only commit is included. (Rebased from https://github.com/bitcoin/bitcoin/pull/22242) ACKs for top commit: TheCharlatan: ACK fae405556d56f6f13ce57f69a06b9ec1e825422b, though I lack historical context to really judge the second commit fa8685597e7302fc136f21b6dd3a4b187fa8e251. stickies-v: ACK fae405556d56f6f13ce57f69a06b9ec1e825422b Tree-SHA512: 9da8f48767ae52d8e8e21c09a40c949cc0838794f1856cc5f58a91acd3f00a3bca818c8082242b3fdc9ca5badb09059570bb3870850d3807b75a8e23b5222da1
2023-09-05Merge bitcoin/bitcoin#28404: Update libsecp256k1 subtree to release 0.4.0fanquake
c0da4f60e2145a9838c0c5d0a02592faf16d2d8d Squashed 'src/secp256k1/' changes from c545fdc374..199d27cea3 (Pieter Wuille) Pull request description: We had previously pulled in a non-released commit along with #27479. The necessary changes have now been released in version 0.4.0, so update to that. ACKs for top commit: hebasto: ACK 0e0fc18c3cf8c0aa4cca85c4294416463f157870, having a zero diff with my local branch that updates the `secp256k1` subtree up to v0.4.0. fanquake: ACK 0e0fc18c3cf8c0aa4cca85c4294416463f157870 Tree-SHA512: 8b771e7da89b9cdb7a680b9dd4eb99a6f737b32914b0b62c485b3c484e5438f9f60942030d3072243aaa196da22d2b1fdb3b6a668d75a46e6ac78c9d86b4bd8b
2023-09-05Merge bitcoin/bitcoin#28291: rpc: removed StrFormatInternalBug quote ↵fanquake
delimitation 6e8f6468cbf1320b70cf01333002a31b44cb7c33 removed StrFormatInternalBug quote delimitation (Reese Russell) Pull request description: This PR rectifies an unnecessary set of quotes delimiting the contents of ```StrFormatInternalBug```. This is a follow up to MarcoFalke https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1297191493. The method of action was to remove the escaped quotes that were a part of strprintf. A single functional test case was modified to reflect the new output format. ```STR_INTERNAL_BUG``` was applied to https://github.com/bitcoin/bitcoin/pull/28123 in ```std::string RPCArg::ToString(const bool oneline)``` in ```rpc/util.cpp``` The results can be seen below. Previously ![image](https://github.com/bitcoin/bitcoin/assets/3104223/53f9ea59-317f-4c62-9fc1-04255eeb4641) This PR ![image](https://github.com/bitcoin/bitcoin/assets/3104223/5c6a3110-f1f3-4b3c-8e8a-9c8f1c3176e7) Additional context can be found here. https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271871716 Thank you. ACKs for top commit: MarcoFalke: review ACK 6e8f6468cbf1320b70cf01333002a31b44cb7c33 stickies-v: ACK 6e8f6468cbf1320b70cf01333002a31b44cb7c33 Tree-SHA512: 35317e31a527630495b566407e37db9941dab7f81cfaeb1ea3309683c48e4273284645ad615f73e646a137b4f2ae35933603e9182a7dbdd22cac98d038c491dc
2023-09-05Remove unused legacy CHashVerifierMarcoFalke
2023-09-05test: add tests that exercise WithParams()MarcoFalke
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2023-09-05Use serialization parameters for CAddress serializationMarcoFalke
This also cleans up the addrman (de)serialization code paths to only allow `Disk` serialization. Some unit tests previously forced a `Network` serialization, which does not make sense, because Bitcoin Core in production will always `Disk` serialize. This cleanup idea was suggested by Pieter Wuille and implemented by Anthony Towns. Co-authored-by: Pieter Wuille <pieter@wuille.net> Co-authored-by: Anthony Towns <aj@erisian.com.au>
2023-09-04Update secp256k1 subtree to upstream release 0.4.0Pieter Wuille
2023-09-04Squashed 'src/secp256k1/' changes from c545fdc374..199d27cea3Pieter Wuille
199d27cea3 Merge bitcoin-core/secp256k1#1415: release: Prepare for 0.4.0 16339804c9 release: Prepare for 0.4.0 d9a85065a9 changelog: Catch up in preparation of release 0b4640aedd Merge bitcoin-core/secp256k1#1413: ci: Add `release` job 8659a01714 ci: Add `release` job f9b38894ba ci: Update `actions/checkout` version 727bec5bc2 Merge bitcoin-core/secp256k1#1414: ci/gha: Add ARM64 QEMU jobs for clang and clang-snapshot 2635068abf ci/gha: Let MSan continue checking after errors in all jobs e78c7b68eb ci/Dockerfile: Reduce size of Docker image further 2f0d3bbffb ci/Dockerfile: Warn if `ulimit -n` is too high when running Docker 4b8a647ad3 ci/gha: Add ARM64 QEMU jobs for clang and clang-snapshot 6ebe7d2bb3 ci/Dockerfile: Always use versioned clang packages 65c79fe2d0 Merge bitcoin-core/secp256k1#1412: ci: Switch macOS from Ventura to Monterey and add Valgrind c223d7e33d ci: Switch macOS from Ventura to Monterey and add Valgrind ea26b71c3a Merge bitcoin-core/secp256k1#1411: ci: Make repetitive command the default one cce0456304 ci: Make repetitive command the default one 317a4c48f0 ci: Move `git config ...` to `run-in-docker-action` 4d7fe60905 Merge bitcoin-core/secp256k1#1409: ci: Move remained task from Cirrus to GitHub Actions 676ed8f9cf ci: Move "C++ (public headers)" from Cirrus to GitHub Actions 61fc3a2dc8 ci: Move "C++ -fpermissive..." from Cirrus to GitHub Actions d51fb0a533 ci: Move "MSan" from Cirrus to GitHub Actions c22ac27529 ci: Move sanitizers task from Cirrus to GitHub Actions 26a989924b Merge bitcoin-core/secp256k1#1410: ci: Use concurrency for pull requests only ee1be62d84 ci: Use concurrency for pull requests only 6ee14550c8 Merge bitcoin-core/secp256k1#1406: ci, gha: Move more non-x86_64 tasks from Cirrus CI to GitHub Actions fc3dea29ea ci: Move "ppc64le: Linux..." from Cirrus to GitHub Actions 7782dc8276 ci: Move "ARM64: Linux..." from Cirrus to GitHub Actions 0a16de671c ci: Move "ARM32: Linux..." from Cirrus to GitHub Actions ea33914e00 ci: Move "s390x (big-endian): Linux..." from Cirrus to GitHub Actions 880be8af99 ci: Move "i686: Linux (Debian stable)" from Cirrus to GiHub Actions 2e6cf9bae5 Merge bitcoin-core/secp256k1#1396: ci, gha: Add "x86_64: Linux (Debian stable)" GitHub Actions job 5373693e45 Merge bitcoin-core/secp256k1#1405: ci: Drop no longer needed workaround ef9fe959de ci: Drop no longer needed workaround e10878f58e ci, gha: Drop `driver-opts.network` input for `setup-buildx-action` 4ad4914bd1 ci, gha: Add `retry_builder` Docker image builder 6617a620d9 ci: Remove "x86_64: Linux (Debian stable)" task from Cirrus CI 03c9e6508c ci, gha: Add "x86_64: Linux (Debian stable)" GitHub Actions job ad3e65d9fe ci: Remove GCC build files and sage to reduce size of Docker image 6b9507adf6 Merge bitcoin-core/secp256k1#1398: ci, gha: Add Windows jobs based on Linux image 87d35f30c0 ci: Rename `cirrus.sh` to more general `ci.sh` d6281dd008 ci: Remove Windows tasks from Cirrus CI 2b6f9cd546 ci, gha: Add Windows jobs based on Linux image 48b1d939b5 Merge bitcoin-core/secp256k1#1403: ci, gha: Ensure only a single workflow processes `github.ref` at a time 0ba2b94551 Merge bitcoin-core/secp256k1#1373: Add invariant checking for scalars 060e32cb60 Merge bitcoin-core/secp256k1#1401: ci, gha: Run all MSVC tests on Windows natively de657c2044 Merge bitcoin-core/secp256k1#1062: Removes `_fe_equal_var`, and unwanted `_fe_normalize_weak` calls (in tests) bcffeb14bc Merge bitcoin-core/secp256k1#1404: ci: Remove "arm64: macOS Ventura" task from Cirrus CI c2f6435802 ci: Add comment about switching macOS to M1 on GHA later 4a24fae0bc ci: Remove "arm64: macOS Ventura" task from Cirrus CI b0886fd35c ci, gha: Ensure only a single workflow processes `github.ref` at a time 3d05c86d63 Merge bitcoin-core/secp256k1#1394: ci, gha: Run "x86_64: macOS Ventura" job on GitHub Actions d78bec7001 ci: Remove Windows MSVC tasks from Cirrus CI 3545dc2b9b ci, gha: Run all MSVC tests on Windows natively 5d8fa825e2 Merge bitcoin-core/secp256k1#1274: test: Silent noisy clang warnings about Valgrind code on macOS x86_64 8e54a346d2 ci, gha: Run "x86_64: macOS Ventura" job on GitHub Actions b327abfcea Merge bitcoin-core/secp256k1#1402: ci: Use Homebrew's gcc in native macOS task d62db57427 ci: Use Homebrew's gcc in native macOS task 54058d16fe field: remove `secp256k1_fe_equal_var` bb4efd6404 tests: remove unwanted `secp256k1_fe_normalize_weak` call eedd781085 Merge bitcoin-core/secp256k1#1348: tighten group magnitude limits, save normalize_weak calls in group add methods (revival of #1032) b2f6712dd3 Merge bitcoin-core/secp256k1#1400: ctimetests: Use new SECP256K1_CHECKMEM macros also for ellswift 9c91ea41b1 ci: Enable ellswift module where it's missing db32a24761 ctimetests: Use new SECP256K1_CHECKMEM macros also for ellswift ce765a5b8e Merge bitcoin-core/secp256k1#1399: ci, gha: Run "SageMath prover" job on GitHub Actions 8408dfdc4c Revert "ci: Run sage prover on CI" c8d9914fb1 ci, gha: Run "SageMath prover" job on GitHub Actions 8d2960c8e2 Merge bitcoin-core/secp256k1#1397: ci: Remove "Windows (VS 2022)" task from Cirrus CI f1774e5ec4 ci, gha: Make MSVC job presentation more explicit 5ee039bb58 ci: Remove "Windows (VS 2022)" task from Cirrus CI 96294c00fb Merge bitcoin-core/secp256k1#1389: ci: Run "Windows (VS 2022)" job on GitHub Actions a2f7ccdecc ci: Run "Windows (VS 2022)" job on GitHub Actions 374e2b54e2 Merge bitcoin-core/secp256k1#1290: cmake: Set `ENVIRONMENT` property for examples on Windows 1b13415df9 Merge bitcoin-core/secp256k1#1391: refactor: take use of `secp256k1_scalar_{zero,one}` constants (part 2) a1bd4971d6 refactor: take use of `secp256k1_scalar_{zero,one}` constants (part 2) b7c685e74a Save _normalize_weak calls in group add methods c83afa66e0 Tighten group magnitude limits 26392da2fb Merge bitcoin-core/secp256k1#1386: ci: print $ELLSWIFT in cirrus.sh d23da6d557 use secp256k1_scalar_verify checks 4692478853 ci: print $ELLSWIFT in cirrus.sh c7d0454932 add verification for scalars c734c64278 Merge bitcoin-core/secp256k1#1384: build: enable ellswift module via SECP_CONFIG_DEFINES ad152151b0 update max scalar in scalar_cmov_test and fix schnorrsig_verify exhaustive test 78ca880788 build: enable ellswift module via SECP_CONFIG_DEFINES 0e00fc7d10 Merge bitcoin-core/secp256k1#1383: util: remove unused checked_realloc b097a466c1 util: remove unused checked_realloc 2bd5f3e618 Merge bitcoin-core/secp256k1#1382: refactor: Drop unused cast 4f8c5bd761 refactor: Drop unused cast 173e8d061a Implement current magnitude assumptions 49afd2f5d8 Take use of _fe_verify_magnitude in field_impl.h 4e9661fc42 Add _fe_verify_magnitude (no-op unless VERIFY is enabled) 690b0fc05a add missing group element invariant checks 175db31149 ci: Drop no longer needed `PATH` variable update on Windows 116d2ab3df cmake: Set `ENVIRONMENT` property for examples on Windows cef373997c cmake, refactor: Use helper function instead of interface library 747ada3587 test: Silent noisy clang warnings about Valgrind code on macOS x86_64 git-subtree-dir: src/secp256k1 git-subtree-split: 199d27cea32203b224b208627533c2e813cd3b21
2023-09-02Merge bitcoin/bitcoin#28383: Update translations for 26.0 soft translation ↵fanquake
string freeze 162ce4e5dd1899c2bd9418d8bf023bf344ffc2cc qt: Update translation source file (Hennadii Stepanov) 19520bff0975d456727575c5834b94f1525221b5 qt: Bump Transifex slug for 26.x (Hennadii Stepanov) 69821b27f59a4fe56a0284908b14ba15d314e6d0 qt: Translation updates from Transifex (Hennadii Stepanov) Pull request description: This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md). Required to open Transifex translations for 26.0 on 2023-09-01 as it's [planned](https://github.com/bitcoin/bitcoin/issues/27758). The previous similar PR: https://github.com/bitcoin/bitcoin/pull/27169. ACKs for top commit: stickies-v: ACK 162ce4e5dd1899c2bd9418d8bf023bf344ffc2cc jarolrod: ACK 162ce4e5dd1899c2bd9418d8bf023bf344ffc2cc Tree-SHA512: a810c727e5c9fb3ffe4ff88ca7c5484178c483b38adb4b67758fb1f3e75b0d0ffb755fdc43de3fa59607f45833ec3594d87649fd6f0a40ed8a249ee5fa6979dd
2023-09-01Merge bitcoin/bitcoin#28350: Log explicit error message when coindb is found ↵Andrew Chow
in inconsistent state df60de770d8e27c00eced29d8a4a4bce7c6e4b30 log: Print error message when coindb is in inconsistent state (Fabian Jahr) Pull request description: While doing manual testing on assumeutxo this week I managed to put the coindb into an inconsistent state twice. For a normal user, this can also happen if their computer crashes during a flush or if they try to stop their node during a flush and then get tired of waiting and just shut their computer down or kill the process. It's an edge case but I wouldn't be surprised if this does happen more often when assumeutxo gets used more widely because there might be multiple flushes happening during loading of the UTXO set in the beginning and users may think something is going wrong because of the unexpected wait or they forgot some configs and want to start over quickly. The problem is, when this happens at first the node starts up normally until it's time to flush again and then it hits an assert that the user can not understand. ``` 2023-08-25T16:31:09Z [httpworker.0] [snapshot] 52000000 coins loaded (43.30%, 6768 MB) 2023-08-25T16:31:16Z [httpworker.0] Cache size (7272532192) exceeds total space (7256510300) 2023-08-25T16:31:16Z [httpworker.0] FlushSnapshotToDisk: flushing coins cache (7272 MB) started Assertion failed: (old_heads[0] == hashBlock), function BatchWrite, file txdb.cpp, line 126. Abort trap: 6 ``` We should at least log an error message that gives users a hint of what the problem is and what they can do to resolve it. I am keeping this separate from the assumeutxo project since this issue can also happen during any regular flush. ACKs for top commit: jonatack: ACK df60de770d8e27c00eced29d8a4a4bce7c6e4b30 achow101: ACK df60de770d8e27c00eced29d8a4a4bce7c6e4b30 ryanofsky: Code review ACK df60de770d8e27c00eced29d8a4a4bce7c6e4b30 jamesob: Code review ACK df60de770d8e27c00eced29d8a4a4bce7c6e4b30 Tree-SHA512: b546aa0b0323ece2962867a29c38e014ac83ae8f1ded090da2894b4ff2450c05229629c7e8892f7b550cf7def4038a0b4119812e548e11b00c60b1dc3d4276d2
2023-09-01qt: Update translation source fileHennadii Stepanov
The diff is generated by executing `make -C src translate`.
2023-09-01qt: Translation updates from TransifexHennadii Stepanov
The diff is generated by executing the `update-translations.py` script.
2023-08-31Merge bitcoin/bitcoin#28364: log: log wtxids when possible, add TXPACKAGES ↵Andrew Chow
category a3b55c94b9ffde07386aa887149ddca91b364967 [doc] move comment about AlreadyHaveTx DoS score to the right place (glozow) 3b8c17838a561616fd7c933753c7b98b6c6c7c99 [log] add more logs related to orphan handling (glozow) 51b3275cd1de467933f13d8b71286bf5ebd12b4b [log] add category TXPACKAGES for orphanage and package relay (glozow) a33dde1e41d8923a46db84b50550175fa6149c48 [log] include wtxid in tx {relay,validation,orphanage} logging (glozow) Pull request description: This was taken from #28031 (see #27463 for project tracking). - Log wtxids in addition to txids when possible. This allows us to track the fate of a transaction from inv to mempool accept/reject through logs. - Additional orphan-related logging to make testing and debugging easier. Suggested in https://github.com/bitcoin/bitcoin/pull/28031#pullrequestreview-1531022386 and https://github.com/bitcoin/bitcoin/pull/28031#discussion_r1269622220 - Add `TXPACKAGES` category for logging. - Move a nearby comment block that was in the wrong place. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/28364/commits/a3b55c94b9ffde07386aa887149ddca91b364967 achow101: ACK a3b55c94b9ffde07386aa887149ddca91b364967 brunoerg: crACK a3b55c94b9ffde07386aa887149ddca91b364967 mzumsande: Code review ACK a3b55c94b9ffde07386aa887149ddca91b364967 Tree-SHA512: 21884ef7c2ea2fd006e715574a9dd3e6cbbe8f82d62c6187fe1d39aad5a834051203fda5f355a06ca40c3e2b9561aec50d7c922a662b1edc96f7b552c9f4b24d
2023-08-31Merge bitcoin-core/gui#749: make '-min' minimize wallet loading dialogHennadii Stepanov
32db15450a9ef2a45de29b3b2ae60491a38edbd6 gui: make '-min' minimize wallet loading dialog (furszy) Pull request description: Simple fix for #748. When '-min' is enabled, no loading dialog should be presented on screen during startup. ACKs for top commit: hebasto: ACK 32db15450a9ef2a45de29b3b2ae60491a38edbd6, tested on Debian 11 + XFCE. Tree-SHA512: d08060b044938c67e8309db77b49ca645850fc21fdd7d78d5368d336fb9f602dcc66ea398a7505b00bf7d43afa07108347c7260480319fad3ec84cb41332f780
2023-08-29[doc] move comment about AlreadyHaveTx DoS score to the right placeglozow
This comment isn't in the right place, as detection of a tx in recent_rejects would cause the function to exit much earlier. Move the comment to the right place and tweak the first sentence for accuracy.
2023-08-29[log] add more logs related to orphan handlingglozow
- Whenever a tx is erased. Allows somebody to see which transactions have been erased due to expiry/overflow, not just how many. - Whenever a tx is added to a peer's workset. - AcceptToMemoryPool when a tx is accepted, mirroring the one logged for a tx received from a peer. This allows someone to see all of the transactions that are accepted to mempool just by looking for ATMP logs. - MEMPOOLREJ when a tx is rejected, mirroring the one logged for a tx received from a peer. This allows someone to see all of the transaction rejections by looking at MEMPOOLREJ logs.
2023-08-29[log] add category TXPACKAGES for orphanage and package relayglozow
2023-08-29[log] include wtxid in tx {relay,validation,orphanage} loggingglozow
2023-08-29Merge bitcoin/bitcoin#28354: test: default acceptnonstdtxn=0 on all chainsfanquake
13eb8aa572644a53ae0d631916cb4cbc273a92d1 doc: Release notes for testnet defaulting to -acceptnonstdtxn=0 (Anthony Towns) e1dc15d69061e69351c72907444e8ded0ac7c88c config: default acceptnonstdtxn=0 on all chains (Anthony Towns) Pull request description: Changes `-acceptnonstxtxn` to default to 0 on testnet, matching the other chains. Allowing non-standard txs on testnet by default contributed to the difficulties RSK described in #26348: "We see that there are two script paths and, to reduce the script size, a single CHECKMULTISIG is used for the two paths, separating the signer count from the CHECKMULTISIG opcode. This script worked on testnet, because it lacks the standard checks performed in Mainnet." ACKs for top commit: MarcoFalke: lgtm ACK 13eb8aa572644a53ae0d631916cb4cbc273a92d1 sipa: utACK 13eb8aa572644a53ae0d631916cb4cbc273a92d1 instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/28354/commits/13eb8aa572644a53ae0d631916cb4cbc273a92d1 theStack: Code-review ACK 13eb8aa572644a53ae0d631916cb4cbc273a92d1 Tree-SHA512: eff7a3f9fc9b94003a730beb96e6f3399bc8b8e93fde4b15f20a11eda61d9a3e076f4423989f98b794b32681abecbc3756a54cd0d37b136e2fb2ffbb47ee7774
2023-08-28Support for serialization parametersMarcoFalke
The moved part can be reviewed with the git options --ignore-all-space --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space (Modified by Marco Falke) Co-authored-by: Pieter Wuille <pieter@wuille.net>
2023-08-28Rename CSerAction* to Action*MarcoFalke
This allows new code, added in the next commit, to conform to the coding guideline: No C-prefix for class names.
2023-08-28Replace READWRITEAS macro with AsBase wrapping functionMarcoFalke
Co-authored-by: Pieter Wuille <pieter@wuille.net>
2023-08-28config: default acceptnonstdtxn=0 on all chainsAnthony Towns
Previously, the default for acceptnonstdtxn defaulted to 0 on all chains except testnet. Change this to be consistent across all chains, and remove the parameter from chainparams entirely.
2023-08-27log: Print error message when coindb is in inconsistent stateFabian Jahr
2023-08-25refactor: Use HashWriter over legacy CHashWriter (via SerializeHash)MarcoFalke
2023-08-25refactor: Use HashWriter over legacy CHashWriterMarcoFalke
2023-08-25wallet: accurately account for the size of the witness stackAntoine Poinsot
When estimating the maximum size of an input, we were assuming the number of elements on the witness stack could be encode in a single byte. This is a valid approximation for all the descriptors we support (including P2WSH Miniscript ones), but may not hold anymore once we support Miniscript within Taproot descriptors (since the max standard witness stack size of 100 gets lifted). It's a low-hanging fruit to account for it correctly, so just do it now.
2023-08-25wallet: use descriptor satisfaction size to estimate inputs sizeAntoine Poinsot
Instead of using the dummysigner to compute a placeholder satisfaction, infer a descriptor on the scriptPubKey of the coin being spent and use the estimation of the satisfaction size given by the descriptor directly. Note this (almost, see next paragraph) exactly conserves the previous behaviour. For instance CalculateMaximumSignedInputSize was previously assuming the input to be spent in a transaction that spends at least one Segwit coin, since it was always accounting for the serialization of the number of witness elements. In this commit we use a placeholder for the size of the serialization of the witness stack size (1 byte). Since the logic in this commit is already tricky enough to review, and that it is only a very tiny approximation not observable through the existing tests, it is addressed in the next commit.
2023-08-25script/signingprovider: introduce a MultiSigningProviderAntoine Poinsot
It is sometimes useful to interface with multiple signing providers at once. For instance when inferring a descriptor with solving information being provided from multiple sources (see next commit). Instead of inneficiently copying the information from one provider into the other, introduce a new signing provider that takes a list of pointers to existing providers.
2023-08-25descriptor: introduce a method to get the satisfaction sizeAntoine Poinsot
In the wallet code, we are currently estimating the size of a signed input by doing a dry run of the signing logic. This is unnecessary as all outputs we are able to sign for can be represented by a descriptor, and we can derive the size of a satisfaction ("signature") from the descriptor itself directly. In addition, this approach does not scale: getting the size of a satisfaction through a dry run of the signing logic is only possible for the most basic scripts. This commit introduces the computation of the size of satisfaction per descriptor. It's a bit intricate for 2 main reasons: - We want to conserve the behaviour of the current dry-run logic used by the wallet that sometimes assumes ECDSA signatures will be low-r, sometimes not (when we don't create them). - We need to account for the witness discount. A single descriptor may sometimes benefit of it, sometimes not (for instance `pk()` if used as top-level versus if used inside `wsh()`).
2023-08-25miniscript: introduce a helper to get the maximum witness sizeAntoine Poinsot
Similarly to how we compute the maximum stack size. Also note how it would be quite expensive to recompute it recursively by accounting for different ECDSA signature sizes. So we just assume high-R everywhere. It's only a trivial difference anyways.
2023-08-25miniscript: make GetStackSize independent of P2WSH contextAntoine Poinsot
It was taking into account the P2WSH script push in the number of stack elements.
2023-08-24Merge bitcoin/bitcoin#28287: rpc, test: add `sendmsgtopeer` rpc and a test ↵Andrew Chow
for net-level deadlock situation b3a93b409e7fb33af77bd34a269a3eae71d3ba83 test: add functional test for deadlock situation (Martin Zumsande) 3557aa4d0ab59c18807661a49070b0e5cbfecde3 test: add basic tests for sendmsgtopeer to rpc_net.py (Martin Zumsande) a9a1d69391596f57851f545fae12f8851149d2c3 rpc: add test-only sendmsgtopeer rpc (Martin Zumsande) Pull request description: This adds a `sendmsgtopeer` rpc (for testing only) that allows a node to send a message (provided in hex) to a peer. While we would usually use a `p2p` object instead of a node for this in the test framework, that isn't possible in situations where this message needs to trigger an actual interaction of multiple nodes. Use this rpc to add test coverage for the bug fixed in #27981 (that just got merged): The test lets two nodes (almost) simultaneously send a single large (4MB) p2p message to each other, which would have caused a deadlock previously (making this test fail), but succeeds now. As can be seen from the discussion in #27981, it was not easy to reproduce this bug without `sendmsgtopeer`. I would imagine that `sendmsgtopeer` could also be helpful in various other test constellations. ACKs for top commit: ajtowns: ACK b3a93b409e7fb33af77bd34a269a3eae71d3ba83 sipa: ACK b3a93b409e7fb33af77bd34a269a3eae71d3ba83 achow101: ACK b3a93b409e7fb33af77bd34a269a3eae71d3ba83 Tree-SHA512: 6e22e72402f3c4dd70cddb9e96ea988444720f7a164031df159fbdd48056c8ac77ac53def045d9208a3ca07437c7c8e34f8b4ebc7066c0a84d81cd53f2f4fa5f
2023-08-24Merge bitcoin/bitcoin#27480: doc: Improve documentation of rpcallowipAndrew Chow
c8e066461b54d745b85411035fcc00a1a4044d76 doc: Improve documentation of rpcallowip rpchelp (willcl-ark) Pull request description: Closes #21070 v21.0 introduced a behaviour changed noted in #21070 where using a config value `rpcallowip=::0` no longer also permitted ipv4 ip addresses. The rpc_bind.py functional test covers this new behaviour already by checking that the list of bind addresses exactly matches what is expected so this commit only updates the documentation. ACKs for top commit: achow101: ACK c8e066461b54d745b85411035fcc00a1a4044d76 pinheadmz: ACK c8e066461b54d745b85411035fcc00a1a4044d76 jonatack: ACK c8e066461b54d745b85411035fcc00a1a4044d76 Tree-SHA512: 332060cf0df0427c6637a9fd1e0783ce0b0940abdb41b0df13f03bfbdc28af067cec8f0b1bbc4e47b3d54fa1b2f110418442b05b39d5e7c7e0b96744ddd7c003
2023-08-24Merge bitcoin/bitcoin#27585: fuzz: improve `coinselection`Andrew Chow
bf26f978ffbe7e2fc681825de631600e24e5c93e fuzz: coinselection, fix `m_cost_of_change` (brunoerg) 6d9b26d56ab5295dfcfe0f80a3069046a263fb2f fuzz: coinselection, BnB should never produce change (brunoerg) b2eb55840778515d61465acc8106b27e16af1c88 fuzz: coinselection, compare `GetSelectedValue` with target (brunoerg) 0df0438c60e27df1aced6d31a192d6f334cef2d1 fuzz: coinselection, improve `ComputeAndSetWaste` (brunoerg) 1e351e5db1ced6a32681ceea8a111148bd83e323 fuzz: coinselection, add coverage for `Merge` (brunoerg) f0244a8614ee35caef03bc326519823972ec61b4 fuzz: coinselection, add coverage for `GetShuffledInputVector`/`GetInputSet` (brunoerg) 808618b8a25b1d9cfc4e4f1a5b4c6fff02972396 fuzz: coinselection, add coverage for `AddInputs` (brunoerg) 90c4e6a241eee605809ab1b4331e620b92f05933 fuzz: coinselection, add coverage for `EligibleForSpending` (brunoerg) 2a031cb2c218e288a9784d677705a7d2bc1c2d2b fuzz: coinselection, add `CreateCoins` (brunoerg) Pull request description: This PR: - Moves coin creation to its own function called `CreateCoins`. - Add coverage for `EligibleForSpending` - Add coverage for `AddInputs`: get result of each algorithm (srd, knapsack and bnb), call `CreateCoins` and add into them. - Add coverage for `GetShuffledInputVector` and `GetInputSet` using the result of each algorithm (srd, knapsack and bnb). - Add coverage for `Merge`: Call SRD with the new utxos and, if successful, try to merge with the previous SRD result. ACKs for top commit: murchandamus: reACK with some minimal fuzzing bf26f978ffbe7e2fc681825de631600e24e5c93e achow101: ACK bf26f978ffbe7e2fc681825de631600e24e5c93e furszy: re-ACK bf26f97 Tree-SHA512: bdd2b0a39de37be0a9b21a7c51260b6b8abe538cc0ea74312eb658b90a121a1ae07306c09fb0e75e93b531ce9ea2402feb041b0d852902d07739257f792e64ab
2023-08-24Merge bitcoin/bitcoin#28165: net: transport abstractionfanquake
8a3b6f33873a1075f932f5d9feb6d82e50d83c0c refactor: make Transport::ReceivedBytes just return success/fail (Pieter Wuille) bb4aab90fd046f2fff61e082a0c0d01c5ee31297 net: move message conversion to wire bytes from PushMessage to SocketSendData (Pieter Wuille) a1a1060fd608a11dc525f76f2f54ab5b177dbd05 net: measure send buffer fullness based on memory usage (Pieter Wuille) 009ff8d65058430d614c9a0e0e6ae931b7255c37 fuzz: add bidirectional fragmented transport test (Pieter Wuille) fb2c5edb79656a0b3b04ded6419928102ad696d6 net: make V1Transport implicitly use current chainparams (Pieter Wuille) 0de48fe858a1ffcced340eef2c849165216141c8 net: abstract sending side of transport serialization further (Pieter Wuille) 649a83c7f73db2ee115f5dce3df16622e318aeba refactor: rename Transport class receive functions (Pieter Wuille) 27f9ba23efe82531a465c5e63bf7dc62b6a3a8db net: add V1Transport lock protecting receive state (Pieter Wuille) 93594e42c3f92d82427d2b284ff0f94cdbebe99c refactor: merge transport serializer and deserializer into Transport class (Pieter Wuille) Pull request description: This PR furthers the P2P message serialization/deserialization abstraction introduced in #16202 and #16562, in preparation for introducing the BIP324 v2 transport (making this part of #27634). However, nothing in this PR is BIP324-specific, and it contains a number of independently useful improvements. The overall idea is to have a single object in every `CNode` (called `m_transport`) that is responsible for converting sent messages to wire bytes, and for converting received wire bytes back to messages, while having as little as possible knowledge about this conversion process in higher-level net code. To accomplish that, there is an abstract `Transport` class with (currently) a single `V1Transport` implementation. Structurally, the above is accomplished by: * Merging the `TransportDeserializer` and `TransportSerializer` classes into a single `Transport` class, which encompasses both the sending and receiving side. For `V1Transport` these two sides are entirely separate, but this assumption doesn't hold for the BIP324 transport where e.g. the sending encryption key depends on the DH key negotiation data received from the other side. Merging the two means a future `V2Transport` can handle all this interaction without callers needing to be aware. * Removing the assumption that each message is sent using a computed header followed by (unmodified) data bytes. To achieve that, the sending side of `Transport` mirrors what the receiver side does: callers can set a message to be sent, then ask what bytes must be sent out, and then allowing them to transition to the next message. * Adding internal locks to protect the sending and receiving state of the `V1Transport` implementation. I believe these aren't strictly needed (opinions welcome) as there is no real way to use `Transport` objects in a multi-threaded fashion without some form of external synchronization (e.g. "get next bytes to send" isn't meaningful to call from multiple threads at the same time without mechanism to control the order they'll actually get sent). Still, I feel it's cleaner to make the object responsible for its own consistency (as we definitely do not want the entire object to be under a single external GUARDED_BY, as that'd prevent simultaneous sending and receiving). * Moving the conversion of messages to bytes on the sending side from `PushMessage` to `SocketSendData`, which is needed to deal with the fact that a transport may not immediately be able to send messages. This PR is not a refactor, though some commits are. Among the semantic changes are: * Changing the send buffer pushback mechanism to trigger based on the memory usage of the buffer rather than the amount of bytes to be sent. This is both closer to the desired behavior, and makes the buffering independent from transport details (which is why it's included here). * When optimistic send is not applicable, the V1 message checksum calculation now runs in the net thread rather than the message handling thread. I believe that's generally an improvement, as the message handling thread is far more computationally bottlenecked already. * The checksum calculation now runs under the `CNode::cs_vSend` lock, which does mean no two checksum calculations for messages sent to the same node can run in parallel, even if running in separate threads. Despite that limitation, having the checksum for non-optimistic sends moved in the net thread is still an improvement, I believe. * Statistics for per-message-type sent bytes are now updated when the bytes are actually handed to the OS rather than in `PushMessage`. This is because the actual serialized sizes aren't known until they've gone through the transport object. A fuzz test of the entire `V1Transport` is included. More elaborate rationale for each of the changes can be found in the commit messages. ACKs for top commit: theStack: re-ACK 8a3b6f33873a1075f932f5d9feb6d82e50d83c0c vasild: ACK 8a3b6f33873a1075f932f5d9feb6d82e50d83c0c dergoegge: Code review ACK 8a3b6f33873a1075f932f5d9feb6d82e50d83c0c Tree-SHA512: 26e9a6df47f1dd3e3f3edb4874edf365728e5a8bbc9d0d4d71fb6000cb2dfde5574902c47ffcf825af6743922f2ff9d31a5a38942a196f4ca6669122e15e42e4
2023-08-24rpc: Add MaybeArg() and Arg() default helperMarcoFalke
2023-08-23refactor: make Transport::ReceivedBytes just return success/failPieter Wuille
2023-08-23net: move message conversion to wire bytes from PushMessage to SocketSendDataPieter Wuille
This furthers transport abstraction by removing the assumption that a message can always immediately be converted to wire bytes. This assumption does not hold for the v2 transport proposed by BIP324, as no messages can be sent before the handshake completes. This is done by only keeping (complete) CSerializedNetMsg objects in vSendMsg, rather than the resulting bytes (for header and payload) that need to be sent. In SocketSendData, these objects are handed to the transport as permitted by it, and sending out the bytes the transport tells us to send. This also removes the nSendOffset member variable in CNode, as keeping track of how much has been sent is now a responsability of the transport. This is not a pure refactor, and has the following effects even for the current v1 transport: * Checksum calculation now happens in SocketSendData rather than PushMessage. For non-optimistic-send messages, that means this computation now happens in the network thread rather than the message handler thread (generally a good thing, as the message handler thread is more of a computational bottleneck). * Checksum calculation now happens while holding the cs_vSend lock. This is technically unnecessary for the v1 transport, as messages are encoded independent from one another, but is untenable for the v2 transport anyway. * Statistics updates about per-message sent bytes now happen when those bytes are actually handed to the OS, rather than at PushMessage time.
2023-08-23net: measure send buffer fullness based on memory usagePieter Wuille
This more accurately captures the intent of limiting send buffer size, as many small messages can have a larger overhead that is not counted with the current approach. It also means removing the dependency on the header size (which will become a function of the transport choice) from the send buffer calculations.
2023-08-23fuzz: add bidirectional fragmented transport testPieter Wuille
This adds a simulation test, with two V1Transport objects, which send messages to each other, with sending and receiving fragmented into multiple pieces that may be interleaved. It primarily verifies that the sending and receiving side are compatible with each other, plus a few sanity checks.
2023-08-23net: make V1Transport implicitly use current chainparamsPieter Wuille
The rest of net.cpp already uses Params() to determine chainparams in many places (and even V1Transport itself does so in some places). Since the only chainparams dependency is through the message start characters, just store those directly in the transport.
2023-08-23net: abstract sending side of transport serialization furtherPieter Wuille
This makes the sending side of P2P transports mirror the receiver side: caller provides message (consisting of type and payload) to be sent, and then asks what bytes must be sent. Once the message has been fully sent, a new message can be provided. This removes the assumption that P2P serialization of messages follows a strict structure of header (a function of type and payload), followed by (unmodified) payload, and instead lets transports decide the structure themselves. It also removes the assumption that a message must always be sent at once, or that no bytes are even sent on the wire when there is no message. This opens the door for supporting traffic shaping mechanisms in the future.