Commit graph

156 commits

Author SHA1 Message Date
glozow
3a29ff5dea
Merge bitcoin/bitcoin#30463: qa: Functional test improvements
a8e3af1a82 qa: Do not assume running `feature_asmap.py` from source directory (Hennadii Stepanov)
9bf7ca6cad qa: Consider `cache` and `config.ini` relative to invocation directory (Hennadii Stepanov)
a0473442d1 scripted-diff: Add `__file__` argument to `BitcoinTestFramework.init()` (Hennadii Stepanov)

Pull request description:

  This PR includes changes split from https://github.com/bitcoin/bitcoin/pull/30454. They improve the functional test framework, allowing users to [run individual functional tests](https://github.com/hebasto/bitcoin/issues/146) from the build directory in the new CMake-based build system.

  This functionality is not available for out-of-source builds using the current Autotools-based build system, which always requires write permissions for the source directory. Nevertheless, this PR can be tested as suggested in https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2232618421:
  1. Make an out-of-source build:
  ```
  $ ./autogen.sh
  $ mkdir ../build && cd ../build
  $ ../bitcoin/configure
  $ make
  ```
  2. Create a symlink in the build directory to a functional test:
  ```
  $ ln --symbolic ../../../bitcoin/test/functional/wallet_disable.py ./test/functional/
  ```
  3. Run this symlink:
  ```
  $ ./test/functional/wallet_disable.py
  ```
  The last command fails on the master branch:
  ```
  Traceback (most recent call last):
    File "/home/hebasto/git/build/./test/functional/wallet_disable.py", line 31, in <module>
      DisableWalletTest().main()
      ^^^^^^^^^^^^^^^^^^^
    File "/home/hebasto/git/bitcoin/test/functional/test_framework/test_framework.py", line 106, in __init__
      self.parse_args()
    File "/home/hebasto/git/bitcoin/test/functional/test_framework/test_framework.py", line 210, in parse_args
      config.read_file(open(self.options.configfile))
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  FileNotFoundError: [Errno 2] No such file or directory: '/home/hebasto/git/bitcoin/test/config.ini'

  ```
  and succeeds with this PR.

ACKs for top commit:
  maflcko:
    tested ACK a8e3af1a82 🎨
  glozow:
    ACK a8e3af1a82, tested with the steps in op
  stickies-v:
    ACK a8e3af1a82

Tree-SHA512: 899e4efc09edec13ea3f5b47825d03173fb21d3569c360deda7fa6a56b99b4d24e09ad4f0883bad1ee926b1c706e47ba07c6a6160c63c07c82b3cf4ae5816e91
2024-07-22 12:08:32 +01:00
Ava Chow
efbf4e71ce
Merge bitcoin/bitcoin#29523: Wallet: Add max_tx_weight to transaction funding options (take 2)
734076c6de [wallet, rpc]: add `max_tx_weight` to tx funding options (ismaelsadeeq)
b6fc5043c1 [wallet]: update the data type of `change_output_size`, `change_spend_size` and `tx_noinputs_size` to `int` (ismaelsadeeq)
baab0d2d43 [doc]: update reason for deducting change output weight (ismaelsadeeq)
7f61d31a5c [refactor]: update coin selection algorithms input parameter `max_weight` name (ismaelsadeeq)

Pull request description:

  This PR taken over from #29264

  The PR added an option `max_tx_weight` to transaction funding RPC's that ensures the resulting transaction weight does not exceed the specified `max_tx_weight` limit.

  If `max_tx_weight` is not given `MAX_STANDARD_TX_WEIGHT` is used as the max threshold.

  This PR addressed outstanding review comments in #29264

  For more context and rationale behind this PR see https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418/11?u=instagibbs

ACKs for top commit:
  achow101:
    ACK 734076c6de
  furszy:
    utACK 734076c6de
  rkrux:
    reACK [734076c](734076c6de)

Tree-SHA512: 013501aa443d239ee2ac01bccfc5296490c27b4edebe5cfca6b96c842375e895e5cfeb5424e82e359be581460f8be92095855763a62779a18ccd5bdfdd7ddce7
2024-07-17 18:27:59 -04:00
Ava Chow
6f9db1ebca
Merge bitcoin/bitcoin#30357: Fix cases of calls to FillPSBT errantly returning complete=true
7e36dca657 test: add test for modififed walletprocesspsbt calls (willcl-ark)
39cea21ec5 wallet: fix FillPSBT errantly showing as complete (willcl-ark)

