aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-07-15Merge bitcoin-core/gui#469: Load Base64 PSBT string from fileHennadii Stepanov
2c3ee4c347838ecadb17a011932dffc077e46630 gui: Load Base64 PSBT string from file (Andrew Chow) Pull request description: Some .psbt files may have the PSBT as a base64 string instead of in binary. We should be able to load those files. ACKs for top commit: jarolrod: tACK 2c3ee4c347838ecadb17a011932dffc077e46630 shaavan: ACK 2c3ee4c347838ecadb17a011932dffc077e46630 Tree-SHA512: 352b0611693c8989ea7d1b8d494ea58c69dc15cf81b8d62271541832e74b0a0399cb6ed4e686ab7c741cb4e5374527e054a9ecfe7355bc6f77d8fdd13569ab76
2022-07-15Merge bitcoin/bitcoin#25551: refactor: Throw exception on invalid Univalue ↵fanquake
pushes over silent ignore fa277cd55dd105018e7d1220b4c3d96779e6b0f4 univalue: Throw exception on invalid pushes over silent ignore (MacroFake) ccccc17b91698aa09ac85f7efea298f3938594ad refactor: Default options in walletcreatefundedpsbt to VOBJ instead of VNULL (MacroFake) Pull request description: The return value of the `push*` helpers is never used, but important to determine if the operation was successful. One way to fix this would be to add the "nodiscard" attribute. However, this would make the code (and this diff) overly verbose for no reason. So fix it by removing the never used return value. Also, fail verbosely in case of a programming mistake. ACKs for top commit: furszy: code ACK fa277cd5 Tree-SHA512: ef212a5bf5ae6bbad20acc4dafa3715521e81544185988d1eab724f440e4864a27e686aff51d5bc51b3017892c2eb8e577bcb8f37e8ddbaa0d8833bb622f2f9c
2022-07-14Merge bitcoin/bitcoin#24148: Miniscript support in Output DescriptorsAndrew Chow
ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 qa: functional test Miniscript watchonly support (Antoine Poinsot) bfb036756ad6e187fd6d3abfefe5804bb54a5c71 Miniscript support in output descriptors (Antoine Poinsot) 4a082887bee76a96deada5dbd7f991c23b301c54 qa: better error reporting on descriptor parsing error (Antoine Poinsot) d25d58bf5f301d3bb8683bd67c8847a4957d8e97 miniscript: add a helper to find the first insane sub with no child (Antoine Poinsot) c38c7c5817b7e73cf0f788855c4aba59c287b0ad miniscript: don't check for top level validity at parsing time (Antoine Poinsot) Pull request description: This adds Miniscript support for Output Descriptors without any signing logic (yet). See the OP of #24147 for a description of Miniscript and a rationale of having it in Bitcoin Core. On its own, this PR adds "watchonly" support for Miniscript descriptors in the descriptor wallet. A follow-up adds signing support. A minified corpus of Miniscript Descriptors for the `descriptor_parse` fuzz target is available at https://github.com/bitcoin-core/qa-assets/pull/92. The Miniscript descriptors used in the unit tests here and in #24149 were cross-tested against the Rust implementation at https://github.com/rust-bitcoin/rust-miniscript. This PR contains code and insights from Pieter Wuille. ACKs for top commit: Sjors: re-utACK ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 achow101: ACK ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/24148/commits/ffc79b8e492c6dd1352e528fd82e45d8d25eaa04 Tree-SHA512: 02d919d38bb626d3c557eca3680ce71117739fa161b7a92cfdb6c9c432ed88870b1ed127ba24248574c40c7428217d7e9bdd986fd8cd7c51fae8c776e1271fb9
2022-07-14Use designated initializers for ChainstateManager::OptionsCarl Dong
This wasn't available at the time when ChainstateManager::Options was introduced but is helpful to be explicit and ensure correctness.
2022-07-14Move ChainstateManagerOpts into kernel:: namespaceCarl Dong
It should have been there in the first place.
2022-07-14Miniscript support in output descriptorsAntoine Poinsot
Miniscript descriptors are defined under P2WSH context (either `wsh()` or `sh(wsh())`). Only sane Miniscripts are accepted, as insane ones (although valid by type) can have surprising behaviour with regard to malleability guarantees and resources limitations. As Miniscript descriptors are longer and more complex than "legacy" descriptors, care was taken in error reporting to help a user determine for what reason a provided Miniscript is insane. Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
2022-07-14qa: better error reporting on descriptor parsing errorAntoine Poinsot
A nit, but was helpful when writing unit tests for Miniscript parsing
2022-07-14miniscript: add a helper to find the first insane sub with no childAntoine Poinsot
This is helpful for finer grained descriptor parsing error: when there are multiple errors to report in a Miniscript descriptor start with the "smallest" fragments: the ones closer to be a leaf. Co-Authored-By: Pieter Wuille <pieter@wuille.net>
2022-07-14miniscript: don't check for top level validity at parsing timeAntoine Poinsot
Letting the caller perform the checks allows for finer-grained error reporting.
2022-07-14Merge bitcoin/bitcoin#25594: refactor: Return BResult from restoreWalletMacroFake
fa475e9c7977a952617738f2ee8cf600c07d4df8 refactor: Return BResult from restoreWallet (MacroFake) fa8de09edc9ec4e6d171df80f746174a0ec58afb Prepare BResult for non-copyable types (MacroFake) Pull request description: This avoids the `error` in-out param (and if `warnings` is added to `BResult`, it will avoid passing that in-out param as well). Also, as it is needed for this change, prepare `BResult` for non-copyable types. ACKs for top commit: w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/25594/commits/fa475e9c7977a952617738f2ee8cf600c07d4df8 ryanofsky: Code review ACK fa475e9c7977a952617738f2ee8cf600c07d4df8. Changes since last review were replacing auto with explicit type and splitting commits Tree-SHA512: 46350883572f13721ddd198f5dfb88d2fa58ebcbda416f74da3563ea15c920fb1e6ff30558526a4ac91c36c21e6afe27751a4e51b7b8bcbcbe805209f4e9014b
2022-07-14Merge bitcoin/bitcoin#25557: p2p: Eliminate atomic for ↵MacroFake
m_last_getheaders_timestamp 613e2211493ae2c78b71d1f4ea62661438d2cb73 test: remove unnecessary parens (Suhas Daftuar) e939cf2b7645c2b20a68cb6129f3aebfdf287d61 Remove atomic for m_last_getheaders_timestamp (Suhas Daftuar) Pull request description: Eliminate the unnecessary atomic guarding `m_last_getheaders_timestamp`, which is only accessed in a single thread (thanks to MarcoFalke for pointing this out). Also address a nit that came up in #25454. ACKs for top commit: MarcoFalke: review ACK 613e2211493ae2c78b71d1f4ea62661438d2cb73 vasild: ACK 613e2211493ae2c78b71d1f4ea62661438d2cb73 Tree-SHA512: 6d6c473735b450b8ad43aae5cf16ed419154d72f4a05c0a6ce6f26caecab9db2361041398b70bf9395611c107d50897f501fa5fdbabb2891144bbc2b479dfdad
2022-07-13univalue: Throw exception on invalid pushes over silent ignoreMacroFake
2022-07-13refactor: Default options in walletcreatefundedpsbt to VOBJ instead of VNULLMacroFake
This should not change behavior and makes the code consistent with other places.
2022-07-13Merge bitcoin/bitcoin#25472: build: Increase MS Visual Studio minimum versionfanquake
630c1711b47ce50805f4dd2883777a100f7e5339 refactor: Drop no longer needed `util/designator.h` (Hennadii Stepanov) 88ec5d40dcf5d9f95217b123b48203b2f334c0a1 build: Increase MS Visual Studio minimum version (Hennadii Stepanov) 555f9dd5d39b316bf404017401b5aedc23ec6226 rpc, refactor: Add `decodepsbt_outputs` (Hennadii Stepanov) 0c432cbbfa9e83a68e061b388745326e278369fb rpc, refactor: Add `decodepsbt_inputs` (Hennadii Stepanov) 01d95a3964267d243df00d9bcc93b28adcfe16d7 rpc, refactor: Add `getblock_prevout` (Hennadii Stepanov) Pull request description: Visual Studio 2022 with `/std:c++20` supports [designated initializers](https://github.com/bitcoin/bitcoin/pull/24531). ACKs for top commit: sipsorcery: reACK 630c1711b47ce50805f4dd2883777a100f7e5339. Tree-SHA512: 5b8933940dd69061c6b077512168bebb6fea05d429b63ffbab191950798b4c825e8484b1a651db0ae13f97eae481097d3c16395659c0f3b9f847af2aaf44b65d
2022-07-13Merge bitcoin/bitcoin#25464: rpc: Reduce Univalue push_backV peak memory ↵fanquake
usage in listtransactions fa8a1c06961f4b1826696e0db8dce81dce627721 rpc: Fix Univalue push_backV OOM in listtransactions (MacroFake) Pull request description: Related to, but not intended as a fix for #25229. Currently the RPC will have the same data stored thrice: * `UniValue ret` (memory filled by `ListTransactions`) * `std::vector<UniValue> vec` (constructed by calling `push_backV`) * `UniValue result` (the actual result, memory filled by `push_backV`) Fix this by filling the memory only once: * `std::vector<UniValue> ret` (memory filled by `ListTransactions`) * Pass iterators to `push_backV` instead of creating a full copy * Move memory into `UniValue result` instead of copying it ACKs for top commit: shaavan: Code Review ACK fa8a1c06961f4b1826696e0db8dce81dce627721 Tree-SHA512: 1c3ca40fc8497134a4141195160e4aa9fe72b3c00c5998c972b58ad0eb498ebea05013f9105bb80e7264c9db1d0e7a2032396a8a4af1f326d831fbee20f32ea3
2022-07-12Remove atomic for m_last_getheaders_timestampSuhas Daftuar
This variable is only used in a single thread, so no atomic or mutex is necessary to guard it.
2022-07-12refactor: Return BResult from restoreWalletMacroFake
2022-07-12Prepare BResult for non-copyable typesMacroFake
2022-07-12Merge bitcoin/bitcoin#25324: refactor: add most of src/util to iwyuMacroFake
07f2c25d04c39a0074e1d9ee1b24b3e359c8153f refactor: add most of src/util to iwyu (fanquake) Pull request description: These files change infrequently, and not much header shuffling is required. We don't add everything in src/util/ yet, because IWYU makes some dubious suggestions, which I'm going to follow up with upstream. Soon we'll swap `src/util/xyz.cpp` for just `src/util/`. ACKs for top commit: hebasto: ACK 07f2c25d04c39a0074e1d9ee1b24b3e359c8153f, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 07d76435c2bff1a62c4967eb0efaafe619cc3bbaf4166741d8520927b24336c01aee59822f8082ee2a01e15046a0f5d506b4b23a6e40ceb750f3226ed8167847
2022-07-12Merge bitcoin/bitcoin#24944: rpc: add getblockfrompeer RPCTypeCheck and ↵MacroFake
invalid input test coverage 2ef5294a5bb68ceb3797d2638567a172cc21699f rpc: add RPCTypeCheck for getblockfrompeer inputs (Jon Atack) 734b9669ff7b2f5e2820993443a6f868f6b0b20a test: add getblockfrompeer coverage of invalid inputs (Jon Atack) Pull request description: The new getblockfrompeer RPC lacks test coverage for invalid arguments, and its error messages are not harmonized with the existing RPCs. Fix all issues. ACKs for top commit: brunoerg: ACK 2ef5294a5bb68ceb3797d2638567a172cc21699f Tree-SHA512: 454782cf6a44fd0e05483bb152153667ef5c8021358385ddcf89724fbbbd35e187362bdff757e00c99319527bc4c0b20c7187f67241d4585d767a29787142f25
2022-07-12Merge bitcoin/bitcoin#25489: wallet: change `ScanForWalletTransactions` to ↵MacroFake
use `Ticks(Dur2 d)` c6c35db0571c2f2bdd34febbfe9acd52a42d7b95 wallet: change `ScanForWalletTransactions` to use `Ticks()` (w0xlt) Pull request description: This PR changes `ScanForWalletTransactions()` to use the `Ticks(Dur2 d)` function (introduced in #25456). ACKs for top commit: MarcoFalke: cr ACK c6c35db0571c2f2bdd34febbfe9acd52a42d7b95 Tree-SHA512: 864e136b470baf22293dc03ae3400bbb34955389a1efc83862f006cfac84da9128c3a201ef051606c06f782a1fde84129261dd4b417cbfff854d5c359a92703e
2022-07-12Merge bitcoin/bitcoin#25591: move-only: Version handshake to libtest_utilMacroFake
fa4be8e7c324835d3b9eecb5a3825a4c8f77fb2f move-only: InitializeNode to handshake helper (MacroFake) fa7098947ceff1964812d430e769af6d1ad561bd move-only: Version handshake to libtest_util (MacroFake) Pull request description: The version handshake after setting up a peer is an integral part of (unit) testing net processing logic. Thus, make the helper accessible in libtest_util. Also, remove the peerman argument from `FillNode`, as it must be equal to connman's peerman, which can then be used instead. ACKs for top commit: dergoegge: ACK fa4be8e7c324835d3b9eecb5a3825a4c8f77fb2f Tree-SHA512: 8296399dc2c29196bd56584c9b61f1c5a088f96dd3438b07b84e1acf525d867f1e37fdfdeede8a831add25848cda0c221ce3fb873e5ae5ca805a1765aa08eb12
2022-07-12wallet: change `ScanForWalletTransactions` to use `Ticks()`w0xlt
2022-07-12Merge bitcoin/bitcoin#25218: refactor: introduce generic 'Result' class and ↵MacroFake
connect it to CreateTransaction and GetNewDestination 111ea3ab711414236f8678566a7884d48619b2d8 wallet: refactor GetNewDestination, use BResult (furszy) 22351725bc4c5eb63ee45f607374bbf2d76e2b8c send: refactor CreateTransaction flow to return a BResult<CTransactionRef> (furszy) 198fcca162f4d2f877feab485e629ff89818ff56 wallet: refactor, include 'FeeCalculation' inside 'CreatedTransactionResult' (furszy) 7a45c33d1f8a758850cf8e7bd6ad508939ba5c0d Introduce generic 'Result' class (furszy) Pull request description: Based on a common function signature pattern that we have all around the sources: ```cpp bool doSomething(arg1, arg2, arg3, arg4, &result_obj, &error_string) { // do something... if (error) { error_string = "something bad happened"; return false; } result = goodResult; return true; } ``` Introduced a generic class `BResult` that encapsulate the function boolean result, the result object (in case of having it) and, in case of failure, the string error reason. Obtaining in this way cleaner function signatures and removing boilerplate code: ```cpp BResult<Obj> doSomething(arg1, arg2, arg3, arg4) { // do something... if (error) return "something bad happened"; return goodResult; } ``` Same cleanup applies equally to the function callers' side as well. There is no longer need to add the error string and the result object declarations before calling the function: Before: ```cpp Obj result_obj; std::string error_string; if (!doSomething(arg1, arg2, arg3, arg4, result_obj, error_string)) { LogPrintf("Error: %s", error_string); } return result_obj; ``` Now: ```cpp BResult<Obj> op_res = doSomething(arg1, arg2, arg3, arg4); if (!op_res) { LogPrintf("Error: %s", op_res.GetError()); } return op_res.GetObjResult(); ``` ### Initial Implementation: Have connected this new concept to two different flows for now: 1) The `CreateTransaction` flow. --> 7ba2b87c 2) The `GetNewDestination` flow. --> bcee0912 Happy note: even when introduced a new class into the sources, the amount of lines removed is almost equal to added ones :). Extra note: this work is an extended version (and a decoupling) of the work that is inside #24845 (which does not contain the `GetNewDestination` changes nor the inclusion of the `FeeCalculation` field inside `CreatedTransactionResult`). ACKs for top commit: achow101: ACK 111ea3ab711414236f8678566a7884d48619b2d8 w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/25218/commits/111ea3ab711414236f8678566a7884d48619b2d8 theStack: re-ACK 111ea3ab711414236f8678566a7884d48619b2d8 MarcoFalke: review ACK 111ea3ab711414236f8678566a7884d48619b2d8 🎏 Tree-SHA512: 6d84d901a4cb923727067f25ff64542a40edd1ea84fdeac092312ac684c34e3688a52ac5eb012717d2b73f4cb742b9d78e458eb0e9cb9d6d72a916395be91f69
2022-07-12Merge bitcoin-core/gui#627: Apply translator comments to reset options ↵Hennadii Stepanov
confirmation dialog d5c141f221d67aaeee989da0db0ac5383d7562d3 qt: apply translator comments to reset options confirmation dialog (Jarol Rodriguez) Pull request description: This is a followup to #617. Because the strings were being concatenated, we can not apply translator comments to all of the revelant strings. This can be tested by applying the following diff to current master and running `make translate`; then check the resulting diff: ```diff diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 462b923d6..3cf165004 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -286,9 +286,17 @@ void OptionsDialog::on_resetButton_clicked() { if (model) { // confirmation dialog + //: Window title text of pop-up window shown when the user has chosen to reset options. QMessageBox::StandardButton btnRetVal = QMessageBox::question(this, tr("Confirm options reset"), + /*: Text explaining that the settings the user changed will not come + into effect until the client is restarted. */ tr("Client restart required to activate changes.") + "<br><br>" + + /*: Text explaining to the user that the client's current settings + will be backed up at a specific location. %1 is a stand-in + argument for the backup location's path. */ tr("Current settings will be backed up at \"%1\".").arg(m_client_model->dataDir()) + "<br><br>" + + /*: Text asking the user to confirm if they would like to proceed + with a client shutdown. */ tr("Client will be shut down. Do you want to proceed?"), QMessageBox::Yes | QMessageBox::Cancel, QMessageBox::Cancel); ``` To apply the above translator comments, what we want to do instead is have a variable in which the translatable strings are appended to using the [QString append function](https://doc.qt.io/qt-5/qstring.html#append). When you run `make translate` with this PR, you will see the translator comments properly applied, as shown below: ``` diff diff --git a/src/qt/locale/bitcoin_en.ts b/src/qt/locale/bitcoin_en.ts index 35d820187..9e5158b3e 100644 --- a/src/qt/locale/bitcoin_en.ts +++ b/src/qt/locale/bitcoin_en.ts @@ -1942,28 +1942,37 @@ Signing is only possible with addresses of the type &apos;legacy&apos;.</source> <translation>default</translation> </message> <message> - <location line="+81"/> + <location line="+86"/> <source>none</source> <translation type="unfinished"></translation> </message> <message> - <location line="+97"/> + <location line="+107"/> <source>Confirm options reset</source> + <extracomment>Window title text of pop-up window shown when the user has chosen to reset options.</extracomment> <translation>Confirm options reset</translation> </message> <message> - <location line="+1"/> - <location line="+70"/> + <location line="-9"/> + <location line="+79"/> <source>Client restart required to activate changes.</source> + <extracomment>Text explaining that the settings changed will not come into effect until the client is restarted.</extracomment> + <translation type="unfinished"></translation> + </message> + <message> + <location line="-75"/> + <source>Current settings will be backed up at &quot;%1&quot;.</source> + <extracomment>Text explaining to the user that the client&apos;s current settings will be backed up at a specific location. %1 is a stand-in argument for the backup location&apos;s path.</extracomment> <translation type="unfinished"></translation> </message> <message> - <location line="-70"/> + <location line="+3"/> <source>Client will be shut down. Do you want to proceed?</source> + <extracomment>Text asking the user to confirm if they would like to proceed with a client shutdown.</extracomment> <translation type="unfinished"></translation> </message> <message> - <location line="+18"/> + <location line="+20"/> <source>Configuration options</source> <extracomment>Window title text of pop-up box that allows opening up of configuration file.</extracomment> <translation type="unfinished"></translation> ``` No difference in rendering between master and PR | master | PR | | ------- | --- | <img width="532" alt="Screen Shot 2022-06-29 at 11 39 17 PM" src="https://user-images.githubusercontent.com/23396902/176588495-9d3761b6-9d96-489a-bbe5-a8907f7d5f99.png"> | <img width="532" alt="Screen Shot 2022-06-29 at 11 39 51 PM" src="https://user-images.githubusercontent.com/23396902/176588513-92e29564-b74a-46f5-a5dd-469c4ee953f7.png"> | ACKs for top commit: shaavan: ACK d5c141f221d67aaeee989da0db0ac5383d7562d3 furszy: Tested ACK d5c141f2, no functional changes. w0xlt: tACK https://github.com/bitcoin-core/gui/pull/627/commits/d5c141f221d67aaeee989da0db0ac5383d7562d3 Tree-SHA512: 6175a096c6f99edb3041cc2429e1ea0670a10cd2cab0364f664a56c7dee1aa8631d52c0a36edb5d571f6ef934e947d45017e446cea7dddae044085c39c8835ef
2022-07-12move-only: InitializeNode to handshake helperMacroFake
2022-07-12move-only: Version handshake to libtest_utilMacroFake
2022-07-12Merge bitcoin/bitcoin#25036: wallet: Save wallet scan progressMacroFake
230a2f4cc3fab9f66b6c24ba809ddbea77755cb7 wallet test: Add unit test for wallet scan save_progress option (Ryan Ofsky) a89ddfbe22b6db5beda678c9493e08fec6144122 wallet: Save wallet scan progress (w0xlt) Pull request description: Currently, the wallet scan progress is not saved. If it is interrupted, it will be necessary to start from scratch on the next load. This PR changes this and the progress is saved right after checking a block. Close https://github.com/bitcoin/bitcoin/issues/25010 ACKs for top commit: furszy: re-ACK 230a2f4 achow101: ACK 230a2f4cc3fab9f66b6c24ba809ddbea77755cb7 ryanofsky: Code review ACK 230a2f4cc3fab9f66b6c24ba809ddbea77755cb7. Only change since last review is tweaking whitespace and adding log print Tree-SHA512: 1a9dec207ed22b3443fb06a4daf967637bc02bcaf71c070b7dc33605d0cab959551e4014c9e92293a63f54c5cbcc98bb9f8844a8c60bc32a1482b1c4130fab32
2022-07-11Merge bitcoin/bitcoin#25562: test: add tests for negative waste during coin ↵Andrew Chow
selection 98ea43d5e9fc7b001f55a5bb1602afc2661cdb0f test: add tests for negative waste during coin selection (ishaanam) Pull request description: #25495 mentions that waste can be negative when the current feerate is less than the long term feerate. There are currently no waste tests for negative waste, so this PR adds two of them. ACKs for top commit: achow101: ACK 98ea43d5e9fc7b001f55a5bb1602afc2661cdb0f glozow: light code review ACK 98ea43d5e9fc7b001f55a5bb1602afc2661cdb0f, good to have tests for negative waste Tree-SHA512: d194d370f1257975959d3c601fea9f82c30c1aabc3e8bedc997c62659283fe681cc527e59df1a0187b3c91e8067c60374dd5ce0237561bd882edafe6a575a9b9
2022-07-10Merge bitcoin-core/gui#471: Add Wallet Restore in the GUIHennadii Stepanov
bc13ec888cdc2791f79eeb6eb76b9134d670043e doc: Add a release note about the "restore wallet" menu item (w0xlt) e7a3f698b5f3f7dde8632c4911abd4e5bc1f06cb gui: Add Wallet Restore in the GUI (w0xlt) Pull request description: This PR adds a menu item to restore a wallet from a backup file in the GUI. Currently this option exists only in RPC interface. Motivation: It makes easier for non-technical users to restore backups. Master | PR | --- | --- <img width="307" alt="master" src="https://user-images.githubusercontent.com/94266259/141673349-0bf8a237-ecec-42e4-a0d7-1d5863940036.png"> | <img width="307" alt="pr" src="https://user-images.githubusercontent.com/94266259/141673350-972dea23-ae56-4283-a365-819da62b7067.png"> | ACKs for top commit: w0xlt: Added a release note in a new commit (https://github.com/bitcoin-core/gui/pull/471/commits/bc13ec888cdc2791f79eeb6eb76b9134d670043e) to not invalidate the ACKs for the previous one. furszy: utACK bc13ec8 shaavan: ACK bc13ec888cdc2791f79eeb6eb76b9134d670043e hebasto: ACK bc13ec888cdc2791f79eeb6eb76b9134d670043e Tree-SHA512: edc3675484238857b77e74382a4041dd5d2cbcda1e2d5bfe52c83d9d7bb7be8a243ecd97e25e994d8c30ab6d7c59ead5a1c953a46dce173666b137eeffc3c94f
2022-07-08Merge bitcoin/bitcoin#25481: wallet: unify max signature logicAndrew Chow
d54c5c8b1b1a38b5b38e6878aea0fa8d6c1ad7e9 wallet: use CCoinControl to estimate signature size (S3RK) a94659c84ee10ac5915eb5a6b654435183d88521 wallet: replace GetTxSpendSize with CalculateMaximumSignedInputSize (S3RK) Pull request description: Currently `DummySignTx` and `DummySignInput` use different ways to determine signature size. This PR unifies the way wallet estimates signature size for various inputs. Instead of passing boolean flags from calling code the `use_max_sig` is now calculated at the place of signature creation using information available in `CCoinControl` ACKs for top commit: achow101: ACK d54c5c8b1b1a38b5b38e6878aea0fa8d6c1ad7e9 theStack: Code-review ACK d54c5c8b1b1a38b5b38e6878aea0fa8d6c1ad7e9 Tree-SHA512: e790903ad4683067070aa7dbf7434a1bd142282a5bc425112e64d88d27559f1a2cd60c68d6022feaf6b845237035cb18ece10f6243d719ba28173b69bd99110a
2022-07-08wallet: refactor GetNewDestination, use BResultfurszy
2022-07-08send: refactor CreateTransaction flow to return a BResult<CTransactionRef>furszy
2022-07-08wallet: refactor, include 'FeeCalculation' inside 'CreatedTransactionResult'furszy
2022-07-08Introduce generic 'Result' classfurszy
Useful to encapsulate the function result object (in case of having it) or, in case of failure, the failure reason. This let us clean lot of boilerplate code, as now instead of returning a boolean and having to add a ref arg for the return object and another ref for the error string. We can simply return a 'BResult<Obj>'. Example of what we currently have: ``` bool doSomething(arg1, arg2, arg3, arg4, &result, &error_string) { do something... if (error) { error_string = "something bad happened"; return false; } result = goodResult; return true; } ``` Example of what we will get with this commit: ``` BResult<Obj> doSomething(arg1, arg2, arg3, arg4) { do something... if (error) return {"something happened"}; // good return {goodResult}; } ``` This allows a similar boilerplate cleanup on the function callers side as well. They don't have to add the extra pre-function-call error string and result object declarations to pass the references to the function.
2022-07-08Merge bitcoin/bitcoin#25337: refactor: encapsulate wallet's address book accessAndrew Chow
d69045e291e32e02d105d1b5ff1c8b86db0ae69e test: add coverage for 'listreceivedbyaddress' no change addrs return (furszy) 324f00a6420bbd64c67c264e50632e6fa36ae732 refactor: 'ListReceived' use optional for filtered address (furszy) b459fc122feace9e9a738c48aab21961cf15dddc refactor: RPC 'ListReceived', encapsulate m_address_book access (furszy) fa9f2ab8fd53075d2a3ec93ddac4908e73525c46 refactor: RPC 'listlabels', encapsulate 'CWallet::ListAddrBookLabels' functionality (furszy) 83e42c4b94e376a19d3eb0a2379769b8b8ac5fc8 refactor: use 'ForEachAddrBookEntry' in RPC 'getaddressesbylabel' (furszy) 2b48642499016cb357e4bcec32481cd50361194e refactor: use ForEachAddrBookEntry in interfaces::getAddresses (furszy) 032842ae4196aaed5ea3567ea01a61ed75ab2edd wallet: implement ForEachAddrBookEntry method (furszy) 09649bc95d5f2855a54a8cf02e65215a3b333c92 refactor: implement general 'ListAddrBookAddresses' for addressbook destinations lookup (furszy) 192eb1e61c3c43baec7f32c498ab0ce0656a58f7 refactor: getAddress don't access m_address_book, use FindAddressEntry function (furszy) Pull request description: ### Context The wallet's `m_address_book` field is being accessed directly from several places across the sources. ### Problem Code structure wise, we shouldn't be accessing it directly. It could end up being modified by mistake (from a place that has nothing to do with the wallet like an RPC command or the GUI) and cause a bigger issue: like an address book entry 'purpose' string change, which if done badly (from 'send' to 'receive'), could end up in a user sharing a "receive" address that he/she doesn't own. ### Solution Encapsulate `m_address_book` access inside the wallet. ------------------------------------------------------- Extra Note: This is the initial step towards decoupling the address book functionality from the wallet's sources. In other words, the creation of the `AddressBookManager` (which will be coming in a follow-up PR). ACKs for top commit: achow101: ACK d69045e291e32e02d105d1b5ff1c8b86db0ae69e theStack: ACK d69045e291e32e02d105d1b5ff1c8b86db0ae69e ✅ w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25337/commits/d69045e291e32e02d105d1b5ff1c8b86db0ae69e Tree-SHA512: dba17acd86f171b4e9af0223bbbcad380048570f6a2f6a92732a51f01abe8806debaf65c9e9e5569fa76a541903cbb50adcb5f56ef77858151c698ae6b218e2a
2022-07-08refactor: add most of src/util to iwyufanquake
These files change infrequently, and not much header shuffling is required. We don't add everything in src/util/ yet, because IWYU makes some dubious suggestions, which I'm going to follow up with upstream.
2022-07-08Merge bitcoin/bitcoin#25353: Add a `-mempoolfullrbf` node settingMacroFake
4c9666bd73645b94ae81be1faf7a0b8237dd6e99 Mention `mempoolfullrbf` in policy/mempool-replacements.md (Antoine Riard) aae66ab43d794bdfaa2dade91760cc55861c9693 Update getmempoolinfo RPC with `mempoolfullrbf` (Antoine Riard) 3e27e317270fdc2dd02794fea9da016387699636 Introduce `mempoolfullrbf` node setting. (Antoine Riard) Pull request description: This is ready for review. Recent discussions among LN devs have brought back on the surface concerns about the security of multi-party funded transactions against pinnings attacks and other mempool-based nuisances. The lack of full-rbf transaction-relay topology connected to miners open the way to cheap and naive DoS against multi-party funded transactions (e.g coinjoins, dual-funded channels, on-chain DLCs, ...) without solutions introducing an overhead cost or centralization vectors afaik . For more details, see [0]. This PR implements a simple `fullrbf` setting, where the node always allows transaction replacement, ignoring BIP125 opt-in flag. The default value of the setting stays **false**, therefore opt-in replacement is still the default Bitcoin Core replacement policy. Contrary to a previous proposal of mine and listening to feedbacks collected since then [1], I think this new setting simply offers more flexibility in a node transaction-relay policy suiting one's application requirements, without arguing a change of the default behavior. I [posted](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-June/020557.html) on the ML to invite operators with a bitcoin application sensitive to full-rbf (e.g dual-funded LN channels service providers) or mempool researchers to join a bootstrapped full-rbf activated peers network for experimentation and learning. If people have strong opinions against the existence of such full-rbf transaction-relay network, I'm proposing to express them on the future thread. [0] https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-May/003033.html [1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-June/019074.html Follow-up suggestions : - soft-enable opt-in RBF in the wallet : https://github.com/bitcoin/bitcoin/pull/25353#issuecomment-1154918789 - p2p discovery and additional outbound connection to full-rbf peers : https://github.com/bitcoin/bitcoin/pull/25353#issuecomment-1156044401 - match the code between RPC, wallet and mempool about disregard of inherited signaling : #22698 ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/25353/commits/4c9666bd73645b94ae81be1faf7a0b8237dd6e99 glozow: ACK 4c9666bd73645b94ae81be1faf7a0b8237dd6e99, a few nits which are non-blocking. w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25353/commits/4c9666bd73645b94ae81be1faf7a0b8237dd6e99 Tree-SHA512: 9e288bf22e06a9808804e58178444ef1830c3fdd42fd8a7cd7ffb101f8f586e08b000679be407d63ca76a56f7216227b368ff630c81f3fac3243db1a1202ab1c
2022-07-07refactor: Drop no longer needed `util/designator.h`Hennadii Stepanov
2022-07-07rpc, refactor: Add `decodepsbt_outputs`Hennadii Stepanov
This change eliminates memory usage spike when compiling with Visual Studio 2022 (at least in Cirrus CI environment). Easy to review using `git diff --color-moved-ws=allow-indentation-change --color-moved=dimmed-zebra`
2022-07-07rpc, refactor: Add `decodepsbt_inputs`Hennadii Stepanov
This change eliminates memory usage spike when compiling with Visual Studio 2022 (at least in Cirrus CI environment). Easy to review using `git diff --color-moved-ws=allow-indentation-change --color-moved=dimmed-zebra`
2022-07-07rpc, refactor: Add `getblock_prevout`Hennadii Stepanov
This change eliminates memory usage spike when compiling with Visual Studio 2022 (at least in Cirrus CI environment). Easy to review using `git diff --color-moved-ws=allow-indentation-change --color-moved=dimmed-zebra`
2022-07-07Merge bitcoin/bitcoin#24832: index: Verify the block filter hash when ↵fanquake
reading the filter from disk. e734228d8585c0870c71ce8ba8c037f8cf8b249a Update GCSFilter benchmarks (Calvin Kim) aee9a8140b3a58b744766f9e89572f1d953a808b Add GCSFilterDecodeSkipCheck benchmark (Patrick Strateman) 299023c1d9962628d158fac0306f8531506a0123 Add GCSFilterDecode and GCSBlockFilterGetHash benchmarks. (Patrick Strateman) b0a53d50d9142bed51a8372eeb848816bfa94da8 Make sanity check in GCSFilter constructor optional (Patrick Strateman) Pull request description: This PR picks up the abandoned #19280 BlockFilterIndex was depending on `GolombRiceDecode()` during the filter decode to sanity check that the filter wasn't corrupt. However, we can check for corruption by ensuring that the encoded blockfilter's hash matches up with the one stored in the index database. Benchmarks that were added in #19280 showed that checking the hash is much faster. The benchmarks were changed to nanobench and the relevant benchmarks were like below, showing a clear win for the hash check method. ``` | ns/elem | elem/s | err% | ins/elem | bra/elem | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|---------------:|--------:|----------:|:---------- | 531.40 | 1,881,819.43 | 0.3% | 3,527.01 | 411.00 | 0.2% | 0.01 | `DecodeCheckedGCSFilter` | 258,220.50 | 3,872.66 | 0.1% | 2,990,092.00 | 586,706.00 | 1.7% | 0.01 | `DecodeGCSFilter` | 13,036.77 | 76,706.09 | 0.3% | 64,238.24 | 513.04 | 0.2% | 0.01 | `BlockFilterGetHash` ``` ACKs for top commit: mzumsande: Code Review ACK e734228d8585c0870c71ce8ba8c037f8cf8b249a theStack: Code-review ACK e734228d8585c0870c71ce8ba8c037f8cf8b249a stickies-v: ACK e734228d8585c0870c71ce8ba8c037f8cf8b249a ryanofsky: Code review ACK e734228d8585c0870c71ce8ba8c037f8cf8b249a, with caveat that I mostly paid attention to the main code, not the changes to the benchmark. Only changes since last review were changes to the benchmark code. Tree-SHA512: 02b86eab7b554e1a57a15b17a4d6d71faa91b556c637b0da29f0c9ee76597a110be8e3b4d0c158d4cab04af0623de18b764837be0ec2a72afcfe1ad9c78a83c6
2022-07-07Merge bitcoin/bitcoin#25500: refactor: Move inbound eviction logic to its ↵fanquake
own translation unit 0101d2bc3c3bcf698d6cc2a237a680fc52395987 [net] Move eviction logic to its own file (dergoegge) c741d748d4d9836940b99091cc7be09c65efcb79 [net] Move ConnectionType to its own file (Cory Fields) a3c27070396ab8c2941c437e8099547e8fc9c110 [net] Add connection type to NodeEvictionCandidate (dergoegge) 42aa5d5b6269d27af525d5001907558442e96023 [net] Add NoBan status to NodeEvictionCandidate (dergoegge) Pull request description: This PR splits of the first couple commits from #25268 that move the inbound eviction logic from `net.{h,cpp}` to `eviction.{h,cpp}`. Please look at #25268 for motivation and conceptual review. ACKs for top commit: jnewbery: utACK 0101d2bc3c3bcf698d6cc2a237a680fc52395987 theuni: utACK 0101d2bc3c3bcf698d6cc2a237a680fc52395987. I quickly verified with `git --color-moved` that the move-only changes are indeed move-only. Tree-SHA512: e0c345a698030e049cb22fe281b44503c04403c5be5a3750ca14bfcc603a162ac6bac9a39552472feb57c460102b7ca91430b8ad6268f2efccc49b5e8959331b
2022-07-07Merge bitcoin/bitcoin#25447: fuzz: add low-level target for txorphanageMacroFake
6eb0909cb7d5883a258f76ad6cf2c989fc6f892f fuzz: add low-level target for txorphanage (chinggg) Pull request description: This adds a low-level fuzz target for orphan transaction handling by creating random transactions and calling all functions in `TxOrphanage`. It cannot simulate real-world `orphan/unorphan` scenarios effectively since it does not maintain any state about the node and the chain. A high-level fuzz target which construct well-designed transaction graphs will be added later. ACKs for top commit: MarcoFalke: review ACK 6eb0909cb7d5883a258f76ad6cf2c989fc6f892f 🐈 Tree-SHA512: b4d64f5941df77d13981f75ec170cef6ffabe782797c982ede7f34134be01dc0026dd7c0bee614bc1d64715e90a933d2a8c95974d402e32eaba8e24cc928299e
2022-07-07test: add tests for negative waste during coin selectionishaanam
2022-07-06Update getmempoolinfo RPC with `mempoolfullrbf`Antoine Riard
2022-07-06Introduce `mempoolfullrbf` node setting.Antoine Riard
This new node policy setting enables to accept replaced-by-fee transaction without inspection of the replaceability signaling as described in BIP125 "explicit signaling". If turns on, the node mempool accepts transaction replacement as described in `policy/mempool-replacements.md`. The default setting value is `false`, implying opt-in RBF is enforced.
2022-07-06[net] Move eviction logic to its own filedergoegge
2022-07-06[net] Move ConnectionType to its own fileCory Fields
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-19 08:48:04 +0000