2575 Commits

Author SHA1 Message Date
MarcoFalke
fa55c1d5fd
build: Add Werror=range-loop-analysis 2020-08-14 15:27:38 +02:00
MarcoFalke
30dd562fd2
Merge #19457: wallet: Cleanup wallettool salvage and walletdb extraneous declarations
0e279fe4899beae8630264ef1fe420dd71f29d5d walletdb: Remove unused static functions from walletdb.h (Andrew Chow)
9f536d4fe949666f14a0bf5b814522cecde71f56 wallettool: Have RecoverDatabaseFile return errors and warnings (Andrew Chow)
06e263a4e368671ebb4e4a77c1447ebd5104a488 Call RecoverDatabaseFile directly from wallettool (Andrew Chow)

Pull request description:

  Followup to #19324 addressing some comments.

  Removes the `SalvageWallet` function in wallettool and instead directly calls `RecoverDatabaseFile` as suggested in https://github.com/bitcoin/bitcoin/pull/19324#discussion_r450379596

  Removes the `LogPrintf`s and `tfm::format`s in `RecoverDatabaseFile` as noted in https://github.com/bitcoin/bitcoin/pull/19324#discussion_r448027237

  Removes the declarations of `VerifyEnvironment` and `VerifyDatabaseFile` that were forgotten in `walletdb.h` as noted in https://github.com/bitcoin/bitcoin/pull/19324#issuecomment-654389079

ACKs for top commit:
  meshcollider:
    Code review ACK 0e279fe4899beae8630264ef1fe420dd71f29d5d
  ryanofsky:
    Code review ACK 0e279fe4899beae8630264ef1fe420dd71f29d5d, just dropped last commit

Tree-SHA512: ffd01f30536c2eab4bf40ba363c3ea916ecef3c8f0c5262040b40498776ffb00f95240204a40e38415d6931800851d0a3fa63ee91efc1d329b60ac317da0363d
2020-08-14 15:12:44 +02:00
fanquake
c0b1706964
Merge #19568: Wallet should not override signing errors
e7448d66803f42984018ef8dfa6699027cb9536d wallet: Don't override signing errors (Fabian Jahr)

