Commit graph

1565 commits

Author SHA1 Message Date
MarcoFalke fa25668e1c
test: Test p2sh-witness and bech32 in wallet_import_rescan 2019-08-14 17:20:07 -04:00
MarcoFalke fa79af2989
test: Replace fragile "rng" with call to random() 2019-08-14 17:19:48 -04:00
MarcoFalke fac3dcf7d0
test: Generate one block for each send in wallet_import_rescan
This ...
* ensures that enough coins are available/spendable, even when more
  variants are added
* ensures that all mempool txs are mined, even when more variants are
  added
* makes the test more specific to test that the confirmation height
  is properly reported and timestamps are correctly handled in the test
  logic
* prepares the test for a future, where blocks are skipped for rescan if
  they are deemed irrelevant by a filter (c.f. BIP157)
2019-08-14 16:22:37 -04:00
fanquake d5a54ce8f0
Merge #15305: [validation] Crash if disconnecting a block fails
a47df13471 [qa] Test disconnect block failure -> shutdown (Suhas Daftuar)
4433ed0f73 [validation] Crash if disconnecting a block fails (Suhas Daftuar)

Pull request description:

  If we're unable to disconnect a block during normal operation, then that is a
  failure of our local system (such as disk failure) or the chain that we are on
  (eg CVE-2018-17144), but cannot be due to failure of the (more work) chain that
  we're trying to validate.

  We should abort rather than stay on a less work chain.

  Fixes #14341.

ACKs for top commit:
  practicalswift:
    utACK a47df13471
  TheBlueMatt:
    utACK a47df13471. Didn't bother to review the test in detail, it looked fine. Debated whether invalidateblock should ever crash the node, but *not* crashing in the case of hitting a pruned block (which is the only change here) is clearly better, even if there are other cases I'd argue we should crash in.
  ryanofsky:
    utACK a47df13471. Only change since last review is new comment.
  promag:
    ACK a47df1347, it takes awhile to quit (RPC connection timeouts) but that's unrelated - hope to fix that soon.
  fanquake:
    ACK a47df13471

Tree-SHA512: 4dec8cef6e7dbbe513c138fc5821a7ceab855e603ece3c16185b51a3830ab7ebbc844a28827bf64e75326f45325991dcb672f13bd7baede53304f27289c4af8d
2019-07-25 09:05:22 +08:00
MarcoFalke 0626b8cbdf
Merge #16445: test: Skip flaky p2p_invalid_messages test on macOS
c3dfc91032 test: Skip flaky p2p_invalid_messages test on macOS (Fabian Jahr)

Pull request description:

  This mitigates https://github.com/bitcoin/bitcoin/issues/15400

  I had a look into the issue today and this seems to be the best we can do given that the root causes some unexpected custom error code from the macOS kernel that python/asyncio doesn't know how to handle properly yet.

Top commit has no ACKs.

Tree-SHA512: 20a0551d45c405b6eb0ae58190b701c7026c52eff6c434bc678f723a4dabf0074e5b52a8bb3d51ee7132dc29419d1e67a24696761c444c62cd4d429ec768e67d
2019-07-24 07:18:45 -04:00
MarcoFalke 67923d6b3c
Merge #16366: init: Use InitError for all errors in bitcoind/qt
fa6f402bde Call node->initError instead of InitError from GUI code (Russell Yanofsky)
fad2502240 init: Use InitError for all errors in bitcoind/qt (MarcoFalke)

