Commit graph

3159 commits

Author SHA1 Message Date
MarcoFalke
143bd108ed
Merge #19137: wallettool: Add dump and createfromdump commands
23cac24dd3 tests: Test bitcoin-wallet dump and createfromdump (Andrew Chow)
a88c320041 wallettool: Add createfromdump command (Andrew Chow)
e1e7a90d5f wallettool: Add dump command (Andrew Chow)

Pull request description:

  Adds two commands to the `bitcoin-wallet` tool: `dump` and `createfromdump`. These commands will be useful for a wallet storage migration in the future. It is also generally useful to have a storage agnostic dump like this. These commands are similar to BDB's `db_dump` and `db_load` tools. This can also be useful for manual construction of a wallet file for tests.

  `dump` outputs every key-value pair from the wallet as comma separated hex. Each key-value pair is on its own line with the key and value in hex separated by a comma. This is output to the file specified by the new `-dumpfile` option.

  `createfromdump` takes a file produced by `dump` and creates a new wallet file with exactly the records specified in that file.

  A new option `-dumpfile` is added to the wallet tool. When used with `dump`, the records will be written to the specified file. When used with `createfromdump`, the file is read and the key-value pairs constructed from it. `createfromdump` requires `-dumpfile`.

  A simple round-trip test is added to the `tool_wallet.py`.

  This PR is based on #19334,

ACKs for top commit:
  Sjors:
    re-utACK 23cac24
  MarcoFalke:
    re review ACK 23cac24dd3 only change is rebase and removing useless shared_ptr wrapper 🎼
  ryanofsky:
    Code review ACK 23cac24dd3. Only changes since last review rebase and changing a pointer to a reference

Tree-SHA512: 2d63cf62baca3d16495aa698dc02f7d889c81b41015e9c92c23c275bb4a690fc176d351c3fd7f310bd6b17f5a936cc9be694cbecd702af741b96c0f530e72fa2
2020-12-17 15:18:37 +01:00
Wladimir J. van der Laan
5b6f970e3f
Merge #20171: Add functional test test_txid_inv_delay
bc4a230087 Remove redundant p2p lock tacking for tx download functional tests (Antoine Riard)
d3b5eac9a9 Add mutation for functional test test_preferred_inv (Antoine Riard)
06efb3163c Add functional test test_txid_inv_delay (Antoine Riard)
a07910abcd test: Makes wtxidrelay support a generic P2PInterface option (Antoine Riard)

Pull request description:

  This is a simple functional test to increase coverage of #19988, checking that txid announcements from txid-relay peers are delayed by TXID_RELAY_DELAY, assuming we have at least another wtxid-relay peer.

  You can verify new test with the following diff :

  ```
  diff --git a/src/net_processing.cpp b/src/net_processing.cpp
  index f14db379f..2a2805df5 100644
  --- a/src/net_processing.cpp
  +++ b/src/net_processing.cpp
  @@ -773,7 +773,7 @@ void PeerManager::AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, std
       auto delay = std::chrono::microseconds{0};
       const bool preferred = state->fPreferredDownload;
       if (!preferred) delay += NONPREF_PEER_TX_DELAY;
  -    if (!gtxid.IsWtxid() && g_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY;
  +    //if (!gtxid.IsWtxid() && g_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY;
       const bool overloaded = !node.HasPermission(PF_RELAY) &&
           m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT;
       if (overloaded) delay += OVERLOADED_PEER_TX_DELAY;
  ```

ACKs for top commit:
  laanwj:
    ACK bc4a230087

Tree-SHA512: 150e806bc5289feda94738756ab375c7fdd23c80c12bd417d3112043e26a91a717dc325a01079ebd02a88b90975ead5bd397ec86eb745c7870ebec379a8aa711
2020-12-16 18:45:11 +01:00
Andrew Chow
23cac24dd3 tests: Test bitcoin-wallet dump and createfromdump 2020-12-16 12:33:09 -05:00
MarcoFalke
69f1ee1922
Merge #20365: wallettool: add parameter to create descriptors wallet
173cc9b7be test: walettool create descriptors (Ivan Metlushko)
345e88eecf wallettool: add param to create descriptors wallet (Ivan Metlushko)
6d3af3ab62 wallettool: pass in DatabaseOptions into MakeWallet (Ivan Metlushko)

