Commit graph

151 commits

Author SHA1 Message Date
Russell Yanofsky
7e66d04770 Drop signal CClientUIInterface::LoadWallet
Github-Pull: #16963
Rebased-From: 81ea66c30e2953dee24d5b127c28daa0d9452a28
2020-01-14 09:34:35 +00:00
Jonas Schnelli
884f7cc81b
Merge #16714: gui: add prune to intro screen with smart default
9924bce317 [gui] intro: enable pruning by default unless disk is big (Sjors Provoost)
c8de347a9d [gui] intro: add prune preference (Sjors Provoost)
1bbc49d207 [gui] intro: inform caller if intro was shown (Sjors Provoost)
1957103786 [gui] add explicit prune setter (Sjors Provoost)
1bccf6a52d [node] add forceSetArg to interface (Sjors Provoost)

Pull request description:

  This adds a checkbox to the intro screen to enable pruning from the get go.

  If the user has plenty of space, it's unchecked by default:

  <img width="671" alt="big" src="https://user-images.githubusercontent.com/10217/63641289-10339000-c6ac-11e9-98d7-caf64dff0da6.png">

  If the user has insufficient space it's checked by default:
  <img width="897" alt="low" src="https://user-images.githubusercontent.com/10217/63641276-d4002f80-c6ab-11e9-9f5b-a53472f814ff.png">

  When the user has barely enough space and is likely to need pruning in the near future, this is shown in yellow and we also check the prune box:

  <img width="662" alt="medium" src="https://user-images.githubusercontent.com/10217/63641294-1c1f5200-c6ac-11e9-8ecb-6b69e42b1ece.png">

  The cut-off for this 10 GB above `m_assumed_blockchain_size` (`=240` in `chainparams.cpp`).

  If the user launches the first time with `-prune=...` then we disable the check box and display the correct size (rounded to GB):
  <img width="658" alt="Schermafbeelding 2019-08-24 om 20 23 14" src="https://user-images.githubusercontent.com/10217/63641351-09594d00-c6ad-11e9-94fe-fe5ed562e109.png">

  The 2 GB default matches the settings default. The user can't change it in the intro screen, but can change it later. I'm tempted to increase that default to 10 GB, and then have the intro screen reduce it if space is really tight.

  Tips for testing:
  * move your existing data dir elsewhere
  * wipe data dir at every restart (behavior is different if it exists)
  * launch with `bitcoin-qt -resetguisettings -lang=en` (there's some space issues in different languages)
  * fake your free space by changing `intro.cpp` line 90: `freeBytesAvailable = 5000000000; // 5 GB`
  * try both testnet and mainnet, because settings are seperate. In particular note how step 7 in `GuiMain` switches where `QTSettings settings` points to; this had me thoroughly confused on testnet, because I was setting them too early.

ACKs for top commit:
  jonasschnelli:
    Tested ACK 9924bce317
  ryanofsky:
    utACK 9924bce317. The changes are very logical, and implement the feature in a clean that way that doesn't add a lot of complication and shouldn't interfere with future improvements. I looked at Luke's branch too, and I think there's also a lot of great stuff there that seems fully compatible with this change.

Tree-SHA512: 9523961451c53aebd347716976bc3a4a398f989dc21e9bbbd357060bd11a8f46c435f068bd421bb31ccb08e55445ef67bc347d8d19a4fb8fde9d6d3f9a3bcbb0
2019-09-12 15:00:51 +02:00
Andrew Chow
9b41cbb28f Expose wallet creation to the GUI via WalletController
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
2019-09-05 20:36:57 -04:00
Sjors Provoost
1bccf6a52d
[node] add forceSetArg to interface 2019-08-24 22:36:11 +02:00
Antoine Riard
a31be09bfd Encapsulate tx status in a Confirmation struct
Instead of relying on combination of hashBlock and nIndex
values to manage tx in its lifecycle, we introduce 4
status : CONFIRMED, UNCONFIRMED, CONFLICTED, ABANDONED.

hashBlock and nIndex magic values should only be used at
serialization/deserialization for backward-compatibility.

At block disconnection, we know flag txn as UNCONFIRMED where
previously they kept their states until being override by a
block connection or abandontransaction call. This is a change
in behavior for which user may have to call abandon twice
if transaction is disconnected and not accepted back in the mempool.

We assert status transitioning right in AddToWallet. Doing so
flagged a misbehavior in ComputeTimeSmart unit test where same
tx is confirmed twice in different block. To avoid inconsistencies
we unconfirmed tx before new connection in different block. We
also remove a cs_main lock in test, as AddToWallet and its
callees don't rely on locked chain.
2019-08-23 14:53:20 -04:00
MarcoFalke
85883a9f8e
Merge #16443: refactor: have CCoins* data managed under CChainState
582d2cd747 Cover UTXO set access with lock annotations (James O'Beirne)
5693530685 refactor: have CCoins* data managed under CChainState (James O'Beirne)
fae6ab6aed refactor: pcoinsTip -> CChainState::CoinsTip() (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):

  Parent PR: #15606
  Issue: #15605
  Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal

  ---

  This change encapsulates UTXO set data within CChainState instances, removing global data `pcoinsTip` and `pcoinsviewdb`. This is necessary if we want to maintain multiple chainstates with their own rendering of the UTXO set.

  We introduce a class CoinsViews which consolidates the construction of a CCoins* hierarchy.

  This commit could be broken into smaller pieces, but it would require more ephemeral diffs to, e.g., temporarily change CCoinsViewDB's constructor invocations.

ACKs for top commit:
  Sjors:
    reACK 582d2cd747
  MarcoFalke:
    ACK 582d2cd747

Tree-SHA512: ec9d904fe5dca8cd2dc4b7916daa5d8bab30856dd4645987300f905e0a19f9919fce4f9d1ff03eda982943ca73e6e9a746be6cf53b46510de36e8c81a1eafba1
2019-08-15 12:47:15 -04:00
Antoine Riard
b7b9f6e4ce Remove p2pEnabled from Chain interface
RPC server starts in warmup mode, it can't
process yet calls, then follows connection manager
initialization and finally RPC server get out of
warmup mode. RPC calls shouldn't be able to get
P2P disabled errors because once we initialize
g_connman it's not unset until shutdown, after
RPC server has been stopped.
2019-08-08 22:57:35 -04:00
James O'Beirne
fae6ab6aed refactor: pcoinsTip -> CChainState::CoinsTip()
This aliasing makes subsequent commits easier to review; eventually CoinsTip()
will return the CCoinsViewCache managed by CChainState.
2019-08-06 13:13:06 -04:00
fanquake
31d98584f3
Merge #16497: gui: Generate bech32 addresses by default (take 2, fixup)
fa5a4cd813 gui: Generate bech32 addresses by default (take 2, fixup) (MarcoFalke)

Pull request description:

  This commit was missing from my previous pull request for some reason 🤔 :

  *    gui: Generate bech32 addresses by default #15711

ACKs for top commit:
  jonasschnelli:
    Tested ACK fa5a4cd813
  promag:
    ACK fa5a4cd813.
  fanquake:
    ACK fa5a4cd813

Tree-SHA512: 4a38df929d7704bf08e50a2e814b2e6cd25c4165d040a84287045b44e32f4708750845520d64170ea58e41de3ca496da4625d3eb375f9528b21b364c22068a6b
2019-08-06 09:39:26 +08:00
Antoine Riard
b8eecf8e79 Remove unused submitToMemoryPool and relayTransactions Chain interfaces 2019-08-01 13:43:29 -04:00
Antoine Riard
8c8aa19b4b Add BroadcastTransaction utility usage in Chain interface
Access through a broadcastTransaction method.
Add a wait_callback flag to turn off race protection when wallet
already track its tx being in mempool

Standardise highfee, absurdfee variable name to max_tx_fee

We drop the P2P check in BroadcastTransaction as g_connman is only
called by RPCs and the wallet scheduler, both of which are initialized
after g_connman is assigned and stopped before g_connman is reset.
2019-08-01 13:43:26 -04:00
MarcoFalke
7821821a23
Merge #16452: refactor: use RelayTransaction in BroadcastTransaction utility
9bc8b28c1d refactor : use RelayTransaction in BroadcastTransaction utility (Antoine Riard)

Pull request description:

  Implementing suggestion in https://github.com/bitcoin/bitcoin/pull/15713#discussion_r306571420.

  Seems a reason of these node utilities is to glue with already there functions, so we should reuse them.

ACKs for top commit:
  MarcoFalke:
    ACK 9bc8b28c1d
  promag:
    ACK 9bc8b28c1d, verified there are no more `PushInventory(CInv(MSG_TX, ...`, nice refactor, 👍 @amitiuttarwar.
  jnewbery:
    ACK 9bc8b28c1d
  jonatack:
    ACK 9bc8b28c1d, second @jnewbery's suggestions, my guess is they could be added without risking delaying this PR.

Tree-SHA512: 841c65d5f0d9ead5380814bb2260d7ebf03f2a9bfa58a1025785d353bdb42f9122cc257993e6a7bd2bd3f2c74db19c5978cc14be0d83258124ca22e33d6d0164
2019-07-31 10:49:28 -04:00
MarcoFalke
fa5a4cd813
gui: Generate bech32 addresses by default (take 2, fixup) 2019-07-30 13:53:21 -04:00
MeshCollider
1139e3cb76
Merge #16415: Get rid of PendingWalletTx class
4d94916f0d Get rid of PendingWalletTx class. (Russell Yanofsky)

Pull request description:

  No reason for this class to exist if it doesn't have any code to run in the destructor. e10e1e8db0 from https://github.com/bitcoin/bitcoin/pull/16208 recently removed the destructor code that would return an unused key if the transaction wasn't committed.

  This is just cleanup, there's no change in behavior.

ACKs for top commit:
  ariard:
    utACK 4d94916. Successfully built both `bitcoind` and `bitcoin-qt`. `PendingWalletTx` was only a wrapper to enforce call to `ReturnDestination` if `CommitTransaction` doesn't `KeepDestination` before.
  promag:
    ACK 4d94916f0d, refactor looks good to me.
  meshcollider:
    utACK 4d94916f0d

Tree-SHA512: f3f93d2f2f5d8f1e7810d609d881c1b1cbbaa8629f483f4293e20b3210292605e947bc4903fde9d2d8736277ca3bd6de182f7eac1e13515d5a327f2ebc130839
2019-07-27 22:35:32 +12:00
Antoine Riard
9bc8b28c1d refactor : use RelayTransaction in BroadcastTransaction utility
To do so, we also refactor RelayTransaction to take a txid
instead of passing a tx
2019-07-24 19:47:56 -04:00
MarcoFalke
67923d6b3c
Merge #16366: init: Use InitError for all errors in bitcoind/qt
fa6f402bde Call node->initError instead of InitError from GUI code (Russell Yanofsky)
fad2502240 init: Use InitError for all errors in bitcoind/qt (MarcoFalke)

Pull request description:

  Using the same InitError for startup error in the daemon and the gui makes it possible to run the tests with the gui again:

  ```sh
  BITCOIND=bitcoin-qt ./test/functional/test_runner.py feature_includeconf feature_config_args

ACKs for top commit:
  hebasto:
    ACK fa6f402bde
  ryanofsky:
    utACK fa6f402bde. Only changes since last review are removing more includes and adding Node::initError method to avoid accessing node `InitError` function and global variables from GUI code.

Tree-SHA512: bd19e08dcea4019dfe40356bc5c63cb583cefed54b6c9dcfb82f1b5b00308d8e2b363549afcaea5e93bf83864dbe0917400c3b70f43a8a5bdff45c9cd34cc294
2019-07-23 18:40:46 -04:00
Russell Yanofsky
4d94916f0d Get rid of PendingWalletTx class.
No reason for this class to exist if it doesn't have any code to run in the
destructor. e10e1e8db0 from
https://github.com/bitcoin/bitcoin/pull/16208 recently removed code destructor
code that would return an unused key if the transaction wasn't committed.
2019-07-18 12:06:23 -04:00
MeshCollider
459baa1756
Merge #16208: wallet: Consume ReserveDestination on successful CreateTransaction
e10e1e8db0 Restrict lifetime of ReserveDestination to CWallet::CreateTransaction (Gregory Sanders)
d9ff862f2d CreateTransaction calls KeepDestination on ReserveDestination before success (Gregory Sanders)

Pull request description:

  The typical usage pattern of `ReserveDestination` is to explicitly `KeepDestination`, or `ReturnDestination` when it's detected it will not be used.

  Implementers such as myself may fail to complete this pattern, and could result in key re-use: https://github.com/bitcoin/bitcoin/pull/15557#discussion_r271956393

  Since ReserveDestination is currently only used directly in the `CreateTransaction`/`CommitTransaction` flow(or fee bumping where it's just used in `CreateTransaction`), I instead make the assumption that if a transaction is returned by `CreateTransaction` it's highly likely that it will be accepted by the caller, and the `ReserveDestination` kept. This simplifies the API as well. There are very few cases where this would not be the case which may result in keys being burned.

  Those failure cases appear to be:
  `CommitTransaction` failing to get the transaction into the mempool
  Belt and suspenders check in `WalletModel::prepareTransaction`

  Alternative to https://github.com/bitcoin/bitcoin/pull/15796

ACKs for top commit:
  achow101:
    ACK e10e1e8db0 Reviewed the diff
  stevenroose:
    utACK e10e1e8db0
  meshcollider:
    utACK e10e1e8db0

Tree-SHA512: 78d047a00f39ab41cfa297052cc1e9c224d5f47d3d2299face650d71827635de077ac33fb4ab9f7dc6fc5a27f4a68415a1bc9ca33a3cb09a78f4f15b2a48411b
2019-07-17 19:45:55 +12:00
Russell Yanofsky
fa6f402bde
Call node->initError instead of InitError from GUI code
Avoids GUI code calling a node function, and having to live in the same process
as g_ui_signals and uiInterface global variables.
2019-07-11 19:39:55 -04:00
Wladimir J. van der Laan
735d6b57e7
Merge #16227: Refactor CWallet's inheritance chain
93ce4a0b6f Move WatchOnly stuff from SigningProvider to CWallet (Andrew Chow)
8f5b81e6ed Remove CCryptoKeyStore and move all of it's functionality into CWallet (Andrew Chow)
37a79a4fcc Move various SigningProviders to signingprovider.{cpp,h} (Andrew Chow)
16f8096e91 Move KeyOriginInfo to its own header file (Andrew Chow)
d9becff4e1 scripted-diff: rename CBasicKeyStore to FillableSigningProvider (Andrew Chow)
a913e3f2fb Move HaveKey static function from keystore to rpcwallet where it is used (Andrew Chow)
c7797ec655 Remove CKeyStore and squash into CBasicKeyStore (Andrew Chow)
1b699a5083 Add HaveKey and HaveCScript to SigningProvider (Andrew Chow)

Pull request description:

  This PR compresses the `CWallet` chain of inheritance from 5 classes to 3 classes. `CBasicKeyStore` is renamed to `FillableSigningProvider` and some parts of it (the watchonly parts) are moved into `CWallet`. `CKeyStore` and `CCrypoKeyStore` are completely removed. `CKeyStore`'s `Have*` functions are moved into `SigningProvider` and the `Add*` moved into `FillableSigningProvider`, thus allowing it to go away entirely. `CCryptoKeyStore`'s functionality is moved into `CWallet`. The new inheritance chain is:

  ```
  SigningProvider -> FillableSigningProvider -> CWallet
  ```

  `SigningProvider` now is the class the provides keys and scripts and indicates whether keys and scripts are present. `FillableSigningProvider` allows keys and scripts to be added to the signing provider via `Add*` functions. `CWallet` handles all of the watchonly stuff (`AddWatchOnly`, `HaveWatchOnly`, `RemoveWatchOnly` which were previously in `CKeyStore`) and key encryption (previously in `CCryptoKeyStore`).

  Implements the 2nd [prerequisite](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#cwallet-subclass-stack) from the wallet restructure.

ACKs for top commit:
  Sjors:
    re-ACK 93ce4a0; it keeps `EncryptSecret`, `DecryptSecret` and `DecryptKey` in `wallet/crypter.cpp`, but makes them not static. It improves alphabetical includes, reorders some function definitions, fixes commit message, brings back lost code comment.
  instagibbs:
    utACK 93ce4a0b6f

Tree-SHA512: 393dfd0623ad2dac38395eb89b862424318d6072f0b7083c92a0d207fd032c48b284f5f2cb13bc492f34557de350c5fee925da02e47daf011c5c6930a721b6d3
2019-07-11 22:42:39 +02:00
Gregory Sanders
e10e1e8db0 Restrict lifetime of ReserveDestination to CWallet::CreateTransaction 2019-07-10 11:38:37 -04:00
Wladimir J. van der Laan
8d1286014c
Merge #16237: Have the wallet give out destinations instead of keys
8e7f930828 Add GetNewChangeDestination for getting new change Destinations (Andrew Chow)
33d13edd2b Replace CReserveKey with ReserveDestinatoin (Andrew Chow)
172213be5b Add GetNewDestination to CWallet to fetch new destinations (Andrew Chow)

Pull request description:

  The wallet should give out destinations instead of keys. It should be the one that handles the conversion from key to destination and the setting of the label, not the caller. In order to do this, two new member functions are introduced `GetNewDestination()` and `GetNewChangeDestination()`. Additionally, `CReserveKey` is changed to be `ReserveDestination` and represents destinations whose keys can be returned to the keypool.

ACKs for top commit:
  instagibbs:
    re-utACK 8e7f930828
  sipa:
    ACK 8e7f930828. Concept ACK as this gives a much cleaner abstraction to work with, and light code review ACK.
  laanwj:
    ACK 8e7f930828

Tree-SHA512: 5be7051409232b71e0ef2c1fd1a3e76964ed2f5b14d47d06edc2ad3b3687abd0be2803a1adc45c0433aa2c3bed172e14f8a7e9f4a23bff70f86260b5a0497500
2019-07-10 11:45:55 +02:00
Andrew Chow
33d13edd2b Replace CReserveKey with ReserveDestinatoin
Instead of reserving keys, reserve destinations which are backed by keys
2019-07-09 16:43:10 -04:00
Andrew Chow
172213be5b Add GetNewDestination to CWallet to fetch new destinations
Instead of having the same multiple lines of code everywhere
that new destinations are fetched, introduce GetNewDestination as
a member function of CWallet which does the key fetching, label
setting, script generation, and destination generation.
2019-07-09 16:43:10 -04:00
Andrew Chow
8f5b81e6ed Remove CCryptoKeyStore and move all of it's functionality into CWallet
Instead of having a separate CCryptoKeyStore that handles the encryption
stuff, just roll it all into CWallet.
2019-07-09 16:20:18 -04:00
practicalswift
c4606b8432 Add Travis check for single parameter constructors not marked "explicit" 2019-06-26 16:57:14 +02:00
Andrew Chow
7c611e2000 Move ismine to wallet module 2019-06-19 18:06:30 -04:00
MarcoFalke
d0f81a96d9
Merge #16129: refactor: Remove unused includes
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift)
eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift)

Pull request description:

  Make reasoning about dependencies easier by not including unused dependencies.

  Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real.

  As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed:

  ```
  $ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \
        sed 's%^%src/%g' | xargs cat | wc -l
  51393
  ```

  Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-)

ACKs for commit 67f4e9:

Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
2019-06-06 16:41:40 +02:00
Wladimir J. van der Laan
5d37c1bde0
Merge #15976: refactor: move methods under CChainState (pt. 1)
403e677c9 refactoring: IsInitialBlockDownload -> CChainState (James O'Beirne)
3ccbc376d refactoring: FlushStateToDisk -> CChainState (James O'Beirne)
4d6688603 refactoring: introduce ChainstateActive() (James O'Beirne)
d7c97edee move-only: make the CChainState interface public (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):

  Parent PR: #15606
  Issue: #15605
  Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal

  ---

  This changeset starts moving functionality intimately related to CChainState into methods. Parameterizing these functions by a particular CChainState is necessary for the use of multiple chainstates simultaneously (e.g. for asynchronous background validation).

  In this change, we
  - make the CChainState interface public - since other units will start to invoke its methods directly,
  - introduce `::ChainstateActive()`, the CChainState equivalent for `::ChainActive()`,
  - and move `IsInitialBlockDownload()` and `FlushStateToDisk()` into methods on CChainState.

  Independent of assumeutxo, these changes better encapsulate chainstate behavior and allow easier use from a testing context.

  There are more methods that we'll move in the future, but they require other substantial changes (i.e. moving ownership of the `CCoinsView*` hierarchy into CChainState) so we'll save them for future PRs.

  ---

  The first move-only commit is most easily reviewed with `git diff ... --color-moved=dimmed_zebra`.

ACKs for commit 403e67:
  Empact:
    utACK 403e677c9e no need to address my nits herein
  Sjors:
    utACK 403e677
  ryanofsky:
    utACK 403e677c9e. Only change since previous review is removing global state comment as suggested.
  MarcoFalke:
    utACK 403e677c9e, though the diff still seems a bit bloated with some unnecessary changes in the second commit.
  promag:
    utACK 403e677 and rebased with current [master](c7cfd20a7).

Tree-SHA512: 6fcf260bb2dc201361170c0b4547405366f5f331fcc3a2bac29b24442814b7b244ca1b58aac5af716885f9a130c343b544590dff780da0bf835c7c5b3ccb2257
2019-06-05 11:56:23 +02:00
practicalswift
eca9767673 Make reasoning about dependencies easier by not including unused dependencies 2019-06-02 17:15:23 +02:00
practicalswift
9f85e9cb3d scripted-diff: Rename LockAnnotation to LockAssertion
-BEGIN VERIFY SCRIPT-
git grep -l LockAnnotation | xargs sed -i 's/LockAnnotation/LockAssertion/'
-END VERIFY SCRIPT-
2019-05-17 13:29:04 +02:00
MarcoFalke
f3d27d126b
Merge #16033: Hold cs_main when reading chainActive via getTipLocator(). Remove assumeLocked().
9402ef0739 Remove temporary method assumeLocked(). Remove LockingStateImpl. Remove redundant cs_main locks. (practicalswift)
593a8e8a2c wallet: Use chain.lock() instead of temporary chain.assumeLocked() (practicalswift)

Pull request description:

  Fixes #16028.

  Problem description:

  `LockAnnotation lock(::cs_main)` is a guarantee to the compiler thread analysis that `::cs_main` is locked (when it couldn't be determined otherwise).

  Despite being annotated with the locking guarantee ...

  65526fc866/src/interfaces/chain.cpp (L134-L138)

  ... `getTipLocator()` reads `chainActive` (via `::ChainActive()`) without holding `cs_main`.

  This can be verified by adding the following `AssertLockHeld(cs_main)`:

  ```
  $ git diff
  diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
  index 59623284d..9fc693a0f 100644
  --- a/src/interfaces/chain.cpp
  +++ b/src/interfaces/chain.cpp
  @@ -134,6 +134,7 @@ class LockImpl : public Chain::Lock
       CBlockLocator getTipLocator() override
       {
           LockAnnotation lock(::cs_main);
  +        AssertLockHeld(::cs_main);
           return ::ChainActive().GetLocator();
       }
       Optional<int> findLocatorFork(const CBlockLocator& locator) override
  $ make check
  ../build-aux/test-driver: line 107: 12881 Aborted                 "$@" > $log_file 2>&1
  FAIL: qt/test/test_bitcoin-qt
  ```

ACKs for commit 9402ef:
  MarcoFalke:
    utACK 9402ef0739
  ryanofsky:
    utACK 9402ef0739. Changes are consolidating commits and removing redundant lock2 cs_main calls

Tree-SHA512: 0a030bf0c07eb53194ecc246f973ef389dd42a0979f51932bf94bdf7e90c52473ae03be49718ee1629582b05dd8e0dc020b5a210318c93378ea4ace90c0f9f72
2019-05-17 07:17:41 -04:00
practicalswift
9402ef0739 Remove temporary method assumeLocked(). Remove LockingStateImpl. Remove redundant cs_main locks. 2019-05-16 21:43:22 +02:00
MarcoFalke
d5931f3676
Merge #15870: wallet: Only fail rescan when blocks have actually been pruned
fa7e311e16 [doc] rpcwallet: Only fail rescan when blocks have been pruned (MarcoFalke)
aaaa57c2aa scripted-diff: Bump copyright headers in wallet (MarcoFalke)
faf3729242 wallet: Only fail rescan when blocks have actually been pruned (MarcoFalke)

Pull request description:

  This brings the behaviour of the import* calls closer to importmulti. After this change, the difference between importmulti and the other import* calls is

  * that in importmulti you can "opt-out" of scanning early blocks by setting a later timestamp.
  * that in importmulti the wallet will successfully import the data, but fail to rescan. Whereas in the other calls, the wallet will abort before importing the data.

ACKs for commit fa7e31:
  promag:
    utACK fa7e311e16.
  jnewbery:
    utACK fa7e311e16

Tree-SHA512: a57d52ffea94b64e0eb9b5d3a7a63031325833908297dd14eb0c5251ffea3b2113b131003f1db4e9599e014369165a57f107a7150bb65e4c791e5fe742f33cb8
2019-05-16 11:18:27 -04:00
James O'Beirne
403e677c9e refactoring: IsInitialBlockDownload -> CChainState
We introduce CChainState.m_cached_finished_ibd because the static state it
replaces would've been shared across all CChainState instances.
2019-05-16 09:06:54 -04:00
MarcoFalke
fa3c651143
[refactor] interfaces: Add missing LockAnnotation for cs_main 2019-05-13 14:46:01 -04:00
MarcoFalke
faf3729242
wallet: Only fail rescan when blocks have actually been pruned 2019-05-06 14:03:56 -04:00
James O'Beirne
631940aab2 scripted-diff: replace chainActive -> ::ChainActive()
Though at the moment ChainActive() simply references `g_chainstate.m_chain`,
doing this change now clears the way for multiple chainstate usage and allows
us to script the diff.

-BEGIN VERIFY SCRIPT-
git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/\(::\)\{0,1\}chainActive/::ChainActive()/g'
-END VERIFY SCRIPT-
2019-05-03 15:02:54 -04:00
MarcoFalke
0936f35f65
Merge #15842: refactor: replace isPotentialtip/waitForNotifications by higher method
422677963a refactor: replace isPotentialtip/waitForNotifications by higher method (Antoine Riard)
edfe9438ca Add WITH_LOCK macro: run code while locking a mutex (Antoine Riard)

Pull request description:

  In Chain interface, instead of a isPotentialTip and a WaitForNotifications method, both used only once in CWallet::BlockUntilSyncedToCurrentChain, combine them in a higher WaitForNotificationsUpToTip method. Semantic should be unchanged, wallet wait for pending notifications to be processed unless block hash points to the current chain tip or a descendant.

ACKs for commit 422677:
  jnewbery:
    ACK 422677963a
  ryanofsky:
    utACK 422677963a. Only change is adding the cs_wallet lock annotation.

Tree-SHA512: 2834ff0218795ef607543fae822e5cce25d759c1a9cfcb1f896a4af03071faed5276fbe0966e0c6ed65dc0e88af161899c5b2ca358a2d24fe70969a550000bf2
2019-05-01 15:02:31 -04:00
MarcoFalke
3356799ee3
Merge #15778: [wallet] Move maxtxfee from node to wallet
5c759c73b2 [wallet] Move maxTxFee to wallet (John Newbery)

Pull request description:

  Closes #15355

  Moves the `-maxtxfee` from the node to the wallet. See discussion in issue for details.

  This is a cleanup. There is no change in behaviour.

  Completes #15620

ACKs for commit 5c759c:
  MarcoFalke:
    utACK 5c759c73b2
  ryanofsky:
    utACK 5c759c73b2. Changes since last review: updated commit message and an error message and method name.
  meshcollider:
    utACK 5c759c73b2

Tree-SHA512: 2f9b2729da3940a5cda994d3f3bc11ee1a52fcc1c5e9842ea0ea63e4eb0300e8416853046776311298bc449ba07554aa46f0f245ce28598a5b0bd7347c12e752
2019-04-27 09:28:54 -04:00
Antoine Riard
422677963a refactor: replace isPotentialtip/waitForNotifications by higher method
Add GUARDED_BY(cs_wallet) annotation to m_last_block_processed, given
that its now guarded by cs_wallet instead of cs_main
2019-04-23 13:53:46 -04:00
MarcoFalke
56376f3365
Merge #15670: refactor: combine Chain::findFirstBlockWithTime/findFirstBlockWithTimeAndHeight
765c0b364d refactor: combine Chain::findFirstBlockWithTime/findFirstBlockWithTimeAndHeight (Antoine Riard)

Pull request description:

  As suggested in #14711, pass height to CChain::FindEarliestAtLeast to
  simplify Chain interface by combining findFirstBlockWithTime and
  findFirstBlockWithTimeAndHeight into one

ACKs for commit 765c0b:
  jnewbery:
    utACK 765c0b364d. Nice work @ariard!
  ryanofsky:
    utACK 765c0b364d. Looks good, thanks for implementing the suggestion!

Tree-SHA512: 63f98252a93da95f08c0b6325ea98f717aa9ae4036d17eaa6edbec68e5ddd65672d66a6af267b80c36311fffa9b415a47308e95ea7718b300b685e23d4e9e6ec
2019-04-19 12:03:12 -04:00
John Newbery
5c759c73b2 [wallet] Move maxTxFee to wallet
This commit moves the maxtxfee setting to the wallet. There is only
one minor behavior change:

- an error message in feebumper now refers to -maxtxfee instead of
maxTxFee.
2019-04-18 11:34:42 -04:00
Gregory Sanders
0ea47ba7b3 generalize bumpfee to add inputs when needed 2019-04-11 07:21:49 -04:00
Russell Yanofsky
b874747b51 Remove access to node globals from wallet-linked code
Remove last few instances of accesses to node global variables from wallet
code. Also remove accesses to node globals from code in policy/policy.cpp that
isn't actually called by wallet code, but does get linked into wallet code.

This is the last change needed to allow bitcoin-wallet tool to be linked
without depending on libbitcoin_server.a, to ensure wallet code doesn't access
node global state and avoid bugs like
https://github.com/bitcoin/bitcoin/pull/15557#discussion_r267735431
2019-04-10 09:51:37 -04:00
Wladimir J. van der Laan
6a135fbe5b
Merge #15638: Move-only: Pull wallet code out of libbitcoin_server
4d074e84a2 [build] Move AnalyzePSBT from psbt.cpp to node/psbt.cpp (Russell Yanofsky)
fd509bd1f7 [docs] Document src subdirectories and different libraries (John Newbery)
9eaeb7fb8d [build] Move wallet load functions to wallet/load unit (John Newbery)
91a25d1e71 [build] Add several util units (John Newbery)
99517866b6 [build] Move several units into common libraries (John Newbery)
0509465542 [build] Move rpc rawtransaction util functions to rpc/rawtransaction_util.cpp (John Newbery)
1acc61f874 [build] Move rpc utility methods to rpc/util (John Newbery)
4a75c9d651 [build] Move policy settings to new src/policy/settings unit (John Newbery)
fdf8888b6f [build] Move CheckTransaction from lib_server to lib_consensus (John Newbery)

Pull request description:

  This is a move-only commit. No code is changing and the moves can be easily verified with:

  ```sh
  git log -p -n1 --color-moved=dimmed_zebra
  ```

  This commit moves functions and variables that wallet code depends on out of libbitcoin_server.a, so the bitcoin-wallet tool can be built without libbitcoin_server.a in #15639, and attempting to access server state from wallet code will result in link errors instead of silently broken code.

  List of moves:

  - `CheckTransaction` moves from `consensus/tx_verify.cpp` to `consensus/tx_check.cpp`
  - `urlDecode` moves from `httpserver.cpp` to `util/url.cpp`
  - `TransactionErrorString` moves from `node/transaction.cpp` to `util/error.cpp`
  - `StringForFeeReason` and `FeeModeFromString` move from `policy/fees.cpp` to `util/fees.cpp`
  - `incrementalRelayFee` `dustRelayFee` and `nBytesPerSigOp` move from `policy/policy.cpp` to `policy/settings.cpp`
  - `SignalsOptInRBF` moves from `policy/rbf.cpp` to `util/rbf.cpp`
  - `fIsBareMultisigStd` moves from `validation.cpp` to `policy/settings.cpp`
  - `ConstructTransaction` `TxInErrorToJSON` and `SignTransaction` move from `rpc/rawtransaction.cpp` to `rpc/rawtransaction_util.cpp`
  - `RPCTypeCheck` `RPCTypeCheckArgument` `RPCTypeCheckObj` `AmountFromValue` `ParseHashV``ParseHashO` `ParseHexV` `ParseHexO` `HelpExampleCli` and `HelpExampleRpc` move from `rpc/server.cpp` to `rpc/util.cpp`
  - `AmountHighWarn` and `AmountErrMsg` move from `ui_interface.cpp` to `util/error.cpp`
  - `FormatStateMessage` and `strMessageMagic` move from `validation.cpp` to `util/validation.cpp`
  - `VerifyWallets` `LoadWallets` `StartWallets` `FlushWallets` `StopWallets` and `UnloadWallets` move from `wallet/init.cpp` to `wallet/node.cpp`

ACKs for commit 4d074e:
  jnewbery:
    utACK 4d074e84a2 (checked by doing the rebase myself and verifying no difference between my branch and 4d074e84a2)

Tree-SHA512: 5e1604a9fb06475f2b96da0de0baa8330f4dda834dc20a0183ef11e1e4c27631d1d1bbb9abf0054efc03d56945fdf9920f63366b6a4f200f665b742a479ff75c
2019-04-10 15:51:37 +02:00
MarcoFalke
e3a0688f82
Merge #15659: [docs] fix findFork comment
c968780785 [docs] fix comment: the return value of findFork is _not_ an ancestor when the specified block is on the active chain (r8921039)

Pull request description:

  The return value of findFork is an ancestor of the specified block only when specified block is _not_ on the active chain. When it is on the active chain, the return value is the specified block itself, not an ancestor of it.

ACKs for commit c96878:
  promag:
    utACK c968780, however comment could be shorter.
  ryanofsky:
    utACK c968780785. Only change since last review is squash

Tree-SHA512: bb05d734059898784c4a59b5b0344719eb4dfb2d49a0f7f705fcb2eb630702e66be81c01299185faf0c219fa9f9aa64cbdf6d5f91e0b3dce0ff420909a454a18
2019-04-10 09:34:02 -04:00
John Newbery
9eaeb7fb8d [build] Move wallet load functions to wallet/load unit
Moves the following wallet load functions to a new wallet/load unit in
the libbitcoin_wallet library. All other functions in wallet/init remain
in libbitcoin_server:

- `VerifyWallets`
- `LoadWallets`
- `StartWallets`
- `FlushWallets`
- `StopWallets`
- `UnloadWallets`
2019-04-09 17:53:08 -04:00
John Newbery
4a75c9d651 [build] Move policy settings to new src/policy/settings unit
This moves the following policy settings functions and globals to a new
src/policy/settings unit in lib_server:

- `incrementalRelayFee`
- `dustRelayFee`
- `nBytesPerSigOp`
- `fIsBareMultisigStd`

These settings are only required by the node and should not be accessed
by other libraries.
2019-04-09 17:53:08 -04:00
John Newbery
52b760fc6a [wallet] Schedule tx rebroadcasts in wallet
Removes the now-unused Broadcast/ResendWalletTransactions interface from
validationinterface.

The wallet_resendwallettransactions.py needs a sleep added at the start
to make sure that the rebroadcast scheduler is warmed up before the next
block is mined.
2019-04-09 10:38:13 -04:00