Commit graph

2528 commits

Author SHA1 Message Date
MarcoFalke
42fe6aad32
Merge #19493: wallet: Fix clang build in Mac
1e58bcc9af 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
Samuel Dobson
160800ac10
Merge #19441: walletdb: don't reinitialize desc cache with multiple cache entries
a66a7a1a70 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 a66a7a1a70
  meshcollider:
    Code review ACK a66a7a1a70

Tree-SHA512: 3df746421a008708eaa3bbbdd12b9ddd3e2ec111d54625a212dca7414b971cc1f6e2b1757b3232c31a2f637d1b1ef43bf3ffa4ac4216646cf1e92db5f79954f1
2020-07-12 00:14:27 +12:00
Samuel Dobson
5f96bce9b7
Merge #18923: wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off
fa73493930 refactor: Use C++11 range-based for loop (MarcoFalke)
fa7b164d62 wallet: Never schedule MaybeCompactWalletDB when -flushwallet is off (MarcoFalke)
faf8401c19 wallet: Pass unused args to StartWallets (MarcoFalke)
fa6c186436 gui tests: Limit life-time of dummy testing setup (MarcoFalke)
fa28a61897 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 fa73493930
  meshcollider:
    utACK fa73493930
  ryanofsky:
    Code review ACK fa73493930. 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
3a9aba21a4 Split SetWalletFlags into Add/LoadWalletFlags (Andrew Chow)
d9cd095b59 Split SetActiveScriptPubKeyMan into Add/LoadActiveScriptPubKeyMan (Andrew Chow)
0122fbab4c 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 3a9aba21a4
  ryanofsky:
    Code review ACK 3a9aba21a4. Only changes since last review tweaks making m_wallet_flags updates more safe
  meshcollider:
    utACK 3a9aba21a4

Tree-SHA512: 365aeaafc5ba42879c0eb797ec3beb29ab70e27f917dc880763f743420b3be6ddf797240996beed8a9ad70fb212c2590253c6b44c9dc244529c3939d9538983f
2020-07-11 23:08:54 +12:00
Samuel Dobson
4fc9224ee7
Merge #18850: wallet: Fix ZapSelectTx to sync wallet spends
9c59f9c285 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 9c59f9c285
  ryanofsky:
    Code review ACK 9c59f9c285. Only change since last review tweaking the for loop as suggested
  jonatack:
    ACK 9c59f9c285 tested rebased on current master b33136b6ba and the new unit test does indeed fail without the change.
  meshcollider:
    utACK 9c59f9c285

Tree-SHA512: 71672a5ab0c659550c3a40577614ea896412b79566b5672636ab18765e4c71b9d0a990d94dc6b6e623b03a05737022b04026b5699438809c7c54782d0fd0a5d2
2020-07-11 22:20:43 +12: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
MarcoFalke
faf8401c19
wallet: Pass unused args to StartWallets
This refactor does not change behavior
2020-07-09 13:07:37 +02:00
John Newbery
e846a2a1d9 refactor: clean up PeriodicFlush() 2020-07-09 07:00:11 +01:00
MarcoFalke
f7c19e829e
Merge #19320: wallet: Replace CDataStream& with CDataStream&& where appropriate
fa8a341b88 wallet: Replace CDataStream& with CDataStream&& where appropriate (MarcoFalke)
fa021e9a5b wallet: Remove confusing double return value ret+success (MarcoFalke)

Pull request description:

  The keys and values are only to be used once because their memory is set
  to zero. Make that explicit by moving the bytes into the lower level
  methods.

ACKs for top commit:
  sipa:
    utACK fa8a341b88
  ryanofsky:
    Code review ACK fa8a341b88. Nice changes.

Tree-SHA512: 5c0218bae0f3cd2a07346f1bbf4ad232e5dde7ef2f807d82cc6cfd208d11fe60c8b0f37e7986087b52fbfc79cdfd33c3c8a5822b3d4d9a44d1c6b09e354fc424
2020-07-09 01:01:21 +02:00
MarcoFalke
171f4a516b
Merge #19324: wallet: Move BerkeleyBatch static functions to BerkeleyDatabase
d8e9ca66d1 walletdb: Move Rewrite into BerkeleyDatabase (Andrew Chow)
91d109156d walletdb: Move PeriodicFlush into WalletDatabase (Andrew Chow)
8f1bcf8b7b walletdb: Combine VerifyDatabaseFile and VerifyEnvironment (Andrew Chow)