Pull request description:

  Rationale: expose and promote descriptor wallets in more places; make cli tool more consistent with `createwallet` rpc.

  Add `-descriptors` parameter which is off by default. When specified it will create a new descriptors wallet with sqlite backend, which is consistent with `createwallet` rpc.

  This PR is based on a suggestion from **ryanofsky** https://github.com/bitcoin/bitcoin/pull/19137#discussion_r516779603

  Example:
  ```
  $ ./src/bitcoin-wallet  -wallet=fewty -descriptors create
  Topping up keypool...
  Wallet info
  ===========
  Name: fewty
  Format: sqlite
  Descriptors: yes
  Encrypted: no
  HD (hd seed available): yes
  Keypool Size: 6000
  Transactions: 0
  Address Book: 0
  ```
  ```
  $ ./src/bitcoin-wallet  -wallet=fewty create
  Topping up keypool...
  Wallet info
  ===========
  Name: fewty
  Format: bdb
  Descriptors: no
  Encrypted: no
  HD (hd seed available): yes
  Keypool Size: 2000
  Transactions: 0
  Address Book: 0
  ```

ACKs for top commit:
  achow101:
    ACK 173cc9b7be
  ryanofsky:
    Code review ACK 173cc9b7be. This seems pretty nicely implemented now, with opportunities to clean up more and dedup later
  MarcoFalke:
    Concept ACK 173cc9b7be 🌠

Tree-SHA512: cc32ba336ff709de2707ee15f495b4617908e8700ede8401a58e894f44cda485c544d644023c9a6604d88a62db9d92152383ee2e8abf691688c25cf6e222c622
2020-12-16 17:43:20 +01:00
MarcoFalke
9dbcd37105
Merge #20569: test: Fix intermittent wallet_multiwallet issue with got_loading_error
fab48da908 test: Fix intermittent wallet_multiwallet issue with got_loading_error (MarcoFalke)
fa8e15f7b7 test: pep8 wallet_multiwallet.py (MarcoFalke)

Pull request description:

  Failing the test after 10 iterations without a loading error is problematic because it may take 11 iterations to get a loading error.

  Fix that by running until a loading error occurs, which should happen in almost all runs within the first 10 iterations.

