0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-18 11:57:37 -05:00
Commit graph

1593 commits

Author SHA1 Message Date
fanquake
a7f4f1a09c
Merge bitcoin/bitcoin#28894: wallet: batch all individual spkms setup db writes in a single db txn
f053024273 wallet: batch external signer descriptor import (Sjors Provoost)
1f65241b73 wallet: descriptors setup, batch db operations (furszy)
3eb769f150 wallet: batch legacy spkm TopUp (furszy)
075aa44ceb wallet: batch descriptor spkm TopUp (furszy)
bb4554c81e bench: add benchmark for wallet creation procedure (furszy)

Pull request description:

  Work decoupled from #28574.

  Instead of performing multiple single write operations per spkm
  setup call, this PR batches them all within a single atomic db txn.

  Speeding up the process and preventing the wallet from entering
  an inconsistent state if any of the intermediate transactions fail
  (which shouldn't happen but.. if it does, it is better to not store
  any spkm rather than storing them partially).

  To compare the changes, added benchmark in the first commit.

ACKs for top commit:
  Sjors:
    re-utACK f053024273
  achow101:
    ACK f053024273
  BrandonOdiwuor:
    ACK f053024273
  theStack:
    Code-review ACK f053024273

Tree-SHA512: aead8548473e17d4d53e8e7039bbaf5e8bf2fe83f33b33f81cdedefe8a31b7003ceb6d5379b1bad1ca2692e909492009a21284ec8338eede078df3d19046ab5a
2023-12-08 11:25:01 +00:00
Sjors Provoost
f053024273
wallet: batch external signer descriptor import
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2023-11-21 23:07:00 -03:00
furszy
1f65241b73
wallet: descriptors setup, batch db operations
Instead of doing one db transaction per descriptor setup,
batch all descriptors' setup writes in a single db txn.

Speeding up the process and preventing the wallet from entering
an inconsistent state if any of the intermediate transactions
fail.
2023-11-21 23:01:42 -03:00
dergoegge
9e58c5bcd9 Use Txid in COutpoint 2023-11-21 13:15:44 +00:00
Ryan Ofsky
f06016d77d wallet: Add asserts to detect unset transaction height values
Also document GetTxDepthInMainChain preconditions better
2023-10-23 17:35:36 -04:00
Ryan Ofsky
262a78b133 wallet, refactor: Add CWalletTx::updateState function
No change in behavior, this just moves code which updates transaction state to
a new method so it can be used after offline processes such as wallet
migration.
2023-10-23 17:35:36 -04:00
Ryan Ofsky
d724bb5291
Merge bitcoin/bitcoin#28609: wallet: Reload watchonly and solvables wallets after migration
4814e4063e test: Check tx metadata is migrated to watchonly (Andrew Chow)
d616d30ea5 wallet: Reload watchonly and solvables wallets after migration (Andrew Chow)
118f2d7d70 wallet: Copy all tx metadata to watchonly wallet (Andrew Chow)
9af87cf348 test: Check that a failed wallet migration is cleaned up (Andrew Chow)

Pull request description:

  Some incomplete/incorrect state as a result of migration can be mitigated/cleaned up by simply restarting the migrated wallets. We already do this for a wallet when it is migrated, but we do not for the new watchonly and solvables wallets that may be created. This PR introduces this behavior, in addition to creating those wallets initially without an attached chain.

  While implementing this, I noticed that not all `CWalletTx` metadata was being copied over to the watchonly wallet and so some data, such as time received, was being lost. This PR fixes this as a side effect of not having a chain attached to the watchonly wallet. A test has also been added.

ACKs for top commit:
  ishaanam:
    light code review ACK 4814e4063e
  ryanofsky:
    Code review ACK 4814e4063e. Just implemented the suggested orderpos, copyfrom, and path set comments since last review
  furszy:
    ACK 4814e406

Tree-SHA512: 0b992430df9f452cb252c2212df8e876613f43564fcd1dc00c6c31fa497adb84dfff6b5ef597590f9b288c5f64cb455f108fcc9b6c9d1fe9eb2c39e7f2c12a89
2023-10-23 17:35:36 -04:00
Andrew Chow
d616d30ea5 wallet: Reload watchonly and solvables wallets after migration
When migrating, create the watchonly and solvables wallets without a
context. Then unload and reload them after migration completes, as we do
for the actual wallet.

There is also additional handling for a failed reload.
2023-10-19 18:06:43 -04:00
Andrew Chow
118f2d7d70 wallet: Copy all tx metadata to watchonly wallet
When moving a tx to the watchonly wallet during migration, make sure
that all of the CWalletTx data follows it.
2023-10-19 18:06:43 -04:00
Andrew Chow
fbcf1029a7
Merge bitcoin/bitcoin#28544: wallet: Add TxStateString function for debugging and logging
8a553c9409 wallet: Add TxStateString function for debugging and logging (Ryan Ofsky)