Pull request description:

  The `BerkeleyBatch` class has 4 static functions that operate on `BerkeleyDatabase` or `BerkeleyEnvironment`. It doesn't make sense for these to be standalone nor for them to be static functions. So instead, move them from `BerkeleyBatch` into `BerkeleyDatabase` and make them member functions instead of static.

  `BerkeleyBatch::VerifyEnvironment` and `BerkeleyBatch::VerifyDatabaseFile` are combined into a single `BerkeleyDatabase::Verify` function that operates on that `BerkeleyDatabase` object.

  `BerkeleyBatch::Rewrite` and `BerkeleyBatch::PeriodicFlush` both took a `BerkeleyDatabase` as an argument and did stuff on it. So we just make it a member function so it doesn't need to take a database as an argument.

  Part of #18971

ACKs for top commit:
  MarcoFalke:
    re-ACK d8e9ca66d1 only change is test fixup 🤞
  promag:
    Code review ACK d8e9ca66d1, good stuff.

Tree-SHA512: 9847e55b13d98bf4e5636cc14bc3f5351d56737f7e320fafffaed128606240765599e5400382c5aecac06690f7e36265ca3e1031f3f6d8a9688f6d5cb1bacd2a
2020-07-05 18:06:00 -04:00
MarcoFalke
5ec19df687
Merge #19277: util: Add Assert identity function
fab80fef61 refactor: Remove unused EnsureChainman (MarcoFalke)
fa34587f1c scripted-diff: Replace EnsureChainman with Assert in unit tests (MarcoFalke)
fa6ef701ad util: Add Assert identity function (MarcoFalke)
fa457fbd33 move-only: Move NDEBUG compile time check to util/check (MarcoFalke)

Pull request description:

  The utility function is primarily useful to dereference pointer types, which are known to be not null at that time.

  For example, the ArgsManager is known to exist when the wallets are started: https://github.com/bitcoin/bitcoin/pull/18923/files#diff-fdb2a1a1d8bc790fcddeb6cf5a42ac55R503 . Instead of silently relying on that assumption, `Assert` can be used to abort the program and avoid UB should the assumption ever be violated.

ACKs for top commit:
  promag:
    Tested ACK fab80fef61.
  ryanofsky:
    Code review ACK fab80fef61

Tree-SHA512: 830fba10152ba17d47c4dd42809c7e26f9fe6d38e17a2d5b3f054fd644a5c4c9841286ac421ec9bb28cea9f5faeb659740fcf00de6cc589d423fee7694c42d16
2020-07-04 08:44:45 -04:00
Andrew Chow
a66a7a1a70 walletdb: don't reinitialize desc cache with multiple cache entries
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.
2020-07-03 21:15:09 -04:00
MarcoFalke
915ac8a861
Merge #19413: refactor: Remove confusing BlockIndex global
fa0dfdf447 refactor: Remove confusing BlockIndex global (MarcoFalke)

Pull request description:

  The global `::BlockIndex()` is problematic for several reasons:

  * It returns a mutable reference to the block tree, without the appropriate lock annotation (`m_block_index` is guarded by `cs_main`). The current code is fine, but in the future this might lead to accidental races and data corruption.
  * The rpc server shouldn't rely on node globals, but rather a context that is passed in to the RPC method.
  * Tests might want to spin up their own block tree, and thus should also not rely on a single global.

  Fix all issues by removing the global

ACKs for top commit:
  promag:
    Code review ACK fa0dfdf447.
  jonatack:
    re-ACK fa0dfdf

