aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-07-15Merge bitcoin-core/gui#827: OptionsDialog: Prefer to stretch actual options ↵Hennadii Stepanov
area rather than waste space b71bfd9eef8b0017cef3c05361c02ddbd837e6ac GUI/OptionsDialog: Prefer to stretch actual options area rather than waste space (Luke Dashjr) Pull request description: ACKs for top commit: hebasto: ACK b71bfd9eef8b0017cef3c05361c02ddbd837e6ac Tree-SHA512: b706a07292fe81379e303f9069fca6efd5ceb15ee5bb77c6aeddbf63f736494ce877b76767ff17d7becf98d07209e51c74bdb99365596b7b9f4904a30438d72d
2024-07-15Merge bitcoin/bitcoin#30373: fuzz: fix key size in `crypter`merge-script
4383dc90bac1b5def73352fe222f99807d8ca4dd fuzz: fix key size in crypter target (brunoerg) Pull request description: Fixes #30251 This PR: 1. Limits `cipher_text_ed` and `random_string` (`SecureString`) size. 2. Replace `ConsumeRandomLengthByteVector` for keys to `ConsumeFixedLengthByteVector` with `WALLET_CRYPTO_KEY_SIZE`. 3. Replace `ConsumeRandomLengthByteVector` for `chSalt` to `ConsumeFixedLengthByteVector` with `WALLET_CRYPTO_SALT_SIZE`. ACKs for top commit: marcofleon: Tested ACK 4383dc90bac1b5def73352fe222f99807d8ca4dd. I ran this: dergoegge: utACK 4383dc90bac1b5def73352fe222f99807d8ca4dd Tree-SHA512: 6f09cca0b4627f49152b685ac03659c01004f2131c6aada7654606ea01f6619b1611b1d17624d2cddce277c1afdddda5f656d99f6ca8f72a22f5c0541762c964
2024-07-15Merge bitcoin-core/gui#795: Keep focus on "Hide" while ModalOverlay is visibleHennadii Stepanov
992b1bbd5da95ee782515fb0f5674bb7a02684b2 qt: keep focus on "Hide" while ModalOverlay is visible (Jadi) Pull request description: During the initial sync, the Tab moves the focus to the widgets of the main window, even when the ModalOverlay is visible. This creates some weird rectangular *selections on the screen*. This PR fixes this by keeping the focus on the "Hide" button while the ModalOverlay is visible. Fixes #783 ACKs for top commit: pablomartin4btc: Concept & approach ACK 992b1bbd5da95ee782515fb0f5674bb7a02684b2 hebasto: re-ACK 992b1bbd5da95ee782515fb0f5674bb7a02684b2 Tree-SHA512: f702a3fd51db4bc10780bccf76394e35a6b5fb45db72c9c23cd10d777106b08c61077d2d989003838921e76d2cb44f809399f31df76448e4305a6c2a71b5c6a3
2024-07-15Merge bitcoin/bitcoin#30412: MiniMiner: use FeeFrac in AncestorFeerateComparatormerge-script
09370529fb9f6d06f6d16bdb1fb336f7a265d8ba fuzz: mini_miner_selection fixups. (glozow) de273d53004f48e4c8c965f7ce0bd375fd8d0d69 MiniMiner: use FeeFrac in AncestorFeerateComparator (glozow) Pull request description: Closes #30284. Closes #30367, see https://github.com/bitcoin/bitcoin/issues/30367#issuecomment-2217459257 Previously, we were only comparing feerates up to 1/1000 precision, since CFeeRate comparison just looks at their respective nSatoshisPerK. This could lead to MiniMiner selecting packages in the wrong order (i.e. by txid) if their feerates were less than 0.001sat/vB different. Fix this by creating + comparing `FeeFrac`s instead. Also, `FeeFrac::Mul` doesn't have the overflow problem. Also added a few minor fuzzer fixups that caught my eye while I was debugging this. ACKs for top commit: ismaelsadeeq: Tested ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba murchandamus: ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba with nits dergoegge: tACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba Tree-SHA512: e5b6d6c3f7289f30cd8280d0a47cd852d0180b83d1b27ff9514f50c97103b0f069484e48cba2ca3a57419beadc1996c1b9dd8d0a0f34bc4f4223d8adaf414ce5
2024-07-14fuzz: limit the number of nested wrappers in descriptorsAntoine Poinsot
The script building logic performs a quadratic number of copies in the number of nested wrappers in the miniscript. Limit the number of nested wrappers to avoid fuzz timeouts. Thanks to Marco Falke for reporting the fuzz timeouts and providing a minimal input to reproduce.
2024-07-14fuzz: limit the number of sub-fragments per fragment for descriptorsAntoine Poinsot
This target may call into logic quadratic over the number of sub-fragments. Limit the number of sub-fragments to keep the runtime reasonable. Thanks to Marco Falke for reporting the fuzz timeouts with a minimized input.
2024-07-14Merge bitcoin-core/gui#825: Show maximum mempool size in information windowHennadii Stepanov
4a028cf54c0502bc9ba0804bf1ae413b20a007cb gui: show maximum mempool size in information window (Sebastian Falbesoner) bbde6ffefea9587b07a9f8d4043b2dd23ef8c3c5 add node interface method for getting maximum mempool size (Sebastian Falbesoner) Pull request description: This PR adds the maximum mempool size to the information window (Menu "Window" -> "Information" -> section "Memory Pool" -> line "Memory usage"). master: ![image](https://github.com/bitcoin-core/gui/assets/91535/157e92f5-7d06-4303-b4ef-bcdfac5527e3) PR: ![image](https://github.com/bitcoin-core/gui/assets/91535/796322aa-9f16-4b09-9893-bf52a3898a5c) ACKs for top commit: MarnixCroes: tested ACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb pablomartin4btc: tACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb luke-jr: tACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb & in Knots hebasto: ACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb, tested on Ubuntu 24.04. Tree-SHA512: c10fb23605d060cea19a86d11822fc4d12496b19547870052aace503670e62e4c4e19ae4c2c4fbf7420a472adb071c9ddebe82447e0cfbce5a6fb9fcd7b9eda3
2024-07-13net: Allow DNS lookups on nodes with IPV6 lo onlyMax Edwards
AI_ADDRCONFIG prevents ::1 from being considered a valid address on hosts that have a IPV6 loopback IP address but no other IPV6 interfaces.
2024-07-12Merge bitcoin/bitcoin#30295: #28984 package rbf followupsmerge-script
3f00aae14092ca076cff7f9ddf6155c601979fcd package rbf: cpfp structure requires package > parent feerate (Greg Sanders) ad7f1f697f01845470f8df0944a94012fabcba09 test package rbf boundary conditions more closely (Greg Sanders) ff4558d441f377a372647972d35b5476b94579c9 doc: reword package RBF documentation (Greg Sanders) de669a883bf65c576cc596b20a576d7bb1618182 doc: replace mention of V3 with TRUC (Greg Sanders) Pull request description: Some suggested nits/changes from #28984 ACKs for top commit: glozow: ACK 3f00aae1409 murchandamus: ACK 3f00aae14092ca076cff7f9ddf6155c601979fcd Tree-SHA512: 79434cc8aba25a43e99793298cdc99cad807db2c3a2e780a31953f244b95eecd97b90559abd67fbf30996c00966675fa257253a7812ec4727420226162c629ae
2024-07-12rest: Reject negative outpoint index in getutxos parsingMarcoFalke
2024-07-12init: change shutdown order of load block thread and schedulerMartin Zumsande
This avoids situations during a reindex in which shutdown doesn't finish since SyncWithValidationInterfaceQueue is called by the load block thread when the scheduler is already stopped.
2024-07-12Merge bitcoin/bitcoin#30372: util: Use SteadyClock in RandAddSeedPerfmonmerge-script
fa360b047fc578fd855b8f7420d84dc967884f4a util: Use SteadyClock in RandAddSeedPerfmon (MarcoFalke) Pull request description: `GetTime` is mockable in tests and system-changeable in production. This should be fine and not lead to issues, but using `SteadyClock` is more correct in this context to do an expensive task only so often. ACKs for top commit: sipa: utACK fa360b047fc578fd855b8f7420d84dc967884f4a TheCharlatan: ACK fa360b047fc578fd855b8f7420d84dc967884f4a Tree-SHA512: 1958b9e9e356c9801ac981014b4b528cfc8ce6612853d8b45f6519b16f0b1839ff765abb8b3368b86f00958ddc6a686f6b90278c57a7ad4858bdf3ea33775cca
2024-07-12Merge bitcoin/bitcoin#30336: depends: update doc in Qt pwd patchmerge-script
f170fe04ca03fe4021cbff7c5450ce3cc7fda17f depends: update doc in Qt pwd patch (fanquake) Pull request description: Now that upstream has gotten around to fixing this. We don't need any more of the patch, and it likely wont apply to our version of Qt in any case. See: https://github.com/qt/qtbase/commit/3388de698bfb9bbc456c08f03e83bf3e749df35c. ACKs for top commit: theuni: ACK f170fe04ca03fe4021cbff7c5450ce3cc7fda17f Tree-SHA512: f6db8ccad591b1bf144ce71f873f42a115d394c432a95b6b855e3e32751e6331145e0d9676657599b25fd369af8c72c1bd34e192a7a1062c15f152421422a9ed
2024-07-12rpc: Use CHECK_NONFATAL over AssertMarcoFalke
2024-07-12logging: Add DisableLogging()Anthony Towns
2024-07-11Merge bitcoin/bitcoin#30353: test: fix inconsistency in fundrawtransaction ↵Ava Chow
weight limits test 00b8e26bd6a47a8207c7571ca7ef9643f0ee2668 test: fix inconsistency in fundrawtransaction weight limits test (furszy) Pull request description: Fix https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1657628378 inconsistency. Currently, the test is passing due to a mistake in the test inputs selection process. We are selecting the parent transaction change output as one of the inputs of the transaction to fund, which helps to surpass the target amount when it shouldn't due to the fee reduction. The failure arises when the test behaves as intended by its coder; that is, when it does not select the change output. In this case, the pre-selected inputs aren't enough to cover the target amount. Fix this by excluding the parent transaction's change output from the inputs selection and including an extra input to cover the tx fee. The CI failure can be replicated with the following patch in master: ```diff diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py --- a/test/functional/wallet_fundrawtransaction.py(revision 9b480f7a25a737c9c4ebc33401e94d66c2da9ec3) +++ b/test/functional/wallet_fundrawtransaction.py(date 1720652934739) @@ -1322,7 +1322,7 @@ outputs = [] for _ in range(1472): outputs.append({wallet.getnewaddress(address_type="legacy"): 0.1}) - txid = self.nodes[0].send(outputs=outputs)["txid"] + txid = self.nodes[0].send(outputs=outputs, change_position=0)["txid"] self.generate(self.nodes[0], 1) # 272 WU per input (273 when high-s); picking 1471 inputs will exceed the max standard tx weight. @@ -1330,7 +1330,7 @@ # 1) Try to fund transaction only using the preset inputs input_weights = [] - for i in range(1471): + for i in range(1, 1472): # skip first output as it is the parent tx change output input_weights.append({"txid": txid, "vout": i, "weight": 273}) assert_raises_rpc_error(-4, "Transaction too large", wallet.fundrawtransaction, hexstring=rawtx, input_weights=input_weights) ``` ACKs for top commit: achow101: ACK 00b8e26bd6a47a8207c7571ca7ef9643f0ee2668 ismaelsadeeq: Code review and Tested ACK 00b8e26bd6a47a8207c7571ca7ef9643f0ee2668 Tree-SHA512: 5ef792961b7fad4999fc30aa03366432103ddf672ca5cbb366c9eab4c2e46d5ae1ab0c073dfc4fbb2b4e63203653bc0e54463c731c5f8655140207ba5f8e542e
2024-07-11Merge bitcoin/bitcoin#30234: Enable clang-tidy checks for self-assignmentmerge-script
26a7f70b5d2b1cbfbf03e0b57b9b5ea92dafbb19 ci: enable self-assignment clang-tidy check (Cory Fields) 32b1d1379258aa57c2a7e278f60d1117fcf17953 refactor: add self-assign checks to classes which violate the clang-tidy check (Cory Fields) Pull request description: See comment here: https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2148229582 Our code failed these checks in three places, which have been fixed up here. Though these appear to have been harmless, adding the check avoids the copy in the self-assignment case so there should be no downside. ~Additionally, minisketch failed the check as well. See https://github.com/sipa/minisketch/pull/87~ Edit: Done After fixing up the violations, turn on the aggressive clang-tidy check. Note for reviewers: `git diff -w` makes this trivial to review. ACKs for top commit: hebasto: ACK 26a7f70b5d2b1cbfbf03e0b57b9b5ea92dafbb19, I have reviewed the code and it looks OK. TheCharlatan: ACK 26a7f70b5d2b1cbfbf03e0b57b9b5ea92dafbb19 Tree-SHA512: 74d8236a1b5a698f2f61c4740c4fc77788b7f882c4b395acc4e6bfef1ec8a4554ea8821a26b14d70cfa6c8e2e9ea305deeea3fbf323967fa19343c007a53c5ba
2024-07-11Merge bitcoin/bitcoin#30406: refactor: modernize-use-equals-defaultmerge-script
3333bae9b2a6c1ee2314d33361c93944c12001f9 tidy: modernize-use-equals-default (MarcoFalke) Pull request description: Prior to C++20, `modernize-use-equals-default` could have been problematic because it could turn a non-aggregate into an aggregate. The risk would be that aggregate initialization would be enabled where the author did not intend to enable it. With C++20, aggregate for those is forbidden either way. (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1008r1.pdf) So enabled it for code clarity, consistency, and possibly unlocking compiler optimizations. See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-equals-default.html ACKs for top commit: stickies-v: ACK 3333bae9b2a6c1ee2314d33361c93944c12001f9 Tree-SHA512: ab42ff01be7ca7e7d8b4c6a485e68426f59627d83dd827cf292304829562348dc17a52ee009f5f6f3c1c2081d7166ffac4baef23197ebeba8de7767c6ddfe255
2024-07-11Merge bitcoin/bitcoin#30146: Add clang-tidy check for thread_local varsmerge-script
34c9cee380e7276cf3f85f2ce56a293e57afd676 clang-tidy: add check for non-trivial thread_local vars (Cory Fields) Pull request description: Forbid thread_local vars with non-trivial destructors. This is a follow-up from: https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1608423170 ACKs for top commit: maflcko: ACK 34c9cee380e7276cf3f85f2ce56a293e57afd676 TheCharlatan: Re-ACK 34c9cee380e7276cf3f85f2ce56a293e57afd676 Tree-SHA512: 3a798607fb189a5bbc714ed6e86dea462fe29d366b790e96d10a7b4ffcf1f194da9b8f4cd0b82154408709b8e3c58d3f613d6311903bd65a76d8b556ab230d21
2024-07-11Merge bitcoin/bitcoin#30383: util: Catch translation string errors at ↵merge-script
compile time fa601ab9f7142cdb18c18c1128fc893cdffb3463 util: Catch translation string errors at compile time (MarcoFalke) Pull request description: The translation helper function `_()` has many problems. For example, the following compiles: ```cpp auto ptr{"wrong"}; _(ptr); _(nullptr); _(0); _(NULL); ``` However, it is wrong, because none of the arguments passed to the function can be picked up by the translation tooling for transifex. Fix all issues by enforcing only real string literals can be passed to the function. ACKs for top commit: ryanofsky: Code review ACK fa601ab9f7142cdb18c18c1128fc893cdffb3463 hebasto: ACK fa601ab9f7142cdb18c18c1128fc893cdffb3463. Tree-SHA512: 33aed02d7e8fc9bfb8f90746f5c8072a8c0910fa900ec3516af2e732780b0fee8b07b6596c0fc210b018c0869111d6c34bf8d083de0e88ecdb4dee88e809186d
2024-07-11Merge bitcoin/bitcoin#30397: refactor: Use designated initializer in ↵merge-script
test/util/net.cpp e233ec036dc972a5847ab769ad22d418fd9404d1 refactor: Use designated initializer (Hodlinator) Pull request description: Block was recently touched (e2d1f84858485650ff743753ffa5c679f210a992) and the codebase recently switched to C++20 which allows this to improve robustness. Follow-up suggested in https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1664818014 ACKs for top commit: maflcko: ACK e233ec036dc972a5847ab769ad22d418fd9404d1 Tree-SHA512: ce3a18f513421e923710a43c8f97db1badb7ff5c6bdbfd62d9543312d2225731db5c14bef16feb47c43b84fad4dc24485086634b680feba422d2b7b363e13fa6
2024-07-11contrib: c++ify test stubs after switching to c++ compilersCory Fields
2024-07-11validation: Don't load a snapshot if it's not in the best header chain.Martin Zumsande
If the snapshot is not an ancestor of the most-work header (m_best_header), syncing from that alternative chain should be prioritised. Therefore don't accept loading a snapshot in this situation. If that other chain turns out to be invalid, m_best_header would be reset and loading the snapshot should be possible again. Because of the work required to generate a conflicting headers chain, this should only be possible under extreme circumstances, such as major forks.
2024-07-11Merge bitcoin/bitcoin#30427: remove truc_policy from libbitcoin_common_a_SOURCESmerge-script
e8c3b7172c33929e4e5bf6059da2d25a4ea8779c remove truc_policy.cpp from libbitcoin_common_a_SOURCES (glozow) Pull request description: Hebasto pointed out that it doesn't need to be there since it's in `libbitcoin_node_a_SOURCES` ACKs for top commit: maflcko: ACK e8c3b7172c33929e4e5bf6059da2d25a4ea8779c hebasto: ACK e8c3b7172c33929e4e5bf6059da2d25a4ea8779c, this change follows the design [docs](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md). ismaelsadeeq: ACK e8c3b7172c33929e4e5bf6059da2d25a4ea8779c Tree-SHA512: ebe6b0dda2d097d88c37d2b071ac99da3e9c519ec473d4b8f870a50f1b24d00e2e5deef317fb0f6a91c96103e7f37468cb8f13395818eab55a42af48df4e0fc6
2024-07-11Merge bitcoin/bitcoin#26596: wallet: Migrate legacy wallets to descriptor ↵glozow
wallets without requiring BDB 8ce3739edbcf6437bf2695087e0ebe8c633df19b test: verify wallet is still active post-migration failure (furszy) 771bc60f134c002a027e799639f0fed59ae8123d wallet: Use LegacyDataSPKM when loading (Ava Chow) 61d872f1b3d0dbfd0de4ff9b7deff40eeffa6a14 wallet: Move MigrateToDescriptor and DeleteRecords to LegacyDataSPKM (Ava Chow) b231f4d556876ae70305e8710e31d53525ded8ae wallet: Move LegacyScriptPubKeyMan::IsMine to LegacyDataSPKM (Ava Chow) 7461d0c006c92ede2f2595b79a5509eaf3509fb7 wallet: Move LegacySPKM data storage and handling to LegacyDataSPKM (Ava Chow) 517e204bacd9dcea6612aae57d5a2813b89cd01d Change MigrateLegacyToDescriptor to reopen wallet as BERKELEY_RO (Ava Chow) Pull request description: #26606 introduced `BerkeleyRODatabase` which is an independent parser for BDB files. This PR uses this in legacy wallet migration so that migration will continue to work once the legacy wallet and BDB are removed. `LegacyDataSPKM` is introduced to have the minimum data and functions necessary for a legacy wallet to be loaded for migration. ACKs for top commit: cbergqvist: ACK 8ce3739edbcf6437bf2695087e0ebe8c633df19b theStack: Code-review ACK 8ce3739edbcf6437bf2695087e0ebe8c633df19b furszy: Code review ACK 8ce3739edbcf6437bf2695087e0ebe8c633df19b Tree-SHA512: dccea12d6c597de15e3e42f97ab483cfd069e103611200279a177e021e8e9c4e74387c4f45d2e58b3a1e7e2bdb32a1d2d2060b1f8086c03eeaa0c68579d9d54e
2024-07-11log: Fix __func__ in LogError in blockstorage moduleMarcoFalke
These errors should never happen. However, when they do happen, it is useful to log the correct error location (function name). For example, this fixes an incorrect "ConnectBlock()" in "WriteUndoDataForBlock".
2024-07-11log: LogError with FlatFilePos in UndoReadFromDiskMarcoFalke
These errors should never happen in normal operation. If they do, knowing the FlatFilePos may be useful to determine if data corruption happened. Also, handle the error pos.IsNull() as part of OpenUndoFile, because it may as well have happened due to data corruption. This mirrors the LogError behavior from ReadBlockFromDisk.
2024-07-11refactor: Mark IsBlockPruned constMarcoFalke
Member fields are used read-only in this method.
2024-07-11remove truc_policy.cpp from libbitcoin_common_a_SOURCESglozow
It doesn't need it Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2024-07-10test: fix inconsistency in fundrawtransaction weight limits testfurszy
Currently, the test is passing due to a mistake in the test inputs selection process. We are selecting the parent transaction change output as one of the inputs of the transaction to fund, which helps to surpass the target amount when it shouldn't due to the fee reduction. The failure arises when the test behaves as intended by its coder; that is, when it does not select the change output. In this case, the pre-selected inputs aren't enough to cover the target amount. Fix this by excluding the parent transaction's change output from the inputs selection and including an extra input to cover the tx fee.
2024-07-10Merge bitcoin/bitcoin#30414: [doc] archive v26.2 release notesAva Chow
3c61cf3986d40ce0eae794f8a9571247f8af99e4 [doc] archive v26.2 release notes (glozow) Pull request description: To create the github release ACKs for top commit: achow101: ACK 3c61cf3986d40ce0eae794f8a9571247f8af99e4 stickies-v: ACK 3c61cf3986d40ce0eae794f8a9571247f8af99e4 Tree-SHA512: 70c316c68f73baae4abf4e5c8999620a7d7aa869a1dd51a4f72dc9093f24a52916249ee460648fe82bc6c1e5d568b9dca185b10b8faa86b499d84e30c65be3fa
2024-07-10Merge bitcoin/bitcoin#29668: prune, rpc: Check undo data when finding ↵Ava Chow
pruneheight 8789dc8f315a9d9ad7142d831bc9412f780248e7 doc: Add note to getblockfrompeer on missing undo data (Fabian Jahr) 4a1975008b602aeacdad9a74d1837a7455148074 rpc: Make pruneheight also reflect undo data presence (Fabian Jahr) 96b4facc912927305b06a233cb8b36e7e5964c08 refactor, blockstorage: Generalize GetFirstStoredBlock (Fabian Jahr) Pull request description: The function `GetFirstStoredBlock()` helps us find the first block for which we have data. So far this function only looked for a block with `BLOCK_HAVE_DATA`. However, this doesn't mean that we also have the undo data of that block, and undo data might be required for what a user would like to do with those blocks. One example of how this might happen is if some blocks were fetched using the `getblockfrompeer` RPC. Blocks fetched from a peer will have data but no undo data. The first commit here allows `GetFirstStoredBlock()` to check for undo data as well by passing a parameter. This alone is useful for #29553 and I would use it there. In the second commit I am applying the undo check to the RPCs that report `pruneheight` to the user. I find this much more intuitive because I think the user expects to be able to do all operations on blocks up until the `pruneheight` but that is not the case if undo data is missing. I personally ran into this once before and now again when testing for assumeutxo when I had used `getblockfrompeer`. The following commit adds test coverage for this change of behavior. The last commit adds a note in the docs of `getblockfrompeer` that undo data will not be available. ACKs for top commit: achow101: ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7 furszy: Code review ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7. stickies-v: ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7 Tree-SHA512: 90ae8bdd07a496ade579aa25240609c61c9ed173ad38d30533f6c631fe674e5a41727478ade69ca4b71a571ad94c9da4b33ebba6b5d8821109313c2de3bdfb3d
2024-07-10Merge bitcoin/bitcoin#29996: Assumeutxo: bugfix on loadtxoutset with a ↵Ava Chow
divergent chain + test 5b7f70ba2661a194a3c476b81e03785feddb4e1c test: loadtxoutset in divergent chain with less work (Alfonso Roman Zubeldia) d35efe1efc0dbeae0667baade2a40be08511c13e p2p: Start downloading historical blocks from common ancestor (Martin Zumsande) Pull request description: This PR adds a test to cover the scenario of loading an assumeutxo snapshot when the current chain tip is not an ancestor of the snapshot block but has less work. During the review process, a bug was discovered where blocks between the last common ancestor and the background tip were not being requested if the background tip was not an ancestor of the snapshot block. mzumsande suggested a fix (65343ec49a6b73c4197dfc38e1c2f433b0a3838a) to start downloading historical blocks from the last common ancestor to address this issue. This fix has been incorporated into the PR with a slight modification. Related to https://github.com/bitcoin/bitcoin/issues/28648 ACKs for top commit: fjahr: tACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c achow101: ACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c mzumsande: Code Review ACK 5b7f70ba2661a194a3c476b81e03785feddb4e1c Tree-SHA512: f8957349686a6a1292165ea9e0fd8c912d21466072632a10f8ef9d852a5f430bc6b2a531e6884a4dbf2e3adb28b3d512b25919e78f5804a67320ef54c3b1aaf6
2024-07-10Merge bitcoin/bitcoin#29274: Fix issues with CI on forksRyan Ofsky
576828e732bacb61b608cd89f669a2936d3e8d00 ci: test-each-commit merge base optional (Sjors Provoost) e9bfbb5414ab14ca14d8edcfdf77f28c9ed67c33 ci: forks can opt-out of CI branch push (Cirrus only) (Sjors Provoost) Pull request description: Maintainer note: `SKIP_BRANCH_PUSH=true` must be set in Cirrus for `bitcoin-core/gui` before merging this. See `https://cirrus-ci.com/github/bitcoin-core/gui` -> Settings. --- I find myself making pull requests against my fork (mostly on top of https://github.com/bitcoin/bitcoin/pull/28983, or asking others to do so. Currently only the Github actions are run on forks, because we use self-hosted runners for the Cirrus tasks. While setting up my own self-hosted runners for my fork, I ran into a number of issues. Some of those were addressed by https://github.com/bitcoin/bitcoin/pull/29441, but remaining issues are: 1. When PRs are opened in the fork, cirrus CI jobs are run twice because PRs and branches reside in the same repository, rather than a main repository and a fork repository, as is the case with bitcoin/bitcoin PRs. Fix this by adding a `SKIP_BRANCH_PUSH` configuration option that allows skipping CI runs not directly associated with a PR. The fix is a generalization of [#20328](https://github.com/bitcoin/bitcoin/pull/20328), which fixed a similar problem for the bitcoin-core/gui mirror repository, and it allows removing a hardcoded reference to that repository. Github actions jobs will still run twice despite this change, see [#29274 (comment)](https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2188840483). Initially this PR tried to prevent that with https://github.com/bitcoin/bitcoin/commit/b9fdd0dc75b5b4944dffc700b0391b38465f754a, but this had some potentially negative side effects, see [#29274 (comment)](https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457587805), so that commit was dropped for now. 2. When PRs are opened in the fork, the "test-each-commit" github action can fail due to not being able to find a recent merge commit. This problem doesn't happen in the bitcoin/bitcoin repository because branches in this repository used as the base for pull requests always point at merge commits. This PR replaces https://github.com/bitcoin/bitcoin/pull/29259 using the self hosted workers via Cirrus instead of Github. You can see this PR in action on this pull request to my fork: https://github.com/Sjors/bitcoin/pull/30 To test it yourself: 1. spin up at least two [self hosted runners](https://github.com/cirruslabs/cirrus-cli/blob/master/PERSISTENT-WORKERS.md). Either use a seperate VM for each, or give them their own user. 3. Install Podman and other CI dependencies (see .cirrus.yml) 4. Give Cirrus access to your fork at https://cirrus-ci.com/settings/github/YOU 5. Get a token from Cirrus and use it to start your worker(s) 6. Optionally set SKIP_BRANCH_PUSH=true ~and NO_ARM=true~ env variables (see .cirrus.yml) make a pull request to your own fork, with this PR as the base branch Security wise: when dealing with code from strangers on the internet, review it first before running the CI. There's a Cirrus check-box that requires approval for people without write access to trigger CI. ACKs for top commit: maflcko: ACK 576828e732bacb61b608cd89f669a2936d3e8d00 ryanofsky: Code review ACK 576828e732bacb61b608cd89f669a2936d3e8d00. Tree-SHA512: fb6be2f228aa62f45a65ce5c613c979b6f387df396f9601ce4622b27aa317a66f198e7d7a194592b0bb397b32a2f50f8be47065834d74af4ea09407c5c8d306d
2024-07-10util: Catch translation string errors at compile timeMarcoFalke
2024-07-09Merge bitcoin/bitcoin#29154: tests: improve wallet multisig descriptor test ↵Ava Chow
and docs d93b79470916b1e6f85c55cc6beb1e41b382196f tests: improve wallet multisig descriptor test and docs (Michael Dietz) Pull request description: It is best to store all key origin information (master key fingerprint and all derivation steps) in the multisig descriptor. Being explicit with this information should be beneficial if this approach is used with other wallets/signers (whether hardware or software). There is no harm including all of this with xpubs (if anything it simplifies the test code) and makes this example/docs more complete and safer incase it is referenced by others. ACKs for top commit: S3RK: Code Review ACK d93b79470916b1e6f85c55cc6beb1e41b382196f achow101: ACK d93b79470916b1e6f85c55cc6beb1e41b382196f Tree-SHA512: 0e5c4d13f060489405e6cf50c8a09911f5a0cee71023649235afd80a5e3aae38d52c6e12ad4660205b9357b09f45596941391bdcf6fceccbe07c4e5a1592a482
2024-07-09Merge bitcoin/bitcoin#30396: random: add benchmarks and drop unnecessary ↵Ava Chow
Shuffle function 6ecda04fefad980872c72fba89844393f5581120 random: drop ad-hoc Shuffle in favor of std::shuffle (Pieter Wuille) da28a26aae3178fb7663efbe20bb650857ace775 bench random: benchmark more functions, and add InsecureRandomContext (Pieter Wuille) 0a9bbc64c157a314e5472ecd98300e30b12d3fdf random bench refactor: move to new bench/random.cpp (Pieter Wuille) Pull request description: This adds benchmarks for various operations on `FastRandomContext` and `InsecureRandomContext`, and then removes the ad-hoc `Shuffle` functions, now that it appears that standard library `std::shuffle` has comparable performance. The other reason for keeping `Shuffle`, namely the fact that libstdc++ used self-move (which debug mode panics on) has been fixed as well (see https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658344049). ACKs for top commit: achow101: ACK 6ecda04fefad980872c72fba89844393f5581120 hodlinator: ACK 6ecda04fefad980872c72fba89844393f5581120 dergoegge: Code review ACK 6ecda04fefad980872c72fba89844393f5581120 Tree-SHA512: 2560b7312410581ff2b9bd0716e0f1558d910b5eadb9544785c972384985ac0f11f72d6b2797cfe2e7eb71fa57c30cffd98cc009cb4ee87a18b1524694211417
2024-07-09Merge bitcoin/bitcoin#29431: test/BIP324: disconnection scenarios during v2 ↵Ava Chow
handshake c9dacd958d7c1e98b08a7083c299d981e4c11193 test: Check that non empty version packet is ignored and no disconnection happens (stratospher) 997cc00b950a7d1b7f2a3971282685f4e81d87d2 test: Check that disconnection happens when AAD isn't filled (stratospher) b5e6238fdbba5c777a5adfa4477dac51a82f4448 test: Check that disconnection happens when garbage sent/received are different (stratospher) ad1482d5a20e6b155184a43d0724d2dcd950ce52 test: Check that disconnection happens when wrong garbage terminator is sent (stratospher) e351576862471fc77b1e798a16833439e23ff0b4 test: Check that disconnection happens when >4095 garbage bytes is sent (stratospher) e075fd131d668d9d1ba3c8566624481c4a57032d test: Introduce test types and modify v2 handshake function accordingly (stratospher) 7d07daa62311bdb0e2ce23d0b55f711f5088bd28 log: Add V2 handshake timeout (stratospher) d4a1da8543522a213ac75761131d878eedfd4a5b test: Make global TRANSPORT_VERSION variable an instance variable (stratospher) c642b08c4e45cb3a625a867ebd66c0ae51bde212 test: Log when the garbage is actually sent to transport layer (stratospher) 86cca2cba230c10324c6aedd12ae9655b83b2856 test: Support disconnect waiting for add_p2p_connection (stratospher) bf9669af9ccc33dcade09bceb27d6745e9d9a75a test: Rename early key response test and move random_bitflip to util (stratospher) Pull request description: Add tests for the following v2 handshake scenarios: 1. Disconnection happens when > `MAX_GARBAGE_LEN` bytes garbage is sent 2. Disconnection happens when incorrect garbage terminator is sent 3. Disconnection happens when garbage bytes are tampered with 4. Disconnection happens when AAD of first encrypted packet after the garbage terminator is not filled 5. bitcoind ignores non-empty version packet and no disconnection happens All these tests require a modified v2 P2P class (different from `EncryptedP2PState` used in `v2_p2p.py`) to implement our custom handshake behaviour based on different scenarios and have been kept in a single test file (`test/functional/p2p_v2_misbehaving.py`). Shifted the test in `test/functional/p2p_v2_earlykeyresponse.py` which is of the same pattern to this file too. ACKs for top commit: achow101: ACK c9dacd958d7c1e98b08a7083c299d981e4c11193 mzumsande: ACK c9dacd958d7c1e98b08a7083c299d981e4c11193 theStack: Code-review ACK c9dacd958d7c1e98b08a7083c299d981e4c11193 Tree-SHA512: 90df81f0c7f4ecf0a47762d290a618ded92cde9f83d3ef3cc70e1b005ecb16125ec39a9d80ce95f99e695d29abd63443240cb5490aa57c5bc8fa2e52149a0672
2024-07-09Merge bitcoin/bitcoin#30329: fuzz: improve utxo_snapshot targetRyan Ofsky
de71d4dece604907afc4fc26b7788e9c1a4cbecb fuzz: improve utxo_snapshot target (Martin Zumsande) Pull request description: Add the possibility of giving more guidance to the creation of the metadata and/or coins, so that the fuzzer gets the chance to reach more error conditions in ActivateSnapshot and sometimes successfully creates a valid snapshot. This also changes the asserts for the success case that were outdated (after #29370) and only didn't result in a crash because the fuzzer wasn't able to reach this code before. ACKs for top commit: maflcko: re-ACK de71d4dece604907afc4fc26b7788e9c1a4cbecb 🎆 fjahr: utACK de71d4dece604907afc4fc26b7788e9c1a4cbecb TheCharlatan: ACK de71d4dece604907afc4fc26b7788e9c1a4cbecb Tree-SHA512: 346974d594164544d8cd3df7d8362c905fd93116215e9f5df308dfdac55bab04d727bfd7fd001cf11318682d11ee329b4b4a43308124c04d64b67840ab8a58a0
2024-07-09Reapply "test: p2p: check that connecting to ourself leads to disconnect"Sebastian Falbesoner
This reverts commit 9ec2c53701a391629b55aeb2804e8060d2c453a4 with a tiny change included (identation of the wait_until call).
2024-07-09net: prevent sending messages in `NetEventsInterface::InitializeNode`Sebastian Falbesoner
Now that the queueing of the VERSION messages has been moved out of `InitializeNode`, there is no need to pass a mutable `CNode` reference any more. With a const reference, trying to send messages in this method would lead to a compile-time error, e.g.: ---------------------------------------------------------------------------------------------------------------------------------- ... net_processing.cpp: In member function ‘virtual void {anonymous}::PeerManagerImpl::InitializeNode(const CNode&, ServiceFlags)’: net_processing.cpp:1683:21: error: binding reference of type ‘CNode&’ to ‘const CNode’ discards qualifiers 1683 | PushNodeVersion(node, *peer); ... ----------------------------------------------------------------------------------------------------------------------------------
2024-07-09net: fix race condition in self-connect detectionSebastian Falbesoner
Initiating an outbound network connection currently involves the following steps after the socket connection is established (see `CConnman::OpenNetworkConnection` method): 1. set up node state 2. queue VERSION message 3. add new node to vector `m_nodes` If we connect to ourself, it can happen that the sent VERSION message (step 2) is received and processed locally *before* the node object is added to the connection manager's `m_nodes` vector (step 3). In this case, the self-connect remains undiscovered, as the detection doesn't find the outbound peer in `m_nodes` yet (see `CConnman::CheckIncomingNonce`). Fix this by swapping the order of 2. and 3., by taking the `PushNodeVersion` call out of `InitializeNode` and doing that in the `SendMessages` method instead, which is only called for `CNode` instances in `m_nodes`. Thanks go to vasild, mzumsande, dergoegge and sipa for suggestions on how to fix this.
2024-07-09Merge bitcoin/bitcoin#30395: rpc: Use untranslated error strings in loadtxoutsetRyan Ofsky
fa5b8920be041380fbfa4c7b443918637423d7a0 rpc: Use untranslated error strings in loadtxoutset (MarcoFalke) fa458657788cc142f14551d86604e3f434d56c0a refactor: Use named arguments to get path arg in loadtxoutset (MarcoFalke) Pull request description: Motivation: * Some are not translated at all, anyway. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663631973 * For others translation is not yet needed, because they are not called by the GUI (yet) * For others translations will never be needed, because they are RPC code. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663611194 Also, while touching this: * Remove the trailing `\n`. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663647981 * Add back the path. See https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663666751 * Use named args to get the path. ACKs for top commit: fjahr: re-ACK fa5b8920be041380fbfa4c7b443918637423d7a0 tdb3: ACK fa5b8920be041380fbfa4c7b443918637423d7a0 ryanofsky: Code review ACK fa5b8920be041380fbfa4c7b443918637423d7a0 Tree-SHA512: 46504dc5fd55a6274ef885dbe071aa9efb25bca247cd68cd86fb2ff066d70d295e0522e1fe42e63f1fdf7e4c89bd696220edaf06e33b804aba746492eafd852e
2024-07-09package rbf: cpfp structure requires package > parent feerateGreg Sanders
2024-07-09test package rbf boundary conditions more closelyGreg Sanders
2024-07-09fuzz: mini_miner_selection fixups.glozow
Delete asserts that are redundant with the == assert. Add assertion that the coinbase isn't already in mock_template_txids.
2024-07-09MiniMiner: use FeeFrac in AncestorFeerateComparatorglozow
Comparing using FeeFracs is more precise, allows us to simply the code since FeeFrac comparison internally does cross-multiplication, and avoids potential overflow in the multiplication. Previously, we were only comparing feerates up to 0.001sat/vB precision, since CFeeRate comparison just looks at their respective nSatoshisPerK. This could lead to MiniMiner selecting packages in the wrong order (i.e. by txid) if their feerates were less than 0.001sat/vB different.
2024-07-09[doc] archive v26.2 release notesglozow
2024-07-09Merge bitcoin/bitcoin#30393: refactor: use existing RNG object in ↵glozow
ProcessGetBlockData fa2e74879a3f7423c8d01aa1376b2bd9ccf5658d net_processing: use existing RNG object in ProcessGetBlockData (MarcoFalke) Pull request description: Small follow-up to commit 8e31cf9c9b5e9fdd01e8b220c08a3ccde5cf584c ACKs for top commit: dergoegge: Code review ACK fa2e74879a3f7423c8d01aa1376b2bd9ccf5658d glozow: ACK fa2e74879a3f7423c8d01aa1376b2bd9ccf5658d Tree-SHA512: 12709c79e6eefad184609b7306e0f65cb00123e39636cf8b7d538feb25c05ba3c36aa41468886c904a5f44fea267e67f9c4fbbab8733753d1c891b90fa40ce8b
2024-07-08refactor: Mark some static global vars as constTheCharlatan
These were found while looking for static mutable state in the kernel library.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 08:15:05 +0000