0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Commit graph

22649 commits

Author SHA1 Message Date
MacroFake
fa4c59d65b
Move blockstorage option logging to LoadChainstate() 2022-09-01 17:07:45 +02:00
MacroFake
fa3358b668
Move validation option logging to LoadChainstate() 2022-08-29 11:58:29 +02:00
Andrew Chow
bbbc26dcfc
Merge bitcoin/bitcoin#25924: scripted-diff: rpc: fix rescan RPC name (s/rescanwallet/rescanblockchain/)
e90a445d7e scripted-diff: rpc: fix rescan RPC name (s/rescanwallet/rescanblockchain/) (Sebastian Falbesoner)

Pull request description:

  There is no RPC call named `rescanwallet`, i.e. fix this by renaming to the actual RPC called `rescanblockchain`.

ACKs for top commit:
  achow101:
    ACK e90a445d7e
  aureleoules:
    ACK e90a445d7e.
  promag:
    ACK e90a445d7e

Tree-SHA512: abf1d1c18de32d87c29e4ff2b782dfb0e4a46dc2c2cc51ab616d12674a0f4a5d22214e00955663ae897cbb88f4f6ced913850f28ea3f5c1b3a54577a25fbf399
2022-08-25 17:12:08 -04:00
MacroFake
d36bec9b3b
Merge bitcoin/bitcoin#25905: refactor: Move ChainstateManager options into m_options struct
7bc33a88f7 refactor: Move ChainstateManager options into m_options struct (Ryan Ofsky)

Pull request description:

  Move `ChainstateManager` options into `m_options` struct to simplify class initialization, organize class members, and to name external option variables differently than internal state variables.

  This change was originally in #25862, but it was suggested to split off in https://github.com/bitcoin/bitcoin/pull/25862#discussion_r951459817 so it could be merged earlier and reduce conflicts with other PRs.

ACKs for top commit:
  naumenkogs:
    ACK 7bc33a88f7

Tree-SHA512: 1c3c77be7db60222732221c087fd01cb802b84ac93333fccb38c8d16645f5f950c3362981021e7a3ae054f19fa7dd9e1cd15daaa101b61ca8853e42a1fd21474
2022-08-25 09:48:12 +02:00
Sebastian Falbesoner
e90a445d7e scripted-diff: rpc: fix rescan RPC name (s/rescanwallet/rescanblockchain/)
There is no RPC call named `rescanwallet`, i.e. fix this by renaming to
the actual RPC called `rescanblockchain`.

-BEGIN VERIFY SCRIPT-
sed -i s/rescanwallet/rescanblockchain/ $(git grep -l rescanwallet)
-END VERIFY SCRIPT-
2022-08-24 23:26:33 +02:00
MacroFake
1420547ec3
Merge bitcoin/bitcoin#25911: net: update hardcoded mainnet seeds for 24.x
9b6f5fafa9 net: update hardcoded mainnet seeds for 24.x (Jon Atack)
7fd9028017 contrib: make-seeds updates for 24.x (Jon Atack)
6075a0be3c net: update manual hardcoded mainnet seeds for 24.x (Jon Atack)

Pull request description:

  Update the hardcoded P2P network seeds for 24.x after updating the manual seeds and the generation script as necessary. Previous update was #24417.

  Can be tested by following the steps in `contrib/seeds/README.md`.

  Tool output:
  ```
  $ python3 makeseeds.py -a asmap-filled.dat < seeds_main.txt > nodes_main.txt
  Loading asmap database "asmap-filled.dat"…Done.
  Loading and parsing DNS seeds…Done.
    IPv4   IPv6  Onion Pass
  472019  73788      0 Initial
  472019  73788      0 Skip entries with invalid address
  472019  73788      0 After removing duplicates
    7766   2310      0 Enforce minimal number of blocks
    6534   1835      0 Require service bit 1
    2808    801      0 Require minimum uptime
    2748    781      0 Require a known and recent user agent
    2727    775      0 Filter out hosts with multiple bitcoin ports
     512    267      0 Look up ASNs and limit results per ASN and per net
  ```

ACKs for top commit:
  laanwj:
    LGTM ACK 9b6f5fafa9
  Emzy:
    ACK 9b6f5fa
  brunoerg:
    ACK 9b6f5fafa9

Tree-SHA512: eef994bbd60524cfd9e4b5b836ddbe615cd2a77466394cdbb70a0ebc30c7822d8605473424f6d4a1ba37313c59373cf10717977a5c07e6f839e5ff86464c8c55
2022-08-24 19:08:24 +02:00
fanquake
2d83a20113
Merge bitcoin/bitcoin#25863: test: remove unused norm_prv parameter in descriptor_tests.cpp.
57d1367fec test: remove unused `norm_prv` parameter (w0xlt)

Pull request description:

  This PR removes the unused `norm_prv` parameter in `src/test/descriptor_tests.cpp`.

ACKs for top commit:
  achow101:
    ACK 57d1367fec

Tree-SHA512: 5b16b6bea94db0b5f2c3675b6529312b50e120d4ec7633e4184dd4ba6fc04e086efb273b9e61f748c8a15cbdc243450b09fc58ec7343379f3151a3b9e7e37106
2022-08-24 16:24:52 +01:00
MacroFake
c89fabff3d
Merge bitcoin/bitcoin#25879: refactor: Make Join() util work with any container type
fa95315655 Use new Join() helper for ListBlockFilterTypes() (MacroFake)
fa1c716955 Make Join() util work with any container type (MacroFake)
faf8da3c8d Remove Join() helper only used in tests (MacroFake)

Pull request description:

  This allows to drop some code

ACKs for top commit:
  naumenkogs:
    ACK fa95315655
  stickies-v:
    ACK [fa95315](fa95315655)