Tree-SHA512: 8f158fc5e1c67e73588a21c25677b3fa0fe442313b13ec24b87054806c59607d6ba0c062a865ce3e0ee568706bd0d1faa84febda21aff5bcd65dab172f74c52f
2020-07-03 07:38:16 -04:00
Samuel Dobson
a24806c25d
Merge #19215: psbt: Include and allow both non_witness_utxo and witness_utxo for segwit inputs
84d295e513 tests: Check that segwit inputs in psbt have both UTXO types (Andrew Chow)
4600479058 psbt: always put a non_witness_utxo and don't remove it (Andrew Chow)
5279d8bc07 psbt: Allow both non_witness_utxo and witness_utxo (Andrew Chow)
72f6bec1da rpc: show both UTXOs in decodepsbt (Andrew Chow)

Pull request description:

  Due to recent changes to hardware wallets, the full previous transaction will need to be provided for segwit inputs. Since some software may be checking for the existence of a `witness_utxo` to determine whether to produce a segwit signature, we keep that field to ease the transition.

  Because all of the sanity checks implemented by the `IsSane` functions were related to having mixed segwit and non-segwit data in a PSBT, those functions are removed as those checks are no longer proper.

  Some tests are updated/removed to accommodate this and a simple test added to check that both UTXOs are being added to segwit inputs.

  As discussed in the wallet IRC meeting, our own signer will not require `non_witness_utxo` for segwit inputs.

ACKs for top commit:
  Sjors:
    utACK 84d295e513 (didn't retest compared to 836d6fc, but fortunately HWI's CI tracks our master branch, with a bunch of hardware wallet simulators)
  ryanofsky:
    Code review re-ACK 84d295e513. No changes since last review, but now I understand the context better. I think it would good to improve the comments as suggested https://github.com/bitcoin/bitcoin/pull/19215#discussion_r447889473 and maybe refer to
  meshcollider:
    utACK 84d295e513

Tree-SHA512: ccc1fd3c16ac3859f5aca4fa489bd40f68be0b81bbdc4dd51188bbf28827a8642dc8b605a37318e5f16cf40f1c4910052dace2f27eca21bb58435f02a443e940
2020-07-03 09:23:22 +12:00
Wladimir J. van der Laan
7173a3c73b
Merge #19396: refactor: Remove confusing OutputType::CHANGE_AUTO
fa927ff884 Enable Wswitch for OutputType (MarcoFalke)
faddad71f6 Remove confusing OutputType::CHANGE_AUTO (MarcoFalke)
fa2eb38352 interfaces: Remove unused getDefaultChangeType (MarcoFalke)

Pull request description:

  `OutputType::CHANGE_AUTO` is problematic for several reasons:

  * An output that is not change must never be described by `CHANGE_AUTO`. Simply allowing that option makes the code confusing and review harder than it needs to be.
  * To make review even harder, `CHANGE_AUTO` requires `-Wswitch` to be disabled for `OutputType`

  Fix both issues by removing `CHANGE_AUTO` and then enabling `-Wswitch` for `OutputType`

ACKs for top commit:
  promag:
    Code review ACK fa927ff884.
  laanwj:
    Code review ACK fa927ff884

Tree-SHA512: 24fd809757aa343866c94dafe9a7130b50cda4f77c97666d407f99b813f75b115a7d8e688a6bc2a737e87cba64ddd4e43f2b3c5538fd35fabb5845807bb39134
2020-07-02 16:10:49 +02:00
MarcoFalke
fa927ff884
Enable Wswitch for OutputType 2020-07-01 18:03:12 -04:00
MarcoFalke
faddad71f6
Remove confusing OutputType::CHANGE_AUTO 2020-07-01 18:02:38 -04:00
MarcoFalke
fa575f3461
wallet: Replace boost::none with nullopt 2020-07-01 17:24:49 -04:00
Andrew Chow
d8e9ca66d1 walletdb: Move Rewrite into BerkeleyDatabase
Make Rewrite actually a member of BerkeleyDatabase instead of a static
function in BerkeleyBatch
2020-07-01 12:32:11 -04:00
Andrew Chow
91d109156d walletdb: Move PeriodicFlush into WalletDatabase
Make PeriodicFlush a non-static member of WalletDatabase instead of
WalletBatch.
2020-07-01 12:32:06 -04:00
Andrew Chow
8f1bcf8b7b walletdb: Combine VerifyDatabaseFile and VerifyEnvironment
Combine these two functions into a single Verify function that is a
member of WalletDatabase. Additionally, these are no longer static.
2020-07-01 12:32:03 -04:00
Wladimir J. van der Laan
26291745ae
Merge #19308: wallet: BerkeleyBatch Handle cursor internally
ca24edfbc1 walletdb: Handle cursor internally (Andrew Chow)

