aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2024-01-03Merge bitcoin/bitcoin#29013: test: doc: follow-up #28368Gloria Zhao
b1318dcc56a0181783ee7ddbd388ae878a0efc52 test: change `m_submitted_in_package` input to fuzz data provider boolean (ismaelsadeeq) 5615e16b705d74bf6ebb7c39523844f97a41cb6f tx fees: update `m_from_disconnected_block` to `m_mempool_limit_bypassed` (ismaelsadeeq) fcd429664818f14cace580513e7e6159335b5416 doc: fix typo and update incorrect comment (ismaelsadeeq) 562664d26374331d291b97e2e2f7fca1f0fd467b test: wait for fee estimator to catch up before estimating fees (ismaelsadeeq) Pull request description: This is a simple PR that does two things 1. Fixes #29000 by waiting for the fee estimator to catch up after `removeForBlock` calls before calling `estimateFee` in the `BlockPolicyEstimates` unit test. 2. Addressed some outstanding review comments from #28368 - Updated `NewMempoolTransactionInfo::m_from_disconnected_block` to `NewMempoolTransactionInfo::m_mempool_limit_bypassed` which now correctly indicates what the boolean does. - Changed input of `processTransaction`'s tx_info `m_submitted_in_package` input from false to fuzz data provider boolean. - Fixed some typos, and update incorrect comment ACKs for top commit: martinus: re-ACK b1318dcc56a0181783ee7ddbd388ae878a0efc52 glozow: utACK b1318dcc56a0181783ee7ddbd388ae878a0efc52 Tree-SHA512: 45268729bc044da4748fe004524e0df696d2ec92c5bd053db9aad6e15675f3838429b2a7b9061a6b694be4dc319d1782a876b44df506ddd439d62ad07252d0e1
2024-01-02Merge bitcoin/bitcoin#29076: fuzz: set `m_fallback_fee` and `m_fee_mode` in ↵Ava Chow
`wallet_fees` target e03d6f7ed534f423f58236866f8e83beee1871e1 fuzz: set `m_fallback_fee`/`m_fee_mode` in `wallet_fees` target (brunoerg) Pull request description: `m_fallback_fee` and `m_fee_mode` are used in `GetMinimumFeeRate` but we're not setting any value for them in `wallet_fees` target. That's the reason fuzzing is never reaching the following code: ![Screenshot 2023-12-13 at 15 04 30](https://github.com/bitcoin/bitcoin/assets/19480819/454ddcaa-75ca-452f-ad13-5f142de0bdce) This PR fixes it. ACKs for top commit: maflcko: review ACK e03d6f7ed534f423f58236866f8e83beee1871e1 achow101: ACK e03d6f7ed534f423f58236866f8e83beee1871e1 murchandamus: ACK e03d6f7ed534f423f58236866f8e83beee1871e1 Tree-SHA512: 5d364f5351d65762a3ddf88e3abb7bda401b7e4955285e083031d216fb50082b1ea98e2c065aff75a5a8a3d1bc4c2e5e3ca9f9478d902ee8f8d4347b6cbe53af
2024-01-02Merge bitcoin/bitcoin#26684: bench: add readblock benchmarkAva Chow
1c4b9cbe906507295d8b7d52855de1441ad411dd bench: add readblock benchmark (Andrew Toth) Pull request description: Requested in https://github.com/bitcoin/bitcoin/pull/13151#issuecomment-385962450. See https://github.com/bitcoin/bitcoin/pull/26415 and https://github.com/bitcoin/bitcoin/pull/21319. Benchmarking shows a >50x increase in speed on both nvme and spinning disk. Benchmark results: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 5,377,375.00 | 185.96 | 0.2% | 60,125,513.00 | 11,633,676.00 | 5.168 | 3,588,800.00 | 0.4% | 0.09 | `ReadBlockFromDiskTest` | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 89,945.58 | 11,117.83 | 0.7% | 12,743.90 | 64,530.33 | 0.197 | 2,595.20 | 0.2% | 0.01 | `ReadRawBlockFromDiskTest` ACKs for top commit: maflcko: lgtm ACK 1c4b9cbe906507295d8b7d52855de1441ad411dd achow101: ACK 1c4b9cbe906507295d8b7d52855de1441ad411dd TheCharlatan: ACK 1c4b9cbe906507295d8b7d52855de1441ad411dd Tree-SHA512: 71dbcd6c7e2be97eb3001e35d0a95ef8e0c9b10dc9193025c7f8e11a09017fa2fbf89489b686353cd88fb409fb729fe2c4a25c567d2988f64c9c164ab09fba9f
2024-01-02test: change `m_submitted_in_package` input to fuzz data provider booleanismaelsadeeq
In reality some mempool transaction might be submitted in a package, so change m_submitted_in_package to fuzz data provider boolean just like m_has_no_mempool_parents.
2024-01-02tx fees: update `m_from_disconnected_block` to `m_mempool_limit_bypassed`ismaelsadeeq
The boolean indicates whether the transaction was added without enforcing mempool fee limits. m_mempool_limit_bypassed is the correct variable name. Also changes NewMempoolTransactionInfo booleans descriptions to the format that is consistent with the codebase.
2024-01-02doc: fix typo and update incorrect commentismaelsadeeq
2023-12-23refactor: share and use `GenerateRandomKey` helperSebastian Falbesoner
Making the `GenerateRandomKey` helper available to other modules via key.{h.cpp} allows us to create random private keys directly at instantiation of CKey, in contrast to the two-step process of creating the instance and then having to call `MakeNewKey(...)`.
2023-12-21Merge bitcoin/bitcoin#29056: refactor: Print verbose serialize compiler ↵Ava Chow
error messages fae526345de539ab8f9b80100f6dfbe8e1d3284b Allow std::byte C-style array serialization (MarcoFalke) fa898e6836a8fc2c7b6c8c15ad21818b16a89863 refactor: Print verbose serialize compiler error messages (MarcoFalke) Pull request description: Currently, trying to serialize an object that can't be serialized will fail with a short error message. For example, the diff and the error message: ```diff diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index d75eb499b4..773f49845b 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -62,6 +62,8 @@ public: BOOST_AUTO_TEST_CASE(sizes) { + int b[4]; + DataStream{} << b << Span{b}; BOOST_CHECK_EQUAL(sizeof(unsigned char), GetSerializeSize((unsigned char)0)); BOOST_CHECK_EQUAL(sizeof(int8_t), GetSerializeSize(int8_t(0))); BOOST_CHECK_EQUAL(sizeof(uint8_t), GetSerializeSize(uint8_t(0))); ``` ``` ./serialize.h:765:6: error: member reference base type 'const int[4]' is not a structure or union 765 | a.Serialize(os); | ~^~~~~~~~~~ ``` ``` ./serialize.h:277:109: error: no matching function for call to 'UCharCast' 277 | template <typename Stream, typename B> void Serialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); } | ^~~~~~~~~ ``` This is fine. However, it would be more helpful for developers and more accurate by the compiler to explain why each function is not selected. Fix this by using C++20 concepts where appropriate. ACKs for top commit: ajtowns: reACK fae526345de539ab8f9b80100f6dfbe8e1d3284b achow101: ACK fae526345de539ab8f9b80100f6dfbe8e1d3284b TheCharlatan: Re-ACK fae526345de539ab8f9b80100f6dfbe8e1d3284b Tree-SHA512: e03a684ccfcc5fbcad7f8a4899945a05989b555175fdcaebdb113aff46b52b4ee7b467192748edf99c5c348a620f8e52ab98bed3f3fca88280a64dbca458fe8a
2023-12-21Merge bitcoin/bitcoin#29027: wallet: fix key parsing check for miniscript ↵Ava Chow
expressions e1281f1bbd884f15d40053c9bc24794d0ce9a58a wallet: fix key parsing check for miniscript expressions in `ParseScript` (brunoerg) Pull request description: In `ParseScript`, when processing miniscript expressions, the way we check for key parsing error is wrong, the actual code is unreachable because we're checking it into `if (node)` (successful parsing) statement. ACKs for top commit: sipa: utACK e1281f1bbd884f15d40053c9bc24794d0ce9a58a RandyMcMillan: utACK https://github.com/bitcoin/bitcoin/commit/e1281f1bbd884f15d40053c9bc24794d0ce9a58a achow101: ACK e1281f1bbd884f15d40053c9bc24794d0ce9a58a Tree-SHA512: c4b3765d32673928a1f6d84ecbaa311870da9a9625753ed15ea57c802a9f16ddafa48c1dc66c0e4be284c5862e7821ed94135498ed9b9f3d7342a080035da289
2023-12-20Merge bitcoin/bitcoin#28372: fuzz: coinselection, improve ↵Ava Chow
`min_viable_change`/`change_output_size` cd810075eddd8b1a7139559b475b56126f70a93d fuzz: coinselection, improve `min_viable_change`/`change_output_size` (brunoerg) Pull request description: Instead of "randomly" fuzzing `min_viable_change` and `change_output_size`, and since they're correlated, this PR changes the approach to fuzz them according to the logic in `CreateTransactionInternal`. ACKs for top commit: murchandamus: ACK cd810075eddd8b1a7139559b475b56126f70a93d achow101: ACK cd810075eddd8b1a7139559b475b56126f70a93d furszy: Code ACK cd810075eddd Tree-SHA512: 4539b469f00cdf666078d80c07ed062726f804e390400348148cd3092db9cdc178c6d00ead39aef19acf97badfb6576ce23546d8967387e81c5398d52d7f4404
2023-12-20Merge bitcoin/bitcoin#29115: [doc]: add doxygen comment describing what ↵glozow
`CheckPackageLimits` returns 19bb65bf255df0f876e37de90fb8c4c6229cdf52 [doc]: add doxygen return comment for CheckPackageLimits (ismaelsadeeq) Pull request description: This PR adds a doxygen comment on `CheckPackageLimits` describing what the method returns. Fixes https://github.com/bitcoin/bitcoin/pull/28863#discussion_r1429805433 ACKs for top commit: Sjors: utACK 19bb65bf255df0f876e37de90fb8c4c6229cdf52 Zero-1729: utACK 19bb65bf255df0f876e37de90fb8c4c6229cdf52 Tree-SHA512: ccf1cc00a44d3fff60f28ad6766019a9f61b349729eab3cb02bc76b13c2e55441348a1602d806e60e4b2eabeb1f5d1ddacddf86c0bcdb78b078bb3a863b650c2
2023-12-19Merge bitcoin/bitcoin#29037: Add multiplication operator to CFeeRateAva Chow
1757452cc55a6dacc62e4258043ee4d711fd281a test: Add tests for CFeeRate multiplication operator (Kashif Smith) 1553c8078698df1058b62e8fdadaf74160977b30 Add multiplication operator to CFeeRate (Murch) Pull request description: Allows us to use `coin_selection_params.m_long_term_feerate * 3` or `3 * coin_selection_params.m_long_term_feerate` instead of `CFeeRate{coin_selection_params.m_long_term_feerate.GetFee(3000)}` inspired by https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1414455724 ACKs for top commit: kevkevinpal: reACK [1757452](https://github.com/bitcoin/bitcoin/pull/29037/commits/1757452cc55a6dacc62e4258043ee4d711fd281a) achow101: ACK 1757452cc55a6dacc62e4258043ee4d711fd281a ajtowns: ACK 1757452cc55a6dacc62e4258043ee4d711fd281a ; lgtm ismaelsadeeq: ACK 1757452cc55a6dacc62e4258043ee4d711fd281a Tree-SHA512: a86faac1efd1b7688630cd811246533d184d56b62064a7fd9007de95dbf81fa668aa2252253d102fba67517b6a4ca2dc367c5388b8ab936215734d7d370740cf
2023-12-19[doc]: add doxygen return comment for CheckPackageLimitsismaelsadeeq
2023-12-18Merge bitcoin/bitcoin#28863: wallet, mempool: propagete `checkChainLimits` ↵glozow
error message to wallet 8dec9c560b53488c1e71d8f74241c7dce42cb387 wallet, mempool: propagete `checkChainLimits` error message to wallet (ismaelsadeeq) Pull request description: * Requested in [#28391 comment](https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1382997719) * The error message is static when a new transaction is created and package limit is reached. `Transaction has too long of a mempool chain` While the [`CTxMempool::CheckPackageLimits`](https://github.com/bitcoin/bitcoin/blob/5800c558eb5efb4839ed00d6967e43306d68e1c3/src/txmempool.cpp#L199) provide explicit information about the error message. * This PR updates [`CTxMempool::CheckPackageLimits`](https://github.com/bitcoin/bitcoin/blob/5800c558eb5efb4839ed00d6967e43306d68e1c3/src/txmempool.cpp#L199) return type to `util::Result<void>`, `CheckPackageLimits` now returns void when package limit is not hit, and returns the error string whenever package limit is hit instead of using out parameter `errString`. * The PR updates [`checkChainLimits`](https://github.com/bitcoin/bitcoin/blob/5800c558eb5efb4839ed00d6967e43306d68e1c3/src/node/interfaces.cpp#L703) return type to `util::Result<void>`. * Now the wallet `CreateTransactionInternal` will have access to the package limit error string whenever its hit. * Also Updated functional test to reflect the error message from `CTxMempool::CheckPackageLimits` output. ACKs for top commit: glozow: utACK 8dec9c560b Sjors: utACK 8dec9c560b53488c1e71d8f74241c7dce42cb387 TheCharlatan: Re-ACK 8dec9c560b53488c1e71d8f74241c7dce42cb387 Tree-SHA512: ddeac18aeba6f8e3be0e3fe76bf3db655352e3b415169f1f83ea1e8976a2f3e3de021c8da6880eb8382ab52d545e418e3f4d57adcc68ecb4f390339710ee6f30
2023-12-18Merge bitcoin/bitcoin#29064: fuzz: Improve fuzzing stability for minisketch ↵fanquake
harness b2fc7a2eda103724ac8cbeaf99df3ce6f5b7d974 [fuzz] Improve fuzzing stability for minisketch harness (dergoegge) Pull request description: The `minisketch` harness has low stability due to: * Rng internal to minisketch * Benchmarkning for the best minisketch impl Fix this by seeding the rng and letting the fuzzer choose the impl. Also see #29018. ACKs for top commit: maflcko: review ACK b2fc7a2eda103724ac8cbeaf99df3ce6f5b7d974 Tree-SHA512: 3d81414299c6803c34e928a53bcf843722fa8c38e1d3676cde7fa80923f9058b1ad4b9a2941f718303a6641b17eeb28b4a22eda09678102e9fb7c4e31d06f8f2
2023-12-18Merge bitcoin/bitcoin#29079: fuzz: Limit p2p fuzz targets to ↵fanquake
MAX_PROTOCOL_MESSAGE_LENGTH fa769d3e41daec696452b8a0a8753ba511b0a4b5 fuzz: Limit p2p fuzz targets to MAX_PROTOCOL_MESSAGE_LENGTH (MarcoFalke) Pull request description: Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65039 ACKs for top commit: dergoegge: utACK fa769d3e41daec696452b8a0a8753ba511b0a4b5 brunoerg: crACK fa769d3e41daec696452b8a0a8753ba511b0a4b5 Tree-SHA512: 46f70d1acf4e2f95055c70162909010c6322f8504a810906e1ab4db470dc2525f9a494b8427b254279bc68b1c8b87338c943787fd5249df7113556740701a51a
2023-12-17wallet, mempool: propagete `checkChainLimits` error message to walletismaelsadeeq
Update CheckPackageLimits to use util::Result to pass the error message instead of out parameter. Also update test to reflect the error message from `CTxMempool` `CheckPackageLimits` output.
2023-12-15Merge bitcoin/bitcoin#29088: tests: Don't depend on value of ↵Ava Chow
DEFAULT_PERMIT_BAREMULTISIG 7b45744df33c6a4759eae1a3984f389cbac837c2 tests: ensure functional tests set permitbaremultisig=1 when needed (Anthony Towns) 7dfabdcf860c529772a54b0e8fa235cbb4a78b4d tests: test both settings for permitbaremultisig in p2sh tests (Anthony Towns) Pull request description: Update unit and functional tests so that they continue to work if the default for `-permitbaremultisig` is changed. ACKs for top commit: maflcko: lgtm ACK 7b45744df33c6a4759eae1a3984f389cbac837c2 instagibbs: crACK https://github.com/bitcoin/bitcoin/pull/29088/commits/7b45744df33c6a4759eae1a3984f389cbac837c2 ajtowns: > crACK [7b45744](https://github.com/bitcoin/bitcoin/commit/7b45744df33c6a4759eae1a3984f389cbac837c2) achow101: ACK 7b45744df33c6a4759eae1a3984f389cbac837c2 glozow: ACK 7b45744df33c6a4759eae1a3984f389cbac837c2, changed default locally and all tests passed Tree-SHA512: f89f9e2bb11f07662cfd57390196df9e531064e1bd662e1db7dcfc97694394ae5e8014e9d209b9405aa09195bf46fc331b7fba10378065cdb270cbd0669ae904
2023-12-15Allow std::byte C-style array serializationMarcoFalke
2023-12-15refactor: Print verbose serialize compiler error messagesMarcoFalke
2023-12-15fuzz: coinselection, improve `min_viable_change`/`change_output_size`brunoerg
Change it to use same approach from `CreateTransactionInternal`.
2023-12-15tests: test both settings for permitbaremultisig in p2sh testsAnthony Towns
2023-12-14Merge bitcoin/bitcoin#29040: refactor: Remove pre-C++20 code, fs::path cleanupAva Chow
66667130416b86208e01a0eb5541a15ea805ac26 refactor: Rename fs::path::u8string() to fs::path::utf8string() (MarcoFalke) 856c88776f8486446602476a1c9e133ac0cff510 ArgsManager: return path by value from GetBlocksDirPath() (Vasil Dimov) fa3d9304e80c214c8b073f12a7f4b08c5a94af04 refactor: Remove pre-C++20 fs code (MarcoFalke) fa00098e1a493aa3cce20335d18e7f5f2fb7a4a8 Add tests for C++20 std::u8string (MarcoFalke) fa2bac08c22182e738a8cabf1b24a9dbf3b092d2 refactor: Avoid copy/move in fs.h (MarcoFalke) faea30227ba633da5ab257d0247853e0927244bb refactor: Use C++20 std::chrono::days (MarcoFalke) Pull request description: This: * Removes dead code. * Avoids unused copies in some places. * Adds copies in other places for safety. ACKs for top commit: achow101: ACK 66667130416b86208e01a0eb5541a15ea805ac26 ryanofsky: Code review ACK 66667130416b86208e01a0eb5541a15ea805ac26. Just documentation change since last review. stickies-v: re-ACK 66667130416b86208e01a0eb5541a15ea805ac26 Tree-SHA512: 6176e44f30b310d51632ec2d3827c3819905d0ddc6a4b57acfcb6cfa1f9735176da75ee8ed4a4abd1296cb0b83bee9374cc6f91ffac87c19b63c435eeadf3f46
2023-12-14Merge bitcoin/bitcoin#28920: wallet: birth time update during tx scanningAva Chow
1ce45baed7dd2da3f1cb85c9c25110e5537451ae rpc: getwalletinfo, return wallet 'birthtime' (furszy) 83c66444d0604f0a9ec3bc3f89d4f1a810b7cda0 test: coverage for wallet birth time interaction with -reindex (furszy) 6f497377aa17cb8a590fd7717fa8ededf4249999 wallet: fix legacy spkm default birth time (furszy) 75fbf444c1e13c6ba0e79a34871534c845a13849 wallet: birth time update during tx scanning (furszy) b4306e3c8db6cbaedc8845c6d21c750b39f682bf refactor: rename FirstKeyTimeChanged to MaybeUpdateBirthTime (furszy) Pull request description: Fixing #28897. As the user may have imported a descriptor with a timestamp newer than the actual birth time of the first key (by setting 'timestamp=now'), the wallet needs to update the birth time when it detects a transaction older than the oldest descriptor timestamp. Testing Notes: Can cherry-pick the test commit on top of master. It will fail there. ACKs for top commit: Sjors: re-utACK 1ce45baed7dd2da3f1cb85c9c25110e5537451ae achow101: ACK 1ce45baed7dd2da3f1cb85c9c25110e5537451ae Tree-SHA512: 10c2382f87356ae9ea3fcb637d7edc5ed0e51e13cc2729c314c9ffb57c684b9ac3c4b757b85810c0a674020b7287c43d3be8273bcf75e2aff0cc1c037f1159f9
2023-12-14Merge bitcoin/bitcoin#28051: Get rid of shutdown.cpp/shutdown.h, use ↵Ava Chow
SignalInterrupt directly 6db04be102807ee0120981a9b8de62a55439dabb Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly (Ryan Ofsky) 213542b625a6a4885fcbdfe236629a5f381eeb05 refactor: Add InitContext function to initialize NodeContext with global pointers (Ryan Ofsky) feeb7b816affa790e02e7ba0780c4ef33d2310ff refactor: Remove calls to StartShutdown from KernelNotifications (Ryan Ofsky) 6824eecaf1e74624cf149ed20abd9145c49d614a refactor: Remove call to StartShutdown from stop RPC (Ryan Ofsky) 1d92d89edbb1812dc353084c62772ebb1024d632 util: Get rid of uncaught exceptions thrown by SignalInterrupt class (Ryan Ofsky) ba93966368d3aaa426b97837ef475ec5aa612f5f refactor: Remove call to ShutdownRequested from IndexWaitSynced (Ryan Ofsky) 42e5829d9710ebebda5de356fab01dd7c149d5fa refactor: Remove call to ShutdownRequested from HTTPRequest (Ryan Ofsky) 73133c36aa9cc09546eabac18d0ea35274dd5d72 refactor: Add NodeContext::shutdown member (Ryan Ofsky) f4a8bd6e2f03e786a84dd7763d1c04665e6371f2 refactor: Remove call to StartShutdown from qt (Ryan Ofsky) f0c73c1336bee74fe2d58474ac36bca28c219e85 refactor: Remove call to ShutdownRequested from rpc/mining (Ryan Ofsky) 263b23f0082c60516acced1b03abb8e4d8f9ee46 refactor: Remove call to ShutdownRequested from chainstate init (Ryan Ofsky) Pull request description: This change drops `shutdown.h` and `shutdown.cpp` files, replacing them with a `NodeContext::shutdown` member which is used to trigger shutdowns directly. This gets rid of an unnecessary layer of indirection, and allows getting rid of the `kernel::g_context` global. Additionally, this PR tries to improve error handling of `SignalInterrupt` code by marking relevant methods `[[nodiscard]]` to avoid the possibility of uncaught exceptions mentioned https://github.com/bitcoin/bitcoin/pull/27861#discussion_r1255496707. Behavior is changing In a few cases which are noted in individual commit messages. Particularly: GUI code more consistently interrupts RPCs when it is shutting down, shutdown state no longer persists between unit tests, the stop RPC now returns an RPC error if requesting shutdown fails instead of aborting, and other failed shutdown calls now log errors instead of aborting. This PR is a net reduction in lines of code, but in some cases the explicit error handling and lack of global shutdown functions do make it more verbose. The verbosity can be seen as good thing if it discourages more code from directly triggering shutdowns, and instead encourages code to return errors or send notifications that could be translated into shutdowns. Probably a number of existing shutdown calls could just be replaced by better error handling. ACKs for top commit: achow101: ACK 6db04be102807ee0120981a9b8de62a55439dabb TheCharlatan: Re-ACK 6db04be102807ee0120981a9b8de62a55439dabb maflcko: ACK 6db04be102807ee0120981a9b8de62a55439dabb 👗 stickies-v: re-ACK 6db04be102807ee0120981a9b8de62a55439dabb Tree-SHA512: 7a34cb69085f37e813c43bdaded1a0cbf6c53bd95fdde96f0cb45346127fc934604c43bccd3328231ca2f1faf712a7418d047ceabd22ef2dca3c32ebb659e634
2023-12-14[fuzz] Improve fuzzing stability for minisketch harnessdergoegge
* Seed minisketch rng * Use fuzzer chosen minisketch impl instead of benchmarking for the best impl
2023-12-14Merge bitcoin/bitcoin#29022: Make bitcoin-tx replaceable value optionalAva Chow
98afe7866185ed4157ffc581763e11dc02fcbae0 doc: Update bitcoin-tx replaceable documentation (Kashif Smith) 94feaf2b66d68b3c849375e1d9d3a81c17cd2045 tests: Add unit tests for bitcoin-tx replaceable command (Kashif Smith) c2b836b119eeed8727d73bcca5e95055eb93fb1a bitcoin-tx: Make replaceable value optional (Kashif Smith) Pull request description: This fixes #28638. The issue was originally raised by dooglus, who also suggested the patch found in this code. Additionally, test coverage has been added and documentation has been updated. ACKs for top commit: achow101: ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 pinheadmz: ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 hernanmarino: Tested ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 instagibbs: untested ACK https://github.com/bitcoin/bitcoin/pull/29022/commits/98afe7866185ed4157ffc581763e11dc02fcbae0 Tree-SHA512: ea1384aba7b0014c8cbeb7280d66b1e617d406fb02471dff33873057132b80518c94c7caa4b0426c26d17ce8aa393107de319dde781ace8df72f0314c8c75159
2023-12-14refactor: Rename fs::path::u8string() to fs::path::utf8string()MarcoFalke
2023-12-14fuzz: Limit p2p fuzz targets to MAX_PROTOCOL_MESSAGE_LENGTHMarcoFalke
2023-12-13fuzz: set `m_fallback_fee`/`m_fee_mode` in `wallet_fees` targetbrunoerg
2023-12-13Merge bitcoin/bitcoin#29065: bench: wallet, fix change position out of range ↵Ava Chow
error 37c75c58202f89b752500f76c872d7f8caf6bdb3 test: wallet, fix change position out of range error (furszy) Pull request description: Fixes #29061. Only the benchmark is affected. Since #25273, the behavior of 'inserting change at a random position' is instructed by passing ´std::nullopt´ instead of -1. Also, added missing documentation about the meaning of 'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()' ACKs for top commit: achow101: ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3 kevkevinpal: ACK [37c75c5](https://github.com/bitcoin/bitcoin/pull/29065/commits/37c75c58202f89b752500f76c872d7f8caf6bdb3) BrandonOdiwuor: ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3 Tree-SHA512: d9a8d8533540455716a5090fcf407573cad9f0d0018a05f903f89e51620302f9b256318db6f7338b85c047f7fab372d724e916b1721d7ed302dbf3d845b08734
2023-12-13Merge bitcoin/bitcoin#28075: util: Remove DirIsWritable, GetUniquePathfanquake
fa3da629a1aebcb4500803d7417feed8e34285b0 Remove DirIsWritable, GetUniquePath (MarcoFalke) fad3a9793b71df5bb0b17cc3758cf3466d08c015 Return LockResult::ErrorWrite in LockDirectory (MarcoFalke) fa0afe740843c308f6287b923f1f4d758cf2a3f6 refactor: Return enum in LockDirectory (MarcoFalke) Pull request description: `GetUniquePath` is only used in tests and in `DirIsWritable`. The check by `DirIsWritable` is redundant with the check done in `LockDirectory`. Fix the redundancy by removing everything, except `LockDirectory`. ACKs for top commit: TheCharlatan: Re-ACK fa3da629a1aebcb4500803d7417feed8e34285b0 hebasto: ACK fa3da629a1aebcb4500803d7417feed8e34285b0, I have reviewed the code and it looks OK. Tree-SHA512: e95f18cd586de7582e9c08ac7ddb860bfcfcbc8963804f45c5784c5e4c0598dc59ae7e45dd4daf30a5020dbf8433f5db2ad06e46a8676371982003790043c6c9
2023-12-12test: wallet, fix change position out of range errorfurszy
Since #25273, the behavior of 'inserting change at a random position' is instructed by passing std::nullopt instead of -1. Also, added missing documentation about the meaning of 'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()'
2023-12-12Merge bitcoin/bitcoin#28994: wallet: skip BnB when SFFO is enabledAndrew Chow
576bee88fd36e207b7288077626947a1fce0fc33 fuzz: disable BnB when SFFO is enabled (furszy) 05e5ff194c7722b4ebc2b9309fc0bf47b3cf1df7 test: add coverage for BnB-SFFO restriction (furszy) 0c5755761c3e544547899ad096121585dffa73df wallet: create tx, log resulting coin selection info (furszy) 5cea25ba795d6eb9ccc721d01560783ae576af34 wallet: skip BnB when SFFO is active (Murch) Pull request description: Solves #28918. Coming from https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1838626406 discussion. The intention is to decouple only the bugfix relevant commits from #28985, allowing them to be included in the 26.x release. This way, we can avoid disabling the coin selection fuzzing test for an entire release. Note: Have introduced few changes to the bug fix commit so that the unit tests pass without the additional burden introduced in #28985. ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33 murchandamus: ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33 achow101: ACK 576bee88fd36e207b7288077626947a1fce0fc33 Tree-SHA512: f5d90eb3f3f524265afe4719495c9bf30f98b9af26cf039f7df5a7db977abae72caa7a3478cdd0ab10cd143bc1662e8fc5286b5bc10fc10f0dd582a45b45c31a
2023-12-12Merge bitcoin/bitcoin#29055: tests, bench: Fix issue with ↵fanquake
CWallet::LoadWallet() being called in the wrong places bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 wallet: Assert that the wallet is not initialized in LoadWallet (Andrew Chow) fb0b6ca4e5d981cf58bf23ae2993117f171608e8 tests, bench: Remove incorrect LoadWallet() calls (Andrew Chow) Pull request description: `CWallet::LoadWallet()` expects to be called after a `CWallet` is constructed, but before any of its member functions called. Doing so invalidates pointers which causes issues with some PRs and branches that I am working on. This was being used incorrectly in a few tests and benchmarks, resulting in segfaults. As a precaution for this kind of issue in the future, I've also added a few asserts to `LoadWallet()` so that developers will notice when it is used incorrectly. As similar issue was fixed in #27666 ACKs for top commit: S3RK: ACK bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 furszy: ACK bd7f5d33 Tree-SHA512: 7664f12b8452994e7fc4d7d4f77697fb5f75edb0dba95ba99a4a23ec03d5b8e0ecbdcb7635547a0e8b4f89f708f98dcb5d039df0559e24b1ae411ed630e16e14
2023-12-12Merge bitcoin/bitcoin#29021: refactor: rpc: Pass CBlockIndex by reference ↵fanquake
instead of pointer fa5989d514d246e56977c528b2dd2abe6dc8efcc refactor: rpc: Pass CBlockIndex by reference instead of pointer (MarcoFalke) fa604eb6cfa7f70ce11c78c1060f0823884c745b refactor: Use reference instead of pointer in IsBlockPruned (MarcoFalke) Pull request description: Follow-up to https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841435462 ACKs for top commit: TheCharlatan: ACK fa5989d514d246e56977c528b2dd2abe6dc8efcc pablomartin4btc: tACK fa5989d514d246e56977c528b2dd2abe6dc8efcc dergoegge: Code review ACK fa5989d514d246e56977c528b2dd2abe6dc8efcc Tree-SHA512: 7449de3e3bb435dcbf438df88df343bb70f6edc3228ee7c0078f912ffb415e951ba30f8ecad916765f8cf896f0d784fe30535c5cf997e303cf5af257ade69773
2023-12-11fuzz: disable BnB when SFFO is enabledfurszy
2023-12-11test: add coverage for BnB-SFFO restrictionfurszy
Verify the transaction creation process does not produce a BnB solution when SFFO is enabled. This is currently problematic because it could require a change output. And BnB is specialized on changeless solutions. Co-authored-by: Andrew Chow <achow101@gmail.com> Co-authored-by: Murch <murch@murch.one>
2023-12-11wallet: Assert that the wallet is not initialized in LoadWalletAndrew Chow
LoadWallet() cannot be run after the wallet has been initialized. So assert that to avoid making this mistake in the future.
2023-12-11tests, bench: Remove incorrect LoadWallet() callsAndrew Chow
LoadWallet() must only be called immediately after a CWallet is constructed, or not at all. Doing so after any other CWallet member functions have been called may cause pointers and other objects setup by other those functions to become invalidated. Since these tests and benchmarks are using completely new wallets with mock databases, it's not necessary to call LoadWallet() anyways, so these can be dropped.
2023-12-11test: Add tests for CFeeRate multiplication operatorKashif Smith
2023-12-11doc: Update bitcoin-tx replaceable documentationKashif Smith
2023-12-11ArgsManager: return path by value from GetBlocksDirPath()Vasil Dimov
`ArgsManager::m_cached_blocks_path` is protected by `ArgsManager::cs_args` and returning a reference to it after releasing the mutex is unsafe. To resolve this, return a copy of the path. This has some performance penalty which is presumably ok, given that paths are a few 100s bytes at most and `GetBlocksDirPath()` is not called often. This silences the following (clang 18): ``` common/args.cpp:288:31: error: returning variable 'm_cached_blocks_path' by reference requires holding mutex 'cs_args' [-Werror,-Wthread-safety-reference-return] 288 | if (!path.empty()) return path; | ^ ``` Do the same with `ArgsManager::GetDataDir()`, `ArgsManager::GetDataDirBase()` and `ArgsManager::GetDataDirNet()`.
2023-12-11refactor: Remove pre-C++20 fs codeMarcoFalke
Treating std::string as UTF-8 is deprecated in std::filesystem::path since C++20. However, it makes this codebase easier to read and maintain to retain the ability for std::string to hold UTF-8.
2023-12-11Add tests for C++20 std::u8stringMarcoFalke
Also, add missing includes: #include <system_error> // for error_code #include <type_traits> // for is_same #include <cerrno> // for errno
2023-12-11refactor: Avoid copy/move in fs.hMarcoFalke
The operator accepts a const& reference, so no copy or move is needed. See https://en.cppreference.com/w/cpp/filesystem/path/append
2023-12-11refactor: Use C++20 std::chrono::daysMarcoFalke
2023-12-11Merge bitcoin/bitcoin#28999: build: Enable -Wunreachable-codefanquake
fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876 build: Enable -Wunreachable-code (MarcoFalke) Pull request description: It seems a bit confusing to write code after a `return`. This can even lead to bugs, or incorrect code, such as https://github.com/bitcoin/bitcoin/pull/28830/files#r1415372320 . (Edit: The linked instance is not found by clang's `-Wunreachable-code`). Fix all issues by enabling `-Wunreachable-code`. This flag also enables `-Wunreachable-code-loop-increment`, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code, so remove that. ACKs for top commit: ajtowns: > ACK [fa8adbe](https://github.com/bitcoin/bitcoin/commit/fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876) stickies-v: ACK fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876 jonatack: ACK fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876 tested with arm64 clang 17.0.6 Tree-SHA512: 12a2f74b69ae002e62ae08038f7458837090a12051a4c154d05ae4bb26fb19fc1fa76c63aedf2b3fbb36f048c593ca3b8c0efe03fe93cf07a0fd114fc84ce1e7
2023-12-11Merge bitcoin/bitcoin#25273: wallet: Pass through transaction locktime and ↵fanquake
preset input sequences and scripts to CreateTransaction 0295b44c257fe23f1ad344aff11372d67864f0db wallet: return CreatedTransactionResult from FundTransaction (Andrew Chow) 758501b71391136c33b525b1a0109b990d4f463e wallet: use optional for change position as an optional in CreateTransaction (Andrew Chow) 2d39db7aa128a948b6ad11242591ef26a342f5b1 wallet: Explicitly preserve scriptSig and scriptWitness in CreateTransaction (Andrew Chow) 14e50746f683361f4d511d384d6f1dc44ed2bf10 wallet: Explicitly preserve transaction version in CreateTransaction (Andrew Chow) 0fefcbb776063b7fcc03c28e544d830a2f540250 wallet: Explicitly preserve transaction locktime in CreateTransaction (Andrew Chow) 4d335bb1e00a414a4740007d5a192a73179b2262 wallet: Set preset input sequence through coin control (Andrew Chow) 596642c5a9f52dda2599b0bde424366bb22b3c6e wallet: Replace SelectExternal with SetTxOut (Andrew Chow) 5321786b9d90eaf35059bb07e6beaaa2cbb257ac coincontrol: Replace HasInputWeight with returning optional from Get (Andrew Chow) e1abfb5b2037ca4fe5a05aa578030c8016491c8b wallet: Introduce and use PreselectedInput class in CCoinControl (Andrew Chow) Pull request description: Currently `FundTransaction` handles transaction locktime and preset input data by extracting the selected inputs and change output from `CreateTransaction`'s results. This means that `CreateTransaction` is actually unaware of any user desired locktime or sequence numbers. This can have an effect on whether and how anti-fee-sniping works. This PR makes `CreateTransaction` aware of the locktime and preset input data by providing them to `CCoinControl`. `CreateTransasction` will then set the sequences, scriptSigs, scriptWItnesses, and locktime as appropriate if they are specified. This allows `FundTransaction` to actually use `CreateTransaction`'s result directly instead of having to extract the parts of it that it wants. Additionally `FundTransaction` will return a `CreateTransactionResult` as `CreateTransaction` does instead of having several output parameters. Lastly, instead of using `-1` as a magic number for the change output position, the change position is changed to be an optional with no value set indicating no desired change output position (when provided as an input parameter) or no change output present (in the result). ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/commit/0295b44c257fe23f1ad344aff11372d67864f0db S3RK: Code review ACK 0295b44c257fe23f1ad344aff11372d67864f0db Tree-SHA512: 016be4d41cbf97e1938506e70959bb5335b87006162a1c1c62fa0adb637cbe7aefb76d342b8efad5f37dc693f270c8d0a0839e239fd1ac32c6941a8172f1a710
2023-12-11Merge bitcoin/bitcoin#29009: fuzz: p2p: Detect peer deadlocksfanquake
9f265d88253ed464413dea5614fa13dea0d8cfd5 fuzz: Detect deadlocks in process_message (dergoegge) fae1e7e012571201fd51c547ba4fb6044be9aeb5 fuzz: p2p: Detect peer deadlocks (MarcoFalke) Pull request description: It may be possible that a peer connection will deadlock, due to software bugs such as https://github.com/bitcoin/bitcoin/pull/18808. Fix this by detecting them in the fuzz target. Can be tested by introducing a bug such as: ```diff diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1067341495..97495a13df 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2436,3 +2436,3 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic if (it != peer.m_getdata_requests.end() && !pfrom.fPauseSend) { - const CInv &inv = *it++; + const CInv& inv = *it; if (inv.IsGenBlkMsg()) { ``` Using a fuzz input such as: ``` $ base64 ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 kNptdNbW1tbWYghvXIpwb25vPQAA////////cwAjLv8AXAB2ZXJhY2sAQW5v/62tra3Pz/////// //////////////////////9c8GZpbHRlcmxvYWQAAAEAAwAAAABVYwC2XABmaWx0ZXJhZGQAAAAX Fxdn/////2V0F861tcqvEmAAACEAAABjYXB0dXJldmUAAH4AgAA1PNfX11x0Z2V0ZGF0YQBDACOw AQMAAAAGIm5GERoLWcqvEmBD61u/KMNPOl4zKh/HKLK3PPGIkQ9eE/////////8AAAAAAAAAAFtb WyjDTzpeMSofx7K3PNfX11x0Z2V0ZGF0YQBDACMwAQMAAAAGIm5GERoLWcqvEmBD61u/KMNPOl4z Kh/Hsrc88YiRD2/Nzc3Nzc3Nzc3NTc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3N zWWj1NTUudTU1NTU1P///0j+P/9cdHR4AAAAAAAAy/4AAHR4AAAAAAAAP8v+AAD/+P////////// AX55bJl8HWnz/////wAgXGF0YVPxY2RkAAAA ``` And running the fuzz target: ``` $ FUZZ=process_messages ./src/test/fuzz/fuzz -runs=1 -timeout=18 ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 3436516708 INFO: Loaded 1 modules (390807 inline 8-bit counters): 390807 [0x55d0d6221e80, 0x55d0d6281517), INFO: Loaded 1 PC tables (390807 PCs): 390807 [0x55d0d6281518,0x55d0d6877e88), ./src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each. Running: ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 ALARM: working on the last Unit for 19 seconds and the timeout value is 18 (use -timeout=N to change) ==375014== ERROR: libFuzzer: timeout after 19 seconds ``` ACKs for top commit: naumenkogs: ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 dergoegge: ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 brunoerg: ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 Tree-SHA512: da83ff90962bb679aae00e8e9dba639c180b7aaba544e0c4d0978d36e28a9ff1cd7a2e13009d8ab407ef57767656aca1ebc767a7d2f1bc880284f8f57c197a50
generated by cgit v1.2.3 (git 2.26.2) at 2025-01-05 07:10:31 +0000