aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-01-11Merge bitcoin/bitcoin#26675: wallet: For feebump, ignore abandoned ↵Andrew Chow
descendant spends f9ce0eadf4eb58d1e2207c27fabe69a5642482e7 For feebump, ignore abandoned descendant spends (John Moffett) Pull request description: Closes #26667 To be eligible for fee-bumping, a transaction must not have any of its outputs (eg - change) spent in other unconfirmed transactions in the wallet. This behavior is currently [enforced](https://github.com/bitcoin/bitcoin/blob/9e229a542ff2107be43eff2e4b992841367f0366/src/wallet/feebumper.cpp#L25-L28) and [tested](https://github.com/bitcoin/bitcoin/blob/9e229a542ff2107be43eff2e4b992841367f0366/test/functional/wallet_bumpfee.py#L270-L286). However, this check shouldn't apply to spends in abandoned descendant transactions, as explained by #26667. `CWallet::IsSpent` already carves out an exception for abandoned transactions, so we can just use that. I've also added a new test to cover this case. ACKs for top commit: Sjors: re-utACK f9ce0eadf4eb58d1e2207c27fabe69a5642482e7 achow101: ACK f9ce0eadf4eb58d1e2207c27fabe69a5642482e7 furszy: ACK f9ce0ead Tree-SHA512: 19d957d1cf6747668bb114e27a305027bfca5a9bed2b1d9cc9e1b0bd4666486c7c4b60b045a7fe677eb9734d746f5de76390781fb1e9e0bceb4a46d20acd1749
2023-01-11Merge bitcoin/bitcoin#26695: bench: BlockAssembler on a mempool with packagesAndrew Chow
04528054fcde61aa00e009dbbe1ac350ca1cf748 [bench] BlockAssembler with mempool packages (glozow) 6ce265acf4ff6ee5057b46bcb8b55abc4422e6f8 [test util] lock cs_main before pool.cs in PopulateMempool (glozow) 8791410662ce3ab7ba6bbe9813c55369edd6e4c9 [test util] randomize fee in PopulateMempool (glozow) cba5934eb697aedbe1966ebc2817ab87232a1b59 [miner] allow bypassing TestBlockValidity (glozow) c0588523083c9c78770b8b19a52a919db56250d9 [refactor] parameterize BlockAssembler::Options in PrepareBlock (glozow) a2de971ba1c588488dde653a76853666429d4911 [refactor] add helper to apply ArgsManager to BlockAssembler::Options (glozow) Pull request description: Performance of block template building matters as miners likely want to be able to start mining on a block with transactions asap after a block is found. We would want to know if a mempool PR accidentally caused, for example, a 100x slowdown. An `AssembleBlock()` bench exists, but it operates on a mempool with 101 transactions, each with 0 ancestors or descendants and with the same fee. Adding a bench with a more complex mempool is useful because (1) it's more realistic (2) updating packages can potentially cause the algorithm to take a long time. ACKs for top commit: kevkevinpal: Tested ACK [0452805](https://github.com/bitcoin/bitcoin/pull/26695/commits/04528054fcde61aa00e009dbbe1ac350ca1cf748) achow101: ACK 04528054fcde61aa00e009dbbe1ac350ca1cf748 stickies-v: ACK 04528054f Tree-SHA512: 38c138d6a75616651f9b1faf4e3a1cd833437a486f4e84308fbee958e8462bb570582c88f7ba7ab99d80191e97855ac2cf27c43cc21585d3e4b0e227effe2fb5
2023-01-11rpc: add minconf and maxconf options to sendallishaanam
2023-01-11Wallet/RPC: Allow specifying min & max chain depth for inputs used by fund callsJuan Pablo Civile
Enables users to craft BIP-125 replacements with changes to the output list, ensuring that if additional funds are needed they will be added.
2023-01-11Merge bitcoin/bitcoin#26821: refactor: Make `ThreadHTTP` return voidAndrew Chow
45553e11c965db218733f9ad32ecde391b393443 refactor: Make `ThreadHTTP` return void (Hennadii Stepanov) Pull request description: The `bool` return value was introduced in 755aa05174e06effd758eeb78c5af9fb465e9611 (https://github.com/bitcoin/bitcoin/pull/8421). It has been not used since 8d3f46ec3938e2ba17654fecacd1d2629f9915fd (https://github.com/bitcoin/bitcoin/pull/14670). No behavior change. ACKs for top commit: achow101: ACK 45553e11c965db218733f9ad32ecde391b393443 brunoerg: crACK 45553e11c965db218733f9ad32ecde391b393443 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26821/commits/45553e11c965db218733f9ad32ecde391b393443 stickies-v: ACK 45553e11c Tree-SHA512: 1593a5740e729967fbe1363235cd5b77ecf431b29bc740a89a6c70fc838ad97a2e4a2cd7cd63aa482f7c50bc2ffabc8cd53e8f64d6032603cb3b662229bc3dc2
2023-01-11rpc: Run type check against RPCArgsMarcoFalke
2023-01-11Merge bitcoin/bitcoin#26328: doc: fix -netinfo relaytxes helpfanquake
0f5fc4f656d0990802ab552c0e620f49e785fed4 doc: fix up -netinfo relaytxes help (Jon Atack) Pull request description: Addresses https://github.com/bitcoin/bitcoin/pull/26109#discussion_r995502563 by Marco Falke (thanks!) ACKs for top commit: mzumsande: Code Review ACK 0f5fc4f656d0990802ab552c0e620f49e785fed4 Tree-SHA512: d7345d1a94b15c4ec1a2bb0be5c04c472411d90cefb4c16ed524933d2bfc36816bb7519c2e109b2e41ff451b039dd2ddaa6d5db917ad54745332f2a1d8b85570
2023-01-11Merge bitcoin/bitcoin#26758: refactor: Add `performance-no-automatic-move` ↵MarcoFalke
clang-tidy check 9567bfeab95cc0932073641dd162903850987d43 clang-tidy: Add `performance-no-automatic-move` check (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#26642 as [requested](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1054673201). For the problem description see https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html. The following types are affected: - `std::pair<CAddress, NodeSeconds>` - `std::vector<CAddress>` - `UniValue`, also see bitcoin/bitcoin#25429 - `QColor` - `CBlock` - `MempoolAcceptResult` - `std::shared_ptr<CWallet>` - `std::optional<SelectionResult>` - `CTransactionRef`, which is `std::shared_ptr<const CTransaction>` ACKs for top commit: andrewtoth: ACK 9567bfeab95cc0932073641dd162903850987d43 aureleoules: ACK 9567bfeab95cc0932073641dd162903850987d43 Tree-SHA512: 9b6a5d539205b41d2c86402d384318ed2e1d89e66333ebd200a48fd7df3ce6f6c60a3e989eda5cc503fb34b8d82526f95e56776e1af51e63b49e3a1fef72dbcb
2023-01-11Merge bitcoin/bitcoin#26646: validation, bugfix: provide more info in ↵glozow
*MempoolAcceptResult 264f9ef17f650035882d24083fb419845942a3ac [validation] return MempoolAcceptResult for every tx on PCKG_TX failure (glozow) dae81e01e8698e04afb0db4d1442659c3b48fcf5 [refactor] rename variables in AcceptPackage for clarity (glozow) da484bc738eca47936a31a2e5ad663417e19f311 [doc] release note effective-feerate and effective-includes RPC results (glozow) 5eab397b9840de5a4729bea723794b529e5fcbb4 [validation] remove PackageMempoolAcceptResult::m_package_feerate (glozow) 601bac88cb95404e7d38ac6348d959c0e06bd922 [rpc] return effective-includes in testmempoolaccept and submitpackage (glozow) 1691eaa818f7a7b22907f756490b842d80a9a21d [rpc] return effective-feerate in testmempoolaccept and submitpackage (glozow) d6c7b78ef2924af72f677ce2a7472c2447141e18 [validation] return wtxids of other transactions whose fees were used (glozow) 1605886380e4d3ff2e1236739fb800fa07322c49 [validation] return effective feerate from mempool validation (glozow) 5d35b4a7dee95ae70bfdcbe79bc39fe488641b23 [test] package validation quits early due to non-policy, non-missing-inputs failure (glozow) be2e4d94e59510e0a98408313feb27e97321b16e [validation] when quitting early in AcceptPackage, set package_state and tx result (glozow) Pull request description: This PR fixes a bug and improves the mempool accept interface to return information more predictably. Bug: In package validation, we first try the transactions individually (see doc/policy/packages.md for more explanation) and, if they all failed for missing inputs and policy-related (i.e. fee) reasons, we'll try package validation. Otherwise, we'll just "quit early" since, for example, if a transaction had an invalid signature, adding a child will not help make it valid. Currently, when we quit early, we're not setting the `package_state` to be invalid, so the caller might think it succeeded. Also, we're returning no results - it makes more sense to return the individual transaction failure. Thanks instagibbs for catching https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1013293248! Also, make the package results interface generally more useful/predictable: - Always return the feerate at which a transaction was considered for `CheckFeeRate` in `MempoolAcceptResult::m_effective_feerate` when it was successful. This can replace the current `PackageMempoolAcceptResult::m_package_feerate`, which only sometimes exists. - Always provide an entry for every transaction in `PackageMempoolAcceptResult::m_tx_results` when the error is `PCKG_TX`. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/26646/commits/264f9ef17f650035882d24083fb419845942a3ac achow101: ACK 264f9ef17f650035882d24083fb419845942a3ac naumenkogs: reACK 264f9ef17f650035882d24083fb419845942a3ac Tree-SHA512: ce7fd9927a80030317cc6157822596e85a540feff5dbf5eea7c62da2eb50c917cdddc9da1e2ff62cc18b98b27d360151811546bd9d498859679a04bbee090837
2023-01-11i2p: use consistent number of tunnels with i2pd and Java I2PVasil Dimov
The default number of tunnels in the Java implementation is 2 and in the C++ i2pd it is 5. Pick a mid-number (3) and explicitly set it in order to get a consistent behavior with both routers. Do this for persistent sessions which are created once at startup and can be used to open up to ~10 outbound connections and can accept up to ~125 incoming connections. Transient sessions already set number of tunnels to 1. Suggested in: https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1367356129 https://geti2p.net/en/docs/api/samv3 Alleviates: https://github.com/bitcoin/bitcoin/issues/26754
2023-01-11i2p: lower the number of tunnels for transient sessionsVasil Dimov
This will lower the load on the I2P network. Since we use one transient session for connecting to just one peer, a higher number of tunnels is unnecessary. This was suggested in: https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1365449401 https://github.com/bitcoin/bitcoin/issues/26754#issuecomment-1367356129 The options are documented in: https://geti2p.net/en/docs/protocol/i2cp#options A tunnel is unidirectional, so even if we make a single outbound connection we still need an inbound tunnel to receive the messages sent to us over that connection. Alleviates: https://github.com/bitcoin/bitcoin/issues/26754
2023-01-11i2p: reuse created I2P sessions if not usedVasil Dimov
In the case of `i2pacceptincoming=0` we use transient addresses (destinations) for ourselves for each outbound connection. It may happen that we * create the session (and thus our address/destination too) * fail to connect to the particular peer (e.g. if they are offline) * dispose the unused session. This puts unnecessary load on the I2P network because session creation is not cheap. Is exaggerated if `onlynet=i2p` is used in which case we will be trying to connect to I2P peers more often. To help with this, save the created but unused sessions and pick them later instead of creating new ones. Alleviates: https://github.com/bitcoin/bitcoin/issues/26754
2023-01-11Merge bitcoin/bitcoin#26838: doc: I2P documentation updatesMarcoFalke
3e1d2941e921f96c489c5101e2a7a7bf7e7d8b73 doc: remove recommended I2P router versions (jonatack) 295849abb56ae5696f9f18f57f6af77087499756 doc: update/clarify/de-emphasize I2P transient address section (jonatack) dffa31945729d110e4fff0fa092c077b5ec9e0eb doc: update bandwidth section of I2P documentation (jonatack) 0ed9cc58928d98633fb59922ebacbcf248d667cb doc: clarify -i2pacceptincoming help documentation (jonatack) Pull request description: Address the documentation updates requested in issue #26754, clarify/simplify the -i2pacceptincoming help, and a few other fixups. ACKs for top commit: willcl-ark: ACK 3e1d294 1440000bytes: ACK https://github.com/bitcoin/bitcoin/pull/26838/commits/3e1d2941e921f96c489c5101e2a7a7bf7e7d8b73 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26838/commits/3e1d2941e921f96c489c5101e2a7a7bf7e7d8b73 vasild: ACK 3e1d2941e921f96c489c5101e2a7a7bf7e7d8b73 Tree-SHA512: e647221884af34646b99150617f4d4cc8d5fce325a769294f49047b9d8c9c8ab2b365cfdd9f56b3bd0303da706233f03d24cececf6e161c53f04ed947751052a
2023-01-10Merge bitcoin/bitcoin#26679: wallet: Skip rescanning if wallet is more ↵Andrew Chow
recent than tip 378400953424598fd78ccec5ba8cc38bc253c550 wallet: Skip rescanning if wallet is more recent than tip (Andrew Chow) Pull request description: If a wallet has key birthdates that are more recent than the currrent chain tip, or a bestblock height higher than the current tip, we should not attempt to rescan as there is nothing to scan for. Fixes #26655 ACKs for top commit: ishaanam: re-utACK 378400953424598fd78ccec5ba8cc38bc253c550 w0xlt: utACK https://github.com/bitcoin/bitcoin/pull/26679/commits/378400953424598fd78ccec5ba8cc38bc253c550 furszy: Code review ACK 37840095 Tree-SHA512: f0d90b62940d97d50f21e1e01fa6dcb54409fad819cea4283612825c4d93d733df323cd92787fed43956b0a8e386a5bf88218f1f5749c913398667a5c8f54470
2023-01-10wallet: Automatically abandon orphaned coinbases and their childrenAndrew Chow
2023-01-10Merge bitcoin/bitcoin#26186: rpc: Sanitize label name in various RPCs with testsAndrew Chow
65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d test: Invalid label name coverage (Aurèle Oulès) 552b51e682b5a52d9e2fbe64e44e623451692bd3 refactor: Add sanity checks in LabelFromValue (Aurèle Oulès) 67e7ba8e1aea58fc864f9bb1fc0e56b70777185e rpc: Sanitize label name in various RPCs (Aurèle Oulès) Pull request description: The following RPCs did not sanitize the optional label name: - importprivkey - importaddress - importpubkey - importmulti - importdescriptors - listsinceblock Thus is was possible to import an address with a label `*` which should not be possible. The wildcard label is used for backwards compatibility in the `listtransactions` rpc. I added test coverage for these RPCs. ACKs for top commit: ajtowns: ACK 65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d achow101: ACK 65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d furszy: diff ACK 65e78bd stickies-v: re-ACK 65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d theStack: re-ACK 65e78bda7cd23ad7b6ede63c6bf5ffe6f552c71d Tree-SHA512: ad99f2824d4cfae352166b76da4ca0069b7c2eccf81aaa0654be25bbb3c6e5d6b005d93960f3f4154155f80e12be2d0cebd5529922ae3d2a36ee4eed82440b31
2023-01-10doc: fix up -netinfo relaytxes helpJon Atack
Co-authored-by: "MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>"
2023-01-10doc: net: fix link to onion address encoding scheme [ONIONADDRESS]Sebastian Falbesoner
Instead of referring to a fixed line number to a file in master (which is obviously always quickly outdated), use a permalink tied to the latest commit.
2023-01-10[validation] return MempoolAcceptResult for every tx on PCKG_TX failureglozow
This makes the interface more predictable and useful. The caller understands one or more transactions failed, and can learn what happened with each transaction. We already have this information, so we might as well return it. It doesn't make sense to do this for other PackageValidationResult values because: - PCKG_RESULT_UNSET: this means everything succeeded, so the individual failures are no longer accurate. - PCKG_MEMPOOL_ERROR: something went wrong with the mempool logic; transaction failures might not be meaningful. - PCKG_POLICY: this means something was wrong with the package as a whole. The caller should use the PackageValidationState to find the error, rather than looking at individual MempoolAcceptResults.
2023-01-10[refactor] rename variables in AcceptPackage for clarityglozow
2023-01-10[validation] remove PackageMempoolAcceptResult::m_package_feerateglozow
This value creates an extremely confusing interface as its existence is dependent upon implementation details (whether something was submitted on its own, etc). MempoolAcceptResult::m_effective_feerate is much more helpful, as it always exists for submitted transactions.
2023-01-10[rpc] return effective-includes in testmempoolaccept and submitpackageglozow
2023-01-10[rpc] return effective-feerate in testmempoolaccept and submitpackageglozow
2023-01-10[validation] return wtxids of other transactions whose fees were usedglozow
2023-01-09Merge bitcoin/bitcoin#26618: rpc: Prevent unloading a wallet when rescanningAndrew Chow
109cbb819ddd20220a255791c40261f746260f42 doc: Add release notes for #26618 (Aurèle Oulès) b13902d2e45ad1e73f79e221ffc16ce26b7c3ba5 rpc: Prevent unloading a wallet when rescanning (Aurèle Oulès) Pull request description: Fixes #26463. This PR prevents a user from unloading a wallet if it is currently rescanning. To test: ```bash ./src/bitcoin-cli -testnet -named createwallet wallet_name=wo disable_private_keys=true ./src/bitcoin-cli -testnet -rpcwallet=wo importdescriptors '[{ "desc": "addr(mmcuW74MyJUZuLnWXGQLoNXPrS9RbFz6gD)#tpnrahgc", "timestamp": 0, "active": false, "internal": false, "next": 0 }]' ./src/bitcoin-cli -testnet unloadwallet wo error code: -4 error message: Wallet is currently rescanning. Abort existing rescan or wait. ACKs for top commit: achow101: ACK 109cbb819ddd20220a255791c40261f746260f42 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26618/commits/109cbb819ddd20220a255791c40261f746260f42 kouloumos: ACK 109cbb819ddd20220a255791c40261f746260f42 promag: ACK 109cbb819ddd20220a255791c40261f746260f42 Tree-SHA512: 15fdddf4cf9f3fa08f52069fe4a25a76e04a55bb2586b031bfb0393dce7f175dcdb52823e132a7dff6a894539beeb980a1aad2a792790be036be6977628149b2
2023-01-09Fix misleading RPC console wallet messageJohn Moffett
In certain circumstances, the GUI console will display the message 'Executing command without any wallet' when it is, in fact, using the default wallet. In RPC calls, if no wallet is explicitly selected and there is exactly one wallet loaded, the default is to act on that loaded wallet. The GUI console acts that way in reality, but erroneously reports that it's not acting on any particular wallet.
2023-01-09doc: clarify -i2pacceptincoming help documentationjonatack
and also hoist the default setting to a constexpr and remove unused f-string operators in a related functional test.
2023-01-08Fix segfault when shutdown during wallet openJohn Moffett
If you open a wallet and send a shutdown signal during that process, the GUI will segfault due to some queued wallet events happening after the wallet controller is deleted. This is a minimal fix for those issues.
2023-01-07Pass MSG_MORE flag when sending non-final network messagesMatt Whitlock
Since Nagle's algorithm is disabled, each and every call to send(2) can potentially generate a separate TCP segment on the wire. This is especially inefficient when sending the tiny header preceding each message payload. Linux implements a MSG_MORE flag that tells the kernel not to push the passed data immediately to the connected peer but rather to collect it in the socket's internal transmit buffer where it can be combined with data from successive calls to send(2). Where available, specify this flag when calling send(2) in CConnman::SocketSendData(CNode &) if the data buffer being sent is not the last one in node.vSendMsg.
2023-01-06[validation] return effective feerate from mempool validationglozow
2023-01-06[test] package validation quits early due to non-policy, non-missing-inputs ↵glozow
failure
2023-01-06[validation] when quitting early in AcceptPackage, set package_state and tx ↵glozow
result Bug: not setting package_state means package_state.IsValid() == true and the caller does not know that this failed. We won't be validating this transaction again, so it makes sense to return this failure to the caller. Rename package_state to package_state_quit_early to make it more clear what this variable is used for and what its scope is. Co-authored-by: Greg Sanders <gsanders87@gmail.com>
2023-01-06p2p, rpc: don't allow past absolute timestamp in `setban`brunoerg
2023-01-06rpc: Return accurate results for scanblocksAurèle Oulès
This makes use of undo data to accurately verify results from blockfilters.
2023-01-06build: Re-enable external signer on WindowsHennadii Stepanov
2023-01-06Merge bitcoin/bitcoin#26823: refactor: Work around ↵MarcoFalke
Werror=free-nonheap-object in AssumeCalculateMemPoolAncestors faa86eeb418ac5a28e7c4aa6cd13f607e151fad8 refactor: Work around Werror=free-nonheap-object in AssumeCalculateMemPoolAncestors (MarcoFalke) Pull request description: This works around the s390x gcc bug mentioned in https://github.com/bitcoin/bitcoin/issues/26820 ACKs for top commit: achow101: ACK faa86eeb418ac5a28e7c4aa6cd13f607e151fad8 Tree-SHA512: 041d5daa157ea1856b0a8027181085d70624f5f8822049ace9963e90c653bbb8c91d1f16b8a5bf460687eb4ed13f1db72e3885a511aadbad6dede93d9f9ccd6d
2023-01-05randomenv: consolidate WIN32 #ifdefsfanquake
Order includes. Remove // for xyz comments
2023-01-05random: remove windows-only compat.h include in randomenvfanquake
Note that this was probably only here to indirectly receive windows.h via another include in compat.h (windows.h or winreg.h aren't included there). Also note that compat.h is already pulled in here for everyone via util/time.h, so including inside a windows only ifdef is secondarily redundant.
2023-01-05refactor: Work around Werror=free-nonheap-object in ↵MarcoFalke
AssumeCalculateMemPoolAncestors
2023-01-05refactor: Make `ThreadHTTP` return voidHennadii Stepanov
The `bool` return value was introduced in 755aa05174e06effd758eeb78c5af9fb465e9611. It has been not used since 8d3f46ec3938e2ba17654fecacd1d2629f9915fd. No behavior change.
2023-01-05Merge bitcoin/bitcoin#26761: wallet: fully migrate address book entries for ↵Andrew Chow
watchonly/solvable wallets 730e14a317ae45fe871c8d6f44a51936756bbbea test: wallet: check that labels are migrated to watchonly wallet (Sebastian Falbesoner) d5f4ae7fac0bceb0c9ad939b9a4fbdb85da0bf95 wallet: fully migrate address book entries for watchonly/solvable wallets (Sebastian Falbesoner) Pull request description: Currently `migratewallet` migrates the address book (i.e. labels and purposes) for watchonly and solvable wallets only in RAM, but doesn't persist them on disk. Fix this by adding another loop for both of the special wallet types after which writes the corresponding NAME and PURPOSE entries to the database in a single batch. Also adds a corresponding test that checks if labels were migrated correctly for a watchonly wallet. ACKs for top commit: achow101: ACK 730e14a317ae45fe871c8d6f44a51936756bbbea furszy: code ACK 730e14a3, left a non-blocking nit. aureleoules: ACK 730e14a317ae45fe871c8d6f44a51936756bbbea Tree-SHA512: 159487e11e858924ef762e0190ccaea185bdff239e3d2280c8d63c4ac2649ec71714dc4d53dec644f03488f91c3b4bbbbf3434dad23bc0fcecb6657f353ea766
2023-01-05Merge bitcoin/bitcoin#23829: refactor: use braced init for integer literals ↵MarcoFalke
instead of c style casts f2fc03ec856d7d19a20c482514350cced38f9504 refactor: use braced init for integer constants instead of c style casts (Pasta) Pull request description: See https://github.com/bitcoin/bitcoin/pull/23810 for more context. This is broken out from that PR, as it is less breaking, and should be trivial to review and merge. EDIT: Long term, the intention is to remove all C-style casts, as they can dangerously introduce reinterpret_casts. This is one step which removes a number of trivially removable C-style casts ACKs for top commit: aureleoules: ACK f2fc03ec856d7d19a20c482514350cced38f9504 Tree-SHA512: 2fd11b92c9147e3f970ec3e130e3b3dce70e707ff02950a8c697d4b111ddcbbfa16915393db20cfc8f384bc76f13241c9b994a187987fcecd16a61f8cc0af14c
2023-01-05refactor: add kernel/cs_main.*fanquake
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2023-01-04Merge bitcoin/bitcoin#26747: wallet: fix confusing error / GUI crash on ↵Andrew Chow
cross-chain legacy wallet restore 21ad4e26ec320dcecc8961888bc82d0bb72d5ed3 test: add coverage for cross-chain wallet restore (Sebastian Falbesoner) 8c7222bda3f7136f312a6e57b76d6a2d0a114f68 wallet: fix GUI crash on cross-chain legacy wallet restore (Sebastian Falbesoner) Pull request description: Restoring a wallet backup from another chain should result in a dedicated error message (we have _"Wallet files should not be reused across chains. Restart bitcoind with -walletcrosschain to override."_ for that). Unfortunately this is currently not the case for legacy wallet restores, as in the course of cleaning up the newly created wallet directory a `filesystem_error` exception is thrown due to the directory not being empty; the wallet database did indeed load successfully (otherwise we wouldn't know that the chain doesn't match) and hence BDB-related files and directories are already created in the wallet directory. For bitcoind, this leads to a very confusing error message: ``` $ ./src/bitcoin-cli restorewallet test123 ~/.bitcoin/regtest/wallets/regtest_wallet/wallet.dat error code: -1 error message: filesystem error: in remove: Directory not empty ["/home/thestack/.bitcoin/wallets/test123"] ``` Even worse, the GUI crashes in such a scenario: ``` libc++abi: terminating with uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in remove: Directory not empty ["/home/thestack/.bitcoin/wallets/foobar"] Abort trap (core dumped) ``` Fix this by simply deleting the whole folder via `fs::remove_all`. With this, the expected error message appears both for the `restorewallet` RPC call and in the GUI (as a message-box): ``` $ ./src/bitcoin-cli restorewallet test123 ~/.bitcoin/regtest/wallets/regtest_wallet/wallet.dat error code: -4 error message: Wallet loading failed. Wallet files should not be reused across chains. Restart bitcoind with -walletcrosschain to override. ``` ACKs for top commit: achow101: ACK 21ad4e26ec320dcecc8961888bc82d0bb72d5ed3 aureleoules: ACK 21ad4e26ec320dcecc8961888bc82d0bb72d5ed3 furszy: utACK 21ad4e26 Tree-SHA512: 313f6494c2fbe823bff9b975cb2d9410bb518977a1e59a5159ee9836bc012947fa50b56be0e41b1a2f50d9c0c7f4fddfdf4fbe479d8a59a6ee44bb389c804abc
2023-01-04Merge bitcoin/bitcoin#26809: compat: use STDIN_FILENO over 0Andrew Chow
585c6722128537f772043ef4c87238e283669b8a compat: use STDIN_FILENO over 0 (fanquake) Pull request description: This is already used throughout this file, and is self-documenting. ACKs for top commit: john-moffett: ACK 585c6722128537f772043ef4c87238e283669b8a achow101: ACK 585c6722128537f772043ef4c87238e283669b8a hebasto: ACK 585c6722128537f772043ef4c87238e283669b8a, I have reviewed the code and it looks OK, I agree it can be merged. kristapsk: utACK 585c6722128537f772043ef4c87238e283669b8a aureleoules: ACK 585c6722128537f772043ef4c87238e283669b8a Tree-SHA512: c0114ae896ba5404be70b804ee9f454d213f1d789c8f5a578c422dd15a308a214e6851fee76c0ec736a212bc86fb33ec17af1b22e5d23422c375ca4458251356
2023-01-04Merge bitcoin/bitcoin#26603: doc: CalculateSequenceLocks: prevHeights ↵glozow
entries are set to 0, not removed f537127271b1f22ee4651915b7b9266e0df72841 doc: fix: prevHeights entries are set to 0, not removed (stickies-v) Pull request description: In [`CalculateSequenceLocks`](https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.h#L69) no items are removed from `prevHeights`, they are just set to 0: https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.cpp#L69-L73 This PR updates the docs to reflect the actual implementation. Seems to have been wrongly documented since introduction in #7184 already ([implementation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R742-R749) and [documentation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R712-R713)) ACKs for top commit: hebasto: ACK f537127271b1f22ee4651915b7b9266e0df72841 Tree-SHA512: 3661501660f6832b2116fd83466ffe95a60b341c14cb09a37489e2a587bea3290b0528690120a0f644c3eea02177aa1fb8968258482fa43b0303e016abb17418
2023-01-04Merge bitcoin/bitcoin#26752: wallet: Remove `mempool_sequence` from ↵glozow
interface methods 55696a0ac30bcfbd555f71cbc8eac23b725f7dcf wallet: remove `mempool_sequence` from `transactionRemovedFromMempool` (w0xlt) bf19069c53501231a2f3ba59afa067913ec4d3b2 wallet: remove `mempool_sequence` from `transactionAddedToMempool` (w0xlt) Pull request description: This PR removes `mempool_sequence` from `transactionRemovedFromMempool` and `transactionAddedToMempool`. `mempool_sequence` is not used in these methods, only in ZMQ notifications. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/26752/commits/55696a0ac30bcfbd555f71cbc8eac23b725f7dcf Tree-SHA512: 621e89230bcb6edfed83e2758601a2b093822fc2dc4e9bfb00487e340f2bc4c5ac3bf6df3ca00b7fe55bb3df15858820f2bf698f403d2e48b915dd9eb47b63e0
2023-01-04Merge bitcoin/bitcoin#26020: test: Change coinselection parameter location ↵Andrew Chow
to make tests independent b942c94d153f83b77ef5d603211252d9abadde95 test: Change coinselection parameter location to make tests independent (yancy) Pull request description: the `subtract_fee_outputs` param is expected to be `true` for all subsequent tests. It should be defined outside of a single test so that if it's removed or changed, all subsequent tests won't fail. Currently if you remove this [test](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L304:L325) the following [test](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L327:L345) fails. This change makes the tests independent. ACKs for top commit: achow101: ACK b942c94d153f83b77ef5d603211252d9abadde95 aureleoules: ACK b942c94d153f83b77ef5d603211252d9abadde95. rajarshimaitra: tACK b942c94d153f83b77ef5d603211252d9abadde95 theStack: ACK b942c94d153f83b77ef5d603211252d9abadde95 Tree-SHA512: 461e19d15351318102ef9f96c68442365d8ca238c48ad7aefe23e8532b33b91dadf6c7840c7894574bccede6da162a55ad7a6f6a330d61a11ce804e68ddc5e9c
2023-01-04Merge bitcoin/bitcoin#25234: bench: add benchmark for wallet ↵Andrew Chow
'AvailableCoins' function. 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81 bench: add benchmark for wallet 'AvailableCoins' function. (furszy) Pull request description: #### Rationale `AvailableCoins` is part of several important flows for the wallet; from RPC commands that create transactions like `fundrawtransaction`, `send`, `walletcreatefundedpsbt`, get the available balance, list the available coins with `listunspent` etc. to GUI connected processes that perform the same or similar actions: tx creation, available balance calculation, present the spendable coins in the coin control dialog. As we are improving this process in #24699, #25005 and there are more structural changes coming on the way. This benchmark aims to ensure us that, at least, there are no regressions (obviously performance improvements are great but, at least for me, this heads into the direction of having a base metric to compare future structural changes). #### Implementation Notes There are 5 new benchmarks, one per wallet supported output type (LEGACY, P2SH_SEGWIT, BECH32, BECH32M), plus a multi-output-type wallet benchmark which contains outputs from all the descriptor types. The test, by default, fills-up the wallet with 1k transactions, 2k outputs. Mainly to not consume much time if the user just want to verify that no substantial regressions were introduced. But, my expectation for those who are focused on this process is to use a much higher number locally to really note the differences across commits. ACKs for top commit: achow101: ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81 hernanmarino: ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81 aureleoules: ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81 Tree-SHA512: d0bb4c165f1efa181b47cb31561e6217eff9135bcd1b6761a7292f9018e456d13d18a1b886c2e2268d35c52f9e1fd8e0f252972424e5c5f00c280620b79c5a1b
2023-01-04refactor: use convenience fn to auto parse non-string parametersstickies-v
Minimizes code duplication and improves function naming by having a single (overloaded) convenience function that both checks if the parameter is a non-string parameter and automatically parses the value if so.