Pull request description:

  While reviewing #17204 I noticed that the errors in `input_errors` from `::SignTransaction` where being overridden by `CWallet::SignTransaction`. For example, a Script related error led to incomplete signature data which led to `CWallet::SignTransaction` reporting that keys were missing, which was a less precise error than the original one.

  Additionally, the error `"Input not found or already spent"` is [duplicated in `sign.cpp`](c7b4968552/src/script/sign.cpp (L481)), so the error here is redundant at the moment. So technically the whole error block could be removed, I think. However, this code is affected by the ongoing work on the wallet so there might be a reason why these errors are here. But even if there is a reason to keep them, I don't think existing, potentially more precise errors should be overridden here unless we want to hide them from the users. I am looking for feedback if this is a work in progress state where these errors could be more useful in the future or if they can be removed.

  On testing: even though [the errors in `CWallet` are covered](https://marcofalke.github.io/btc_cov/total.coverage/src/wallet/wallet.cpp.gcov.html), all tests still pass after removing them. I am not sure if there is a desire to cover these specific error messages, tests in `test/functional/rpc_signrawtransaction.py` seem to aim for a more generic approach.

ACKs for top commit:
  achow101:
    ACK e7448d66803f42984018ef8dfa6699027cb9536d
  meshcollider:
    Code review ACK e7448d66803f42984018ef8dfa6699027cb9536d

Tree-SHA512: 3e2bc11d05379d2aef87b093a383d1b044787efc70e35955b2f8ecd028b6acef02f386180566af6a1a63193635f5d685466e2f6141c96326c49ffc5c81ca3e23
2020-08-14 16:04:31 +08:00
Samuel Dobson
609ce2d0da
Merge #19644: rpc: document returned error fields as optional if applicable
f110b7c722eb150816a26cab161ac2b8c0f58609 rpc: document returned error fields as optional if applicable (Sebastian Falbesoner)

Pull request description:

  The following RPCs return error fields (named `"error"` or `"errors"`) that are optional, but don't show up as optional in the help text yet:
  * `analyzepsbt`
  * `estimatesmartfee`
  * `signrawtransactionwithkey`
  * `signrawtransactionwithwallet`

  The following RPC has the errors field already marked as optional, but doesn't match the usual format in the description (like `"if there are any"` in parantheses):
  * `estimaterawfee`

  This PR adds the missing optional flags and adapts the description strings. Inspired by a recent PR #19634 by justinmoon.

  The instances were found via `git grep "RPCResult.*\"error"`. Note that there is one RPC so far where the return error is not optional (i.e. in case of no error, the field is included in the result, but is just empty), namely `bumpfee`.

ACKs for top commit:
  adaminsky:
    ACK `f110b7c`
  laanwj:
    ACK f110b7c722eb150816a26cab161ac2b8c0f58609, new documentation looks consistent with actual behavior
  achow101:
    ACK f110b7c722eb150816a26cab161ac2b8c0f58609
  meshcollider:
    utACK f110b7c722eb150816a26cab161ac2b8c0f58609

Tree-SHA512: 30c00f78a575b60e32b4536496af986d53a25f33e6ebbf553adcdcf825ad21a44f90267f3d1ea53326dac83bcfa9983fdb3dad6d3126e20f97f3c08ce286e188
2020-08-14 09:57:58 +12:00
Wladimir J. van der Laan
6757b3ac8f
Merge #19655: rpc: Catch listsinceblock target_confirmations exceeding block count
c133cdcdc3397a734d57e05494682bf9bf6f4c15 Cap listsinceblock target_confirmations param (Adam Stein)

Pull request description:

  This addresses an issue brought up in #19587.

  Currently, the `target_confirmations` parameter to `listsinceblock` is not checked for being too large. When `target_confirmations` is greater than one more than the current number of blocks, `listsinceblock` fails with error code -1. In comparison, when `target_confirmations` is less than 1,  a -8 "Invalid parameter" error code is thrown.

  This PR fixes the issue by returning a -8 "Invalid parameter" error if the `target_confirmations` value corresponds to a block with more confirmations than the genesis block. This happens if `target_confirmations` exceeds one more than the number of blocks.

ACKs for top commit:
  laanwj:
    Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15
  ryanofsky:
    Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15. Just suggested changes since last review. Thanks!

Tree-SHA512: 02680f4cb937d2c24d5019abd0ebfa188b8a50679a1e64e9c26bfe5c17eef6aea906832e6e2d492ba8a2ea160041bf185d66795ee691e340f6793db03c21b89a
2020-08-13 12:12:33 +02:00
Samuel Dobson
8a85377cd0
Merge #18654: rpc: separate bumpfee's psbt creation function into psbtbumpfee
79d6332e9e4fc01e6418247c31e31b4faa1b3b84 moveonly: Fix indentation in bumpfee RPC (Andrew Chow)
431071c28ae35be8aa012df51233be19067d625c Hide bumpfee's psbt creation behavior behind -deprecatedrpc (Andrew Chow)
4638224f64ba7c8ea7c4fb550ec89c6a6d8c7887 Add psbtbumpfee RPC (Andrew Chow)

Pull request description:

  Adds a new RPC `psbtbumpfee` which always creates a psbt. `bumpfee` will then only be able to create and broadcast fee bumping transactions instead of changing its behavior based on `IsWalletSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)`.

  Split from #18627

ACKs for top commit:
  Sjors:
    re-utACK 79d6332
  meshcollider:
    utACK 79d6332e9e4fc01e6418247c31e31b4faa1b3b84
  fjahr:
    Code review ACK 79d6332e9e4fc01e6418247c31e31b4faa1b3b84

Tree-SHA512: 1c92c4b4461bb30e78be3ee73165f624398ef33996ce36043b61a8931be667030d0fca12fd0b30097b78c56e4e9092c69582b237cbdac51d56f6be23d8c0f1bb
2020-08-13 12:21:11 +12:00
Adam Stein
c133cdcdc3
Cap listsinceblock target_confirmations param
Previously, listsinceblock would fail with error code -1 when the
target_confirmations exceeded the number of confirmations of the genesis
block. This commit allows target_confirmations to refer to a lastblock
hash with more confirmations than exist in the chain by setting the
lastblock hash to the genesis hash in this case. This allows for
`listsinceblock "" 6` to not fail if the block count is less than 5
which may happen on regtest.

Includes update to the functional test for listsinceblock to test for
this case.
2020-08-12 15:16:22 -07:00
Wladimir J. van der Laan
b75f2ad72d
Merge #19660: refactor: Make HexStr take a span
0a8aa626dd69a357e1b798b07b64cf4177a464a3 refactor: Make HexStr take a span (Wladimir J. van der Laan)

Pull request description:

  Make `HexSt`r take a span of bytes, instead of an awkward pair of templated iterators. This simplifies most of the uses.

ACKs for top commit:
  elichai:
    Code review ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3
  hebasto:
    re-ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3
  jonatack:
    re-ACK 0a8aa626dd69a357e1b798b07b64cf4177a464a3

Tree-SHA512: 6e178ece5cbac62119c857a10299b1e85422938084c3f03063e17119a5129e0c28016e05a6fabaa4c271a7e0a37c7cd89fa47c435ee19b38a5acfe80d00de992
2020-08-09 15:35:58 +02:00
Wladimir J. van der Laan
0a8aa626dd refactor: Make HexStr take a span
Make HexStr take a span of bytes, instead of an awkward pair of
templated iterators.
2020-08-06 19:41:43 +02:00
fanquake
bb2a9f9c8c
Merge #19634: rpc: Document getwalletinfo's unlocked_until field as optional
f916847d2b56f2935c169e1b95b350a477c804cc rpc: Document getwalletinfo's unlocked_until field as optional (Justin Moon)

Pull request description:

  The `getwalletinfo` RPC command's `unlocked_until` field is [optional in the code](f916847d2b/src/wallet/rpcwallet.cpp (L2397)), but wasn't marked as optional in the docs.

ACKs for top commit:
  theStack:
    ACK f916847d2b
  achow101:
    ACK f916847d2b56f2935c169e1b95b350a477c804cc
  kristapsk:
    ACK f916847d2b56f2935c169e1b95b350a477c804cc

Tree-SHA512: 8d82f0992fdaf8160000acf4a6e7e7f9ff289a90a983be2e078cf754f4b03601637e5f405afa66bd55adef9b347fa5eac5cc1822033b2ac08c587609cf3dfe0f
2020-08-04 09:14:45 +08:00
Sebastian Falbesoner
f110b7c722 rpc: document returned error fields as optional if applicable
Affects the following RPCs:
- analyzepsbt
- estimatesmartfee
- signrawtransactionwithkey
- signrawtransactionwithwallet

For the RPC estimaterawfee, the description message was adapted
to match the other optional ones.
2020-08-02 18:44:41 +02:00
Justin Moon
f916847d2b rpc: Document getwalletinfo's unlocked_until field as optional 2020-07-31 12:27:48 -05:00
Pieter Wuille
77c507358b Make Hash[160] consume range-like objects 2020-07-30 13:57:54 -07:00
MarcoFalke
62d137ac3b
Merge #19561: refactor: Pass ArgsManager into functions that register args
8ed9002cd14165f751442f738fbf1fb8a37611b2 refactor: use local argsmanager in CRegTestParams (Ivan Metlushko)
9b20f6682845870d6ac53a01d3166fb83c467e7d scripted-diff: Replace gArgs with local argsman (Ivan Metlushko)
a316e9ce265212a7c6c4ef7922420f6ecba9e7b0 refactor: add unused ArgsManager to replace gArgs (Ivan Metlushko)

Pull request description:

  Rationale: reduce use of gArgs to decouple code and simplify future maintenance and easier unit testing.

  This PR is continuation of work started in  #18926 and #18662
  It covers only places that register args in ArgsManager with `AddArgs()` or `AddHiddenArgs()`.

  Closes #19511

ACKs for top commit:
  MarcoFalke:
    ACK 8ed9002cd14165f751442f738fbf1fb8a37611b2 👛

Tree-SHA512: 7e6ba8e8357a48833c71e9c3942a769acb3d93bdcc6748a8ef2b7c4461a2499419b60896abf1d8b6bf8e88ee2590284cdd5da64220243ac22375300bcb8fe3e8
2020-07-30 17:08:46 +02:00
Andrew Chow
0fcff547d5 walletdb: Ensure that having no database handle is a failure
Previously having no database handle could still be considered a success
when BerkeleyDatabase and BerkeleyBatch were used for dummy database
things. With dedicated DummyDatabase and DummyBatch classes now, these
should fail.
2020-07-29 12:30:24 -04:00
Andrew Chow
da039d2a91 Remove BDB dummy databases 2020-07-29 12:30:23 -04:00
Andrew Chow
0103d6434e Introduce DummyDatabase and use it in the tests 2020-07-29 12:28:30 -04:00
Ivan Metlushko
9b20f66828 scripted-diff: Replace gArgs with local argsman
-BEGIN VERIFY SCRIPT-
sed -i -e 's/gArgs.Add/argsman.Add/g' `git grep -l "gArgs.Add"`
-END VERIFY SCRIPT-
2020-07-29 16:39:00 +07:00
Ivan Metlushko
a316e9ce26 refactor: add unused ArgsManager to replace gArgs 2020-07-29 16:36:44 +07:00
Andrew Chow
0e279fe489 walletdb: Remove unused static functions from walletdb.h
VerifyEnvironment and VerifyDatabaseFile were removed, but their
declarations weren't. Remove those.
2020-07-26 20:22:49 -04:00
Andrew Chow
9f536d4fe9 wallettool: Have RecoverDatabaseFile return errors and warnings
Instead of logging or printing these errors and warnings, return them to
the caller.
2020-07-26 20:22:45 -04:00
Fabian Jahr
e7448d6680
wallet: Don't override signing errors 2020-07-25 00:00:36 +02:00
Andrew Chow
74507ce71e walletdb: Remove BerkeleyBatch friend class from BerkeleyDatabase 2020-07-22 23:30:19 -04:00
Andrew Chow
00f0041351 No need to check for duplicate fileids in all dbenvs
Since we have .walletlock in each directory, we don't need the duplicate
fileid checks across all dbenvs as it shouldn't be possible anyways.
2020-07-22 23:30:19 -04:00
Andrew Chow
d86efab370 walletdb: Move Db->open to BerkeleyDatabase::Open
Instead of opening the Db handle in BerkeleyBatch, make BerkeleyDatabase
do that.
2020-07-22 23:30:19 -04:00
Andrew Chow
4fe4b3bf1b walletdb: track database file use as m_refcount within BerkeleyDatabase
Instead of having BerkeleyEnvironment track the file use count, make
BerkeleyDatabase do it itself.
2020-07-22 23:30:19 -04:00
Andrew Chow
65fb8807ac Combine BerkeleyEnvironment::Verify into BerkeleyDatabase::Verify 2020-07-22 23:30:19 -04:00
Samuel Dobson
9d4b3d86b6
Merge #19334: wallet: Introduce WalletDatabase abstract class
d416ae560e46a4846a3fd5990b7d390d2ef30ec8 walletdb: Introduce WalletDatabase abstract class (Andrew Chow)
2179dbcbcd0b9bef7ad9c907b85294b9a1bccf0f walletdb: Add BerkeleyDatabase::Open dummy function (Andrew Chow)
71d28e7cdca1c8553531bb3a4725d7916363ec5c walletdb: Introduce AddRef and RemoveRef functions (Andrew Chow)
27b27663849932971eb5deadb1f19234b9cd97ea walletdb: Move BerkeleyDatabase::Flush(true) to Close() (Andrew Chow)

Pull request description:

  A `WalletDatabase` abstract class is created from `BerkeleyDatabase` and is implemented by `BerkeleyDatabase`. First, to get to the point that this is possible, 4 functions need to be added to `BerkeleyDatabase`: `AddRef`, `RemoveRef`, `Open`, and `Close`.

  First the increment and decrement of `mapFileUseCount` is refactored into separate functions `AddRef` and `RemoveRef`.

  `Open` is introduced as a dummy function. This will raise an exception so that it always fails.

  `Close` is refactored from `Flush`. The `shutdown` argument in `Flush` is removed and instead `Flush(true)` is now the `Close` function.

  Split from #18971

  Requires #19325

ACKs for top commit:
  ryanofsky:
    Code review ACK d416ae560e46a4846a3fd5990b7d390d2ef30ec8. Only changes since last review were rebasing after base PR #19334 merge, and adding cs_db lock in BerkeleyDatabase destructor, which should avoid races accessing env->m_databases and env->m_fileids
  fjahr:
    Code review ACK d416ae560e46a4846a3fd5990b7d390d2ef30ec8
  meshcollider:
    Code review & test run ACK d416ae560e46a4846a3fd5990b7d390d2ef30ec8

Tree-SHA512: 98d05ec093d7446c4488e2b0914584222a331e9a2f4d5be6af98e3f6d78fdd8e75526c12f91a8a52d4820c25bce02aa02aabe92d38bee7eb2fce07d0691b7b0d
2020-07-23 15:22:25 +12:00
Wladimir J. van der Laan
93decbc7a4
Merge #19370: Static asserts for consistency of fee defaults
1554b54d47d7e24ce2491f57d24e56d38ceb7649 Static asserts for consistency of fee defaults. (Daniel Kraft)

Pull request description:

  This adds `static_assert`'s that ensure that the default values given for fee levels in the wallet (minimum fee and incremental feerate increase) are at least as high as the corresponding levels configured in the core node policy.  Since the core policy values are enforced by the network, it makes sense for the wallet to be conservative and above (or at least not below) this.

ACKs for top commit:
  laanwj:
    code review ACK 1554b54d47d7e24ce2491f57d24e56d38ceb7649, these assumptions seem straightforward

Tree-SHA512: 50e5adf082f467062334377f82a3ee75bcfd436afc65bd0eb33c8d0549d6d90fd1f48c31f60cabe523eb59be9efa8ae0879e9e09cd51ca9c1bd466631ce03cf4
2020-07-22 19:25:07 +02:00
Andrew Chow
06e263a4e3 Call RecoverDatabaseFile directly from wallettool
When using the salvage command, call RecoverDatabaseFile directly
instead of SalvageWallet. Also removes SalvageWallet as it is no longer
needed.

SalvageWallet was doing an additional verify on the database which would
caause the salvage to sometimes fail. This is not needed.
2020-07-22 11:55:15 -04:00
MarcoFalke
c7007babb7
Merge #18907: walletdb: Don't remove database transaction logs and instead error
d0ea9bab2804928c9f40def61fd99064d2d8f9b8 walletdb: Don't remove database transaction logs and instead error (Andrew Chow)

Pull request description:

  Instead of removing the database transaction logs and retrying the
  wallet loading, just return an error message to the user. Additionally,
  speciically for DB_RUNRECOVERY, notify the user that this could be due
  to different BDB versions.

  Kind of implements the suggestion from https://github.com/bitcoin/bitcoin/pull/18870#discussion_r421647964

ACKs for top commit:
  Sjors:
    re-utACK d0ea9bab2804928c9f40def61fd99064d2d8f9b8
  ryanofsky:
    Code review ACK d0ea9bab2804928c9f40def61fd99064d2d8f9b8. Only changes since last review are rebase and expanding error and commit messages.

Tree-SHA512: f6e67dc70f58188742a5c8af7cdc63a2b58779aa0d26ae7f1e75805a239f1a342433860e5a238d6577fae5ab04b9d15e7f11c55b867065dfd13781a6a62e4958
2020-07-22 08:58:55 +02:00
Andrew Chow
d416ae560e walletdb: Introduce WalletDatabase abstract class
Make WalletDatabase actually an abstract class and not just a typedef
for BerkeleyDatabase. Have BerkeleyDatabase inherit this class.
2020-07-14 11:07:16 -04:00
Andrew Chow
2179dbcbcd walletdb: Add BerkeleyDatabase::Open dummy function
Adds an Open function for the class abstraction that does nothing for
now.
2020-07-14 11:07:16 -04:00
Andrew Chow
71d28e7cdc walletdb: Introduce AddRef and RemoveRef functions
Refactor mapFileUseCount increment and decrement to separate functions
AddRef and RemoveRef
2020-07-14 11:07:16 -04:00
Andrew Chow
27b2766384 walletdb: Move BerkeleyDatabase::Flush(true) to Close()
Instead of having Flush optionally shutdown the database and
environment, add a Close() function that does that.
2020-07-14 11:07:16 -04:00
MarcoFalke
a924f61cae
Merge #19325: wallet: Refactor BerkeleyDatabase to introduce DatabaseBatch abstract class
b82f0ca4d5465b36debb6c57f335bdccf4899c49 walletdb: Add MakeBatch function to BerkeleyDatabase and use it (Andrew Chow)
eac9200814fa01da6522625be01dded730b26751 walletdb: Refactor DatabaseBatch abstract class from BerkeleyBatch (Andrew Chow)

Pull request description:

  In order to support alternative database systems, we need to have a generic `Batch` class. This PR adds a `DatabaseBatch` abstract class which is implemented by `BerkeleyBatch`. `DatabaseBatch` is now the class that is used by `WalletBatch` to interact with the database. To be able to get the correct type of `DatabaseBatch`, `BerkeleyDatabase` now has a `MakeBatch` function which returns a newly constructed `std::unique_ptr<DatabaseBatch>`. For `BerkeleyDatabase`, that will be `std::unique_ptr<BerkeleyBatch>`.

  The `Read`, `Write`, `Erase`, and `Exists` template functions are moved from `BerkeleyBatch`.

  Part of #18971

  Requires #19308 and #19324

ACKs for top commit:
  Sjors:
    re-utACK b82f0ca4d5465b36debb6c57f335bdccf4899c49
  MarcoFalke:
    ACK b82f0ca4d5465b36debb6c57f335bdccf4899c49 🌘
  meshcollider:
    LGTM, utACK b82f0ca4d5465b36debb6c57f335bdccf4899c49

Tree-SHA512: 6d2d41631c0983391dbecd702e881c6775b155c90b275df97f7157e42608ed251744f9d7ce5173d02a6c5cc38d90b611880fac7fa635d3d8c4d590681f56ac6a
2020-07-14 16:21:01 +02:00
Andrew Chow
d0ea9bab28 walletdb: Don't remove database transaction logs and instead error
Instead of removing the database transaction logs and retrying the
wallet loading, just return an error message to the user. Additionally,
specifically for DB_RUNRECOVERY, notify the user that this could be due
to different BDB versions. This error is pretty much only caused by
compiling with a newer version of BDB and then trying to open the wallet
with a version compiled with an older version of BDB.
2020-07-13 11:00:54 -04:00
Samuel Dobson
4db44acf2d
Merge #18202: refactor: consolidate sendmany and sendtoaddress code
08fc6f6cfc3b06fd170452a766696d7b833113fa [rpc] refactor: consolidate sendmany and sendtoaddress code (Sjors Provoost)

Pull request description:

  I consolidated code between these two RPC calls, since `sendtoaddress` is essentially `sendmany` with 1 destination.

  Unless I overlooked something, the only behaviour change is that some `sendtoaddress` error codes changed from `-4` to `-6`. The release note mentions this.

  Salvaged from #18201.

ACKs for top commit:
  fjahr:
    Code review ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
  jonatack:
    ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
  meshcollider:
    Code review & functional test run ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa

Tree-SHA512: 7b66c52fa0444a4d02fc3f81d9c2a386794d447616026a30111eda35fb46510475eea6506a9ceda00bb4e0230ebb758da5d236b3ac05c954c044fa68a1e3e909
2020-07-12 14:42:35 +12:00
Samuel Dobson
32302e5c88
Merge #19490: wallet: Fix typo in comments; Simplify assert
facd7dd3d1f9d51e1133974ff69eeb48f5ae282b wallet: Fix typo in comments; Simplify assert (MarcoFalke)

Pull request description:

  Follow up to https://github.com/bitcoin/bitcoin/pull/19046#discussion_r443783101 and https://github.com/bitcoin/bitcoin/pull/19046#discussion_r443793690

ACKs for top commit:
  practicalswift:
    ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b
  jonatack:
    ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b
  hebasto:
    ACK facd7dd3d1f9d51e1133974ff69eeb48f5ae282b, spelling verified with `test/lint/lint-spelling.sh`: all remaining warnings are false positive.

Tree-SHA512: 2b185d138058840db56726bb6bcc42e5288a954e2a410c49e04806a047fbbdaf0bb2decc70ecf7613c69caa766655705ca44151613e7ea5015b386d1e726d870
2020-07-12 13:34:37 +12:00
MarcoFalke
42fe6aad32
Merge #19493: wallet: Fix clang build in Mac
1e58bcc9afefcf009653567c6373b4f7facba8f5 wallet: Fix clang build in Mac (Anthony Fieroni)

Pull request description:

  Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>

Top commit has no ACKs.

Tree-SHA512: 19312929af14dab97c37cf4547fbd6589a6de960f1a499c2118bb684240639af4b127cf8dc4d201b41d253cfbb645614a0606d4ecce29f300b10c210d38a961b
2020-07-11 19:12:43 +02:00
Anthony Fieroni
1e58bcc9af wallet: Fix clang build in Mac
Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>
2020-07-11 19:33:57 +03:00
MarcoFalke
facd7dd3d1
wallet: Fix typo in comments; Simplify assert 2020-07-11 14:24:36 +02:00
Samuel Dobson
160800ac10
Merge #19441: walletdb: don't reinitialize desc cache with multiple cache entries
a66a7a1a7060bb422eba3b8c214852416c4280d1 walletdb: don't reinitialize desc cache with multiple cache entries (Andrew Chow)

Pull request description:

  When loading descriptor caches, we would accidentally reinitialize the descriptor cache when seeing that one already exists. This should have only been initializing the cache when one does not exist. However this code itself is unnecessary as the act of looking up the cache to add to it will initialize it if it didn't already exist.

  This issue could be hit by trying to load a wallet that had imported a multisig descriptor. The wallet would fail to load.

  A test has been added to wallet_importdescriptors.py to catch this case. Another test case has also been added to check that loading a wallet with only single key descriptors works.

ACKs for top commit:
  hugohn:
    tACK [a66a7a1](a66a7a1a70)
  jonatack:
    ACK a66a7a1a706
  meshcollider:
    Code review ACK a66a7a1a7060bb422eba3b8c214852416c4280d1

Tree-SHA512: 3df746421a008708eaa3bbbdd12b9ddd3e2ec111d54625a212dca7414b971cc1f6e2b1757b3232c31a2f637d1b1ef43bf3ffa4ac4216646cf1e92db5f79954f1
2020-07-12 00:14:27 +12:00
Samuel Dobson
5f96bce9b7
Merge #18923: wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off
fa73493930e35850e877725167dc9d42e47015c8 refactor: Use C++11 range-based for loop (MarcoFalke)
fa7b164d62d9f12e9cda79bf28bf435acf2d1e38 wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off (MarcoFalke)
faf8401c195f52470d1ca6e2c94cb3820e57ee41 wallet: Pass unused args to StartWallets (MarcoFalke)
fa6c186436337c8ed7d9e1ab065377f8cda5c0b7 gui tests: Limit life-time of dummy testing setup (MarcoFalke)
fa28a618972911239a119248ab1194702a5c36d8 test: Add smoke test to check that wallets are flushed by default (MarcoFalke)

Pull request description:

  User-facing, this is a refactor. Internally, the scheduler does not have to call a mostly empty function every half a second.

ACKs for top commit:
  jnewbery:
    utACK fa73493930e35850e877725167dc9d42e47015c8
  meshcollider:
    utACK fa73493930e35850e877725167dc9d42e47015c8
  ryanofsky:
    Code review ACK fa73493930e35850e877725167dc9d42e47015c8. Just rebased since last review

Tree-SHA512: 99e1fe1b2c22a3f4b19de3e566241d38693f4fd8d5a68ba1838d86740aa6c08e3325c11a072e30fd262a8861af4278bed52eb9374c85179b8f536477f528247c
2020-07-11 23:23:28 +12:00
Samuel Dobson
89899a3448
Merge #19046: Replace CWallet::Set* functions that use memonly with Add/Load variants
3a9aba21a49a6d80bd187940d5e26893937b6832 Split SetWalletFlags into Add/LoadWalletFlags (Andrew Chow)
d9cd095b5965fc20c09f401370e7ba99446663e3 Split SetActiveScriptPubKeyMan into Add/LoadActiveScriptPubKeyMan (Andrew Chow)
0122fbab4c340b23ae56173de6c5ab866ba25ab8 Split SetHDChain into AddHDChain and LoadHDChain (Andrew Chow)

Pull request description:

  `SetHDChaiin`, `SetActiveScriptPubKeyMan`, and `SetWalletFlags` have a `memonly` argument which is kind of confusing, as noted in https://github.com/bitcoin/bitcoin/pull/17681#discussion_r427633081. This PR replaces those functions with `Add*` and `Load*` variants so that they follow the pattern used elsewhere in the wallet.

  `AddHDChain`, `AddActiveScriptPubKeyMan`, and `AddWalletFlags` both set their respective variables in `CWallet` and writes them to disk. These functions are used by the actions which modify the wallet such as `sethdseed`, `importdescriptors`, and creating a new wallet.

  `LoadHDChain`, `LoadActiveScriptPubKeyMan`, and `LoadWalletFlags` just set the `CWallet` variables. These functions are used by `LoadWallet` when loading the wallet from disk.

ACKs for top commit:
  jnewbery:
    Code review ACK 3a9aba21a49a6d80bd187940d5e26893937b6832
  ryanofsky:
    Code review ACK 3a9aba21a49a6d80bd187940d5e26893937b6832. Only changes since last review tweaks making m_wallet_flags updates more safe
  meshcollider:
    utACK 3a9aba21a49a6d80bd187940d5e26893937b6832

Tree-SHA512: 365aeaafc5ba42879c0eb797ec3beb29ab70e27f917dc880763f743420b3be6ddf797240996beed8a9ad70fb212c2590253c6b44c9dc244529c3939d9538983f
2020-07-11 23:08:54 +12:00
Samuel Dobson
4fc9224ee7
Merge #18850: wallet: Fix ZapSelectTx to sync wallet spends
9c59f9c285303659ee1beed7555bbb322e6e6981 Fix ZapSelectTx to sync wallet spends (Anthony Fieroni)

Pull request description:

  Signed-off-by: Anthony Fieroni <bvbfan@abv.bg>

ACKs for top commit:
  achow101:
    ACK 9c59f9c285303659ee1beed7555bbb322e6e6981
  ryanofsky:
    Code review ACK 9c59f9c285303659ee1beed7555bbb322e6e6981. Only change since last review tweaking the for loop as suggested
  jonatack:
    ACK 9c59f9c285303659ee1beed7555bbb322e6e6981 tested rebased on current master b33136b6ba9887f7d and the new unit test does indeed fail without the change.
  meshcollider:
    utACK 9c59f9c285303659ee1beed7555bbb322e6e6981

Tree-SHA512: 71672a5ab0c659550c3a40577614ea896412b79566b5672636ab18765e4c71b9d0a990d94dc6b6e623b03a05737022b04026b5699438809c7c54782d0fd0a5d2
2020-07-11 22:20:43 +12:00
Andrew Chow
b82f0ca4d5 walletdb: Add MakeBatch function to BerkeleyDatabase and use it
Instead of having WalletBatch construct the BerkeleyBatch, have
BerkeleyDatabase do it and return a std::unique_ptr<BerkeleyBatch>
2020-07-09 11:43:54 -04:00
Andrew Chow
eac9200814 walletdb: Refactor DatabaseBatch abstract class from BerkeleyBatch 2020-07-09 11:43:52 -04:00
MarcoFalke
fa73493930
refactor: Use C++11 range-based for loop 2020-07-09 13:08:42 +02:00
MarcoFalke
fa7b164d62
wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off 2020-07-09 13:07:41 +02:00