0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-21 12:22:50 -05:00
Commit graph

4371 commits

Author SHA1 Message Date
Ayush Singh
d7290d662f fuzz: wallet, add target for Crypter 2024-05-02 01:14:15 +05:30
Ava Chow
2d3056751b
Merge bitcoin/bitcoin#29906: Disable util::Result copying and assignment
6a8b2befea refactor: Avoid copying util::Result values (Ryan Ofsky)
834f65e824 refactor: Drop util::Result operator= (Ryan Ofsky)

Pull request description:

  This PR just contains the first two commits of #25665.

  It disables copying of `util::Result` objects because unnecessary copies are inefficient and not possible after #25665, which makes `util::Result` object move-only.

  It disables the assignment operator and replaces it with an `Update()` method, because #25665 adds more information to `util::Result` objects (warning and error messages and failure values) and having an assignment operator that overwrites data instead of merging it would make it easy to accidentally erase existing information while trying to assign new information.

ACKs for top commit:
  stickies-v:
    re-ACK 6a8b2befea
  achow101:
    ACK 6a8b2befea
  furszy:
    re-ACK 6a8b2befea

Tree-SHA512: 3f21af9031d50d6c68cca69133de03080f69b1ddcf8b140bdeb762069f14645209b2586037236d15b6ebd8973af0fbefd7e83144aeb7b84078a4cb4df812f984
2024-04-30 12:19:03 -04:00
Ryan Ofsky
19865a8350
Merge bitcoin/bitcoin#29277: RPC: access RPC arguments by name
30a6c99935 rpc: access some args by name (stickies-v)
bbb31269bf rpc: add named arg helper (stickies-v)
13525e0c24 rpc: add arg helper unit test (stickies-v)

Pull request description:

  Adds string overloads for the `RPCHelpMan::Arg` and `RPCHelpMan::MaybeArg` helpers to be able to access RPC arguments by name instead of index number. Especially in RPCs with a large number of parameters, this can be quite helpful.

  Example usage:
  ```cpp
  const auto action{self.Arg<std::string>("action")};
  ```

  Most of the LoC is adding test coverage and documentation updates. No behaviour change.

  An alternative approach to #27788 with significantly less overhaul.

ACKs for top commit:
  fjahr:
    Code review ACK 30a6c99935
  maflcko:
    ACK 30a6c99935 🥑
  ryanofsky:
    Code review ACK 30a6c99935. Nice change! Implementation is surprisingly simple and additional unit test coverage is welcome, too.

Tree-SHA512: 4904f5f914fe1d421d32f60edb7c5a028c8ea0f140a2f207a106b4752d441164e073066a6bf2e17693f859fe847815a96609d3cf521e0ac4178d8cd09362ea3d
2024-04-29 10:38:50 -04:00
Ryan Ofsky
6a8b2befea refactor: Avoid copying util::Result values
Copying util::Result values is less efficient than moving them because they
allocate memory and contain strings. Also this is needed to avoid compile
errors in https://github.com/bitcoin/bitcoin/pull/25722 which adds a
std::unique_ptr member to util::Result which implicity disables copying.
2024-04-25 16:08:24 -04:00
Ryan Ofsky
834f65e824 refactor: Drop util::Result operator=
`util::Result` objects are aggregates that can hold multiple fields with
different information. Currently Result objects can only hold a success value
of an arbitrary type or a single bilingual_str error message. In followup PR
https://github.com/bitcoin/bitcoin/pull/25722, Result objects may be able to
hold both success and failure values of different types, plus error and warning
messages.

Having a Result::operator= assignment operator that completely erases all
existing Result information before assigning new information is potentially
dangerous in this case. For example, code that looks like it is assigning a
warning value could erase previously-assigned success or failure values.
Conversely, code that looks like it is just assigning a success or failure
value could erase previously assigned error and warning messages.

To prevent potential bugs like this, disable Result::operator= assignment
operator.

It is possible in the future we may want to re-enable operator= in limited
cases (such as when implicit conversions are not used) or add a Replace() or
Reset() method that mimicks default operator= behavior. Followup PR
https://github.com/bitcoin/bitcoin/pull/25722 also adds a Result::Update()
method providing another way to update an existing Result object.

