3318 Commits

Author SHA1 Message Date
Ryan Ofsky
d216bc8d76 Re-enable walletinit_verify_walletdir_no_trailing2 test disabled in #20744
This should also fix an init error if a -walletdir with a trailing slash
is used on windows. This appears to be a real error and regression
introduced with #20744.

On windows (or at least wine), fs calls that actuallly access the
filesystem like fs::equivalent or fs::exists seem to treat directory
paths with trailing slashes as not existing, so it's necessary to
normalize these paths before using them. This change passes canonical
paths to fs calls validating the -walletdir path to fix this.
2022-02-04 09:10:19 -05:00
Kiminuo
41d7166c8a
refactor: replace boost::filesystem with std::filesystem
Warning: Replacing fs::system_complete calls with fs::absolute calls
in this commit may cause minor changes in behaviour because fs::absolute
no longer strips trailing slashes; however these changes are believed to
be safe.

Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2022-02-03 18:35:52 +08:00
Andrew Chow
02e1d8d06f
Merge bitcoin/bitcoin#24083: Revert "Add to spends only transcations from me"
3ee6d0788ec1b90f7c39c9644dba4011f7cf5db4 test: add more wallet conflicts assertions (S3RK)
3b98bf9c43ece060d57d7ae31624d4a8220de266 Revert "Add to spends only transcations from me" (S3RK)

Pull request description:

  This reverts commit d04566415e16ae685af066384f346dff522c068f from #22929.

  This commit was based on invalid assumption that `mapTxSpends` should contain only outgoing txs and broke wallet conflicts feature.

ACKs for top commit:
  achow101:
    ACK 3ee6d0788ec1b90f7c39c9644dba4011f7cf5db4

Tree-SHA512: bf5a77ced6bac57d5eb85771d9189c53e1edc295d179ed5a1bdce18e365794a9101b4cecf35387b27f67260db3b47f7214e7876e490494529b748cceeb95632d
2022-02-01 14:46:11 -05:00
MarcoFalke
5f4c07b799
Merge bitcoin/bitcoin#24136: Extract CTxIn::MAX_SEQUENCE_NONFINAL constant, rework BIP 65/68/112 docs
fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88 Extract CTxIn::MAX_SEQUENCE_NONFINAL constant (MarcoFalke)

Pull request description:

  Extracting the constant makes it possible to attach documentation to it.

  Also, rework the docs for the other "sequence constants".

ACKs for top commit:
  w0xlt:
    reACK fa4339e for specifying the transaction version.
  darosior:
    re-ACK fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88
  luke-jr:
    crACK fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88

Tree-SHA512: 8d8f3dd5afb33eb5b72aa558e1e03de874c5ed02aa1084888e92ed86f3aaa5c725db45ded02e14cdfa67a92ac6774e97185b697f20a8ab63abbfcaa2fcd1fc6a
2022-01-31 09:40:33 +01:00
laanwj
196b459920
Merge bitcoin/bitcoin#23438: refactor: Use spans of std::byte in serialize
fa5d2e678c809c26bd40d7e7c171529d3ffb5903 Remove unused char serialize (MarcoFalke)
fa24493d6394b3a477535f480664c9596f18e3c5 Use spans of std::byte in serialize (MarcoFalke)
fa65bbf217b725ada35107b4ad646d250228355c span: Add BytePtr helper (MarcoFalke)