Tree-SHA512: efd65b65722f46b221bd53140ff22bd8e45adc83617980233f28f695be3108a6ab01affd751d715134ffcb9762228ba8952e9467e590cff022c83e0f5404cb74
2022-08-24 11:30:44 +02:00
MacroFake
fa95315655
Use new Join() helper for ListBlockFilterTypes() 2022-08-24 10:15:32 +02:00
MacroFake
fa1c716955
Make Join() util work with any container type
Also, remove helper that is only used in tests.
2022-08-24 10:15:11 +02:00
Jon Atack
9b6f5fafa9 net: update hardcoded mainnet seeds for 24.x
following the steps in contrib/seeds/README.md
2022-08-24 00:04:00 +02:00
Ryan Ofsky
7bc33a88f7 refactor: Move ChainstateManager options into m_options struct
Move ChainstateManager options into m_options struct to simplify class
initialization, organize class members, and to name external option variables
differently than internal state variables.

This change was originally in #25862, but it was suggested to split off in
https://github.com/bitcoin/bitcoin/pull/25862#discussion_r951459817 so it could
be merged earlier and reduce conflicts with other PRs.
2022-08-22 13:19:15 -04:00
Andrew Chow
2bd9aa5a44
Merge bitcoin/bitcoin#25647: wallet: return change from SelectionResult
4fef534428 wallet: use GetChange() when computing waste (S3RK)
87e0ef9031 wallet: use GetChange() in tx building (S3RK)
15e97a6886 wallet: add SelectionResult::GetChange (S3RK)
72cad28da0 wallet: calculate and store min_viable_change (S3RK)
e3210a7225 wallet: account for preselected inputs in target (S3RK)
f8e796348b wallet: add SelectionResult::Merge (S3RK)
06f558e4e2 wallet: accurate SelectionResult::m_target (S3RK)
c8cf08ea74 wallet: ensure m_min_change_target always covers change fee (S3RK)

Pull request description:

  Benefits:
  1. more accurate waste calculation for knapsack. Waste calculation is now consistent with tx building code. Before we always assumed change for knapsack even when the solution is changeless4.
  2. simpler tx building code. Only create change output when it's needed
  3. makes it easier to correctly account for fees for CPFP inputs (should be done in a follow up)

  In the first three commits we fix the code to accurately track selection target in `SelectionResult::m_target`
  Then we introduce new variable `min_change` that represents the minimum viable change amount
  Then we introduce `SelectionResult::GetChange()` which incapsulates dropping change for fee logic and uses correct values of `SelectionResult::m_target`
  Then we use `SelectionResult::GetChange()` in both tx building and waste calculation code

  This PR is a refactoring and shouldn't change the behaviour.
  There is only one known small change (arguably a bug fix). Before we dropped change output if it's smaller than `cost_of_change` after paying change fees. This is incorrect as `cost_of_change` already includes `change_fee`.

ACKs for top commit:
  achow101:
    ACK 4fef534428
  Xekyo:
    crACK 4fef534428
  furszy:
    Code review ACK 4fef5344
  w0xlt:
    ACK 4fef534428

Tree-SHA512: 31a7455d4129bc39a444da0f16ad478d690d4d9627b2b8fdb5605facc6488171926bf02f5d7d9a545b2b59efafcf5bb3d404005e4da15c7b44b3f7d441afb941
2022-08-22 12:42:36 -04:00
glozow
375ebadbf8
fixups for BIP125 doc cleanup
Grammar and readability fixups.
Clarifies "bip125-replaceable" helpstrings.
2022-08-22 14:59:58 +01:00
fanquake
c5f0cbefa3
Merge bitcoin/bitcoin#25775: docs: remove non-signaling mentions of BIP125
1dc03dda05 [doc] remove non-signaling mentions of BIP125 (glozow)
32024d40f0 scripted-diff: remove mention of BIP125 from non-signaling var names (glozow)

Pull request description:

  We have pretty thorough documentation of our RBF policy in doc/policy/mempool-replacements.md. It enumerates each rule with several sentences of rationale. Also, each rule pretty much has its own function (3 and 4 share one), with extensive comments. The doc states explicitly that our rules are similar but differ from BIP125, and contains a record of historical changes to RBF policy.

  We should not use "BIP125" as synonymous with our RBF policy because:
  - Our RBF policy is different from what is specified in BIP125, for example:
      - the BIP does not mention our rule about the replacement feerate being higher (our Rule 6)
      - the BIP uses minimum relay feerate for Rule 4, while we have used incremental relay feerate since #9380
      - the "inherited signaling" question (CVE-2021-31876). Call it discrepancy, ambiguous wording, doc misinterpretation, or implementation details, I would recommend users refer to doc/policy/mempool-replacements.md
      - the signaling policy is configurable, see #25353
  - Our RBF policy may change further
  - We have already marked BIP125 as only "partially implemented" in docs/bips.md since 1fd49eb498
  - See comments from people who are not me recently:
      - https://github.com/bitcoin/bitcoin/pull/25038#discussion_r909507429
      - https://github.com/bitcoin/bitcoin/pull/25575#issuecomment-1179519204

  This PR removes all non-signaling mentions of BIP125 (if people feel strongly, we can remove all mentions of BIP125 period). It may be useful to refer to the concept of "tx opts in to RBF if it has at least one nSequence less than (0xffffffff - 1)" as "BIP125 signaling" because:
  - It is succint.
  - It has already been widely marketed as BIP125 opt-in signaling.
  - Our API uses it when referring to signaling (e.g. getmempoolentry["bip125-replaceable"] and wallet error message "not BIP 125 replaceable"). Changing those is more invasive.
  - If/when we have other ways to signal in the future, we can disambiguate them this way. See #25038 which proposes another way of signaling, and where I pulled these commits from.

  Alternatives:
  - Changing our policy to match BIP125. This doesn't make sense as, for example, we would have to remove the requirement that a replacement tx has a higher feerate (Rule 6).
  - Changing BIP125 to match what we have. This doesn't make sense as it would be a significant change to a BIP years after it was finalized and already used as a spec to implement RBF in other places.
  - Document our policy as a new BIP and give it a number. This might make sense if we don't expect things to change a lot, and can be done as a next step.

