b745e149c26507e31cdaed0412dcd9963647568d [docs] Expand help text for importmulti changes (John Newbery)
Pull request description:
Expands the RPC help text for changes to the importmulti RPC method.
Tree-SHA512: e90e5abf66bba3863e7519b5f79c26d18a4d624e6e7878293bdd4ebb57f1a01c67de52e4a5621901a8cb87fb3516264b3b1a826997c7c3c17b11216f1f1a3db0
a6b5ec18f rpc: creates possibility to preserve labels on importprivkey (marcoagner)
Pull request description:
Closes#13087.
As discussed in the issue, this is a feature request instead of a bug report since the behaviour was as intended (i.e. label with default: `''`). With this, the old behaviour is kept while the possibility to achieve the preservation of labels, as expected in the open issue, is added.
Tree-SHA512: b33be50e1e7f62f7ddfae953177ba0926e2d848961f9fac7501c2b513322c0cb95787745d07d137488267bad1104ecfdbe800c6747f94162eb07c976835c1386
a652ba6293ef8d144935dc882b5f0003c987fa22 rpc/wallet: initialize nFeeRequired to avoid using garbage value on failure (Karl-Johan Alm)
Pull request description:
Initialize the `nFeeRequired` variable to avoid using an uninitialized value for errors happening before it is set to 0.
Note: this originally fixed `nFeeRet` in `wallet.cpp`.
ACKs for top commit:
promag:
ACK a652ba6293ef8d144935dc882b5f0003c987fa22.
Sjors:
utACK a652ba6293ef8d144935dc882b5f0003c987fa22
practicalswift:
ACK a652ba6293ef8d144935dc882b5f0003c987fa22 -- patch looks correct
meshcollider:
utACK a652ba6293ef8d144935dc882b5f0003c987fa22
Tree-SHA512: 0d12f1ffd0851ed5ce6d109d2c87f55e8b1d57da297e684feeabb57229200c4078f029c55ca5aa5712bd18e26dda3ce538443dfe68a7a6d504428068f81fded0
2dbfb37b407ed23b517f507d78fb77334142dce5 Fix Char as Bool in interfaces (Jeremy Rubin)
Pull request description:
In a few places in src/wallet/wallet.h, we use a char when semantically we want a bool.
This is kind of an issue because it means we can unserialize the same transaction with different fFromMe flags (as differing chars) and evaluate the following section in wallet/wallet.cpp
```c++
if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe)
{
wtx.fFromMe = wtxIn.fFromMe;
fUpdated = true;
}
```
incorrectly (triggering an fUpdated where both fFromMe values represent true, via different chars).
I don't think this is a vulnerability, but it's just a little messy and unsemantic, and could lead to issues with stored wtxIns not being findable in a map by their hash.
The serialize/unserialize code for bool internally uses a char, so it should be safe to make this substitution.
NOTE: Technically, this is a behavior change -- I haven't checked too closely that nowhere is depending on storing information in this char. Theoretically, this could break something because after this change a tx unserialized with such a char would preserve it's value, but now it is converted to a ~true~ canonical bool.
ACKs for top commit:
achow101:
Code review ACK 2dbfb37b407ed23b517f507d78fb77334142dce5
meshcollider:
Code review ACK 2dbfb37b407ed23b517f507d78fb77334142dce5
Tree-SHA512: 8c0dc9cf672aa2276c694facbf50febe7456eaa8bf2bd2504f81a61052264b8b30cdb5326e1936893adc3d33504667aee3c7e207a194c71d87b3e7b5fe199c9d
faaac5caaab4d5131040292f4ef2404074ad268b RPCTypeCheck bip32derivs arg in walletcreatefunded (Gregory Sanders)
1f0c4282e961baea85d5f74d7493bd7459784391 QA: add basic walletcreatefunded optional arg test (Gregory Sanders)
1f18d7b591ffcc8bb9422a9b728bd9a0d8da6a2a walletcreatefundedpsbt: remove duplicate replaceable arg (Gregory Sanders)
2252ec50085c151e7998ca9a30cda6a33ee862b6 Allow ConstructTransaction to not throw error with 0-input txn (Gregory Sanders)
Pull request description:
1) Previously an empty input argument transaction that is marked for replaceability fails to pass the `SignalsOptInRBF` check right before funding it. Explicitly check for that condition before throwing an error.
2) The rpc call had two separate `replaceable` arguments, each of which being used in mutually exclusive places. I preserved the `options` version to retain compatability with `fundtransaction`.
Tree-SHA512: 26eb0c9e2d38ea51d11f741d61100223253271a084adadeb7e78c6d4e9004636f089e4273c5bf64a41bd7e9ff795317acf30531cb36aeb0d8db9304b3c8270c3
d0a3feea73e0751f325ef7f1de20fa668f27332e Change docs for walletcreatefundedpsbt RPC method (Ivan Vershigora)
Pull request description:
`sequence` field in the list of inputs currently marked as "required". Actually it can be omitted and it's value depends on `locktime` and `options.replaceable` fields. Just the same as in `createpsbt` call.
ACKs for top commit:
achow101:
ACK d0a3feea73e0751f325ef7f1de20fa668f27332e
Tree-SHA512: 3f429a2c2eea283a47fb5002a99f7e2a5ed6f67df9fd895c1ab938256c48a6497ed6ac2673d8fe8968dfb67b939f4a84570899d9faf52f3abd6ec90c0703d1bd
f4b00b70e Import public keys in order (Andrew Chow)
9e1551b9c Test pubkey import to keypool (Andrew Chow)
513719c5f Add option to importmulti add an imported pubkey to the keypool (Andrew Chow)
9b81fd19a Fetch keys from keypool when private keys are disabled (Andrew Chow)
99cccb900 Add a method to add a pubkey to the keypool (Andrew Chow)
Pull request description:
If the wallet has private keys disabled, allow importing public keys into the keypool. A `keypool` option has been added to `importmulti` in order to signal that the keys should be added to the keypool.
Tree-SHA512: e88ea7bf726c13031aa739389a0c2662e6b22a4f9a4dc45b042418c692a950d98f170e0db80eb59e9c9063cda8765eaa85b2927d1790b9625744f7a87bad5fc8
Using wallet flags should be enough here, no need for any additional conditions.
NOTE: Wallets created prior to 4627 have wallet version set to FEATURE_HD even when no HD chain was initialized, that's why they have issues without this fix.
Fixes 4699
0bedcbafd Use a single wallet batch for UpgradeKeyMetadata (Jonas Schnelli)
Pull request description:
Opening wallets (the first time) after #14021 took on my end around 30 seconds due to the keymetadata migration (tested on regtest).
Using a single wallet batch reduces the required time for the migration down to <1s on my system for a default 2k keypool wallet.
Tree-SHA512: f68739e452d382f5294186f47511b94884a1a0868688dd3179034a7e091a67f93bc9dd45cdfc9fa6b1fe90362772b719278012f2f56b752b803c87db8597a7b0
cb3511b9d Add release notes for importing key origin info change (Andrew Chow)
4c75a69f3 Test importing descriptors with key origin information (Andrew Chow)
02d6586d7 Import KeyOriginData when importing descriptors (Andrew Chow)
3d235dff5 Implement a function to add KeyOriginInfo to a wallet (Andrew Chow)
eab63bc26 Store key origin info in key metadata (Andrew Chow)
345bff601 Remove hdmasterkeyid (Andrew Chow)
bac8c676a Add a method to CWallet to write just CKeyMetadata (Andrew Chow)
e7652d3f6 Add WriteHDKeypath function and move *HDKeypath to util/bip32.{h,cpp} (Andrew Chow)
c45415f73 Refactor keymetadata writing to a separate method (Andrew Chow)
Pull request description:
This PR allows for key origin data as defined by the descriptors document to be imported to the wallet when importing a descriptor using `importmulti`. This allows the `walletprocesspsbt` to include the BIP 32 derivation paths for keys that it is watching that are from a different HD wallet.
In order to make this easier to use, a new field `hdmasterkeyfingerprint` has been added to `getaddressinfo`. Additionally I have removed `hdmasterkeyid` as was planned. I think that this API change is fine since it was going to be removed in 0.18 anyways. `CKeyMetadata` has also been extended to store key origin info to facilitate this.
Tree-SHA512: 9c7794f3c793da57e23c5abbdc3d58779ee9dea3d53168bb86c0643a4ad5a11a446264961e2f772f35eea645048cb60954ed58050002caee4e43cd9f51215097
* style: use clang-tidy style named parameters
* refactor: make IsTimeOutOfBounds testable by having current time be a parameter
* style: use x-> not (*x).
* refactor: make SelectCoinsGroupedByAddresses return a vector, remove out param
previous semantics was return false if the vecTally vector was empty. Now we just let the caller check if it is empty or not
* refactor: fix some sign-compare warnings
* refactor: consistently pre-declare stuff as struct / class inline with underlying type
* refactor: don't return const bool
* refactor: use ref to string
* refactor: use = default for CompactTallyItem
* refactor: adjust "initialization" ordering
* refactor: adjust how we handle negatives in GetProjectedMNPayees, use std::min
* refactor: don't bind a reference to a temporary value
* refactor: use a ref
* refactor: ensure attempt in SelectMemberForRecovery is non-negative.
* refactor: remove unused this capture
* refactor: fix numerous sign-compare warnings
* refactor: more consistently use size_t, use empty()
0d101a340c44841cbbc5982d55354b1787bc39e2 test: Add test for maxtxfee option (MarcoFalke)
177550101b600ccb32886695326eb72cd9752c8b wallet: Remove unreachable code in CreateTransaction (MarcoFalke)
5c1b9714cb0a13be28324f91f4ec9ca66a1de8c7 wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa)
Pull request description:
Follow up to #16257, this PR makes `bumpfee` aware of `-maxtxfee`.
It also prevents dangling locked unspents when calling `fundrawtransaction` - because the previous check was after `LockCoin`.
ACKs for top commit:
MarcoFalke:
re-ACK 0d101a340c44841cbbc5982d55354b1787bc39e2, only change is small test fixup
Tree-SHA512: 3464b24ae7cd4e72ed41438c6661828ba1304af020f05da62720b23668ae734e16cf47c6d97e150cc84ef631ee099b16fc786c858f3d089905845437338fd512
7ba2d57852 Fix ListCoins test failure due to unset g_wallet_allow_fallback_fee (Russell Yanofsky)
Pull request description:
New global variable was introduced in #11882 and not setting it causes:
```
wallet/test/wallet_tests.cpp(638): error in "ListCoins": check wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy) failed
wallet/test/wallet_tests.cpp(679): error in "ListCoins": check list.begin()->second.size() == 2 failed [1 != 2]
wallet/test/wallet_tests.cpp(686): error in "ListCoins": check available.size() == 2 failed [1 != 2]
wallet/test/wallet_tests.cpp(705): error in "ListCoins": check list.begin()->second.size() == 2 failed [1 != 2]
```
It's possible to reproduce the failure reliably by running:
```
src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/ListCoins
```
Failures happen nondeterministically because boost test framework doesn't run tests in a specified order, and tests that run previously can set the global variables and mask the bug.
This is similar to bugs #12150 and #12424. Example travis failures are:
https://travis-ci.org/bitcoin/bitcoin/jobs/348296805#L2676https://travis-ci.org/bitcoin/bitcoin/jobs/348362560#L2769https://travis-ci.org/bitcoin/bitcoin/jobs/348362563#L2824
Tree-SHA512: ca37b554a75c12ac2d534de62bf74eb9e0b29e4399ebf1fa10053a40887e55e9e7135f754a01e5a67499cc8677ae226542146b370b1e83d08bb63d79ff379073
fa4a605a4c611abe9af4c18aab20f4d1d039170f Remove wallet settings from chainparams (MarcoFalke)
Pull request description:
Feels a bit odd to have wallet setting in the chainparams, so remove them from there
ACKs for top commit:
promag:
ACK fa4a605a4c611abe9af4c18aab20f4d1d039170f, missed s/2018/2019?
practicalswift:
utACK fa4a605a4c611abe9af4c18aab20f4d1d039170f
darosior:
ACK fa4a605a4c611abe9af4c18aab20f4d1d039170f
Tree-SHA512: 2b3a5ee85d36af290d7db80bed1339e3c684607f1ce61cc65c906726e9174e40325fb1f67a34d8780f2a61fa39a1785e7c3a1cef5b6d6c364f38db5300cdbe3a
3f592b8 [QA] add wallet-rbf test (Jonas Schnelli)
8222e05 Disable wallet fallbackfee by default on mainnet (Jonas Schnelli)
Pull request description:
Removes the default fallback fee on mainnet (but keeps it on testnet/regtest).
Transactions using the fallbackfee in case the fallback fee has not been set are getting rejected.
Tree-SHA512: e54d2594b7f954e640cc513a18b0bfbe189f15e15bdeed4fe02b7677f939bca1731fef781b073127ffd4ce08a595fb118259b8826cdaa077ff7d5ae9495810db
05b56d1c937b7667ad51400d2f9fb674af72953f [wallet] Remove CMerkleTx serialization logic (John Newbery)
783a76f23ba4f33b6e6f609eaf3bf41afd9bcd6f [wallet] Flatten CWalletTx class hierarchy (John Newbery)
b3a9d179f23f654b8fba63924bbca5fd31ad4bb0 [wallet] Move CMerkleTx functions into CWalletTx (John Newbery)
Pull request description:
CMerkleTx is only used as a base class for
CWalletTx. It was previously also used for vtxPrev which
was removed in 93a18a3650.
This PR moves all of the CMerkleTx members and logic
into CWalletTx. The CMerkleTx class is kept for deserialization
and serialization of old wallet files.
This makes the refactor in #15931 cleaner.
ACKs for top commit:
laanwj:
ACK 05b56d1c937b7667ad51400d2f9fb674af72953f. Looks good to me.
Tree-SHA512: 3d3a0069ebb536b12a328f1261e7dc55158a71088d445ae4b4ace4142c432dc296f58c8183b1922e54a60b8cc77e9d17c3dce7478294cd68693594baacf2bab3
* Make numerous coinjoin functions constexpr
* remove empty function
Signed-off-by: Pasta <pasta@dashboost.org>
* remove InitStandardDenominations calls
Signed-off-by: Pasta <pasta@dashboost.org>
* use default constructors, add constexpr to CCoinJoinStatusUpdate constructor
Signed-off-by: Pasta <pasta@dashboost.org>
* remove default constructor
Signed-off-by: Pasta <pasta@dashboost.org>
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift)
eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift)
Pull request description:
Make reasoning about dependencies easier by not including unused dependencies.
Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real.
As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed:
```
$ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \
sed 's%^%src/%g' | xargs cat | wc -l
51393
```
Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-)
ACKs for commit 67f4e9:
Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
99e88a372 rpc: Remove dependency on interfaces::Chain in SignTransaction (Antoine Riard)
Pull request description:
Assuming wallet RPCs and node RPCs will go into different processes, signrawtransactionwithkey doesn't need to access Coins via interfaces::Chain, it may use directly utility in node/coins.cpp
Obviously will need rebase after #15638
Tree-SHA512: 42ee8fcbcd38643bbd82210db6f68249bed5ee036a4c930a1db534d0469a133e287b8869c977bf0cc79a7296dde04f72adb74d24e1cd20f4a280f4c2b7fceb74
318b1f7af1 [wallet] Close bdb when flushing wallet. (John Newbery)
Pull request description:
bdb would not be closed when closing the wallet in wallet-tool. Fix this by calling wallet->flush with true.
Tree-SHA512: f722e527e4806eca5254221e944f57853d11bf89a9264309fa558a6cc2b23feefb7bb2963e87b4fad9cfb31ac4cffe563688988e0614a481a8ff1d393aceb132
fa4c8679ed94f215ce895938f7c3c169a2ce101e rpc: Avoid creating non-standard raw transactions (MarcoFalke)
Pull request description:
Multiple OP_RETURN outputs in a transaction are not standard and unlikely to be relayed, so avoid creating them.
Apart from that, the logic was broken in that it duplicated the same hex-data for each data output: Closes#14868.
Tree-SHA512: b08d08062b5622e8a7b497e490ccaf53b06e844c863fda3bf3f932a98684a809e8341aeb98232059a795afb32d8770a6c5591a66f8e6ee372b672af245607887
f471a3be00c2b6433b8c258b716982c0539da13f scripted diff: Improve invalid vout value rpc error message (Nima Yazdanmehr)
Pull request description:
Since the `vout` value can start at `0`, the error message for *negative* values can be improved to something like: `vout cannot be negative`.
ACKs for top commit:
fanquake:
ACK f471a3be00c2b6433b8c258b716982c0539da13f
promag:
Code review ACK f471a3be00c2b6433b8c258b716982c0539da13f.
Tree-SHA512: fbdee3d0ddd5b58eb93934a1217b44e125a9ad39e672b1f35c7609c6c5fcf45ae1b731d3d6135b7225d98792dbfc34a50907b8c41274a5b029d7b5c59f886560
765c0b364d refactor: combine Chain::findFirstBlockWithTime/findFirstBlockWithTimeAndHeight (Antoine Riard)
Pull request description:
As suggested in #14711, pass height to CChain::FindEarliestAtLeast to
simplify Chain interface by combining findFirstBlockWithTime and
findFirstBlockWithTimeAndHeight into one
ACKs for commit 765c0b:
jnewbery:
utACK 765c0b364d41e9a251c3f88cbe203645854fd790. Nice work @ariard!
ryanofsky:
utACK 765c0b364d41e9a251c3f88cbe203645854fd790. Looks good, thanks for implementing the suggestion!
Tree-SHA512: 63f98252a93da95f08c0b6325ea98f717aa9ae4036d17eaa6edbec68e5ddd65672d66a6af267b80c36311fffa9b415a47308e95ea7718b300b685e23d4e9e6ec
184f8785f wallet_bumpfee.py: add test for change key preservation (Gregory Sanders)
d08becff8 add functional tests for feerate bumpfee with adding inputs (Gregory Sanders)
0ea47ba7b generalize bumpfee to add inputs when needed (Gregory Sanders)
Pull request description:
When targeting a feerate using `bumpfee`, call a new function that directly uses `CWallet::CreateTransaction` and coin control to get the desired result. This allows us to get a superset of previous behavior, with an arbitrary RBF bump of a transaction provided it passes the preconditional checks and spare confirmed utxos are available.
Note(s):
0) The coin selection will use knapsack solver for the residual selection.
1) This functionality, just like knapsack coin selection in general, will hoover up negative-value inputs when given the chance.
2) Newly added inputs must be confirmed due to current Core policy. See error: `replacement-adds-unconfirmed`
3) Supporting this with `totalFee` is difficult since the "minimum total fee" option in `CreateTransaction` logic was (rightly)taken out in #10390 .
ACKs for commit 184f87:
jnewbery:
utACK 184f8785f710d58d9ef82e611591c9cbff5ab89d
Tree-SHA512: fb6542bdfb2c6010e328ec475cf9dcbff4eb2b1a1b27f78010214534908987a5635797196fa05edddffcbcf2987335872dc644a99261886d5cbb34a8f262ad3e
0000ff0aa763e3be524ac4537a41048a26529fb2 txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWN (MarcoFalke)
Pull request description:
The `remove*` methods set the removal reason to `UNKNOWN` by default. This is nowhere used; Except in tests, where the value doesn't matter. Fix that by removing the confusing default.
ACKs for top commit:
practicalswift:
utACK 0000ff0aa763e3be524ac4537a41048a26529fb2
promag:
ACK 0000ff0aa763e3be524ac4537a41048a26529fb2.
jonasschnelli:
utACK 0000ff0aa763e3be524ac4537a41048a26529fb2
Tree-SHA512: ffc8b35dd3291a81225171577c743c8bb2645638cab02960b6361174cb68afd739aaab7ab8661d65de5750d37daf16bb7eee9338958d8609093a8d46c2ada1ab
a607c9ae4c2730fca5ce340c400c95c87e498a7c [Doc] importmulti: add missing description of keypool option (David A. Harding)
Pull request description:
Option was added in #14075 but not documented there.
CC: @achow101
Tree-SHA512: dcb6421fa1be3d733d7a00c1b57ffd591fe76c02d1c479e729089c118bec52f53bd7ebdb5454b3b1c7603ab189e91682a688b673a7f6b04fa8610c4249711217
* Remove KeePass integration
This integration is not actively supported. It has zero tests, little documentation, and has not really been actively maintained. As far as I can tell, noone uses this integration, and even if they do, they will simply have to copy/paste password from keepass instead of using this integration.
* continued
662d1171d9e29964b039ba4c5bc8a2304426c003 Add option to create an encrypted wallet (Andrew Chow)
Pull request description:
This PR adds a new `passphrase` argument to `createwallet` which will create a wallet that is encrypted with that passphrase.
This is built on #15226 because it needs to first create an empty wallet, then encrypt the empty wallet and generate new keys that have only been stored in an encrypted state.
ACKs for commit 662d11:
laanwj:
utACK 662d1171d9e29964b039ba4c5bc8a2304426c003
jnewbery:
Looks great. utACK 662d1171d9e29964b039ba4c5bc8a2304426c003
Tree-SHA512: a53fc9a0f341eaec1614eb69abcf2d48eb4394bc89041ab69bfc05a63436ed37c65ad586c07fd37dc258ac7c7d5e4f7f93b4191407f5824bbf063b4c50894c4a
7687f7873 [wallet] Support creating a blank wallet (Andrew Chow)
Pull request description:
Alternative (kind of) to #14938
This PR adds a `blank` parameter to the `createwallet` RPC to create a wallet that has no private keys initially. `sethdseed` can then be used to make a clean wallet with a custom seed. `encryptwallet` can also be used to make a wallet that is born encrypted.
Instead of changing the version number as done in #14938, a wallet flag is used to indicate that the wallet should be blank. This flag is set at creation, and then unset when the wallet is no longer blank. A wallet becomes non-blank when a HD seed is set or anything is imported. The main change to create a blank wallet is primarily taken from #14938.
Also with this, the term "blank wallet" is used instead of "empty wallet" to avoid confusion with wallets that have balance which would also be referred to as "empty".
This is built on top of #15225 in order to fix GUI issues.
Tree-SHA512: 824d685e11ac2259a26b5ece99c67a7bda94a570cd921472c464243ee356b7734595ad35cc439b34357135df041ed9cba951e6edac194935c3a55a1dc4fcbdea
084e17cebd424b8e8ced674bc810eef4e6ee5d3b Remove unused includes (practicalswift)
Pull request description:
As requested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/16273#issuecomment-521332089:
This PR removes unused includes.
Please note that in contrast to #16273 I'm limiting the scope to the trivial cases of pure removals (i.e. no includes added) to make reviewing easier.
I'm seeking "Concept ACK":s for this obviously non-urgent minor cleanup.
Rationale:
* Avoids unnecessary re-compiles in case of header changes.
* Makes reasoning about code dependencies easier.
* Reduces compile-time memory usage.
* Reduces compilation time.
* Warm fuzzy feeling of being lean :-)
ACKs for top commit:
ryanofsky:
Code review ACK 084e17cebd424b8e8ced674bc810eef4e6ee5d3b. PR only removes include lines and it still compiles. In the worst case someone might have to explicitly add an include later for something now included implicitly. But maybe some effort was taken to avoid this, and it wouldn't be a tragedy anyway.
Tree-SHA512: 89de56edc6ceea4696e9579bccff10c80080821685b9fb4e8c5ef593b6e43cf662f358788701bb09f84867693f66b2e4db035b92b522a0a775f50b7ecffd6a6d
80031045fc6435ef4eb5dd1aee773d938c57a0fd Clarify includeWatching for fundrawtransaction (Steven Roose)
Pull request description:
Might be sufficient to solve https://github.com/bitcoin/bitcoin/issues/16396, https://github.com/bitcoin/bitcoin/issues/7879 and https://github.com/bitcoin/bitcoin/issues/14405.
ACKs for top commit:
Sjors:
ACK 8003104. This will always be confusing, but at least it gives a bunch more clues for the user to google.
Tree-SHA512: 9b8002c259c50f93d89fc5574105aae6152858d8d45c07b4c3d5b7023adafe73c7a98a290874ff3fbbb7dfad2ac1bdf4acb8769a2a1c14e38484922f44e84e54
510c6532ba Extract ParseDescriptorRange (Ben Woosley)
Pull request description:
So as to be consistently informative when the checks fail, and
to protect against unintentional divergence among the checks.
ACKs for commit 510c65:
meshcollider:
Oh apologies, yes. Thanks :) utACK 510c6532ba
MarcoFalke:
utACK 510c6532bae9abc5beda1c126c945923a64680cb
sipa:
utACK 510c6532bae9abc5beda1c126c945923a64680cb
Tree-SHA512: b1f0792bfaa163890a20654a0fc2c4c4a996659916bf5f4a495662436b39326692a1a0c825caafd859e48c05f5dd1865c4f7c28092be5074edda3c94f94f9f8b
ca253f6ebf Make deriveaddresses use stop/[start,stop] notation for ranges (Pieter Wuille)
1675b7ce55 Use stop/[start,stop] notation in importmulti desc range (Pieter Wuille)
4566011631 Add support for stop/[start,stop] ranges to scantxoutset (Pieter Wuille)
6b9f45e81b Support ranges arguments in RPC help (Pieter Wuille)
7aa6a8aefb Add ParseRange function to parse args of the form int/[int,int] (Pieter Wuille)
Pull request description:
This introduces a consistent notation for RPC arguments in `scantxoutset`, `importmulti`, and `deriveaddresses`, either:
* `"range" : int` to just specify the end of the range
* `"range" : [int,int]` to specify both the begin and the end of the range.
For `scantxoutset`, this is a backward compatible new feature. For the two other RPCs, it's an incompatible change, but neither of them has been in a release so far. Because of that non-released reason, this only makes sense in 0.18, in my opinion.
I suggest this as an alternative to #15496, which only makes `deriveaddresses` compatible with `importmulti`, but not with the existing `scantxoutset` RPC. I also think `[int,int]` is more convenient than `{"start":int,"stop":int}`.
I realize this is technically a feature added to `scantxoutset` after the feature freeze. If desired, I'll drop the `scantxoutset` changes.
Tree-SHA512: 1cbebb90cf34f106786dbcec7afbf3f43fb8b7e46cc7e6763faf1bc1babf12375a1b3c3cf86ee83c21ed2171d99b5a2f60331850bc613db25538c38b6a056676
9a841696c1e7147e259e5a387566e461abc144ec tests: Reduce compilation time and unneccessary recompiles by removing unused includes in tests (practicalswift)
Pull request description:
Reduce compilation time and unneccessary recompiles by removing unused includes in tests.
A subset of #16273 ("refactor: Reduce total compilation time by 2% and avoid unnecessary recompiles by removing unused includes") as requested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/16273#issuecomment-505022643.
ACKs for top commit:
Sjors:
ACK 9a84169 on macOS 10.14.5 (I rebased on #16289)
Tree-SHA512: bcb6ecffef689a9839bee1a5cb93abe83db1f30819a54226c5630fee456b5a5d187507d06861454adfda939c3556a975113f97662e415cb47fa0327ea4fd09fb
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
d6b3640ac732f6f66a8cb6761084d1beecc8a876 [test] walletcreatefundedpsbt: check RBF is disabled when -walletrbf=0 (Sjors Provoost)
9ed062b5685eb6227d694572cb0f7bfbcc151b36 [doc] rpc: remove "fallback to" from RBF default help (Sjors Provoost)
4fcb698bc2bb74171cd3a14b94f9882d8e19e9fb [rpc] walletcreatefundedpsbt: use wallet default RBF (Sjors Provoost)
Pull request description:
The `walletcreatefundedpsbt` RPC call currently ignores `-walletrbf` and defaults to not use RBF. This PR fixes that.
This PR also replaces UniValue in `ConstructTransaction` with a `bool` in preparation of moving this helper method out of the RPC codebase entirely. This may be a bit overkill, but does slightly simplify it.
Fixes#15878
ACKs for top commit:
achow101:
Code Review ACK d6b3640ac732f6f66a8cb6761084d1beecc8a876
l2a5b1:
re-ACK d6b3640
MarcoFalke:
ACK d6b3640ac732f6f66a8cb6761084d1beecc8a876
Tree-SHA512: 55b9bccd1ef36b54f6b34793017dc0721103099ad3761b3b04862291ee13d6915915d4dbb1a8567924fa56e5e95dfe10eec070e06701610e70c87f8ea92b2a00
96b6dd468a4cb6077d1a2267d620d99d39aac7d0 Remove redundant pre-TopUpKeypool checks (Gregory Sanders)
Pull request description:
TopUpKeypool already has a quick check for `IsLocked()`
ACKs for top commit:
achow101:
ACK 96b6dd468a4cb6077d1a2267d620d99d39aac7d0 Reviewed the diff and checked that the `if (!IsLocked()) TopUpKeypool()` pattern is changed everywhere.
Tree-SHA512: 36f5ae1be611404656ac855763e569fd3b5e932db8170f39ebda74300aa02062774b2c28ce6cf00f2ccc0e3550de58df36efa9097e24f0a51f2809b8a489c95a
a2a6c8f4535c0c3c5f05714d64b238fc5a839233 rpc: remove duplicate solvable field from getaddressinfo (fanquake)
Pull request description:
Also added optional to `iscompressed`.
Tree-SHA512: 28442a9dbfb2a9992b9b57142fa13d374d39444f04ae63460cb6330d896160cfd4b9651a3e231893eac3142ce55eff597a54cbafd3b57ffa46d3711c64044acb
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
df0e97ccb1 RPC: Hint for importmulti in help output of importpubkey and importaddress (Kristaps Kaupe)
Pull request description:
Similar to #12702. Hint for `importmulti` also in help output of `importpubkey` and `importaddress`.
ACKs for commit df0e97:
promag:
utACK df0e97ccb13a28825a2731b95d2bb17f355f6920.
jonasschnelli:
utACK df0e97ccb13a28825a2731b95d2bb17f355f6920
Tree-SHA512: db7358d7f4d463a50874e605bbca35a1a40dbefbb1d35cf51fe2f2aa34bef90c3ca398f4ffbcb9d7d43887a03eb8d81b6ef59066a3c7eda18a7eea876f6592e7
fabc57e07dc34c103552cb51c9277bb48ef97739 test: Log to debug.log in all tests (MarcoFalke)
fa4a04a5a942d582c62773d815c7e1e9897975d0 test: use common setup in gui tests (MarcoFalke)
fad3d2a624377de4b0311e6ddd446c36dafd1ddc test: Create data dir in BasicTestingSetup (MarcoFalke)
Pull request description:
This makes it easier to debug a frozen test or a test that failed. To debug a failed test, remove the line `fs::remove_all(m_path_root);`.
The pull is done in three commits:
* Create a datadir for every unit test once (and only once). This requires the `SetDataDir` function to go away.
* Use the common setup in the gui unit tests. Some of those tests are testing the init sequence, so we'd have to undo some of what the testing setup did.
* Log to the debug.log in all tests
ACKs for top commit:
laanwj:
ACK fabc57e07dc34c103552cb51c9277bb48ef97739
Tree-SHA512: 73444210b88172669e2cd22c2703a1e30e105185d2d5f03decbdedcfd09c64ed208d3716c59c8bebb0e44214cee5c8095e3e995d049e1572ee98f1017e413665
* merge 15638: Move CheckTransaction from lib_server to lib_consensus
* merge 15638: Move policy settings to new src/policy/settings unit
* merge 15638: Move rpc utility methods to rpc/util
* merge 15638: Move rpc rawtransaction util functions to rpc/rawtransaction_util.cpp
* merge 15638: Move several units into common libraries
* merge 15638: Move wallet load functions to wallet/load unit
* merge 15638: Document src subdirectories and different libraries
* [build] Add several util units (cleanup)
* build: resolve missing declarations by re-specifying headers