Pull request description:

  Fixes: #30077

  Fix cases of calls to `FillPSBT` returning `complete=true` when it's not
  the case.

  This can happen when some inputs have been signed but the transaction is
  subsequently modified, e.g. in the context of PayJoins.

  Also fixes a related bug where a finalized hex string is attempted to be
  added during `walletprocesspsbt` but a CHECK_NONFATAL causes an abort.

ACKs for top commit:
  achow101:
    ACK 7e36dca657
  ismaelsadeeq:
    Tested ACK 7e36dca657
  pinheadmz:
    re-ACK 7e36dca657

Tree-SHA512: e35d19789899c543866d86d513506494d672e4bed9aa36a995dbec4e72f0a8ec5536b57c4a940a18002ae4a8efd0b007c77ba64e57cd52af98e4ac0e7bf650d6
2024-07-16 17:10:19 -04:00
Hennadii Stepanov
a0473442d1
scripted-diff: Add __file__ argument to BitcoinTestFramework.init()
-BEGIN VERIFY SCRIPT-
sed -i -e 's/\s*().main\s*()/(__file__).main()/' $(git ls-files test/functional/*.py)
sed -i -e 's/def __init__(self)/def __init__(self, test_file)/' test/functional/test_framework/test_framework.py
-END VERIFY SCRIPT-
2024-07-16 22:06:47 +01:00
Ryan Ofsky
1f9d30744d
Merge bitcoin/bitcoin#29855: psbt: Check non witness utxo outpoint early
9e13ccc50e psbt: Check non witness utxo outpoint early (Ava Chow)

Pull request description:

  A common issue that our fuzzers keep finding is that outpoints don't exist in the non witness utxos. Instead of trying to track this down and checking in various individual places, do the check early during deserialization. This also unifies the error message returned for this class of problems.

ACKs for top commit:
  maflcko:
    lgtm ACK 9e13ccc50e
  S3RK:
    tACK 9e13ccc50e
  dergoegge:
    utACK 9e13ccc50e

Tree-SHA512: 81b8055b146c6358052226578ddfec0ae5bd877968c7f4f62dc3d6a684545ea568f37c7f1bd619918441af9e453ba8b26531a2280d218da37fa15480f1b45d0e
2024-07-08 13:56:52 -04:00
willcl-ark
7e36dca657
test: add test for modififed walletprocesspsbt calls
This test checks that we can successfully process PSBTs and opt out of
finalization.

Previously trying to call `walletprocesspsbt` would attempt to
auto-finalize (as a convenience), and would not permit opt-out of
finalization, instead aborting via `CHECK_NONFATAL`.
2024-07-03 09:16:17 +01:00
ismaelsadeeq
734076c6de [wallet, rpc]: add max_tx_weight to tx funding options
This allows a transaction's weight to be bound under a certain
weight if possible and desired. This can be beneficial for future
RBF attempts, or whenever a more restricted spend topology is
desired.

Co-authored-by: Greg Sanders <gsanders87@gmail.com>
2024-06-27 15:31:21 +01:00
merge-script
9e0e51b1d9
Merge bitcoin/bitcoin#29870: rpc: Reword SighashFromStr error message
fa6ab0d020 rpc: Reword SighashFromStr error message (MarcoFalke)

Pull request description:

  Put quotes around the parameter. In theory, `std::quoted` should be used, but that seems overkill.

  This should avoid error messages such as `A valid sighash parameter is not a valid sighash parameter. (code -8)`.

  Also, it should fix fuzz false positives when searching for internal bugs in the `rpc` fuzz target. For example, `ZGVzY3JpcHRvcnByb2Nlc3Nwc2J0XP9ce1tdXOVJbnRlcm5hbCBidWcgZGV0ZWN0ZWQAXQ0AHfcAXQ1p7TJv`.

ACKs for top commit:
  dergoegge:
    ACK fa6ab0d020
  brunoerg:
    utACK fa6ab0d020

Tree-SHA512: e2c0cc0126de61873a863af38b7b0a23d2dadd596ca0418dae2ad091e8acfb6a9d657c376d59187bb008989dc78c6b44fe518590e5217e4049a867b220c9fb18
2024-04-24 20:55:27 +08:00
MarcoFalke
fa6ab0d020
rpc: Reword SighashFromStr error message 2024-04-15 10:01:15 +02:00
Ava Chow
9e13ccc50e psbt: Check non witness utxo outpoint early
A common issue that our fuzzers keep finding is that outpoints don't
exist in the non witness utxos. Instead of trying to track this down and
checking in various individual places, do the check early during
deserialization.
2024-04-11 10:58:03 -04:00
Sebastian Falbesoner
f81fad5e0f test: introduce and use calculate_input_weight helper
Rather than manually estimating an input's weight by adding up all the
involved components (fixed-size skeleton, compact-serialized lengths,
and the actual scriptSig / witness stack items) we can simply take use
of the serialization classes `CTxIn` / `CTxInWitness` instead, to
achieve the same with significantly less code.

The new helper is used in the functional tests rpc_psbt.py and
wallet_send.py, where the previous manual estimation code was
duplicated.
2024-04-05 01:06:14 +02:00
Max Edwards
3c49e69670 test: fix weight estimates in functional tests
Updates the weight estimate to be more accurate by removing byte buffers and calculating the length of the count of scriptWitnesses rather than just using the count itself.
2024-03-01 11:43:36 +00:00
fanquake
f028470654
Merge bitcoin/bitcoin#28727: test: replace random_bytes with random.randbytes
fe3ac3700d test: replace random_bytes with randbytes #28720 (ns-xvrn)

Pull request description:

  With Python upgraded to 3.9 replaced the `random_bytes` function in util of functional tests and replaced it's usage with `random.randbytes`.

  Closes #28720.

ACKs for top commit:
  maflcko:
    lgtm ACK fe3ac3700d
  BrandonOdiwuor:
    ACK fe3ac3700d
  stickies-v:
    ACK fe3ac3700d, thanks for picking this up
  kristapsk:
    utACK fe3ac3700d

Tree-SHA512: f65a75e73ebd840c2936eb133d42bccd552f25b717c8ca25c18d06e0593e12f292389cfcc0a0b0759004b67a46ea0c8ac237973ef90f246139778230be1e64e1
2023-10-29 10:14:59 +01:00
ns-xvrn
fe3ac3700d test: replace random_bytes with randbytes #28720 2023-10-25 08:56:41 -04:00
Sebastian Falbesoner
73a339abc3 test: refactor: support sending funds with outpoint result
This commit introduces a helper `create_outpoints` to execute the
`send` RPC and immediately return the target address outpoints as UTXO
dictionary in the common format, making the tests more readable and
avoiding unnecessary duplication.
2023-10-24 11:13:51 +02:00
ismaelsadeeq
2b4edf889a test: check descriptorprocesspsbt return hex encoded tx
Test that if the processed psbt is complete the hex encoded tx
is returned and remove unneccessary rpc call to finalize the
psbt.
2023-09-15 16:48:36 +01:00
ismaelsadeeq
4614332fc4
test: remove unnecessary finalizepsbt rpc calls 2023-09-06 11:52:19 -04:00
Matthew Zipkin
e3d484b603
wallet rpc: return final tx hex from walletprocesspsbt if complete 2023-09-05 09:14:32 -04:00
Jon Atack
90c8f79e94 test: remove redundant test values
as they are parsed identically.

See AmountFromValue() / ParseFixedPoint() / UniValue#getValStr()
2023-07-27 06:54:32 -06:00
Jon Atack
647d95aae9 test: add coverage for passing an invalid sighashtype
in RPCs descriptorprocesspbst, walletprocesspbst, signrawtransactionwithkey,
and signrawtransactionwithwallet.
2023-07-27 06:37:54 -06:00
Sebastian Falbesoner
1a572ce7d6 test: refactor: introduce generate_keypair helper with WIF support
In functional tests it is a quite common scenario to generate fresh
elliptic curve keypairs, which is currently a bit cumbersome as it
involves multiple steps, e.g.:

    privkey = ECKey()
    privkey.generate()
    privkey_wif = bytes_to_wif(privkey.get_bytes())
    pubkey = privkey.get_pubkey().get_bytes()

Simplify this by providing a new `generate_keypair` helper function that
returns the private key either as `ECKey` object or as WIF-string
(depending on the boolean `wif` parameter) and the public key as
byte-string; these formats are what we mostly need (currently we don't
use `ECPubKey` objects from generated keypairs anywhere).

With this, most of the affected code blocks following the pattern above
can be replaced by one-liners, e.g.:

    privkey, pubkey = generate_keypair(wif=True)

Note that after this commit, the only direct uses of `ECKey` remain in
situations where we want to set the private key explicitly, e.g. in
MiniWallet (test/functional/test_framework/wallet.py) or the test for
the signet miner script (test/functional/tool_signet_miner.py).
2023-06-19 17:38:14 +02:00
Andrew Chow
34ac3f438a
Merge bitcoin/bitcoin#26485: RPC: Accept options as named-only parameters
2cd28e9fef rpc: Add check for unintended option/parameter name clashes (Ryan Ofsky)
95d7de0964 test: Update python tests to use named parameters instead of options objects (Ryan Ofsky)
96233146dd RPC: Allow RPC methods accepting options to take named parameters (Ryan Ofsky)
702b56d2a8 RPC: Add add OBJ_NAMED_PARAMS type (Ryan Ofsky)

Pull request description:

  Allow RPC methods which take an `options` parameter (`importmulti`, `listunspent`, `fundrawtransaction`, `bumpfee`, `send`, `sendall`, `walletcreatefundedpsbt`, `simulaterawtransaction`), to accept the options as named parameters, without the need for nested JSON objects.

  This makes it possible to make calls like:

  ```sh
  src/bitcoin-cli -named bumpfee txid fee_rate=10
  ```

  instead of

  ```sh
  src/bitcoin-cli -named bumpfee txid options='{"fee_rate": 10}'
  ```

  RPC help is also updated to show options as top level named arguments instead of as nested objects.

  <details><summary>diff</summary>
  <p>

  ```diff
  @@ -15,16 +15,17 @@

   Arguments:
   1. txid                           (string, required) The txid to be bumped
  -2. options                        (json object, optional)
  +2. options                        (json object, optional) Options object that can be used to pass named arguments, listed below.
  +
  +Named Arguments:
  -     {
  -       "conf_target": n,          (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks
  +conf_target                       (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks

  -       "fee_rate": amount,        (numeric or string, optional, default=not set, fall back to wallet fee estimation)
  +fee_rate                          (numeric or string, optional, default=not set, fall back to wallet fee estimation)
                                     Specify a fee rate in sat/vB instead of relying on the built-in fee estimator.
                                     Must be at least 1.000 sat/vB higher than the current transaction fee rate.
                                     WARNING: before version 0.21, fee_rate was in BTC/kvB. As of 0.21, fee_rate is in sat/vB.

  -       "replaceable": bool,       (boolean, optional, default=true) Whether the new transaction should still be
  +replaceable                       (boolean, optional, default=true) Whether the new transaction should still be
                                     marked bip-125 replaceable. If true, the sequence numbers in the transaction will
                                     be left unchanged from the original. If false, any input sequence numbers in the
                                     original transaction that were less than 0xfffffffe will be increased to 0xfffffffe
  @@ -32,11 +33,10 @@
                                     still be replaceable in practice, for example if it has unconfirmed ancestors which
                                     are replaceable).

  -       "estimate_mode": "str",    (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
  +estimate_mode                     (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
                                     "unset"
                                     "economical"
                                     "conservative"
  -     }

   Result:
   {                    (json object)
  ```

  </p>
  </details>

  **Review suggestion:** To understand this PR, it is probably easiest to review the commits in reverse order because the last commit shows the external API changes, the middle commit shows the internal API changes, and the first commit contains the low-level implementation.

ACKs for top commit:
  achow101:
    ACK 2cd28e9fef

Tree-SHA512: 50f6e78fa622826dab3f810400d8c1a03a98a090b1f2fea79729c58ad8cff955554bd44c2a5975f62a526b900dda68981862fd7d7d05c17f94f5b5d847317436
2023-06-01 15:30:31 -04:00
Andrew Chow
22139f6e83
Merge bitcoin/bitcoin#25796: rpc: add descriptorprocesspsbt rpc
1bce12acd3 test: add test for `descriptorprocesspsbt` RPC (ishaanam)
fb2a3a70e8 rpc: add descriptorprocesspsbt rpc (ishaanam)

Pull request description:

  This PR implements an RPC called `descriptorprocesspsbt`. This RPC is based off of `walletprocesspsbt`, but instead of interacting with the wallet to update, sign and finalize a psbt, it instead accepts an array of output descriptors and uses that information along with information from the mempool, txindex, and the utxo set to do so. `utxoupdatepsbt` also updates a psbt in this manner, but doesn't sign or finalize it. Because of this overlap, a helper function that is added in this PR is called by both `utxoupdatepsbt` and `descriptorprocesspsbt`. Whether or not the helper function signs a psbt is dictated by if the HidingSigningProvider passed to it contains any private information. There is also a test added in this PR for this new RPC that uses p2wsh, p2wpkh, and legacy outputs.
  Edit: see https://github.com/bitcoin/bitcoin/pull/25796#issuecomment-1228830963

ACKs for top commit:
  achow101:
    re-ACK 1bce12acd3
  instagibbs:
    reACK 1bce12acd3

Tree-SHA512: e1d0334739943e71f2ee68b4db7637ebe725da62e7aa4be071f71c7196d2a5970a31ece96d91e372d34454cde8509e95ab0eebd2c8edb94f7d5a781a84f8fc5d
2023-05-22 11:28:11 -04:00
ishaanam
1bce12acd3 test: add test for descriptorprocesspsbt RPC 2023-05-06 10:03:25 -04:00
Andrew Chow
30bf70c8b6
Merge bitcoin/bitcoin#27325: test: various converttopsbt check cleanups in rpc_psbt.py
afc2dd5484 test: various `converttopsbt` check cleanups in rpc_psbt.py (Sebastian Falbesoner)

Pull request description:

  In the functional test rpc_psbt.py, some comments around the `converttopsbt` RPC checks are wrong or outdated and can be removed:

  > _Error could be either "TX decode failed" (segwit inputs causes
  > parsing to fail) or "Inputs must not have scriptSigs and
  > scriptWitnesses"_

  Decoding a valid TX with at least one input always succeeds with the [heuristic](e352f5ab6b/src/core_read.cpp (L126)), i.e. this comment is not right and we can assert for the error string "Inputs must not have scriptSigs and scriptWitnesses" on the calls below.

  > _We must set iswitness=True because the serialized transaction has
  > inputs and is therefore a witness transaction_

  This is also unneeded (and confusing, w.r.t. "is therefore a witness transaction"?), for a TX with one input there is no need to set the `iswitness` parameter. For sake of completeness, we still keep one variant where iswitness is explicitly set to true.

  Lastly, there is a superflous `converttopsbt` call on the raw tx which is the same as just [about ~10 lines above](https://github.com/bitcoin/bitcoin/blob/master/test/functional/rpc_psbt.py#L393-L397), so it can be removed.

ACKs for top commit:
  ismaelsadeeq:
    tested ACK afc2dd5484
  achow101:
    ACK afc2dd5484

Tree-SHA512: 467efefdb3f61efdb79145044b90fc8dc2f0c425f078117a99112b0074e7d4a32c34e464f665fbf8de70d06caaa5d4ad5908c1d75d2e7607eccb0837480afab3
2023-05-04 11:08:16 -04:00
Ryan Ofsky
95d7de0964 test: Update python tests to use named parameters instead of options objects 2023-05-03 11:27:51 -05:00
Andrew Chow
2755aa5121
Merge bitcoin/bitcoin#25939: rpc: In utxoupdatepsbt also look for the tx in the txindex
6e9f8bb050 rpc, tests: in `utxoupdatepsbt` also look for the transaction in the txindex (ishaanam)
a5b4883fb4 rpc: extract psbt updating logic into ProcessPSBT (ishaanam)

Pull request description:

  Previously the `utxoupdatepsbt` RPC, added in #13932, only updated the inputs spending segwit outputs with the `witness_utxo`, because the full transaction is needed for legacy inputs. Before this RPC looked for just the utxos in the utxo set and the mempool. This PR makes it so that if the user has txindex enabled, then the full transaction is looked for there for all inputs. If it is not found in the txindex or  txindex isn't enabled, then the mempool is checked for the full transaction. If the transaction an input is spending from is still not found at that point, then the utxo set is searched and the inputs spending segwit outputs are updated with just the utxo.

ACKs for top commit:
  achow101:
    ACK 6e9f8bb050
  Xekyo:
    ACK 6e9f8bb050

Tree-SHA512: 078db3c37a1ecd5816d80a42e8bd1341e416d661f508fa5fce0f4e1249fefd7b92a0d45f44957781cb69d0953145ef096ecdd4545ada39062be27742402dac6f
2023-04-21 14:06:12 -04:00
Sebastian Falbesoner
afc2dd5484 test: various converttopsbt check cleanups in rpc_psbt.py
In the functional test rpc_psbt.py, some comments around the
`converttopsbt` RPC checks are wrong or outdated and can be
removed:

> Error could be either "TX decode failed" (segwit inputs causes
> parsing to fail) or "Inputs must not have scriptSigs and
> scriptWitnesses"

Decoding a valid TX with at least one input always succeeds with the
heuristic, i.e. this comment is not right and we can assert for the
error string "Inputs must not have scriptSigs and scriptWitnesses"
on the calls below.

> We must set iswitness=True because the serialized transaction has
> inputs and is therefore a witness transaction

This is also unneeded (and confusing, w.r.t. "is therefore a witness
transaction"?), for a TX with one input there is no need to set the
`iswitness` parameter. For sake of completeness, we still keep one
variant where iswitness is explicitly set to true.

Lastly, there is a superflous `converttopsbt` call on the raw tx which
is the same as just about ~10 lines above, so it can be removed.
2023-03-24 17:54:09 +01:00
Andrew Chow
fc7c21f664
Merge bitcoin/bitcoin#27271: RPC: Fix fund transaction crash when at 0-value, 0-fee
d7cc503843 Fix fund transaction case at 0-value, 0-fee (Greg Sanders)

Pull request description:

  and when no inputs are pre-selected.

  triggered via:

  walletcreatefundedpsbt '[]' '[{"data": "deadbeef"}]' 0 '{"fee_rate": "0"}'

ACKs for top commit:
  achow101:
    ACK d7cc503843
  josibake:
    ACK d7cc503843
  furszy:
    Crashes sucks code ACK d7cc5038

Tree-SHA512: 3f5e10875666aaf52c11d6a38b951aa75d0cbe684cc7f904e199f7a864923bf31d03a654687f8b746cae0eebb886a799bff2c6d200699438480d4c0ff8785f3a
2023-03-22 12:54:26 -04:00
ishaanam
6e9f8bb050 rpc, tests: in utxoupdatepsbt also look for the transaction in the txindex
Previously only the segwit utxos being spent by the psbt were looked for and
added to the psbt. Now, the full transaction corresponding to each of these
utxos (legacy and segwit) is looked for in the txindex and mempool and added
to the psbt. If txindex is disabled and the transaction is not in the mempool,
then we fall back to getting just the utxo (if segwit) from the utxo set.
2023-03-18 20:58:15 -04:00
Greg Sanders
d7cc503843 Fix fund transaction case at 0-value, 0-fee 2023-03-16 14:58:41 -04:00
Sebastian Falbesoner
3dd2f6461b test: psbt: check non-witness UTXO removal for segwit v1 input 2023-03-05 04:07:11 +01:00
Sebastian Falbesoner
dd78e3fa43 test: speedup rpc_psbt.py by whitelisting peers (immediate tx relay)
master branch:
    0m36.86s real     0m03.26s user     0m01.69s system
    0m35.71s real     0m03.78s user     0m01.64s system
    0m45.76s real     0m03.12s user     0m01.27s system

PR branch:
    0m13.04s real     0m02.66s user     0m00.93s system
    0m14.08s real     0m02.81s user     0m00.82s system
    0m14.05s real     0m02.50s user     0m00.93s system
2023-03-05 01:46:57 +01:00
Juan Pablo Civile
a07a413466 Wallet/RPC: Allow specifying min & max chain depth for inputs used by fund calls
Enables users to craft BIP-125 replacements with changes to the output
list, ensuring that if additional funds are needed they will be added.
2023-01-11 17:08:23 -05:00
Andrew Chow
3f8591d46b
Merge bitcoin/bitcoin#26661: wallet: Coin Selection, return accurate error messages
76dc547ee7 gui: create tx, launch error dialog if backend throws runtime_error (furszy)
f4d79477ff wallet: coin selection, add duplicated inputs checks (furszy)
0aa065b14e wallet: return accurate error messages from Coin Selection (furszy)
7e8340ab1a wallet: make SelectCoins flow return util::Result (furszy)
e5e147fe97 wallet: refactor eight consecutive 'AttemptSelection' calls into a loop (furszy)

Pull request description:

  Work decoupled from #25806, which cleanup and improves the Coin Selection flow further.

  Adding the capability to propagate specific error messages from the Coin Selection process to the user.
  Instead of always returning the general "Insufficient funds" message which is not always accurate to what happened internally.
  Letting us instruct the user how to proceed under certain circumstances.

  The following error messages were added:

  1) If the selection result exceeds the maximum transaction weight,
     we now will return:
  -> "The inputs size exceeds the maximum weight. Please try sending
  a smaller amount or manually consolidating your wallet's UTXOs".

  2) If the user pre-selected inputs and disallowed the automatic coin
     selection process (no other inputs are allowed), we now will
     return:
  -> "The preselected coins total amount does not cover the transaction
  target. Please allow other inputs to be automatically selected or include
  more coins manually".

  3) The double-counted preset inputs during Coin Selection error will now
  throw an "internal bug detected" message instead of crashing the node.

  The essence of this work comes from several comments:
  1. https://github.com/bitcoin/bitcoin/pull/26560#discussion_r1037395665
  2. https://github.com/bitcoin/bitcoin/pull/25729#discussion_r940619491
  3. https://github.com/bitcoin/bitcoin/pull/25269#pullrequestreview-1135240825
  4. https://github.com/bitcoin/bitcoin/issues/23144 (which is connected to #24845)

ACKs for top commit:
  ishaanam:
    crACK 76dc547ee7
  achow101:
    ACK 76dc547ee7
  aureleoules:
    ACK 76dc547ee7
  theStack:
    ACK 76dc547ee7 🌇

Tree-SHA512: 9de30792d7a5849cae77747aa978e70390b66ee9d082779a56088a024f82e725b0af050e6603aece0ac8229f6d73bc471ba97b4ab69dc7eddf419f5f56ae89a5
2023-01-03 18:53:36 -05:00
Hennadii Stepanov
306ccd4927
scripted-diff: Bump copyright headers
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-

Commits of previous years:
- 2021: f47dda2c58
- 2020: fa0074e2d8
- 2019: aaaaad6ac9
2022-12-24 23:49:50 +00:00
furszy
0aa065b14e
wallet: return accurate error messages from Coin Selection
and not the general "Insufficient funds" when the wallet
actually have funds.

Two new error messages:

1) If the selection result exceeds the maximum transaction weight,
   we now will return: "The inputs size exceeds the maximum weight".

2) If the user preselected inputs and disallowed the automatic coin
   selection process (no other inputs are allowed), we now will
   return: "The preselected coins total amount does not cover the
   transaction target".
2022-12-21 23:14:50 -03:00
Andrew Chow
8597260872
Merge bitcoin/bitcoin#26480: test: Remove wallet option from non-wallet tests
fa10f193b5 test: Set default in add_wallet_options if only one type can be chosen (MacroFake)
555519d082 test: Remove wallet option from non-wallet tests (MacroFake)
fac8d59d31 test: Set -disablewallet when no wallet has been compiled (MacroFake)
fa68937b89 test: Make requires_wallet private (MacroFake)

Pull request description:

  The tests have several issues:

  * Some tests that are wallet-type specific offer the option to run the test with the incompatible type

  For example, `wallet_dump.py` offers `--descriptors` and on current master fails with `JSONRPCException: Invalid public key`. After the changes here, it fails with a clear error: `unrecognized arguments: --descriptors`.

  * Tests that don't use the wallet at all offer the option to run it with a wallet type. This is confusing and wastes developers time if they are "tricked" into running the test for both wallet types, even though no wallet code is executed at all.

  For example, `feature_addrman.py` will happily accept and run with `--descriptors` or `--legacy-wallet`. After the changes here, it no longer silently ignores the flag, but reports a clear error: `unrecognized arguments`.

ACKs for top commit:
  achow101:
    ACK fa10f193b5

Tree-SHA512: a5784da7305f4ec58c0013f433289000d94fc3d434b00fc329ffa37b812e2cd1da0071e34c3462bf79d904808564f2ae6d3d582f6b86b26215f9b07391b58460
2022-11-28 11:16:49 -05:00
MacroFake
48174c0f28
Merge bitcoin/bitcoin#26240: rpc: Adjust RPCTypeCheckObj error string
2dede9f675 Adjust RPCTypeCheckObj error string (Leonardo Araujo)

Pull request description:

  Unifies the JSON type error strings as mentioned in #26214. Also refer to #25737.

ACKs for top commit:
  furszy:
    ACK 2dede9f6

Tree-SHA512: c918889e347ba32cb6d0e33c0de5956c2077dd40c996151e16741b0c4983ff098c60258206ded76ad7bbec4876c780c6abb494a97e4f1e05717d28a59b9167a6
2022-11-14 12:09:06 +01:00
MacroFake
555519d082
test: Remove wallet option from non-wallet tests
Review note: The changes are complete, because self.options.descriptors
is set to None in parse_args (test_framework.py).

A value of None implies -disablewallet, see the previous commit.

So if a call to add_wallet_options is missing, it will lead to a test
failure when the wallet is compiled in.
2022-11-10 17:19:13 +01:00
furszy
5baedc3351
wallet: remove fetch pre-selected-inputs responsibility from SelectCoins
so if there is an error in any of the pre-set coins, we can fail right away
without computing the wallet available coins set (calling `AvailableCoins`)
which is a slow operation as it goes through the entire wallet's txes map.

----------------------

And to make the Coin Selection flow cleared, have decoupled SelectCoins in two functions:

1) AutomaticCoinSelection.
2) SelectCoins.

1) AutomaticCoinSelection:
   Receives a set of coins and selects the best subset of them to
   cover the target amount.

2) SelectCoins
   In charge of select all the user manually selected coins first ("pre-set inputs"), and
   if coin_control 'm_allow_other_inputs=true', call 'AutomaticCoinSelection' to select a
   subset of coins owned by the wallet to cover for the target - preset_inputs.total_amount
   remaining value.
2022-10-26 15:54:31 -03:00
Greg Sanders
e133264c5b Add test for PSBT input verification 2022-10-17 11:13:30 -04:00
Leonardo Araujo
2dede9f675 Adjust RPCTypeCheckObj error string 2022-10-10 18:08:00 -03:00
Andrew Chow
22c051ca70 tests: Test that PSBT_OUT_TAP_TREE is combined correctly 2022-10-06 15:32:45 -04:00
Andrew Chow
70a55c059b psbt: Avoid unsigned int overflow in PSBT_IN_TAP_BIP32_DERIVATION 2022-08-10 11:58:17 -04:00
MacroFake
c5ba1d92b6
Merge bitcoin/bitcoin#25610: wallet, rpc: Opt in to RBF by default
ab3c06db1a doc: Release notes for default RBF (Andrew Chow)
61d9149e78 rpc: Default rbf enabled (Andrew Chow)
e3c33637ba wallet: Enable -walletrbf by default (Andrew Chow)

Pull request description:

  The GUI currently opts in to RBF by default, but RPCs do not, and `-walletrbf` is default disabled. This PR makes the default in those two places to also opt in.

  The last time this was proposed (#9527), the primary objections were the novelty at the time, the inability to bump transactions, and the gui not having the option to disable rbf. In the 5 years since, RBF usage has steadily grown, with ~27% of txs opting in. The GUI has the option to enable/disable RBF, and is also defaulted to having it enabled. And we have the ability to bump RBF'd transactions in both the RPC and the GUI. So I think it makes sense to finally change the default to always opt in to RBF.

ACKs for top commit:
  darosior:
    reACK ab3c06db1a
  aureleoules:
    ACK ab3c06db1a.
  glozow:
    utACK ab3c06db1a

Tree-SHA512: 81b012c5033e270f86a87a6a196ccc549eb54b158eebf88e917cc6621d40d7bdcd1566b602688907dd5d364b95a557b29f97dce869cea512e339588262c027b6
2022-08-01 10:53:11 +02:00
Sebastian Falbesoner
4e616d20c9 test: check that combining PSBTs with different txs fails 2022-07-23 09:08:54 +02:00
Sebastian Falbesoner
71a751f6c3 test: add test for decoding PSBT with per-input preimage types 2022-07-19 17:44:50 +02:00
MacroFake
1b285b7807
Merge bitcoin/bitcoin#25590: wallet: Precompute Txdata after setting PSBT inputs' UTXOs
d2ed97656b wallet: Precompute Txdata after setting PSBT inputs' UTXOs (Andrew Chow)

Pull request description:

  If we are given a PSBT that is missing one or more input UTXOs, our
  PrecomputedTransactionData will be incorrect and missing information
  that it should otherwise have, and therefore we may not produce a
  signature when we should. To avoid this problem, we can do the
  precomputation after we have set the UTXOs the wallet is able to set for
  the PSBT.

  Also adds a test for this behavior.

ACKs for top commit:
  instagibbs:
    reACK d2ed97656b
  Sjors:
    ACK d2ed97656b
  aureleoules:
    ACK d2ed97656b.

Tree-SHA512: 71beb6c7946096e82cfca83f36277302aa9e69d27b4f6d73d7d8f2f9f0ea1c0d653e846fa6aebee5e4763f56f950b4481240e953f6a2412caa84908d519171e1
2022-07-19 10:58:25 +02:00
Andrew Chow
61d9149e78 rpc: Default rbf enabled 2022-07-15 11:46:34 -04:00