aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2021-10-28[validation] re-introduce bool for whether a transaction is RBFglozow
This bool was originally part of Workspace and was removed in #22539 when it was no longer needed in Finalize(). Re-introducing it because, once again, multiple functions will need to know whether we're doing an RBF. Member of MemPoolAccept so that we can use this to inform package RBF in the future.
2021-10-28[validation/refactor] store precomputed txdata in workspaceglozow
We want to be able to re-use the precomputed transaction data between PolicyScriptChecks and ConsensusScriptChecks in AcceptMultipleTransactions.
2021-10-28[validation] case-based constructors for ATMPArgsglozow
No change in behavior. ATMPArgs can continue to have granular rules like switching BIP125 on/off while we create an interface for the different sets of rules for single transactions vs multiple-testmempoolaccept vs package validation. This is a cleaner interface than manually constructing the args, which makes it easy to mix up ordering, use the wrong default, etc. It also means we don't need to edit ATMP/single transaction validation code every time we update ATMPArgs for package validation.
2021-10-27Merge bitcoin/bitcoin#23305: test: refactor: add `script_util` helper for ↵MarcoFalke
creating bare multisig scripts 4718897ce3a7c728ff7aebbadabcc8ed7a0b8d6e test: add script_util helper for creating bare multisig scripts (Sebastian Falbesoner) Pull request description: This PR is a follow-up to #22363 and #23118 and introduces a helper `keys_to_multisig_script` for creating bare multisig outputs in the form of ``` OP_K PubKey1 PubKey2 ... PubKeyN OP_N OP_CHECKMULTISIG ``` The function takes a list of pubkeys (both hex- and byte-strings are accepted due to the `script_util.check_key` helper being used internally) and optionally a threshold _k_. If no threshold is passed, a n-of-n multisig output is created, with _n_ being the number of passed pubkeys. ACKs for top commit: shaavan: utACK 4718897ce3a7c728ff7aebbadabcc8ed7a0b8d6e rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23305/commits/4718897ce3a7c728ff7aebbadabcc8ed7a0b8d6e Tree-SHA512: b452d8a75b0d17316b66ac4ed4c6893fe59c7c417719931d4cd3955161f59afca43503cd09b83a35b5a252a122eb3f0fbb9da9f0e7c944cf8da572a02219ed9d
2021-10-26Merge bitcoin/bitcoin#23006: multiprocess: Add new bitcoin-gui, bitcoin-qt, ↵MarcoFalke
bitcoin-wallet init implementations d5f985e51f2863fda0c0d632e836eba42a5c3e15 multiprocess: Add new bitcoin-gui, bitcoin-qt, bitcoin-wallet init implementations (Russell Yanofsky) Pull request description: Add separate `interfaces::Init` subclasses for `bitcoin-wallet`, `bitcoin-gui`, and `bitcoin-qt` binaries instead of sharing `bitcoind` and `bitcoin-node` init subclasses in different binaries. After this, the new init subclasses can be customized in #10102, so node and wallet code is dropped from the `bitcoin-gui` binary and wallet code is dropped from into the `bitcoin-node` binary. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). ACKs for top commit: lsilva01: reACK d5f985e hebasto: re-ACK d5f985e51f2863fda0c0d632e836eba42a5c3e15, only suggested changes since my [previous](https://github.com/bitcoin/bitcoin/pull/23006#pullrequestreview-787537444) review. Tree-SHA512: 6784210bd9ce3a6fbc66852680d0e9bc513c072dc538aeb7f48cb6a41580d3f567ccef04f975ee767a714a4b05d4d87273e94a79abda1b9c25d5ac4bbe752006
2021-10-26test: add script_util helper for creating bare multisig scriptsSebastian Falbesoner
2021-10-26Merge bitcoin/bitcoin#23332: doc: Fix CWalletTx::Confirmation docMarcoFalke
fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac doc: Fix CWalletTx::Confirmation doc (MarcoFalke) Pull request description: Follow-up to: * commit 700c42b85d20e624bef4228eef062c93084efab5, which replaced pIndex with block_hash in AddToWalletIfInvolvingMe. * commit 9700fcb47feca9d78e005b8d18b41148c8f6b25f, which replaced posInBlock with confirm.nIndex. ACKs for top commit: laanwj: Code review ACK fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac ryanofsky: Code review ACK fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac Tree-SHA512: e7643b8e9200b8ebab3eda30435ad77006568a03476006013c9ac42c826c84e42e29bcdefc6f443fe4d55e76e903bfed5aa7a51f831665001c204634b6f06508
2021-10-25Merge bitcoin/bitcoin#23306: Make AddrMan support multiple ports per IPMarcoFalke
92617b7a758c0425330fba4b886296730567927c Make AddrMan support multiple ports per IP (Pieter Wuille) Pull request description: For a long part of Bitcoin's history, this codebase has aggressively avoided making automatic connections to anything but nodes running on port 8333. I'd like to propose changing that, and this is a first PR necessary for that. The folklore justification (eventually actually added as a comment to the codebase in #20668) is that this is to prevent the Bitcoin P2P network from being leveraged to perform a DoS attack on other services, if their IP/port would get rumoured. It appears, at least the current network scale - and probably significantly larger - that the impact is very low at best (see calculations by vasild in https://github.com/bitcoin/bitcoin/issues/5150#issuecomment-853888909 e.g.). Another possible justification would be a risk that treating different IP:port combinations separately would help perform Eclipse attacks (by an attacker rumouring their own IP with many ports). This concern is (a) no different than what is possible with IPv6 (where large ranges of IP addresses are very cheaply available), and (b) already hopefully sufficiently addressed by addrman's design (which limits access through based selected based on network groups). And this policy has downsides too; in particular, a fixed port is easy to detect, and a very obvious sign a Bitcoin node is running there. One obstacle in moving away from a default port that is the fact that addrman is currently restricted to a single entry per IP address. If ports are no longer expected to be generally always the default one, we need to deal with the case where conflicting information is relayed. It turns out there is a very natural solution to this: treat (IP,port) combination exactly as we're treating IPs now; this automatically means that the same IP may appear with multiple ports, simply because those would be distinct entries. Given that indexing into addrman's bucket _already_ uses the port number, the only change required is making all addrman lookup be (IP,port) (aka `CService`) based, rather than IP (aka `CNetAddr`) based. This PR doesn't include any change to the actual outbound connection preference logic, as perhaps that's something that we want to phase in more gradually. ACKs for top commit: jnewbery: Code review ACK 92617b7a758c0425330fba4b886296730567927c naumenkogs: ACK 92617b7a758c0425330fba4b886296730567927c ajtowns: ACK 92617b7a758c0425330fba4b886296730567927c vasild: ACK 92617b7a758c0425330fba4b886296730567927c Tree-SHA512: 9eef06ce97a8b54a3f05fb8acf6941f253a9a5e0be8ce383dd05c44bb567cea243b74ee5667178e7497f6df2db93adab97ac66edbc37c883fd8ec840ee69a33f
2021-10-25Merge bitcoin/bitcoin#23312: tests: reduce feature_segwit.py usage of the ↵MarcoFalke
legacy wallet e9ade032f3283971025943d750b1305bc8da56fc tests: Add feature_segwit.py --descriptors to test_runner.py (Andrew Chow) ae6cbcc90926b65099ed8747e7a13a4aefba787a tests: restrict feature_segwit legacy wallet import tests (Andrew Chow) 1d13c44a4cb02b0acef6c6a108410cfc7eb20f71 tests: Use descriptors for feature_segwit multisig setup (Andrew Chow) Pull request description: `feature_segwit.py` has tests for some legacy wallet behavior, but otherwise does not really need the legacy wallet. Those parts now require `--legacy-wallet` to be provided (as with other legacy wallet tests). Other parts of the test are changed to work with descriptor wallets as well. ACKs for top commit: lsilva01: tACK e9ade03 Tree-SHA512: 4ae76a4d2a8d318e7f8ad4c984748e3cdd67ed00359fcd798a08492ed9399b1d01be88c9ebea3d6c996fbe190cf41708a15c98f088f0cb5c47d6d00fff258944
2021-10-25Merge bitcoin/bitcoin#23311: wallet: Use PACKAGE_NAME to mention our softwareMarcoFalke
da791c7f66243080177f92ce5f38c49305da63dc wallet: Use PACKAGE_NAME to mention our software (Hennadii Stepanov) Pull request description: This PR replaces "bitcoin" and "bitcoind" with `PACKAGE_NAME` in wallet log and error messages. ACKs for top commit: jonatack: ACK da791c7f66243080177f92ce5f38c49305da63dc lsilva01: Tested ACK da791c7 on Ubuntu 20.04. brunoerg: tACK da791c7f66243080177f92ce5f38c49305da63dc stratospher: Tested ACK da791c7 shaavan: ACK da791c7f66243080177f92ce5f38c49305da63dc Tree-SHA512: c613446d9c8c3f85e6f5fec77382c9bc17746a853c89e72e1a3a79bf355d7bd9e455bbde8f9e02a894d225a67029c732cdc68ec8c58ac8237dde27d39dae8be7
2021-10-25Merge bitcoin/bitcoin#23157: txmempool -/-> validation 1/2: improve ↵MarcoFalke
performance of check() and remove dependency on validation 082c5bf099c64e3d27abe9b68a71ce500b693e7e [refactor] pass coinsview and height to check() (glozow) ed6115f1eae0eb4669601106a9aaff078a2f3a74 [mempool] simplify some check() logic (glozow) 9e8d7ad5d9cc4b013826daead9cee09aad539401 [validation/mempool] use Spend/AddCoin instead of UpdateCoins (glozow) 09d18916afb0ecae90700d4befd9d5dc52767970 MOVEONLY: remove single-use helper func CheckInputsAndUpdateCoins (glozow) e8639ec26aaf4de3fae280963434bf1cf2017b6f [mempool] remove now-unnecessary code (glozow) 54c6f3c1da01090aee9691a2c2bee0984a054ce8 [mempool] speed up check() by using coins cache and iterating in topo order (glozow) 30e240f65e69c6dffcd033afc63895345bd51f53 [bench] Benchmark CTxMemPool::check() (glozow) cb1407196fba648aa75504e3ab3d46aa0181563a [refactor/bench] make mempool_stress bench reusable and parameterizable (glozow) Pull request description: Remove the txmempool <-> validation circular dependency by removing txmempool's dependency on validation. There are two functions in txmempool that need validation right now: `check()` and `removeForReorg()`. This PR removes the dependencies in `check()`. This PR also improves the performance of `CTxMemPool::check()` by walking through the entries exactly once, in ascending ancestorcount order, which guarantees that we see parents before children. ACKs for top commit: jnewbery: reACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e GeneFerneau: tACK [082c5bf](https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e) rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e theStack: Code-review ACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e Tree-SHA512: 40ac622af1627b5c3e6abb4f0f035d833265a8c5e8dc88faf5354875dfb5137f137825e54bbd2a2668ed37b145c5d02285f776402629f58596e51853a9a79d29
2021-10-25Merge bitcoin/bitcoin#22711: test: check for specific block reject reasons ↵MarcoFalke
in p2p_segwit.py 45827fd718d51acffdbeb3bb12d1eb96e3fa8bc0 test: check for block reject reasons in p2p_segwit.py [2/2] (Sebastian Falbesoner) 4eb532ff8b5ea9338e6cb1b927abaa43f8e3c94e test: check for block reject reasons in p2p_segwit.py [1/2] (Sebastian Falbesoner) b1488c4dcecb9dda9d7c583c1c9e1bc0852c79b2 test: fix reference to block processing test in p2p_segwit.py (Sebastian Falbesoner) Pull request description: In the test `p2p_segwit.py`, there are many instances where we send a segwit block to a node with the expectation that it is rejected. For this purpose, the helper function `test_witness_block` exists which allows also to check for a specific reject `reason` that is asserted in the debug log: https://github.com/bitcoin/bitcoin/blob/502d22ceed1f90ed41336260f8eb428d3acaf514/test/functional/p2p_segwit.py#L119-L120 This PR aims to increase the precision of the tests by adding the expected reject reasons to all `test_witness_block` call instances (found via `grep`ing after `test_witness_block(.*accepted=False`). For some blocks that are rejected due to failed script verification, the exact reason is only shown in the debug log if parallel script verification is disabled. For this reason, the addition of the reasons is split up in two commits: * first, all instances are tackled that are not subject to script verification, i.e. it doesn't matter whether parallel script verification is enabled or not (e.g. `bad-blk-weight`, `bad-witness-merkle-match`...) * then, we explicitely set `-par=1` to only use one script thread, and add the remaining reasons (`non-mandatory-script-verify-flag` with the more specific reason in parantheses) ACKs for top commit: stratospher: tested ACK 45827fd. Tree-SHA512: 00f31867f11d48b38a42b1e79a1303bda1c797ccf3d8c73e6107d70b062604d51ee2a3f2251e7f068dfafdaf09469d27dfee438d9bc9ebaef7febc4b6ef90a95
2021-10-25Merge bitcoin/bitcoin#23338: tests: speed up coinselector_testsMarcoFalke
a52f1d13409e4ef46277596ec13fa8b421fa1329 walletdb: Use SQLiteDatabase for mock wallet databases (Andrew Chow) a78c2298080f173d0266e708267458a72eb2f600 tests: Place into mapWallet in coinselector_tests (Andrew Chow) Pull request description: #23288 changed coinselector_tests to use `DescriptorScriptPubKeyMan`, but it also ended up significantly slowing down the test, from 4 seconds to over 1 minute. It appears that the source of this slow down is with `CWallet::AddToWallet`, and primarily due to writing data to the mock wallet database. Because the only thing that is actually needed is for the created transaction to be placed into `CWallet::mapWallet`, this PR removes the call to `AddToWallet` and just places the transaction into `mapWallet` directly. This reduces the test time to 5 seconds. To speed things up further, `CreateMockWalletDatabase` is changed to make a `SQLiteDatabase` instead of a `BerkeleyDatabase`. This is safe because there are no tests that require a specific mock database type. ACKs for top commit: brunoerg: tACK a52f1d13409e4ef46277596ec13fa8b421fa1329 lsilva01: tACK a52f1d1. Performed 74.36% better on Ubuntu 20.04 (VM, 12 MB, 8vCPU). glozow: utACK a52f1d13409e4ef46277596ec13fa8b421fa1329 Tree-SHA512: da77936bfd2e816d2e71703567b9389d0ee79f3a4a690802ffe3469df5bed371b296cb822b897f625654dab9436d91fd6bc02364a518a47d746e487d70a72595
2021-10-23Merge bitcoin/bitcoin#23215: ci: Add vcpkg tools cachefanquake
f778845d972578e7abdced64ec6129d809c816f6 ci: Add vcpkg tools cache (Hennadii Stepanov) Pull request description: On master (02feae54a53ff654f7cecf17ed467edcd612cd0c) vcpkg downloads some tools that are used internally: ``` ... A suitable version of cmake was not found (required v3.20.0). Downloading portable cmake v3.20.0... Downloading cmake... https://github.com/Kitware/CMake/releases/download/v3.20.2/cmake-3.20.2-windows-i386.zip -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\cmake-3.20.2-windows-i386.zip Extracting cmake... A suitable version of 7zip was not found (required v18.1.0). Downloading portable 7zip v18.1.0... Downloading 7zip... https://www.nuget.org/api/v2/package/7-Zip.CommandLine/18.1.0 -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\7-zip.commandline.18.1.0.nupkg Extracting 7zip... A suitable version of nuget was not found (required v5.5.0). Downloading portable nuget v5.5.0... Downloading nuget... https://dist.nuget.org/win-x86-commandline/v5.5.1/nuget.exe -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\22ea847d-nuget.exe Detecting compiler hash for triplet x64-windows-static... A suitable version of powershell-core was not found (required v7.1.0). Downloading portable powershell-core v7.1.0... Downloading powershell-core... https://github.com/PowerShell/PowerShell/releases/download/v7.1.3/PowerShell-7.1.3-win-x86.zip -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\PowerShell-7.1.3-win-x86.zip Extracting powershell-core... ... ``` If any of downloads above fails the whole CI task fails (see #23107). The most recent failure examples in the master branch: - https://github.com/bitcoin/bitcoin/commit/c001da306b29c46ef1e7421002c3aba3ff5ed514, [log](https://api.cirrus-ci.com/v1/task/5182966176415744/logs/build.log) - https://github.com/bitcoin/bitcoin/commit/12ff8993bcc7315f4a9b69bf16def980bb0e5197, [log](https://api.cirrus-ci.com/v1/task/5367684129882112/logs/build.log) This PR adds vcpkg tools cache. Closes #23107. --- Note for reviewers. Some patches from the initial PR were split into separated PRs: #23310 and #23329. Therefore, a discussion here could be outdated or irrelevant until the recent [push](https://github.com/bitcoin/bitcoin/pull/23215#issuecomment-949556531). ACKs for top commit: sipsorcery: ACK f778845d972578e7abdced64ec6129d809c816f6. Tree-SHA512: 201f4e4d38c00cbec6aaeca4f3e1b60f1c65289fb68b82cead47f37f6af5ac42dd539cf8ed3566f5bd9afe4a7762d42bbabb316d58f47352d4e7bfc2214806fe
2021-10-22walletdb: Use SQLiteDatabase for mock wallet databasesAndrew Chow
Default to SQLiteDatabase instead of BerkeleyDatabase for CreateDummyWalletDatabase. Most tests already use descriptor wallets and the mock db doesn't really matter for tests. The tests where it does matter will make the db directly.
2021-10-22tests: Place into mapWallet in coinselector_testsAndrew Chow
Instead of using AddToWallet so that making a COutput will work, directly add the transaction into wallet.mapWallet. This bypasses many checks that AddToWallet will do which are pointless and just slow down this test.
2021-10-22Merge bitcoin-core/gui#454: Use only Qt translation primitives in GUI codeHennadii Stepanov
58765a450c40152db8160bca8a6b0f5b754c5858 qt: Use only Qt translation primitives in GUI code (W. J. van der Laan) Pull request description: Use `Object::tr`, `QT_TRANSLATE_NOOP`, and `QCoreApplication::translate` as appropriate instead of using `_()` which doesn't get picked up. Replaces bitcoin/bitcoin#22764 Edit: I checked that the strings end up in the appropriate context in `bitcoin_en.ts` after `make translate`: - "Settings file could not be read" "Settings file could not be written" end up in `bitcoin-core` - "(press q to shutdown and continue later)" and "press q to shutdown" end up in `SplashScreen` ACKs for top commit: hebasto: ACK 58765a450c40152db8160bca8a6b0f5b754c5858 Tree-SHA512: d0cc5901426c47d411d0fe75aac195b9684b51385f74c161da772dbf9f0977f7ad7a0976e17366f49b40718e9b6eb87c3e93306dc845f97adddbc47d00731742
2021-10-22Make AddrMan support multiple ports per IPPieter Wuille
2021-10-22Merge bitcoin/bitcoin#23336: refactor: Make GenTxid boolean constructor privateMarcoFalke
fa4ec1c0bdaef9f082a6661d7faf16149774e145 Make GenTxid boolean constructor private (MarcoFalke) faeb9a575367119dbff60c35fa2c13547718e179 remove unused CTxMemPool::info(const uint256& txid) (MarcoFalke) Pull request description: This boolean argument is either verbose (when used with a named arg) or unintuitive and dangerous (when used as a plain bool). Fix that by making the constructor private. ACKs for top commit: laanwj: Code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145 jnewbery: Code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145 glozow: code review ACK fa4ec1c0bdaef9f082a6661d7faf16149774e145 Tree-SHA512: bf08ee09168885cfda71e5a01ec412b93964662a90dd9d91e75f7fdf2eaff7c21a95204d0e90b00438bfeab564d0aea66bdb9c0394ee7a05743e65a817159446
2021-10-22Merge bitcoin/bitcoin#23139: rpc: fix "trusted" field in ↵W. J. van der Laan
TransactionDescriptionString(), add coverage 66f6efc70a72cc1613906fd3c10281f9af0ba0db rpc: improve TransactionDescriptionString() "generated" help (Jon Atack) 296cfa312fd9ce19f1f820aeafa37d87764ad21d test: add listtransactions/listsinceblock "trusted" coverage (Jon Atack) d95913fc432f0fde9dec743884b14c5df83727af rpc: fix "trusted" description in TransactionDescriptionString (Jon Atack) Pull request description: The RPC gettransaction, listtransactions, and listsinceblock helps returned by `TransactionDescriptionString()` inform the user that the `trusted` boolean field is only present if the transaction is trusted and safe to spend from. The field is in fact returned by `WalletTxToJSON()` when the transaction has 0 confirmations (or negative confirmations, if conflicted), and it can be true or false. This patch fixes the help, adds test coverage, and touches up the help for the neighboring `generate` field. ACKs for top commit: rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23139/commits/66f6efc70a72cc1613906fd3c10281f9af0ba0db theStack: Tested ACK 66f6efc70a72cc1613906fd3c10281f9af0ba0db Tree-SHA512: 4c2127765b82780e07bbdbf519d27163d414d9f15598e01e02210f210e6009be344c84951d7274e747b1386991d4c3b082cd25aebe885fb8cf0b92d57178f68e
2021-10-22Merge bitcoin/bitcoin#22789: external_signer: improve fingerprint matching ↵W. J. van der Laan
logic (stop on first match) d047ed729f1d4732d23324fc76849f3c657cdbe4 external_signer: improve fingerprint matching logic (stop on first match) (Sebastian Falbesoner) Pull request description: The fingerprint matching logic in `ExternalSigner::SignTransaction` currently always iterates all inputs of a PSBT, even after a match has already been found. I guess the reason for that is not that it was not thought of, but rather the fact that breaking out of a nested loop is simply not possible (at least not without adding ugly constructs like gotos or extra state variables). This PR fixes this by using `std::any_of` from C++'s standard library, see http://www.cplusplus.com/reference/algorithm/any_of/ ACKs for top commit: lsilva01: Code Review ACK https://github.com/bitcoin/bitcoin/pull/22789/commits/d047ed729f1d4732d23324fc76849f3c657cdbe4 Sjors: utACK d047ed7 Zero-1729: crACK d047ed729f1d4732d23324fc76849f3c657cdbe4 mjdietzx: Code review ACK d047ed729f1d4732d23324fc76849f3c657cdbe4 hebasto: ACK d047ed729f1d4732d23324fc76849f3c657cdbe4, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 447e7c0c6a5b5549a2c09d52e55ba4146302c1a06e4d96de11f6945d09f98c89129cba221202dff7e0718e01a83dd173b9f19b1f02b6be228978f3f6e35d8096
2021-10-22Merge bitcoin/bitcoin#23181: refactor: remove references to deprecated ↵W. J. van der Laan
values under std::allocator ea4b61a1570178ebe5851b5fb4065222e3926f7e refactor: remove references to deprecated values under std::allocator (Pasta) Pull request description: Removes usages of allocator::pointer, allocator::const_pointer, allocator::reference and allocator::const_reference which are deprecated in c++17 and **removed** in c++20. See https://en.cppreference.com/w/cpp/memory/allocator Also prefers `using` over `typedef` see: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rt-using I'll be happy to revert this if requested so ACKs for top commit: laanwj: Re-ACK ea4b61a1570178ebe5851b5fb4065222e3926f7e Tree-SHA512: 9353e47a7de27bcd91b341eb2d832318b51fce9f508fcc791f05c02c5a160f430f4e7214e76f4b3e29639750d311c679789d8b7409255b13637391e4575c9ebe
2021-10-22Merge bitcoin/bitcoin#23288: tests: remove usage of LegacyScriptPubKeyMan ↵W. J. van der Laan
from some wallet tests 2d2edc1248a2e49636409b07448676e5bfe44956 tests: Use Descriptor wallets for generic wallet tests (Andrew Chow) 99516285b7cf2664563712d95d95f54e1985c0c2 tests: Use legacy change type in subtract fee from outputs test (Andrew Chow) dcd6eeb64adb2b532f5003cbb86ba65b3c08a87b tests: Use descriptors in psbt_wallet_tests (Andrew Chow) 4b1588c6bd96743b333cc291e19a9fc76dc8cdf1 tests: Use DescriptorScriptPubKeyMan in coinselector_tests (Andrew Chow) 811319fea4295bfff05c23c0dcab1e24c85e8544 tests, gui: Use DescriptorScriptPubKeyMan in GUI tests (Andrew Chow) 9bf02438727e1052c69d906252fc2a451c923409 bench: Use DescriptorScriptPubKeyMan for wallet things (Andrew Chow) 5e54aa9b90c5d4d472be47a7fca969c5e7b92e88 bench: remove global testWallet from CoinSelection benchmark (Andrew Chow) a5595b1320d0ebd2c60833286799ee42108a7c01 tests: Remove global vCoins and testWallet from coinselector_tests (Andrew Chow) Pull request description: Currently, various tests use `LegacyScriptPubKeyMan` because it was convenient for the refactor that introduced the `ScriptPubKeyMan` interface. However, with the legacy wallet slated to be removed, these tests should not continue to use `LegacyScriptPubKeyMan` as they are not testing any specific legacy wallet behavior. These tests are changed to use `DescriptorScriptPubKeyMan`s. Some of the coin selection tests and benchmarks had a global `testWallet`, but this seemed to cause some issues with ensuring that descriptors were set up in that wallet for each test. Those have been restructured to not have any global variables that may be modified between tests. The tests which test specific legacy wallet behavior remain unchanged. ACKs for top commit: laanwj: Code review ACK 2d2edc1248a2e49636409b07448676e5bfe44956 brunoerg: tACK 2d2edc1248a2e49636409b07448676e5bfe44956 Tree-SHA512: 6d60e5978e822d48e46cfc0dae4635fcb1939f21ea9d84eb72e36112e925554b7ee8f932c7ed0c4881b6566c6c19260bec346abdff1956ca9f300b30fb4e2dd1
2021-10-22Merge bitcoin/bitcoin#23333: wallet: fix segfault by avoiding invalid ↵fanquake
default-ctored `external_spk_managers` entry 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31 wallet: fix segfault by avoiding invalid default-ctored `external_spk_managers` entry (Sebastian Falbesoner) Pull request description: Fixes #23321 (bug reported by Josef Vondrlik (josef-v)). In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): ``` $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") ``` ACKs for top commit: achow101: Code Review ACK 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31 lsilva01: Tested ACK 6911ab9 on Ubuntu 20.04. instagibbs: ACK 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31 Tree-SHA512: 76aa96847cf2739413fb68fb902afef0b3ab9381178dd62fb0abac69f853f1f6523d73c60e610375b9a7730f275eda9162503b89f5be6e6e349a8d047b59c8dc
2021-10-22Merge bitcoin/bitcoin#23140: Make CAddrman::Select_ select buckets, not ↵W. J. van der Laan
positions, first 632aad9e6d8369750f4327a886ca5b3d3fed89bd Make CAddrman::Select_ select buckets, not positions, first (Pieter Wuille) Pull request description: The original CAddrMan behaviour (before #5941) was to pick a uniformly random non-empty bucket, and then pick a random element from that bucket. That commit, which introduced deterministic placement of entries in buckets, changed this to picking a uniformly random non-empty bucket position instead. I believe that was a mistake. Buckets are our best metric for spreading out requests across independently-controlled nodes. That does mean that if a bucket has fewer entries, its entries are supposed to be picked more frequently. This PR reverts to the original high-level behavior, but on top of the deterministic placement logic. ACKs for top commit: jnewbery: utACK 632aad9e6d8369750f4327a886ca5b3d3fed89bd naumenkogs: ACK 632aad9e6d8369750f4327a886ca5b3d3fed89bd mzumsande: ACK 632aad9e6d8369750f4327a886ca5b3d3fed89bd Tree-SHA512: 60768afba2b6f0abd0dddff04381cab5acf374df48fc0e883ee16dde7cf7fd33056a04b573cff24a1b4d8e2a645bf0f0b3689eec84da4ff330e7b59ef142eca1
2021-10-22ci: Add vcpkg tools cacheHennadii Stepanov
This change avoids downloading of the cached vcpkg tools that could fail accidentally, and it makes CI task more robust.
2021-10-22Merge bitcoin/bitcoin#23002: Make descriptor wallets by defaultMarcoFalke
9c1052a5218e191fd23c0d9fc06f2fca34b03411 wallet: Default new wallets to descriptor wallets (Andrew Chow) f19ad404631010a5e2dac2c7cbecd057b005fe2a rpc, wallet: Descriptor wallets are no longer experimental (Andrew Chow) Pull request description: Changes the default wallet type from legacy to descriptors. Descriptor wallets will now by the default type. Additionally, descriptor wallets will no longer be marked as experimental. This follows the timeline proposed in #20160 ACKs for top commit: lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/23002/commits/9c1052a5218e191fd23c0d9fc06f2fca34b03411 on Ubuntu 20.04 prayank23: tACK https://github.com/bitcoin/bitcoin/pull/23002/commits/9c1052a5218e191fd23c0d9fc06f2fca34b03411 meshcollider: Code review ACK 9c1052a5218e191fd23c0d9fc06f2fca34b03411 Tree-SHA512: 834e6fec88e0c18673af7ebe135bd5333694d1be502164eb93a90e3e76c27974165aa4e59426945100c88e4eca07356e16886ef5b05cf789683ecb23fc71a12a
2021-10-22Merge bitcoin/bitcoin#23042: net: Avoid logging AlreadyHaveTx when ↵fanquake
disconnecting misbehaving peer fa2662c293ec0aaa93092b59b6632f74729c4283 net: Avoid logging AlreadyHaveTx when disconnecting misbehaving peer (MarcoFalke) Pull request description: There is no need to log `AlreadyHaveTx` for an inv when a peer is marked for disconnection due to sending that inv. In fact, I find it confusing that a `block-relay-only` connection calls `AlreadyHaveTx` at all. Also there is no need to call `AddKnownTx` when the peer is marked for disconnection. ACKs for top commit: naumenkogs: ACK fa2662c293ec0aaa93092b59b6632f74729c4283 jnewbery: Code review ACK fa2662c293ec0aaa93092b59b6632f74729c4283 dunxen: Concept and code review ACK fa2662c293ec0aaa93092b59b6632f74729c4283 Tree-SHA512: 9996b807a824021f992b5281d82ff0cbbe6a442c2fedf7dfd6adda64ccc5e0ef4fb0ff91ab75086f975837bbbb7a5934ac7e671a80dcababa7203c92fc0c7f84
2021-10-22Make GenTxid boolean constructor privateMarcoFalke
2021-10-22remove unused CTxMemPool::info(const uint256& txid)MarcoFalke
2021-10-22Merge bitcoin/bitcoin#23325: mempool: delete exists(uint256) functionMarcoFalke
4307849256761fe2440d82bbec892d0e8e6b4dd4 [mempool] delete exists(uint256) function (glozow) d50fbd4c5b4bc72415854d582cedf94541a46983 create explicit GenTxid::{Txid, Wtxid} ctors (glozow) Pull request description: We use the same type for txids and wtxids, `uint256`. In places where we want the ability to pass either one, we distinguish them using `GenTxid`. The (overloaded) `CTxMemPool::exists()` function is defined as follows: ```c bool exists(const uint256& txid) const { return exists(GenTxid{false, txid}); } ``` It always assumes that a uint256 is a txid, which is a footgunny interface. Querying by wtxid returns a false negative if the transaction has a witness. :bug: Another approach would be to try both: ```c bool exists(const uint256& txid) const { return exists(GenTxid{false, txid}) || exists(GenTxid{false, txid}); } ``` But that's slower and wrongfully placing the burden on the callee; the caller always knows whether the hash is a txid or a wtxid. ACKs for top commit: laanwj: Code review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4 jnewbery: Tested and code review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4 MarcoFalke: review ACK 4307849256761fe2440d82bbec892d0e8e6b4dd4 👘 Tree-SHA512: 8ed167a96f3124b6c14e41073c8358658114ce121a15a4cca2db7a5ac565903a6236e34e88ac03382b8bb8b68e3999abbfc5718bc8c22476554d6b49a5298eec
2021-10-22Merge bitcoin/bitcoin#23335: refactor: include a missing <limits> header in ↵MarcoFalke
fs.cpp 077a875d94b51e3c87381133657be98989c8643e refactor: include a missing <limits> header in fs.cpp (Joan Karadimov) Pull request description: I get this compilation error on versions `0.21.1` and `22.0`: ``` fs.cpp: In member function 'bool fsbridge::FileLock::TryLock()': fs.cpp:123:89: error: 'numeric_limits' is not a member of 'std' 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~~~~~~~~~~~~ fs.cpp:123:109: error: expected primary-expression before '>' token 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^ fs.cpp:123:112: error: '::max' has not been declared; did you mean 'std::max'? 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~ | std::max In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39, from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40, from ./fs.h:9, from fs.cpp:5: C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here 300 | max(const _Tp& __a, const _Tp& __b, _Compare __comp) | ^~~ fs.cpp:123:124: error: 'numeric_limits' is not a member of 'std' 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~~~~~~~~~~~~ fs.cpp:123:144: error: expected primary-expression before '>' token 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^ fs.cpp:123:147: error: '::max' has not been declared; did you mean 'std::max'? 123 | if (!LockFileEx(hFile, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, std::numeric_limits<DWORD>::max(), std::numeric_limits<DWORD>::max(), &overlapped)) { | ^~~ | std::max In file included from C:/dev/msys64/mingw64/include/c++/11.2.0/bits/char_traits.h:39, from C:/dev/msys64/mingw64/include/c++/11.2.0/string:40, from ./fs.h:9, from fs.cpp:5: C:/dev/msys64/mingw64/include/c++/11.2.0/bits/stl_algobase.h:300:5: note: 'std::max' declared here 300 | max(const _Tp& __a, const _Tp& __b, _Compare __comp) | ^~~ ``` It appears that `std::numeric_limits<T>::max` is used without the `limits` header being included. Probably on other STL implementations it's included transitively, but not in the one in MinGW. Including it fixes the compilation problem. Environment: OS: Windows 10 Compiler: gcc 11.2.0 Qt: 5.15.2 (included in msys2) Using the latest mingw w64 shipped with msys2. ACKs for top commit: fanquake: ACK 077a875d94b51e3c87381133657be98989c8643e - Thanks. hebasto: ACK 077a875d94b51e3c87381133657be98989c8643e Tree-SHA512: 2289cb72fa3a28470f4250833be66079482d1392189b1e4679330dad109a8ae67b1d6d51cb635dbc1947c00c6c4cfbc4d7c9ae02269b932cfa1475583adaf73d
2021-10-22Merge bitcoin/bitcoin#23323: doc: Add note on deleting past-EOL release branchesMarcoFalke
fa38d98aa98bcf34b5b59ff894bbb5da67355b29 doc: Add note on deleting past-EOL release branches (MarcoFalke) Pull request description: This is being done for years now, but wasn't documented. Some reasons to do it: * Backports to those branches are unlikely to be tested both on CI (since it is often fragile and broken for stale branches) and by users (since those users likely don't exist). If a user exists, they are better off backporting any fixes they need from the last still-supported branch and test them on their own infrastructure. * Community support of those branches is still possible, though this will need to be done in another project to relieve the burden on this project. * All release tags will remain, so no historic code is lost. ACKs for top commit: hebasto: ACK fa38d98aa98bcf34b5b59ff894bbb5da67355b29 fanquake: ACK fa38d98aa98bcf34b5b59ff894bbb5da67355b29 - I think this is fine as-is. Tree-SHA512: caa714af541a6902925c89cc6a896b125f61bd77e901c5d384d84b34def2ee654bdae9f3e995001154c29672f60d2b689d0ff92d345666564fd5aa321a5b3fe7
2021-10-22Merge bitcoin/bitcoin#23329: ci: Define cases when invalidate vcpkg binary cachefanquake
e8692cf2c151d2abc206ca699e04ae05d4c31dcd ci: Improve vcpkg binary cache settings (Hennadii Stepanov) b00646bc770e0b70400fdc4545756405fb307dba ci, refactor: Rename VCPKG_TAG variable and vcpkg_cache script (Hennadii Stepanov) Pull request description: On master (c8bae2be341c921823eee95a9eec7e2b74f2f0ae), the size of the vcpkg binary cache is potentially unbounded. The reason of such behavior is the internal caching logic, following which, any change in the [set of parameters](https://vcpkg.io/en/docs/users/binarycaching.html#implementation-notes-internal-details-subject-to-change-without-notice) defined by the vcpkg implementation will cause compiling of a new binaries following by adding them to the current cache. This PR defines two obvious cases when the vcpkg binary cache will be invalidated. ACKs for top commit: sipsorcery: ACK e8692cf2c151d2abc206ca699e04ae05d4c31dcd. Tree-SHA512: 125312b9b90a9f932702ae3a8c0ed9939fca73feb92b5cdd2ebff181ae7ac50a17f8956ff11dc115da05f79030a1b56decfa25b26e37faf3505a1f30ddd8a80f
2021-10-22refactor: include a missing <limits> header in fs.cppJoan Karadimov
... needed for std::numeric_limits<T>::max on WIN32
2021-10-21doc: Fix CWalletTx::Confirmation docMarcoFalke
Follow-up to: * commit 700c42b85d20e624bef4228eef062c93084efab5, which replaced pIndex with block_hash in AddToWalletIfInvolvingMe. * commit 9700fcb47feca9d78e005b8d18b41148c8f6b25f, which replaced posInBlock with confirm.nIndex.
2021-10-21Merge bitcoin/bitcoin#23331: test: Replace log with assert_equal in ↵MarcoFalke
wallet_abandonconflict fa6c62f34b50818102ad58f2ffd152189f54d973 test: Replace log with assert_equal in wallet_abandonconflict (MarcoFalke) Pull request description: This will make the test fail as soon as the bug is fixed, forcing it to be updated. Otherwise, the bug might be fixed (or made worse) accidentally, leaving the test in an inconsistent state. ACKs for top commit: theStack: Concept and code-review ACK fa6c62f34b50818102ad58f2ffd152189f54d973 brunoerg: tACK fa6c62f34b50818102ad58f2ffd152189f54d973 Tree-SHA512: 416f72380164bf3f93332a5cfa81a8e61d8ada3714ef6815889ed3cf2d16c09411760dee4acf19629227e565b765b3dea491a0b23efd38eb370254cfadf7c441
2021-10-21Merge bitcoin/bitcoin#23293: doc: Add comment to COIN constant.W. J. van der Laan
1946af2c45372e3de39000a45a5954bb5870bc1b Add comment to COIN constant. (Kennan Mell) Pull request description: The COIN constant is critical in understanding Bitcoin's supply, but what it represents isn't clear from the name of the constant. Adding a comment clarifies the meaning of the constant for future readers. ACKs for top commit: lsilva01: ACK 1946af2 shaavan: ACK 1946af2c45372e3de39000a45a5954bb5870bc1b Tree-SHA512: ba27c6bd4a4c92664a71ef081104e9e50e47022d68eb955c247b7c49a0046a42bf64bf23a14563c646411b7be6027fea918cc62baa01bbf319b82d14d368f280
2021-10-21Merge bitcoin/bitcoin#23218: p2p: Use mocktime for ping timeoutW. J. van der Laan
fadf1186c899f45787a91c28120b0608bdc6c246 p2p: Use mocktime for ping timeout (MarcoFalke) Pull request description: It is slightly confusing to use mocktime for some times, but not others. Start fixing that by making the ping timeout use mocktime. The only downside would be that tests that use mocktime disconnect peers after this patch. However, I don't think this is an issue, as the inactivity check is already disabled for all functional tests after commit 6d76b57ca0cdf6f9c19ce065b9a4a628930a78b5. Only one unit test needed the inactivity check disabled as part of this patch. A nice side effect of this patch is that the `p2p_ping` functional test now runs 4 seconds faster. ACKs for top commit: laanwj: Code review ACK fadf1186c899f45787a91c28120b0608bdc6c246 Tree-SHA512: e9e7b21040a89d9d574b3038f85a67e6336de6cd6e41aa286769cd03cada6e75a94ec01700e052e56d822ef85d7813cc06bf7e67b81543eff8917a16cdccf942
2021-10-21Merge bitcoin/bitcoin#23271: crypto: Fix K1/K2 use in the comments in ↵W. J. van der Laan
ChaCha20-Poly1305 AEAD be7f4130f996b2564041719177f0a907e5c2011b Fix K1/K2 use in the comments in ChaCha20-Poly1305 AEAD (=) Pull request description: As per [#22331](https://github.com/bitcoin/bitcoin/pull/22331) and the [Detailed Construction of the ChaCha20Forward4064-Poly1305@Bitcoin cipher suite](https://gist.github.com/jonasschnelli/c530ea8421b8d0e80c51486325587c52#detailed-construction) mentioned in BIP 324, K1 is used for encrypting the associated data(message length) and instantiating the Poly1305 MAC while K2 is used for encrypting the payload. This PR fixes the comments which need to be updated in: 1. The test vector in `src/test/crypto_tests.cpp` 2. In `src/crypto/chacha_poly_aead.h`, `m_chacha_main` is a K2 ChaCha20 cipher instance and should be used for encrypting the payload. Also, `m_chacha_header` is a K1 ChaCha20 cipher instance and is used for encrypting the length and instantiating the Poly1305 MAC. ACKs for top commit: siv2r: ACK be7f413 jonatack: ACK be7f4130f996b2564041719177f0a907e5c2011b Zero-1729: ACK be7f413 shaavan: reACK be7f4130f996b2564041719177f0a907e5c2011b Tree-SHA512: 9d3d0f45cf95d0a87b9f04c26f04b9ea78b2f2fa578d3722146a79dd0d377b9867532fc62e02b8e1487420df7702a1f033d15db562327535940c2049cbde401f
2021-10-21Merge bitcoin/bitcoin#23324: Print peer counts for all reachable networks in ↵W. J. van der Laan
-netinfo 96f469f91bc02a19703344cc439eab064b72081a netinfo: print peer counts for all reachable networks (Jon Atack) Pull request description: instead of only for networks we have peer connections to. Users reported the previous behavior caused confusion, as no column was printed when a network was reachable but no peers were connected. Users expected a column to be printed with 0 peers. This commit aligns behavior with that expectation. In addition, the ipv4, ipv6, and onion columns were always printed whether or not they were reachable. With this change, only the reachable ones will be returned. Example with CJDNS reachable but no CJDNS peers (built on #23077 and #23175): before ``` ipv4 ipv6 onion i2p total block manual in 0 0 12 5 17 out 8 1 6 4 19 2 8 total 8 1 18 9 36 ``` after ``` ipv4 ipv6 onion i2p cjdns total block manual in 0 0 12 5 0 17 out 8 1 6 4 0 19 2 8 total 8 1 18 9 0 36 ``` There is one additional space between the in/out/total row headers and the network counts. ACKs for top commit: jsarenik: Tested ACK 96f469f91 prayank23: utACK https://github.com/bitcoin/bitcoin/commit/96f469f91bc02a19703344cc439eab064b72081a laanwj: Code review and lightly tested ACK 96f469f91bc02a19703344cc439eab064b72081a naumenkogs: ACK 96f469f91bc02a19703344cc439eab064b72081a. hebasto: ACK 96f469f91bc02a19703344cc439eab064b72081a, tested by comparing the output of `watch src/bitcoin-cli -netinfo` with the Peer tab of the Node window in the GUI :tiger2: Tree-SHA512: 3489f40148a2bd0afc9eef1e1577d44150c1fccec8dbf2a675bc23aa9343bfcae6c4039f5b96e54730668c83f40bc932fb6808f5540e86ff7249fde8dc0fff67
2021-10-21[mempool] delete exists(uint256) functionglozow
Allowing callers to pass in a uint256 (which could be txid or wtxid) but then always assuming that it's a txid is a footgunny interface.
2021-10-21create explicit GenTxid::{Txid, Wtxid} ctorsglozow
2021-10-21wallet: fix segfault by avoiding invalid default-ctored ↵Sebastian Falbesoner
`external_spk_managers` entry In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") Bug reported by Josef Vondrlik (josef-v).
2021-10-21test: Replace log with assert_equal in wallet_abandonconflictMarcoFalke
This will make the test fail as soon as the bug is fixed, forcing it to be updated. Otherwise, the bug might be fixed (or made worse) accidentally, leaving the test in an inconsistent state.
2021-10-21Merge bitcoin/bitcoin#23267: test: bip125-replaceable in listsinceblockMarcoFalke
b7884dd1b68814c59ff4fb5f7a199e306b015e85 test: bip125-replaceable in listsinceblock (brunoerg) Pull request description: This PR adds test coverage for bip125-replaceable in listsinceblock. I added this test into wallet_listtransactions.py instead of putting it into wallet_listsinceblock.py to utilize the scenario already created in wallet_listtransactions.py and avoid repetition. ACKs for top commit: theStack: ACK b7884dd1b68814c59ff4fb5f7a199e306b015e85 promag: ACK b7884dd1b68814c59ff4fb5f7a199e306b015e85. stratospher: tested ACK b7884dd. Verified the bip125-replaceable status of some transactions with listsinceblock. lsilva01: tACK b7884dd on Ubuntu 20.04 Tree-SHA512: 510dfe5a6f9d68e5a656514d356dc8fe99324296ed8caa78f0eb4b6c6906cf70b1fb50bde80aa6f61d726b2fa1d4ce1fe48c635ce24285588e56ceff92291617
2021-10-21Merge bitcoin/bitcoin#23281: test: check that bumpfee RPC fails for txs with ↵MarcoFalke
descendants in mempool 4ac8c89ad96de9ad61cad756b10c9dee2d9e1405 test: check that bumpfee RPC fails for txs with descendants in mempool (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for the bumpfee RPC error _"Transaction has descendants in the mempool"_, https://github.com/bitcoin/bitcoin/blob/6419bdfeb130b20ccfed229d9ba7eca7f385d036/src/wallet/feebumper.cpp#L29-L32 which is thrown if the bumped tx has descendants in the mempool and is _not_ connected to the bitcoin wallet (for those, the error "Transaction has descendants in the Wallet" is thrown a few lines above). To achieve that, the test framework's MiniWallet is used. ACKs for top commit: brunoerg: tACK 4ac8c89ad96de9ad61cad756b10c9dee2d9e1405 promag: Code review ACK 4ac8c89ad96de9ad61cad756b10c9dee2d9e1405. Nice stuff! lsilva01: Tested ACK 4ac8c89 cad756b10c9dee2d9e1405 on Ubuntu 20.04. stratospher: tested ACK 4ac8c89. Tree-SHA512: 83e99f9dd2b140c0c0597c0c36c9c948fa334871be40e58a5e004440698d9685661c69bb83ab937d30f692545a3799705f991b31904f2ef31a2fbc3ae1179fa8
2021-10-21Merge bitcoin/bitcoin#23287: test: get and decode tx with a single ↵MarcoFalke
`gettransaction` RPC call 130ee481082d2612d452d7d69131ade935b225b5 test: get and decode tx with a single `gettransaction` RPC call (Sebastian Falbesoner) Pull request description: Rather than subsequently calling `gettransaction` and `decoderawtransaction` to get the decoded information for a specific tx-id, we can simply use the verbose version of `gettransaction`, which returns this in a 'decoded' key. I.e. ``` node.decoderawtransaction(node.gettransaction(txid)['hex']) ``` can simply be replaced by: ``` node.gettransaction(txid=txid, verbose=True)['decoded'] ``` Rationale: shorter code, shorter test logs, less RPC calls. ACKs for top commit: stratospher: tested ACK 130ee48 amadeuszpawlik: tACK 130ee481082d2612d452d7d69131ade935b225b5 lsilva01: Tested ACK 130ee48 on Ubuntu 20.04. shaavan: ACK 130ee481082d2612d452d7d69131ade935b225b5 Tree-SHA512: cf0bd26e1e21b8022fb8062857906e0706f0ee32d3277f985c461e2519405afe445ab005f5f763fb268c7b4d6e48b2d47eda7af8621b3bce67cece8dfc9bc153
2021-10-21Merge bitcoin/bitcoin#23328: ci: Disable syscall sandbox in valgrind ↵MarcoFalke
functional tests fa44406ffd34670af929f14484042e1de29ffcdd ci: Disable syscall sandbox in valgrind functional tests (MarcoFalke) Pull request description: Otherwise this will fail: ``` $ valgrind ./src/bitcoind -regtest -datadir=/tmp -sandbox=log-and-abort ==204660== Memcheck, a memory error detector ==204660== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==204660== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info ==204660== Command: ./src/bitcoind -regtest -datadir=/tmp -sandbox=log-and-abort ==204660== Bad system call (core dumped) ACKs for top commit: practicalswift: cr ACK fa44406ffd34670af929f14484042e1de29ffcdd Tree-SHA512: 41853e6d5697d99bd5775a9b9017859290b3119e83726036d7e58856fcbb3459ef794b129bfaba6eca54b9bc034e2baf74cc2c177767ad5d5af3f3be2a45507f
2021-10-21Merge bitcoin/bitcoin#23137: Move-only: bloom to src/commonfanquake
fa2d611bedc2a755dcf84a82699c70b57b903cf6 style: Sort (MarcoFalke) fa1e5de2db2c7c95b96773a4ac231ab4249317e9 scripted-diff: Move bloom to src/common (MarcoFalke) fac303c504ab19b863fddc7a0093068fee9d4ef3 refactor: Remove unused MakeUCharSpan (MarcoFalke) Pull request description: To avoid having all files at the top level `./src` directory, start moving them to their respective sub directory according to https://github.com/bitcoin/bitcoin/issues/15732. `bloom` currently depends on libconsensus (`CTransaction`, `CScript`, ...) and it is currently located in the libcommon. Thus, move it to `src/common/`. (libutil in `src/util/` is for stuff that doesn't depend on libconsensus). ACKs for top commit: theStack: Code-review ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6 ryanofsky: Code review ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6 fanquake: ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6 - source shuffle starts now. Tree-SHA512: d2fbc31b81741e9f0be539e1149542c9ca39958c240e12e8e757d882beccd0f0debdc10dcce146a05f03ef9f5c6247900a461a7a4799b515e8716dfb9af1fde2
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-08 15:03:19 +0000