Commit graph

2198 commits

Author SHA1 Message Date
Luke Dashjr
bca8665d08 scripted-diff: Wallet: Rename incorrectly named *UsedDestination
These functions are used to mark/check if a key of our own has been used to spend (and only for avoid-reuse wallets), which has nothing to do with the destination/address itself.
Give them more accurate names to avoid confusion.

-BEGIN VERIFY SCRIPT-
sed -i -e 's/UsedDestination/SpentKey/g' $(git grep -l 'UsedDestination' ./src)
-END VERIFY SCRIPT-
2020-02-21 21:16:40 +00:00
fanquake
56fc2dfcc3
Merge #18122: rpc: update validateaddress RPCExamples to bech32
7f1475c711 rpc: update validateaddress RPCExamples to bech32 (Sebastian Falbesoner)

Pull request description:

  Another small step to get rid of legacy addresses in the RPC help texts and by that encourage the use of bech32 addresses by default. The (invalid) address is the same as in the `getaddressinfo` RPC (see 2ee0cb3330, kudos to jonatack!), I don't think it adds any value to have a different example address per RPC.

ACKs for top commit:
  fanquake:
    ACK 7f1475c711
  MarcoFalke:
    ACK 7f1475c711

Tree-SHA512: 2350f61fa942a9053f9f5c860ea446965dc7209c71c81bdb98a859d03ca23b225ad72c9c506e4a55c8d8988823d9cfbe808c1a452a1eeadb70ab186b146dd4ca
2020-02-20 20:28:46 +08:00
Samuel Dobson
68e841e0af
Merge #18067: wallet: Improve LegacyScriptPubKeyMan::CanProvide script recognition
a304a3632f Revert "Store p2sh scripts in AddAndGetDestinationForScript" (Russell Yanofsky)
eb7d8a5b07 [test] check for addmultisigaddress regression (Sjors Provoost)
005f8a92cc wallet: Improve LegacyScriptPubKeyMan::CanProvide script recognition (Russell Yanofsky)

Pull request description:

  Make `LegacyScriptPubKeyMan::CanProvide` method able to recognize p2sh scripts when the redeem script is present in the `mapScripts` map without the p2sh script also having to be added to the `mapScripts` map. This restores behavior prior to #17261, which I think broke backwards compatibility with old wallet files by no longer treating addresses created by `addmultisigaddress` calls before #17261 as solvable.

  The reason why tests didn't fail with the CanProvide implementation in #17261 is because of a workaround added in 4a7e43e846 "Store p2sh scripts in AddAndGetDestinationForScript", which masked the problem for new `addmultisigaddress` RPC calls without fixing it for multisig addresses already created in old wallet files.

  This change adds a lot of comments and allows reverting commit 4a7e43e846 "Store p2sh scripts in AddAndGetDestinationForScript", so the `AddAndGetDestinationForScript()` function, `CanProvide()` method, and `mapScripts` map should all be more comprehensible

ACKs for top commit:
  Sjors:
    re-ACK a304a3632f (rebase, slight text changes and my test)
  achow101:
    re-ACK a304a3632f
  meshcollider:
    utACK a304a3632f

Tree-SHA512: 03b625220c49684c376a8062d7646aeba0e5bfe043f977dc7dc357a6754627d594e070e4d458d12d2291888405d94c1dbe08c7787c318374cedd5755e724fb6e
2020-02-19 14:28:41 +13:00
Wladimir J. van der Laan
051439813e
Merge #13339: wallet: Replace %w by wallet name in -walletnotify script
4e9efac678 test: Check wallet name in -walletnotify script (João Barbosa)
9a5b5ee81f wallet: Replace %w by wallet name in -walletnotify script (João Barbosa)

Pull request description:

  Fixes #13237.

ACKs for top commit:
  laanwj:
    ACK 4e9efac678

Tree-SHA512: 189dd1c785485f2e974d7c12531851b2a977778b3b954aa95efd527322ba3345924cfd587fb9c90b0fa979202af0ab2d90e53d125fe266a36c94f757e4176203
2020-02-17 11:59:23 +01:00
MarcoFalke
263f53e2d0
Merge #18098: scripted-diff: Add missing spaces in RPCResult, Normalize type names
fad027fb0c scripted-diff: Add missing spaces in RPCResult, Fix type names (MarcoFalke)