ACKs for top commit:
  darosior:
    ACK 1dc03dda05
  ariard:
    ACK 1dc03dda
  t-bast:
    ACK 1dc03dda05

Tree-SHA512: a3adc2039ec5785892d230ec442e50f47f7062717392728152bbbe27ce1c564141f85253143f53cb44e1331cf47476d74f5d2f4b3cd873fc3433d7a0aa783e02
2022-08-22 10:35:26 +01:00
fanquake
607d5a46aa
Merge bitcoin/bitcoin#23202: wallet: allow psbtbumpfee to work with txs with external inputs
c3b099ace0 wallet, tests: Test bumpfee's max input weight calculation (Andrew Chow)
116a620ce7 Make DUMMY_CHECKER availble outside of script/sign.cpp (Andrew Chow)
ff638323d1 test, bumpfee: Check that psbtbumpfee can bump txs with external inputs (Andrew Chow)
1bc8106d4c bumpfee: be able to bump fee of a tx with external inputs (Andrew Chow)
31dd3dc9e5 bumpfee: Clear scriptSigs and scriptWitnesses before calculated max size (Andrew Chow)
a0c3afb898 bumpfee: extract weights of external inputs when bumping fee (Andrew Chow)
612f1e44fe bumpfee: Calculate fee by looking up UTXOs (Andrew Chow)

Pull request description:

  This PR allows `psbtbumpfee` to return a PSBT for transactions that contain external inputs. This does not work for bumping in the GUI nor `bumpfee` because these need private keys available to sign and send the transaction. But `psbtbumpfee` returns a psbt, so it is fine to not be able to sign.

  In order to correctly estimate the size of the inputs for coin selection, the fee bumper will use the size of the inputs of the transaction being bumped. Because the sizes of signatures are not guaranteed, for external inputs, the fee bumper will verify the scripts with a special SignatureChecker which will compute the weight of all of the signatures in that input, and compute their weights if those signatures were maximally sized. This allows the fee bumper to obtain a max size estimate for each external input.

  Builds on #23201 as it relies on the ability to pass weights in to coin selection.

  Closes #23189

ACKs for top commit:
  ishaanam:
    reACK c3b099ace0
  t-bast:
    Re-ran my tests agains c3b099ace0, ACK

Tree-SHA512: 40016ec52d351430977579cfa2694c7e6764f42c9ce09d3a6f1753b767f86053f296d9de988248df033be6d725d67badbf2a5ef82c8ace23c61487729b7691e5
2022-08-22 10:12:19 +01:00
fanquake
0f35f4ddf4
Merge bitcoin/bitcoin#25786: refactor: Make adjusted time type safe
eeee5ada23 Make adjusted time type safe (MacroFake)
fa3be799fe Add time helpers (MacroFake)

Pull request description:

  This makes follow-ups easier to review. Also, it makes sense by itself.

ACKs for top commit:
  ryanofsky:
    Code review ACK eeee5ada23. Confirmed type changes and equivalent code changes only.

Tree-SHA512: 51bf1ae5428552177286113babdd49e82459d6c710a07b6e80a0a045d373cf51045ee010461aba98e0151d8d71b9b3b5f8f73e302d46ba4558e0b55201f99e9f
2022-08-22 10:00:46 +01:00
w0xlt
57d1367fec test: remove unused norm_prv parameter 2022-08-21 18:26:11 -03:00
Sebastian Falbesoner
706c8e0969 refactor: use strprintf for creating unknown-service-flag string
No need to use a stringstream here.
2022-08-20 15:11:28 +02:00
Andrew Chow
c3b099ace0 wallet, tests: Test bumpfee's max input weight calculation 2022-08-19 14:37:36 -04:00
Andrew Chow
116a620ce7 Make DUMMY_CHECKER availble outside of script/sign.cpp 2022-08-19 14:37:36 -04:00
MacroFake
faf8da3c8d
Remove Join() helper only used in tests
Also remove redundant return type that can be deduced by the compiler.
2022-08-19 19:44:16 +02:00
Andrew Chow
6b56873b41
Merge bitcoin/bitcoin#25784: Wallet: Document expectations for AddWalletFlags (now InitWalletFlags) correctly
0cb6d2aec6 Bugfix: Wallet: Document expectations for AddWalletFlags (now InitWalletFlags) correctly (Luke Dashjr)

Pull request description:

  Includes some slight refactoring (return type changed, current status checked)

ACKs for top commit:
  achow101:
    ACK 0cb6d2aec6
  w0xlt:
    ACK 0cb6d2aec6
  ryanofsky:
    Code review ACK 0cb6d2aec6. This is a clarifying change, and should prevent the InitWalletFlags method being called incorrectly. I left a comment suggestion, but feel free to ignore it.

Tree-SHA512: fa18e9471b5e89d35cbc01526e6d4dbe4eee8faa9646847248909af1751b33014a6f9a42fe70a1331c0d73adea79008b8fc3ae2b51a641eba3e36d5c631327f6
2022-08-19 12:12:27 -04:00
Andrew Chow
0f0508bc72
Merge bitcoin/bitcoin#25869: wallet: remove UNKNOWN type from OUTPUT_TYPES array
5b4fdbbff5 wallet: remove UNKNOWN type from OUTPUT_TYPES array (furszy)