Pull request description:

  Instead of returning a Dbc (BDB cursor object) and having the caller deal with the cursor, make BerkeleyBatch handle the cursor internally.

  Split from #18971

ACKs for top commit:
  ryanofsky:
    Code review ACK ca24edfbc1. Changes since last review: StartCursor rename, moving CloseCursor calls near returns
  promag:
    Code review ACK ca24edfbc1.

Tree-SHA512: f029b498c7f275aedca53ce7ade7cb99c82975fd6cad17346a4990fb3bcc54e2a5309b32053bd13def9ee464d331b036ac79abb8fc4fa561170c6cfc85283447
2020-07-01 16:00:32 +02:00
Wladimir J. van der Laan
bb588669f9
Merge #19331: build: Do not include server symbols in wallet
faca73000f ci: Install fixed version of clang-format for linters (MarcoFalke)
fa4695da4c build: Sort Makefile.am after renaming file (MarcoFalke)
cccc2784a3 scripted-diff: Move ui_interface to the node lib (MarcoFalke)
fa72ca6a9d qt: Remove unused includes (MarcoFalke)
fac96e6450 wallet: Do not include server symbols (MarcoFalke)
fa0f6c58c1 Revert "Fix link error with --enable-debug" (MarcoFalke)

Pull request description:

  This reverts a hacky workaround from commit b83cc0f, which only happens to work due to compiler optimizations. Then, it actually fixes the linker error.

  The underlying problem is that the wallet includes symbols from the server (ui_interface), which usually results in linker failures. Though, in this specific case the linker failures have not been observed (unless `-O0`) because our compilers were smart enough to strip unused symbols.

  Fix the underlying problem by creating a new header-only with the needed symbol and move ui_interface to node to clarify that this is part of libbitcoin_server.

