2709 Commits

Author SHA1 Message Date
MarcoFalke
b103fdcb3b
Merge #19763: net: don't try to relay to the address' originator
7fabe0f359ae16ed36ce4ca2c33631d038c21448 net: don't relay to the address' originator (Vasil Dimov)

Pull request description:

  For each address to be relayed we "randomly" pick 2 nodes to send the
  address to (in `RelayAddress()`). However we do not take into
  consideration that it does not make sense to relay the address back to
  its originator (`CNode::PushAddress()` will do nothing in that case).

  This means that if the originator is among the "randomly" picked nodes,
  then we will relay to one node less than intended.

  Fix this by skipping the originating node when choosing candidates to
  relay to.

ACKs for top commit:
  sdaftuar:
    ACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 (this time I looked at the test, and verified the test breaks in expected ways if I break the code).
  jnewbery:
    utACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 (only net_processing changes. I haven't reviewed the test changes)
  jonatack:
    re-ACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 per `git range-diff b76abae fd897f8 7fabe0f`, change since last review is rebase and more readable Doxygen documentation

Tree-SHA512: c6a9d11c7afc97ab4e8960513f6416648d4a8c0c64b713c145a7482a7b9e54946f81386a3351e3ec0011e5594ba5ccff4d10c6f656bb80680d9f0d0a63366165
2020-12-14 13:37:21 +01:00
MarcoFalke
b18978066d
Merge #20079: p2p: Treat handshake misbehavior like unknown message
faaad1bbac46cfeb22654b4c59f0aac7a680c03a p2p: Ignore version msgs after initial version msg (MarcoFalke)
fad68afcff731153d1c83f7f56c91ecbb264b59a p2p: Ignore non-version msgs before version msg (MarcoFalke)

Pull request description:

  Handshake misbehaviour doesn't cost us more than any other unknown message, so it seems odd to treat it differently

ACKs for top commit:
  jnewbery:
    utACK faaad1bbac46cfeb22654b4c59f0aac7a680c03a
  practicalswift:
    ACK faaad1bbac46cfeb22654b4c59f0aac7a680c03a: patch looks correct

Tree-SHA512: 9f30c3b5c1f6604fd02cff878f10999956152419a3dd9825f8267cbdeff7d06787418b41c7fde8a00a5e557fe89204546e05d5689042dbf7b07fbb7eb95cddff
2020-12-12 12:32:46 +01:00
Vasil Dimov
7fabe0f359
net: don't relay to the address' originator
For each address to be relayed we "randomly" pick 2 nodes to send the
address to (in `RelayAddress()`). However we do not take into
consideration that it does not make sense to relay the address back to
its originator (`CNode::PushAddress()` will do nothing in that case).

This means that if the originator is among the "randomly" picked nodes,
then we will relay to one node less than intended.

Fix this by skipping the originating node when choosing candidates to
relay to.
2020-12-10 14:41:41 +01:00
MarcoFalke
fa918dd537
test: Use Popen.wait instead of RPC in assert_start_raises_init_error 2020-12-10 12:36:41 +01:00
Wladimir J. van der Laan
eb53c03b36
Merge #20595: Improve heuristic hex transaction decoding
0f949cde3dff15170db7930b0f7345ff995c267d Add regression test for incorrect decoding (Pieter Wuille)
39c42c442044aef611d03ee7053d2dd6df63deb7 Improve heuristic hex transaction decoding (Pieter Wuille)

Pull request description:

  The current hex tx decoding logic will refuse to decode valid extended-encoded transactions if the result fails the heuristic sanity check, even when the legacy-encoding fails. Fix this.

  Fixes #20579

ACKs for top commit:
  achow101:
    Code review ACK 0f949cde3dff15170db7930b0f7345ff995c267d
  jonatack:
    Tested ACK 0f949cde3dff15170db7930b0f7345ff995c267d
  laanwj:
    Code review ACK 0f949cde3dff15170db7930b0f7345ff995c267d

Tree-SHA512: bd6dc80d824eb9a87026a623be910cac92173f8ce1c8b040c2246348c3cf0c6d64bcc40127b859e5e4da1efe88cf02a6945f7ebb91079799395145cb09d9c7a5
2020-12-10 11:22:10 +01:00
MarcoFalke
38176dc665
Merge #20573: wallet, bugfix: allow send with string fee_rate amounts
6fa72ceb8021c3b5aea62f6cfe92665c29212923 test: add coverage for passing fee rate as a string (Jon Atack)
ce207d6b93d35bc02fcd2dd28f1fd95869261d43 wallet, bugfix: allow send to take string fee rate values (Jon Atack)

Pull request description:

  RPC send currently only accepts fee rates as numbers, which is a user-facing bug. It should accept fee rates as an amount, e.g. a string or a number, as documented in its help and like sendtoaddress, sendmany, fundrawtransaction, walletcreatefundedpsbt, and bumpfee. Provide a fix and regression test coverage.

ACKs for top commit:
  MarcoFalke:
    review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923
  achow101:
    Code review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923
  promag:
    Code review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923.

Tree-SHA512: 735f9269cb1b81953764b5283449c0b154bd62de034225be5bcedc515c84faf767fe8fe0741008679fe412922c847b00d116cb11aab775236b779c847ba87167
2020-12-10 08:46:01 +01:00
MarcoFalke
22f13c1e08
Merge #19776: net, rpc: expose high bandwidth mode state via getpeerinfo
343dc4760fd2407895fc8b3417a504b194429156 test: add test for high-bandwidth mode states in getpeerinfo (Sebastian Falbesoner)
dab6583307ceb7dd94affcc3482ddcc1a5747147 doc: release note for new getpeerinfo fields "bip152_hb_{from,to}" (Sebastian Falbesoner)
a7ed00f8bbc07dfc09f9e0a5bae10a1afe7612bb rpc: expose high-bandwidth mode states via getpeerinfo (Sebastian Falbesoner)
30bc8fab6833e0447ceadd3fff1566a680e33a98 net: save high-bandwidth mode states in CNodeStats (Sebastian Falbesoner)

Pull request description:

  Fixes #19676, "_For every peer expose through getpeerinfo RPC whether or not we selected them as HB peers, and whether or not they selected us as HB peers._" See [BIP152](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki), in particular the [protocol flow diagram](https://github.com/bitcoin/bips/raw/master/bip-0152/protocol-flow.png).  The newly introduced states are changed on the following places in the code:
  * on reception of a `SENDCMPCT` message with valid version, the field `m_highbandwidth_from` is changed depending on the first integer parameter in the message (1=high bandwidth, 0=low bandwidth), i.e. it just mirrors the field `CNodeState.fPreferHeaderAndIDs`.
  * after adding a `SENDCMPCT` message to the send queue, the field `m_highbandwidth_to` is changed depending on how the first integer parameter is set (same as above)

  Note that after receiving `VERACK`, the node also sends `SENDCMPCT`, but that is only to announce the preferred version and never selects high-bandwidth mode, hence there is no need to change the state variables there, which are initialized to `false` anyways.

ACKs for top commit:
  naumenkogs:
    reACK 343dc4760fd2407895fc8b3417a504b194429156
  jonatack:
    re-ACK 343dc4760fd2407895fc8b3417a504b194429156 per `git range-diff 7ea6499 4df1d12 343dc47`

Tree-SHA512: f4999e6a935266812c2259a9b5dc459710037d3c9e938006d282557cc225e56128f72965faffb207fc60c6531fab1206db976dd8729a69e8ca29d4835317b99f
2020-12-10 08:21:36 +01:00
Wladimir J. van der Laan
42ed7f51fa
Merge #20606: Remove unused bits from service flags enum
fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6 Remove unused bits from service flags enum (MarcoFalke)

Pull request description:

  Remove service bits that haven't been observed on the active network for years and won't ever be observed on the network with this meaning. Keeping this dead assignment in our source code forever doesn't add any value.

  I somehow forgot to do this in commit fa0d0ff6e1bee60fde63724ae28a51aac5a94d4a.

ACKs for top commit:
  laanwj:
    Code review ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6
  practicalswift:
    cr ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6
  fanquake:
    ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6

Tree-SHA512: 376e5ac05940493cf2209fea60515c843e978c4b476f2524f6bf7a37a646d237c3ddcf6c0fa23641f9ba550f625609703d9b51b4be631a7f2a90e1092b557232
2020-12-09 16:37:56 +01:00
MarcoFalke
90ef622ab5
Merge #20564: Don't send 'sendaddrv2' to pre-70016 software, and send before 'verack'
1583498fb6781c01ca2f33c09319ed793964c574 Send and require SENDADDRV2 before VERACK (Pieter Wuille)
c5a89196602e43ebb1cdc9cd4f08d153419c13e1 Don't send 'sendaddrv2' to pre-70016 software (Pieter Wuille)

Pull request description:

  BIP155 defines addrv2 and sendaddrv2 for all protocol versions, but some implementations reject messages they don't know. As a courtesy, don't send it to nodes with a version before 70016, as no software is known to support BIP155 that doesn't announce at least that protocol version number.

  Also move the sending of sendaddrv2 earlier (before sending verack), as proposed in https://github.com/bitcoin/bips/pull/1043. This has the side effect that local address broadcast of torv3 will work (as it'll only trigger after we know whether or not the peer supports addrv2).

ACKs for top commit:
  MarcoFalke:
    ACK 1583498fb6781c01ca2f33c09319ed793964c574
  jnewbery:
    ACK 1583498fb6781c01ca2f33c09319ed793964c574
  jonatack:
    ACK 1583498fb6781c01ca2f33c09319ed793964c574
  vasild:
    ACK 1583498

Tree-SHA512: 3bd5833fa8c8567b6dedd99e4a9b6bb71c127aa66d5284b217503c86d597dc59aa7382c41f3a4bf561bb658b89db81d1a7703a700eef4ffc17cb916660e23a82
2020-12-09 07:01:57 +01:00
Pieter Wuille
0f949cde3d Add regression test for incorrect decoding 2020-12-08 13:13:40 -08:00
Pieter Wuille
1583498fb6 Send and require SENDADDRV2 before VERACK
See the corresponding BIP change: https://github.com/bitcoin/bips/pull/1043
2020-12-08 09:40:10 -08:00
MarcoFalke
fa40168ab3
Remove unused bits from service flags enum 2020-12-08 18:36:51 +01:00
Wladimir J. van der Laan
d38feb6134
Merge #20535: test: Fix intermittent feature_taproot issue
fa275e1539941b49fe206ff0bf110e3362bec6bb test: Fix intermittent feature_taproot issue (MarcoFalke)

Pull request description:

  The nodes might disconnect (e.g. due to "Timeout downloading block" https://cirrus-ci.com/task/5313800947630080?command=ci#L1763) and the test fails to continue.

  Fix that by reconnecting the nodes.

ACKs for top commit:
  laanwj:
    code review ACK fa275e1539941b49fe206ff0bf110e3362bec6bb

Tree-SHA512: 2871183c8058d8292c9c4ef56ea3d19d5616ca712ebdaabb6609f8c9cd2e16c9ac2ce26aa1e94b346872b7b6fec56b59af151af83de3a5aa08bed01bfcc7187a
2020-12-07 20:37:18 +01:00
Wladimir J. van der Laan
5c4911e7e7
Merge #20568: doc: Use FeeModes doc helper in estimatesmartfee
fa8abdc9953e381715493b259908e246914793b0 rpc: Use FeeModes doc helper in estimatesmartfee (MarcoFalke)

Pull request description:

  Not sure why this doesn't use the doc helper, probably an oversight?

ACKs for top commit:
  laanwj:
    Code review ACK fa8abdc9953e381715493b259908e246914793b0

Tree-SHA512: 1f2dc8356e3476ddcf9cafafa7f9865ad95bed1e3067c0edab8e3c483e374bdbdbecc066167554b4a1b479e28f6a52c4ae6a75a70c67ee4e1ff4f3ba36b04001
2020-12-07 14:09:53 +01:00
MarcoFalke
03b1db6114
Merge #18766: Disable fee estimation in blocksonly mode (by removing the fee estimates global)
4e28753f60613ecd35cdef87bef5f99c302c3fbd feestimator: encapsulate estimation file logic (Antoine Poinsot)
e8ea6ad9c16997bdc7e22a20eca16e234290b7ff init: don't create a CBlockPolicyEstimator if we don't relay transactions (Antoine Poinsot)
86ff2cf202bfb9d9b50800b8ffe3fead3f77f5fa Remove the remaining fee estimation globals (Antoine Poinsot)
03bfeee957ab7e3b6aece82b9561774648094f54 interface: remove unused estimateSmartFee method from node (Antoine Poinsot)

Pull request description:

  If the `blocksonly` mode is turned on after running with transaction
  relay enabled for a while, the fee estimation will serve outdated data
  to both the internal wallet and to external applications that might be
  feerate-sensitive and make use of `estimatesmartfee` (for example a
  Lightning Network node).

  This has already caused issues (for example https://github.com/bitcoin/bitcoin/issues/16840 (C-lightning), or https://github.com/lightningnetwork/lnd/issues/2562 (LND)) and it seems prudent to fail rather than to give inaccurate values.

  This fixes #16840, and closes #16890 which tried to fix the symptoms (RPC) but not the cause as mentioned by sdaftuar :
  > If this is a substantial problem, then I would think we should take action to protect our own wallet users as well (rather than hide the results of what our fee estimation would do!).

ACKs for top commit:
  MarcoFalke:
    re-ACK 4e28753f60 👋
  jnewbery:
    utACK 4e28753f60613ecd35cdef87bef5f99c302c3fbd

Tree-SHA512: c869cf03b86d8194002970bbc84662dae76874967949b9be0d9a4511a1eabcb1627c38aca3154da9dcece1a4c49ec02bd4f9fcca2ec310986e07904559e63ba8
2020-12-07 12:59:48 +01:00
MarcoFalke
fa8abdc995
rpc: Use FeeModes doc helper in estimatesmartfee
Can be reviewed with --ignore-all-space
2020-12-07 09:28:47 +01:00
Jonas Schnelli
eab63b971d
Merge #19847: rpc, refactor: Avoid duplicate set lookup in gettxoutproof
52fc39917fc52c2ff279fe434431e18900b347bd rpc: Reject empty txids in gettxoutproof (João Barbosa)
73dc19a330f8cb063af46e6c4246f2e64a04bdc1 rpc, refactor: Avoid duplicate set lookup in gettxoutproof (João Barbosa)

Pull request description:

ACKs for top commit:
  jonasschnelli:
    code review ACK 52fc39917fc52c2ff279fe434431e18900b347bd

Tree-SHA512: 76b18e5235e8b2d394685515a4a60335666eeb0f6b31c1d397f7db2fbe681bc817b8cd3e8f6708b9dacd6113e4e1d94837072cae27834b8a1a22d2717db8191e
2020-12-07 09:17:08 +01:00
MarcoFalke
64156ad4d1
Merge #19893: test: Remove or explain syncwithvalidationinterfacequeue
fa6af312277bb1b7e57d9b764d411c5b0873829f test: Document why syncwithvalidationinterfacequeue is needed in tests (MarcoFalke)
fa135a13b8ddaa117bd090ec43a3eab3a95755c1 Revert "test: Add missing sync_all to wallet_balance test" (MarcoFalke)

Pull request description:

  syncwithvalidationinterfacequeue is a hidden test-only RPC, so it should not be used when it is not needed. Thus, either remove it or explain why it is needed.

ACKs for top commit:
  fjahr:
    Code review ACK fa6af312277bb1b7e57d9b764d411c5b0873829f

Tree-SHA512: de30db4ab521184091ee5beeab02989138cf7cf05088f766a2fb106151b239310b63d5380cb79e2a072f72c5ae9513aecae8eb9c1c7be713771585c3cb04d63a
2020-12-06 19:13:10 +01:00
Jon Atack
6fa72ceb80
test: add coverage for passing fee rate as a string 2020-12-04 22:35:31 +01:00
Jon Atack
ce207d6b93
wallet, bugfix: allow send to take string fee rate values 2020-12-04 22:12:36 +01:00
João Barbosa
52fc39917f rpc: Reject empty txids in gettxoutproof 2020-12-04 18:38:23 +00:00
Andrew Chow
773c42b265 tests: Test that a fully signed tx given to signrawtx is unchanged
Tests that a fully signed transaction given to
signrawtransactionwithwallet is both unchanged and marked as complete.
This tests for a regression in 0.20 where the transaction would not be
marked as complete.
2020-12-03 15:53:58 -05:00
Antoine Poinsot
e8ea6ad9c1
init: don't create a CBlockPolicyEstimator if we don't relay transactions
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-12-03 12:56:37 +01:00
MarcoFalke
681ce59d0e
Merge #20466: test: Fix intermittent p2p_fingerprint issue
fad7be584ffaf8099cc099d9378ba831c9483260 test: Fix intermittent p2p_finerprint issue (MarcoFalke)

Pull request description:

  A single sync_with_ping can't be used to drop a block announcement, as the block might be sent *after* the ping has been responded to.

  Fix that by waiting for the block.

ACKs for top commit:
  theStack:
    ACK fad7be584ffaf8099cc099d9378ba831c9483260

Tree-SHA512: d43ba9d07273486858f65a26326cc6637ef743bf7b400e5048ba7eac266fb1893283e6503dd49f179caa1abab2977315fb70ba9fba34be9a817a74259d8e4034
2020-12-03 10:09:30 +01:00
MarcoFalke
283f22cabb
Merge #20461: rpc: Validate -rpcauth arguments
053b4fbad8729308774d5e7b53f53c12627fa88b doc: Release note regarding -rpcauth validation (João Barbosa)
46001323b1f4a57d8d6805f1bc39a5b8d401f0c5 rpc: Validate -rpcauth arguments (João Barbosa)
d37c813a43166f559a4e2d1c22e7243f70301291 rpc: Refactor to process -rpcauth once (João Barbosa)

Pull request description:

  Invalid `-rpcauth` arguments are currently silently ignored. This make server initialization fail if any `-rpcauth` is invalid.

ACKs for top commit:
  MarcoFalke:
    review ACK 053b4fbad8729308774d5e7b53f53c12627fa88b
  jonatack:
    ACK 053b4fbad8729308774d5e7b53f53c12627fa88b
  ryanofsky:
    Code review ACK 053b4fbad8729308774d5e7b53f53c12627fa88b. Only changes since last review are moving a variable declaration and adding a comment, release notes, and a `const`.

Tree-SHA512: c99923d4a121f0c9f882b07f5402ea53e9b2d9455ad34468a094ffab1d64df26c82e1279734c0d42bc2e113eae7b581fbc3be52f3ed4a2d7450d11793afcf406
2020-12-02 09:37:37 +01:00
MarcoFalke
607c844f37
Merge #20540: test: Fix wallet_multiwallet issue on windows
fada2dfcac1c4b47ee76b877d91d515cf1d36410 test: Fix wallet_multiwallet issue on windows (MarcoFalke)

Pull request description:

  The error message on windows:

  > 2020-11-30T18:10:47.536032Z ListWalletDir: Error scanning C:\Users\user\AppData\Local\Temp\test_runner_₿_🏃_20201130_181042\wallet_multiwallet_0\node0\regtest\wallets\self_walletdat_symlink: boost::filesystem::status: The name of the file cannot be resolved by the system: "C:\Users\user\AppData\Local\Temp\test_runner_₿_🏃_20201130_181042\wallet_multiwallet_0\node0\regtest\wallets\self_walletdat_symlink\wallet.dat"

ACKs for top commit:
  promag:
    Code review ACK fada2dfcac1c4b47ee76b877d91d515cf1d36410. Although it could ignore (don't log) directories that lead to no permission error.
  fanquake:
    ACK fada2dfcac1c4b47ee76b877d91d515cf1d36410

Tree-SHA512: b475162cc3cd1574209d916605b229a79c8089714295f5e16569b71f958f0007d54dc76833938492d931387784588b11b73e3ef00f963540af42c079417f8d72
2020-12-02 08:31:02 +01:00
MarcoFalke
f17e8ba3a1
Merge #20207: Follow-up extra comments on taproot code and tests
2d8099c713dfd4b546150fd53c2e4f364b9009f4 Mention units of MAX_STANDARD_ policy constants (Pieter Wuille)
84e29c7c0141b52044020ec0c5dfa8a462b7e97f Mention in validation that IsWitnessStandard tests for P2TR (Pieter Wuille)
f867cbcc268a3bfaeef5510a7e40e6d3c0818b6d Clean up assets test minimizer LDFLAGS (Pieter Wuille)
ea0e78677bdbe3313f594118c500cf7784c56970 Document additional IsWitnessStandard behavior (Pieter Wuille)
6040de9a46725826330cd63cdf76e2121a18e728 Add comments on CPubKey::IsValid (Pieter Wuille)
8dbb7de67ce0a71f5fc54289c0ff048ac8dd0acc Add comments to VerifyTaprootCommitment (Pieter Wuille)
cdf900cbf26db05c7edb398ea645f1d23049d810 Document need_vin_vout_mismatch argument to make_spender (Pieter Wuille)
18246ed5f09dd078fa1410b7ec2ba4379cc5e032 Fix and improve taproot_construct comments (Pieter Wuille)

Pull request description:

  Addressing some review comments raised here: https://github.com/bitcoin/bitcoin/pull/19953#pullrequestreview-512238027 and https://github.com/bitcoin/bitcoin/pull/19953#pullrequestreview-513499921

ACKs for top commit:
  jonatack:
    ACK 2d8099c per `git range-diff 5009159 4f10965 2d8099c`
  ariard:
    ACK 2d8099c, only changes are comment light improvements on IsValid/IsWitnessStandard.

Tree-SHA512: c4881546c379ea8efc7ef99a43cbf3b9cd3f9dde5fd97a07ee66f2b593c78aef0bd8784853c5c9c737b66c269241a1048bbbdd6c964a3d872efd8ba0ec410b68
2020-12-01 15:11:51 +01:00
MarcoFalke
fada2dfcac
test: Fix wallet_multiwallet issue on windows 2020-12-01 13:38:23 +01:00
MarcoFalke
fa275e1539
test: Fix intermittent feature_taproot issue 2020-11-30 20:45:09 +01:00
MarcoFalke
7ae86b3c68
Merge #20522: [test] Fix sync issue in disconnect_p2ps
3ebde2143aa98af213872b98b474d904e55056f7 [test] Fix wait condition in disconnect_p2ps (Amiti Uttarwar)

Pull request description:

  #19315 currently has a [test failure](https://cirrus-ci.com/task/4545582645641216) because of a race. `disconnect_p2ps` is intended to have a `wait_until` clause that prevents this race, but the conditional doesn't match since its comparing two different object types. `MY_SUBVERSION` is defined in messages.py as a byte string, but is compared to the value returned by the RPC. This PR simply converts types to ensure they match, which should prevent the race from occurring.

  HUGE PROPS TO jnewbery for discovering the issue 🔎

ACKs for top commit:
  jnewbery:
    ACK 3ebde2143aa98af213872b98b474d904e55056f7
  glozow:
    Code review ACK 3ebde2143a

Tree-SHA512: ca096b80a3e4d757a645f38846d6dc89d6a3d35c3435513a72d278e305faddd4aff9e75a767941b51b2abbf59c82679bac1e9a0140d6f285efe3053e51bcc2a8
2020-11-28 18:06:43 +01:00
MarcoFalke
1ae5758981
Merge #20448: RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint wallet
89bdad5b25ae4ac03a486f729a5b58ae6f21946d RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint (Luke Dashjr)

Pull request description:

  Allow specifying the `wallet_name` param to `unloadwallet` on RPC wallet endpoints, so long as it matches the endpoint wallet.

ACKs for top commit:
  jonatack:
    ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d
  MarcoFalke:
    review ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d

Tree-SHA512: efb399c33f7b5596870a26a8680f453ca47aa7a6db4e550f9435d13044f1c4bad0ae11e8f0205213409d08b75c4188c3be782e54aafab1f65b97eb8cf5c252a9
2020-11-28 10:14:45 +01:00
Amiti Uttarwar
3ebde2143a [test] Fix wait condition in disconnect_p2ps
MY_SUBVERSION is defined in messages.py as a byte string, but here we were
comparing this value to the value returned by the RPC. Convert to ensure the
types match.
2020-11-27 14:54:55 -08:00
Pieter Wuille
cdf900cbf2 Document need_vin_vout_mismatch argument to make_spender 2020-11-26 14:56:25 -08:00
Pieter Wuille
18246ed5f0 Fix and improve taproot_construct comments 2020-11-26 14:56:25 -08:00
MarcoFalke
afdfd3c8c1
Merge #20403: wallet: upgradewallet fixes, improvements, test coverage
3eb6f8b2e61c24a22ea9396d86672307845f35eb wallet (not for backport): improve upgradewallet error messages (Jon Atack)
ca8cd893bb56bf5d455154b0498b1f58f77d20ed wallet: fix and improve upgradewallet error responses (Jon Atack)
99d56e357159c7154f69f28cb5587c5ca20d6594 wallet: fix and improve upgradewallet result responses (Jon Atack)
2498b04ce88696a3216fc38b7d393906b733e8b1 Don't upgrade to HD split if it is already supported (Andrew Chow)
c46c18b788cb0862aafbb116fd37936cbed6a431 wallet: refactor GetClosestWalletFeature() (Jon Atack)

Pull request description:

  This follows up on #18836 and #20282 to fix and improve the as-yet unreleased `upgradewallet` feature and also implement review follow-up in https://github.com/bitcoin/bitcoin/pull/18836#discussion_r519328607.

  This PR fixes 4 upgradewallet issues:

  - this bug: https://github.com/bitcoin/bitcoin/pull/20403#discussion_r526063920
  - it returns nothing in the absence of an RPC error, which isn't reassuring for users
  - it returns the same thing both in the case of a successful upgrade and when no upgrade took place
  - the error message object is currently dead code

  This PR fixes the above and provides:

  ...user feedback to not silently return without upgrading
  ```
  {
    "wallet_name": "disable private keys",
    "previous_version": 169900,
    "current_version": 169900,
    "result": "Already at latest version. Wallet version unchanged."
  }
  ```
  ...better feedback after successfully upgrading
  ```
  {
    "wallet_name": "watch-only",
    "previous_version": 159900,
    "current_version": 169900,
    "result": "Wallet upgraded successfully from version 159900 to version 169900."
  }
  ```
  ...helpful error responses
  ```
  {
    "wallet_name": "blank",
    "previous_version": 169900,
    "current_version": 169900,
    "error": "Cannot downgrade wallet from version 169900 to version 159900. Wallet version unchanged."
  }
  {
    "wallet_name": "blank",
    "previous_version": 130000,
    "current_version": 130000,
    "error": "Cannot upgrade a non HD split wallet from version 130000 to version 169899 without upgrading to support pre-split keypool. Please use version 169900 or no version specified."
  }
  ```
  updated help:
  ```
  upgradewallet ( version )

  Upgrade the wallet. Upgrades to the latest version if no version number is specified.
  New keys may be generated and a new wallet backup will need to be made.
  Arguments:
  1. version    (numeric, optional, default=169900) The version number to upgrade to. Default is the latest wallet version.

  Result:
  {                            (json object)
    "wallet_name" : "str",     (string) Name of wallet this operation was performed on
    "previous_version" : n,    (numeric) Version of wallet before this operation
    "current_version" : n,     (numeric) Version of wallet after this operation
    "result" : "str",          (string, optional) Description of result, if no error
    "error" : "str"            (string, optional) Error message (if there is one)
  }
  ```

ACKs for top commit:
  achow101:
    ACK  3eb6f8b
  MarcoFalke:
    review ACK 3eb6f8b2e61c24a22ea9396d86672307845f35eb 🛡

Tree-SHA512: b767314069e26b5933b123acfea6aa40708507f504bdb22884da020a4ca1332af38a7072b061e36281533af9f4e236d94d3c129daf6fe5b55241127537038eed
2020-11-25 12:46:27 +01:00
MarcoFalke
efd4cdb81f
Merge #20456: test: Fix intermittent issue in mempool_compatibility
fa05d19bd6ba619bb3f9aabc05c439cd18d34544 test: Fix intermittent issue in mempool_compatibility (MarcoFalke)

Pull request description:

  Fixes https://cirrus-ci.com/task/5141306890518528?command=ci#L6076

  The version is too old to understand getmempoolinfo()[loaded], so it is never called when starting (See https://cirrus-ci.com/task/5141306890518528?command=ci#L5541)

ACKs for top commit:
  achow101:
    ACK fa05d19bd6ba619bb3f9aabc05c439cd18d34544

Tree-SHA512: e912d5dff6236d2d4ac608f9f3b3e255cc2b611f36d79bfe4e2a940709833a947c682d7703327887e1753eab30b95eb2615d7e2c21ce4bca4f089e717cbb51c4
2020-11-25 08:19:59 +01:00
MarcoFalke
ca4a784942
Merge #20410: wallet: Do not treat default constructed types as None-type
fa69c2c78455fd0dc436018fece9ff7fc83a180d wallet: Do not treat default constructed types as None-type (MarcoFalke)
fac4e136fa3d0fab7fde900a6be921313e16e7a6 refactor: Change pointer to reference because it can not be null (MarcoFalke)

Pull request description:

  Equating `0==None` and `""==None` is confusing, unneeded and undocumented

ACKs for top commit:
  jonatack:
    ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
  achow101:
    ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
  Sjors:
    tACK fa69c2c78455fd0dc436018fece9ff7fc83a180d modulo `unset`

Tree-SHA512: c4c8d0ad80c6697621d356a9545caf28ca2facc82bb2fa8e70eceb52372d25f0685237c73688c4b01da0e75d213c77c0d45011a8bdfe81ea783d85f045786dac
2020-11-25 08:02:19 +01:00
MarcoFalke
3a32b62fa7
Merge #20462: RPC/Wallet: unloadwallet: Clarify docs/error when both the RPC request and wallet_name parameter specify a wallet
b1f59d55d920d2b35269b474762f94fec87bfb16 RPC/Wallet: unloadwallet: Clarify docs/error when both the RPC endpoint and wallet_name parameter specify a wallet (Luke Dashjr)

Pull request description:

  Just documentation clarifications from #20448

ACKs for top commit:
  MarcoFalke:
    review ACK b1f59d55d920d2b35269b474762f94fec87bfb16
  jonatack:
    re-ACK b1f59d55d920d2b35269b474762f94fec87bfb16  per `git diff e8303a0 b1f59d5`

Tree-SHA512: ac068b0aa7ceed49496367fdd9425b59dbba18b56e89b26afc22a6c8ece51f0b92a169cacd55740b1cadab2b32f4f8e8700e609066ab7e59d3b53c7891da585e
2020-11-24 12:07:09 +01:00
Luke Dashjr
89bdad5b25 RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint 2020-11-24 05:33:18 +00:00
Luke Dashjr
b1f59d55d9 RPC/Wallet: unloadwallet: Clarify docs/error when both the RPC endpoint and wallet_name parameter specify a wallet 2020-11-24 05:31:58 +00:00
João Barbosa
46001323b1 rpc: Validate -rpcauth arguments 2020-11-23 21:02:54 +00:00
MarcoFalke
fad7be584f
test: Fix intermittent p2p_finerprint issue 2020-11-23 20:58:50 +01:00
Sjors Provoost
b87caf10b5
test: add is_bdb_compiled helper 2020-11-23 11:30:55 +01:00
MarcoFalke
fa05d19bd6
test: Fix intermittent issue in mempool_compatibility 2020-11-23 09:27:36 +01:00
MarcoFalke
816132e6eb
Merge #20426: wallet: allow zero-fee fundrawtransaction/walletcreatefundedpsbt and other fixes
9f08780dd7946b63476e9736745131db8e7f4e93 Use the correct incremental fee constant in bumpfee help (Jon Atack)
3f1e10b2b1cd11f7112fbad6355464bd4adbbc5c Update feeRate (BTC/kvB) to fee_rate (sat/vB) in wallet_bumpfee (Jon Atack)
1b3d7009280595108eb22ac1188bc4367804fc5d Allow zero-fee fundrawtxn and walletcreatefundedpsbt calls (Jon Atack)

Pull request description:

  - Fixes https://github.com/bitcoin/bitcoin/pull/20305/files#r525406176. A check to raise an error on zero-fee txns was mistakenly extended in a0d4957 from the bumpfee and send{toaddress, many} RPCs to also include fundrawtransaction and walletcreatefundedpsbt. This commit re-overrides zero fee rate checking for these two RPCs, not only for the feeRate (BTC/kvB) arg to return to previous behavior, but also for the new fee_rate (sat/vB) arg. Negative fee rates will still raise "amount out of range" by the MoneyRange check in src/bitcoin-tx.cpp::AmountFromValue.

  - Updates a wallet bumpfee test from feeRate (BTC/kvB) to fee_rate (sat/vB)

  - Fixes https://github.com/bitcoin/bitcoin/pull/20305/files#r525405363 to use the correct incremental fee rate constant in the bumpfee help  (thanks Marco Falke for the catch) and rectifies "1.000 sat/vB sat/vB" in the help to "1.000 sat/vB"

ACKs for top commit:
  MarcoFalke:
    review ACK 9f08780dd7946b63476e9736745131db8e7f4e93  🐾
  promag:
    Code review ACK 9f08780dd7946b63476e9736745131db8e7f4e93.
  Xekyo:
    Code review reACK 9f08780dd7946b63476e9736745131db8e7f4e93.

Tree-SHA512: 413dfb4f23ebaf3d2ef210dd04610a843272e64eabba428699f5de4d646a86ac4911dab66b5e2f5ebea53b76e4be8347ef40824c1592c750d5eaa12579d3cdf6
2020-11-21 07:47:40 +01:00
Jon Atack
3f1e10b2b1
Update feeRate (BTC/kvB) to fee_rate (sat/vB) in wallet_bumpfee
as the feeRate argument should soon be deprecated.

Also loosen one test (and a similar one) that caused a one-off CI failure with:
expected message
'Insufficient total fee 0.00000141, must be at least 0.00001704 (oldFee 0.00000999 + incrementalFee 0.00000705)'
actual message
'Insufficient total fee 0.00000141, must be at least 0.00001712 (oldFee 0.00001007 + incrementalFee 0.00000705)'
2020-11-20 09:40:47 +01:00
Jon Atack
1b3d700928
Allow zero-fee fundrawtxn and walletcreatefundedpsbt calls
A check to raise an error on zero-fee txns was mistakenly extended in commit
a0d4957 from the bumpfee and send{toaddress, many} RPCs to also include
fundrawtransaction and walletcreatefundedpsbt.

This commit overrides zero fee rate checking for these two RPCs, not only for
the feeRate (BTC/kvB) arg to return to previous behavior, but also for the new
fee_rate (sat/vB) arg.
2020-11-20 09:40:44 +01:00
MarcoFalke
3d632c3f49
Merge #20428: tests: shrink feature_taproot transfer of funds tx
7ffac12545328cadd92a3caec4f1c6ca7c127493 tests: shrink feature_taproot transfer of funds tx (Anthony Towns)

Pull request description:

  When moving funds from node 1 to node 0 for the pre-activation tests, there can be a large number of inputs, potentially resulting in a tx that is larger than standardness rules allow, or that takes a long time to sign. This just takes the top 500 outputs, which is plenty (~90% of the wallet balance).

ACKs for top commit:
  luke-jr:
    utACK 7ffac12545328cadd92a3caec4f1c6ca7c127493
  MarcoFalke:
    cr ACK 7ffac12545328cadd92a3caec4f1c6ca7c127493

Tree-SHA512: 68445b4827dddb9a8e8614d61a68f5bbd7152557bf940be0a75741cb49deeff1566198da1a777ac66cb3fed93e64a30bf895875d6cc6ae9e48275e3febb620a6
2020-11-20 09:30:12 +01:00
Anthony Towns
7ffac12545 tests: shrink feature_taproot transfer of funds tx 2020-11-20 07:29:42 +10:00
Jon Atack
3eb6f8b2e6
wallet (not for backport): improve upgradewallet error messages 2020-11-19 20:00:56 +01:00