Pull request description:

  Fixing https://github.com/bitcoin/bitcoin/pull/25734#discussion_r949502998 ->  https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=50329

  The `OUTPUT_TYPES` array contain the known active output types only.
  And it's solely used to create/walk-through the active spkms.

  So, no need to add the `UNKNOWN` type here.

ACKs for top commit:
  achow101:
    ACK 5b4fdbbff5
  w0xlt:
    ACK 5b4fdbbff5
  LarryRuane:
    ACK 5b4fdbbff5

Tree-SHA512: dee2dc362a1b0c777555e5ee4d355a3351340591d0096f74e8c3a25f374cb2d9aef26145977ff4dd0f8cc940da9464eb5541eb2895bc19f8cbd6bb6d292ab9a9
2022-08-19 12:01:23 -04:00
Andrew Chow
1bc8106d4c bumpfee: be able to bump fee of a tx with external inputs
In some cases, notably psbtbumpfee, it is okay, and potentially desired,
to be able to bump the fee of a transaction which contains external
inputs.
2022-08-19 11:27:01 -04:00
Andrew Chow
31dd3dc9e5 bumpfee: Clear scriptSigs and scriptWitnesses before calculated max size
The max size calculation expects some inputs to have empty scriptSigs
and witnesses, so we need to clear these before doing that calculation.
2022-08-19 11:27:01 -04:00
Andrew Chow
a0c3afb898 bumpfee: extract weights of external inputs when bumping fee
When bumping the fee of a transaction containing external inputs,
determine the weights of those inputs. Because signatures can have a
variable size, the script is executed with a special SignatureChecker
which will compute the total weight of the signatures in the transaction
and the weight if they were all maximum size signatures. This allows us
to compute the maximum weight of the input for use during coin
selection.
2022-08-19 11:27:01 -04:00
Andrew Chow
612f1e44fe bumpfee: Calculate fee by looking up UTXOs
Instead of calculating the fee by using what is stored in the wallet,
calculate it by looking up the UTXOs.
2022-08-19 11:27:01 -04:00
MacroFake
9eaef10801
Merge bitcoin/bitcoin#25707: refactor: Make const references to avoid unnecessarily copying objects and enable two clang-tidy checks
ae7ae36d31 tidy: Enable two clang-tidy checks (Aurèle Oulès)
081b0e53e3 refactor: Make const refs vars where applicable (Aurèle Oulès)