ACKs for top commit:
  Sjors:
    ACK faca730
  laanwj:
    ACK faca73000f
  hebasto:
    re-ACK faca73000f, since the [previous](https://github.com/bitcoin/bitcoin/pull/19331#pullrequestreview-434420539) review:

Tree-SHA512: e9731f249425aaea50b6db5fc7622e10078cf006721bb87989cac190a2ff224412f6f8a7dd83efd018835302337611f5839e29e15bef366047ed591cef58dfb4
2020-07-01 15:38:18 +02:00
MarcoFalke
fa0dfdf447
refactor: Remove confusing BlockIndex global 2020-06-29 20:28:47 -04:00
MarcoFalke
5c3c7cc50c
Merge #19300: wallet: Handle concurrent wallet loading
9b009fae6e qa: Test concurrent wallet loading (João Barbosa)
b9971ae585 wallet: Handle concurrent wallet loading (João Barbosa)

Pull request description:

  This PR handles concurrent wallet loading.

  This can be tested by running in parallel the following script a couple of times:
  ```sh
  for i in {1..10}
  do
    src/bitcoin-cli -regtest loadwallet foo
    src/bitcoin-cli -regtest unloadwallet foo
  done
  ```

  Eventually the error occurs:
  ```
  error code: -4
  error message:
  Wallet already being loading.
  ```

  For reference, loading and already loaded wallet gives:
  ```
  error code: -4
  error message:
  Wallet file verification failed. Error loading wallet w1. Duplicate -wallet filename specified.
  ```

  Fixes #19232.

ACKs for top commit:
  MarcoFalke:
    Concept ACK 9b009fae6e I have not reviewed the code
  hebasto:
    ACK 9b009fae6e, tested on Linux Mint 20 (x86_64):
  ryanofsky:
    Code review good-but-not-ideal ACK 9b009fae6e

Tree-SHA512: 0ccd77b03c0926e4c4e51efb31e193b93cb4b9ffe8bac6bb018f7344c55dfd939b873b8cf5e657dca73e6202eb75aa672de2acb787cc133184b0b3b51e47b972
2020-06-29 11:14:26 -04:00
MarcoFalke
d3a5dbfd1f
Merge #19114: scripted-diff: TxoutType C++11 scoped enum class
fa32adf9dc scripted-diff: TxoutType C++11 scoped enum class (MarcoFalke)
fa95a694c4 doc: Update outdated txnouttype documentation (MarcoFalke)
fa58469c77 rpc: Properly use underlying type in GetAllOutputTypes (MarcoFalke)
fa41c65702 rpc: Simplify GetAllOutputTypes with the Join helper (MarcoFalke)

Pull request description:

  Non-scoped enums can accidentally and silently decay into an integral type. Also, the symbol names of the keys are exported to the surrounding (usually global) namespace.

  Fix both issues by switching to an `enum class TxoutType` in a (mostly) scripted-diff.

ACKs for top commit:
  practicalswift:
    ACK fa32adf9dc -- patch looks correct
  hebasto:
    re-ACK fa32adf9dc, since fa5997bd6fc82e16b597ea96e3c5c665f1f174ab (https://github.com/bitcoin/bitcoin/pull/19114#pullrequestreview-421425198) rebased only (verified with `git range-diff`).

Tree-SHA512: f42a9db47f9be89fa4bdd8d2fb05a16726286d8b12e3d87327b67d723f91c7d5a57deb4b2ddae9e1d16fee7a5f8c00828b6dc8909c5db680fc5e0a3cf07cd465
2020-06-28 14:20:00 -04:00
MarcoFalke
cccc2784a3
scripted-diff: Move ui_interface to the node lib
-BEGIN VERIFY SCRIPT-

 # Move files
 git mv src/ui_interface.h                                          src/node/ui_interface.h
 git mv src/ui_interface.cpp                                        src/node/ui_interface.cpp
 sed -i -e 's/BITCOIN_UI_INTERFACE_H/BITCOIN_NODE_UI_INTERFACE_H/g' src/node/ui_interface.h

 # Adjust includes and makefile
 sed -i -e 's|ui_interface|node/ui_interface|g' $(git grep -l ui_interface)

 # Sort includes
 git diff -U0 | clang-format-diff -p1 -i -v

-END VERIFY SCRIPT-
2020-06-27 11:49:28 -04:00
MarcoFalke
fac96e6450
wallet: Do not include server symbols
ui_interface is in libbitcoin_server and can not be included in the
wallet because the wallet does not link with server symbols.
2020-06-27 11:39:09 -04:00
Wladimir J. van der Laan
f32f7e907a
Merge #11413: [wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option
25dac9fa65 doc: add release notes for explicit fee estimators and bumpfee change (Karl-Johan Alm)
05227a3554 tests for bumpfee / estimate_modes (Karl-Johan Alm)
3404c1b753 policy: optional FeeEstimateMode param to CFeeRate::ToString (Karl-Johan Alm)
6fcf448430 rpc/wallet: add two explicit modes to estimate_mode (Karl-Johan Alm)
b188d80c2d MOVEONLY: Make FeeEstimateMode available to CFeeRate (Karl-Johan Alm)
5d1a411eb1 fees: add FeeModes doc helper function (Karl-Johan Alm)
91f6d2bc8f rpc/wallet: add conf_target as alias to confTarget in bumpfee (Karl-Johan Alm)
69158b41fc added CURRENCY_ATOM to express minimum indivisible unit (Karl-Johan Alm)

Pull request description:

  This lets users pick their own fees when using `sendtoaddress`/`sendmany` if they prefer this over the estimators.

ACKs for top commit:
  Sjors:
    re-utACK 25dac9fa65: rebased, more fancy C++,
  jonatack:
    ACK 25dac9fa65 I think this should be merged after all this time, even though it looks to me like there are needed follow-ups, fixes and test coverage to be added (see further down), which I don't mind helping out with, if wanted.
  fjahr:
    Code review ACK 25dac9fa65

Tree-SHA512: f31177e6cabf3187a43cdfe93477144f8e8385c7344613743cbbd16e8490d53ff5144aec7b9de6c9a65eb855b55e0f99d7f164dee4b6bf3cfea4dce51cf11d33
2020-06-25 19:53:42 +02:00
Andrew Chow
4600479058 psbt: always put a non_witness_utxo and don't remove it
Offline signers will always need a non_witness_utxo so make sure it is
there.
2020-06-24 16:32:19 -04:00
Andrew Chow
5279d8bc07 psbt: Allow both non_witness_utxo and witness_utxo 2020-06-24 16:31:42 -04:00
Wladimir J. van der Laan
bd93e32292 refactor: Replace HexStr(o.begin(), o.end()) with HexStr(o)
HexStr can be called with anything that bas `begin()` and `end()` functions,
so clean up the redundant calls.
2020-06-24 18:41:45 +02:00
Karl-Johan Alm
6fcf448430
rpc/wallet: add two explicit modes to estimate_mode 2020-06-24 16:01:37 +09:00
Karl-Johan Alm
5d1a411eb1
fees: add FeeModes doc helper function 2020-06-24 15:52:05 +09:00
Andrew Chow
ca24edfbc1 walletdb: Handle cursor internally
Instead of returning a Dbc (BDB cursor object) and having the caller
deal with the cursor, make BerkeleyBatch handle the cursor internally.

This prepares BerkeleyBatch to work with other database systems as Dbc
objects are BDB specific.
2020-06-22 15:36:23 -04:00
Andrew Chow
3a9aba21a4 Split SetWalletFlags into Add/LoadWalletFlags
Remove memonly bool and follow typical Add and Load pattern used
everywhere else.
2020-06-22 14:59:09 -04:00
Samuel Dobson
c27330897d
Merge #18027: "PSBT Operations" dialog
931dd47608 Make lint-spelling.py happy (Glenn Willen)
11a0ffb29d [gui] Load PSBT from clipboard (Glenn Willen)
a6cb0b0c29 [gui] PSBT Operations Dialog (sign & broadcast) (Glenn Willen)
5dd0c03ffa FillPSBT: report number of inputs signed (or would sign) (Glenn Willen)
9e7b23b733 Improve TransactionErrorString messages. (Glenn Willen)

Pull request description:

  Add a "PSBT Operations" dialog, reached from the "Load PSBT..." menu item, giving options to sign or broadcast the loaded PSBT as appropriate, as well as copying the result to the clipboard or saving it to a file.

  This is based on Sjors' #17509, and depends on that PR going in first. (It effectively replaces the small "load PSBT" dialog from that PR with a more feature-rich one.)

  Some notes:
  * The way I display status information is maybe unusual (a status bar, rather than messageboxes.) I think it's helpful to have the information in it be persistent rather than transitory. But if people dislike it, I would probably move the "current state of the transaction" info to the top line of the main label, and the "what action just happened, and did it succeed" info into a messagebox.
  * I don't really know much about the translation/localization stuff. I put tr() in all the places it seemed like it ought to go. I did not attempt to translate the result of TransactionErrorString (which is shared by GUI and non-GUI code); I don't know if that's correct, but it matches the "error messages in logs should be googleable in English" heuristic. I don't know whether there are things I should be doing to reduce translator effort (like minimizing the total number of distinct message strings I use, or something.)
  * I don't really know how (if?) automated testing is applied to GUI code. I can make a list of PSBTs exercising all the codepaths for manual testing, if that's the right approach. Input appreciated.

ACKs for top commit:
  instagibbs:
    tested ACK 931dd47608
  Sjors:
    re-tACK 931dd47608
  jb55:
    ACK 931dd47608
  achow101:
    ACK 931dd47608

Tree-SHA512: ade52471a2242f839a8bd6a1fd231443cc4b43bb9c1de3fb5ace7c5eb59eca99b1f2e9f17dfdb4b08d84d91f5fd65677db1433dd03eef51c7774963ef4e2e74f
2020-06-21 22:57:33 +12:00
MarcoFalke
fa32adf9dc
scripted-diff: TxoutType C++11 scoped enum class
-BEGIN VERIFY SCRIPT-
 # General rename helper: $1 -> $2
 rename_global() { sed -i "s/\<$1\>/$2/g" $(git grep -l "$1"); }

 # Helper to rename TxoutType $1
 rename_value() {
   sed -i "s/    TX_$1,/    $1,/g" src/script/standard.h;  # First strip the prefix in the definition (header)
   rename_global TX_$1 "TxoutType::$1";                    # Then replace globally
 }

 # Change the type globally to bring it in line with the style-guide
 # (clsses are UpperCamelCase)
 rename_global 'enum txnouttype' 'enum class TxoutType'
 rename_global      'txnouttype'            'TxoutType'

 # Now rename each enum value
 rename_value 'NONSTANDARD'
 rename_value 'PUBKEY'
 rename_value 'PUBKEYHASH'
 rename_value 'SCRIPTHASH'
 rename_value 'MULTISIG'
 rename_value 'NULL_DATA'
 rename_value 'WITNESS_V0_KEYHASH'
 rename_value 'WITNESS_V0_SCRIPTHASH'
 rename_value 'WITNESS_UNKNOWN'

-END VERIFY SCRIPT-
2020-06-21 06:41:55 -04:00
Samuel Dobson
02b26ba1c1
Merge #19200: rpc: remove deprecated getaddressinfo fields
bc01f7ae05 doc: release note for rpc getaddressinfo removals (Jon Atack)
90e989390e rpc: getaddressinfo RPCResult fixup (Jon Atack)
a8507c99da rpc: remove deprecated getaddressinfo `labels: purpose` (Jon Atack)
645a8653c8 rpc: remove deprecated getaddressinfo `label` field (Jon Atack)

Pull request description:

  These were deprecated in #17578 and #17585, with expected 0.21 removal notified in the 0.20 release notes.
  ```
  - The `getaddressinfo` RPC has had its `label` field deprecated
    (re-enable for this release using the configuration parameter
    `-deprecatedrpc=label`).  The `labels` field is altered from returning
    JSON objects to returning a JSON array of label names (re-enable
    previous behavior for this release using the configuration parameter
    `-deprecatedrpc=labelspurpose`).  Backwards compatibility using the
    deprecated configuration parameters is expected to be dropped in the
    0.21 release.  (#17585, #17578)
  ```

ACKs for top commit:
  Sjors:
    utACK bc01f7a
  adamjonas:
    utACK bc01f7a
  meshcollider:
    utACK bc01f7ae05

Tree-SHA512: ae1af381e32c4c3bde8b061a56382838513a9a82c88767843cdeae3a2ab8aa7d8c2e66e106d2b31ea07d74bb80c191a2f842c9aaecc7c5438ad9a9bc66d1b251
2020-06-21 21:07:00 +12:00
Samuel Dobson
6bb5f6d8e3
Merge #16377: [rpc] don't automatically append inputs in walletcreatefundedpsbt
e5327f947c [rpc] fundrawtransaction: add_inputs option to control automatic input adding (Sjors Provoost)
79804fe24b [rpc] walletcreatefundedpsbt: don't automatically append inputs (Sjors Provoost)

Pull request description:

  When the user doesn't specificy inputs, it makes sense to automatically select them. But when the user does specify inputs, `walletcreatefundedpsbt` now fails if the amount is insufficient, unless `addInputs` is set to `true`.

  Similarly for `fundrawtransaction` if the original transaction already specified inputs, we only add more if `addInputs` is set to `true`.

  This protects against fat finger mistakes in the amount or fee rate (see also #16257). The behavior is also more similar to GUI coin selection.

ACKs for top commit:
  achow101:
    ACK e5327f947c
  meshcollider:
    utACK e5327f947c

Tree-SHA512: d8653b820914396c7c25b0d0a2b7e92de214aa023bc1aa085feb37d3b20fab361ebea90416a7db989f19bdc37e26cf0adfbcb712c80985c87afa67a9bd44fecb
2020-06-21 20:52:34 +12:00
Samuel Dobson
bd331bd745
Merge #17938: Disallow automatic conversion between disparate hash types
4d7369125a Disallow automatic conversion between hash types (Ben Woosley)
fa9ef2cdbe Remove an apparently unnecessary conversion (Ben Woosley)
966a22d859 Explicitly support conversion between equivalent hash types (Ben Woosley)
f32c1e07fd Use explicit conversion from WitnessV0KeyHash -> CKeyID (Ben Woosley)
2c54217f91 Use explicit conversion from PKHash -> CKeyID (Ben Woosley)
a9e451f144 Convert CPubKey to WitnessV0KeyHash directly (Ben Woosley)
3fcc468123 Prefer explicit CScriptID construction (Ben Woosley)
0a5ea32ce6 Prefer explicit uint160 conversion (Ben Woosley)

Pull request description:

  This bases the script/standard hash types, TxDestination-related and CScriptID on a base template which does not silently convert the underlying `uintN` type.

  Inspired by and built on #17924. Commits are small and focused to ease review.

  Note some of these changes may be relative to existing bugs of the same sort as #17924. See particularly "Convert CPubKey to WitnessV0KeyHash directly" and "Remove an apparently unnecessary conversion".

ACKs for top commit:
  achow101:
    ACK 4d7369125a
  meshcollider:
    re-utACK 4d7369125a

Tree-SHA512: f1b3284ddc6fb6c6e726f2c22668b6d732d45eb5418262ed2b9c728f60be7be43dfb414b6ddd9915025c8dcd7f360dc3b46e997a945a2feb95b0e5c4f05d6b54
2020-06-21 20:26:59 +12:00
MarcoFalke
fa8a341b88
wallet: Replace CDataStream& with CDataStream&& where appropriate
The keys and values are only to be used once because their memory is set
to zero. Make that explicit by moving the bytes into the lower level
methods.
2020-06-20 08:42:35 -04:00
MarcoFalke
fa021e9a5b
wallet: Remove confusing double return value ret+success
Also, remove redundant comments
2020-06-20 08:41:19 -04:00
MarcoFalke
879acc681a
Merge #19018: docs: fixing description of the field sequence in walletcreatefundedpsbt RPC method
d0a3feea73 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 d0a3feea73

Tree-SHA512: 3f429a2c2eea283a47fb5002a99f7e2a5ed6f67df9fd895c1ab938256c48a6497ed6ac2673d8fe8968dfb67b939f4a84570899d9faf52f3abd6ec90c0703d1bd
2020-06-20 07:30:24 -04:00
Karl-Johan Alm
91f6d2bc8f
rpc/wallet: add conf_target as alias to confTarget in bumpfee 2020-06-20 15:35:21 +09:00
MarcoFalke
d4f9ae0025
Merge #19054: wallet: Skip hdKeypath of 'm' when determining inactive hd seeds
951bca61d7 tests: feature_backwards_compatibility.py test 0.16 up/downgrade (Andrew Chow)
3a03a11e8c Skip hdKeypath of 'm' (Andrew Chow)

Pull request description:

  Previously the seed was stored with keypath 'm' so we need to skip this as well when determining inactive seeds.

  Fixes #19051

ACKs for top commit:
  Sjors:
    ACK 951bca61d7
  instagibbs:
    re-utACK 951bca61d7
  ryanofsky:
    Code review ACK 951bca61d7. No significant changes since last review, just updated comment and some test tweaks

Tree-SHA512: 930f77e7097c9cf4f1012e540bd2b1a72fd279262517f10c1531b2ad48c632ef95e0dd4edea81bcc3b3db306479d34e5e79e5d6c4ed31dfa4b77a4231436436e
2020-06-19 16:14:47 -04:00
Ben Woosley
fa9ef2cdbe
Remove an apparently unnecessary conversion
CScript -> CScriptID -> ScriptHash is unnecessary because
ScriptHash and CScriptID do the same thing.
2020-06-19 12:14:08 -07:00
Ben Woosley
f32c1e07fd
Use explicit conversion from WitnessV0KeyHash -> CKeyID
These types are equivalent, in data etc, so they need only their
data cast across.
2020-06-19 12:14:08 -07:00