aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-08-11[coin selection] consolidate m_change_target and m_min_change_targetglozow
These values are both intended for the same thing. Their divergence seems to be the result of an incomplete rename.
2022-08-11script/sign: remove needless IsSolvable() utilityAntoine Poinsot
It was used back when we didn't have a concept of descriptor. Now we can check for solvability using descriptors.
2022-08-11Merge bitcoin/bitcoin#25820: [test] make tx6 child of tx5, not tx3, in rbf_testsfanquake
49db42cdf56be1a76ab381d37870aa45e17ab666 [test] make tx6 child of tx5, not tx3, in rbf_tests (glozow) Pull request description: A small overlooked oopsie from #25674. There is no effect on the test results because tx3 and tx5 pay the same fee, but this was the intended configuration, as the comment suggests. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/25820/commits/49db42cdf56be1a76ab381d37870aa45e17ab666 darosior: Github diff ACK 49db42cdf56be1a76ab381d37870aa45e17ab666. Should have catched this. :/ Tree-SHA512: 2f54337ac3edc38707115cde5b466a85b8a6ac0a0a507effa0e9fecb12c9be196ecd1b16702bc23ba617cfb6a3b5db27d3b71616b3c2dadb186c699c4609831e
2022-08-11[test] make tx6 child of tx5, not tx3, in rbf_testsglozow
There is no effect on the test results because tx3 and tx5 pay the say fee, but this was the intended configuration, as the comment suggests.
2022-08-11Merge bitcoin/bitcoin#25812: psbt: Avoid unsigned int overflow in ↵fanquake
PSBT_IN_TAP_BIP32_DERIVATION 70a55c059b014c7a687de7a4813a90c65148aed4 psbt: Avoid unsigned int overflow in PSBT_IN_TAP_BIP32_DERIVATION (Andrew Chow) Pull request description: Fixes #25749 ACKs for top commit: instagibbs: ACK 70a55c059b014c7a687de7a4813a90c65148aed4 darosior: re-utACK 70a55c059b014c7a687de7a4813a90c65148aed4 jonatack: Review ACK 70a55c059b014c7a687de7a4813a90c65148aed4, this should avoid the issue reported in https://github.com/bitcoin/bitcoin/issues/25749 Tree-SHA512: 6bb58e1cda9a5baa50fcd24f818b5b27ed94f0d33da3f71f6e457618176611bf2a84e1864e9a48d9303c301252bc4c1dee8b19a67dd713e849fb9442851ca341
2022-08-11Merge bitcoin/bitcoin#25816: msvc: Drop ↵MacroFake
`_SILENCE_CXX17_OLD_ALLOCATOR_MEMBERS_DEPRECATION_WARNING` 9f9339c69206d81d355345ce66c1e23dcfe64c31 msvc: Drop `_SILENCE_CXX17_OLD_ALLOCATOR_MEMBERS_DEPRECATION_WARNING` (Hennadii Stepanov) Pull request description: It is no longer needed. ACKs for top commit: sipsorcery: tACK 9f9339c69206d81d355345ce66c1e23dcfe64c31. Tree-SHA512: 7bcb9df4629726ddb8b23e73b186635be54a5e5379928ce250ba2fba7a6d6f1dda98429b8329790e34fcb3861a8b00c6954746ea78e99693b86c51017c4713e0
2022-08-10Merge bitcoin/bitcoin#25642: Don't wrap around when deriving an extended key ↵Andrew Chow
at a too large depth fb9faffae3a26b8aed8b671864ba679747163019 extended keys: fail to derive too large depth instead of wrapping around (Antoine Poinsot) 8dc6670ce159c2b080e9f735c6603a601d40b6ac descriptor: don't assert success of extended key derivation (Antoine Poinsot) 50cfc9e7613d6cf6b534df6e551238b80678c70d (pubk)key: mark Derive() as nodiscard (Antoine Poinsot) 0ca258a5ace798c4e54308aa8a09b1ab3302cd7e descriptor: never ignore the return value when deriving an extended key (Antoine Poinsot) d3599c22bd4c6b3cfaaadd675e95ebe3b3cb1749 spkman: don't ignore the return value when deriving an extended key (Antoine Poinsot) Pull request description: We would previously silently wrap the derived child's depth back to `0`. Instead, explicitly fail when trying to derive an impossible depth, and handle the error in callers. An extended fuzzing corpus of `descriptor_parse` triggered this behaviour, which was reported by MarcoFalke. Fixes #25751. ACKs for top commit: achow101: re-ACK fb9faffae3a26b8aed8b671864ba679747163019 instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/25642/commits/fb9faffae3a26b8aed8b671864ba679747163019 Tree-SHA512: 9f75c23572ce847239bd15e5497df2960b6bd63c61ea72347959d968b5c4c9a4bfeee284e76bdcd7bacbf9eeb70feee85ffd3e316f353ca6eca30e93aafad343
2022-08-10Merge bitcoin/bitcoin#25810: scripted-diff: test: rename ↵MacroFake
`MAX_{ANCESTORS,DESCENDANTS}` to `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` b4a5ab96b42957a0e2110525b9e2e450deda09c1 test: refactor: deduplicate `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` constants (Sebastian Falbesoner) 0fda1c7df6165a60f63ced139ed10169f5df55f8 scripted-diff: test: rename `MAX_{ANCESTORS,DESCENDANTS}` to `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` (Sebastian Falbesoner) Pull request description: This PR renames the default in-mempool max ancestors/descendants constants `MAX_ANCESTORS`/`MAX_DESCENDANTS` in the functional tests to match the ones in the codebase: https://github.com/bitcoin/bitcoin/blob/c012875b9ded0a5183602f002738ca823d559518/src/policy/policy.h#L58-L59 https://github.com/bitcoin/bitcoin/blob/c012875b9ded0a5183602f002738ca823d559518/src/policy/policy.h#L62-L63 The custom limit constants `MAX_ANCESTORS_CUSTOM`/`MAX_DESCENDANTS_CUSTOM` are also renamed accordingly to better fit to this naming style. In the second commit, the default constants are deduplicated by moving them into the `messages.py` module. (Not sure if this module is really appropriate, as it doesn't have a connection to messages. If someone has a good suggestion, would be glad to hear it.) ACKs for top commit: w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25810/commits/b4a5ab96b42957a0e2110525b9e2e450deda09c1 glozow: utACK b4a5ab96b42957a0e2110525b9e2e450deda09c1 fanquake: ACK b4a5ab96b42957a0e2110525b9e2e450deda09c1 Tree-SHA512: a15c8256170afce3e383fceddcb562f588a02be97ce4202c84a2ebf22d73ab843f5e5a7d7c98e9ea044d8bcb7a4aeae0081d0e84c53e8fc0edffbcca00460139
2022-08-10Merge bitcoin/bitcoin#25811: doc: test: suggest multi-line imports in ↵MacroFake
functional test style guide 4edc6893825fd8c45c53c81c73a6a7801e1b458c doc: test: suggest multi-line imports in functional test style guide (Sebastian Falbesoner) Pull request description: As long as I remember contributing to functional tests (~2-3 years), it was always kind of an unwritten rule that multi-line imports are preferred over single-line imports in order to reduce the possibility of potential merge conflicts -- at least if more than one symbol from a module is imported. This PR adds this rule to the style guide and adapts the example test accordingly. (Inspired by https://github.com/bitcoin/bitcoin/pull/25792#discussion_r941180819). ACKs for top commit: kouloumos: ACK 4edc6893825fd8c45c53c81c73a6a7801e1b458c 1440000bytes: ACK https://github.com/bitcoin/bitcoin/pull/25811/commits/4edc6893825fd8c45c53c81c73a6a7801e1b458c w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25811/commits/4edc6893825fd8c45c53c81c73a6a7801e1b458c fanquake: ACK 4edc6893825fd8c45c53c81c73a6a7801e1b458c Tree-SHA512: c7b6ff62f601f4e57cc8334d291662987d6737ebca8d81c063280409f4412302172f1404ec16afc9a13007bcdba55bdab66b9b80363287e287888929cb386584
2022-08-10Merge bitcoin/bitcoin#25813: build: move raw rule into Makefile.amMacroFake
d8b26abed91c421e8517a2c9a60b57d988121b3a build: move raw rule into Makefile.am (fanquake) Pull request description: The same rule is used by the tests and benchmarks to generate headers, and currently causes #25501. Just deduplicate the code into Makefile.am. Fixes: #25501. ACKs for top commit: hebasto: ACK d8b26abed91c421e8517a2c9a60b57d988121b3a, tested on Ubuntu 22.04, the moved code was verified using `git diff --color-moved=dimmed-zebra HEAD~1..HEAD`. jarolrod: tACK d8b26abed91c421e8517a2c9a60b57d988121b3a Tree-SHA512: 249813318c92f992a89002fb9b96e70fca6ca97b2136ba0a7f5cc312e9abe24fbbe9a8faddb3bc1c0d775ae901bc91eab63ba564810bb2e3b9d56a2b1a107eb1
2022-08-10Merge bitcoin/bitcoin#25616: refactor: Return `util::Result` from ↵MacroFake
WalletLoader methods 07df6cda1468ed45ac227ac6f0169b040e5c0bf3 wallet: Return `util::Result` from WalletLoader methods (w0xlt) Pull request description: This PR adds a method that implement common logic to WalletLoader methods and change them to return `BResult<std::unique_ptr<Wallet>>`. Motivation: #25594 changed `restoreWallet` to return `BResult` but this method shares a common pattern with `createWallet` and `loadWallet`. This PR keeps the same pattern to all WalletLoader methods. ACKs for top commit: jonatack: Review ACK 07df6cda1468ed45ac227ac6f0169b040e5c0bf3 theStack: Code-review ACK 07df6cda1468ed45ac227ac6f0169b040e5c0bf3 Tree-SHA512: 2fe321134883f7cce60206888113800afef0fa168dab184e1a8692cd21a231970eb9c25c7220ea39e5d457134002d47f0974463925db76abbf8dfcd421629c63
2022-08-10psbt: Avoid unsigned int overflow in PSBT_IN_TAP_BIP32_DERIVATIONAndrew Chow
2022-08-10msvc: Drop `_SILENCE_CXX17_OLD_ALLOCATOR_MEMBERS_DEPRECATION_WARNING`Hennadii Stepanov
It is no longer needed.
2022-08-10validation tests: Use existing {Chainstate,Block}ManCarl Dong
Use {Chain,}TestingSetup's existing {Chainstate,Block}Manager and avoid unnecessarily creating a local one. This also helps reduce the code diff for a later commit where we change {Chainstate,Block}Manager's constructor signature.
2022-08-10wallet: Return `util::Result` from WalletLoader methodsw0xlt
2022-08-10refactor: improve readability for AttemptSelectionjosibake
it was pointed out by a few reviewers that the code block at the end of attempt selection was difficult to follow and lacked comments. refactor to get rid of triple nested if statement and improve readibility.
2022-08-10test: only run test for descriptor walletsjosibake
since this test uses bech32m, we skip unless sqlite is used, which is the same as checking if we are using descriptor wallets or not
2022-08-10test: add missing BOOST_ASSERTjosibake
this was missed in the original PR
2022-08-10wallet: switch to new shuffle, erase, push_backjosibake
switch to new methods, remove old code. this also updates the Size, All, and Clear methods to now use the coins map. this commit is not strictly a refactor because previously coin selection was never run over the UNKNOWN type until the last step when being run over all. now that we are iterating over each, it is run over UNKNOWN but this is expected to be empty most of the time. Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2022-08-10scripted-diff: Uppercase function namesjosibake
Change `CoinsResult` functions to uppercase to be consistent with the style guide. -BEGIN VERIFY SCRIPT- git grep -l "available_coins" | grep -v mempool_stress.cpp | xargs sed -i "s/available_coins\.\(size\|all\|clear\)/available_coins\.\u\1/" git grep -l AvailableCoins | xargs sed -i "/AvailableCoins/ s/\(all()\|size()\|clear()\)/\u\1/" sed -i "s/\(clear()\|all()\|size()\)/\u&/g" src/wallet/spend.h sed -i "/CoinsResult::/ s/\(clear()\|all()\|size()\)/\u&/" src/wallet/spend.cpp sed -i "s/result.size/result.Size/" src/wallet/spend.cpp sed -i "s/this->size/this->Size/" src/wallet/spend.cpp -END VERIFY SCRIPT-
2022-08-10refactor: add new helper methodsjosibake
add Shuffle, Erase, and Add to CoinsResult struct add a helper function for mapping TxoutType to OutputType Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2022-08-10net: simplify GetLocalAddress()Vasil Dimov
There is no need to use two variables `ret` and `addr` of the same type `CService` and assign one to the other in a strange way like `ret = CService{addr}`.
2022-08-10build: move raw rule into Makefile.amfanquake
The same rule is used by the tests and benchmarks to generate headers, and currently causes #25501. Just deduplicate the code into Makefile.am.
2022-08-10Merge bitcoin/bitcoin#25656: refactor: wallet: return util::Result from ↵MacroFake
`GetReservedDestination` methods 76b3c37fcb93b4bcb047e0500fdaa605160e25d5 refactor: wallet: return util::Result from `GetReservedDestination` methods (Sebastian Falbesoner) Pull request description: This PR is a follow-up to #25218, as suggested in comment https://github.com/bitcoin/bitcoin/pull/25218#discussion_r907710067. The interfaces of the methods `ReserveDestination::GetReservedDestination`, `{Legacy,Descriptor,}ScriptPubKeyMan::GetReservedDestination` are improved by returning `util::Result<CTxDestination>` instead of `bool` in order to get rid of the two `CTxDestination&` and `bilingual_str&` out-parameters. ACKs for top commit: furszy: ACK 76b3c37f Tree-SHA512: bf15560a88d645bcf8768024013d36012cd65caaa4a613e8a055dfd8f29cb4a219c19084606992bad177920cdca3a732ec168e9b9526f9295491f2cf79cc6815
2022-08-10Merge bitcoin/bitcoin#25794: test, tracing: don't rely on `block_connected` ↵MacroFake
USDT event order in tests 0532aa7444e7b40027b9b67876077f2a042ae329 test: don't rely on usdt block_conn event order (0xb10c) Pull request description: Relying on block_connected event order in the USDT interface tests turned out to be brittle. Closes https://github.com/bitcoin/bitcoin/issues/25793 Closes https://github.com/bitcoin/bitcoin/issues/25764 Top commit has no ACKs. Tree-SHA512: 40b5012ac80a8eac9d2f374cd39304488009c29adb474dc5e8c03b96c354be358298d2ddee8de480afecc187e1bf42ee119b7aa6216b086a8bf77b7e1310213c
2022-08-10Merge bitcoin/bitcoin#25731: test: negative/unknown `rpcserialversion` ↵MacroFake
should throw an init error 155344960b16d4b27dec3197dc273b03e6aed57d test: negative/unknown `rpcserialversion` should throw an init error (brunoerg) Pull request description: This PR adds test coverage for the following init errors: https://github.com/bitcoin/bitcoin/blob/41205bf442254d17bc7885f3b2693749da714a0e/src/init.cpp#L1025-L1030 Top commit has no ACKs. Tree-SHA512: 4456949e9a13908a5a59b13ed57bc3002b7ffd626e8dfb0346aa2600937ba1ef1b69cbae45cdb6bbc1c019dbcd64844e736a2ddd671a4704e53804355b6ea9f9
2022-08-10refactor: add UNKNOWN OutputTypejosibake
add to enum, array and handle UNKNOWN in various case statements
2022-08-09RPC/Mining: Clean out pre-Segwit miner compatibility codeLuke Dashjr
2022-08-09Merge bitcoin/bitcoin#23480: Add rawtr() descriptor for P2TR with specified ↵Andrew Chow
(tweaked) output key 544b4332f0e122167bdb94dc963405422faa30cb Add wallet tests for spending rawtr() (Pieter Wuille) e1e3081200a71b6c9b0dcf236bc2a37ed1aa7552 If P2TR tweaked key is available, sign with it (Pieter Wuille) 8d9670ccb756592bddb2a269bf5078d62658537b Add rawtr() descriptor for P2TR with unknown tweak (Pieter Wuille) Pull request description: It may be useful to be able to represent P2TR outputs in descriptors whose script tree and/or internal key aren't known. This PR does that, by adding a `rawtr(KEY)` descriptor, where the KEY represents the output key directly. If the private key corresponding to that output key is known, it also permits signing with it. I'm not convinced this is desirable, but presumably "tr(KEY)" sounds more intended for direct use than "rawtr(KEY)". ACKs for top commit: achow101: ACK 544b4332f0e122167bdb94dc963405422faa30cb sanket1729: code review ACK 544b4332f0e122167bdb94dc963405422faa30cb w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/23480/commits/544b4332f0e122167bdb94dc963405422faa30cb Tree-SHA512: 0de08de517468bc22ab0c00db471ce33144f5dc211ebc2974c6ea95709f44e830532ec5cdb0128c572513d352120bd651c4559516d4500b5b0a3d257c4b45aca
2022-08-09doc: test: suggest multi-line imports in functional test style guideSebastian Falbesoner
2022-08-09test: refactor: deduplicate `DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` constantsSebastian Falbesoner
2022-08-09scripted-diff: test: rename `MAX_{ANCESTORS,DESCENDANTS}` to ↵Sebastian Falbesoner
`DEFAULT_{ANCESTOR,DESCENDANT}_LIMIT` -BEGIN VERIFY SCRIPT- ren() { sed -i "s:$1:$2:g" $(git grep -l "$1" ./test); } ren MAX_ANCESTORS_CUSTOM CUSTOM_ANCESTOR_LIMIT ren MAX_DESCENDANTS_CUSTOM CUSTOM_DESCENDANT_LIMIT ren MAX_ANCESTORS DEFAULT_ANCESTOR_LIMIT ren MAX_DESCENDANTS DEFAULT_DESCENDANT_LIMIT -END VERIFY SCRIPT-
2022-08-09Merge bitcoin/bitcoin#24564: doc: Clarify that CheckSequenceLocksAtTip is a ↵glozow
validation function fa8671018766b2f0e18c94cff3ab2a67c6b3a41d Clarify that CheckSequenceLocksAtTip is a validation function (MarcoFalke) Pull request description: It has been pointed out that a bug in this function can prevent block template creation. ( https://github.com/bitcoin/bitcoin/pull/24080#issuecomment-1065148776 ) So it seems that the scope of this function is more than "policy". Rename it back to "validation", to partially revert commit fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35. ACKs for top commit: ajtowns: ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d - looks fine to me glozow: ACK fa8671018766b2f0e18c94cff3ab2a67c6b3a41d Tree-SHA512: 2e0df8c70df4cbea857977f140a8616cfa7505e74df66c9c9fbcf184670ce3ce7567183c3f76e6f3fe8ca6de0e065b9babde6352d6cb495e71ea077ddedbc3f4
2022-08-09build: package test_bitcoin in Windows installerfanquake
2022-08-09build: remove entire docs dir from Windows installerfanquake
2022-08-08Merge bitcoin/bitcoin#25782: test: check that `verifymessage` RPC fails for ↵Andrew Chow
non-P2PKH addresses 68006c10abbfec0f16b90efa69b7880a5e17f186 test: check that `verifymessage` RPC fails for non-P2PKH addresses (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for the `verifymessage` RPC, for the case that a non-P2PKH (but otherwise valid) address is passed: https://github.com/bitcoin/bitcoin/blob/e09ad284c762a79d59417389e9056c18e25d9770/src/util/message.cpp#L38-L40 https://github.com/bitcoin/bitcoin/blob/e09ad284c762a79d59417389e9056c18e25d9770/src/rpc/signmessage.cpp#L48-L49 The passed addresses to trigger the error are of the types nested segwit (P2SH-P2WPKH) and native segwit (P2WPKH) and are created with a helper function `addresses_from_privkey` using descriptors and the `deriveaddresses` RPC. At some point in the future, if we have BIP322 support, all those will likely succeed and can then be moved from error-throwing to the succedding assert loop. ACKs for top commit: achow101: ACK 68006c10abbfec0f16b90efa69b7880a5e17f186 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25782/commits/68006c10abbfec0f16b90efa69b7880a5e17f186 Tree-SHA512: fec4ed97460787c2ef3d04e3fce89c9365c87207c8358b59c41890f3738355c002e64f289ab4aef794ef4dfd5c867be8b67d736fb620489204f2c6bfb8d3363c
2022-08-08Merge bitcoin/bitcoin#25804: Update translations for 24.0 string freezefanquake
ff52b24e5c9b3dca269e66e0d78051a2aa5f1462 qt: Update translation source file (Hennadii Stepanov) 15f762fc65af5bd34f4ea52f2b43bcc86dcea80a qt: Bump Transifex slug for 24.x (Hennadii Stepanov) Pull request description: Required to open Transifex translations for 24.0 (see bitcoin/bitcoin#24987). ACKs for top commit: laanwj: Changes-match-release-process ACK ff52b24e5c9b3dca269e66e0d78051a2aa5f1462 jarolrod: ACK ff52b24e5c9b3dca269e66e0d78051a2aa5f1462 Tree-SHA512: f3e65b1608818084f4a3adddd2a58541ebe91ebcdb3717da2eb6f4147a0fc5f0d536a2e9f8b4eacc2a580b12c619d9eec391bfdcc5e81fa02f527408ec73a984
2022-08-08Merge bitcoin/bitcoin#25790: wallet: improve ↵Andrew Chow
`{LoadActive,Deactivate}ScriptPubKeyMan` log b5a762a35368ad5ab07018e5da14229291a54b94 wallet: improve `{LoadActive,Deactivate}ScriptPubKeyMan` log (w0xlt) Pull request description: This PR includes the output type description in the log. It currently shows the enum position, which is only useful if the reader knows the code. Master: ``` Setting spkMan to active: id = 9f..04, type = 3, internal = 0 Setting spkMan to active: id = 3d..21, type = 2, internal = 0 Setting spkMan to active: id = 69..d4, type = 0, internal = 1 Setting spkMan to active: id = 97..ea, type = 1, internal = 1 ``` PR: ``` Setting spkMan to active: id = 6a..4f, type = bech32m, internal = false Setting spkMan to active: id = 83..dc, type = legacy, internal = true Setting spkMan to active: id = 7e..5d, type = p2sh-segwit, internal = true Setting spkMan to active: id = bd..d2, type = bech32, internal = true Setting spkMan to active: id = 13...7c, type = bech32m, internal = true ``` ACKs for top commit: S3RK: Code review ACK b5a762a35368ad5ab07018e5da14229291a54b94 achow101: ACK b5a762a35368ad5ab07018e5da14229291a54b94 theStack: Code-review ACK b5a762a35368ad5ab07018e5da14229291a54b94 Tree-SHA512: 5a79706d5452e523b0456fb8435545c6c8e550b6722c0d7966af79011275a97ed97cab297562e031d601aa855118082c5b770af118783b1faaaec0cba9f9ee6a
2022-08-08qt: Update translation source fileHennadii Stepanov
2022-08-08qt: Bump Transifex slug for 24.xHennadii Stepanov
2022-08-08refactor: Drop `boost/algorithm/string/replace.hpp` dependencyHennadii Stepanov
2022-08-08test: Add test case for `ReplaceAll()` functionHennadii Stepanov
2022-08-07outputtype: remove redundant check for uncompressed keys in ↵Antoine Poinsot
AddAndGetDestinationForScript It's already checked by its (only) caller, AddAndGetMultisigDestination.
2022-08-07build: fix cleanup of test logsfanquake
make clean currently looks for test.cpp.log, when it should be test.log.
2022-08-06test: don't rely on usdt block_conn event order0xb10c
Relying on block_connected event order in the USDT interface tests turned out to be brittle. Fixes https://github.com/bitcoin/bitcoin/issues/25793 Fixes https://github.com/bitcoin/bitcoin/issues/25764
2022-08-05wallet: improve `{LoadActive,Deactivate}ScriptPubKeyMan` logw0xlt
2022-08-05Merge bitcoin/bitcoin#25788: guix: patch NSIS to remove .reloc sections from ↵Andrew Chow
installer stubs 7a0b129c41d9fefdbc20d6d04983dd87bb8379e7 guix: patch NSIS to remove .reloc sections from install stubs (fanquake) Pull request description: With the release of binutils/ld 2.36, ld swapped to much improved default settings when producing windows binaries with mingw-w64. One of these changes was to stop stripping the .reloc section from binaries, which is required for working ASLR. When we switched to using a newer Guix time-machine in #23778, we begun using binutils 2.37 to produce releases. Since then, our windows installer (produced with makensis) has not functioned correctly when run on a Windows system with the "Force randomization for images (Mandatory ASLR)" option enabled. Note that all of our other release binaries, which all contain .reloc sections, function fine under the same option, so it cannot be just the presence of a .reloc section that is the issue. The root cause of the problem is that when we compile NSIS (makensis), a number of exe installer stubs are produced at the same time, for use later when makensis is actually run. Given the new linker defaults, the stubs will contain .reloc sections, when previously they would not. It seems that, in combination with how makensis mutates the stub when it actually builds the installer, causes the problem. According to upstream, https://sourceforge.net/p/nsis/bugs/1131/#abb6: > Looks like the problem is the very existance of the .reloc section. > It's not supposed to be there, and makensis doesn't handle it. The most recent .reloc related upstream activity is in https://sourceforge.net/p/nsis/bugs/1283/, where the conclusion again seemed to be that .relo sections are not wanted, but there hasn't been any further follow up. For now, restore pre-binutils-2.36 behaviour, by passing `-Wl,--disable-reloc-section` to the linker when building the installer stubs, which fixes the produced installer. The underlying issue can be further investigated in future. .reloc section stripping is something we've accounted for previously, see #18702, and related upstream discussion is in this thread: https://sourceware.org/bugzilla/show_bug.cgi?id=19011. Fixes #25726. Guix Build (x86_64): ```bash 7e0723388913ac1ec9f650b943c6b23351ba0cd921c0ec830abf16b16724d503 guix-build-7a0b129c41d9/output/dist-archive/bitcoin-7a0b129c41d9.tar.gz c3bb9c68895ffafa2900b0d18c1268e299d012a7dc70593f20f9900cf116eb05 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/SHA256SUMS.part b57aa99c242b0aae64653c64ada38f6d3f0cbd902bbc096d3dc529fdcf87d681 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-debug.zip 341d99afc9961299883be6cd9666e8bc0f3f6296cff758719a32d27419acad36 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-setup-unsigned.exe 1d9ef48d3c9ed93a925962356b41cdaeb9d09fd758de193cd4d5f4d1ec6791eb guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-unsigned.tar.gz 28c81d99a9a4bd6648449393f91db213369e958add579ba9e9a1721540d2c4f7 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64.zip ``` Guix Build (arm64): ```bash 7e0723388913ac1ec9f650b943c6b23351ba0cd921c0ec830abf16b16724d503 guix-build-7a0b129c41d9/output/dist-archive/bitcoin-7a0b129c41d9.tar.gz c3bb9c68895ffafa2900b0d18c1268e299d012a7dc70593f20f9900cf116eb05 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/SHA256SUMS.part b57aa99c242b0aae64653c64ada38f6d3f0cbd902bbc096d3dc529fdcf87d681 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-debug.zip 341d99afc9961299883be6cd9666e8bc0f3f6296cff758719a32d27419acad36 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-setup-unsigned.exe 1d9ef48d3c9ed93a925962356b41cdaeb9d09fd758de193cd4d5f4d1ec6791eb guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64-unsigned.tar.gz 28c81d99a9a4bd6648449393f91db213369e958add579ba9e9a1721540d2c4f7 guix-build-7a0b129c41d9/output/x86_64-w64-mingw32/bitcoin-7a0b129c41d9-win64.zip ``` ACKs for top commit: achow101: ACK 7a0b129c41d9fefdbc20d6d04983dd87bb8379e7 hebasto: ACK 7a0b129c41d9fefdbc20d6d04983dd87bb8379e7 jarolrod: ACK 7a0b129c41d9fefdbc20d6d04983dd87bb8379e7 Tree-SHA512: 9e14e98207d20236b833603319fc4bb335c878a7c179ab495b33d143e2a900c6926125536bbb7499ee4f0f676cd5ea45c8c86cd7e544ed9a76bb298f98db6197
2022-08-05Merge bitcoin/bitcoin#24699: wallet: Improve AvailableCoins performance by ↵Andrew Chow
reducing duplicated operations bc886fcb31e1afa7bbf7b86bfd93e51da7076ccf Change mapWallet to be a std::unordered_map (Andrew Chow) 272356024db978c92112167f8d8e4cc62adad63d Change getWalletTxs to return a set instead of a vector (Andrew Chow) 97532867cf51db3e941231fbdc60f9f4fa0012a0 Change mapTxSpends to be a std::unordered_multimap (Andrew Chow) 1f798fe85ba952273005f68e36ed48cfc36f4c9d wallet: Cache SigningProviders (Andrew Chow) 8a105ecd1aeff15f84c3883e2762bf71ad59d920 wallet: Use CalculateMaximumSignedInputSize to indicate solvability (Andrew Chow) Pull request description: While running my coin selection simulations, I noticed that towards the end of the simulation, the wallet would become slow to make new transactions. The wallet generally performs much more slowly when there are a large number of transactions and/or a large number of keys. The improvements here are focused on wallets with a large number of transactions as that is what the simulations produce. Most of the slowdown I observed was due to `DescriptorScriptPubKeyMan::GetSigningProvider` re-deriving keys every time it is called. To avoid this, it will now cache the `SigningProvider` produced so that repeatedly fetching the `SigningProvider` for the same script will not result in the same key being derived over and over. This has a side effect of making the function non-const, which makes a lot of other functions non-const as well. This helps with wallets with lots of address reuse (as my coin selection simulations are), but not if addresses are not reused as keys will end up needing to be derived the first time `GetSigningProvider` is called for a script. The `GetSigningProvider` problem was also exacerbated by unnecessarily fetching a `SigningProvider` for the same script multiple times. A `SigningProvider` is retrieved to be used inside of `IsSolvable`. A few lines later, we use `GetTxSpendSize` which fetches a `SigningProvider` and then calls `CalculateMaximumSignedInputSize`. We can avoid a second call to `GetSigningProvider` by using `CalculateMaximumSignedInputSize` directly with the `SigningProvider` already retrieved for `IsSolvable`. There is an additional slowdown where `ProduceSignature` with a dummy signer is called twice for each output. The first time is `IsSolvable` checks that `ProduceSignature` succeeds, thereby informing whether we have solving data. The second is `CalculateMaximumSignedInputSize` which returns -1 if `ProduceSignature` fails, and returns the input size otherwise. We can reduce this to one call of `ProduceSignature` by using `CalculateMaximumSignedInputSize`'s result to set `solvable`. Lastly, a lot of time is spent looking in `mapWallet` and `mapTxSpends` to determine whether an output is already spent. The performance of these lookups is slightly improved by changing those maps to use `std::unordered_map` and `std::unordered_multimap` respectively. ACKs for top commit: Xekyo: ACK bc886fcb31e1afa7bbf7b86bfd93e51da7076ccf furszy: diff re-reACK bc886fcb Tree-SHA512: fd710fe1224ef67d2bb83d6ac9e7428d9f76a67f14085915f9d80e1a492d2c51cb912edfcaad1db11c2edf8d2d97eb7ddd95bfb364587fb1f143490fd72c9ec1
2022-08-05Merge bitcoin/bitcoin#22751: rpc/wallet: add simulaterawtransaction RPCAndrew Chow
db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 rpc/wallet: add simulaterawtransaction RPC (Karl-Johan Alm) 701a64f548662e01821765b2934b6e4b321fda6d test: add support for Decimal to assert_approx (Karl-Johan Alm) Pull request description: (note: this was originally titled "add analyzerawtransaction RPC") This command iterates over the inputs and outputs of the given transactions, and tallies up the balance change for the given wallet. This can be useful e.g. when verifying that a coin join like transaction doesn't contain unexpected inputs that the wallet will then sign for unintentionally. I originally proposed this to Elements (https://github.com/ElementsProject/elements/pull/1016) and it was suggested that I propose this upstream. There is an alternative #22776 to instead add this info to `getbalances` when providing an optional transaction as argument. ACKs for top commit: jonatack: ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 achow101: re-ACK db10cf8ae36693cb4d3ed1b47b84709cf9c0d849 Tree-SHA512: adf222ec7dcdc068d007ae6f465dbc35b692dc7bb2db337be25340ad0c2f9c64cfab4124df23400995c700f41c83c29a2c34812121782c26063b100c7969b89d
2022-08-05guix: patch NSIS to remove .reloc sections from install stubsfanquake
With the release of binutils/ld 2.36, ld swapped to much improved default settings when producing windows binaries with mingw-w64. One of these changes was to stop stripping the .reloc section from binaries, which is required for working ASLR. .reloc section stripping is something we've accounted for previously, see #18702. The related upstream discussion is in this thread: https://sourceware.org/bugzilla/show_bug.cgi?id=19011. When we switched to using a newer Guix time-machine in #23778, we begun using binutils 2.37 to produce releases. Since then, our windows installer (produced with makensis) has not functioned correctly when run on a Windows system with the "Force randomization for images (Mandatory ASLR)" option enabled. Note that all of our other release binaries, which all contain .reloc sections, function fine under the same option, so it cannot be just the presence of a .reloc section that is the issue. For now, restore makensis to it's pre-binutils-2.36 behaviour, which fixes the produced installer. The underlying issue can be further investigated in future.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 01:48:57 +0000