aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2024-06-13refactor: remove warnings globalsstickies-v
2024-06-13introduce and use the generalized `node::Warnings` interfacestickies-v
Instead of having separate warning functions (and globals) for each different warning that can be raised, encapsulate this logic into a single class and allow to (un)set any number of warnings. Introduces behaviour change: - the `-alertnotify` command is executed for all `KernelNotifications::warningSet` calls, which now also covers the `LARGE_WORK_INVALID_CHAIN` warning. - previously, warnings were returned based on a predetermined order, e.g. with the "pre-release test build" warning always first. This is no longer the case, and Warnings::GetMessages() will return messages sorted by the id of the warning. Removes warnings.cpp from kernel.
2024-06-12Merge bitcoin/bitcoin#29015: kernel: Streamline util libraryAva Chow
c7376babd19d0c858fef93ebd58338abd530c1f4 doc: Clarify distinction between util and common libraries in libraries.md (Ryan Ofsky) 4f74c59334d496f28e1a5c0d84c412f9020b366f util: Move util/string.h functions to util namespace (Ryan Ofsky) 4d05d3f3b42a41525aa6ec44b90f543dfab53ecf util: add TransactionError includes and namespace declarations (Ryan Ofsky) 680eafdc74021c1e0893c3a62404e607fd4724f5 util: move fees.h and error.h to common/messages.h (Ryan Ofsky) 02e62c6c9af4beabaeea58fb1ea3ad0dc5094678 common: Add PSBTError enum (Ryan Ofsky) 0d44c44ae33434f366229c612d6edeedf7658963 util: move error.h TransactionError enum to node/types.h (Ryan Ofsky) 9bcce2608dd2515dc35a0f0866abc9d43903c795 util: move spanparsing.h to script/parsing.h (Ryan Ofsky) 6dd2ad47922694d2ab84bad4dac9dd442c5df617 util: move spanparsing.h Split functions to string.h (Ryan Ofsky) 23cc8ddff472d259605d7790ba98a1900e77efab util: move HexStr and HexDigit from util to crypto (TheCharlatan) 6861f954f8ff42c87ad638037adae86a5bd89600 util: move util/message to common/signmessage (Ryan Ofsky) cc5f29fbea15d33e4d1aa95591253c6b86953fe7 build: move memory_cleanse from util to crypto (Ryan Ofsky) 5b9309420cc9721a0d5745b6ad3166a4bdbd1508 build: move chainparamsbase from util to common (Ryan Ofsky) ffa27af24da81a97d6c4912ae0e10bc5b6f17f69 test: Add check-deps.sh script to check for unexpected library dependencies (Ryan Ofsky) Pull request description: Remove `fees.h`, `errors.h`, and `spanparsing.h` from the util library. Specifically: - Move `Split` functions from `util/spanparsing.h` to `util/string.h`, using `util` namespace for clarity. - Move remaining spanparsing functions to `script/parsing.h` since they are used for descriptor and miniscript parsing. - Combine `util/fees.h` and `util/errors.h` into `common/messages.h` so there is a place for simple functions that generate user messages to live, and these functions are not part of the util library. Motivation for this change is that the util library is a dependency of the kernel, and we should remove functionality from util that shouldn't be called by kernel code or kernel applications. These changes should also improve code organization and make functions easier to discover. Some of these same moves are (or were) part of #28690, but did not help with code organization, or made it worse, so it is better to move them and clean them up in the same PR so code only has to change one time. ACKs for top commit: achow101: ACK c7376babd19d0c858fef93ebd58338abd530c1f4 TheCharlatan: Re-ACK c7376babd19d0c858fef93ebd58338abd530c1f4 hebasto: re-ACK c7376babd19d0c858fef93ebd58338abd530c1f4. Tree-SHA512: 5bcef16c1255463b1b69270548711e7ff78ca0dd34e300b95e3ca1ce52ceb34f83d9ddb2839e83800ba36b200de30396e504bbb04fa02c6d0c24a16d06ae523d
2024-06-12Merge bitcoin/bitcoin#30229: fuzz: Use std::span in FuzzBufferTypemerge-script
faa41e29d5b90e62179d651f4010272dae685621 fuzz: Use std::span in FuzzBufferType (MarcoFalke) Pull request description: The use of `Span` is problematic, because it lacks methods such as `rbegin`, leading to compile failures when used: ``` error: no member named 'rbegin' in 'Span<const unsigned char>' ``` One could fix `Span`, but it seems better to use `std::span`, given that `Span` will be removed anyway in the long term. ACKs for top commit: dergoegge: utACK faa41e29d5b90e62179d651f4010272dae685621 Tree-SHA512: 54bcaf51c83a1b48739cd7f1e8445c6eba0eb04231bce5c35591a47dddb3890ffcaf562cf932930443c80ab0e66950c4619560e6692240de0c52aeef3214facd
2024-06-12Merge bitcoin/bitcoin#30230: fuzz: add I2P harnessmerge-script
193c748e44f8647a056121fc9cbb9c2efbcbfc49 fuzz: add I2P harness (marcofleon) Pull request description: Addresses https://github.com/bitcoin/bitcoin/issues/28803. This updated harness sets mock time at the beginning of each iteration and deletes the private key file at the end of each iteration. Mock time is used to make the fuzz test more stable, as `GetTime` is called at points in `i2p`. Deleting the private key file ensures that each iteration is independent from the last. Now, a new key is generated in `i2p` every time, so the fuzzer can eventually make progress through the target code. Re-working this harness also led me and dergoegge to resolve a couple of issues in `FuzzedSock`, which allows for full coverage of the `i2p` code. Those changes can be seen in https://github.com/bitcoin/bitcoin/pull/30211. The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness. <details> <summary>I2P dict</summary> ``` "HELLO VERSION" "HELLO REPLY RESULT=OK VERSION=" "HELLO REPLY RESULT=NOVERSION" "HELLO REPLY RESULT=I2P_ERROR" "SESSION CREATE" "SESSION STATUS RESULT=OK DESTINATION=" "SESSION STATUS RESULT=DUPLICATED_ID" "SESSION STATUS RESULT=DUPLICATED_DEST" "SESSION STATUS RESULT=INVALID_ID" "SESSION STATUS RESULT=INVALID_KEY" "SESSION STATUS RESULT=I2P_ERROR MESSAGE=" "SESSION ADD" "SESSION REMOVE" "STREAM CONNECT" "STREAM STATUS RESULT=OK" "STREAM STATUS RESULT=INVALID_ID" "STREAM STATUS RESULT=INVALID_KEY" "STREAM STATUS RESULT=CANT_REACH_PEER" "STREAM STATUS RESULT=I2P_ERROR MESSAGE=" "STREAM ACCEPT" "STREAM FORWARD" "DATAGRAM SEND" "RAW SEND" "DEST GENERATE" "DEST REPLY PUB= PRIV=" "DEST REPLY RESULT=I2P_ERROR" "NAMING LOOKUP" "NAMING REPLY RESULT=OK NAME= VALUE=" "DATAGRAM RECEIVED DESTINATION= SIZE=" "RAW RECEIVED SIZE=" "NAMING REPLY RESULT=INVALID_KEY NAME=" "NAMING REPLY RESULT=KEY_NOT_FOUND NAME=" "MIN" "MAX" "STYLE" "ID" "SILENT" "DESTINATION" "NAME" "SIGNATURE_TYPE" "CRYPTO_TYPE" "SIZE" "HOST" "PORT" "FROM_PORT" "TRANSIENT" "STREAM" "DATAGRAM" "RAW" "MASTER" "true" "false" ``` </details> I'll add this dict to qa-assets later on. ACKs for top commit: dergoegge: tACK 193c748e44f8647a056121fc9cbb9c2efbcbfc49 brunoerg: ACK 193c748e44f8647a056121fc9cbb9c2efbcbfc49 vasild: ACK 193c748e44f8647a056121fc9cbb9c2efbcbfc49 Tree-SHA512: 09ae4b3fa0738aa6f159f4d920493bdbce786b489bc8148e7a135a881e9dba93d727b40f5400c9510e218dd2cfdccc7ce2d3ac9450654fb29c78aac59af92ec3
2024-06-12fuzz: Use std::span in FuzzBufferTypeMarcoFalke
2024-06-12Merge bitcoin/bitcoin#29325: consensus: Store transaction nVersion as uint32_tmerge-script
429ec1aaaaafab150f11e27fcf132a99b57c4fc7 refactor: Rename CTransaction::nVersion to version (Ava Chow) 27e70f1f5be1f536f2314cd2ea42b4f80d927fbd consensus: Store transaction nVersion as uint32_t (Ava Chow) Pull request description: Given that the use of a transaction's nVersion is always as an unsigned int, it doesn't make sense to store it as signed and then cast it to unsigned everywhere it is used and displayed. Since a few alternative implementations have recently been revealed to have made an error with this signedness that would have resulted in consensus failure, I think it makes sense for us to just make this always unsigned to make it clear that the version is treated as unsigned. This would also help us avoid future potential issues with signedness of this value. I believe that this is safe and does not actually change what transactions would or would not be considered both standard and consensus valid. Within consensus, the only use of the version in consensus is in BIP68 validation which was already casting it to uint32_t. Within policy, although it is used as a signed int for the transaction version number check, I do not think that this change would change standardness. Standard transactions are limited to the range [1, 2]. Negative numbers would have fallen under the < 1 condition, but by making it unsigned, they are still non-standard under the > 2 condition. Unsigned and signed ints are serialized and unserialized the same way so there is no change in serialization. ACKs for top commit: maflcko: ACK 429ec1aaaaafab150f11e27fcf132a99b57c4fc7 🐿 glozow: ACK 429ec1aaaa shaavan: ACK 429ec1aaaaafab150f11e27fcf132a99b57c4fc7 💯 Tree-SHA512: 0bcd92a245d7d16c3665d2d4e815a4ef28207ad4a1fb46c6f0203cdafeab1b82c4e95e4bdce7805d80a4f4a46074f6542abad708e970550d38a00d759e3dcef1
2024-06-11Merge bitcoin/bitcoin#30160: util: add BitSetAva Chow
47f705b33fc1381d96c99038e2110e6fe2b2f883 tests: add fuzz tests for BitSet (Pieter Wuille) 59a6df6bd584701f820ad60a10d9d477bf0236b5 util: add BitSet (Pieter Wuille) Pull request description: Extracted from #30126. This introduces the `BitSet` data structure, inspired by `std::bitset`, but with a few features that cannot be implemented on top without efficiency loss: * Finding the first set bit (`First`) * Finding the last set bit (`Last`) * Iterating over all set bits (`begin` and `end`). And a few other operators/member functions that help readability for #30126: * `operator-` for set subtraction * `Overlaps()` for testing whether intersection is non-empty * `IsSupersetOf()` for testing (non-strict) supersetness * `IsSubsetOf()` for testing (non-strict) subsetness * `Fill()` to construct a set with all numbers from 0 to n-1, inclusive * `Singleton()` to construct a set with one specific element. Everything is tested through a simulation-based fuzz test that compares the behavior with normal `std::bitset` equivalent operations. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/30160/commits/47f705b33fc1381d96c99038e2110e6fe2b2f883 achow101: ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883 cbergqvist: re-ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883 theStack: Code-review ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883 Tree-SHA512: e451bf4b801f193239ee434b6b614f5a2ac7bb49c70af5aba24c2ac0c54acbef4672556800e4ac799ae835632bdba716209c5ca8c37433a6883dab4eb7cd67c1
2024-06-11Merge bitcoin/bitcoin#28339: validation: improve performance of CheckBlockIndexAva Chow
5bc2077e8f592442b089affdf0b5795fbc053bb8 validation: allow to specify frequency for -checkblockindex (Martin Zumsande) d5a631b9597e5029a5048d9b8ad84ea4536bbac0 validation: improve performance of CheckBlockIndex (Martin Zumsande) 32c80413fdb063199f3bee719c4651bd63f05fce bench: add benchmark for checkblockindex (Martin Zumsande) Pull request description: `CheckBlockIndex() ` are consistency checks that are currently enabled by default on regtest. The function is rather slow, which is annoying if you * attempt to run it on other networks, especially if not fully synced * want to generate a long chain on regtest and see block generation slow down because you forgot to disable `-checkblockindex` or don't know it existed. One reason why it's slow is that in order to be able to traverse the block tree depth-first from genesis, it inserts pointers to all block indices into a `std::multimap` - for which inserts and lookups become slow once there are hundred thousands of entries. However, typically the block index is mostly chain-like with just a few forks so a multimap isn't really needed for the most part. This PR suggests to store the block indices of the chain ending in the best header in a vector instead, and store only the rest of the indices in a multimap. This does not change the actual consistency checks that are being performed for each index, just the way the block index tree is stored and traversed. This adds a bit of complication to make sure each block is visited (note that there are asserts that check it), making sure that the two containers are traversed correctly, but it speeds up the function considerably: On master, a single invocation of `CheckBlockIndex` takes ~1.4s on mainnet for me (4.9s on testnet which has >2.4 million blocks). With this branch, the runtime goes down to ~0.27s (0.85s on testnet).This is a speedup by a factor ~5. ACKs for top commit: achow101: ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8 furszy: ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8 ryanofsky: Code review ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8. Just added suggested assert and simplification since last review Tree-SHA512: 6b9c3e3e5069d6152b45a09040f962380d114851ff0f9ff1771cf8cad7bb4fa0ba25cd787ceaa3dfa5241fb249748e2ee6987af0ccb24b786a5301b2836f8487
2024-06-11Merge bitcoin/bitcoin#28830: [refactor] Check CTxMemPool options in ctorAva Chow
09ef322acc0a88a9e119f74923399598984c68f6 [[refactor]] Check CTxMemPool options in constructor (TheCharlatan) Pull request description: The tests should run the same checks on the mempool options that the init code also applies. The downside to this patch is that the log line may now be printed more than once in the for loop. This was originally noticed here https://github.com/bitcoin/bitcoin/pull/25290#discussion_r900272797. ACKs for top commit: stickies-v: re-ACK 09ef322acc0a88a9e119f74923399598984c68f6 . Fixed unreachable assert and updated docstring, and also added an exception for "-maxmempool must be at least " in the `tx_pool` fuzz test, which makes sense when looking at how the mempool options are constructed in `SetMempoolConstraints`. achow101: ACK 09ef322acc0a88a9e119f74923399598984c68f6 ryanofsky: Code review ACK 09ef322acc0a88a9e119f74923399598984c68f6. Just fuzz test error checking fix and updated comment since last review Tree-SHA512: eb3361411c2db70be17f912e3b14d9cb9c60fb0697a1eded952c3b7e8675b7d783780d45c52e091931d1d80fe0f0280cee98dd57a3100def13af20259d9d1b9e
2024-06-11Merge bitcoin/bitcoin#30254: test: doc: fix units in tx-size standardness ↵glozow
test (s/vbytes/weight units) d1581c6048478cf70c5fb9ec5ebc178f16b376b8 test: doc: fix units in tx size standardness test (s/vbytes/weight units) (Sebastian Falbesoner) Pull request description: This small fixup PR is a late follow-up for #17947 (commit 4537ba5f21ad8afb705325cd8e15dd43877eb28f), where the wrong units has been used in the comments for the large tx composition. ACKs for top commit: tdb3: ACK d1581c6048478cf70c5fb9ec5ebc178f16b376b8 ismaelsadeeq: ACK d1581c6048478cf70c5fb9ec5ebc178f16b376b8 glozow: ACK d1581c6048478cf70c5fb9ec5ebc178f16b376b8 Tree-SHA512: ea2de42174f9dca0608275ea377c852ebddc5a04a2b32248ce808aea33d7e00cdee3a225b24c0cf426c69646cccbbc31273c62f7bc1647bb3443a61de3b15670
2024-06-10Merge bitcoin/bitcoin#30132: indexes: Don't wipe indexes again when ↵Ryan Ofsky
continuing a prior reindex f68cba29b3be0dec7877022b18a193a3b78c1099 blockman: Replace m_reindexing with m_blockfiles_indexed (Ryan Ofsky) 1b1c6dcca0cc891bd35d29b61628c39098cd94ce test: Add functional test for continuing a reindex (TheCharlatan) 201c1a92824c71ae646d5bba9963871b1d704cc1 indexes: Don't wipe indexes again when already reindexing (TheCharlatan) 804f09dfa116300914e2aeef05ed9710dd504e8c kernel: Add less confusing reindex options (Ryan Ofsky) e17255322378076edce3ef6f06cd36ca58d2e236 validation: Remove needs_init from LoadBlockIndex (TheCharlatan) 533eab7d67d78f217f74909662133086b79ea808 bugfix: Streamline setting reindex option (TheCharlatan) Pull request description: When restarting `bitcoind` during an ongoing reindex without setting the `-reindex` flag again, the block and coins db is left intact, but any data from the optional indexes is discarded. While not a bug per se, wiping the data again is wasteful, both in terms of having to write it again, as well as potentially leading to longer startup times. So keep the index data instead when continuing a prior reindex. Also includes a bugfix and smaller code cleanups around the reindexing code. The bug was introduced in b47bd959207e82555f07e028cc2246943d32d4c3: "kernel: De-globalize fReindex". ACKs for top commit: stickies-v: ACK f68cba29b3be0dec7877022b18a193a3b78c1099 fjahr: Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099 furszy: Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099 ryanofsky: Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code. Tree-SHA512: b252228cc76e9f1eaac56d5bd9e4eac23408e0fc04aeffd97a85417f046229364673ee1ca7410b9b6e7b692b03f13ece17c42a10176da0d7e975a8915deb98ca
2024-06-10tests: add fuzz tests for BitSetPieter Wuille
2024-06-10Merge bitcoin/bitcoin#30235: build: warn on self-assignmentmerge-script
15796d4b61342f75548b20a18c670ed21d102ba8 build: warn on self-assignment (Cory Fields) 53372f21767be449bb452fc3f5fe7f16286ae371 refactor: disable self-assign warning for tests (Cory Fields) Pull request description: Belt-and suspenders after #30234. Self-assignment should be safe _and_ discouraged. We used to opt out of this warning because something deep in our serialization/byteswapping code could self-assign, but that doesn't appear to be the case anymore. ACKs for top commit: maflcko: ACK 15796d4b61342f75548b20a18c670ed21d102ba8 fanquake: ACK 15796d4b61342f75548b20a18c670ed21d102ba8 - not a huge fan of inline pragma usage, but this seems fine, given it's to work around an already-fixed compiler bug, and we'll only be carrying it for a shortish time in any case. Tree-SHA512: 1f95f7c730b974ad1da55ebd381040bac312f2f380fff9d569ebab91d7c1963592a84d1613d81d96238c6f5a66aa40deebba68a76f6b24b02150d0a77c769654
2024-06-09test: doc: fix units in tx size standardness test (s/vbytes/weight units)Sebastian Falbesoner
2024-06-08Merge bitcoin/bitcoin#30238: json-rpc 2.0 followups: docs, tests, climerge-script
1f6ab1215bbb1f8a5f1743c3c413b95ad08090df minor: remove unnecessary semicolons from RPC content type examples (Matthew Zipkin) b22529529823c0cb5916ac318c8536e9107b7e78 test: use json-rpc 2.0 in all functional tests by default (Matthew Zipkin) 391843b0297db03d71a8d88ab77609e2ad230bf2 bitcoin-cli: use json-rpc 2.0 (Matthew Zipkin) d39bdf339772166a5545ae811e58b7764af093a8 test: remove unused variable in interface_rpc.py (Matthew Zipkin) 0ead71df8c83a2f9eae1220544ec84dcf38a0326 doc: update and link for JSON-RPC 2.0 (Matthew Zipkin) Pull request description: This is a follow-up to #27101. - Addresses [post-merge comments ](https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1606723428) - bitcoin-cli uses JSON-RPC 2.0 - functional tests use JSON-RPC 2.0 by default (exceptions are in the regression tests added by #27101) ACKs for top commit: tdb3: ACK 1f6ab1215bbb1f8a5f1743c3c413b95ad08090df cbergqvist: ACK 1f6ab1215bbb1f8a5f1743c3c413b95ad08090df Tree-SHA512: 49bf14c70464081280216ece538a2f5ec810bac80a86a83ad3284f0f1b017edf755a1a74a45be279effe00218170cafde7c2de58aed07097a95c2c6b837a6b6c
2024-06-07refactor: Rename CTransaction::nVersion to versionAva Chow
In order to ensure that the change of nVersion to a uint32_t in the previous commit has no effect, rename nVersion to version in this commit so that reviewers can easily spot if a spot was missed or if there is a check somewhere whose semantics have changed.
2024-06-07kernel: Add less confusing reindex optionsRyan Ofsky
Drop confusing kernel options: BlockManagerOpts::reindex ChainstateLoadOptions::reindex ChainstateLoadOptions::reindex_chainstate Replacing them with more straightforward options: ChainstateLoadOptions::wipe_block_tree_db ChainstateLoadOptions::wipe_chainstate_db Having two options called "reindex" which did slightly different things was needlessly confusing (one option wiped the block tree database, and the other caused block files to be rescanned). Also the previous set of options did not allow rebuilding the block database without also rebuilding the chainstate database, when it should be possible to do those independently.
2024-06-07consensus: Store transaction nVersion as uint32_tAva Chow
Given that the use of a transaction's nVersion is always as an unsigned int, it doesn't make sense to store it as signed and then cast it to unsigned.
2024-06-07Merge bitcoin/bitcoin#29496: policy: bump TX_MAX_STANDARD_VERSION to 3Ava Chow
30a01134cdec37e7467fcd6eee8b0ae3890a131c [doc] update bips.md for 431 (glozow) 9dbe6a03f0d6e70ccdf8e8715f888c0c17216bee [test] wallet uses CURRENT_VERSION which is 2 (glozow) 539404fe0fc0346b3aa77c330b38a5a0ad6565b2 [policy] make v3 transactions standard (glozow) 052ede75aff5c9f3a0a422ef413852eabeecc665 [refactor] use TRUC_VERSION in place of 3 (glozow) Pull request description: Make `nVersion=3` (which is currently nonstandard on mainnet) standard. Note that we will treat these transactions as Topologically Restricted Until Confirmation (TRUC). Spec is in BIP 431 and implementation is in #28948, #29306, and #29873 See #27463 for overall project tracking, and #29319 for information about relevance to cluster mempool. ACKs for top commit: sdaftuar: utACK 30a01134c achow101: ACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c instagibbs: utACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c murchandamus: ACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c ismaelsadeeq: ACK 30a01134cdec37e7467fcd6eee8b0ae3890a131c 🛰️ Tree-SHA512: 2a4aec0442c860e792a061d83e36483c1f1b426f946efbdf664c8db97a596e498b535707e1d3a900218429486ea69fd4552e3d476526a6883cbd5556c6534b48
2024-06-07minor: remove unnecessary semicolons from RPC content type examplesMatthew Zipkin
2024-06-06tests: add fuzz tests for VecDequePieter Wuille
2024-06-06fuzz: add I2P harnessmarcofleon
2024-06-06refactor: disable self-assign warning for testsCory Fields
clang-16 and earlier detect "foo -= foo" and "foo /= foo" as self-assignments.
2024-06-04Merge bitcoin/bitcoin#30218: refactor: remove unused `CKey::Negate` methodAva Chow
8801e319d51209fe3a3b06e2aab5f96ceead290d refactor: remove unused `CKey::Negate` method (Sebastian Falbesoner) Pull request description: This method was introduced as a pre-requirement for the v2 transport protocol back then (see PR #14047, commit 463921bb), when it was still BIP151. With the replacement BIP324, this is not needed anymore, and it's also unlikely that for any other proposal we'd ever need to negate private keys at this abstraction level. I'd argue that this operation is usually something that should happen within a secp256k1 module (like e.g. done in MuSig2, Silent Payments...). (If there is really demand in the future, it's also trivial to reintroduce the method.) ACKs for top commit: laanwj: ACK 8801e319d51209fe3a3b06e2aab5f96ceead290d sipa: ACK 8801e319d51209fe3a3b06e2aab5f96ceead290d achow101: ACK 8801e319d51209fe3a3b06e2aab5f96ceead290d Tree-SHA512: 7bc1566399635c5c6e4940a2724c865d5443eb190024379099330c023c516f1e4f423ed9e8c42bc93413b723a5464ec79d3f879f58c0e598fe24f495238df4ec
2024-06-04Merge bitcoin/bitcoin#29997: rpc: Remove index-based Arg accessorAva Chow
fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54 rpc: Remove index-based Arg accessor (MarcoFalke) Pull request description: The index-based Arg accessor is redundant with the name-based one. It does not provide any benefit to the code reader, or otherwise, so remove it. ACKs for top commit: stickies-v: re-ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54, addressed doc nits achow101: ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54 ryanofsky: Code review ACK fa3169b0732d7eb4b9166e7ecc6b7cfb669a9b54. One changes since last review are some documentation improvements Tree-SHA512: f9da1c049dbf38c3b47a8caf8d24d195c2d4b88c7ec45a9ccfb78f1e39f29cb86869f84b308f6e49856b074c06604ab634c90eb89c9c93d2a8169e070aa1bd40
2024-06-04Merge bitcoin/bitcoin#30211: fuzz: Make FuzzedSock fuzz friendliermerge-script
22d0f1a27ef7733b51b3c2138a8d01713df8f248 [fuzz] Avoid endless waiting in FuzzedSock::{Wait,WaitMany} (marcofleon) a7fceda68bb62fe3d9060fcf52e33b2f64a2acf9 [fuzz] Make peeking through FuzzedSock::Recv fuzzer friendly (dergoegge) 865cdf3692590bc6b1121524fe1bee188788b791 [fuzz] Use fuzzer friendly ConsumeRandomLengthByteVector in FuzzedSock::Recv (dergoegge) Pull request description: `FuzzedSock` has a few issues that block a fuzzer from making progress. See commit messages for details. ACKs for top commit: marcofleon: Tested ACK 22d0f1a27ef7733b51b3c2138a8d01713df8f248 brunoerg: utACK 22d0f1a27ef7733b51b3c2138a8d01713df8f248 Tree-SHA512: 2d66fc94ba58b6652ae234bd1dcd33b7d685b5054fe83e0cd624b053dd51519c23148f43a865ab8c8bc5fc2dc25e701952831b99159687474978a90348faa4c5
2024-06-03refactor: remove unused `CKey::Negate` methodSebastian Falbesoner
This method was introduced as a pre-requirement for the v2 transport protocol back then (see PR #14047, commit 463921bb), when it was still BIP151. With the replacement BIP324, this is not needed anymore, and it's also unlikely that any other proposal would need to negate private keys at this abstraction level. (If there is really demand, it's trivial to reintroduce the method.)
2024-06-03Merge bitcoin/bitcoin#30215: doc: JSON-RPC request Content-Type is ↵merge-script
application/json 3c08e11c3ea4499e8d20609e2417cac859b3e98e doc: JSON-RPC request Content-Type is application/json (Luke Dashjr) Pull request description: Specify json content type in RPC examples. Picks up #29946. Which needed rebasing and the commit message fixing, ACKs for top commit: laanwj: ACK 3c08e11c3ea4499e8d20609e2417cac859b3e98e tdb3: ACK for 3c08e11c3ea4499e8d20609e2417cac859b3e98e Tree-SHA512: 770bbbc0fb324cb63628980b13583cabf02e75079851850170587fb6eca41a70b01dcedaf1926bb6488eb9816a3cc6616fe8cee8c4b7e09aa39b7df5834ca0ec
2024-06-03[fuzz] Avoid endless waiting in FuzzedSock::{Wait,WaitMany}marcofleon
Currently, when the FuzzedDataProvider of a FuzzedSock runs out of data, FuzzedSock::Wait and WaitMany will simulate endless waiting as the requested events are never simulated as occured. Fix this by simulating event occurence when ConsumeBool() returns false (e.g. when the data provider runs out). Co-authored-by: dergoegge <n.goeggi@gmail.com>
2024-06-03[fuzz] Make peeking through FuzzedSock::Recv fuzzer friendlydergoegge
FuzzedSock only supports peeking at one byte at a time, which is not fuzzer friendly when trying to receive long data. Fix this by supporting peek data of arbitrary length instead of only one byte.
2024-06-03Merge bitcoin/bitcoin#30167: doc, rpc: Release notes and follow-ups for #29612merge-script
efc1b5be8a4696c0db19ba18316b2d4ed09e10f2 test: Add coverage for txid coins count check when loading snapshot (Fabian Jahr) 6b6084850b8c2ebcdbeecdb406e8732adaa6d23c assumeutxo: Add network magic ctor param to SnapshotMetadata (Fabian Jahr) 1f1f9984555d49f07ae20cb3a8153a177c546beb assumeutxo: Deserialize trailing byte instead of Txid (Fabian Jahr) 359967e310794e0bbdbe2bca38ee440a88bc4f43 doc: Add release notes for #29612 (Fabian Jahr) Pull request description: This adds release notes for #29612 and addresses post-merge review comments. ACKs for top commit: maflcko: utACK efc1b5be8a4696c0db19ba18316b2d4ed09e10f2 theStack: utACK efc1b5be8a4696c0db19ba18316b2d4ed09e10f2 Tree-SHA512: 3b270202e4f7b2576090ef1d970fd54a6840d96fc3621dddd28e888fb8696a97ff69af2e000bcee3b364316ca3f6e2a9b2f1694c6184f0e704dc487823127ce4
2024-05-31doc: JSON-RPC request Content-Type is application/jsonLuke Dashjr
Specify json content type in RPC examples
2024-05-31[fuzz] Use fuzzer friendly ConsumeRandomLengthByteVector in FuzzedSock::Recvdergoegge
See comment on FuzzedDataProvider::ConsumeRandomLengthString.
2024-05-31[refactor] use TRUC_VERSION in place of 3glozow
2024-05-29Merge bitcoin/bitcoin#30156: fuzz: More accurate coverage reportsmerge-script
949abebea0059edd929b653b4b475a5880fc0a3e [fuzz] Avoid collecting initialization coverage (dergoegge) Pull request description: Our coverage reports include coverage of initialization code, which can be misleading when trying to evaluate the coverage a fuzz harness achieves through fuzzing alone. This PR proposes to make fuzz coverage reports more accurate by resetting coverage counters after initialization code has been run. This makes it easier to evaluate which code was actually reached through fuzzing (e.g. to spot fuzz blockers). ACKs for top commit: maflcko: utACK 949abebea0059edd929b653b4b475a5880fc0a3e brunoerg: nice, utACK 949abebea0059edd929b653b4b475a5880fc0a3e Tree-SHA512: c8579bda4f3d71d199b9331fbe6316fce375a906743d0bc216bb94958dc03fdc9a951ea50cfeb487494a75668ae3c16471a82f7e5fdd912d781dc29d063e2c5b
2024-05-24assumeutxo: Add network magic ctor param to SnapshotMetadataFabian Jahr
This prevents SnapshotMetadata from using any globals implicitly.
2024-05-23Merge bitcoin/bitcoin#29612: rpc: Optimize serialization and enhance ↵Ava Chow
metadata of dumptxoutset output 542e13b2937356810bda2c41be83c3b1675e2f2f rpc: Enhance metadata of the dumptxoutset output (Fabian Jahr) 4d8e5edbaa94805be41ae4c8aa2f4bf7aaa276fe assumeutxo: Add documentation on dumptxoutset serialization format (Fabian Jahr) c14ed7f384075330361df636f40121cf25a066d6 assumeutxo: Add test for changed coin size value (Fabian Jahr) de95953d870c41436de67d56c93259bc66fe1434 rpc: Optimize serialization disk space of dumptxoutset (Fabian Jahr) Pull request description: The second attempt at implementing the `dumptxoutset` space optimization as suggested in #25675. Closes #25675. This builds on the work done in #26045, addresses open feedback, adds some further improvements (most importantly usage of compact size), documentation, and an additional test. The [original snapshot at height 830,000](https://github.com/bitcoin/bitcoin/pull/29551) came in at 10.82 GB. With this change, the same snapshot is 8.94 GB, a reduction of 17.4%. This also enhances the metadata of the output file and adds the following data to allow for better error handling and make future upgrades easier: - A newly introduced utxo set magic - A version number - The network magic - The block height ACKs for top commit: achow101: ACK 542e13b2937356810bda2c41be83c3b1675e2f2f TheCharlatan: Re-ACK 542e13b2937356810bda2c41be83c3b1675e2f2f theStack: ACK 542e13b2937356810bda2c41be83c3b1675e2f2f Tree-SHA512: 0825d30e5c3c364062db3c6cbca4e3c680e6e6d3e259fa70c0c2b2a7020f24a47406a623582040988d5c7745b08649c31110df4c10656aa25f3f27eb35843d99
2024-05-23[fuzz] Avoid collecting initialization coveragedergoegge
2024-05-23Merge bitcoin/bitcoin#29873: policy: restrict all TRUC (v3) transactions to ↵Ava Chow
10kvB 154b2b2296edccb5ed24e829798dacb6195edc11 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits (glozow) a29f1df289cf27c6cbd565448548b3dc1392a9b0 [policy] restrict all v3 transactions to 10kvB (glozow) d578e2e3540e085942001350ff3aeb047bdac973 [policy] explicitly require non-v3 for CPFP carve out (glozow) Pull request description: Opening for discussion / conceptual review. We like the idea of a smaller maximum transaction size because: - It lowers potential replacement cost (i.e. harder to do Rule 3 pinning via gigantic transaction) - They are easier to bin-pack in block template production - They equate to a tighter memory limit in data structures that are bounded by a number of transactions (e.g. orphanage and vExtraTxnForCompact). For example, the current memory bounds for orphanage is 100KvB * 100 = 40MB, and guaranteeing 1 tx per peer would require reserving a pretty large space. History for `MAX_STANDARD_TX_WEIGHT=100KvB` (copied from https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510): - 2010-09-13 In https://github.com/bitcoin/bitcoin/commit/3df62878c3cece15a8921fbbdee7859ee9368768 satoshi added a 100kB (MAX_BLOCK_SIZE_GEN/5 with MBS_GEN = MAX_BLOCK_SIZE/2) limit on new transactions in CreateTransaction() - 2013-02-04 https://github.com/bitcoin/bitcoin/pull/2273 In gavin gave that constant a name, and made it apply to transaction relay as well Lowering `MAX_STANDARD_TX_WEIGHT` for all txns is not being proposed, as there are existing apps/protocols that rely on large transactions. However, it's been brought up that we should consider this for TRUCs (which is especially designed to avoid Rule 3 pinning). This reduction should be ok because using nVersion=3 isn't standard yet, so this wouldn't break somebody's existing use case. If we find that this is too small, we can always increase it later. Decreasing would be much more difficult. ~[Expected size of a commitment transaction](https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction) is within (900 + 172 * 483 + 224) / 4 = 21050vB~ EDIT: this is incorrect, but perhaps not something that should affect how we choose this number. ACKs for top commit: sdaftuar: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 achow101: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 instagibbs: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 t-bast: ACK https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11 murchandamus: crACK 154b2b2296edccb5ed24e829798dacb6195edc11 Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
2024-05-23Merge bitcoin/bitcoin#30115: rpc: avoid copying into UniValueRyan Ofsky
d7707d9843b03f20d2a8c5a45d7b3db58e169e6f rpc: avoid copying into UniValue (Cory Fields) Pull request description: These are the simple (and hopefully obviously correct) copies that can be moves instead. This is a follow-up from https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2108751842 As it turns out, there are hundreds of places where we copy UniValues needlessly. It should be the case that moves are always preferred over copies, so there should be no downside to these changes. willcl-ark, however, noticed that memory usage may increase in some cases. Logically this makes no sense to me. The only plausible explanation imo is that because the moves are faster, more ops/second occur in some cases. This list of moves was obtained by changing the function signatures of the UniValue functions to accept only rvalues, then compiling and fixing them up one by one. There still exist many places where copies are being made. These can/should be fixed up, but weren't done here for the sake of doing the easy ones first. I ran these changes through clang-tidy with `performance-move-const-arg` and `bugprone-use-after-move` and no bugs were detected (though that's obviously not to say it can be trusted 100%). As stated above, there are still lots of other less trivial fixups to do after these including: - Using non-const UniValues where possible so that moves can happen - Refactoring code in order to be able to move a UniValue without introducing a use-after-move - Refactoring functions to accept UniValues by value rather than by const reference ACKs for top commit: achow101: ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f ryanofsky: Code review ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f. No changes since last review other than rebase. I agree benchmarks showing increased peak memory usage and RSS are surprising, but number of allocations is down as expected, and runtime is also decreased. willcl-ark: ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f Tree-SHA512: 7f511be73984553c278186286a7d161a34b2574c7f5f1a0edc87c2913b4c025a0af5241ef9af2df17547f2e4ef79710aa5bbb762fc9472435781c0488dba3435
2024-05-21Merge bitcoin/bitcoin#30137: build: Remove `--enable-threadlocal`merge-script
17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af build: remove --enable-threadlocal (fanquake) 1e7c20bc19a216269c646177ab90cfa084c096a5 doc: remove comment about using thread_local (fanquake) 5bba43312c0ceccfe18bd4d086e12ec0497ed926 build: Enable `thread_local` for MinGW-w64 builds (Hennadii Stepanov) Pull request description: Includes a commit from #30099. Closes #29952. ACKs for top commit: laanwj: Code review ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af maflcko: utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af hebasto: ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af. theuni: utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af Tree-SHA512: 2aad6d19e79c4d6d7aefd0f41b215ac8d9320f5908808221d78e6ee1c77503832a02759bee2ad397e235b6739e22aca8dcf5c5ef8854deb8c697b18ac56a06da
2024-05-21Merge bitcoin/bitcoin#29421: net: make the list of known message types a ↵Ava Chow
compile time constant b3efb486732f3caf8b8a8e9d744e6d20ae4255ef protocol: make message types constexpr (Vasil Dimov) 2fa9de06c2c8583ee8e2434dc97014b26e218ab5 net: make the list of known message types a compile time constant (Vasil Dimov) Pull request description: Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size. --- This change is part of https://github.com/bitcoin/bitcoin/pull/29418 but it makes sense on its own and would be good to have it, regardless of the fate of https://github.com/bitcoin/bitcoin/pull/29418. Also, if this is merged, that would reduce the size of https://github.com/bitcoin/bitcoin/pull/29418, thus the current standalone PR. ACKs for top commit: achow101: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef jonatack: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef maflcko: utACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef 🎊 willcl-ark: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef Tree-SHA512: 6d3860c138c64514ebab13d97ea67893e2d346dfac30a48c3d9bc769a1970407375ea4170afdb522411ced306a14a9af4eede99e964d1fb1ea3efff5d5eb57af
2024-05-21[fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limitsglozow
2024-05-21rpc: Optimize serialization disk space of dumptxoutsetFabian Jahr
Co-authored-by: Aurèle Oulès <aurele@oules.com> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2024-05-21build: remove --enable-threadlocalfanquake
2024-05-21doc: remove comment about using thread_localfanquake
Followup to https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1605655974.
2024-05-20rpc: avoid copying into UniValueCory Fields
These are simple (and hopefully obviously correct) copies that can be moves instead.
2024-05-17[[refactor]] Check CTxMemPool options in constructorTheCharlatan
This ensures that the tests run the same checks on the mempool options that the init code also applies.
2024-05-17Merge bitcoin/bitcoin#29817: kernel: De-globalize fReindexAva Chow
b47bd959207e82555f07e028cc2246943d32d4c3 kernel: De-globalize fReindex (TheCharlatan) Pull request description: fReindex is one of the last remaining globals exposed by the kernel library, so move it into the blockstorage class to reduce the amount of global mutable state and make the kernel library a bit less awkward to use. --- This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). ACKs for top commit: achow101: ACK b47bd959207e82555f07e028cc2246943d32d4c3 ryanofsky: Code review ACK b47bd959207e82555f07e028cc2246943d32d4c3. I rereviewed the whole PR, but the only change since last review was reverting the bugfix https://github.com/bitcoin/bitcoin/pull/29817#discussion_r1578327024 and make the change a pure refactoring. mzumsande: Code Review ACK b47bd959207e82555f07e028cc2246943d32d4c3 stickies-v: ACK b47bd959207e82555f07e028cc2246943d32d4c3 Tree-SHA512: f7399d01f93bc0c0c7428fe95d19b9d29b4ed00a4f1deabca78fb0c4fecb434ec971e890feecb105938b5247c926850b1b7b4a4a9caa333a061e40777d0c8463
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-05 22:50:17 +0000