Pull request description:

  This makes the rendered diff smaller when the RPCResult is machine generated later on (Previous attempts: #14601 and #14459)

ACKs for top commit:
  Sjors:
    ACK fad027fb0c

Tree-SHA512: 48afd571b1cd349ca0b29bb444c1c7cda657e07dd96c610d479f931ccd938186aec98e533d0552b5b10afc9a3d7b911359260a49448e8e1106e3647b2c71f3ba
2020-02-16 17:26:21 -08:00
Wladimir J. van der Laan
470664f2b7
Merge #17746: refactor: rpc: Remove vector copy from listtransactions
25bc17fceb refactor: rpc: Remove vector copy from listtransactions (João Barbosa)

Pull request description:

  Current approach
   - copy accumulated `ret` vector to `arrTmp`
   - drop unnecessary elements from `arrTmp`
   - reverse `arrTmp`
   - clear `ret`
   - copy `arrTmp` to the `ret`

  New approach
   - create a vector from the accumulated `ret` with just the necessary elements already reversed
   - copy it to the result

  This PR doesn't change behavior.

ACKs for top commit:
  ryanofsky:
    Code review ACK 25bc17fceb. Just comment and commit message tweaks since last review

Tree-SHA512: 87906561e3accdbdb0f4a8194cbcd76ea53ae53d0ce135b90bc54a5f77e300b14ef08505e7daf1fe52426f135442a743da5a027416a769bd454922357cebe7c0
2020-02-13 18:50:02 +01:00
João Barbosa
25bc17fceb refactor: rpc: Remove vector copy from listtransactions
No change in behavior.
2020-02-13 15:43:35 +00:00
Sebastian Falbesoner
7f1475c711 rpc: update validateaddress RPCExamples to bech32
also contains the following changes:
- rpc: factor out example bech32 address for RPCExamples
- doc: update developer notes wrt RPCExamples addresses
 (mention the EXAMPLE_ADDRESS constant as an example for an invalid bech32
  address suitable for RPCExamples help documentation)
2020-02-13 12:57:37 +01:00
Russell Yanofsky
005f8a92cc wallet: Improve LegacyScriptPubKeyMan::CanProvide script recognition
Make LegacyScriptPubKeyMan::CanProvide method able to recognize p2sh scripts
when the redeem script is present in the mapScripts map without the p2sh script
also having to be added to the mapScripts map. This restores behavior prior to
https://github.com/bitcoin/bitcoin/pull/17261, which I think broke backwards
compatibility with old wallet files by no longer treating addresses created by
`addmultisigaddress` calls before #17261 as solvable.

The reason why tests didn't fail with the CanProvide implementation in #17261
is because of a workaround added in 4a7e43e846
"Store p2sh scripts in AddAndGetDestinationForScript", which masked the problem
for new `addmultisigaddress` RPC calls without fixing it for multisig addresses
already created in old wallet files.

This change adds a lot of comments and allows reverting commit
4a7e43e846 "Store p2sh scripts in
AddAndGetDestinationForScript", so the AddAndGetDestinationForScript() function,
CanProvide() method, and mapScripts map should all be more comprehensible
2020-02-12 11:48:30 -05:00
MarcoFalke
fad027fb0c
scripted-diff: Add missing spaces in RPCResult, Fix type names
This makes the rendered diff smaller when the RPCResult is machine
generated later on

-BEGIN VERIFY SCRIPT-
 # Add space after dictionary key and before colon
 sed -i --regexp-extended -e 's/(^ +" +\\"[a-zA-Z_]+\\"): ?/\1 : /g' $(git grep -l '\\":')
 # Rename (array) to (json array)
 sed -i -e 's/ (array) / (json array) /g' $(git grep -l '(array)' ./src)
 # Rename (object) to (json object)
 sed -i -e 's/ (object) / (json object) /g' $(git grep -l '(object)' ./src)
 # Rename (bool) to (boolean)
 sed -i -e 's/ (bool) / (boolean) /g' $(git grep -l '(bool)' ./src)
 # Rename (int) to (numeric)
 sed -i -e 's/  (int) /  (numeric) /g' $(git grep -l '(int)' ./src)
-END VERIFY SCRIPT-
2020-02-09 05:12:43 -08:00
MarcoFalke
75fb37ce68
Merge #18032: rpc: Output a descriptor in createmultisig and addmultisigaddress
19a354b11f Output a descriptor in createmultisig and addmultisigaddress (Andrew Chow)

Pull request description:

  Give a descriptor from `createmultisig` and `addmultisigaddress`.

  Extracted from #16528 with `addmultisgaddress` and tests added.

ACKs for top commit:
  Sjors:
    tACK 19a354b11f
  MarcoFalke:
    ACK 19a354b11f
  promag:
    Code review ACK 19a354b11f.
  meshcollider:
    utACK 19a354b11f

Tree-SHA512: e813125fbbc358ea8d45b1748de16a29a94efd83175b748fb8fa3b0bfc8e783ed36b6c554d84f5d4ead1ba252a83a3e937b6c3f75da7b8d3b4e55f94d6013771
2020-02-09 04:55:45 -08:00
Wladimir J. van der Laan
712b7d9b47
Merge #17804: doc: Misc RPC help fixes
fa5c6622c8 doc: Use proper RPC help syntax in importmulti (MarcoFalke)
fab63111be doc: Remove duplicate "comment" from listsinceblock RPC help (MarcoFalke)
fa04cd6cfc doc: Properly document proxy_randomize_credentials as bool in getnetworkinfo (MarcoFalke)
fa9dec7c39 doc: Fix syntax error (trailing square bracket) in finalizepsbt (MarcoFalke)
faff5a60ed doc: Fix syntax error (trailing square bracket) in walletprocesspsbt (MarcoFalke)
fa0545901d doc: Add missing "optional" to "long" estimaterawfee RPC help (MarcoFalke)

Pull request description:

  This fixes documentation of the following RPCs:

  * estimaterawfee (hidden)
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/walletprocesspsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/rawtransactions/finalizepsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/network/getnetworkinfo/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/listsinceblock/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/

  <!-- Also, it comes with a scripted diff to normalize whitespace and type names. (Previous attempts: #14601 and #14459)

ACKs for top commit:
  laanwj:
    ACK fa5c6622c8

Tree-SHA512: 5a10956e12f8ce23e93a2ce8bafd6cae759d8a21658f79397e3bfce3e4aabd9658bdbd40acde49323dca958a9befee7166654994208c182dd60f483109621e17
2020-02-05 14:54:42 +01:00
Wladimir J. van der Laan
b30a1f3e39
Merge #18052: Remove false positive GCC warning
e9434ee03e Remove false positive GCC warning (Hennadii Stepanov)

Pull request description:

  On master (f05c1ac444) GCC compiler fires a false positive `-Wmaybe-uninitialized`:

  ```
  wallet/wallet.cpp: In static member function ‘static std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain&, const WalletLocation&, std::__cxx11::string&, std::vector<std::__cxx11::basic_string<char> >&, uint64_t)’:
  wallet/wallet.cpp:3913:27: warning: ‘*((void*)& time_first_key +8)’ may be used uninitialized in this function [-Wmaybe-uninitialized]
           Optional<int64_t> time_first_key;
                             ^~~~~~~~~~~~~~
  ```

  The same as #15292.

  This PR leverages a workaround and removes the warning.

ACKs for top commit:
  laanwj:
    ACK e9434ee03e, removes the warning for me (gcc 7.4.0)
  kristapsk:
    ACK e9434ee03e

Tree-SHA512: 8820a8ba6a75aa6b1ac675a38c883a77f12968b010533b6383180aa66e7e0d570bf6300744903ead91cf9084e5345144959cd6b0cea1b763190b8dd49bacce75
2020-02-05 14:43:28 +01:00
Samuel Dobson
6d0e532ae0
Merge #17585: rpc: deprecate getaddressinfo label
d3bc184081 doc: update release notes with getaddressinfo label deprecation (Jon Atack)
72af93f364 test: getaddressinfo label deprecation test (Jon Atack)
d48875fa20 rpc: deprecate getaddressinfo label field (Jon Atack)
dc0cabeda4 test: remove getaddressinfo label tests (Jon Atack)
c7654af6f8 doc: address pr17578 review feedback (Jon Atack)

Pull request description:

  This PR builds on #17578 (now merged) and deprecates the rpc getaddressinfo `label` field. The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=label`.

  See http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 and https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001 for more context.

  Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=label` flag while verifying the rpc getaddressinfo output and help text.

  Next step: add support for multiple labels.

ACKs for top commit:
  jnewbery:
    ACK d3bc184081
  laanwj:
    ACK d3bc184081
  meshcollider:
    utACK d3bc184081

Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d
2020-02-02 21:35:46 +13:00
Hennadii Stepanov
e9434ee03e
Remove false positive GCC warning 2020-02-01 23:07:19 +02:00
Andrew Chow
19a354b11f Output a descriptor in createmultisig and addmultisigaddress 2020-01-30 23:55:36 -05:00
Anthony Towns
1115ba693b psbt_wallet_tests: use unique_ptr for GetSigningProvider 2020-01-30 16:09:13 +10:00
Samuel Dobson
2d6e76af24
Merge #17261: Make ScriptPubKeyMan an actual interface and the wallet to have multiple
3f373659d7 Refactor: Replace SigningProvider pointers with unique_ptrs (Andrew Chow)
3afe53c403 Cleanup: Drop unused GUI learnRelatedScripts method (Andrew Chow)
e2f02aa59e Refactor: Copy CWallet signals and print function to LegacyScriptPubKeyMan (Andrew Chow)
c729afd0a3 Box the wallet: Add multiple keyman maps and loops (Andrew Chow)
4977c30d59 refactor: define a UINT256_ONE global constant (Andrew Chow)
415afcccd3 HD Split: Avoid redundant upgrades (Andrew Chow)
01b4511206 Make UpgradeKeyMetadata work only on LegacyScriptPubKeyMan (Andrew Chow)
4a7e43e846 Store p2sh scripts in AddAndGetDestinationForScript (Andrew Chow)
501acb5538 Always try to sign for all pubkeys in multisig (Andrew Chow)
81610eddbc List output types in an array in order to be iterated over (Andrew Chow)
eb81fc3ee5 Refactor: Allow LegacyScriptPubKeyMan to be null (Andrew Chow)
fadc08ad94 Locking: Lock cs_KeyStore instead of cs_wallet in legacy keyman (Andrew Chow)
f5be479694 wallet: Improve CWallet:MarkDestinationsDirty (João Barbosa)

Pull request description:

  Continuation of wallet boxes project.

  Actually makes ScriptPubKeyMan an interface which LegacyScriptPubkeyMan. Moves around functions and things from CWallet into LegacyScriptPubKeyMan so that they are actually separate things without circular dependencies.

  ***

  Introducing the `ScriptPubKeyMan` (short for ScriptPubKeyManager) for managing scriptPubKeys and their associated scripts and keys. This functionality is moved over from `CWallet`. Instead, `CWallet` will have a pointer to a `ScriptPubKeyMan` for every possible address type, internal and external. It will fetch the correct `ScriptPubKeyMan` as necessary. When fetching new addresses, it chooses the `ScriptPubKeyMan` based on address type and whether it is change. For signing, it takes the script and asks each `ScriptPubKeyMan` for whether that `ScriptPubKeyMan` considers that script `IsMine`, whether it has that script, or whether it is able to produce a signature for it. If so, the `ScriptPubKeyMan` will provide a `SigningProvider` to the caller which will use that in order to sign.

  There is currently one `ScriptPubKeyMan` - the `LegacyScriptPubKeyMan`. Each `CWallet` will have only one `LegacyScriptPubKeyMan` with the pointers for all of the address types and change pointing to this `LegacyScriptPubKeyMan`. It is created when the wallet is loaded and all keys and metadata are loaded into it instead of `CWallet`. The `LegacyScriptPubKeyMan` is primarily made up of all of the key and script management that used to be in `CWallet`. For convenience, `CWallet` has a `GetLegacyScriptPubKeyMan` which will return the `LegacyScriptPubKeyMan` or a `nullptr` if it does not have one (not yet implemented, but callers will check for the `nullptr`). For purposes of signing, `LegacyScriptPubKeyMan`'s `GetSigningProvider` will return itself rather than a separate `SigningProvider`. This will be different for future `ScriptPubKeyMan`s.

  The `LegacyScriptPubKeyMan` will also handle the importing and exporting of keys and scripts instead of `CWallet`. As such, a number of RPCs have been limited to work only if a `LegacyScriptPubKeyMan` can be retrieved from the wallet. These RPCs are `sethdseed`, `addmultisigaddress`, `importaddress`, `importprivkey`, `importpubkey`, `importmulti`, `dumpprivkey`, and `dumpwallet`. Other RPCs which relied on the wallet for scripts and keys have been modified in order to take the `SigningProvider` retrieved from the `ScriptPubKeyMan` for a given script.

  Overall, these changes should not effect how everything actually works and the user should experience no difference between having this change and not having it. As such, no functional tests were changed, and the only unit tests changed were those that were directly accessing `CWallet` functions that have been removed.

  This PR is the last step in the [Wallet Structure Changes](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes).

ACKs for top commit:
  instagibbs:
    re-utACK 3f373659d7
  Sjors:
    re-utACK 3f373659d7 (it still compiles on macOS after https://github.com/bitcoin/bitcoin/pull/17261#discussion_r370377070)
  meshcollider:
    Tested re-ACK 3f373659d7

Tree-SHA512: f8e2b8d9efa750b617691e8702d217ec4c33569ec2554a060141d9eb9b9a3a5323e4216938e2485c44625d7a6e0925d40dea1362b3af9857cf08860c2f344716
2020-01-30 17:21:21 +13:00
Samuel Dobson
aabec94541
Merge #17719: Document better -keypool as a look-ahead safety mechanism
f41d589669 Document better -keypool as a look-ahead safety mechanism (Antoine Riard)

Pull request description:

  If after a backup, an address is issued beyond the initial
  keypool range and none of the addresses in this range
  is seen onchain, if a wallet is restored from backup, even in
  case of rescan, funds may be loss due to the look-ahead
  buffer not being incremented and so restored wallet not detecting
  onchain out-of-range address as derived from its seed.

  This scenario is theoretically unavoidable due to the requirement
  of the keypool to have a max size. However, given the default
  keypool size, this is unlikely. Document better keypool size
  implications to avoid user setting a too low value.

  While reviewing #17681, it took me a while to figure out the safety implications of keypool, I find it would be better to document this a bit farther to avoid users shooting themselves in the foot. For further context & discussion, see https://github.com/bitcoin/bitcoin/pull/17681#issuecomment-563613452

ACKs for top commit:
  ryanofsky:
    Code review ACK f41d589669. Just "Warning:" prefix added since the last review
  jonatack:
    ACK f41d589669 code review and build/test. The added `Warning:` since last review is a good addition.

Tree-SHA512: d3d0ee88fcdfc5c8841a2bd4bada0e4eeb412a0dce5054e5fb023643c2fa57206a0f3efb06890c245528dc4431413ed2fd5645b9319d26245d044c490b7f0db0
2020-01-30 09:10:15 +13:00
fanquake
1326092e6c
Merge #17156: psbt: check that various indexes and amounts are within bounds
deaa6dd144 psbt: check output index is within bounds before accessing (Andrew Chow)
f1ef7f0aa4 Don't calculate tx fees for PSBTs with invalid money values (Andrew Chow)

Pull request description:

  Fixes #17149

  Two classes of issues were found by the psbt fuzzer: values out of range and causing overflows, and prevout indexes being out of range. This PR fixes both.

  When accessing a specific output using the index given in the tx, check that it is actually a possible output before trying to access the output.

  When summing and checking amounts for `decodepsbt` and `analyzepsbt`, make sure that the values are actually valid money values.. Otherwise, stop summing and don't show the fee. For `analyzepsbt`, return that the next role is the Creator since the Creator needs to remake the transaction to be valid.

ACKs for top commit:
  practicalswift:
    ACK deaa6dd144 -- only change since last ACK was the addition of tests
  gwillen:
    tested ACK deaa6dd, would also like to see this merged!

Tree-SHA512: 06c36720bbb5a7ab1c29f7d15878bf9f0d3e5760c06bff479d412e1bf07bb3e0e9ab6cca820a4bfedaab71bfd7af813807e87cbcdf0af25cc3f66a53a06dbcfd
2020-01-29 19:39:50 +08:00
Andrew Chow
3f373659d7 Refactor: Replace SigningProvider pointers with unique_ptrs
Needed for future ScriptPubKeyMans which may need to create
SigningProviders dynamically and thus a normal pointer is not enough

This commit does not change behavior.
2020-01-23 16:35:08 -05:00
Andrew Chow
e2f02aa59e Refactor: Copy CWallet signals and print function to LegacyScriptPubKeyMan
This commit does not change behavior.
2020-01-23 16:35:08 -05:00
Andrew Chow
c729afd0a3 Box the wallet: Add multiple keyman maps and loops
Add wallet logic for dealing with multiple ScriptPubKeyMan instances. This
doesn't change current behavior because there is still only a single
LegacyScriptPubKeyMan. But in the future the new logic will be used to support
descriptor wallets.
2020-01-23 16:35:08 -05:00
Andrew Chow
4977c30d59 refactor: define a UINT256_ONE global constant
Instead of having a uint256 representations of one scattered throughout
where it is used, define it globally in uint256.h
2020-01-23 16:35:08 -05:00
Andrew Chow
415afcccd3 HD Split: Avoid redundant upgrades
This avoids repeaded upgrades when support for more multiple keyman references
is added in the next commit:
https://github.com/bitcoin/bitcoin/pull/16341#discussion_r322370108
2020-01-23 16:35:08 -05:00
Andrew Chow
01b4511206 Make UpgradeKeyMetadata work only on LegacyScriptPubKeyMan 2020-01-23 16:35:08 -05:00
Andrew Chow
eb81fc3ee5 Refactor: Allow LegacyScriptPubKeyMan to be null
In CWallet::LoadWallet, use this to detect and empty wallet with no keys

This commit does not change behavior.
2020-01-23 16:34:28 -05:00
Andrew Chow
fadc08ad94 Locking: Lock cs_KeyStore instead of cs_wallet in legacy keyman
This commit only affects locking behavior and doesn't have other changes.
2020-01-23 16:34:28 -05:00
João Barbosa
f5be479694 wallet: Improve CWallet:MarkDestinationsDirty 2020-01-23 16:34:28 -05:00
MarcoFalke
fa5c6622c8
doc: Use proper RPC help syntax in importmulti 2020-01-23 10:23:30 -05:00
MarcoFalke
fab63111be
doc: Remove duplicate "comment" from listsinceblock RPC help
Also, properly document all (json object) and (json array)
2020-01-23 10:21:00 -05:00
MarcoFalke
faff5a60ed
doc: Fix syntax error (trailing square bracket) in walletprocesspsbt 2020-01-23 10:19:51 -05:00
Wladimir J. van der Laan
5d2ff75e20
Merge #17945: doc: Fix doxygen errors
297e098557 Fix doxygen errors (Ben Woosley)

Pull request description:

  These are all the remaining errors identified via -Werror=documentation, e.g.:
  ```
    ./rpc/rawtransaction_util.h:31:13: error: parameter 'prevTxs' not found in the function declaration [-Werror,-Wdocumentation]
      * @param  prevTxs       Array of previous txns outputs that tx depends on but may not yet be in the block chain
                ^~~~~~~
    ./rpc/rawtransaction_util.h:31:13: note: did you mean 'prevTxsUnival'?
      * @param  prevTxs       Array of previous txns outputs that tx depends on but may not yet be in the block chain
                ^~~~~~~
                prevTxsUnival

    netbase.cpp:766:11: error: parameter 'outProxyConnectionFailed[out]' not found in the function declaration [-Werror,-Wdocumentation]
     * @param outProxyConnectionFailed[out] Whether or not the connection to the
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    netbase.cpp:766:11: note: did you mean 'outProxyConnectionFailed'?
     * @param outProxyConnectionFailed[out] Whether or not the connection to the
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
              outProxyConnectionFailed
  ```

  You can use this to run with `-Wdocumentation` yourself: #14920

ACKs for top commit:
  laanwj:
    ACK 297e098557

Tree-SHA512: a232d893b170873d923e77fa56c56a6567e7fd120b5af1f52cfeeae1093eec55621604cc80a523678f6fedc8bbb31228c4aa8dc2a630ce9ffc91525988522af7
2020-01-20 20:35:17 +01:00
Ben Woosley
297e098557
Fix doxygen errors
Identified via -Wdocumentation, e.g.:

  ./rpc/rawtransaction_util.h:31:13: error: parameter 'prevTxs' not found in the function declaration [-Werror,-Wdocumentation]
    * @param  prevTxs       Array of previous txns outputs that tx depends on but may not yet be in the block chain
              ^~~~~~~
  ./rpc/rawtransaction_util.h:31:13: note: did you mean 'prevTxsUnival'?
    * @param  prevTxs       Array of previous txns outputs that tx depends on but may not yet be in the block chain
              ^~~~~~~
              prevTxsUnival

  netbase.cpp:766:11: error: parameter 'outProxyConnectionFailed[out]' not found in the function declaration [-Werror,-Wdocumentation]
   * @param outProxyConnectionFailed[out] Whether or not the connection to the
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  netbase.cpp:766:11: note: did you mean 'outProxyConnectionFailed'?
   * @param outProxyConnectionFailed[out] Whether or not the connection to the
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            outProxyConnectionFailed
2020-01-16 18:25:11 -08:00
Samuel Dobson
7fb94c0ed4
Merge #17889: wallet: Improve CWallet:MarkDestinationsDirty
2b1641492f wallet: Improve CWallet:MarkDestinationsDirty (João Barbosa)

Pull request description:

  Improve `CWallet:MarkDestinationsDirty` by skipping transactions that already have the cache invalidated. Skipping a transaction avoids at worst case extracting all output destinations.

ACKs for top commit:
  meshcollider:
    re-utACK 2b1641492f

Tree-SHA512: 479dc2dde4b653b856e3d6a0c59a34fe33e963eb131a2d88552a8b30471b8725a087888fe5d7db6e4ee19b74072fe64441497f033be7d1931637f756e0d8fef5
2020-01-17 14:15:33 +13:00
João Barbosa
2b1641492f wallet: Improve CWallet:MarkDestinationsDirty 2020-01-17 01:12:17 +00:00
Wladimir J. van der Laan
f018d0c9cd
Merge #17924: Bug: IsUsedDestination shouldn't use key id as script id for ScriptHash
6dd59d2e49 Don't allow implementers to think ScriptHash(Witness*()) results in nesting computation (Gregory Sanders)
4b8f1e989f IsUsedDestination shouldn't use key id as script id for ScriptHash (Gregory Sanders)

Pull request description:

  Regression introduced in https://github.com/bitcoin/bitcoin/pull/17621 which causes p2sh-segwit addresses to be erroneously missed.

  Tests are only failing in 0.19 branch, likely because that release still uses p2sh-segwit addresses rather than bech32 by default.

  I'll devise a test case to catch this going forward.

ACKs for top commit:
  achow101:
    ACK 6dd59d2e49
  MarcoFalke:
    ACK 6dd59d2
  meshcollider:
    Code review ACK 6dd59d2e49

Tree-SHA512: b3e0f320c97b8c1f814cc386840240cbde2761fee9711617b713d3f75a4a5dce2dff2df573d80873df42a1f4b74e816ab8552a573fa1d62c344997fbb6af9950
2020-01-16 19:23:33 +01:00
João Barbosa
9a5b5ee81f wallet: Replace %w by wallet name in -walletnotify script
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
2020-01-15 11:47:26 +00:00
Samuel Dobson
ac61ec9da6
Merge #17843: wallet: Reset reused transactions cache
6fc554f591 wallet: Reset reused transactions cache (Fabian Jahr)

Pull request description:

  Fixes #17603 (together with #17824)

  `getbalances` is using the cache within `GetAvailableCredit` under certain conditions [here](35fff5be60/src/wallet/wallet.cpp (L1826)). For a wallet with `avoid_reuse` activated this can lead to inconsistent reporting of `used` transactions/balances between `getbalances` and `listunspent` as pointed out in #17603. When an address is reused before the first transaction is spending from this address, the cache is not updated even after the transaction is sent. This means the remaining outputs at the reused address are not showing up as `used` in `getbalances`.

  With this change, any newly incoming transaction belonging to the wallet marks all the other outputs at the same address as dirty.

ACKs for top commit:
  kallewoof:
    Code review re-ACK 6fc554f591
  promag:
    ACK 6fc554f591.
  achow101:
    Re-ACK 6fc554f591
  meshcollider:
    Code review ACK 6fc554f591

Tree-SHA512: c4cad2c752176d16d77b4a4202291d20baddf9f27250896a40274d74a6945e0f6b34be04c2f9b1b2e756d3ac669b794969df8f82a98e0b16f10e92f276649ea2
2020-01-15 22:11:33 +13:00
MarcoFalke
e09c701e01 scripted-diff: Bump copyright of files changed in 2020
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
2020-01-15 02:18:00 +07:00
MarcoFalke
6cbe620964 scripted-diff: Replace CCriticalSection with RecursiveMutex
-BEGIN VERIFY SCRIPT-
 # Delete outdated alias for RecursiveMutex
 sed -i -e '/CCriticalSection/d'                 ./src/sync.h
 # Replace use of outdated alias with RecursiveMutex
 sed -i -e 's/CCriticalSection/RecursiveMutex/g' $(git grep -l CCriticalSection)
-END VERIFY SCRIPT-
2020-01-15 01:43:46 +07:00
Gregory Sanders
4b8f1e989f IsUsedDestination shouldn't use key id as script id for ScriptHash 2020-01-14 13:23:24 -05:00
Fabian Jahr
6fc554f591
wallet: Reset reused transactions cache
If a destination is reused we mark the cache of the other transactions going to that destination dirty so they are not accidentally reported as trusted when the cache is hit.
2020-01-13 13:40:06 +01:00
Jon Atack
d48875fa20
rpc: deprecate getaddressinfo label field 2020-01-09 18:08:18 +01:00
Jon Atack
c7654af6f8
doc: address pr17578 review feedback
- https://github.com/bitcoin/bitcoin/pull/17578#discussion_r363975411
- https://github.com/bitcoin/bitcoin/pull/17578#discussion_r363969721
- https://github.com/bitcoin/bitcoin/pull/17578#discussion_r362703553
2020-01-09 17:29:49 +01:00
Wladimir J. van der Laan
6196e93001
Merge #16963: wallet: Fix unique_ptr usage in boost::signals2
6d6a7a8403 gui: Fix duplicate wallet showing up (João Barbosa)
81ea66c30e Drop signal CClientUIInterface::LoadWallet (Russell Yanofsky)

Pull request description:

  This PR includes 2 fixes:
   - prevent GUI LoadWallet handlers from crashing on startup when multiple handlers are attached, because the first handler takes ownership of the wallet unique pointer. Now every handler will receive its own unique pointer;

   - prevent showing a wallet twice in the GUI on startup due to a race with `loadwallet`.

  Fixes #16937

ACKs for top commit:
  fjahr:
    code review ACK 6d6a7a8403
  ryanofsky:
    Code review ACK 6d6a7a8403. No changes since last ACK other than rebase due to #17070
  kallewoof:
    Code review ACK 6d6a7a8403

Tree-SHA512: 7f0658c9011f81dfa176a094c2263448ee1d14fda7dc94e8b55ee9c8b81538bd2d1e4bf8a8dbfcd029ebfc9feb6d3cda9dee3f911122df0a4b1e0ca75f653ba4
2020-01-08 15:58:33 +01:00
Samuel Dobson
cab3859a35
Merge #17677: Activate watchonly wallet behavior for LegacySPKM only
e1e1442f3e Activate no-privkey -> ISMINE_WATCH_ONLY behavior for LegacySPKM only (Gregory Sanders)

Pull request description:

  Slight cleanup following https://github.com/bitcoin/bitcoin/pull/16944

  This should allow future scriptpubkeymans to transparently work, since the current plan is to have ismine always be spendable.

ACKs for top commit:
  achow101:
    ACK e1e1442f3e
  Sjors:
    Code review ACK e1e1442f3e
  meshcollider:
    Code review ACK e1e1442f3e

Tree-SHA512: c0a86587d33b8b1646494a5cb0bf8681ee4a88e6913918157746943a0996b501903e0e6ee954cf04154c1e0faee0cbb375c74ca789f46ba9244eb5296632b042
2020-01-08 11:30:10 +13:00
Samuel Dobson
7ea3b85ecf
Merge #17578: rpc: simplify getaddressinfo labels, deprecate previous behavior
8925df86c4 doc: update release notes (Jon Atack)
8bb405bbad test: getaddressinfo labels purpose deprecation test (Jon Atack)
60aba1f2f1 rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack)
7851f14ccf rpc: incorporate review feedback from PR 17283 (Jon Atack)

Pull request description:

  This PR builds on #17283 (now merged) and is followed by #17585.

  It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs.

  before
  ```
    "labels": [
      {
        "name": "DOUBLE SPEND",
        "purpose": "receive"
      }
  ```
  after
  ```
    "labels": [
      "DOUBLE SPEND"
    ]
  ```

  The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`.

  For context, see:
  - https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001
  - http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427)
  - http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622

  Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output.

  Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those.

ACKs for top commit:
  jnewbery:
    reACK 8925df8
  promag:
    Code review ACK 8925df86c4.
  meshcollider:
    Code review ACK 8925df86c4

Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438
2020-01-08 11:25:14 +13:00
Samuel Dobson
45f151913e
Merge #16373: bumpfee: Return PSBT when wallet has privkeys disabled
091a876664 Test watchonly wallet bumpfee with PSBT return (Gregory Sanders)
e9b4f9419c bumpfee: Return PSBT when wallet has privkeys disabled (Gregory Sanders)
75a5e478b6 Change bumpfee to use watch-only funds for legacy watchonly wallets (Gregory Sanders)

Pull request description:

  The main use-case here is for using with watch-only wallets with PSBT-signing cold wallets of all kinds.

ACKs for top commit:
  achow101:
    ACK 091a876664
  Sjors:
    Tested ACK 091a876664
  meshcollider:
    utACK 091a876664

Tree-SHA512: f7cf663e1af0b029e5c99eac88c5fdc3bc9e9a3841da8a608e8a9957e9bcf6a78864b8c2706fcaf78a480ffe11badd80c4fad29f97c0bb929e0470fafda5c22e
2020-01-08 10:41:19 +13:00
Samuel Dobson
bcb4cdcca3
Merge #17621: IsUsedDestination should count any known single-key address
09502452bb IsUsedDestination should count any known single-key address (Gregory Sanders)

Pull request description:

  This plugs the privacy leak detailed at https://github.com/bitcoin/bitcoin/issues/17605, at least for the single-key case.

ACKs for top commit:
  meshcollider:
    Code Review ACK 09502452bb

Tree-SHA512: e1d68281675f05072b3087171cba1df9416a69c9ccf70c72e8555e55eadda2d0fd339e5a894e3a3438ff94b9e3827fb19b8b701faade70c08756b19ff157ee0c
2020-01-08 10:31:51 +13:00