Co-authored-by: stickies-v <stickies-v@protonmail.com>
2024-04-25 16:08:24 -04:00
Ryan Ofsky
16a6174613
Merge bitcoin/bitcoin#29904: refactor: Use our own implementation of urlDecode
992c714451 common: Don't terminate on null character in UrlDecode (Fabian Jahr)
099fa57151 scripted-diff: Modernize name of urlDecode function and param (Fabian Jahr)
8f39aaae41 refactor: Remove hooking code for urlDecode (Fabian Jahr)
650d43ec15 refactor: Replace libevent use in urlDecode with our own code (Fabian Jahr)
46bc6c2aaa test: Add unit tests for urlDecode (Fabian Jahr)

Pull request description:

  Fixes #29654 (as a side-effect)

  Removing dependencies is a general goal of the project and the xz backdoor has been an additional wake up call recently. Libevent shows many of the same symptoms, few maintainers and slow releases. While libevent can not be removed completely over night we should start removing it’s usage where it's possible, ideally with the end goal to removing it completely.

  This is a pretty easy win in that direction. The [`evhttp_uridecode` function from libevent](e0a4574ba2/http.c (L3542)) we were using in `urlDecode` could be easily emulated in fewer LOC. This also ports the [applicable test vectors over from libevent](https://github.com/libevent/libevent/blob/master/test/regress_http.c#L3430).

ACKs for top commit:
  achow101:
    ACK 992c714451
  theStack:
    Code-review ACK 992c714451
  maflcko:
    ACK 992c714451 👈
  stickies-v:
    ACK 992c714451

Tree-SHA512: 78f76ae7ab3b6710eab2aaac20f55eb0da7803e057eaa6220e865f328666a5399ef1a479702aaf630b2f974ad3aa15e2b6adac9c11bc8c3d4be21e8af1667fea
2024-04-25 13:02:43 -04:00
Fabian Jahr
099fa57151
scripted-diff: Modernize name of urlDecode function and param
-BEGIN VERIFY SCRIPT-
sed -i 's/urlDecode/UrlDecode/g' $(git grep -l 'urlDecode' ./src)
sed -i 's/urlEncoded/url_encoded/g' $(git grep -l 'urlEncoded' ./src)
-END VERIFY SCRIPT-
2024-04-24 23:26:24 +02:00
Fabian Jahr
8f39aaae41
refactor: Remove hooking code for urlDecode
The point of this was to be able to build bitcoin-tx and bitcoin-wallet without libevent, see #18504.

Now that we use our own implementation of urlDecode this is not needed anymore.

Co-authored-by: stickies-v <stickies-v@protonmail.com>
2024-04-24 23:23:38 +02:00
Fabian Jahr
650d43ec15
refactor: Replace libevent use in urlDecode with our own code 2024-04-24 23:23:34 +02:00
Ava Chow
a7129f827c
Merge bitcoin/bitcoin#24313: Improve display address handling for external signer
4357158c47 wallet: return and display signer error (Sjors Provoost)
dc55531087 wallet: compare address returned by displayaddress (Sjors Provoost)
6c1a2cc09a test: use h marker for external signer mock (Sjors Provoost)

Pull request description:

  * HWI returns the requested address: as a sanity check, we now compare that to what we expected
     * external signer documentation now reflects that HWI alternatives must implement this check
  * both RPC and GUI will now return an error text, rather than just fail (the GUI even failed silently in some cases)

ACKs for top commit:
  brunoerg:
    ACK 4357158c47
  achow101:
    ACK 4357158c47

Tree-SHA512: 4f56edf3846745c8e7d08ef55cf29e8bb468256457149377c5f02da097931f9ca0c06bdbd856dc2385cde4fd11e4dc3b634c5a48814ff27f5562c8a25d43da93
2024-04-23 17:20:54 -04:00
merge-script
aaab5fb3c5
Merge bitcoin-core/gui#806: refactor: Misc int sign change fixes
05416422d3 refactor: Avoid implicit-integer-sign-change in processNewTransaction (MarcoFalke)
321f105d08 refactor: Avoid implicit-signed-integer-truncation-or-sign-change in FreedesktopImage (MarcoFalke)
6d8eecd33a refactor: Avoid implicit-integer-sign-change in createTransaction (MarcoFalke)

Pull request description:

  This is allowed by the language. However, the `integer` sanitizer complains about it. Thus, fix it, so that the `integer` sanitizer can be used in the future to catch unintended sign changes.

  Fixes #805.

ACKs for top commit:
  pablomartin4btc:
    tACK 05416422d3
  hebasto:
    ACK 05416422d3, I have reviewed the code and it looks OK.

Tree-SHA512: eaa941479bd7bee196eb8b31d93b8e1db122410cf62e8ec4cbbec35cfd14cc766081c3df5dd14a228e21ad2678d8b8ba0d2649e5934c994a90ae96d8b264b4ce
2024-04-18 09:46:04 +01:00
Sjors Provoost
4357158c47
wallet: return and display signer error
Both RPC and GUI now render a useful error message instead of (silently) failing.

Replace bool with util::Result<void> to clarify that this either succeeds or returns an error message.
2024-04-16 17:47:43 +02:00
Sjors Provoost
dc55531087
wallet: compare address returned by displayaddress
Update external signer documentation to reflect this requirement, which HWI already implements.
2024-04-16 17:47:43 +02:00
dergoegge
78407b99ed [clang-tidy] Enable the misc-no-recursion check
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Co-authored-by: Gloria Zhao <gloriajzhao@gmail.com>
2024-04-07 14:04:45 +01:00
Ryan Ofsky
4373414d26
Merge bitcoin/bitcoin#29130: wallet: Add createwalletdescriptor and gethdkeys RPCs for adding new automatically generated descriptors
746b6d8839 test: Add test for createwalletdescriptor (Ava Chow)
2402b63062 wallet: Test upgrade of pre-taproot wallet to have tr() descriptors (Ava Chow)
460ae1bf67 wallet, rpc: Add createwalletdescriptor RPC (Ava Chow)
8e1a475062 wallet: Be able to retrieve single key from descriptors (Ava Chow)
85b1fb19dd wallet: Add GetActiveHDPubKeys to retrieve xpubs from active descriptors (Ava Chow)
73926f2d31 wallet, descspkm: Refactor wallet descriptor generation to standalone func (Andrew Chow)
54e74f46ea wallet: Refactor function for single DescSPKM setup (Andrew Chow)
3b09d0eb7f tests: Test for gethdkeys (Ava Chow)
5febe28c9e wallet, rpc: Add gethdkeys RPC (Ava Chow)
66632e5c24 wallet: Add IsActiveScriptPubKeyMan (Ava Chow)
fa6a259985 desc spkm: Add functions to retrieve specific private keys (Ava Chow)
fe67841464 descriptor: Be able to get the pubkeys involved in a descriptor (Ava Chow)
ef6745879d key: Add constructor for CExtKey that takes CExtPubKey and CKey (Ava Chow)

Pull request description:

  This PR adds a `createwalletdescriptor` RPC which allows users to add new automatically generated descriptors to their wallet, e.g. to upgrade a 0.21.x wallet to contain a taproot descriptor. This RPC takes 3 arguments: the output type to create a descriptor for, whether the descriptor will be internal or external, and the HD key to use if the user wishes to use a specific key. The HD key is an optional parameter. If it is not specified, the wallet will use the key shared by the active descriptors, if they are all single key. For most users in the expected upgrade scenario, this should be sufficient. In more advanced cases, the user must specify the HD key to use.

  Currently, specified HD keys must already exist in the wallet. To make it easier for the user to know, `gethdkeys` is also added to list out the HD keys in use by all of the descriptors in the wallet. This will include all HD keys, whether we have the private key, for it, which descriptors use it and their activeness, and optionally the extended private key. In this way, users with more complex wallets will be still be able to get HD keys from their wallet for use in other scenarios, and if they want to use `createwalletdescriptor`, they can easily get the keys that they can specify to it.

  See also https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1866961865

ACKs for top commit:
  Sjors:
    re-utACK 746b6d8839
  furszy:
    ACK 746b6d8
  ryanofsky:
    Code review ACK 746b6d8839, and this looks ready to merge. There were various suggested changes since last review where main change seems to be switching `gethdkeys` output to use normalized descriptors (removing hardened path components).

Tree-SHA512: f2849101e6fbf1f59cb031eaaaee97af5b1ae92aaab54c5716940d210f08ab4fc952df2725b636596cd5747b8f5beb1a7a533425bc10d09da02659473516fbda
2024-03-29 06:39:57 -04:00
Ryan Ofsky
c8e3978114
Merge bitcoin/bitcoin#27307: wallet: track mempool conflicts with wallet transactions
5952292133 wallet, rpc: show mempool conflicts in `gettransaction` result (ishaanam)
54e07ee22f wallet: track mempool conflicts (ishaanam)
d64922b590 wallet refactor: use CWalletTx member functions to determine tx state (ishaanam)
ffe5ff1fb6 scripted-diff: wallet: s/TxStateConflicted/TxStateBlockConflicted (ishaanam)
180973a941 test: Add tests for wallet mempool conflicts (ishaanam)

Pull request description:

  The `mempool_conflicts` variable is added to `CWalletTx`, it is a set of txids of txs in the mempool conflicting with the wallet tx or a wallet tx's parent. This PR only changes how mempool-conflicted txs are dealt with in memory.

  `IsSpent` now returns false for an output being spent by a mempool conflicted transaction where it previously returned true.

  A txid is added to `mempool_conflicts` during  `transactionAddedToMempool`. A txid is removed from `mempool_conflicts` during  `transactionRemovedFromMempool`.

  This PR also adds a `mempoolconflicts` field to the `gettransaction` wallet RPC result.

  Builds on #27145
  Second attempt at #18600

ACKs for top commit:
  achow101:
    ACK 5952292133
  ryanofsky:
    Code review ACK 5952292133. Just small suggested changes since last review
  furszy:
    ACK 59522921

Tree-SHA512: 615779606723dbb6c2e302681d8e58ae2052ffee52d721ee0389746ddbbcf4b4c4afacf01ddf42b6405bc6f883520524186a955bf6b628fe9b3ae54cffc56a29
2024-03-27 12:45:08 -04:00
MarcoFalke
6d8eecd33a refactor: Avoid implicit-integer-sign-change in createTransaction 2024-03-21 19:32:12 +01:00
Ava Chow
460ae1bf67 wallet, rpc: Add createwalletdescriptor RPC 2024-03-20 16:15:43 -04:00
Ava Chow
8e1a475062 wallet: Be able to retrieve single key from descriptors
Adds CWallet::GetKey which retrieves a single key from the descriptors
stored in the wallet.
2024-03-20 16:15:43 -04:00
Ava Chow
85b1fb19dd wallet: Add GetActiveHDPubKeys to retrieve xpubs from active descriptors 2024-03-20 16:15:43 -04:00
Andrew Chow
73926f2d31 wallet, descspkm: Refactor wallet descriptor generation to standalone func 2024-03-20 16:15:43 -04:00
Andrew Chow
54e74f46ea wallet: Refactor function for single DescSPKM setup
We will need access to a function that sets up a singular
DescriptorSPKM, so refactor this out of the multiple DescriptorSPKM
setup function.
2024-03-20 16:15:43 -04:00
Ava Chow
5febe28c9e wallet, rpc: Add gethdkeys RPC
gethdkeys retrieves all HD keys stored in the wallet's descriptors.
2024-03-20 16:15:43 -04:00
ishaanam
5952292133 wallet, rpc: show mempool conflicts in gettransaction result 2024-03-20 15:05:37 -04:00
ishaanam
54e07ee22f wallet: track mempool conflicts
Behavior changes are:
- if a tx has a mempool conflict, the wallet will not attempt to
  rebroadcast it
- if a txo is spent by a mempool-conflicted tx, that txo is no
  longer considered spent
2024-03-20 15:05:34 -04:00
Ava Chow
66632e5c24 wallet: Add IsActiveScriptPubKeyMan
Given a ScriptPubKeyMan, it's useful to ask the wallet whether it is
currently active.
2024-03-18 11:53:23 -04:00
Ava Chow
fa6a259985 desc spkm: Add functions to retrieve specific private keys 2024-03-18 11:53:20 -04:00
ishaanam
d64922b590 wallet refactor: use CWalletTx member functions to determine tx state 2024-03-15 18:03:42 -04:00
ishaanam
ffe5ff1fb6 scripted-diff: wallet: s/TxStateConflicted/TxStateBlockConflicted
-BEGIN VERIFY SCRIPT-
sed -i 's/TxStateConflicted/TxStateBlockConflicted/g' src/wallet/wallet.cpp src/wallet/interfaces.cpp src/wallet/transaction.h src/wallet/transaction.cpp
sed -i 's/isConflicted/isBlockConflicted/g' src/wallet/transaction.h src/wallet/wallet.cpp
-END VERIFY SCRIPT-
2024-03-14 17:38:39 -04:00
Ava Chow
b4a05751b6
Merge bitcoin/bitcoin#29586: wallet: default wallet migration, modify inconvenient backup filename
a951dba3a9 wallet: default wallet migration, modify inconvenient backup filename (furszy)

Pull request description:

  Fixes #29584

  On default legacy wallets, the backup filename starts with an "-" due to the wallet name being empty. This is inconvenient for systems who treat what follows the initial "-" character as flags.

  Note:
  As the user can freely set the wallet name to anything, we could also guard the backup filename against other inconvenient characters in the future (we need to be careful here, because the wallet name could also be a path).

ACKs for top commit:
  achow101:
    ACK a951dba3a9
  brunoerg:
    utACK a951dba3a9
  vasild:
    ACK  a951dba3a9

Tree-SHA512: 6347bb12cfb50526a4baad96e4f1df9d82b493f79f0a0f7e0a1c8335a86a1e8e147c7b7f95cec6ede6f4507506a7eaf7972bd35131a2d5ed4cbbf38d94f0a9ca
2024-03-11 08:59:06 -04:00
Ava Chow
c07935bcf5
Merge bitcoin/bitcoin#28960: kernel: Remove dependency on CScheduler
d5228efb53 kernel: Remove dependency on CScheduler (TheCharlatan)
06069b3913 scripted-diff: Rename MainSignals to ValidationSignals (TheCharlatan)
0d6d2b650d scripted-diff: Rename SingleThreadedSchedulerClient to SerialTaskRunner (TheCharlatan)
4abde2c4e3 [refactor] Make MainSignals RAII styled (TheCharlatan)
84f5c135b8 refactor: De-globalize g_signals (TheCharlatan)
473dd4b97a [refactor] Prepare for g_signals de-globalization (TheCharlatan)
3fba3d5dee [refactor] Make signals optional in mempool and chainman (TheCharlatan)

Pull request description:

  By defining a virtual interface class for the scheduler client, users of the kernel can now define their own event consuming infrastructure, without having to spawn threads or rely on the scheduler design.

  Removing `CScheduler` also allows removing the thread and exception modules from the kernel library.

  To make the `CMainSignals` class easier to use from a kernel library perspective, remove its global instantiation and adopt RAII practices.

  Renames `CMainSignals` to `ValidationSignals`, which more accurately describes its purpose and scope.

  Also make the `ValidationSignals` in the `ChainstateManager` and CTxMemPool` optional. This could be useful in the future for using or testing these classes without having to instantiate any form of signal handling.

  ---

  This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). It improves the kernel API and removes two modules from the kernel library.

ACKs for top commit:
  maflcko:
    re-ACK d5228efb53 🌄
  ryanofsky:
    Code review ACK d5228efb53. Just comment change since last review.
  vasild:
    ACK d5228efb53
  furszy:
    diff ACK d5228ef

Tree-SHA512: e93a5f10eb6182effb84bb981859a7ce750e466efd8171045d8d9e7fe46e4065631d9f6f533c5967c4d34c9bb7d7a67e9f4593bd4c5b30cd7b3bbad7be7b331b
2024-03-08 20:58:04 -05:00
furszy
a951dba3a9
wallet: default wallet migration, modify inconvenient backup filename
On default legacy wallets, the backup filename starts with an "-" due
to the wallet name being empty. This is inconvenient for systems who
treat what follows the initial "-" character as flags.
2024-03-08 19:40:11 -03:00
stickies-v
30a6c99935
rpc: access some args by name
Use the new key-based Arg helper in a few locations to show how
it is used.
2024-03-01 13:51:21 +00:00
UdjinM6
367bb7a80c
wallet: Avoid updating ReserveDestination::nIndex when GetReservedDestination fails 2024-02-28 13:00:00 +03:00
Lőrinc
b03b20685a Fix CI-detected codespell warnings 2024-02-23 23:01:07 +01:00
Murch
9dae3b970a [fuzz] Avoid partial negative result 2024-02-21 15:49:05 -05:00
Ava Chow
fe67841464 descriptor: Be able to get the pubkeys involved in a descriptor 2024-02-20 11:20:58 -05:00
fanquake
45b2a91897
Merge bitcoin/bitcoin#29404: refactor: bitcoin-config.h includes cleanup
9d1dbbd4ce scripted-diff: Fix bitcoin_config_h includes (TheCharlatan)

Pull request description:

  As mentioned in https://github.com/bitcoin/bitcoin/pull/26924#issuecomment-1403449932 and https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399, it is currently not safe to remove `bitcoin-config.h` includes from headers because some unrelated file might be depending on it.

  See also #26972 for discussion.

  Solve this by including the file directly everywhere it's required, regardless of whether or not it's already included by another header.

  There should be no functional change here, but it will allow us to safely remove includes from headers in the future.

  ~I'm afraid it's a bit tedious to reproduce these commits, but it's reasonably straightforward:~
  Edit: See note below

  ```bash
  # All commands executed from the src/ subdir.

  # Collect all tokens from bitcoin-config.h.in
  # Isolate the tokens and remove blank lines
  # Replace newlines with | and remove the last trailing one
  # Collect all files which use these tokens
  # Filter out subprojects (proper forwarding can be verified from Makefiles)
  # Filter out .rc files
  # Save to a text file
  git grep -E -l `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-with-config-include.txt

  # Find all files from the above list which don't include bitcoin-config.h
  git grep -L -E "config/bitcoin-config.h" -- `cat files-with-config-include.txt`

  # Include them manually with the exception of some files in crypto:
  # crypto/sha256_arm_shani.cpp crypto/sha256_avx2.cpp crypto/sha256_sse41.cpp crypto/sha256_x86_shani.cpp
  # These are exceptions which don't use bitcoin-config.h, rather the Makefile.am adds these cppflags manually.

  # Commit changes. This should match the first commit of this PR.

  # Use the same search as above to find all files which DON'T use any config tokens
  git grep -E -L `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-without-config-include.txt

  # Manually remove the includes and commit changes. This should match the second commit of this PR.
  ```

  Edit: I'll keep this old description for posterity, but the manual approach has been replaced with a scripted diff from TheCharlatan

ACKs for top commit:
  maflcko:
    ACK 9d1dbbd4ce 🚪
  TheCharlatan:
    ACK 9d1dbbd4ce
  hebasto:
    ACK 9d1dbbd4ce, I have reviewed the code and it looks OK.
  fanquake:
    ACK 9d1dbbd4ce

Tree-SHA512: f11ddc4ae6a887f96b954a6b77f310558ddb271088a3fda3edc833669c4251b7f392515224bbb8e5f67eb2c799b4ffed3b07d96454e82ec635c686d0df545872
2024-02-20 13:07:48 +00:00
fanquake
b1a46b212f
Merge bitcoin/bitcoin#26008: wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets
e041ed9b75 wallet: Retrieve ID from loaded DescSPKM directly (Ava Chow)
39640dd34e wallet: Use scriptPubKeyCache in GetSolvingProvider (Ava Chow)
b410f68791 wallet: Use scriptPubKey cache in GetScriptPubKeyMans (Ava Chow)
edf4e73a16 wallet: Use scriptPubKey cache in IsMine (Ava Chow)
37232332bd wallet: Cache scriptPubKeys for all DescriptorSPKMs (Ava Chow)
99a0cddbc0 wallet: Introduce a callback called after TopUp completes (Ava Chow)
b276825932 bench: Add a benchmark for ismine (Ava Chow)

Pull request description:

  Wallets that have a ton of non-ranged descriptors (such as a migrated non-HD wallet) perform fairly poorly due to looping through all of the wallet's `ScriptPubKeyMan`s. This is done in various places, such as `IsMine`, and helper functions for fetching a `ScriptPubKeyMan` and a `SolvingProvider`. This also has a bit of a performance impact on standard descriptor wallets, although less noticeable due to the small number of SPKMs.

  As these functions are based on doing `IsMine` for each `ScriptPubKeyMan`, we can improve this performance by caching `IsMine` scriptPubKeys for all descriptors and use that to determine which `ScriptPubKeyMan` to actually use for those things. This cache is used exclusively and we no longer iterate the SPKMs.

  Also added a benchmark for `IsMine`.

ACKs for top commit:
  ryanofsky:
    Code review ACK e041ed9b75. Just suggested changes since last review
  josibake:
    ACK e041ed9b75
  furszy:
    Code review ACK e041ed9b

Tree-SHA512: 8e7081991a025e682e9dea838b4543b0d179832d1c47397fb9fe7a97fa01eb699c15a5d5a785634926844fc83a46e6ac07ef753119f39d84423220ef8a548894
2024-02-20 10:17:46 +00:00
Ava Chow
e041ed9b75 wallet: Retrieve ID from loaded DescSPKM directly
Instead of iterating m_spk_managers a DescriptorSPKM has been loaded in
order to get it's ID to compare, have LoadDescriptorSPKM return a
reference to the loaded DescriptorSPKM so it can be queried directly.
2024-02-16 14:36:10 -05:00
Ava Chow
39640dd34e wallet: Use scriptPubKeyCache in GetSolvingProvider 2024-02-16 14:36:10 -05:00
Ava Chow
b410f68791 wallet: Use scriptPubKey cache in GetScriptPubKeyMans 2024-02-16 14:36:10 -05:00
Ava Chow
edf4e73a16 wallet: Use scriptPubKey cache in IsMine 2024-02-16 14:36:10 -05:00
Ava Chow
37232332bd wallet: Cache scriptPubKeys for all DescriptorSPKMs
Have CWallet maintain a cache of all known scriptPubKeys for its
DescriptorSPKMs in order to improve performance of the functions that
require searching for scriptPubKeys.
2024-02-16 14:36:09 -05:00
Ava Chow
99a0cddbc0 wallet: Introduce a callback called after TopUp completes
After TopUp completes, the wallet containing each SPKM will want to know
what new scriptPubKeys were generated. In order for all TopUp calls
(including ones internal the the SPKM), we use a callback function in
the WalletStorage interface.
2024-02-16 14:35:39 -05:00
fanquake
3cbc8cbc71
Merge bitcoin/bitcoin#28037: rpc: Drop migratewallet experimental warning
f1684bb88a rpc: mention that migratewallet can take a while (Andrew Chow)
9ecff997e1 rpc: Drop migratewallet experimental warning (Andrew Chow)

Pull request description:

  The migration process itself hasn't fundamentally changed since it was added, so I think it's reasonable to say that it is no longer experimental.

ACKs for top commit:
  maflcko:
    lgtm ACK f1684bb88a
  josibake:
    ACK f1684bb88a
  furszy:
    ACK f1684bb88a
  ryanofsky:
    Code review ACK f1684bb88a
  willcl-ark:
    ACK f1684bb88a

Tree-SHA512: 99b176cddbf3878c76bd4c80c030106200bf03139785e26dbae3341e1a675b623a13cd6dc7a0bb78344335bf859ae7548d97b2b58eb650c6e7b305d7cdc86e40
2024-02-16 12:28:05 +00:00
TheCharlatan
84f5c135b8
refactor: De-globalize g_signals 2024-02-15 14:37:01 +01:00
TheCharlatan
9d1dbbd4ce scripted-diff: Fix bitcoin_config_h includes
-BEGIN VERIFY SCRIPT-

regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)'

exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp"

git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done;

git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done;

for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done;

-END VERIFY SCRIPT-

The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with:
./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -)

The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile.

The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing.

The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing.

The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
2024-02-13 20:10:44 +00:00
furszy
77331aa2a1
wallet: simplify EraseRecords by using 'ErasePrefix' 2024-02-12 16:06:13 -03:00
furszy
33757814ce
wallet: bdb batch 'ErasePrefix', do not create txn internally
Transactions are intended to be started on upper layers rather than
internally by the bdb batch object. This enables us to consolidate
different write operations within a procedure in the same db txn,
improving consistency due to the atomic property of the transaction,
as well as its performance due to the reduction of disk write
operations.

Important Note:
This approach also ensures that the BerkeleyBatch::ErasePrefix
function behaves exactly as the SQLiteBatch::ErasePrefix function,
which does not create a db txn internally.

Furthermore, since the `BerkeleyBatch::ErasePrefix' implementation
erases records one by one (by traversing the db), this change
ensures that the function is always called within an active txn
context. Without this measure, there's a potential risk to consistency;
certain records may be removed while others could persist due to an
internal failure during the procedure.
2024-02-12 16:05:15 -03:00