Pull request description:

  This changes the serialize code (`.read()` and `.write()` functions) to take a `Span` instead of a pointer and size. This is a breaking change for the serialize interface, so at no additional cost we can also switch to `std::byte` (instead of using `char`).

  The benefits of using `Span`:
  * Less verbose and less fragile code when passing an already existing `Span`(-like) object to or from serialization

  The benefits of using `std::byte`:
  * `std::byte` can't accidentally be mistaken for an integer

  The goal here is to only change serialize to use spans of `std::byte`. If needed, `AsBytes`,  `MakeUCharSpan`, ... can be used (temporarily) to pass spans of the right type.

  Other changes that are included here:

  * [#22167](https://github.com/bitcoin/bitcoin/pull/22167) (refactor: Remove char serialize by MarcoFalke)
  * [#21906](https://github.com/bitcoin/bitcoin/pull/21906) (Preserve const in cast on CTransactionSignatureSerializer by promag)

ACKs for top commit:
  laanwj:
    Concept and code review ACK fa5d2e678c809c26bd40d7e7c171529d3ffb5903
  sipa:
    re-utACK fa5d2e678c809c26bd40d7e7c171529d3ffb5903

Tree-SHA512: 08ee9eced5fb777cedae593b11e33660bed9a3e1711a7451a87b835089a96c99ce0632918bb4666a4e859c4d020f88fb50f2dd734216b0c3d1a9a704967ece6f
2022-01-27 19:19:12 +01:00
laanwj
cf5bb048e8
Merge bitcoin/bitcoin#22932: Add CBlockIndex lock annotations, guard nStatus/nFile/nDataPos/nUndoPos by cs_main
6ea56827842b9b2bd730edc38f3a7b1f46f6247b Guard CBlockIndex::nStatus/nFile/nDataPos/nUndoPos by cs_main (Jon Atack)
5d59ae0ba88849b1eb0d7350871bc19fcd5ef601 Remove/inline ReadRawBlockFromDisk(block_data, pindex, message_start) (Hennadii Stepanov)
eaeeb88768db529b5241ccd42f1e87579908b4df Require IsBlockPruned() to hold mutex cs_main (Jon Atack)
ca47b005770f71aa229ecc1f7b8146a96ff02151 Require CBlockIndex::IsValid() to hold cs_main (Vasil Dimov)
e9f3aa5f6a7b39e8d5f2069617e5e382798d8d60 Require CBlockIndex::RaiseValidity() to hold cs_main (Vasil Dimov)
8ef457cb83fac796f8b6a56977b1016193fc1185 Require CBlockIndex::IsAssumedValid() to hold cs_main (Vasil Dimov)
572393448b4d32f91b92edc84b4200ab52d62422 Require CBlockIndex::GetUndoPos() to hold mutex cs_main (Jon Atack)
2e557ced2830fc54476e598d52225f1679205e7d Require WriteUndoDataForBlock() to hold mutex cs_main (Jon Atack)
6fd4341c10b319399c58d71c4ddeae4417e337d7 Require CBlockIndex::GetBlockPos() to hold mutex cs_main (Jon Atack)

Pull request description:

  Issues:

  - `CBlockIndex` member functions `GetBlockPos()`, `GetUndoPos()`, `IsAssumedValid()`, `RaiseValidity()`, and `IsValid()` and block storage functions `WriteUndoDataForBlock()` and `IsBlockPruned()` are missing thread safety lock annotations to help ensure that they are called with mutex cs_main to avoid bugs like #22895. Doing this also enables the next step:

  - `CBlockIndex::nStatus` may be racy, i.e. potentially accessed by multiple threads, see #17161. A solution is to guard it by cs_main, along with fellow data members `nFile`, `nDataPos` and `nUndoPos`.

  This pull:

  - adds thread safety lock annotations for the functions listed above
  - guards `CBlockIndex::nStatus`, `nFile`, `nDataPos` and `nUndoPos` by cs_main

  How to review and test:
  - debug build with clang and verify there are no `-Wthread-safety-analysis` warnings
  - review the code to verify each annotation or lock is necessary and sensible, or if any are missing
  - look for whether taking a lock can be replaced by a lock annotation instead
  - for more information about Clang thread safety analysis, see
      - https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
      - https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lockingmutex-usage-notes
      - https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads-and-synchronization

  Mitigates/potentially closes #17161.

ACKs for top commit:
  laanwj:
    Code review ACK 6ea56827842b9b2bd730edc38f3a7b1f46f6247b

Tree-SHA512: 3ebf429c8623c51f944a7245a2e48d2aa088dec4c4914b40aa6049e89856c1ee8586f6e2e3b65195190566637a33004468b51a781e61a082248748015167569b
2022-01-27 10:57:33 +01:00
MarcoFalke
fa4339e4c1
Extract CTxIn::MAX_SEQUENCE_NONFINAL constant 2022-01-26 15:10:44 +01:00
Andrew Chow
e30b6ea194
Merge bitcoin/bitcoin#24067: wallet: Actually treat (un)confirmed txs as (un)confirmed
fac816544317cee6553d60cb0f5f24f6f9ec98de Remove unused checkFinalTx (MarcoFalke)
fa272eab44553df9b0bed3ca20caf2a7bd193680 wallet: Avoid dropping confirmed coins (MarcoFalke)
888841ea8d38fc059ca06ef67af7f31f8d8418a4 interfaces: Remove unused is_final (MarcoFalke)
dddd05e7a3389fcbd90bb4acdfe1f59945d9f381 qt: Treat unconfirmed txs as unconfirmed (MarcoFalke)

Pull request description:

  The wallet has several issues:

  ## Unconfirmed txs in the GUI

  The GUI clumsily attempts to guess if unconfirmed txs are locked until a future time. This is currently based on the locktime only, not nSequence, thus wrong. Fix this by removing the clumsy code and treat all unconfirmed txs as unconfirmed. The GUI already prints whether a tx is in the mempool, in which case the user knows that the tx wasn't locked until a future time. If the tx is not in the mempool, it might be better to report the exact reject reason from the mempool instead of using incorrect heuristics.

  ## Confirmed txs in the wallet

  The wallet drops coins that it incorrectly assumes to be locked until a future time, even if they are already confirmed in the chain. This is because the wallet is using the wrong time (adjusted network time) instead of MTP, due to the `-1` default argument of `CheckFinalTx`.

  The issues are fixed in separate commits and there is even a test.

ACKs for top commit:
  achow101:
    ACK fac816544317cee6553d60cb0f5f24f6f9ec98de
  prayank23:
    reACK fac8165443
  glozow:
    code review ACK fac8165443, I understand now how this fixes both issues.

Tree-SHA512: 210afb855f4c6d903fee49eba6b1a9735d699cf0168b669eabb38178e53b3a522258b7cc669f52489c6cd3e38bf358afde12eef3ba2e2f2ffaeb06b8f652ccd0
2022-01-25 16:17:51 -05:00
Jon Atack
6fd4341c10
Require CBlockIndex::GetBlockPos() to hold mutex cs_main 2022-01-25 20:43:12 +01:00
MarcoFalke
fa272eab44 wallet: Avoid dropping confirmed coins 2022-01-25 10:15:12 +01:00
Andrew Chow
6fa762a372 rpc, wallet: Allow users to specify input weights
Coin selection requires knowing the weight of a transaction so that fees
can be estimated. However for external inputs, the weight may not be
avialble, and solving data may not be enough as the input could be one
that we do not support. By allowing users to specify input weights,
those external inputs can be included in the transaction.

Additionally, if the weight for an input is specified, that value will
always be used, regardless of whether the input is in the wallet or
solving data is available. This allows us to account for scenarios where
the wallet may be more conservative and estimate a larger input than may
actually be created.

For example, we assume the maximum DER signature size, but an external
input may be signed by a wallet which does nonce grinding in order to get
a smaller signature. In that case, the user can specify the smaller
input weight to avoid overpaying transaction fees.
2022-01-24 11:29:38 -05:00
Andrew Chow
808068e90e wallet: Allow user specified input size to override
If the user specifies an input size, allow it to override any input size
calculations during coin selection.
2022-01-24 11:23:31 -05:00
Andrew Chow
4060c50d7e wallet: add input weights to CCoinControl
In order to allow coin selection to take weights from the user,
CCoinControl needs to be able to set and get them.
2022-01-24 11:23:31 -05:00
Andrew Chow
dc5d6b0d47 fs: Make compatible with boost 1.78 2022-01-19 16:25:20 -05:00
S3RK
3b98bf9c43 Revert "Add to spends only transcations from me"
This reverts commit d04566415e16ae685af066384f346dff522c068f.
2022-01-17 08:54:08 +01:00
MarcoFalke
888841ea8d
interfaces: Remove unused is_final 2022-01-14 11:28:45 +01:00
MarcoFalke
c561f2f06e
Merge bitcoin/bitcoin#23497: Add src/node/ and src/wallet/ code to node:: and wallet:: namespaces
e5b6aef61221b621ad77b5f075a16897e08835bf Move CBlockFileInfo::ToString method where class is declared (Russell Yanofsky)
f7086fd8ff084ab0dd656d75b7485e59263bdfd8 Add src/wallet/* code to wallet:: namespace (Russell Yanofsky)
90fc8b089d591cabff60ee829a33f96c37fd27ba Add src/node/* code to node:: namespace (Russell Yanofsky)

Pull request description:

  There are no code changes, this is just adding `namespace` and `using` declarations and `node::` or `wallet::` qualifiers in some places.

  Motivations for this change are:

  - To make it easier to see when node and wallet code is being accessed places where it shouldn't be. For example if GUI code is accessing node and wallet internals or if wallet and node code are referencing each other.
  - To make source code organization clearer ([#15732](https://github.com/bitcoin/bitcoin/issues/15732)), being able to know that `wallet::` code is in `src/wallet/`, `node::` code is in `src/node/`, `init::` code is in `src/init/`, `util::` code is in `src/util/`, etc.

  Reviewing with `git log -p -n1 -U0 --word-diff-regex=.` can be helpful to verify this is only updating declarations, not changing code.

ACKs for top commit:
  achow101:
    ACK e5b6aef61221b621ad77b5f075a16897e08835bf
  MarcoFalke:
    Concept ACK e5b6aef61221b621ad77b5f075a16897e08835bf 🍨

Tree-SHA512: 3797745c90246794e2d55a2ee6e8b0ad5c811e4e03a242d3fdfeb68032f8787f0d48ed4097f6b7730f540220c0af99ef423cd9dbe7f76b2ec12e769a757a2c8d
2022-01-11 11:11:00 +01:00
fanquake
fa74718414
Merge bitcoin/bitcoin#24026: Block unsafe std::string fs::path conversion copy_file calls
3a45dc36a663ea67f13e7d5c413518b9385ec594 Change type of `backup_file` parameter in RestoreWallet/restoreWallet (Hennadii Stepanov)
213172c7348091794b173c2edf5f7f3bbe010912 refactor: Block unsafe std::string fs::path conversion copy_file calls (Hennadii Stepanov)

Pull request description:

  This PR is an optional prerequisite for bitcoin/bitcoin#20744 "Use std::filesystem. Remove Boost Filesystem & System" which:
  - makes further code changes safer
  - prevents [some](https://cirrus-ci.com/task/6525835388649472) test failures on native Windows

ACKs for top commit:
  ryanofsky:
    Code review ACK 3a45dc36a663ea67f13e7d5c413518b9385ec594. Looks great! Thanks for debugging and fixing this and making #20744 smaller!

Tree-SHA512: c6dfaef6b45b9c268bc9ee9b943b9d679152c9d565ca4f86da8d33f8eb9b3cdbe9ba6df7b7578eacc0d00db6551048beff97419f86eb4b1d3182c43e2b4eb9a5
2022-01-11 13:54:51 +08:00
Hennadii Stepanov
3a45dc36a6
Change type of backup_file parameter in RestoreWallet/restoreWallet
`fs::path` looks more native than `std::string` for a parameter which
represents a backup file. This change eliminates back-and-forth type
conversions.
2022-01-11 00:00:00 +02:00
Hennadii Stepanov
213172c734
refactor: Block unsafe std::string fs::path conversion copy_file calls
There is no change in behavior. This just helps prepare for the
transition from boost::filesystem to std::filesystem by avoiding
copy_file calls that will be unsafe after the transition to
std::filesystem to due lack of a boost::filesystem::path::imbue
equivalent and inability to set a predictable locale.
2022-01-10 23:48:23 +02:00
Andrew Chow
80ceede7a0
Merge bitcoin/bitcoin#23884: doc: Remove outdated scriptChange TODO comment
fa8e01a5b8c67fcb8acaaf58a17ed24710415b6c doc: Remove outdated scriptChange TODO comment (MarcoFalke)

Pull request description:

  This was added in commit bf798734db4539a39edd6badf54a1c3aecf193e5 (raw multisig). Raw multisig isn't a thing, so remove the TODO.

ACKs for top commit:
  S3RK:
    ACK fa8e01a5b8c67fcb8acaaf58a17ed24710415b6c
  achow101:
    ACK fa8e01a5b8c67fcb8acaaf58a17ed24710415b6c

Tree-SHA512: 01d521ca3605ab130c43531da4922ea85461ca5e7436267a34fb5df348009e086b3c66d85532c62255d9a0ba43db56424884808e773d0ef0177035dfb25d6a6c
2022-01-10 16:46:28 -05:00
fanquake
4ada74206a
Merge bitcoin/bitcoin#23974: Make blockstorage globals private members of BlockManager
fa68a6c2fc6754c160e0f98007785602201b3c47 scripted-diff: Rename touched member variables (MarcoFalke)
facd3df21f344dd84e5f28862056700c1fded17c Make blockstorage globals private members of BlockManager (MarcoFalke)
faa8c2d7d75f8d9782709e73e00e35851e233392 doc: Clarify nPruneAfterHeight for signet (MarcoFalke)
fad381b2f8e1beb18f748fbeb820e63545b9b0fd test: Load genesis block to allow flush (MarcoFalke)
fab262174b96854d2df5bee7da578990c9e9cb1e Move blockstorage-related unload to BlockManager::Unload (MarcoFalke)
fa467f3913918701c765f9bc754203b4591b894f move-only: Create WriteBlockIndexDB helper (MarcoFalke)
fa88cfd3f9896d5b56ea6c111a23f90a79253c18 Move functions to BlockManager (MarcoFalke)

Pull request description:

  Globals aren't too nice because they hide dependencies, also they make testing harder.

  Fix that by removing some.

ACKs for top commit:
  Sjors:
    ACK fa68a6c2fc6754c160e0f98007785602201b3c47
  ryanofsky:
    Code review ACK fa68a6c2fc6754c160e0f98007785602201b3c47. Nice changes!

Tree-SHA512: 6abc5929a5e43a05e238276721d46a64a44f23dca18c2caa9775437a32351d6815d88b88757254686421531d0df13861bbd3a202e13a3192798d87a96abef65d
2022-01-07 11:14:16 +08:00
Russell Yanofsky
f7086fd8ff Add src/wallet/* code to wallet:: namespace 2022-01-06 22:14:16 -05:00
Russell Yanofsky
90fc8b089d Add src/node/* code to node:: namespace 2022-01-06 22:14:16 -05:00
laanwj
801aaac2b3
Merge bitcoin/bitcoin#23834: wallettool: Check that the dumpfile checksum is the correct size
ac617cc141fe05bea0dc5e8f9df3da43c0945842 wallettool: Check that the dumpfile checksum is the correct size (Andrew Chow)

Pull request description:

  After parsing the checksum, make sure that it is the size that we expect it to be.

  This issue was reported by Pedro Baptista.

ACKs for top commit:
  laanwj:
    Code review ACK ac617cc141fe05bea0dc5e8f9df3da43c0945842

Tree-SHA512: 8135b3fb1f4f6b6c91cfbac7d1d3421f1f6c664a742c92940f68eae857f92ce49d042cc3aa5c2df6ef182825271483d65efc7543ec7a8ff047fd7c08666c8899
2022-01-05 19:16:28 +01:00
MarcoFalke
fa88cfd3f9
Move functions to BlockManager
Needed for a later commit
2022-01-05 15:07:28 +01:00
MarcoFalke
fa24493d63
Use spans of std::byte in serialize
This switches .read() and .write() to take spans of bytes.
2022-01-02 11:40:31 +01:00
Hennadii Stepanov
f47dda2c58
scripted-diff: Bump copyright headers
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-

Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2021-12-30 19:36:57 +02:00
fanquake
5d2344885b
Merge bitcoin/bitcoin#23840: doc: Fix -changetype help text
fa993d0e7e4d27ca1cf7d88a5a6ec5b93b7ed503 doc: Fix -changetype help text (MarcoFalke)

Pull request description:

  This was forgotten in commit 3ac38058ce0e80a9f4276bfa82951decdb237e9a

ACKs for top commit:
  shaavan:
    ACK fa993d0e7e4d27ca1cf7d88a5a6ec5b93b7ed503
  w0xlt:
    ACK fa993d0
  josibake:
    ACK fa993d0e7e

Tree-SHA512: 9f84b1168e3b3ab06e5c1f4915a1874598b273099eb5878ed28c3a66f1484e34c836fd3c68c4131bee541f3428052f6b66e02b192170752d1082de689d44cd4d
2021-12-28 20:23:31 +08:00
MarcoFalke
fa8e01a5b8
doc: Remove outdated scriptChange TODO comment 2021-12-27 19:35:22 +01:00
MarcoFalke
fa562fdd5e
doc: Remove fixed TODO from wallet/feebumper
Fixed in commit 9522b53a91f28032c34b94662d50b000534708ce
2021-12-27 19:26:33 +01:00
Andrew Chow
ac617cc141 wallettool: Check that the dumpfile checksum is the correct size
After parsing the checksum, make sure that it is the size that we expect
it to be.
2021-12-26 12:33:23 -05:00
Russell Yanofsky
ff5f6dea53 scripted-diff: Rename interfaces::WalletClient to interfaces::WalletLoader
Name has been confusing since it was introduced, and it was pointed in
recent review club as https://bitcoincore.reviews/10102 that it was
particularly unclear how interfaces::WalletClient was different from
interfaces::Wallet.

-BEGIN VERIFY SCRIPT-
ren() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }
ren WalletClient WalletLoader
ren walletClient walletLoader
ren wallet_client wallet_loader
ren "wallet clients release the wallet" "wallet pointer owners release the wallet"
ren "wallet client" "wallet loader"
ren "Wallet client" "Wallet loader"
-END VERIFY SCRIPT-
2021-12-22 13:44:55 -05:00
MarcoFalke
fa993d0e7e
doc: Fix -changetype help text
This was forgotten in commit 3ac38058ce0e80a9f4276bfa82951decdb237e9a
2021-12-22 11:12:25 +01:00
Sjors Provoost
56113daef4
wallet: add taprootEnabled() to interface 2021-12-21 11:31:38 +07:00
Andrew Chow
887796a5ff
Merge bitcoin/bitcoin#23341: RPC: Better safety with newkeypool command and wallet backups
a2a92317ad4ab88cfca234f68337a7cd37897f10 rpc: Add warning to user about newkeypool command (Samuel Dobson)

Pull request description:

  This PR prevents `newkeypool` from being run on non-HD wallets, because this would require a new backup every time, so it isn't very safe.

  David Harding also suggested [here](https://github.com/bitcoin/bitcoin/pull/23093#issuecomment-945350003) that the RPC help text should include a warning to the users about the interaction between newkeypool.

ACKs for top commit:
  achow101:
    ACK a2a92317ad4ab88cfca234f68337a7cd37897f10

Tree-SHA512: 0aa497900f1d179764bce13ffce0bb081ba2ca354492bf2e04b21d0212e960b3ed13a386fbf65602b6b50461f4056a0285752ef707d312da28e82449cd8ea048
2021-12-20 14:51:39 -05:00
MarcoFalke
fada6c65d2
wallet: Strictly match tx change type to improve privacy 2021-12-17 07:59:39 +01:00
MarcoFalke
ce463cf450
Merge bitcoin/bitcoin#23750: rpcwallet: mention labels are disabled for ranged descriptors
65efbba45d817261f590d043c69a9981e6b637bd rpcwallet: mention labels are deactivated for ranged descriptors (Antoine Poinsot)

Pull request description:

  It was confusing when trying to use it as a blackbox. So mention it so next ones don't have to open the said box :)

  See #23749 for context

ACKs for top commit:
  Sjors:
    utACK 65efbba45d817261f590d043c69a9981e6b637bd
  achow101:
    ACK 65efbba45d817261f590d043c69a9981e6b637bd

Tree-SHA512: d8a3d1f81c16d95855ac2b01e8fd20e83d6dac1721b3da464a9a890e46102992a6882918be87b2a28b929349ee7f1beb1af6c88b22f065fbbb6948275a6d2b8f
2021-12-16 08:46:09 +01:00
MarcoFalke
a30642926a
Merge bitcoin/bitcoin#23721: wallet, refactor: Move restorewallet() logic to the wallet section
62fa61fa4a33ff4d108a65d656ffe2cac4330824 refactor: remove the wallet folder if the restore fails (w0xlt)
abbb7eccef3fc1c36f34756458d2792f6661e29f refactor: Move restorewallet() RPC logic to the wallet section (w0xlt)
4807f73f48f4ff1084fcf7aee94e5b14592bfda8 refactor: Implement restorewallet() logic in the wallet section (w0xlt)

Pull request description:

  Currently `restorewallet()` logic is written in the RPC layer and it can´t be reused by GUI. So it moves this to the wallet section and then, GUI can access it.

  This is necessary to implement the "Restore Wallet" menu item in the GUI (which is already implemented  in https://github.com/bitcoin-core/gui/pull/471 ).

  This commit also simplifies error handling and adds a new behavior: if the restore fails, the invalid wallet folder is removed.

ACKs for top commit:
  achow101:
    ACK 62fa61fa4a33ff4d108a65d656ffe2cac4330824
  shaavan:
    crACK 62fa61fa4a33ff4d108a65d656ffe2cac4330824

Tree-SHA512: 7ccfbad5943f38616ba0c2dd443c97a4b5bc1f6612dbf5a9e7a0263100aba36671fae929a2e7688442667be394645f44484af137a4802f204a33c4689eb27c39
2021-12-16 08:42:44 +01:00
w0xlt
62fa61fa4a refactor: remove the wallet folder if the restore fails 2021-12-15 18:41:40 -03:00
w0xlt
abbb7eccef refactor: Move restorewallet() RPC logic to the wallet section
It also simplifies restorewallet() and loadwallet() RPC error handling.
2021-12-15 18:41:40 -03:00
w0xlt
4807f73f48 refactor: Implement restorewallet() logic in the wallet section
Currently restorewallet() logic is written in the RPC layer
and it can´t be reused by GUI. So it reimplements this in the
wallet and interface sections and then, GUI can access it.
2021-12-14 19:18:56 -03:00
MarcoFalke
fa26c55644
wallet: Replace Assume with Assert where needed in coinselection 2021-12-13 14:45:41 +01:00
Antoine Poinsot
65efbba45d
rpcwallet: mention labels are deactivated for ranged descriptors 2021-12-12 12:24:06 +01:00
Samuel Dobson
a2a92317ad rpc: Add warning to user about newkeypool command 2021-12-12 19:05:18 +13:00
MarcoFalke
ac92ab6da5
Merge bitcoin/bitcoin#23113: Add warnings to createmultisig and addmultisig if using uncompressed keys
d5cab1a96d26e66d342fb5ec35c809bb82869d00 Add createmultisig and addmultisigaddress warnings release note (Samuel Dobson)
e46fc935aa6eb392ef9411dad310697aba601f6a Add warnings field to addmultisigaddress to warn about uncompressed keys (Samuel Dobson)
d1a9742623e2a8f3307c84c2df6993f62617a6f6 Add warnings field to createmultisig to warn about uncompressed keys (Samuel Dobson)

Pull request description:

  Fixes #21368

  Currently, if there are any uncompressed keys when calling `AddAndGetMultisigDestination`, it will just default to a legacy address regardless of the chosen `address_type`. Rather than keeping this silent behaviour which may be confusing to users, we explicitly add a `warnings` field which will warn the user why their address format is different.

ACKs for top commit:
  achow101:
    ACK d5cab1a96d26e66d342fb5ec35c809bb82869d00

Tree-SHA512: c2ac7f7689251bd4fcd8c26506f053921fbaf34c7a26a74e82ebc7f82cc0bd25407fd7954bf98365dcafa51fa45dcdbee6214320580ca69509690c3555e71cc0
2021-12-11 09:41:25 +01:00
MarcoFalke
011d6e429b
Merge bitcoin/bitcoin#22514: psbt: Actually use SIGHASH_DEFAULT for PSBT signing
c0405ee27fa23a9868f04c052bdc94d7accc57e2 rpc: Document that DEFAULT is for Taproot, ALL for everything else (Andrew Chow)
d3992669df826899a3de78a77a366dab46028026 psbt: Actually use SIGHASH_DEFAULT (Andrew Chow)
eb9a1a2c595a03475fd4275b104676b7e2200f07 psbt: Make sighash_type std::optional<int> (Andrew Chow)

Pull request description:

  Make the behavior align with the help text by actually using SIGHASH_DEFAULT as the default sighash for signing PSBTs.

ACKs for top commit:
  Sjors:
    re-utACK c0405ee27fa23a9868f04c052bdc94d7accc57e2

Tree-SHA512: 5199fb41de416b2f10ac451f824e7c94b428ba11fdb9e50f0027c692e959ce5813a340c34a4e52d7aa128e12008303d80939a693eff36a869720e45442119828
2021-12-10 10:17:36 +01:00
fanquake
09ad512369
Merge bitcoin/bitcoin#23628: Check descriptors returned by external signers
5493e925013245d5ad0f7ea8784fe07f531803d0 Check descriptors returned by external signers (sstone)

Pull request description:

  Check that descriptors returned by external signers have been parsed properly when creating a new wallet.
  See https://github.com/bitcoin/bitcoin/issues/23627 for context.

  The problem is that parsing an invalid descriptor will return `null` which is not checked for in `CWallet::SetupDescriptorScriptPubKeyMans()`.

  I'm not completely sure what the best fix is since there several strategies for dealing with errors in the current codebase but the proposed fix is very simple and consistent with other validation checks in `CWallet::SetupDescriptorScriptPubKeyMans()`.

ACKs for top commit:
  jamesob:
    Code review ACK 5493e92501
  achow101:
    ACK 5493e925013245d5ad0f7ea8784fe07f531803d0

Tree-SHA512: 63259f4aa519405a86c554b6813efdb741314bdaa18bf005b70ea8bb92a27abc6e2b65f7c584641dc257fc78a6499f42b51b5310c243e611c4663430dccf3d04
2021-12-10 09:17:35 +08:00
W. J. van der Laan
c840ab0231
Merge bitcoin/bitcoin#22019: wallet: Introduce SelectionResult for encapsulating a coin selection solution
05300c14392facf38330eb4fcd8e695a838b76f3 Use SelectionResult in SelectCoins (Andrew Chow)
9d9b101d2019d8237546eedd022e74519feb07bb Use SelectionResult in AttemptSelection (Andrew Chow)
bb50850a447bdf461ffb76d47d4a4db904fce324 Use SelectionResult for waste calculation (Andrew Chow)
e8f7ae5eb3c682d1a80b503f71e06ce76af1b65c Make an OutputGroup for preset inputs (Andrew Chow)
51a9c00b4de707e0a6a1a68ca6f8e38d86c72d94 Return SelectionResult from SelectCoinsSRD (Andrew Chow)
0ef6184575e77b17f5ec6d7ca086900aca79f6d7 Return SelectionResult from KnapsackSolver (Andrew Chow)
60d2ca72e3f4c56433c63b929a88e7a2def06399 Return SelectionResult from SelectCoinsBnB (Andrew Chow)
a339add471717623915cd1a846ade4dab2c89deb Make member variables of SelectionResult private (Andrew Chow)
cbf0b9f4ff438865a71c7ceb0a543c18a34f41f0 scripted-diff: Use SelectionResult in coin selector tests (Andrew Chow)
9d1d86da04d5d4768975338841285e90b01130b8 Introduce SelectionResult struct (Andrew Chow)
94d851d28cb909a8f1f8ab795f1d9fc74bebfc7f Fix bnb_search_test to use set equivalence for (Andrew Chow)

Pull request description:

  Instead of returning a set of selected coins and their total value as separate items, encapsulate both of these, and other variables, into a new `SelectionResult` struct. This allows us to have all of the things relevant to a coin selection solution be in a single object. `SelectionResult` enables us to implement the waste calculation in a cleaner way.

  All of the coin selection functions (`SelectCoinsBnB`, `KnapsackSolver`, `AttemptSelection`, and `SelectCoins`) are changed to use a `SelectionResult` as the output parameter.

  Based on #22009

ACKs for top commit:
  laanwj:
    Code review ACK 05300c14392facf38330eb4fcd8e695a838b76f3

Tree-SHA512: e4dbb4d78a6cda9c237d230b19e7265591efac5a101a64e6970f0654e2c4f93d13bb5d07b98e8c7b8d37321753dbfc94c28c3a7810cb1c59b5bc29b08a8493ef
2021-12-09 17:21:46 +01:00
MarcoFalke
7ce8d74156
Merge bitcoin/bitcoin#23346: util, refactor: Improve headers for bitcoin-wallet tool
3431839c33fa3892c982322e4add39e28ddba719 util, refactor: Improve headers for bitcoin-wallet tool (Hennadii Stepanov)

Pull request description:

  This PR:
  - removes unneeded `#include <wallet/wallet.h>` from `<wallet/wallettool.h>`
  - introduces class forward declaration in `<wallet/wallettool.h>`
  - added `#include <config/bitcoin-config.h>` to `wallet/wallettool.cpp` where the `USE_BDB` macro is used

Top commit has no ACKs.

Tree-SHA512: a0de560d821f8b570ae806a1165b9b382c9e0b339687d932052fa4c38ab2ba493e7e050f19adc02ad7db40c42cf88ac1d37209f9071494a0ab268ed33ff22b9f
2021-12-09 13:44:24 +01:00