Pull request description:

  I found this useful while debugging silent conflict between #10102 and #27469 recently

ACKs for top commit:
  ishaanam:
    utACK 8a553c9409
  achow101:
    ACK 8a553c9409
  furszy:
    Code ACK 8a553c9

Tree-SHA512: 87965c66bcb59a21e7639878bb567e583a0e624735721ff7ad1104eed6bb9fba60607d0e3de7be3304232b3a55f48bab7039ea9c26b0e81963e59f9acd94f666
2023-10-17 15:28:05 -04:00
MarcoFalke
fa05a726c2
tidy: modernize-use-emplace 2023-10-12 11:27:19 +02:00
Andrew Chow
e7b0004b37
Merge bitcoin/bitcoin#27596: assumeutxo (2)
edbed31066 chainparams: add signet assumeutxo param at height 160_000 (Sjors Provoost)
b8cafe3871 chainparams: add testnet assumeutxo param at height 2_500_000 (Sjors Provoost)
99839bbfa7 doc: add note about confusing HaveTxsDownloaded name (James O'Beirne)
7ee46a755f contrib: add script to demo/test assumeutxo (James O'Beirne)
42cae39356 test: add feature_assumeutxo functional test (James O'Beirne)
0f64bac603 rpc: add getchainstates (James O'Beirne)
bb05857794 refuse to activate a UTXO snapshot if mempool not empty (James O'Beirne)
ce585a9a15 rpc: add loadtxoutset (James O'Beirne)
62ac519e71 validation: do not activate snapshot if behind active chain (James O'Beirne)
9511fb3616 validation: assumeutxo: swap m_mempool on snapshot activation (James O'Beirne)
7fcd21544a blockstorage: segment normal/assumedvalid blockfiles (James O'Beirne)
4c3b8ca35c validation: populate nChainTx value for assumedvalid chainstates (James O'Beirne)
49ef778158 test: adjust chainstate tests to use recognized snapshot base (James O'Beirne)
1019c39982 validation: pruning for multiple chainstates (James O'Beirne)
373cf91531 validation: indexing changes for assumeutxo (James O'Beirne)
1fffdd76a1 net_processing: validationinterface: ignore some events for bg chain (James O'Beirne)
fbe0a7d7ca wallet: validationinterface: only handle active chain notifications (James O'Beirne)
f073917a9e validationinterface: only send zmq notifications for active (James O'Beirne)
4d8f4dcb45 validation: pass ChainstateRole for validationinterface calls (James O'Beirne)
1e59acdf17 validation: only call UpdatedBlockTip for active chainstate (James O'Beirne)
c6af23c517 validation: add ChainstateRole (James O'Beirne)
9f2318c76c validation: MaybeRebalanceCaches when chain leaves IBD (James O'Beirne)
434495a8c1 chainparams: add blockhash to AssumeutxoData (James O'Beirne)
c711ca186f assumeutxo: remove snapshot during -reindex{-chainstate} (James O'Beirne)
c93ef43e4f bugfix: correct is_snapshot_cs in VerifyDB (James O'Beirne)
b73d3bbd23 net_processing: Request assumeutxo background chain blocks (Suhas Daftuar)

Pull request description:

  - Background and FAQ: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
  - Prior progress/project: https://github.com/bitcoin/bitcoin/projects/11
  - Replaces https://github.com/bitcoin/bitcoin/pull/15606, which was closed due to Github slowness. Original description and commentary can be found there.

  ---

  This changeset finishes the first phase of the assumeutxo project. It makes UTXO snapshots loadable via RPC (`loadtxoutset`) and adds `assumeutxo` parameters to chainparams. It contains all the remaining changes necessary to both use an assumedvalid snapshot chainstate and do a full validation sync in the background.

  This may look like a lot to review, but note that
  - ~200 lines are a (non-essential) demo shell script
  - Many lines are functional test, documentation, and relatively dilute RPC code.

  So it shouldn't be as burdensome to review as the linecount might suggest.

  - **P2P**: minor changes are made to `init.cpp` and `net_processing.cpp` to make simultaneous IBD across multiple chainstates work.
  - **Pruning**: implement correct pruning behavior when using a background chainstate
  - **Blockfile separation**: to prevent "fragmentation" in blockfile storage, have background chainstates use separate blockfiles from active snapshot chainstates to avoid interleaving heights and impairing pruning.
  - **Indexing**: some `CValidationInterface` events are given with an additional parameter, ChainstateRole, and all indexers ignore events from ChainstateRole::ASSUMEDVALID so that indexation only happens sequentially.
  - Have `-reindex` properly wipe snapshot chainstates.
  - **RPC**: introduce RPC commands `loadtxoutset` and (hidden) `getchainstates`.
  - **Release docs & first assumeutxo commitment**: add notes and a particular assumeutxo hash value for first AU-enabled release.
    - This will complete the project and allow use of UTXO snapshots for faster node bootstrap.

  The next phase, if it were to be pursued, would be coming up with a way to distribute the UTXO snapshots over the P2P network.

  ---

  ### UTXO snapshots

  Create your own with `./contrib/devtools/utxo_snapshot.sh`, e.g.
  ```shell
  ./contrib/devtools/utxo_snapshot.sh 788000 utxo.dat ./src/bitcoin-cli -datadir=$(pwd)/testdata`)
  ```
  or use the pre-generated ones listed below.

  - Testnet: **2'500'000** (Sjors):
    - torrent: `magnet:?xt=urn:btih:511e09f4bf853aefab00de5c070b1e031f0ecbe9&dn=utxo-testnet-2500000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
    - sha256: `79db4b025448cc0ac388d8589a28eab02de53055d181e34eb47391717aa16388`
  - Signet: **160'000** (Sjors):
    - torrent: `magnet:?xt=urn:btih:9da986cb27b3980ea7fd06b21e199b148d486880&dn=utxo-signet-160000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
    - sha256: `eeeca845385ba91e84ef58c09d38f98f246a24feadaad57fe1e5874f3f92ef8c`
  - Mainnet: **800'000** (Sjors):
    - Note: this needs the following commit cherry-picked in: 24deb2022b
    - torrent: `magnet:?xt=urn:btih:50ee955bef37f5ec3e5b0df4cf0288af3d715a2e&dn=utxo-800000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`

  ### Testing

  #### For fun (~5min)

  If you want to do a quick test, you can run `./contrib/devtools/test_utxo_snapshots.sh` and follow the instructions. This is mostly obviated by the functional tests, though.

  #### For real (longer)

  If you'd like to experience a real usage of assumeutxo, you can do that too.
  I've cut a new snapshot at height 788'000 (http://img.jameso.be/utxo-788000.dat - but you can do it yourself with `./contrib/devtools/utxo_snapshot.sh` if you want). Download that, and then create a datadir for testing:
  ```sh
  $ cd ~/src/bitcoin  # or whatever

  # get the snapshot
  $ curl http://img.jameso.be/utxo-788000.dat > utxo-788000.dat

  # you'll want to do this if you like copy/pasting
  $ export AU_DATADIR=/home/${USER}/au-test # or wherever

  $ mkdir ${AU_DATADIR}
  $ vim ${AU_DATADIR}/bitcoin.conf

  dbcache=8000  # or, you know, something high
  blockfilterindex=1
  coinstatsindex=1
  prune=3000
  logthreadnames=1
  ```
  Obtain this branch, build it, and then start bitcoind:
  ```sh
  $ git remote add jamesob https://github.com/jamesob/bitcoin
  $ git fetch jamesob assumeutxo
  $ git checkout jamesob/assumeutxo

  $ ./configure $conf_args && make  # (whatever you like to do here)

  # start 'er up and watch the logs
  $ ./src/bitcoind -datadir=${AU_DATADIR}
  ```
  Then, in some other window, load the snapshot
  ```sh
  $ ./src/bitcoin-cli -datadir=${AU_DATADIR} loadtxoutset $(pwd)/utxo-788000.dat
  ```

  You'll see some log messages about headers retrieval and waiting to see the snapshot in the headers chain. Once you get the full headers chain, you'll spend a decent amount of time (~10min) loading the snapshot, checking it, and flushing it to disk. After all that happens, you should be syncing to tip in pretty short order, and you'll see the occasional `[background validation]` log message go by.

  In yet another window, you can check out chainstate status with
  ```sh
  $ ./src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates
  ```
  as well as usual favorites like `getblockchaininfo`.

ACKs for top commit:
  achow101:
    ACK edbed31066

Tree-SHA512: 6086fb9a38dc7df85fedc76b30084dd8154617a2a91e89a84fb41326d34ef8e7d7ea593107afba01369093bf8cc91770621d98f0ea42a5b3b99db868d2f14dc2
2023-10-02 17:09:44 -04:00
James O'Beirne
fbe0a7d7ca wallet: validationinterface: only handle active chain notifications 2023-09-30 06:38:47 -04:00
James O'Beirne
4d8f4dcb45 validation: pass ChainstateRole for validationinterface calls
This allows consumers to decide how to handle events from background or
assumedvalid chainstates.
2023-09-30 06:38:47 -04:00
Ryan Ofsky
8a553c9409 wallet: Add TxStateString function for debugging and logging
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2023-09-27 13:47:38 -04:00
Andrew Chow
4660fc82a1 wallet: Check last block and conflict height are valid in MarkConflicted
MarkConflicted calculates conflict confirmations incorrectly when both
the last block processed height and the conflicting height are negative
(i.e. uninitialized). If either are negative, we should not be marking
conflicts and should exit early.
2023-09-26 21:28:20 -04:00
Hennadii Stepanov
1d4846a844
Merge bitcoin-core/gui#738: Add menu option to migrate a wallet
48aae2cffe gui: Add File > Migrate Wallet (Andrew Chow)
577be889cd gui: Optionally return passphrase after unlocking (Andrew Chow)
5b3a85b4c6 interfaces, wallet: Expose migrate wallet (Andrew Chow)

Pull request description:

  GUI users need to be able to migrate wallets without going to the RPC console.

ACKs for top commit:
  jarolrod:
    ACK 48aae2cffe
  pablomartin4btc:
    tACK 48aae2cffe
  hebasto:
    ACK 48aae2cffe

Tree-SHA512: 2d02b1e85e7d6cfbf503f417f150cdaa0c63822942e9a6fe28c0ad3e7f40a957bb01a375c909a60432dc600e84574881aa446c7ec983b56f0bb23f07ef15de54
2023-09-20 22:32:31 +01:00
Andrew Chow
abe4fedab7
Merge bitcoin/bitcoin#28125: wallet: bugfix, disallow migration of invalid scripts
8e7e3e6149 test: wallet, verify migration doesn't crash for an invalid script (furszy)
1de8a2372a wallet: disallow migration of invalid or not-watched scripts (furszy)

Pull request description:

  Fixing #28057.

  The legacy wallet allows to import any raw script (#28126), without
  checking if it was valid or not. Appending it to the watch-only set.

  This causes a crash in the migration process because we are only
  expecting to find valid scripts inside the legacy spkm.

  These stored scripts internally map to `ISMINE_NO` (same as if they
  weren't stored at all..).

  So we need to check for these special case, and take into account that
  the legacy spkm could be storing invalid not watched scripts.

  Which, in code words, means `IsMineInner()` returning
  `IsMineResult::INVALID` for them.

  Note:
  To verify this, can run the test commit on top of master.
  `wallet_migration.py` will crash without the bugfix commit.

ACKs for top commit:
  achow101:
    ACK 8e7e3e6149

Tree-SHA512: c2070e8ba78037a8f573b05bf6caa672803188f05429adf5b93f9fc1493faedadecdf018dee9ead27c656710558c849c5da8ca5f6f3bc9c23b3c4275d2fb50c7
2023-09-19 13:10:57 -04:00
Andrew Chow
ad0c469d98 wallet: Use CTxDestination in CRecipient rather than scriptPubKey 2023-09-12 12:14:31 -04:00
Andrew Chow
d2ccca253f
Merge bitcoin/bitcoin#26567: Wallet: estimate the size of signed inputs using descriptors
10546a569c wallet: accurately account for the size of the witness stack (Antoine Poinsot)
9b7ec393b8 wallet: use descriptor satisfaction size to estimate inputs size (Antoine Poinsot)
8d870a9873 script/signingprovider: introduce a MultiSigningProvider (Antoine Poinsot)
fa7c46b503 descriptor: introduce a method to get the satisfaction size (Antoine Poinsot)
bdba7667d2 miniscript: introduce a helper to get the maximum witness size (Antoine Poinsot)
4ab382c2cd miniscript: make GetStackSize independent of P2WSH context (Antoine Poinsot)

Pull request description:

  The wallet currently estimates the size of a signed input by doing a dry run of the signing logic. This is unnecessary since all outputs we can sign for can be represented by a descriptor, and we can derive the size of a satisfaction ("signature") directly from the descriptor itself.
  In addition, the current approach does not generalize well: dry runs of the signing logic are only possible for the most basic scripts. See for instance the discussion in #24149 around that.

  This introduces a method to get the maximum size of a satisfaction from a descriptor, and makes the wallet use that instead of the dry-run.

ACKs for top commit:
  sipa:
    utACK 10546a569c
  achow101:
    re-ACK 10546a569c

Tree-SHA512: 43ed1529fbd30af709d903c8c5063235e8c6a03b500bc8f144273d6184e23a53edf0fea9ef898ed57d8a40d73208b5d935cc73b94a24fad3ad3c63b3b2027174
2023-09-06 13:31:03 -04:00
Antoine Poinsot
9b7ec393b8
wallet: use descriptor satisfaction size to estimate inputs size
Instead of using the dummysigner to compute a placeholder satisfaction,
infer a descriptor on the scriptPubKey of the coin being spent and use
the estimation of the satisfaction size given by the descriptor
directly.

Note this (almost, see next paragraph) exactly conserves the previous
behaviour. For instance CalculateMaximumSignedInputSize was previously
assuming the input to be spent in a transaction that spends at least one
Segwit coin, since it was always accounting for the serialization of the
number of witness elements.

In this commit we use a placeholder for the size of the serialization of
the witness stack size (1 byte). Since the logic in this commit is
already tricky enough to review, and that it is only a very tiny
approximation not observable through the existing tests, it is addressed
in the next commit.
2023-08-25 12:40:12 +02:00
fanquake
00fc7cdc25
Merge bitcoin/bitcoin#28200: refactor: Remove unused includes from wallet.cpp
fa6286891f Remove unused includes from wallet.cpp (MarcoFalke)
fa8fdbe229 Remove unused includes from blockfilter.h (MarcoFalke)
fad8c36aa9 move-only: Create src/kernel/mempool_removal_reason.h (MarcoFalke)
fa57608800 Remove unused includes from txmempool.h (MarcoFalke)

Pull request description:

  This makes compilation of wallet.cpp use a few % less memory and time, locally.

  Created in the context of https://github.com/bitcoin/bitcoin/issues/28109, but I don't think it is enough to actually fix this problem.

ACKs for top commit:
  hebasto:
    ACK fa6286891f, I have reviewed the code and it looks OK.

Tree-SHA512: 06f1120af2a8ef3368dbd9ae747acda88ace2507bd261bcc10341d476a0b3d71c8485377ea6c108b47df3e4c13b7f75a15f486bafa6a8466303168dde16ebbc8
2023-08-22 10:34:10 +01:00
fanquake
7bf078f2b7
Merge bitcoin/bitcoin#28237: refactor: Enforce C-str fmt strings in WalletLogPrintf()
fa60fa3b0c bitcoin-tidy: Apply bitcoin-unterminated-logprintf to spkm as well (MarcoFalke)
faa11434fe refactor: Enable all clang-tidy plugin bitcoin tests (MarcoFalke)
fa6dc57760 refactor: Enforce C-str fmt strings in WalletLogPrintf() (MarcoFalke)
fa244f3321 doc: Fix bitcoin-unterminated-logprintf tidy comments (MarcoFalke)

Pull request description:

  All fmt functions only accept a raw C-string as argument.

  There should never be a need to pass a format string that is not a compile-time string literal, so disallow it in `WalletLogPrintf()` to avoid accidentally introducing it.

  Apart from consistency, this also fixes the clang-tidy plugin bug https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1286821141.

ACKs for top commit:
  theuni:
    ACK fa60fa3b0c

Tree-SHA512: fa6f4984c50f9b34e850bdfee7236706af586e512d866cc869cf0cdfaf9aa707029c210ca72d91f85e75fcbd8efe0d77084701de8c3d2004abfd7e46b6fa9072
2023-08-18 11:38:38 +01:00
MarcoFalke
fa6286891f
Remove unused includes from wallet.cpp
This removes unused includes, such as undo.h or txmempool.h from
wallet.cpp.

Also, add missing ones, according to IWYU.
2023-08-18 08:20:43 +02:00
Andrew Chow
91d924ede1 Rename script/standard.{cpp/h} to script/solver.{cpp/h}
Since script/standard only contains things that are used by the Solver
and its callers, rename the files to script/solver.
2023-08-14 17:39:49 -04:00
Andrew Chow
f3c9078b4c Clean up things that include script/standard.h
Remove standard.h from files that don't use anything in it, and include
it in files that do.
2023-08-14 17:38:27 -04:00
furszy
1de8a2372a
wallet: disallow migration of invalid or not-watched scripts
The legacy wallet allowed to import any raw script, without checking if
it was valid or not. Appending it to the watch-only set.

This causes a crash in the migration process because we are only
expecting to find valid scripts inside the legacy spkm.

These stored scripts internally map to `ISMINE_NO` (same as if they
weren't stored at all..).

So we need to check for these special case, and take into account that
the legacy spkm could be storing invalid not watched scripts.

Which, in code words, means IsMineInner() returning IsMineResult::INVALID
for them.
2023-08-10 10:35:29 -03:00
MarcoFalke
fa6dc57760
refactor: Enforce C-str fmt strings in WalletLogPrintf() 2023-08-08 10:55:11 +02:00
fanquake
0a1029aa29
lint: remove /* Continued */ markers from codebase 2023-08-03 17:52:24 +01:00
furszy
a277f8357a
wallet: migration bugfix, persist empty labels
addressbook records with no associated label could be
treated as change. And we don't want that for external
addresses.
2023-07-06 16:11:55 -03:00
furszy
1b64f6498c
wallet: migration bugfix, clone 'send' record label to all wallets 2023-07-06 13:48:11 -03:00
glozow
ecf3baffc0
Merge bitcoin/bitcoin#27869: wallet: Give deprecation warning when loading a legacy wallet
8fbb6e99bf wallet: Give deprecation warning when loading a legacy wallet (Andrew Chow)

Pull request description:

  Next step in legacy wallet deprecation.

ACKs for top commit:
  S3RK:
    reACK 8fbb6e99bf
  jonatack:
    re-ACK 8fbb6e99bf

Tree-SHA512: 902984b09452926cf199f06e5fb56e4985325cdd5e0dcc829992158488f42d5fbc33e9a30a29303feac24c8315193e8d31712022e2a0503abd6b67169a0027f4
2023-07-06 10:47:41 +01:00
Andrew Chow
7952a5934a
Merge bitcoin/bitcoin#27927: util: Allow std::byte and char Span serialization
fa38d86235 Use only Span{} constructor for byte-like types where possible (MarcoFalke)
fa257bc831 util: Allow std::byte and char Span serialization (MarcoFalke)

Pull request description:

  Seems odd to require developers to cast all byte-like spans passed to serialization to `unsigned char`-spans. Fix that by passing and accepting byte-like spans as-is. Finally, add tests and update the code to use just `Span` where possible.

ACKs for top commit:
  sipa:
    utACK fa38d86235
  achow101:
    ACK fa38d86235
  ryanofsky:
    Code review ACK fa38d86235. This looks great. The second commit really removes a lot of boilerplate and shows why the first commit is useful.

Tree-SHA512: 788592d9ff515c3ebe73d48f9ecbb8d239f5b985af86f09974e508cafb0ca6d73a959350295246b4dfb496149bc56330a0b5d659fc434ba6723dbaba0b7a49e5
2023-06-28 15:12:12 -04:00
Ryan Ofsky
d9c7c2fd3e
Merge bitcoin/bitcoin#24914: wallet: Load database records in a particular order
3c83b1d884 doc: Add release note for wallet loading changes (Andrew Chow)
2636844f53 walletdb: Remove loading code where the database is iterated (Andrew Chow)
cd211b3b99 walletdb: refactor decryption key loading (Andrew Chow)
31c033e5ca walletdb: refactor defaultkey and wkey loading (Andrew Chow)
c978c6d39c walletdb: refactor active spkm loading (Andrew Chow)
6fabb7fc99 walletdb: refactor tx loading (Andrew Chow)
abcc13dd24 walletdb: refactor address book loading (Andrew Chow)
405b4d9147 walletdb: Refactor descriptor wallet records loading (Andrew Chow)
30ab11c497 walletdb: Refactor legacy wallet record loading into its own function (Andrew Chow)
9e077d9b42 salvage: Remove use of ReadKeyValue in salvage (Andrew Chow)
ad779e9ece walletdb: Refactor hd chain loading to its own function (Andrew Chow)
72c2a54ebb walletdb: Refactor encryption key loading to its own function (Andrew Chow)
3ccde4599b walletdb: Refactor crypted key loading to its own function (Andrew Chow)
7be10adff3 walletdb: Refactor key reading and loading to its own function (Andrew Chow)
52932c5adb walletdb: Refactor wallet flags loading (Andrew Chow)
01b35b55a1 walletdb: Refactor minversion loading (Andrew Chow)

Pull request description:

  Currently when we load a wallet, we just iterate through all of the records in the database and add them completely statelessly. However we have some records which do rely on other records being loaded before they are. To deal with this, we use `CWalletScanState` to hold things temporarily until all of the records have been read and then we load the stateful things.

  However this can be slow, and with some future improvements, can cause some pretty drastic slowdowns to retain this pattern. So this PR changes the way we load records by choosing to load the records in a particular order. This lets us do things such as loading a descriptor record, then finding and loading that descriptor's cache and key records. In the future, this will also let us use `IsMine` when loading transactions as then `IsMine` will actually be working as we now always load keys and descriptors before transactions.

  In order to get records of a specific type, this PR includes some refactors to how we do database cursors. Functionality is also added to retrieve a cursor that will give us records beginning with a specified prefix.

  Lastly, one thing that iterating the entire database let us do was to find unknown records. However even if unknown records were found, we would not do anything with this information except output a number in a log line. With this PR, we would no longer be aware of any unknown records. This does not change functionality as we don't do anything with unknown records, and having unknown records is not an error. Now we would just not be aware that unknown records even exist.

ACKs for top commit:
  MarcoFalke:
    re-ACK 3c83b1d884 🍤
  furszy:
    reACK 3c83b1d8
  ryanofsky:
    Code review ACK 3c83b1d884. Just Marco's suggested error handling fixes since last review

Tree-SHA512: 15fa56332fb2ce4371db468a0c674ee7a3a8889c8cee9f428d06a7d1385d17a9bf54bcb0ba885c87736841fe6a5c934594bcf4476a473616510ee47862ef30b4
2023-06-27 19:03:15 -04:00
Andrew Chow
abcc13dd24 walletdb: refactor address book loading
Instead of loading address book records as we come across them when
iterating the database, load them explicitly

Due to exception handling changes, deserialization errors are now
treated as critical.

The error message for noncritical errors has also been updated to
reflect that there's more data that could be missing than just address
book entries and tx data.
2023-06-27 11:04:18 -04:00
MarcoFalke
fa38d86235
Use only Span{} constructor for byte-like types where possible
This removes bloat that is not needed.
2023-06-27 10:13:37 +02:00
Andrew Chow
8fbb6e99bf wallet: Give deprecation warning when loading a legacy wallet 2023-06-23 16:37:22 -04:00
Ryan Ofsky
ee22ca59a2
Merge bitcoin/bitcoin#26740: wallet: Migrate wallets that are not in a wallet dir
a1e653828b test: Add test for migrating default wallet and plain file wallet (Andrew Chow)
bdbe3fd76b wallet: Generated migrated wallet's path from walletdir and name (Andrew Chow)

Pull request description:

  This PR fixes an assertion error that is hit during the setup of the new database during migration of a wallet that was not contained in a wallet dir. Also added a test for this case as well as one for migrating the default wallet.

ACKs for top commit:
  ryanofsky:
    Code review ACK a1e653828b
  furszy:
    ACK a1e65382

Tree-SHA512: 96b218c0de8567d8650ec96e1bf58b0f8ca4c4726f5efc6362453979b56b9d569baea0bb09befb3a5aed8d16d29bf75ed5cd8ffc432bbd4cbcad3ac5574bc479
2023-06-20 16:10:44 -04:00
Andrew Chow
5b3a85b4c6 interfaces, wallet: Expose migrate wallet 2023-06-13 19:00:58 -04:00
Andrew Chow
bdbe3fd76b wallet: Generated migrated wallet's path from walletdir and name
Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>
2023-06-12 15:13:50 -04:00
Ryan Ofsky
153a6882f4
Merge bitcoin/bitcoin#27576: kernel: Remove args, settings, chainparams, chainparamsbase from kernel library
db77f87c63 scripted-diff: move settings to common namespace (TheCharlatan)
c27e4bdc35 move-only: Move settings to the common library (TheCharlatan)
c2dae5d7d8 kernel: Remove chainparams, chainparamsbase, args, settings from kernel library (TheCharlatan)
05870b1c92 refactor: Remove gArgs access from validation.cpp (TheCharlatan)
8789b11114 refactor: Add path argument to FindSnapshotChainstateDir (TheCharlatan)
ef95be334f refactor: Add stop_at_height option in ChainstateManager (TheCharlatan)

Pull request description:

  This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/27587 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel".

  ---

  This completes the removal of the node's chainparams, chainparamsbase, args and settings files and their respective classes from the kernel library. This is the last pull request in a long series working towards decoupling the `ArgsManager` and the `gArgs` global from kernel code. These prior pull requests are: https://github.com/bitcoin/bitcoin/pull/26177 https://github.com/bitcoin/bitcoin/pull/27125 https://github.com/bitcoin/bitcoin/pull/25527 https://github.com/bitcoin/bitcoin/pull/25487 https://github.com/bitcoin/bitcoin/pull/25290

ACKs for top commit:
  MarcoFalke:
    lgtm ACK db77f87c63 🍄
  hebasto:
    ACK db77f87c63, I have reviewed the code and it looks OK.
  ryanofsky:
    Code review ACK db77f87c63. Looks great!

Tree-SHA512: cbfbd705d056f2f10f16810d4f869eb152362fff2c5ddae5e1ac6785deae095588e52ad48b29d921962b085e51de1e0ecab6e50f46149ffe3c16250608a2c93a
2023-06-09 14:58:49 -04:00
Ryan Ofsky
5cd0717a54 streams: Drop confusing DataStream::Serialize method and << operator
DataStream Serialize method has surprising behavior because it just serializes
raw bytes without a length prefix. When you serialize a string or vector, a
length prefix is serialized before the raw object contents so the object can be
unambiguously deserialized later. But DataStreams don't support deserializing
at all and just dump the raw bytes.

Having this inconsistency is not necessary and could be confusing (see
https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1212315030) so this
PR just drops the DataStream::Serialize method.
2023-06-01 10:27:33 -04:00
TheCharlatan
db77f87c63
scripted-diff: move settings to common namespace
-BEGIN VERIFY SCRIPT-
sed -i 's/namespace\ util/namespace\ common/g' src/common/settings.cpp src/common/settings.h
sed -i 's/util\:\:GetSetting/common\:\:GetSetting/g' $( git grep -l 'util\:\:GetSetting')
sed -i 's/util\:\:Setting/common\:\:Setting/g' $( git grep -l 'util\:\:Setting')
sed -i 's/util\:\:FindKey/common\:\:FindKey/g' $( git grep -l 'util\:\:FindKey')
sed -i 's/util\:\:ReadSettings/common\:\:ReadSettings/g' $( git grep -l 'util\:\:ReadSettings')
sed -i 's/util\:\:WriteSettings/common\:\:WriteSettings/g' $( git grep -l 'util\:\:WriteSettings')
-END VERIFY SCRIPT-
2023-05-30 17:26:51 +02:00
fanquake
dfe658009d
Merge bitcoin/bitcoin#27759: Fix #includes in src/wallet
1f97572b9c Fix `#include`s in `src/wallet` (Hennadii Stepanov)

Pull request description:

  This PR is a minimum required changes to fix https://github.com/bitcoin/bitcoin/pull/27571#discussion_r1195497290.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 1f97572b9c

Tree-SHA512: de885210076d23f3394c42ca50e6ae2470c0ae6523399a2fa3ebb7c06383bdacef9c26166fa19747200396bed796c8772165e24416eb30ed8edd024e3394b2fe
2023-05-29 16:33:14 +01:00
Andrew Chow
7d33ae755d
Merge bitcoin/bitcoin#27145: wallet: when a block is disconnected, update transactions that are no longer conflicted
89df7987c2 Add wallets_conflicts (Antoine Riard)
dced203162 wallet, tests: mark unconflicted txs as inactive (ishaanam)
096487c4dc wallet: introduce generic recursive tx state updating function (ishaanam)

Pull request description:

  This implements a fix for #7315. Previously when a block was disconnected any transactions that were conflicting with transactions mined in that block were not updated to be marked as inactive. The fix implemented here is described on the [Bitcoin DevWiki](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking#idea-refresh-conflicted). A test which tested the previous behavior has also been updated.

  Second attempt at #17543

ACKs for top commit:
  achow101:
    ACK 89df7987c2
  rajarshimaitra:
    tACK 89df7987c2.
  glozow:
    ACK 89df7987c2
  furszy:
    Tested ACK 89df7987

Tree-SHA512: 3133b151477e8818302fac29e96de30cd64c09a8fe3a7612074a34ba1a332e69148162e6cb3f1074d9d9c9bab5ef9996967d325d8c4c99ba42b5fe3b66a60546
2023-05-27 13:07:09 -04:00
Hennadii Stepanov
1f97572b9c
Fix #includes in src/wallet 2023-05-25 15:52:08 +01:00
furszy
82bb7831fa
wallet: skip block scan if block was created before wallet birthday
To avoid wasting processing power, we can skip blocks that occurred
before the wallet's creation time,  since these blocks are guaranteed
not to contain any relevant wallet data.

This has direct implications (an speed improvement) on the underlying
blockchain synchronization process as well.

The reason is that the validation interface queue is limited to
10 tasks per time. This means that no more than 10 blocks can be
waiting for the wallet(s) to be processed while we are synchronizing
the chain (activating the best chain to be more precise).
Which can be a bottleneck if blocks arrive and are processed faster
from the network than what they are  processed by the wallet(s).
2023-05-25 10:45:38 -03:00
furszy
a082434d12
refactor: single method to append new spkm to the wallet 2023-05-25 10:38:20 -03:00
ishaanam
dced203162 wallet, tests: mark unconflicted txs as inactive
In `blockDisconnected`, for each transaction in the block, look
for any wallet transactions spending the same inputs. If any of
these transactions were marked conflicted, they are now marked as
inactive.

Co-authored-by: ariard <antoine.riard@gmail.com>
2023-05-14 10:45:21 -04:00
MarcoFalke
fa422aeec2
scripted-diff: Use UniValue::find_value method
-BEGIN VERIFY SCRIPT-
 sed --regexp-extended -i 's/find_value\(([^ ,]+), /\1.find_value(/g' $(git grep -l find_value)
-END VERIFY SCRIPT-
2023-05-09 18:47:14 +02:00