| Age | Commit message (Collapse) | Author |
|---|
|
fa93ef5a8aeae36304c792697a78af2d07fd9f41 refactor: Take Span in SetSeed (MarcoFalke)
Pull request description:
This makes calling code less verbose and less fragile. Also, by adding
the CKey::data() member function, it is now possible to call HexStr()
with a CKey object.
ACKs for top commit:
sipa:
utACK fa93ef5a8aeae36304c792697a78af2d07fd9f41
laanwj:
Code review ACK fa93ef5a8aeae36304c792697a78af2d07fd9f41
theStack:
Code-review ACK fa93ef5a8aeae36304c792697a78af2d07fd9f41
Tree-SHA512: 73fb999320719ad4b9ab5544018a7a083d140545c2807ee3582ecf7f441040a30b5157e85790b6b840af82f002a7faf30bd8162ebba5caaf2067391c43dc7e25
|
|
11115169a14d0d0be5b7b1c3f6fdc9673a9098d9 ci: Build fuzz with libsqlite3-dev (MarcoFalke)
fa7c6efca66627e4c76adecc824f96da220af69c fuzz: Add wallet fuzz test (MarcoFalke)
fa59d2ce5b8d6fe8c610f170a13675c756aef58f refactor: Use local args instead of global gArgs in CWallet::Create (MarcoFalke)
fadb44606f26a80daf4320eee046c9572e85fe3e build: Inline FUZZ_SUITE_LDFLAGS_COMMON (MarcoFalke)
Pull request description:
Initial sketch to fuzz descriptor wallets. Can be improved in the future.
ACKs for top commit:
mjdietzx:
Code review ACK 1111516
Tree-SHA512: b1d2f24504d1ed5f3c6a031210f04c27c13d4e15576c4acbf50ded37ac45f7b7a5c7553e91d81d4a06e9ea73b3d745a552218d3ef3b2932fa5325a8331b0d3fd
|
|
This change suppress the "keypoololdest" field in the getwalletinfo RPC
response for blank descriptor wallets.
|
|
This change gets rid of the magic number 0 in the
DescriptorScriptPubKeyMan::GetOldestKeyPoolTime() function.
No behavior change.
|
|
psbt_wallet_tests/psbt_updater_test
68018e4c3e76f7e5bebf5f90ffd972c7bf01e0a0 test: Avoid excessive locking of `cs_wallet` (Hennadii Stepanov)
7986faf2e09ea85b1d4564ce910f07a4c4de8685 test: Fix segfault in the psbt_wallet_tests/psbt_updater_test (Hennadii Stepanov)
Pull request description:
The dcd6eeb64adb2b532f5003cbb86ba65b3c08a87b commit (bitcoin/bitcoin#23288) introduced an intermittent failure in the `psbt_wallet_tests/psbt_updater_test` unit test. See bitcoin/bitcoin#23368.
The test failure can be easily made reproducible with the following patch:
```diff
--- a/src/scheduler.cpp
+++ b/src/scheduler.cpp
@@ -57,6 +57,8 @@ void CScheduler::serviceQueue()
Function f = taskQueue.begin()->second;
taskQueue.erase(taskQueue.begin());
+ UninterruptibleSleep(100ms);
+
{
// Unlock before calling f, so it can reschedule itself or another task
// without deadlocking:
```
This PR implements an idea which was mentioned in the [comment](https://github.com/bitcoin/bitcoin/issues/23368#issuecomment-953796339):
> Yes, as I said before this looks like a race where the wallet is deleted before stopping the scheduler: [#23368 (comment)](https://github.com/bitcoin/bitcoin/issues/23368#issuecomment-952808824)
>
> IIRC, the order should be:
>
> * stop scheduler
>
> * delete wallet
>
> * delete scheduler
The second commit introduces a refactoring with no behavior change.
Fixes bitcoin/bitcoin#23368.
ACKs for top commit:
mjdietzx:
Code review ACK 68018e4c3e76f7e5bebf5f90ffd972c7bf01e0a0
Tree-SHA512: d9103f6252aab807453628159dec243bc543a2595eecaa04ec761dca3c2370085592c55d6f50967d69a4ac6e8b5827eec30dd9b025132c99b0bb9aa5911ad915
|
|
This makes calling code less verbose and less fragile. Also, by adding
the CKey::data() member function, it is now possible to call HexStr()
with a CKey object.
|
|
ArgsManager flags
c5d7e34bd9a4ad752c5ec88032420e2e90ab17ab scripted-diff: disable unimplemented ArgsManager BOOL/INT/STRING flags (Russell Yanofsky)
b8c069b7a952e326d2d974cc671889d1a3b38aa4 refactor: Add explicit DISALLOW_NEGATION ArgsManager flag to clarify flag usage (Russell Yanofsky)
26a50ab322614bceb5bc62e2c282f83e5987bad8 refactor: Split InterpretOption into Interpret{Key,Value} functions (Russell Yanofsky)
Pull request description:
This is preparation for #16545 or another PR implementing type validation for ArgsManager settings. It fixes misleading usages of existing flags, prevents flags from being similarly misused in the future, and allows validation logic to be added without breaking backwards compatibility.
---
Currently, ALLOW_{INT|BOOL|STRING} flags don't do any real validation, so current uses of these flags are misleading and will also break backwards compatibility whenever these flags are implemented in a future PR (draft PR is #16545).
An additional complication is that while these flags don't do any real settings validation, they do affect whether setting negation syntax is allowed.
Fix this mess by disabling ALLOW_{INT|BOOL|STRING} flags until they are implemented, and adding an unambiguous DISALLOW_NEGATION flag. This is done in three commits, with the first commit cleaning up some code, the second commit adding the DISALLOW_NEGATION flag, and the next commit disabling the ALLOW_{INT|BOOL|STRING} flags.
None of the changes affect behavior in any way.
ACKs for top commit:
ajtowns:
utACK c5d7e34bd9a4ad752c5ec88032420e2e90ab17ab
promag:
Code review ACK c5d7e34bd9a4ad752c5ec88032420e2e90ab17ab, which as the new argument `-legacy`.
Tree-SHA512: cad0e06361e8cc584eb07b0a1f8b469e3beea18abb458c4e43d9d16e9f301b12ebf1d1d426a407fbd96f99724ad6c0eae5be05c713881da7c55e0e08044674eb
|
|
|
|
The bug was introduced in dcd6eeb64adb2b532f5003cbb86ba65b3c08a87b.
|
|
|
|
9ba7c44265a47880585e39d0167d057ba935ff16 refactor: get wallet path relative to wallet_dir (Michael Dietz)
Pull request description:
Now that boost has been updated > 1.60 (see #22320), we can simplify how we get
wallet path relative to wallet_dir by using:
`boost::filesystem::lexically_relative`, removing a TODO.
Test coverage comes from `test/functional/wallet_multiwallet.py`
I first tried this in #20265 which was my first attempted PR, and funny enough exactly 1 year later I'm opening this one to hopefully finally close this.
ACKs for top commit:
ryanofsky:
Code review ACK 9ba7c44265a47880585e39d0167d057ba935ff16. Basically this same code change is made in #20744 commit b70c84348ac7a8e427a1183f894c73e52c734529, so this PR helps simplify that one
lsilva01:
Code Review ACK 9ba7c44
Tree-SHA512: 6ccb91a18bcb52c3ae0c789a94a18fb5be7db7769fd1121552d63f259fbd32b50c3dcf169cec0b02f978321db3bc60eb4b881b8327e9764f32e700236e0d8a35
|
|
Now that boost has been updated > 1.60, we can simplify how we get
wallet path relative to wallet_dir by using:
`boost::filesystem::lexically_relative`
|
|
54011e7aa274bdc1b921440cc8b4623aa1e0d89e refactor: use CWallet const shared pointers when possible (Karl-Johan Alm)
96461989a2de737151bc4fb216221bf49cb53ce6 refactor: const shared_ptrs (Karl-Johan Alm)
Pull request description:
```C++
const std::shared_ptr<CWallet> wallet = x;
```
means we can not do `wallet = y`, but we can totally do `wallet->DestructiveOperation()`, contrary to what that line looks like.
This PR
* introduces a new convention: always use const shared pointers to `CWallet`s (even when we mutate the pointed-to thing)
* uses `const shared_ptr<const CWallet>` everywhere where wallets are not modified
In the future, this should preferably apply to all shared pointers, not limited to just `CWallet`s.
Both of these serve the same purpose: to dispell the misconception that `const shared_ptr<X>` immutates `X`. It doesn't, and it's dangerous to leave this misconception as is, for obvious reasons.
ACKs for top commit:
theStack:
re-ACK 54011e7aa274bdc1b921440cc8b4623aa1e0d89e
Tree-SHA512: 3bf4062fc821751be30770c6b4ead10a016847970f155a0a5156f304347d221b9830840030c2fbfba8cd1e282f4eda45f5b4107fe6df8138afdcb6c2e95a2836
|
|
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
|
|
This commit does not change behavior in any way. See previous commit for
complete rationale, but these flags are being disabled because they
aren't implemented and will otherwise break backwards compatibility when
they are implemented.
-BEGIN VERIFY SCRIPT-
sed -i 's:\(ALLOW_.*\) \(//!< unimplemented\):// \1\2:' src/util/system.h
sed -i '/DISALLOW_NEGATION.*scripted-diff/d' src/util/system.cpp
git grep -l 'ArgsManager::ALLOW_\(INT\|STRING\)' | xargs sed -i 's/ArgsManager::ALLOW_\(INT\|STRING\)/ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION/g'
git grep -l 'ALLOW_BOOL' -- ':!src/util/system.h' | xargs sed -i 's/ALLOW_BOOL/ALLOW_ANY/g'
-END VERIFY SCRIPT-
|
|
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
|
|
While const shared_ptr<X> gives us an immutable shared pointer to a mutable X (we can't set it to some other X later), shared_ptr<const X> gives us a shared pointer to an immutable X. Importantly, we can recast shared_ptr<X> into shared_ptr<const X>, but not the other way around. We do this for two reasons: because it makes the code safer to guarantee the wallet is not modified, and because it further dispells the misconception that const shared_ptr<X> gives immutability to X.
|
|
Introduce convention to use const shared pointers everywhere, unless the shared pointer is modified at some point, which it very rarely is.
We want this convention, as it helps alleviate the misconception that a const shared pointer somehow results in a pointer to an immutable object, which is false.
|
|
|
|
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.
|
|
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.
|
|
For BnB, we only want to check that sets are equivalent with their
values, whereas in knapsack we care about the outpoints.
|
|
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
|
|
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
|
|
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
|
|
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
|
|
|
|
Can be reviewed with --word-diff-regex=.
|
|
Follow-up to:
* commit 700c42b85d20e624bef4228eef062c93084efab5, which replaced pIndex
with block_hash in AddToWalletIfInvolvingMe.
* commit 9700fcb47feca9d78e005b8d18b41148c8f6b25f, which replaced
posInBlock with confirm.nIndex.
|
|
`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).
|
|
|
|
::ChainActive()
a0efe529e4fd053b890450413b9ca5e1bcd8f2c2 Fix outdated comments referring to ::ChainActive() (Samuel Dobson)
Pull request description:
After #21866 there are a few outdated comments referring to `::ChainActive()`, which should instead refer to `ChainstateManager::ActiveChain()`.
ACKs for top commit:
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/23258/commits/a0efe529e4fd053b890450413b9ca5e1bcd8f2c2
Tree-SHA512: 80da19c105ed29ac247e6df4c8e916c3bf3f37230b63f07302114eef9c115add673e9649f0bbe237295be0c6da7b1030b5b93e14daf6768f17ce5de7cf2c9ff2
|
|
For the generic wallet tests, make DescriptorScriptPubKeyMans. There are
still some wallet tests that test legacy wallet things. Those remain
unchanged.
|
|
The subtract fee from outputs assumes that the leftover input amount
will be dropped to fees. However this only happens if that amount is
less than the cost of change. In the event that it is higher than the
cost of change, the leftover amount will actually become a change
output. To avoid this scenario, force a change type which has a high
cost of change.
|
|
|
|
|
|
To avoid issues with test data leaking across tests cases, the global
vCoins and testWallet are removed from coinselector_tests and all of the
relevant functions reworked to not need them.
|
|
fs::path(std::string) constructor and fs::path::string() method
6544ea5035268025207d2402db2f7d90fde947a6 refactor: Block unsafe fs::path std::string conversion calls (Russell Yanofsky)
b39a477ec69a51b2016d3a8c70c0c77670f87f2b refactor: Add fs::PathToString, fs::PathFromString, u8string, u8path functions (Russell Yanofsky)
Pull request description:
The `fs::path` class has a `std::string` constructor which will implicitly convert from strings. Implicit conversions like this are not great in general because they can hide complexity and inefficiencies in the code, but this case is especially bad, because after the transition from `boost::filesystem` to `std::filesystem` in #20744 the behavior of this constructor on windows will be more complicated and can mangle path strings. The `fs::path` class also has a `.string()` method which is inverse of the constructor and has the same problems.
Fix this by replacing the unsafe method calls with `PathToString` and `PathFromString` function calls, and by forbidding unsafe method calls in the future.
ACKs for top commit:
kiminuo:
ACK 6544ea5035268025207d2402db2f7d90fde947a6
laanwj:
Code review ACK 6544ea5035268025207d2402db2f7d90fde947a6
hebasto:
re-ACK 6544ea5035268025207d2402db2f7d90fde947a6, only added `fsbridge_stem` test case, updated comment, and rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22937#pullrequestreview-765503126) review. Verified with the following command:
Tree-SHA512: c36324740eb4ee55151146626166c00d5ccc4b6f3df777e75c112bcb4d1db436c1d9cc8c29a1e7fb96051457d317961ab42e6c380c3be2771d135771b2b49fa0
|
|
when upgrading non-HD to HD
6531599f422524fbbcc43816121e7536cf79d66c test: Add check that newkeypool flushes change addresses too (Samuel Dobson)
84fa19c77a2c8d0d01add2daf18b42af07c17710 Add release notes for keypool flush changes (Samuel Dobson)
f9603ee4e05d7f0bd7d81f5cf24168c1aec8e5b0 Add test for flushing keypool with newkeypool (Samuel Dobson)
6f6f7bb36c492fa76aeda6513be58ca822ea1968 Make legacy wallet upgrades from non-HD to HD always flush the keypool (Samuel Dobson)
2434b1078147e71b09c4c1bf0b7ce3f6729a7713 Fix outdated keypool size default (Samuel Dobson)
22cc797ca5c1e70a4afb8e43f6917b4c9fe74e20 Add newkeypool RPC to flush the keypool (Samuel Dobson)
Pull request description:
This PR makes two main changes:
1) Adds a new RPC `newkeypool` which will entirely flush and refill the keypool.
2) When upgradewallet is called on old, non-HD wallets upgrading them to HD, we now always flush the keypool and generate a new one, to immediately start using the HD generated keys.
This PR is motivated by a number of users with old, pre-compressed-key wallets upgrading them and being confused about why they still can't generate p2sh-segwit or bech32 addresses -- this is due to uncompressed keys remaining in the keypool post-upgrade and being illegal in these newer address formats. There is currently no easy way to flush the keypool other than to call `getnewaddress` a hundred/thousand times or an ugly hack of using a `sethdseed` call.
ACKs for top commit:
laanwj:
re-ACK 6531599f422524fbbcc43816121e7536cf79d66c
meshcollider:
Added new commit 6531599f422524fbbcc43816121e7536cf79d66c to avoid invalidating previous ACKs.
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/23093/commits/6531599f422524fbbcc43816121e7536cf79d66c
Tree-SHA512: 50c79c5d42dd27ab0ecdbfdc4071fdaa1b2dbb2f9195ed325b007106ff19226419ce57fe5b1539c0c24101b12f5e034bbcfb7bbb0451b766cb1071295383d774
|
|
|
|
Fix typo and grammar in the coin selection algorithm's description.
|
|
43568782c23185a0599a6e60d61db4716da1cda1 External input fund support cleanups (Gregory Sanders)
Pull request description:
Minor cleanups to https://github.com/bitcoin/bitcoin/pull/17211
ACKs for top commit:
achow101:
ACK 43568782c23185a0599a6e60d61db4716da1cda1
meshcollider:
utACK 43568782c23185a0599a6e60d61db4716da1cda1
benthecarman:
ACK 43568782c23185a0599a6e60d61db4716da1cda1
Tree-SHA512: 865f8a3804f8c0027f5393a0539041158166a919378f2c3bc99b936843eee2329372bcc2af888fa62babfa5f6baf4f13d4cfef7b4e26a7265a82a908f9719ad6
|
|
fafff132cf4e5c2950d28f63cb4320236d1a5495 doc: Extract FundTxDoc (MarcoFalke)
Pull request description:
No need to duplicate the documentation for the same field(s) three times.
Fix that by de-duplicating it for the fields: conf_target, estimate_mode, replaceable, and solving_data.
Can be reviewed with `--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`.
ACKs for top commit:
fanquake:
ACK fafff132cf4e5c2950d28f63cb4320236d1a5495
Tree-SHA512: 098ddad3904b80b24c9e7b57ca8e807a6ccc3899eac2c9986d71ba3873c2b580bbb95f2fdfbf94b2db02f81c7b0ebf438a90324c23389b7b968ca85ae8475373
|
|
|
|
|
|
|
|
|
|
Synchronize error checking for external inputs
Rename external output Select to SelectExternal
Const FundTransaction variables
|
|
There is no change in behavior. This just helps prepare for the
transition from boost::filesystem to std::filesystem by avoiding calls
to methods which will be unsafe after the transaction to std::filesystem
to due lack of a boost::filesystem::path::imbue equivalent and inability
to set a predictable locale.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Co-authored-by: Kiminuo <kiminuo@protonmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
|
|
|
