aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2024-06-14netbase: extend CreateSock() to support creating arbitrary socketsVasil Dimov
Allow the callers of `CreateSock()` to pass all 3 arguments to the `socket(2)` syscall. This makes it possible to create sockets of any domain/type/protocol.
2024-06-14Merge bitcoin/bitcoin#30281: Update leveldb subtree to latest upstreammerge-script
a37778d4d32b4ddeff96f68a130dc8da3a84b278 Squashed 'src/leveldb/' changes from e2f10b4e47..688561cba8 (fanquake) Pull request description: Includes https://github.com/bitcoin-core/leveldb-subtree/pull/41 which is used in #30234. ACKs for top commit: theuni: utACK 95812d912b6335caa7af2a084d84447fb4aad156 Tree-SHA512: 3d943695a3d33816cf5558b183f5629aa92a500a1544eecedf84952e93c8592a8cf0d554b88281fc0bad3c9e920ebcff1ed8edc12f8e73f36ed5335482beb829
2024-06-13Merge bitcoin/bitcoin#29607: refactor: Reduce memory copying operations in ↵Ava Chow
bech32 encoding 07f64177a49f1b6b4d486d10cf67fddfa3c995eb Reduce memory copying operations in bech32 encode (Lőrinc) d5ece3c4b5e109f65f5d3315c43239dd87bb2c81 Reserve hrp memory in Decode and LocateErrors (Lőrinc) Pull request description: Started optimizing the base conversions in [TryParseHex](https://github.com/bitcoin/bitcoin/pull/29458), [Base58](https://github.com/bitcoin/bitcoin/pull/29473) and [IsSpace](https://github.com/bitcoin/bitcoin/pull/29602) - this is the next step. Part of this change was already merged in https://github.com/bitcoin/bitcoin/pull/30047, which made decoding `~26%` faster. Here I've reduced the memory reallocations and copying operations in bech32 encode, making it `~15%` faster. > make && ./src/bench/bench_bitcoin --filter='Bech32Encode' --min-time=1000 Before: ``` | ns/byte | byte/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 19.97 | 50,074,562.72 | 0.1% | 1.06 | `Bech32Encode` ``` After: ``` | ns/byte | byte/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 17.33 | 57,687,668.20 | 0.1% | 1.10 | `Bech32Encode` ``` ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/pull/29607/commits/07f64177a49f1b6b4d486d10cf67fddfa3c995eb sipa: utACK 07f64177a49f1b6b4d486d10cf67fddfa3c995eb achow101: ACK 07f64177a49f1b6b4d486d10cf67fddfa3c995eb Tree-SHA512: 511885217d044ad7ef2bdf9203b8e0b94eec8b279bc193bb7e63e29ab868df6d21e9e4c7a24390358e1f9c131447ee42039df72edcf1e2b11e1856eb2b3e10dd
2024-06-13Update leveldb-subtree subtree to latest upstreamfanquake
2024-06-13Merge bitcoin/bitcoin#30270: Update minisketch subtree to ↵merge-script
eb37a9b8e79f9e49d73b96a49bf97a96d9eb676c cb59af38e72ab189b052ec84e2d1027473235ba4 Squashed 'src/minisketch/' changes from 3472e2f5ec..eb37a9b8e7 (fanquake) Pull request description: Includes https://github.com/sipa/minisketch/pull/87 which is used in https://github.com/bitcoin/bitcoin/pull/30234. Includes https://github.com/sipa/minisketch/pull/88 which is used in https://github.com/bitcoin/bitcoin/pull/29876. ACKs for top commit: sipa: utACK 89464ad59cf11f68315ea3104236989e5b429d15 theuni: utACK 89464ad59cf11f68315ea3104236989e5b429d15 Tree-SHA512: 838a8c60856bfdf714da7d5d97e31d458290849ba5007d5c5bb7abb83d413ada6b4c16e45b0e060ff892b5785e6b664be9b6a666d04f0a414b0e359d64d3ad44
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-12Update minisketch subtree to latest masterfanquake
2024-06-12Squashed 'src/minisketch/' changes from 3472e2f5ec..eb37a9b8e7fanquake
eb37a9b8e7 Merge sipa/minisketch#87: Avoid copy in self-assign fe6557642e Merge sipa/minisketch#88: build: Add `-Wundef` 8ea298bfa7 Avoid copy in self-assign 978a3d8869 build: Add `-Wundef` 3387044179 Merge sipa/minisketch#86: doc: fix typo in sketch_impl.h 15c2d13b60 doc: fix typo in sketch_impl.h 7be08b8a46 Merge sipa/minisketch#85: Fixes for integer precision loss 00fb4a4d83 Avoid or make integer precision conversion explicit 9d62a4d27c Avoid the need to cast/convert to size_t for vector operations 19e06cc7af Prevent overflows from large capacity/max_elements git-subtree-dir: src/minisketch git-subtree-split: eb37a9b8e79f9e49d73b96a49bf97a96d9eb676c
2024-06-12fuzz: Use std::span in FuzzBufferTypeMarcoFalke
2024-06-12Merge bitcoin/bitcoin#30268: util: add missing VecDeque includeglozow
f51da34ec1a806d321a468691fa66082eef10ad9 utils: add missing include (Cory Fields) Pull request description: Noticed when testing `VecDeque` with no other includes. For libc++, need type_traits for `std::is_trivially_destructible_v`. ACKs for top commit: maflcko: ACK f51da34ec1a806d321a468691fa66082eef10ad9 glozow: ACK f51da34ec1a806d321a468691fa66082eef10ad9 sipa: utACK f51da34ec1a806d321a468691fa66082eef10ad9 Tree-SHA512: bf96910abe9aaddd8586e6cc8f68a9bbac4c26d976ebeebcfa86b86c0da5783c1cbdbc7fa09b62cdcfde19e6442eb65a66bf1e2e80408d68e9dd9689dc22b0fa
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#29521: cli: Detect port errors in rpcconnect and rpcportAva Chow
24bc46c83b39149f4845a575a82337eb46d91bdb cli: Add warning for duplicate port definition (tdb3) e208fb5d3bea4c1fb750cb0028819635ecdeb415 cli: Sanitize ports in rpcconnect and rpcport (tdb3) Pull request description: Adds invalid port detection to bitcoin-cli for -rpcconnect and -rpcport. In addition to detecting malformed/invalid ports (e.g. those outside of the 16-bit port range, not numbers, etc.), bitcoin-cli also now considers usage of port 0 to be invalid. bitcoin-cli previously considered port 0 to be valid and attempted to use it to reach bitcoind. Functional tests were added for invalid port detection as well as port prioritization. Additionally, a warning is provided when a port is specified in both -rpcconnect and -rpcport. This PR is an alternate approach to PR #27820 (e.g. SplitHostPort is unmodified). Considered an alternative to 127.0.0.1 being specified in functional tests, but at first glance, this might need an update to test_framework/util.py (e.g. rpc_url), which might be left to a future PR. ACKs for top commit: S3RK: light code review ACK 24bc46c83b39149f4845a575a82337eb46d91bdb achow101: ACK 24bc46c83b39149f4845a575a82337eb46d91bdb cbergqvist: re ACK 24bc46c83b39149f4845a575a82337eb46d91bdb Tree-SHA512: c83ab6a30a08dd1ac8b368a7dcc2b4f23170f0b61dd67ffcad7bcda05096d333bcb9821fba11018151f55b2929c0a333bfec15b8bb863d83f41fc1974c6efca5
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-11utils: add missing includeCory Fields
Noticed when testing VecDeque with no other includes. For libc++, need type_traits for std::is_trivially_destructible_v.
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-10util: add BitSetPieter Wuille
This adds a bitset module that implements a BitSet<N> class, a variant of std::bitset with a few additional features that cannot be implemented in a wrapper without performance loss (specifically, finding first and last bit set, or iterating over all set bits).
2024-06-10Merge bitcoin/bitcoin#30257: build: Remove --enable-gprofmerge-script
fa780e1c25e8e98253e32d93db65f78a0092433f build: Remove --enable-gprof (MarcoFalke) Pull request description: It is unclear what benefit this option has, given that: * `gprof` requires re-compilation (`perf` and other tools can instead be used on existing executables) * `gprof` requires hardening to be disabled * `gprof` doesn't work with `clang` * `perf` is documented in the dev-notes, and test notes, and embedded into the functional test framework; `gprof` isn't * Anyone really wanting to use it could pass the required flags to `./configure` * I couldn't find any mention of the use of `gprof` in the discussions in this repo, apart from the initial pull request adding it (cfaac2a60f3ac63ae8deccb03d88bd559449b78c) * Keeping it means that it needs to be maintained and ported to CMake Fix all issues by removing it. ACKs for top commit: TheCharlatan: ACK fa780e1c25e8e98253e32d93db65f78a0092433f hebasto: ACK fa780e1c25e8e98253e32d93db65f78a0092433f, I have reviewed the code and it looks OK. willcl-ark: crACK fa780e1c25e8e98253e32d93db65f78a0092433f Tree-SHA512: 0a9ff363ac2bec8b743878a4e3147f18bc16823d00c5007568432c36320bd0199b13b6d0ce828a9a83c2cc434c058afaa64eb2eccfbd93ed85b81ce10c41760c
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-09build: Remove --enable-gprofMarcoFalke
This reverts cfaac2a60f3ac63ae8deccb03d88bd559449b78c
2024-06-09test: doc: fix units in tx size standardness test (s/vbytes/weight units)Sebastian Falbesoner
2024-06-09refactor: performance-for-range-copy in psbt.hMarcoFalke
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-07blockman: Replace m_reindexing with m_blockfiles_indexedRyan Ofsky
This is a just a mechanical change, renaming and inverting the meaning of the indexing variable. "m_blockfiles_indexed" is a more straightforward name for this variable because this variable just indicates whether or not <datadir>/blocks/blk?????.dat files have been indexed in the <datadir>/blocks/index LevelDB database. The name "m_reindexing" was more confusing, it could be true even if -reindex was not specified, and false when it was specified. Also, the previous name unnecessarily required thinking about the whole reindexing process just to understand simple checks in validation code about whether blocks were indexed. The motivation for this change is to follow up on previous commits, moving away from having multiple variables called "reindex" internally, and instead naming variables individually after what they do and represent.
2024-06-07indexes: Don't wipe indexes again when already reindexingTheCharlatan
Before this change continuing a reindex without the -reindex flag set would leave the block and coins db intact, but discard the data of the optional indexes. While not a bug per se, wiping the data again is wasteful, both in terms of having to write it again, and potentially leading to longer startup times. When initially running a reindex, both the block index and any further activated indexes are wiped. On an index's Init(), both the best block stored by the index and the chain's tip are null. An index's m_synced member is therefore true. This means that it will process blocks through validation events while the reindex is running. Currently, if the reindex is continued without the user re-specifying the reindex flag, the block index is preserved but further index data is wiped. This leads to the stored best block being null, but the chain tip existing. The m_synced member will be set to false. The index will not process blocks through the validation interface, but instead use the background sync once the reindex is completed. If the index is preserved (this change) after a restart its best block may potentially match the chain tip. The m_synced member will be set to true and the index can process validation events during the rest of the reindex.
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-07bitcoin-cli: use json-rpc 2.0Matthew Zipkin
2024-06-07validation: Remove needs_init from LoadBlockIndexTheCharlatan
It does not control any actual logic and the log message as well as the comment are obsolete, since no database initialization takes place there anymore. Log messages indicating when indexes and chainstate databases are loaded exist in other places.
2024-06-07bugfix: Streamline setting reindex optionTheCharlatan
Reverts a bug introduced in b47bd959207e82555f07e028cc2246943d32d4c3 "kernel: De-globalize fReindex". The change leads to a GUI user being prompted to re-index on a chainstate loading failure more than once as well as the node actually not reindexing if the user chooses to. Fix this by setting the reindexing option instead of the atomic, which can be safely re-used to indicate that a reindex should be attempted. The bug specifically is caused by the chainman, and thus the blockman and its m_reindexing atomic being destroyed on every iteration of the for loop. The reindex option for ChainstateLoadOptions is currently also set in a confusing way. By using the reindex atomic, it is not obvious in which scenario it is true or false. The atomic is controlled by both the user passing the -reindex option, the user chosing to reindex if something went wrong during chainstate loading when running the gui, and by reading the reindexing flag from the block tree database in LoadBlockIndexDB. In practice this read is done through the chainstate module's CompleteChainstateInitialization's call to LoadBlockIndex. Since this is only done after the reindex option is set already, it does not have an effect on it. Make this clear by using the reindex option from the blockman opts which is only controlled by the user.
2024-06-06tests: add fuzz tests for VecDequePieter Wuille
2024-06-06util: add VecDequePieter Wuille
This is an STL-like container that interface-wise looks like std::deque, but is backed by a (fixed size, with vector-like capacity/reserve) circular buffer.
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-05Reduce memory copying operations in bech32 encodeLőrinc
Here I've reduced the memory reallocations and copying operations in bech32 encode, making it ~15% faster. make && ./src/bench/bench_bitcoin --filter='Bech32Encode' --min-time=1000 Before: | ns/byte | byte/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 19.97 | 50,074,562.72 | 0.1% | 1.06 | `Bech32Encode` After: | ns/byte | byte/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 17.33 | 57,687,668.20 | 0.1% | 1.10 | `Bech32Encode` Co-authored-by: josibake <josibake@protonmail.com>
2024-06-05Reserve hrp memory in Decode and LocateErrorsLőrinc
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#28307: rpc, wallet: fix incorrect segwit redeem script ↵Ava Chow
size limit 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 test: addmultisigaddress, coverage for script size limits (furszy) 53302a09817e5b799d345dfea432546a55a9d727 bugfix: addmultisigaddress, add unsupported operation for redeem scripts over 520 bytes (furszy) 9be6065cc03f2408f290a332b203eef9c9cebf24 test: coverage for 16-20 segwit multisig scripts (furszy) 9d9a91c4ea6b3bb32ef4131bca86f1d6683fc901 rpc: bugfix, incorrect segwit redeem script size used in signrawtransactionwithkey (furszy) 0c9fedfc45fa7cbd6801ca5fd756863ec9a6911c fix incorrect multisig redeem script size limit for segwit (furszy) f7a173b5785cda460470df9a74a0e0f94d7f9a18 test: rpc_createmultisig, decouple 'test_sortedmulti_descriptors_bip67' (furszy) 4f33dbd8f8c0e29f37b04e6af6d2c7905ecceaf6 test: rpc_createmultisig, decouple 'test_mixing_uncompressed_and_compressed_keys' (furszy) 25a81705d376e8c96dad45436ae3fca975b3daf5 test: rpc_createmultisig, remove unnecessary checkbalances() (furszy) b5a328943362cfac6e90fd4e1b167c357d53b7d4 test: refactor, multiple cleanups in rpc_createmultisig.py (furszy) 3635d432681847313c098f9827483372a840e70f test: rpc_createmultisig, remove manual wallet initialization (furszy) Pull request description: Fixing https://github.com/bitcoin/bitcoin/issues/28250#issuecomment-1674830104 and more. Currently, redeem scripts longer than 520 bytes, which are technically valid under segwit rules, have flaws in the following processes: 1) The multisig creation process fails to deduce the output descriptor, resulting in the generation of an incorrect descriptor. Additionally, the accompanying user warning is also inaccurate. 2) The `signrawtransactionwithkey` RPC command fail to sign them. 3) The legacy wallet `addmultisigaddress` wrongly discards them. The issue arises because most of these flows are utilizing the legacy spkm keystore, which imposes the [p2sh max redeem script size rule](https://github.com/bitcoin/bitcoin/blob/ded687334031f4790ef6a36b999fb30a79dcf7b3/src/script/signingprovider.cpp#L160) on all scripts. Which blocks segwit redeem scripts longer than the max element size in all the previously mentioned processes (`createmultisig`, `addmultisigaddress`, and `signrawtransactionwithkey`). This PR fixes the problem, enabling the creation of multisig output descriptors involving more than 15 keys and allowing the signing of these scripts, along with other post-segwit redeem scripts that surpass the 520-byte p2sh limit. Important note: Instead of adding support for these longer redeem scripts in the legacy wallet, an "unsupported operation" error has been added. The reasons behind this decision are: 1) The introduction of this feature brings about a compatibility-breaking change that requires downgrade protection; older wallets would be unable to interact with these "new" legacy wallets. 2) Considering the ongoing deprecation of the legacy spkm, this issue provides another compelling reason to transition towards descriptors. Testing notes: To easily verify each of the fixes, I decoupled the tests into standalone commits. So they can be cherry-picked on top of master. Where `rpc_createmultisig.py` (with and without the `--legacy-wallet` arg) will fail without the bugs fixes commits. Extra note: The initial commits improves the `rpc_createmultisig.py` test in many ways. I found this test very antiquated, screaming for an update and cleanup. ACKs for top commit: pinheadmz: ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 theStack: Code-review ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 achow101: ACK 2451a217dd2c21b6d2f2b2699ceddd0bf9073019 Tree-SHA512: 71794533cbd46b3a1079fb4e9d190d3ea3b615de0cbfa443466e14f05e4616ca90e12ce2bf07113515ea8113e64a560ad572bb9ea9d4835b6fb67b6ae596167f
2024-06-04Merge bitcoin/bitcoin#28074: fuzz: wallet, add target for `Crypter`Ava Chow
d7290d662f494503f28e087dd728b492c0bb2c5f fuzz: wallet, add target for Crypter (Ayush Singh) Pull request description: This PR adds fuzz coverage for `wallet/crypter`. Motivation: Issue [27272](https://github.com/bitcoin/bitcoin/issues/27272#issue-1628327906) I ran this for a long time with Sanitizers on and had no crashes; the average `exec/sec` also looks good to me. However, I would really appreciate it if some of the reviewers could try it on their machines too, and give their feedback. ACKs for top commit: maflcko: utACK d7290d662f494503f28e087dd728b492c0bb2c5f achow101: ACK d7290d662f494503f28e087dd728b492c0bb2c5f brunoerg: utACK d7290d662f494503f28e087dd728b492c0bb2c5f Tree-SHA512: f5c496cabdd3263a7e1ad49eeff702725336f76bf19a82e5dbbead082e990889dd43c851d0d2d6ab740f44b8ec2aa06defd9ff6b02be68b5f8b4eaf963f88599
2024-06-04Merge bitcoin/bitcoin#30047: refactor: Model the bech32 charlimit as an EnumAva Chow
7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd refactor: replace hardcoded numbers (Lőrinc) 5676aec1e1a6d2c6fd3099e120e263a0a7def089 refactor: Model the bech32 charlimit as an Enum (josibake) Pull request description: Broken out from #28122 --- Bech32(m) was defined with a 90 character limit so that certain guarantees for error detection could be made for segwit addresses (see https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#checksum-design). However, there is nothing about the encoding scheme itself that requires a limit of 90 and in practice bech32(m) is being used without the 90 char limit (e.g. lightning invoices, silent payments). Further, increasing the character limit doesn't do away with error detection, it simply changes the guarantee. The primary motivation for this change is for being able to parse BIP352 v0 silent payment addresses (see https://github.com/bitcoin/bitcoin/pull/28122/commits/622c7a98b9f08177a3cfb601306daabb101af1fd), which require up to 118 characters. In addition to BIP352, modeling the character limit as an enum allows us to easily support new address types that use bech32m and specify their own character limit. ACKs for top commit: paplorinc: re-ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd achow101: ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd theuni: utACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd Tree-SHA512: 9c793d657448c1f795093b9f7d4d6dfa431598f48d54e1c899a69fb2f43aeb68b40ca2ff08864eefeeb6627d4171877234b5df0056ff2a2b84415bc3558bd280
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#28979: wallet, rpc: document and update `sendall` ↵Ava Chow
behavior around unconfirmed inputs 71aae72e1fc998b2629d68a7301d85dc1b65641e test: test sendall does ancestor aware funding (ishaanam) 36757941a05b65c2b61a83820afdf5effd8fc9a2 wallet, rpc: implement ancestor aware funding for sendall (ishaanam) 544131f3fba9ea07fee29f9d3ee0116cd5d8a5b2 rpc, test: test sendall spends unconfirmed change and unconfirmed inputs when specified (ishaanam) Pull request description: This PR: - Adds a functional test that `sendall` spends unconfirmed change - Adds a functional test that `sendall` spends regular unconfirmed inputs when specified by user - Adds ancestor aware funding to `sendall` by using `calculateCombinedBumpFee` and adjusting the effective value accordingly - Adds a functional test for ancestor aware funding in `sendall` ACKs for top commit: S3RK: ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e achow101: ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e furszy: ACK 71aae72e1f Tree-SHA512: acaeb7c65166ce53123a1d6cb5012197202246acc02ef9f37a28154cc93afdbd77c25e840ab79bdc7e0b88904014a43ab1ddea79d5337dc310ea210634ab61f0
generated by cgit v1.2.3 (git 2.26.2) at 2025-02-17 00:38:33 +0000