ACKs for top commit:
  ryanofsky:
    Code review ACK fab48da908. This seems like a good workaround. I think more ideally think load and unload RPCs would not have racy status reporting (suggested previously https://github.com/bitcoin/bitcoin/pull/19300#pullrequestreview-435362710 and

Tree-SHA512: 6b80b26d916276efe2a01af93bca7dbf71a3e67db9d3deb15175070719bf7d1325a1410d93e74c0316942e388faa2ba185dc9d3759c82d1c73c3c509b9997f05
2020-12-16 15:47:23 +01:00
MarcoFalke
a023094fc4
Merge #20276: test: run mempool_expiry.py even with wallet disabled
3b064fcb9d test: run mempool_expiry.py even with wallet disabled (Michael Dietz)

Pull request description:

  Run the mempool expiry test even when the wallet was not compiled, as proposed in https://github.com/bitcoin/bitcoin/issues/20078.

ACKs for top commit:
  MarcoFalke:
    ACK 3b064fcb9d

Tree-SHA512: 5860dc021d02bc3752268ec1e859505bec87174953223b34b1af8a8e4ab66d645458fbf9571c0b816a9de891c3ff41314996e580869671fccd6972c093e78154
2020-12-16 14:52:34 +01:00
MarcoFalke
8bb40d5f56
Merge #20560: fuzz: Link all targets once
fa13e1b0c5 build: Add option --enable-danger-fuzz-link-all (MarcoFalke)
44444ba759 fuzz: Link all targets once (MarcoFalke)

Pull request description:

  Currently the linker is invoked more than 150 times when compiling with `--enable-fuzz`. This is problematic for several reasons:

  * It wastes disk space north of 20 GB, as all libraries and sanitizers are linked more than 150 times
  * It wastes CPU time, as the link step can practically not be cached (similar to ccache for object files)
  * It makes it a blocker to compile the fuzz tests by default for non-fuzz builds #19388, for the aforementioned reasons
  * The build file is several thousand lines of code, without doing anything meaningful except listing each fuzz target in a highly verbose manner
  * It makes writing new fuzz tests unnecessarily hard, as build system knowledge is required; Compare that to boost unit tests, which can be added by simply editing an existing cpp file
  * It encourages fuzz tests that re-use the `buffer` or assume the `buffer` to be concatenations of seeds, which increases complexity of seeds and complexity for the fuzz engine to explore; Thus reducing the effectiveness of the affected fuzz targets

  Fixes #20088

ACKs for top commit:
  practicalswift:
    Tested ACK fa13e1b0c5
  sipa:
    ACK fa13e1b0c5. Reviewed the code changes, and tested the 3 different test_runner.py modes (run once, merge, generate). I also tested building with the new --enable-danger-fuzz-link-all

Tree-SHA512: 962ab33269ebd51810924c51266ecc62edd6ddf2fcd9a8c359ed906766f58c3f73c223f8d3cc49f2c60f0053f65e8bdd86ce9c19e673f8c2b3cd676e913f2642
2020-12-15 19:00:36 +01:00
MarcoFalke
b103fdcb3b
Merge #19763: net: don't try to relay to the address' originator
7fabe0f359 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 7fabe0f359 (this time I looked at the test, and verified the test breaks in expected ways if I break the code).
  jnewbery:
    utACK 7fabe0f359 (only net_processing changes. I haven't reviewed the test changes)
  jonatack:
    re-ACK 7fabe0f359 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
faaad1bbac p2p: Ignore version msgs after initial version msg (MarcoFalke)
fad68afcff 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 faaad1bbac
  practicalswift:
    ACK faaad1bbac: patch looks correct

Tree-SHA512: 9f30c3b5c1f6604fd02cff878f10999956152419a3dd9825f8267cbdeff7d06787418b41c7fde8a00a5e557fe89204546e05d5689042dbf7b07fbb7eb95cddff
2020-12-12 12:32:46 +01:00
Hennadii Stepanov
e1e68b6305
test: Fix inconsistent lock order in wallet_tests/CreateWallet 2020-12-10 20:49:06 +02:00
MarcoFalke
da957cd62e
Merge #20613: test: Use Popen.wait instead of RPC in assert_start_raises_init_error
fa918dd537 test: Use Popen.wait instead of RPC in assert_start_raises_init_error (MarcoFalke)

Pull request description:

  Using RPC (`wait_for_rpc_connection`) has several issue:

  * It polls in a loop, which might be slow
  * It tries to read the RPC cookie file, which might not be present, thus leading to intermittent issues

  Fix both by using `Popen.wait`

ACKs for top commit:
  laanwj:
    Code review ACK ~~faf7b05be9c86ee61c39e5314511fe2410128a6b~~ fa918dd537
  darosior:
    ACK fa918dd537

Tree-SHA512: 5368ad0d0ea2deb0af9582a42667c9290efe8f2705f37a236afc2c7908b04265ab342e2dd356a57156e99389f4a27ab6da9fa7bf9161fb7568240aa005e693b9
2020-12-10 18:38:18 +01:00
Michael Dietz
3b064fcb9d
test: run mempool_expiry.py even with wallet disabled
Test coverage is also extended in this commit to:
Ensure that another transaction in the mempool is not evicted
when other transactions expire. Note: this other transaction does
not have any ancestors in the mempool that expired. Otherwise it
would be evicted from the mempool, and we already test this case.
2020-12-10 10:14:35 -06: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
e20b488395
Merge #20567: test: Add option to git-subtree-check to do full check, add help
34c80d9eee test: Add option to git-subtree-check to do full check, add help (Wladimir J. van der Laan)

Pull request description:

  This adds a brief help text to `git-subtree-check.sh` and adds an option to do a full remote check instead of having two different code paths with a successful exit status. Also make it explicit that the CI is not doing this.

ACKs for top commit:
  fjahr:
    tested ACK 34c80d9eee

Tree-SHA512: 20f672fd3b3c1d633eccf9998fdd738194cdd7d10cc206691f2dcc28bbbf8187b8d06b87814f875a06145b179f5ca1f4f4f9922972be72759cf5ac6e0c11abd1
2020-12-10 12:04:28 +01:00
Wladimir J. van der Laan
eb53c03b36
Merge #20595: Improve heuristic hex transaction decoding
0f949cde3d Add regression test for incorrect decoding (Pieter Wuille)
39c42c4420 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 0f949cde3d
  jonatack:
    Tested ACK 0f949cde3d
  laanwj:
    Code review ACK 0f949cde3d

Tree-SHA512: bd6dc80d824eb9a87026a623be910cac92173f8ce1c8b040c2246348c3cf0c6d64bcc40127b859e5e4da1efe88cf02a6945f7ebb91079799395145cb09d9c7a5
2020-12-10 11:22:10 +01:00
MarcoFalke
38176dc665
Merge #20573: wallet, bugfix: allow send with string fee_rate amounts
6fa72ceb80 test: add coverage for passing fee rate as a string (Jon Atack)
ce207d6b93 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 6fa72ceb80
  achow101:
    Code review ACK 6fa72ceb80
  promag:
    Code review ACK 6fa72ceb80.

Tree-SHA512: 735f9269cb1b81953764b5283449c0b154bd62de034225be5bcedc515c84faf767fe8fe0741008679fe412922c847b00d116cb11aab775236b779c847ba87167
2020-12-10 08:46:01 +01:00
MarcoFalke
22f13c1e08
Merge #19776: net, rpc: expose high bandwidth mode state via getpeerinfo
343dc4760f test: add test for high-bandwidth mode states in getpeerinfo (Sebastian Falbesoner)
dab6583307 doc: release note for new getpeerinfo fields "bip152_hb_{from,to}" (Sebastian Falbesoner)
a7ed00f8bb rpc: expose high-bandwidth mode states via getpeerinfo (Sebastian Falbesoner)
30bc8fab68 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 343dc4760f
  jonatack:
    re-ACK 343dc4760f per `git range-diff 7ea6499 4df1d12 343dc47`

Tree-SHA512: f4999e6a935266812c2259a9b5dc459710037d3c9e938006d282557cc225e56128f72965faffb207fc60c6531fab1206db976dd8729a69e8ca29d4835317b99f
2020-12-10 08:21:36 +01:00
MarcoFalke
44444ba759
fuzz: Link all targets once 2020-12-10 07:15:42 +01:00
Wladimir J. van der Laan
42ed7f51fa
Merge #20606: Remove unused bits from service flags enum
fa40168ab3 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 fa0d0ff6e1.

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

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'
1583498fb6 Send and require SENDADDRV2 before VERACK (Pieter Wuille)
c5a8919660 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 1583498fb6
  jnewbery:
    ACK 1583498fb6
  jonatack:
    ACK 1583498fb6
  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
fa275e1539 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 fa275e1539

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
fa8abdc995 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 fa8abdc995

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)
4e28753f60 feestimator: encapsulate estimation file logic (Antoine Poinsot)
e8ea6ad9c1 init: don't create a CBlockPolicyEstimator if we don't relay transactions (Antoine Poinsot)
86ff2cf202 Remove the remaining fee estimation globals (Antoine Poinsot)
03bfeee957 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 4e28753f60

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
52fc39917f rpc: Reject empty txids in gettxoutproof (João Barbosa)
73dc19a330 rpc, refactor: Avoid duplicate set lookup in gettxoutproof (João Barbosa)

Pull request description:

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

Tree-SHA512: 76b18e5235e8b2d394685515a4a60335666eeb0f6b31c1d397f7db2fbe681bc817b8cd3e8f6708b9dacd6113e4e1d94837072cae27834b8a1a22d2717db8191e
2020-12-07 09:17:08 +01:00
MarcoFalke
64156ad4d1
Merge #19893: test: Remove or explain syncwithvalidationinterfacequeue
fa6af31227 test: Document why syncwithvalidationinterfacequeue is needed in tests (MarcoFalke)
fa135a13b8 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 fa6af31227

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
MarcoFalke
fab48da908
test: Fix intermittent wallet_multiwallet issue with got_loading_error 2020-12-04 10:32:00 +01:00
MarcoFalke
fa8e15f7b7
test: pep8 wallet_multiwallet.py 2020-12-04 10:27:38 +01:00
Wladimir J. van der Laan
34c80d9eee test: Add option to git-subtree-check to do full check, add help
This adds a brief help text to `git-subtree-check.sh` and adds and an
option to do a full remote check instead of having two different code
paths with a successful exit status. Also make it explicit that the CI
is not doing this.
2020-12-04 09:36:17 +01: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
Antoine Poinsot
86ff2cf202
Remove the remaining fee estimation globals
This moves the CBlockPolicyEstimator to the NodeContext, which get rids
of two globals and allows us to conditionally create the
CBlockPolicyEstimator (and to remove a circular dep).

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
fad7be584f 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 fad7be584f

Tree-SHA512: d43ba9d07273486858f65a26326cc6637ef743bf7b400e5048ba7eac266fb1893283e6503dd49f179caa1abab2977315fb70ba9fba34be9a817a74259d8e4034
2020-12-03 10:09:30 +01:00
fanquake
0a13d15c14
Merge #20530: lint, refactor: Update cppcheck linter to c++17 and improve explicit usage
1e62350ca2 refactor: Improve use of explicit keyword (Fabian Jahr)
c502a6dbfb lint: Use c++17 std in cppcheck linter (Fabian Jahr)

Pull request description:

  I found the `extended-lint-cppcheck` linter still uses `std=c++11` when reviewing #20471. The only difference in the output after this change is one line is missing:

  ```
  src/script/descriptor.cpp:159:5: warning: Struct 'PubkeyProvider' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
  ```

  After some digging, I am still not sure why this one is ignored with c++17 when 40 other`noExplicitConstructor` warnings were still appearing.

  In the second commit, I fix these warnings, adding `explicit` where appropriate and adding fixes to ignore otherwise.

ACKs for top commit:
  practicalswift:
    cr ACK 1e62350ca2: patch looks correct!
  MarcoFalke:
    review ACK 1e62350ca2

Tree-SHA512: dff7b324429a57160e217cf38d9ddbb6e70c6cb3d3e3e0bd4013d88e07afc2292c3df94d0acf7122e9d486322821682ecf15c8f2724a78667764c05d47f89a12
2020-12-02 20:52:19 +08:00
MarcoFalke
283f22cabb
Merge #20461: rpc: Validate -rpcauth arguments
053b4fbad8 doc: Release note regarding -rpcauth validation (João Barbosa)
46001323b1 rpc: Validate -rpcauth arguments (João Barbosa)
d37c813a43 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 053b4fbad8
  jonatack:
    ACK 053b4fbad8
  ryanofsky:
    Code review ACK 053b4fbad8. 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
fada2dfcac 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 fada2dfcac. Although it could ignore (don't log) directories that lead to no permission error.
  fanquake:
    ACK fada2dfcac

Tree-SHA512: b475162cc3cd1574209d916605b229a79c8089714295f5e16569b71f958f0007d54dc76833938492d931387784588b11b73e3ef00f963540af42c079417f8d72
2020-12-02 08:31:02 +01:00
Fabian Jahr
1e62350ca2
refactor: Improve use of explicit keyword 2020-12-01 18:36:39 +01:00
Fabian Jahr
c502a6dbfb
lint: Use c++17 std in cppcheck linter 2020-12-01 18:32:26 +01:00
MarcoFalke
f17e8ba3a1
Merge #20207: Follow-up extra comments on taproot code and tests
2d8099c713 Mention units of MAX_STANDARD_ policy constants (Pieter Wuille)
84e29c7c01 Mention in validation that IsWitnessStandard tests for P2TR (Pieter Wuille)
f867cbcc26 Clean up assets test minimizer LDFLAGS (Pieter Wuille)
ea0e78677b Document additional IsWitnessStandard behavior (Pieter Wuille)
6040de9a46 Add comments on CPubKey::IsValid (Pieter Wuille)
8dbb7de67c Add comments to VerifyTaprootCommitment (Pieter Wuille)
cdf900cbf2 Document need_vin_vout_mismatch argument to make_spender (Pieter Wuille)
18246ed5f0 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
3ebde2143a [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 3ebde2143a
  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
89bdad5b25 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 89bdad5b25
  MarcoFalke:
    review ACK 89bdad5b25

Tree-SHA512: efb399c33f7b5596870a26a8680f453ca47aa7a6db4e550f9435d13044f1c4bad0ae11e8f0205213409d08b75c4188c3be782e54aafab1f65b97eb8cf5c252a9
2020-11-28 10:14:45 +01:00