Pull request description:

  I added const references to some variables to avoid unnecessarily copying objects.

  Also added two clang-tidy checks : [performance-for-range-copy](https://releases.llvm.org/11.1.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-for-range-copy.html) and [performance-unnecessary-copy-initialization](https://releases.llvm.org/12.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.html).

ACKs for top commit:
  vasild:
    ACK ae7ae36d31
  MarcoFalke:
    review ACK ae7ae36d31

Tree-SHA512: f6ac6b0cd0eee1e0c34d2f186484bc0f7ec6071451cccb33fa88a67d93d92b304e2fac378b88f087e94657745bca4e966dbc443759587400eb01b1f3061fde8c
2022-08-19 17:11:06 +02:00
MacroFake
d480586ecb
Merge bitcoin/bitcoin#25808: fs: work around u8path deprecated-declaration warnings with libc++
ced00f5a2e fs: work around u8path deprecated-declaration warnings with libc++ (fanquake)

Pull request description:

  When building in c++20 mode using libc++, the following warning is emitted:
  ```bash
  ./fs.h:72:29: warning: 'u8path<std::string>' is deprecated [-Wdeprecated-declarations]
      return std::filesystem::u8path(utf8_str);
                              ^
  /usr/lib/llvm-14/bin/../include/c++/v1/__filesystem/u8path.h:72:27: note: 'u8path<std::string>' has been explicitly marked deprecated here
  _LIBCPP_INLINE_VISIBILITY _LIBCPP_DEPRECATED_WITH_CHAR8_T
                            ^
  /usr/lib/llvm-14/bin/../include/c++/v1/__config:1042:43: note: expanded from macro '_LIBCPP_DEPRECATED_WITH_CHAR8_T'
                                            ^
  /usr/lib/llvm-14/bin/../include/c++/v1/__config:1007:48: note: expanded from macro '_LIBCPP_DEPRECATED'
                                                 ^
  1 warning generated.
  ```

  as [`u8path<std::string>`](https://en.cppreference.com/w/cpp/filesystem/path/u8path) is deprecated starting with C++20.

  Fixes: #24682.

ACKs for top commit:
  MarcoFalke:
    review ACK ced00f5a2e
  hebasto:
    ACK ced00f5a2e

Tree-SHA512: f012c4f0bec691090eb3ff128ee0cdc392f73e7857b97131da924ab18c088a82d2fba95316d405feb8b744cba63bfeff7b08143086c173fddbf972139ea0ac0b
2022-08-19 13:50:24 +02:00
fanquake
ced00f5a2e
fs: work around u8path deprecated-declaration warnings with libc++
When building in c++20 mode using libc++, the following warning is
emitted:
```bash
./fs.h:72:29: warning: 'u8path<std::string>' is deprecated [-Wdeprecated-declarations]
    return std::filesystem::u8path(utf8_str);
                            ^
/usr/lib/llvm-14/bin/../include/c++/v1/__filesystem/u8path.h:72:27: note: 'u8path<std::string>' has been explicitly marked deprecated here
_LIBCPP_INLINE_VISIBILITY _LIBCPP_DEPRECATED_WITH_CHAR8_T
                          ^
/usr/lib/llvm-14/bin/../include/c++/v1/__config:1042:43: note: expanded from macro '_LIBCPP_DEPRECATED_WITH_CHAR8_T'
                                          ^
/usr/lib/llvm-14/bin/../include/c++/v1/__config:1007:48: note: expanded from macro '_LIBCPP_DEPRECATED'
                                               ^
1 warning generated.
```

as u8path<std::string> is deprecated starting with c++20.

Fixes: #24682.

Co-authored-by: MacroFake <falke.marco@gmail.com>
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2022-08-19 08:58:56 +01:00
fanquake
0425ce577f
Merge bitcoin/bitcoin#25679: wallet: Correctly identify external inputs that are also in the wallet
ef8e2a5b09 tests: Test that external inputs of txs in wallet is handled correctly (Andrew Chow)
eb879634db wallet: Try estimating input size with external data if wallet fails (Andrew Chow)
a537d7aaa0 wallet: SelectExternal actually external inputs (Andrew Chow)
f2d00bfe1a wallet: Add CWallet::IsMine(COutPoint) (Andrew Chow)

Pull request description:

  if a transaction is being funded that has an external input, and that input's parent is also in the wallet, we will fail to detect that and fail to fund the transaction. In order to correctly detect such inputs, we need to be doing `IsMine` on all specified inputs in order to use `Select` and `SelectExternal` correctly. Additionally `SelectCoins` needs to call `CalculateMaximumSignedInputSize` with the correct parameters which depends on whether the wallet is able to solve for the input. Because there are some situations where the wallet could find an external input to belong to it (e.g. watching an address - unable to solve, but will be ISMINE_WATCHONLY), instead of switching which `CalculateMaximumSignedInputSize` to use, we should call the one that uses the wallet, and if that fails, try again with the one that uses external solving data.

  Also adds a test for this case.

ACKs for top commit:
  instagibbs:
    ACK ef8e2a5b09
  furszy:
    ACK ef8e2a5b
  ishaanam:
    reACK ef8e2a5b09

Tree-SHA512: a43c4aefeed4605f33a36ce87ebb916e2c153fea6d415b02c9a89275e84a7e3bf12840b33c296d2d2bde46350390da48d9262f9567338e3f21d5936aae4caa1e
2022-08-19 08:53:44 +01:00
Andrew Chow
888628cee0
Merge bitcoin/bitcoin#25827: descriptor: check if rawtr has only one key.
416ceb8661 descriptor: check if `rawtr` has only one key. (w0xlt)

Pull request description:

  If I understand `rawtr` descriptor correctly, it should only allow `rawtr(KEY)`, not `rawtr(KEY1, KEY2, ...)` or other concatenations.

  On master branch, `rawtr(KEY1, KEY2, ...)` will produce the `rawtr(KEY1)` descriptor ignoring the `KEY2, ...` with no error messages or warnings.

  For example, the code below will print `rawtr(tprv8ZgxMBicQKsPefef2Doobbq3xTCaVTHcDn6me82KSXY1vY9AJAWD5u7SDM4XGLfc4EoXRMFrJKpp6HNmQWA3FTMRQeEmMJYJ9RPqe9ne2hU/*)#lx9qryfh`
  for the supposedly invalid descriptor
  `rawtr(tprv8ZgxMBicQKsPefef2Doobbq3xTCaVTHcDn6me82KSXY1vY9AJAWD5u7SDM4XGLfc4EoXRMFrJKpp6HNmQWA3FTMRQeEmMJYJ9RPqe9ne2hU/*, tprv8ZgxMBicQKsPezQ2KGArMRovTEbCGxaLgBgaVcTvEx8mby8ogX2bgC4HBapH4yMwrz2FpoCuA17eocuUVMgEP6fnm83YpwSDTFrumw42bny/*)`
  ```python
          self.nodes[1].createwallet(wallet_name="rawtr_multi", descriptors=True, blank=True)
          rawtr_multi = self.nodes[1].get_wallet_rpc("rawtr_multi")
          rawtr_multi_desc = "rawtr(tprv8ZgxMBicQKsPefef2Doobbq3xTCaVTHcDn6me82KSXY1vY9AJAWD5u7SDM4XGLfc4EoXRMFrJKpp6HNmQWA3FTMRQeEmMJYJ9RPqe9ne2hU/*, tprv8ZgxMBicQKsPezQ2KGArMRovTEbCGxaLgBgaVcTvEx8mby8ogX2bgC4HBapH4yMwrz2FpoCuA17eocuUVMgEP6fnm83YpwSDTFrumw42bny/*)#uv78hkt0"
          result = rawtr_multi.importdescriptors([{"desc": rawtr_multi_desc, "active": True, "timestamp": "now"}])

          print(rawtr_multi.listdescriptors(True))
  ```

  This PR adds a check that prevents `rawtr` descriptors from being created if more than one key is entered, shows an error message, and adds a test for this case.

ACKs for top commit:
  achow101:
    ACK 416ceb8661
  sipa:
    ACK 416ceb8661

Tree-SHA512: a2009e91f1bca6ee79cc68f65811caa6a21fc8b80acd8dc58e283f424b41fe53b0db7ce3693b1c7e2184ff571e6d1fbb9f5ccde89b65d3026726f3393c492044
2022-08-18 16:50:43 -04:00
furszy
5b4fdbbff5
wallet: remove UNKNOWN type from OUTPUT_TYPES array
This array contains the known active output types only.
And it's solely used to create/walk-through the active spkms.
2022-08-18 16:47:15 -03:00
Andrew Chow
eb879634db wallet: Try estimating input size with external data if wallet fails
Instead of choosing whether to use the wallet or external data when
estimating the size of an input, first use the wallet, then try external
data if that failed.
2022-08-18 11:00:13 -04:00
Andrew Chow
a537d7aaa0 wallet: SelectExternal actually external inputs
If an external input's utxo was created by a transaction that the wallet
knows about, then it would not be selected using SelectExternal. This
results in either funding failure or incorrect weight calculation.
2022-08-18 11:00:12 -04:00
Andrew Chow
a8f69541ad
Merge bitcoin/bitcoin#25748: refactor: Avoid copies in FlatSigningProvider Merge
fa3f15f2dd refactor: Avoid copies in FlatSigningProvider Merge (MacroFake)

Pull request description:

  `Merge` will create several copies unconditionally:
  * To initialize the args `a`, and `b`
  * `ret`, which is the merge of the two args

  So change the code to let the caller decide how many copies they need/want:
  * `a`, and `b` must be explicitly moved or copied by the caller
  * `ret` is no longer needed, as `a` can be used for it in place "for free"

ACKs for top commit:
  achow101:
    ACK fa3f15f2dd
  furszy:
    looks good, ACK fa3f15f2
  ryanofsky:
    Code review ACK fa3f15f2dd. Confirmed that all the places `std::move` was added the argument actually did seem safe to move from. Compiler enforces that temporary copies are explicitly created in non-move cases.

Tree-SHA512: 7c027ccdea1549cd9f37403344ecbb76e008adf545f6ce52996bf95e89eb7dc89af6cb31435a9289d6f2eea1c416961b2fb96348bc8a211d550728f1d99ac49c
2022-08-17 17:57:33 -04:00
w0xlt
416ceb8661 descriptor: check if rawtr has only one key. 2022-08-17 13:54:51 -03:00
fanquake
a75b7796b7
Merge bitcoin/bitcoin#25077: Fix chain tip data race and corrupt rest response
fac04cb6ba refactor: Add lock annotations to Active* methods (MacroFake)
fac15ff673 Fix logical race in rest_getutxos (MacroFake)
fa97a528d6 Fix UB/data-race in RPCNotifyBlockChange (MacroFake)
fa530bcb9c Add ChainstateManager::GetMutex(), an alias for ::cs_main (MacroFake)

Pull request description:

  This fixes two issues:

  * A data race in `ActiveChain`, which returns a reference to the chain (a `std::vector`), which is not thread safe. See also below traceback.
  * A corrupt rest response, which returns a blockheight and blockhash, which are unrelated to each other and to the result, as the chain might advance between each call without cs_main held.

  The issues are fixed by taking cs_main and holding it for the required time.

  ```
  ==================
  WARNING: ThreadSanitizer: data race (pid=32335)
    Write of size 8 at 0x7b3c000008f0 by thread T22 (mutexes: write M131626, write M151, write M131553):
      #0 std::__1::enable_if<(is_move_constructible<CBlockIndex**>::value) && (is_move_assignable<CBlockIndex**>::value), void>::type std::__1::swap<CBlockIndex**>(CBlockIndex**&, CBlockIndex**&) /usr/lib/llvm-13/bin/../include/c++/v1/__utility/swap.h:39:7 (bitcoind+0x501239)
      #1 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::__swap_out_circular_buffer(std::__1::__split_buffer<CBlockIndex*, std::__1::allocator<CBlockIndex*>&>&) /usr/lib/llvm-13/bin/../include/c++/v1/vector:977:5 (bitcoind+0x501239)
      #2 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::__append(unsigned long) /usr/lib/llvm-13/bin/../include/c++/v1/vector:1117:9 (bitcoind+0x501239)
      #3 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::resize(unsigned long) /usr/lib/llvm-13/bin/../include/c++/v1/vector:2046:15 (bitcoind+0x4ffe29)
      #4 CChain::SetTip(CBlockIndex*) src/chain.cpp:19:12 (bitcoind+0x4ffe29)
      #5 CChainState::ConnectTip(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, ConnectTrace&, DisconnectedBlockTransactions&) src/validation.cpp:2748:13 (bitcoind+0x475d00)
      #6 CChainState::ActivateBestChainStep(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, bool&, ConnectTrace&) src/validation.cpp:2884:18 (bitcoind+0x47739e)
      #7 CChainState::ActivateBestChain(BlockValidationState&, std::__1::shared_ptr<CBlock const>) src/validation.cpp:3011:22 (bitcoind+0x477baf)
      #8 node::ThreadImport(ChainstateManager&, std::__1::vector<fs::path, std::__1::allocator<fs::path> >, ArgsManager const&) src/node/blockstorage.cpp:883:30 (bitcoind+0x23cd74)
      #9 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7::operator()() const src/init.cpp:1657:9 (bitcoind+0x15863e)
      #10 decltype(static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(fp)()) std::__1::__invoke<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x15863e)
      #11 void std::__1::__invoke_void_return_wrapper<void, true>::__call<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (bitcoind+0x15863e)
      #12 std::__1::__function::__alloc_func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (bitcoind+0x15863e)
      #13 std::__1::__function::__func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (bitcoind+0x15863e)
      #14 std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (bitcoind+0x88891f)
      #15 std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (bitcoind+0x88891f)
      #16 util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (bitcoind+0x88891f)
      #17 decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x157e6a)
      #18 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (bitcoind+0x157e6a)
      #19 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (bitcoind+0x157e6a)
    Previous read of size 8 at 0x7b3c000008f0 by main thread:
      #0 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::size() const /usr/lib/llvm-13/bin/../include/c++/v1/vector:680:61 (bitcoind+0x15179d)
      #1 CChain::Tip() const src/./chain.h:449:23 (bitcoind+0x15179d)
      #2 ChainstateManager::ActiveTip() const src/./validation.h:927:59 (bitcoind+0x15179d)
      #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1841:35 (bitcoind+0x15179d)
      #4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2)
      #5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2)
    Location is heap block of size 232 at 0x7b3c00000870 allocated by main thread:
      #0 operator new(unsigned long) <null> (bitcoind+0x132668)
      #1 ChainstateManager::InitializeChainstate(CTxMemPool*, std::__1::optional<uint256> const&) src/validation.cpp:4851:21 (bitcoind+0x48e26b)
      #2 node::LoadChainstate(bool, ChainstateManager&, CTxMemPool*, bool, Consensus::Params const&, bool, long, long, long, bool, bool, std::__1::function<bool ()>, std::__1::function<void ()>) src/node/chainstate.cpp:31:14 (bitcoind+0x24de07)
      #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1438:32 (bitcoind+0x14e994)
      #4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2)
      #5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2)
    Mutex M131626 (0x7b3c00000898) created at:
      #0 pthread_mutex_lock <null> (bitcoind+0xda898)
      #1 std::__1::mutex::lock() <null> (libc++.so.1+0x49f35)
      #2 node::ThreadImport(ChainstateManager&, std::__1::vector<fs::path, std::__1::allocator<fs::path> >, ArgsManager const&) src/node/blockstorage.cpp:883:30 (bitcoind+0x23cd74)
      #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7::operator()() const src/init.cpp:1657:9 (bitcoind+0x15863e)
      #4 decltype(static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(fp)()) std::__1::__invoke<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x15863e)
      #5 void std::__1::__invoke_void_return_wrapper<void, true>::__call<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (bitcoind+0x15863e)
      #6 std::__1::__function::__alloc_func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (bitcoind+0x15863e)
      #7 std::__1::__function::__func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (bitcoind+0x15863e)
      #8 std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (bitcoind+0x88891f)
      #9 std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (bitcoind+0x88891f)
      #10 util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (bitcoind+0x88891f)
      #11 decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x157e6a)
      #12 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (bitcoind+0x157e6a)
      #13 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (bitcoind+0x157e6a)
    Mutex M151 (0x55aacb8ea030) created at:
      #0 pthread_mutex_init <null> (bitcoind+0xbed2f)
      #1 std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3)
      #2 __libc_start_main <null> (libc.so.6+0x29eba)
    Mutex M131553 (0x7b4c000042e0) created at:
      #0 pthread_mutex_init <null> (bitcoind+0xbed2f)
      #1 std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3)
      #2 std::__1::__unique_if<CTxMemPool>::__unique_single std::__1::make_unique<CTxMemPool, CBlockPolicyEstimator*, int const&>(CBlockPolicyEstimator*&&, int const&) /usr/lib/llvm-13/bin/../include/c++/v1/__memory/unique_ptr.h:728:32 (bitcoind+0x15c81d)
      #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1426:24 (bitcoind+0x14e7b4)
      #4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2)
      #5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2)
    Thread T22 'b-loadblk' (tid=32370, running) created by main thread at:
      #0 pthread_create <null> (bitcoind+0xbd5bd)
      #1 std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-13/bin/../include/c++/v1/__threading_support:443:10 (bitcoind+0x155e06)
      #2 std::__1:🧵:thread<void (*)(char const*, std::__1::function<void ()>), char const (&) [8], AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, void>(void (*&&)(char const*, std::__1::function<void ()>), char const (&) [8], AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/thread:307:16 (bitcoind+0x155e06)
      #3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1656:29 (bitcoind+0x150164)
      #4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2)
      #5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2)
  SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-13/bin/../include/c++/v1/__utility/swap.h:39:7 in std::__1::enable_if<(is_move_constructible<CBlockIndex**>::value) && (is_move_assignable<CBlockIndex**>::value), void>::type std::__1::swap<CBlockIndex**>(CBlockIndex**&, CBlockIndex**&)
  ==================
  ```

  From https://cirrus-ci.com/task/5612886578954240?logs=ci#L4868

ACKs for top commit:
  achow101:
    re-ACK fac04cb6ba
  theStack:
    Code-review ACK fac04cb6ba

Tree-SHA512: 9d619f99ff6373874c7ffe1db20674575605646b4b54b692fb54515a4a49f110a770026d7320ed6dfeaa7976be4cd89e93f821acdbf22c7662bd1c5be0cedcd2
2022-08-17 15:04:14 +01:00
fanquake
0ae0aa251b
Merge bitcoin/bitcoin#24678: Prevent wallet unload on GetWalletForJSONRPCRequest
f59959e381 wallet: Prevent wallet unload on GetWalletForJSONRPCRequest (João Barbosa)

Pull request description:

  Don't extend shared ownership of all wallets to `GetWalletForJSONRPCRequest` scope.

ACKs for top commit:
  achow101:
    ACK f59959e381
  shaavan:
    Code Review ACK f59959e381
  theStack:
    Concept and code-review ACK f59959e381

Tree-SHA512: 7c0294098b5c32acaab8cc6fcf17a581d580ad1a557ba0602a9506074ac035815739afb4a25b3e61be9132535c7fc3ec7ef5137c1dfc9d4078f13663d508ef55
2022-08-17 14:39:50 +01:00
fanquake
95d4744f02
Merge bitcoin/bitcoin#19391: RPC/Mining: Clean out pre-Segwit miner compatibility code
90a5dfa509 RPC/Mining: Clean out pre-Segwit miner compatibility code (Luke Dashjr)

Pull request description:

  This is dead code post-Segwit.

ACKs for top commit:
  achow101:
    ACK 90a5dfa509

Tree-SHA512: 5970aa3548d2a7da7c6e83fb9b910529faab10251b115122cec833bb7d3a54c7cb0714c1a873807be04c7817bb827c7ece1e20e8fa4c907aa58688487d0ec44d
2022-08-17 14:07:41 +01:00
Andrew Chow
f2d00bfe1a wallet: Add CWallet::IsMine(COutPoint)
It is useful to have an IsMine function that can take an outpoint.
2022-08-16 20:17:01 -04:00
Andrew Chow
64f7a1940d
Merge bitcoin/bitcoin#25734: wallet, refactor: #24584 follow-ups
8cd21bb279 refactor: improve readability for AttemptSelection (josibake)
f47ff71761 test: only run test for descriptor wallets (josibake)
0760ce0b9e test: add missing BOOST_ASSERT (josibake)
db09aec937 wallet: switch to new shuffle, erase, push_back (josibake)
b6b50b0f2b scripted-diff: Uppercase function names (josibake)
3f27a2adce refactor: add new helper methods (josibake)
f5649db9d5 refactor: add UNKNOWN OutputType (josibake)

Pull request description:

  This PR is to address follow-ups for #24584, specifically:

  * Remove redundant, hard-to-read code by adding a new `OutputType` and adding shuffle, erase, and push_back methods for `CoinsResult`
  * Add missing `BOOST_ASSERT` to unit test
  * Ensure functional test only runs if using descriptor wallets
  * Improve readability of `AttemptSelection` by removing triple-nested if statement

  Note for reviewers: commit `refactor: add new helper methods` should throw an "unused function warning"; the function is used in the next commit. Also, commit `wallet: switch to new shuffle, erase, push_back` will fail to compile, but this is fixed in the next commit with a scripted-diff. the commits are separate like this (code change then scripted-diff) to improve legibility.

ACKs for top commit:
  achow101:
    ACK 8cd21bb279
  aureleoules:
    ACK 8cd21bb279.
  LarryRuane:
    Concept, code review ACK 8cd21bb279
  furszy:
    utACK 8cd21bb2. Left a small, non-blocking, comment.

Tree-SHA512: a1bbc5962833e3df4f01a4895d8bd748cc4c608c3f296fd94e8afd8797b8d2e94e7bd44d598bd76fa5c9f5536864f396fcd097348fa0bb190a49a86b0917d60e
2022-08-16 20:00:19 -04:00
Andrew Chow
c336f813b3
Merge bitcoin/bitcoin#25504: RPC: allow to track coins by parent descriptors
a6b0c1fcc0 doc: add releases notes for 25504 (listsinceblock updates) (Antoine Poinsot)
0fd2d14454 rpc: add an include_change parameter to listsinceblock (Antoine Poinsot)
55f98d087e rpc: output parent wallet descriptors for coins in listunspent (Antoine Poinsot)
b724476158 rpc: output wallet descriptors for received entries in listsinceblock (Antoine Poinsot)
55a82eaf91 wallet: allow to fetch the wallet descriptors for a given Script (Antoine Poinsot)

Pull request description:

  Wallet descriptors are useful for applications using the Bitcoin Core wallet as a backend for tracking coins, as they allow to track coins for multiple descriptors in a single wallet. However there is no information currently given for such applications to link a coin with an imported descriptor, severely limiting the possibilities for such applications of using multiple descriptors in a single wallet. This PR outputs the matching imported descriptor(s) for a given received coin in `listsinceblock` (and friends).

  It comes from a need for an application i'm working on, but i think it's something any software using `bitcoind` to track multiple descriptors in a single wallet would have eventually. For instance i'm thinking about the BDK project. Currently, the way to achieve this is to import raw addresses with labels and to have your application be responsible for wallet things like the gap limit.

  I'll add this to the output of `listunspent` too if this gets a few Concept ACKs.

ACKs for top commit:
  instagibbs:
    ACK a6b0c1fcc0
  achow101:
    re-ACK a6b0c1fcc0

Tree-SHA512: 7a5850e8de98b439ddede2cb72de0208944f8cda67272e8b8037678738d55b7a5272375be808b0f7d15def4904430e089dafdcc037436858ff3292c5f8b75e37
2022-08-16 13:08:05 -04:00
Antoine Poinsot
0fd2d14454
rpc: add an include_change parameter to listsinceblock
It's useful for an external application tracking coins to not be limited
by our change detection. For instance, for a watchonly wallet with two
descriptors a transaction from one to the other would be considered a
change output and not be included in the result (if the address was not
generated by this wallet).
2022-08-16 18:33:05 +02:00
MacroFake
fac04cb6ba
refactor: Add lock annotations to Active* methods
This is a refactor, putting the burden to think about thread safety to
the caller. Otherwise, there is a risk that the caller will assume
thread safety where none exists, as is evident in the previous two
commits.
2022-08-16 17:26:40 +02:00
MacroFake
fac15ff673
Fix logical race in rest_getutxos
Calling ActiveHeight() and ActiveTip() subsequently without holding the
::cs_main lock over both calls may result in a height that does not
correspond to the tip due to a race.

Fix this by holding the lock.
2022-08-16 17:26:28 +02:00
MacroFake
fa97a528d6
Fix UB/data-race in RPCNotifyBlockChange
ActiveTip() is *not* thread-safe, as the required ::cs_main lock will be
released as ActiveChainstate() returns.

ActiveTip() is an alias for ActiveChainstate().m_chain.Tip(), so m_chain
may be involved in a data-race (UB).
2022-08-16 17:25:49 +02:00
MacroFake
fa530bcb9c
Add ChainstateManager::GetMutex(), an alias for ::cs_main 2022-08-16 17:25:19 +02:00
fanquake
cf39913e57
Merge bitcoin/bitcoin#25803: refactor: Drop boost/algorithm/string/replace.hpp dependency
fea75ad3ca refactor: Drop `boost/algorithm/string/replace.hpp` dependency (Hennadii Stepanov)
857526e8cb test: Add test case for `ReplaceAll()` function (Hennadii Stepanov)

Pull request description:

  A new implementation of the `ReplaceAll()` seems enough for all of our purposes.

ACKs for top commit:
  adam2k:
    ACK Tested fea75ad3ca
  theStack:
    Code-review ACK fea75ad3ca

Tree-SHA512: dacfffc9d2bd1fb9f034baf8c045b1e8657b766db2f0a7f8ef7e25ee6cd888f315b0124c54aba7a29ae59186b176ef9868a8b709dc995ea215c6b4ce58e174d9
2022-08-16 09:19:28 +01:00