Pull request description:

  Using the same InitError for startup error in the daemon and the gui makes it possible to run the tests with the gui again:

  ```sh
  BITCOIND=bitcoin-qt ./test/functional/test_runner.py feature_includeconf feature_config_args

ACKs for top commit:
  hebasto:
    ACK fa6f402bde
  ryanofsky:
    utACK fa6f402bde. Only changes since last review are removing more includes and adding Node::initError method to avoid accessing node `InitError` function and global variables from GUI code.

Tree-SHA512: bd19e08dcea4019dfe40356bc5c63cb583cefed54b6c9dcfb82f1b5b00308d8e2b363549afcaea5e93bf83864dbe0917400c3b70f43a8a5bdff45c9cd34cc294
2019-07-23 18:40:46 -04:00
Fabian Jahr c3dfc91032 test: Skip flaky p2p_invalid_messages test on macOS 2019-07-23 15:59:27 -04:00
Wladimir J. van der Laan 51a6e2c419
Merge #15681: [mempool] Allow one extra single-ancestor transaction per package
50cede3f5a [mempool] Allow one extra single-ancestor transaction per package (Matt Corallo)

Pull request description:

  This implements the proposed policy change from [1], which allows
  certain classes of contract protocols involving revocation
  punishments to use CPFP. Note that some such use-cases may still
  want some form of one-deep package relay, though even this alone
  may greatly simplify some lightning fee negotiation.

  [1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html

ACKs for top commit:
  ajtowns:
    ACK 50cede3f5a -- looked over code again, compared with previous commit, compiles, etc.
  sdaftuar:
    ACK 50cede3f5a
  ryanofsky:
    utACK 50cede3f5a. Changes since last review: adding EXTRA_DESCENDANT_TX_SIZE_LIMIT constant, changing max ancestor size from 1,000,000 to nLimitAncestorSize constant (101,000), fixing test comment and getting rid of unused test node.

Tree-SHA512: b052c2a0f384855572b4579310131897b612201214b5abbb225167224e4f550049e300b471dbf320928652571e92ca2d650050b7cf39ac92b3bc1d2bcd386c1c
2019-07-19 20:00:12 +02:00
fanquake 59ce537a49
Merge #16152: Disable bloom filtering by default.
bead32e31e Add release notes for DEFAULT_BLOOM change (Matt Corallo)
f27309f55c Move DEFAULT_PEERBLOOMFILTERS from validation.h to net_processing.h (Matt Corallo)
5efcb77283 Disable bloom filtering by default. (Matt Corallo)

Pull request description:

  BIP 37 bloom filters have been well-known to be a significant DoS
  target for some time. However, in order to provide continuity for
  SPV clients relying on it, the NODE_BLOOM service flag was added,
  and left as a default, to ensure sufficient nodes exist with such a
  flag.

  NODE_BLOOM is, at this point, well-established and, as long as
  there exist 0.18 nodes with default config (which I'd anticipate
  will be true for many years), will be available from some peers. By
  that time, the continued slowdown of BIP 37-based filtering will
  likely have rendered it useless (though this is already largely the
  case). Further, BIP 37 was deliberately never updated to support
  witness-based filtering as newer wallets are expected to migrate to
  some yet-to-be-network-exposed filters.

ACKs for top commit:
  jnewbery:
    ACK bead32e31e
  kallewoof:
    ACK bead32e31e

Tree-SHA512: ecd901898e8efe1a7c82b471af0acc2373c2282ac633eb58d9aae7c35deda1999d0f79fb0485e6cecbda7246aeda00206cd82c7fa36866e2ac64705ba93f9390
2019-07-19 17:33:56 +08:00
Jonas Schnelli 024ecd7e01
QA: Fix race condition in wallet_encryption test 2019-07-18 22:31:24 +02:00
fanquake 0515406acb
Merge #16374: test: Enable passing wildcard test names to test runner from root
e142ee03e7 doc: describe how to pass wildcard names to test runner (Jon Atack)
6a7a70b8cf test: enable passing wildcards with path to test runner (Jon Atack)

Pull request description:

  Currently, passing wildcard testname args to the test runner from outside the test/functional/ directory does not work, even though developers expect it to. See these recent IRC discussions for more background: http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-10.html#l-262 (lines 262 to 323) and http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-11.html#l-134.

  1. [BUGFIX] Enable passing wildcards with paths. Examples:
      - `test/functional/test_runner.py test/functional/wallet*`
      - `functional/test_runner.py functional/wallet*`
      - `test/functional/test_runner.py ./test/functional/tool* test/functional/mempool*`
      - A current limitation this PR does not change: 9 test files with arguments in their filename are not picked up by wildcard search.

  2. [Docs] Describe how to pass wildcard names (multiple and with paths) to the test runner in test/README.md.

ACKs for top commit:
  jnewbery:
    tested ACK e142ee03e7
  jachiang:
    Tested ACK e142ee03e7. Thanks a lot for this fix!
  MarcoFalke:
    ACK e142ee03e7, fine with me

Tree-SHA512: cb3d994880cdc9b8918546b573a25faa5b4c7339826ac7cfe20f076aac6e731a34271609c0cf5a7ee5e4a2d5ae205298319d24bf36ef5b5d569a1a0c57883e54
2019-07-18 10:05:08 +08:00
MarcoFalke 24dbcf3808
Merge #15891: test: Require standard txs in regtest by default
fa89badf88 test: Require standard txs in regtest (MarcoFalke)
fa9b419160 test: Add test that mainnet requires standard txs (MarcoFalke)
fa613ca0a8 chainparams: Remove unused fMineBlocksOnDemand (MarcoFalke)

Pull request description:

  I don't see a reason why regtest should allow non-standard txs, as it makes testing mainnet behaviour such as #15846 unnecessarily hard and unintuitive.

  Of course, testnet policy remains unchanged to allow propagation of non-standard txs.

ACKs for top commit:
  ajtowns:
    ACK fa89badf88

Tree-SHA512: c4c675affb054868850bd2683aa07f4c741a448cbacb2ea8334191e105f426b0790fe6a468be61e9c5880d24154f7bf1c7075051697172dce92180c1bc3a1c90
2019-07-16 16:10:17 -04:00
MarcoFalke 8f9725c83f
Merge #16390: qa: Add --filter option to test_runner.py
1a62425260 qa: Add --filter option to test_runner.py (João Barbosa)

Pull request description:

  Allows to run functional tests like:
  ```sh
  test/functional/test_runner.py --filter wallet
  ```

ACKs for top commit:
  jonatack:
    ACK 1a62425260

Tree-SHA512: 53199e01da3b2e0112843c1c68c69d8fd7fc9bb6a6cb45a81c324973c4824ebf5fef574f9efab81a64d52e397e25d979ae40f0eaba35afb771e80012768f0b08
2019-07-16 09:01:34 -04:00
João Barbosa 1a62425260 qa: Add --filter option to test_runner.py 2019-07-16 01:00:27 +01:00
MarcoFalke 0822b44d8a
Merge #15282: test: Replace hard-coded hex tx with class in test framework
8f250ab788 TEST: Replace hard-coded hex tx with classes (Steven Roose)

Pull request description:

  Came across these breaking Elements.

ACKs for top commit:
  MarcoFalke:
    ACK 8f250ab788
  instagibbs:
    utACK 8f250ab788

Tree-SHA512: e8615dad4cda0beea4b0c7d4951a467fb9882a0a64d49c9b5ecf167369ea62a3fe5348e2401153162b0ccadecdb128492c94be36ebb881c3c42659626d86eda8
2019-07-15 08:02:07 -04:00
Jon Atack 6a7a70b8cf
test: enable passing wildcards with path to test runner
Currently, passing wildcard testname args to the test runner from outside the `test/functional/` directory does not work. See this recent IRC discussion for more background: http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-10.html#l-262 (lines 262 to 323).

This small change enables passing multiple wildcards with paths, as long as the paths are coherent. Examples:
  - test/functional/test_runner.py test/functional/wallet*
  - functional/test_runner.py functional/wallet*
  - test/functional/test_runner.py ./test/functional/tool* test/functional/mempool*

A current limitation that this PR does not change: 9 test files with arguments in their name are not picked up by wildcard search.

- Squashed commit: non-mutating version

- Squashed commit: minor code optimisation
2019-07-15 10:13:39 +02:00
Wladimir J. van der Laan 536590f358
Merge #16334: test: rpc_users: Also test rpcauth.py with password.
e263a343d4 test: rpc_users: Make variable names more clear. (Carl Dong)
830dc2dd0f test: rpc_users: Also test rpcauth.py with specified password. (Carl Dong)
c73d871799 test: rpc_users: Add function for testing auth params. (Carl Dong)
604e2a997f test: rpc_users: Add function for auth'd requests. (Carl Dong)

Pull request description:

  Fixes #14758

  First two commits are tidy-ups which I feel are worthwhile as they are very straightforward, cut down the file by 50%, and made the final diff more minimal. Happy to squash after review.

ACKs for top commit:
  laanwj:
    ACK e263a343d4

Tree-SHA512: aa75c48570a87060238932d4c68e17234e158077f6195fb4917367e1ecc565e3cd8dd0ae51f9159ddd3d03742739680391bc1246454302db22d4a608c0633e80
2019-07-12 21:28:47 +02:00
MarcoFalke fad2502240
init: Use InitError for all errors in bitcoind/qt
Also, remove unused <boost/thread.hpp> include (and others)
2019-07-11 19:39:25 -04:00
Wladimir J. van der Laan 6c1e45c4c4
Merge #16322: wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction
0d101a340c test: Add test for maxtxfee option (MarcoFalke)
177550101b wallet: Remove unreachable code in CreateTransaction (MarcoFalke)
5c1b9714cb wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa)

Pull request description:

  Follow up to #16257, this PR makes `bumpfee` aware of `-maxtxfee`.

  It also prevents dangling locked unspents when calling `fundrawtransaction` - because the previous check was after `LockCoin`.

ACKs for top commit:
  MarcoFalke:
    re-ACK 0d101a340c, only change is small test fixup

Tree-SHA512: 3464b24ae7cd4e72ed41438c6661828ba1304af020f05da62720b23668ae734e16cf47c6d97e150cc84ef631ee099b16fc786c858f3d089905845437338fd512
2019-07-10 14:00:52 +02:00
MarcoFalke 0d101a340c test: Add test for maxtxfee option 2019-07-10 11:58:46 +01:00
Matt Corallo 50cede3f5a [mempool] Allow one extra single-ancestor transaction per package
This implements the proposed policy change from [1], which allows
certain classes of contract protocols involving revocation
punishments to use CPFP. Note that some such use-cases may still
want some form of one-deep package relay, though even this alone
may greatly simplify some lightning fee negotiation.

[1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
2019-07-09 15:46:25 -04:00
Carl Dong e263a343d4
test: rpc_users: Make variable names more clear. 2019-07-08 16:13:35 -04:00
Carl Dong 830dc2dd0f
test: rpc_users: Also test rpcauth.py with specified password. 2019-07-08 16:13:34 -04:00
Carl Dong c73d871799
test: rpc_users: Add function for testing auth params. 2019-07-08 16:13:33 -04:00
Carl Dong 604e2a997f
test: rpc_users: Add function for auth'd requests. 2019-07-08 16:13:32 -04:00
Wladimir J. van der Laan b5fa2319d8
Merge #15687: test: tool wallet test coverage for unexpected writes to wallet
7195fa792f test: Tool wallet test coverage for unexpected writes to wallet (Jon Atack)
3bf2b3a37b test: Split tool_wallet.py test into subtests (Jon Atack)
1eb13f09a9 test: Add log messages to test/functional/tool_wallet.py (Jon Atack)

Pull request description:

  This pull request adds test coverage in `test/functional/tool_wallet.py` to reproduce unexpected writes to the wallet as described in https://github.com/bitcoin/bitcoin/issues/15608 and serve as a benchmark for fixing the issue:

  - Wallet tool `info` unexpectedly writes to the wallet file if the wallet file permissions are read/write.

  - Wallet tool `info` raises with "Error loading . Is wallet being used by another process?" if the wallet file permissions are read-only.

  Goals:

  1. Reproduce the reported issue, define the current unexpected behavior, and add test coverage to guide a future fix. Add debug-level logging for sanity checking and commented-out assertions to be uncommented when fixing the issue. Add the same coverage to the wallet tool create test and the getwalletinfo test as regression tests while fixing the issue.

  2. Add info log messages as there are currently none in the test file.

  3. Split the tests out to separate functions as per review feedback.

  Thanks to Marco Falke for pointing me in the right direction.

ACKs for top commit:
  laanwj:
    code review ACK 7195fa792f

Tree-SHA512: 16a41cce989c8f819cf5b02c6cf8ea84653ede2738fb402f6c36cf4dc075b424dff3e2c73a1cfa1ec9c75f614675baecc71e588845a2596db06ba0957db2df7b
2019-07-08 14:57:06 +02:00
Jon Atack 7195fa792f
test: Tool wallet test coverage for unexpected writes to wallet
This commit adds test coverage in `test/functional/tool_wallet.py` to reproduce unexpected writes to the wallet as described in https://github.com/bitcoin/bitcoin/issues/15608:

    - wallet tool `info` unexpectedly writes to the wallet file if the wallet file permissions are read/write.

    - wallet tool `info` raises with "Error loading . Is wallet being used by another process?" if the wallet file permissions are read-only.

1. Reproduce the reported issue, define the current unexpected behavior, and add test coverage to guide a future fix in the form of commented-out assertions to be uncommented when testing/fixing.

2. Provisionally extend the same coverage to the wallet tool create test and the getwalletinfo test as regression tests while fixing the issue.

3. Add some logging for sanity checking.

------

Changes after rebase:

5. Make wallet_path an instance method instead of a function in tool_wallet.py as per Marco Falke review suggestion.

6. Assert wallet permissions instead of logging them in tool_wallet.py. This ran into an issue with Appveyor keeping permissions at 666 so allowed for 666 as a workaround.

7. Change the added logging from info to debug level.

8. More helpful assertions order in tool_wallet.py#assert_tool_output. This change makes #assert_tool_output raise "Error loading wallet.dat. Is wallet being used by another process?" rather than a less-helpful message when debugging the read-only wallet permissions issue.
2019-07-08 13:02:28 +02:00
Jon Atack 3bf2b3a37b
test: Split tool_wallet.py test into subtests
as per Marco Falke review suggestion.
2019-07-08 13:02:23 +02:00
Jon Atack 1eb13f09a9
test: Add log messages to test/functional/tool_wallet.py
and update code comments as per Python PEP 8 style guide.
2019-07-08 12:33:41 +02:00
fanquake 584168c7f9
Merge #16326: [RPC] add new utxoupdatepsbt arguments to the CRPCCommand and CPRCCvertParam tables
91cc18f602 [docs] Add release notes for PR 15427 (John Newbery)
3b11420b3c [RPC] add new utxoupdatepsbt arguments to the CRPCCommand and CPRCConvertParam tables (John Newbery)

Pull request description:

  The new `descriptors` argument was not added to the CRPCCommand and CPRCCvertParam tables, meaning that it couldn't be used with bitcoin-cli or named arguments.

  Before this PR:

  ```
  > bitcoin-cli utxoupdatepsbt 'cHNidP8BAFMCAAAAAYCdwVRx2X3o4KHx5tAMsN1ddp51MbfWsietjfMbl5HtAAAAAAD/////AQDh9QUAAAAAF6kUW+rtEOi4nk9rpw2F5XZl1dd8ehGHAAAAAAAAAA==' "[{\"desc\":\"sh(wpkh([bd50871a/0h/0h/0h]03895c66337b38699bfafff1084ad35bc347fac4f4e5e5fe5eb7dd81155280db53))\"}]"
  error code: -3
  error message:
  Expected type array, got string
  > bitcoin-cli --named utxoupdatepsbt psbt='cHNidP8BAFMCAAAAAYCdwVRx2X3o4KHx5tAMsN1ddp51MbfWsietjfMbl5HtAAAAAAD/////AQDh9QUAAAAAF6kUW+rtEOi4nk9rpw2F5XZl1dd8ehGHAAAAAAAAAA==' descriptors="[{\"desc\":\"sh(wpkh([bd50871a/0h/0h/0h]03895c66337b38699bfafff1084ad35bc347fac4f4e5e5fe5eb7dd81155280db53))\"}]"
  error code: -8
  error message:
  Unknown named parameter descriptors
  ```

  After this PR:

  ```
  bitcoin-cli utxoupdatepsbt 'cHNidP8BAFMCAAAAAYCdwVRx2X3o4KHx5tAMsN1ddp51MbfWsietjfMbl5HtAAAAAAD/////AQDh9QUAAAAAF6kUW+rtEOi4nk9rpw2F5XZl1dd8ehGHAAAAAAAAAA==' "[{\"desc\":\"sh(wpkh([bd50871a/0h/0h/0h]03895c66337b38699bfafff1084ad35bc347fac4f4e5e5fe5eb7dd81155280db53))\"}]"
  cHNidP8BAFMCAAAAAYCdwVRx2X3o4KHx5tAMsN1ddp51MbfWsietjfMbl5HtAAAAAAD/////AQDh9QUAAAAAF6kUW+rtEOi4nk9rpw2F5XZl1dd8ehGHAAAAAAAAAA==
  bitcoin-cli --named utxoupdatepsbt psbt='cHNidP8BAFMCAAAAAYCdwVRx2X3o4KHx5tAMsN1ddp51MbfWsietjfMbl5HtAAAAAAD/////AQDh9QUAAAAAF6kUW+rtEOi4nk9rpw2F5XZl1dd8ehGHAAAAAAAAAA==' descriptors="[{\"desc\":\"sh(wpkh([bd50871a/0h/0h/0h]03895c66337b38699bfafff1084ad35bc347fac4f4e5e5fe5eb7dd81155280db53))\"}]"
  cHNidP8BAFMCAAAAAYCdwVRx2X3o4KHx5tAMsN1ddp51MbfWsietjfMbl5HtAAAAAAD/////AQDh9QUAAAAAF6kUW+rtEOi4nk9rpw2F5XZl1dd8ehGHAAAAAAAAAA==
  ```

ACKs for top commit:
  promag:
    ACK 91cc18f.
  fanquake:
    re-ACK 91cc18f602

Tree-SHA512: 279b2339a5cac17e363002e4ab743e251d6757c904c89f1970575bdce18d4f63d5e13507e171bf2bdc1bf6dd457db345a4b11b15d4ff71b96c2fedc4ffe52b23
2019-07-07 09:51:58 +08:00
John Newbery 3b11420b3c [RPC] add new utxoupdatepsbt arguments to the CRPCCommand and CPRCConvertParam tables
The new `descriptors` argument needs to be added to the Command and
ConvertParams tables to by usable as a named argument and by
bitcoin-cli.

Also update the test to use named arguments to test this.
2019-07-04 08:02:23 -04:00
MarcoFalke 1381ddbcfc
Merge #16329: test: Add tests for getblockchaininfo.softforks
faf6caf4da test: Add tests for getblockchaininfo.softforks (MarcoFalke)

Pull request description:

ACKs for top commit:
  laanwj:
    Code review ACK faf6caf4da

Tree-SHA512: 8a0bb3b648f18fdba7a36a960d70c6217fd7312cf2ef52b3b911be0d7f1d27c5c50856946d7e6cb81d96c081913b7308cc5f9d89af34518439ff4ada024441da
2019-07-03 12:11:46 -04:00
Wladimir J. van der Laan 11de669d8b
Merge #16325: rpc: Clarify that block count means height excl genesis
fab0c820fa rpc: Clarify that block count means height excl genesis (MarcoFalke)

Pull request description:

  There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in https://github.com/bitcoin/bitcoin/pull/16292#issuecomment-506303256.

  However, it really returns the height, which is `0` for the genesis block.

  So clarify that and also remove the misleading "longest blockchain" comment.

  Finally, fix the wallet test that incorrectly used this rpc.

ACKs for top commit:
  instagibbs:
    utACK fab0c820fa
  promag:
    ACK fab0c82, sorry for the misconception.

Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
2019-07-03 14:49:40 +02:00
Wladimir J. van der Laan 9339008a9d
Merge #15483: rpc: Adding a 'logpath' entry to getrpcinfo
8a6810d0d2 Add a 'logpath' field to getrpcinfo (darosior)

Pull request description:

  as discussed in #15438

ACKs for top commit:
  laanwj:
    Tested ACK 8a6810d0d2

Tree-SHA512: 752c7d90f670677c8144efb338c5c97c2264f85f1e65e031fd5a44f04230b6eafbabd0f634db263eb42c25642ecc1c4b1b602d4735e3fab07ec00b566134ddab
2019-07-03 14:35:58 +02:00
MarcoFalke e06067387e
Merge #16250: signrawtransactionwithkey: report error when missing redeemScript/witnessScript
01174596e6 signrawtransactionwithkey: report error when missing redeemScript/witnessScript param (Anthony Towns)

Pull request description:

  Adding support for "witnessScript" as an alternative to "redeemScript" when using "signrawtransactionwithkey" meant that the `RPCTypeCheckObj()` call in `SignTransaction` can't error out just because either parameter is missing -- it's only a problem if both are missing, which isn't a state `RPCTypeCheckObj()` tests for. This results in the regression described in #16249. This patch adds some code to test for this case and give a similar error, namely:

      error code: -8
      error message:
      Missing redeemScript/witnessScript

  Fixes: #16249

ACKs for top commit:
  meshcollider:
    utACK 01174596e6
  promag:
    ACK 01174596e. Could also write test without `dict`/`del`:

Tree-SHA512: cf51346b7dea551b7f18f2a93c2a336a293b2535c62c03a5263cd2be8c58cf0cc302891da659c167e88ad1a68a756472c3c07e99f71627c61d32886fc5a3a353
2019-07-02 17:21:02 -04:00
MarcoFalke faf6caf4da
test: Add tests for getblockchaininfo.softforks 2019-07-02 15:25:13 -04:00
MarcoFalke fab0c820fa
rpc: Clarify that block count means height excl genesis 2019-07-02 12:28:21 -04:00
João Barbosa 5c1b9714cb wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction 2019-07-02 16:13:39 +01:00
Wladimir J. van der Laan 2f717fb5cd
Merge #15427: Add support for descriptors to utxoupdatepsbt
26fe9b9909 Add support for descriptors to utxoupdatepsbt (Pieter Wuille)
3135c1a2d2 Abstract out UpdatePSBTOutput from FillPSBT (Pieter Wuille)
fb90ec3c33 Abstract out EvalDescriptorStringOrObject from scantxoutset (Pieter Wuille)
eaf4f88734 Abstract out IsSegWitOutput from utxoupdatepsbt (Pieter Wuille)

Pull request description:

  This adds a descriptors argument to the `utxoupdatepsbt` RPC. This means:
  * Input and output scripts and keys will be filled in when known.
  * P2SH-witness inputs will be filled in from the UTXO set when a descriptor is provided that shows they're spending segwit outputs.

  This also moves some (newly) shared code to separate functions: `UpdatePSBTOutput` (an analogue to `SignPSBTInput`), `IsSegWitOutput`, and `EvalDescriptorStringOrObject` (implementing the string or object notation parsing used in `scantxoutset`).

ACKs for top commit:
  jnewbery:
    utACK 26fe9b9909
  laanwj:
    utACK 26fe9b9909 (will hold merging until response to promag's comments)
  promag:
    ACK 26fe9b9, checked refactors and tests look comprehensive. Still missing a release note but can be added later.

Tree-SHA512: 1d833b7351b59d6c5ded6da399ff371a8a2a6ad04c0a8f90e6e46105dc737fa6f2740b1e5340280d59e01f42896c40b720c042f44417e38dfbee6477b894b245
2019-07-02 16:53:22 +02:00
Wladimir J. van der Laan 6c21a801f3
Merge #15538: wallet_bumpfee.py: Make sure coin selection produces change
276972cb95 wallet_bumpfee.py: Make sure coin selection produces change (Gregory Sanders)

Pull request description:

  I was hitting the case where change-less transactions were being made.

ACKs for top commit:
  ryanofsky:
    utACK 276972cb95

Tree-SHA512: e2b7a50363daddd3ee749cacfc9d3d685a6c0c7e3e48118bb60131d205bf83ea06cdd66b69dfa3bd4dbb3bbf2b5b673d7225171486ae72fc762e5dabe2c01ef5
2019-07-02 14:49:26 +02:00
Steven Roose 8f250ab788
TEST: Replace hard-coded hex tx with classes 2019-07-01 18:06:01 +01:00
Wladimir J. van der Laan 1212808762
Merge #16257: [wallet] abort when attempting to fund a transaction above -maxtxfee
806b0052c3 [wallet] abort when attempting to fund a transaction above maxtxfee (Sjors Provoost)

Pull request description:

  `FundTransaction` calls `GetMinimumFee` which, when the fee rate is absurdly high, quietly reduces the fee to `-maxtxfee`.

  Becaue an absurdly high fee rate is usually the result of a fat finger, aborting seems safer behavior.

  Before:
  ```
  bitcoin-cli walletcreatefundedpsbt '[]' '[{"tb1q...": 0.01}]' 0 '{"feeRate": 10}' true
  {
    "psbt": "cHNidP8...gAA=",
    "fee": 0.10000000,
    "changepos": 1
  }

  ```

  After:
  ```
  bitcoin-cli walletcreatefundedpsbt '[]' '[{"tb1q...": 0.01}]' 0 '{"feeRate": 10}' true
  error code: -25
  error message:
  Fee exceeds maximum configured by -maxtxfee
  ```

  QT still checks the max fee rate as expected:
  <img width="566" alt="Schermafbeelding 2019-06-20 om 19 52 00" src="https://user-images.githubusercontent.com/10217/59888424-a2aa7100-9395-11e9-8ae6-8a3c1f7de585.png">

ACKs for top commit:
  laanwj:
    Code review ACK 806b0052c3

Tree-SHA512: bee95811711cdab100b614d2347921407af3b400aea613ca156953ed3f60b924ad29a1d335bd0e240c0b7c0fbb360226bab03294d226a5560cdf2a3f21e6d406
2019-07-01 16:03:37 +02:00
Sjors Provoost 806b0052c3
[wallet] abort when attempting to fund a transaction above maxtxfee
FundTransaction calls GetMinimumFee which, when the fee rate is absurdly high, quietly reduced the fee to -maxtxfee. Becaue an absurdly high fee rate is usually the result of a fat finger, aborting seems safer behavior.
2019-06-28 22:44:38 -04:00
MarcoFalke fa815255c7
test: Add missing sync_all to wallet_balance test 2019-06-28 11:19:37 -04:00
Anthony Towns 01174596e6 signrawtransactionwithkey: report error when missing redeemScript/witnessScript param 2019-06-25 12:37:08 +10:00
MeshCollider 2cbcc55ba6
Merge #16239: wallet/rpc: follow-up clean-up/fixes to avoid_reuse
71d0344cf2 docs: release note wording (Karl-Johan Alm)
3d2ff37913 wallet/rpc: use static help text (Karl-Johan Alm)
53c3c1ea9e wallet/rpc/getbalances: add entry for 'mine.used' balance in results (Karl-Johan Alm)

Pull request description:

  This addresses a few remaining issues pointed out in #13756:

  * First commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r284907468
  * Second commit addresses https://github.com/bitcoin/bitcoin/pull/13756#discussion_r294868973

  Ping jnewbery and achow101 as they pointed out these issues.

ACKs for commit 71d034:
  jnewbery:
    ACK 71d0344cf2
  meshcollider:
    re-utACK 71d0344cf2

Tree-SHA512: 5e28822af0574ad07dbbed21aa2fe7866bf5770b4c0a1c150ad0da8af3152bcfb7170330a7497fa500326c594740ecf63733cf58325821e2811d7b911d5783a0
2019-06-22 22:00:10 +12:00
MarcoFalke fa89badf88
test: Require standard txs in regtest 2019-06-21 16:45:16 -04:00
Karl-Johan Alm 53c3c1ea9e
wallet/rpc/getbalances: add entry for 'mine.used' balance in results 2019-06-22 02:45:40 +09:00
MeshCollider 303ec103ba
Merge #16026: Ensure that uncompressed public keys in a multisig always returns a legacy address
a49503402b Make and get the multisig redeemscript and destination in one function instead of two (Andrew Chow)

Pull request description:

  `CreateMultisigRedeemscript()` is changed to `AddAndGetMultisigDestination()` so that the process of constructing the redeemScript and then getting the `CTxDestination` are done in the same function. This allows that function to see what the keys in the multisig are so that the correct address type is returned from `AddAndGetDestinationForScript()`.

  This only effects the `createmultisig` and `addmultisigaddress` RPCs and does not change signing logic as #16022 does.

  Alternative to #16022 and #16012

  Fixes #16011

ACKs for commit a49503:

Tree-SHA512: 5b0154a714deea3b2cc3a54beb420c95eeeacf4ca30c40ca80940d9d640f8b03611b0fc14c2f0710bfd8a79e8d27ad7d9ae380b4b83d52b40ab201624f2a63f0
2019-06-21 19:44:08 +12:00
MarcoFalke ea45967e85
Merge #16234: test: Add test for unknown args
fa7dd88b71 test: Add test for unknown args (MarcoFalke)

Pull request description:

  Currently uncovered.

  Further reading:

  * https://marcofalke.github.io/btc_cov/total.coverage/src/util/system.cpp.gcov.html
  *  Fail on unknown config file options #15021

ACKs for commit fa7dd8:
  promag:
    ACK fa7dd88b71, tests looks good to me.
  hebasto:
    ACK fa7dd88b71, I have tested the code.

Tree-SHA512: 86ab370ce8e85925f945a52e81457b5678d71bbabcef01205a97782b780003f363552e0bad1ff678bccc784f82c6b511c3b88de3f8f25f62b0b713c387950564
2019-06-20 16:59:17 -04:00