11a32722f09f1d81f34bd09b26248ba99f2e7f07 test: run mempool_resurrect.py even with wallet disabled (Michael Dietz)
Pull request description:
Another functional test rewritten as proposed in #20078
**Request for help:**
`node.gettransaction(txid)` fails for transactions sent with `wallet.send_self_transfer`. Even though the `txid`s look correct, are added to the mempool correctly, and removed from the mempool when a block is mined - all as expected.
However, `node.gettransaction(txid)` throws the error:
```sh
Traceback (most recent call last):
File "/Users/michaeldietz/Documents/bitcoin/test/functional/test_framework/test_framework.py", line 126, in main
self.run_test()
File "/Users/michaeldietz/Documents/bitcoin/test/functional/mempool_resurrect.py", line 43, in run_test
assert_equal(len(list(filter(lambda txid: node.gettransaction(txid)["confirmations"] > 0, spends_ids))), len(spends_ids))
File "/Users/michaeldietz/Documents/bitcoin/test/functional/mempool_resurrect.py", line 43, in <lambda>
assert_equal(len(list(filter(lambda txid: node.gettransaction(txid)["confirmations"] > 0, spends_ids))), len(spends_ids))
File "/Users/michaeldietz/Documents/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
File "/Users/michaeldietz/Documents/bitcoin/test/functional/test_framework/authproxy.py", line 146, in __call__
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: Invalid or non-wallet transaction id (-5)
```
Anyone know what's going wrong / can point me in the right direction if I'm making a mistake, or `MiniWallet` needs to be improved for this to work correctly?
ACKs for top commit:
MarcoFalke:
ACK 11a32722f09f1d81f34bd09b26248ba99f2e7f07
Tree-SHA512: 13d83a13ec23920db716e99b68670e61329d1cc73b12063d85bc1679ee6425a9951da4d2e392ca1f27760be7be049ccdc6f504e192ed5cd24ed0ba003b66fab3
fae32f295cc5b57c1cb95090bb60cddb42f9778a wallet: Add missing check for -descriptors wallet tool option (MarcoFalke)
faf8f61368696b9cbbea55ead30d6a48203235ff test: Add missing check for is_sqlite_compiled (MarcoFalke)
fa7dde1c418e2e700853bd30cc9e012c4e4c5ef2 wallet: Pass ArgsManager into ExecuteWalletToolFunc instead of using global (MarcoFalke)
Pull request description:
Also, fix a test failure when compiled without sqlite
ACKs for top commit:
ryanofsky:
Code review ACK fae32f295cc5b57c1cb95090bb60cddb42f9778a. Thanks for implementing the -descriptors check and dealing with the test failure!
jonatack:
Code review utACK fae32f295cc5b57c1cb95090bb60cddb42f9778a
Tree-SHA512: 3d7710694085822739a8316e4abc6db270799ca6ff6b0f9e5563ae240da65ae6a9cab7ba2647feae6ba540dac40b55b38ed41c8f6ed0bf02a3d1536284448927
95487b055328b590ba83f258de9637ab0f9a2f17 doc: Drop mentions of Travis CI as it is no longer used (Hennadii Stepanov)
09d105ef0f8b4b06bf248721a1209c9e16e9db75 ci: Drop travis_fold feature as Travis CI is no longer used (Hennadii Stepanov)
Pull request description:
As Travis CI is no longer used, this PR:
- drops `travis_fold` feature
- drops mentions of Travis CI in docs
ACKs for top commit:
MarcoFalke:
ACK 95487b055328b590ba83f258de9637ab0f9a2f17
Tree-SHA512: 2e259bb8b1e37bcefc1251737bb2716f06ddb57c490010b373825c4e70f42ca38efae69a2f63f21f577d7cee3725b94097bdddbd313f8ebf499281cf97c53cef
23cac24dd3f2aaf88aab978e7ef4905772815cd2 tests: Test bitcoin-wallet dump and createfromdump (Andrew Chow)
a88c320041bd1cd1786b2dfd9ab698a67c2a57c6 wallettool: Add createfromdump command (Andrew Chow)
e1e7a90d5f0616a46ffadd62a9f1c65406cca6b4 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 23cac24dd3f2aaf88aab978e7ef4905772815cd2 only change is rebase and removing useless shared_ptr wrapper 🎼
ryanofsky:
Code review ACK 23cac24dd3f2aaf88aab978e7ef4905772815cd2. Only changes since last review rebase and changing a pointer to a reference
Tree-SHA512: 2d63cf62baca3d16495aa698dc02f7d889c81b41015e9c92c23c275bb4a690fc176d351c3fd7f310bd6b17f5a936cc9be694cbecd702af741b96c0f530e72fa2
bc4a23008762702ffcd6868bcdb8fe2a732640ba Remove redundant p2p lock tacking for tx download functional tests (Antoine Riard)
d3b5eac9a989878e2e09e5fde71c49149b123f18 Add mutation for functional test test_preferred_inv (Antoine Riard)
06efb3163cdf30e74df3f78afc4896b0f55ce937 Add functional test test_txid_inv_delay (Antoine Riard)
a07910abcd580ed07187794cf0e1faf040bb4212 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 bc4a23008762702ffcd6868bcdb8fe2a732640ba
Tree-SHA512: 150e806bc5289feda94738756ab375c7fdd23c80c12bd417d3112043e26a91a717dc325a01079ebd02a88b90975ead5bd397ec86eb745c7870ebec379a8aa711
173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 test: walettool create descriptors (Ivan Metlushko)
345e88eecf1b28607d5da3af38e19794a8a115ce wallettool: add param to create descriptors wallet (Ivan Metlushko)
6d3af3ab627096a824cb6a7ca1ebeddc7530361c 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 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12
ryanofsky:
Code review ACK 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12. This seems pretty nicely implemented now, with opportunities to clean up more and dedup later
MarcoFalke:
Concept ACK 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 🌠
Tree-SHA512: cc32ba336ff709de2707ee15f495b4617908e8700ede8401a58e894f44cda485c544d644023c9a6604d88a62db9d92152383ee2e8abf691688c25cf6e222c622
fab48da908f3f81135b9163edf5011d1e5f6ef6e test: Fix intermittent wallet_multiwallet issue with got_loading_error (MarcoFalke)
fa8e15f7b75e35846b86e8627a3612e31eb22dcb 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 fab48da908f3f81135b9163edf5011d1e5f6ef6e. 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
3b064fcb9dd3df6c438a440f0fea86e9cf7b5f57 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 3b064fcb9dd3df6c438a440f0fea86e9cf7b5f57
Tree-SHA512: 5860dc021d02bc3752268ec1e859505bec87174953223b34b1af8a8e4ab66d645458fbf9571c0b816a9de891c3ff41314996e580869671fccd6972c093e78154
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
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
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.
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.
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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.
