| Age | Commit message (Collapse) | Author |
|---|
|
p2p_lock acquires)
5b77f8098de537898151ab116d0e547fd6ff9466 test: add p2p_lock acquires in p2p_leak_tx.py (Sebastian Falbesoner)
cc8c6823b4a8b74922f78ce6ce527ced9325bd49 test: use MiniWallet for p2p_leak_tx.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (p2p_leak_tx.py) to be run even with the Bitcoin Core wallet disabled by using the new MiniWallet instead, as proposed in #20078. It also adds missing p2p_lock acquires that need to be held while modifying internal p2p Interface state (in this case the `last_message` dictionary) to avoid data races.
ACKs for top commit:
laanwj:
Code review ACK 5b77f8098de537898151ab116d0e547fd6ff9466
Tree-SHA512: 6661bc6e3491a9af4bf040f379e5955c525136397e99d3eadde92e247580d0d87efff750e6d3b1f6d9a4e578144a433a982f574ef056b44dd6bca33873a1bae6
|
|
|
|
This test can now be run even with the Bitcoin Core wallet disabled.
|
|
dcf0cb477699d11afd0ff37c8bfb2b1b4f7f1ee5 tor: make a TORv3 hidden service instead of TORv2 (Vasil Dimov)
353a3fdaad055eea42a0baf7326bdd591f541170 net: advertise support for ADDRv2 via new message (Vasil Dimov)
201a4596d92d640d5eb7e76cc8d959228fa09dbb net: CAddress & CAddrMan: (un)serialize as ADDRv2 (Vasil Dimov)
1d3ec2a1fda7446323786a52da1fd109c01aa6fb Support bypassing range check in ReadCompactSize (Pieter Wuille)
Pull request description:
This PR contains the two remaining commits from #19031 to complete the [BIP155](https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki) implementation:
`net: CAddress & CAddrMan: (un)serialize as ADDRv2`
`net: advertise support for ADDRv2 via new message`
plus one more commit:
`tor: make a TORv3 hidden service instead of TORv2`
ACKs for top commit:
jonatack:
re-ACK dcf0cb477699d11afd0ff37c8bfb2b1b4f7f1ee5 per `git diff 9b56a68 dcf0cb4` only change since last review is an update to the release notes which partially picked up the suggested text. Running a node on this branch and addnode-ing to 6 other Tor v3 nodes, I see "addrv2" and "sendaddrv2" messages in getpeerinfo in both the "bytesrecv_per_msg" and "bytessent_per_msg" JSON objects.
sipa:
ACK dcf0cb477699d11afd0ff37c8bfb2b1b4f7f1ee5
hebasto:
re-ACK dcf0cb477699d11afd0ff37c8bfb2b1b4f7f1ee5, the node works flawlessly in all of the modes: Tor-only, clearnet-only, mixed.
laanwj:
Edit: I have to retract this ACK for now, I'm having some problems with this PR on a FreeBSD node. It drops all outgoing connections with this dcf0cb477699d11afd0ff37c8bfb2b1b4f7f1ee5 merged on master (12a1c3ad1a43634d2a98717e49e3f02c4acea2fe).
ariard:
Code Review ACK dcf0cb4
Tree-SHA512: 28d4d0d817b8664d2f4b18c0e0f31579b2f0f2d23310ed213f1f436a4242afea14dfbf99e07e15889bc5c5c71ad50056797e9307ff8a90e96704f588a6171308
|
|
Introduce a new message `sendaddrv2` to signal support for ADDRv2.
Send the new message immediately after sending the `VERACK` message.
Add support for receiving and parsing ADDRv2 messages.
Send ADDRv2 messages (instead of ADDR) to a peer if he has
advertised support for it.
Co-authored-by: Carl Dong <contact@carldong.me>
|
|
907f142fc7e1d35f443be076367739faf11cc2cc rpc: change no wallet loaded message to be clearer (Andrew Chow)
Pull request description:
Changes the no wallet is loaded rpc error message to be clearer that no wallet is loaded and how the user can load or create a wallet. Also changes the error code from METHOD_NOT_FOUND to RPC_WALLET_NOT_FOUND as that makes more sense.
ACKs for top commit:
MarcoFalke:
review ACK 907f142fc7e1d35f443be076367739faf11cc2cc
kristapsk:
ACK 907f142fc7e1d35f443be076367739faf11cc2cc. In addition to standard tests, just in case tested that this doesn't break anything with JoinMarket.
meshcollider:
utACK 907f142fc7e1d35f443be076367739faf11cc2cc
Tree-SHA512: 4b413e6ab5430ec75a79de9db6583f2f3f38ccdf71aa373d8386a56e64f07f92200c8107c8c82c92c7c431d739615977c208b771a24c5960fa8676789b5497a2
|
|
|
|
Changes the no wallet is loaded rpc error message to be clearer that no
wallet is loaded and how the user can load or create a wallet. Also
changes the error code from METHOD_NOT_FOUND to RPC_WALLET_NOT_FOUND as
that makes more sense.
|
|
clients
b048b275d9711f70847afaea5450f17a0f7e673a [validation] Remove absurdfee from accepttomempool (John Newbery)
932564b9cfda8446a957649c2316a52e868ad5d4 scripted-diff: update max-fee-exceeded error message to include RPC (gzhao408)
8f1290c60159a3171c27250bc95687548c5c1b84 [rpc/node] check for high fee before ATMP in clients (gzhao408)
Pull request description:
Picked up from #15810. Add separate fee-checking logic for clients that need to enforce max fee rates, then remove the `absurdFee` logic from ATMP.
ATMP's `nAbsurdFee` argument is used to enforce user-specific behavior (it is not policy since it isn't applied consistently: it is only ever used in RPC and wallet, and set to 0 everywhere else internally). It should be removed from `AcceptToMemoryPool` because (1) validation results/mempool behavior should not be user-specific and (2) enforcing a max fee rate should be the responsibility of the client instead of the mempool.
Note: this PR does not intend to _remove_ protection from high fees, just re-delegate the responsibility to clients.
ACKs for top commit:
jnewbery:
utACK b048b275d9711f70847afaea5450f17a0f7e673a
LarryRuane:
re-ACK b048b275d9711f70847afaea5450f17a0f7e673a
MarcoFalke:
re-ACK b048b275d9 , only change is squashing one commit 🏦
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/19339/commits/b048b275d9711f70847afaea5450f17a0f7e673a
Tree-SHA512: 57c17ba16d230a4cae2896dd6a64c924f307757824e35784bf96da7b10aff2d8ea910710edf35e981035623a155f8766209a92a0fdb856549fde78bc3eaae4d2
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/Fee exceeds maximum configured by \-\maxtxfee/Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)/g' src/util/error.cpp test/functional/rpc_fundrawtransaction.py test/functional/rpc_psbt.py test/functional/rpc_rawtransaction.py test/functional/wallet_bumpfee.py test/functional/wallet_create_tx.py
-END VERIFY SCRIPT-
|
|
Check absurd fee in BroadcastTransaction and RPC,
return TransactionError::MAX_FEE_EXCEEDED instead
of TxValidationResult::TX_NOT_STANDARD because this
is client preference, not a node-wide policy.
|
|
a56e9f5670136b29e48aabfc7f38569222fe4635 test: Assert exclusive PSBT funding options (Oliver Gugger)
64bc5efd39bad3212a1d57fd6234b544a14bb974 test: Assert PSBT change type (Oliver Gugger)
Pull request description:
Increases test coverage of the `walletcreatefundedpsbt` RPC.
Tests the following combinations:
- Make sure the global option `-changetype` is used as the default value for the `change_type` option if not specified.
- Make sure the global option `-changetype` can be overwritten by explicitly setting the `change_type` option of the `walletcreatefundedpsbt` RPC call.
- Make sure the options `change_type` and `changeAddress` are mutually exclusive.
ACKs for top commit:
achow101:
ACK a56e9f5670136b29e48aabfc7f38569222fe4635
Tree-SHA512: bf0fb20c890887b7228ad9277fdb32f367ba772eed6efbe2b4f471f808d4d435110256601e8ebd9bea57026d9f22f3cc3c26a009b017e3da6d8fc6896313def5
|
|
3491bf358a81d41a386cd14581d15396354a6e6c test: Mention commit id in scripted diff error (Wladimir J. van der Laan)
Pull request description:
Add commit id to make spotting the issue easier.
ACKs for top commit:
robot-dreams:
ACK 3491bf358a81d41a386cd14581d15396354a6e6c
sipa:
utACK 3491bf358a81d41a386cd14581d15396354a6e6c
hebasto:
~ACK~ Concept ACK 3491bf358a81d41a386cd14581d15396354a6e6c, should help in situations like https://travis-ci.org/github/bitcoin/bitcoin/jobs/732481553
Tree-SHA512: 1ae66fa760f9e5d52e029bae71f6b5863f1efd7b95de3723ea09290944c9d7687f5ec6927aa115a3aebd6f2b993baa0c2433975c6ad5cd2858089013362eb599
|
|
|
|
|
|
|
|
f471a3be00c2b6433b8c258b716982c0539da13f scripted diff: Improve invalid vout value rpc error message (Nima Yazdanmehr)
Pull request description:
Since the `vout` value can start at `0`, the error message for *negative* values can be improved to something like: `vout cannot be negative`.
ACKs for top commit:
fanquake:
ACK f471a3be00c2b6433b8c258b716982c0539da13f
promag:
Code review ACK f471a3be00c2b6433b8c258b716982c0539da13f.
Tree-SHA512: fbdee3d0ddd5b58eb93934a1217b44e125a9ad39e672b1f35c7609c6c5fcf45ae1b731d3d6135b7225d98792dbfc34a50907b8c41274a5b029d7b5c59f886560
|
|
Make sure the options "change_type" and "changeAddress" of the
walletcreatefundedpsbt RPC cannot both be specified a the same time.
|
|
c1585bca8dae01dee6a1dd8eadae2f8b100503df test: Get rid of default wallet hacks (Russell Yanofsky)
ed3acda33b75d1b546ee696a63def239bcdd62de test, refactor: add default_wallet_name and wallet_data_filename variables (Russell Yanofsky)
Pull request description:
Changes:
- Get rid of setup_nodes (`-wallet`, `-nowallet`, `-disablewallet`) argument rewriting
- Get rid of hardcoded wallet `""` names and `-wallet=""` args
Motivation:
- Simplify test framework behavior so it's easier to write new tests without having arguments mangled by the framework
- Make tests more readable, replacing unexplained `""` string literals with `default_wallet_name` references
- Make it trivial to update default wallet name and wallet data filename for sqlite #19077 testing
- Stop relying on `-wallet` arguments to create wallets, so it is easy to change `-wallet` option in the future to only load existing wallets not create new ones (to avoid accidental wallet creation, and encourage use of wallet encryption and descriptor features)
ACKs for top commit:
MarcoFalke:
ACK c1585bca8dae01dee6a1dd8eadae2f8b100503df, only effective change is adding documentation 🎵
Tree-SHA512: f62dec7cbdacb5f330aa0e1eec89ab4d065540d91495bbedcb375eda1c080b45ce9edb310ce253c44c4839f1b4cc2c7df9816c58402d5d43f94a437e301ea8bc
|
|
Make sure the wallet's default change type is respected by default when
funding a PSBT but can be overwritten by the "change_type" option.
|
|
e66870c5a4c2adbd30dca67d409fd5cd98697587 zmq: Append address to notify log output (nthumann)
241803da211265444e65f254f24dd184f2457fa9 test: Add zmq test to support multiple interfaces (nthumann)
a0b2e5cb6aa8db0563fac7d67a949b9baefe3a25 doc: Add release notes to support multiple interfaces (nthumann)
b1c3f180ecb63f3960506d202feebaa4271058ae doc: Adjust ZMQ usage to support multiple interfaces (nthumann)
347c94f551c3f144c44e00373e4dd61ff6d908b7 zmq: Add support to listen on multiple interfaces (Nicolas Thumann)
Pull request description:
This PR adds support for ZeroMQ to listen on multiple interfaces, just like the RPC server.
Currently, if you specify more than one e.g. `zmqpubhashblock` paramter, only the first one will be used. Therefore a user may be forced to listen on all interfaces (e.g. `zmqpubhashblock=0.0.0.0:28332`), which can result in an increased attack surface.
With this PR a user can specify multiple interfaces to listen on, e.g.
`-zmqpubhashblock=tcp://127.0.0.1:28332 -zmqpubhashblock=tcp://192.168.1.123:28332`.
ACKs for top commit:
laanwj:
Code review ACK e66870c5a4c2adbd30dca67d409fd5cd98697587
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/18309/commits/e66870c5a4c2adbd30dca67d409fd5cd98697587
Tree-SHA512: f38ab4a6ff00dc821e5f4842508cefadb701e70bb3893992c1b32049be20247c8aa9476a1f886050c5f17fe7f2ce99ee30193ce2c81a7482a5a51f8fc22300c7
|
|
6fccad7f711df330e461c1fab3f758d078345ed5 signet: do not log signet startup messages for other chains (Jon Atack)
Pull request description:
The following signet startup messages are printed to the debug log immediately on node startup for all chains. This behavior occurs on master as a side effect after the merge of #20014. This PR removes the first message and moves the signet derived magic logging to `init.cpp`.
```
$ ./src/bitcoind
2020-09-30T14:25:15Z Using default signet network
2020-09-30T14:25:15Z Signet derived magic (message start): 0a03cf40
2020-09-30T14:25:15Z Bitcoin Core version v0.20.99.0 ...
```
ACKs for top commit:
MarcoFalke:
ACK 6fccad7f711df330e461c1fab3f758d078345ed5
kallewoof:
utACK 6fccad7f711df330e461c1fab3f758d078345ed5
hebasto:
ACK 6fccad7f711df330e461c1fab3f758d078345ed5
Tree-SHA512: 33821dce89b24caf7b7c1ecb41e572ecfb26e6958a1316d359ff240e6ef97c4a1f2cf1b4b974596b252815f9df23960ce385c132ebdbc855bbe6123c3b0b003a
|
|
and move signet network magic logging from chainparams.cpp to init.cpp
|
|
825fcae484f31182041dfacbf820e818d759b130 [tests] Replace bytes literals with hex literals (John Newbery)
64eca45100536579a3849631e59d4277bbc25be1 [tests] Fix pep8 style violations in address.py (John Newbery)
b230f8b3f3adcb1e2ae299094f9ae0a8bc7cc3d0 [tests] Correct docstring for address.py (John Newbery)
ea70e6a2ca0e183ef40cdb9b3b86f39e94366015 [tests] Tidy up imports in address.py (John Newbery)
7f639df0b8a15aaeccedab00b634925f568c2c9a [tests] Remove unused optional verify_checksum parameter (John Newbery)
011e784f74411bd5d5dbccfd3af39e0937fd8933 [tests] Rename segwit encode and decode functions (John Newbery)
e4557133f595f357df5e16ae4f2f19c579631396 [tests] Move bech32 unit tests to test framework (John Newbery)
Pull request description:
Lots of small fixes:
- moving unit tests to test_framework implementation files
- renaming functions to be clearer
- removing multiple imports
- removing unreadable byte literals from the code
- fixing pep8 violations
- correcting out-of-date docstring
ACKs for top commit:
jonatack:
re-ACK 825fcae484f31182041dfacbf820e818d759b130 per `git range-diff a0a422c 7edcdcd 825fcae` and verified `wallet_address_types.py` and `wallet_basic.py --descriptors` (the failure on one travis job) are green locally.
MarcoFalke:
ACK 825fcae484f31182041dfacbf820e818d759b130
fanquake:
ACK 825fcae484f31182041dfacbf820e818d759b130 - looks ok to me.
Tree-SHA512: aea509c27c1bcb94bef11205b6a79836c39c62249672815efc9822f411bc2e2336ceb3d72b3b861c3f4054a08e16edb28c6edd3aa5eff72eec1d60ea6ca82dc4
|
|
|
|
-BEGIN VERIFY SCRIPT-
r() { sed -i 's/vout must be positive/vout cannot be negative/g' $1 }
r $(git grep -l 'vout must be positive')
-END VERIFY SCRIPT-
|
|
|
|
69cf5d4eeb73f7d685e915fc17af64634d88a4a2 [test] Make sure send rpc returns fee reason (Sishir Giri)
d5863c0b3e20d56acf7246008b7832efde68ab21 [send] Make send RPCs return fee reason (Sishir Giri)
Pull request description:
Whenever a wallet funds a transaction, the fee reason is reported to the user only if the verbose is set to true. I added an extra parameter to `CreateTransaction` function in wallet.cpp. Then I implemented the fee reason return logic in `SendMoney` in rpcwallet.cpp, followed by verbose parameter in `sendtoaddress` and `sendmany` functions. I also added a fee reason test case in walletbasic.py.
link to the issue: https://github.com/MarcoFalke/bitcoin-core/issues/22#issue-616251578
ACKs for top commit:
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/19501/commits/69cf5d4eeb73f7d685e915fc17af64634d88a4a2
meshcollider:
utACK 69cf5d4eeb73f7d685e915fc17af64634d88a4a2
Tree-SHA512: 2e3af32dcfbd5511ba95f8bc8edca7acfe709a8430ff03e43172e5d0af3dfa4b2f57906978e7f272d878043b9ed8c6004674cf47d7496b005d5f612e9a58aa0e
|
|
arguments (instead of continuing without proxy server)
9b4fa0af40cd88ed25dd77962235fbf268bdcaa7 net: Print error message if -proxy is specified without arguments (instead of continuing without proxy server) (practicalswift)
Pull request description:
Exit with error message if `-proxy` is specified without arguments (instead of continuing without proxy server).
Continuing without a proxy server when the end-user has specified `-proxy` may result in accidental loss of privacy. (The end-user might think he/she is using a proxy when he/she is not.)
Before this patch:
```
$ src/bitcoind -proxy
…
2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -listen=0
2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -upnp=0
2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -discover=0
2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -listen=0 -> setting -listenonion=0
…
2020-09-23T00:24:33Z init message: Starting network threads...
```
`bitcoind` is now running *without* a proxy server (`GetProxy(…, …) == false`, `HaveNameProxy() == false`, etc.).
Note that the "-proxy set" log messages above which the end-user might interpret as "good, my traffic is now routed via the proxy".
After this patch:
```
$ src/bitcoind -proxy
Error: No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>.
$ echo $?
1
```
ACKs for top commit:
laanwj:
re-ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7
kristapsk:
ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7, I have tested the code.
hebasto:
re-ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7
Tree-SHA512: 4ba7a011991699a54b5bb87ec68367c681231bf5dcd36f8c89ff9ddc2e8d29df453817b7e362597e652ad6b341a22b7274be0fd78d435e5f0fd8058e5221c4ce
|
|
- Get rid of hardcoded wallet "" names and -wallet="" args
- Get rid of setup_nodes (-wallet, -nowallet, -disablewallet) argument rewriting
Motivation:
- Simplify test framework behavior so it's easier to write new tests without
having arguments mangled by the framework
- Make tests more readable, replacing unexplained "" string literals with
default_wallet_name references
- Make it trivial to update default wallet name and wallet data filename for
sqlite #19077 testing
- Stop relying on -wallet arguments to create wallets, so it is easy to change
-wallet option in the future to only load existing wallets not create new
ones (to avoid accidental wallet creation, and encourage use of wallet
encryption and descriptor features)
|
|
a3abeec33a6ae903e514c7a7b6f587b7c17288a0 policy/fees: remove a floating-point division by zero (Antoine Poinsot)
c36869bbf6a38626833b4aea53be024c48ede475 policy/fees: unify some duplicated for loops (Antoine Poinsot)
569d92a4d2924a1f6d50775980b591552f6372e7 policy/fees: small readability improvements (Antoine Poinsot)
5b8cb35621891b681f9b49a9de5f6d8da4ccdecc policy/fee: remove requireGreater parameter in EstimateMedianVal() (Antoine Poinsot)
dba8196b447b6a85be66890db70928100e867d8b policy/fees: correct decay explanation comments (Antoine Poinsot)
Pull request description:
This (*does not* change behaviour and) cleans up a bit of unused code in `CBlockPolicyEstimator` and friends, and slightly improves readability of the rest (comment correction etc.). The last commit is a small reformatting one which I could not resist but am happy to remove at will.
ACKs for top commit:
jnewbery:
utACK a3abeec33a6ae903e514c7a7b6f587b7c17288a0
MarcoFalke:
ACK a3abeec33a6ae903e514c7a7b6f587b7c17288a0 💹
ariard:
Code Review ACK a3abeec.
Tree-SHA512: b7620bcd23a2ffa8f7ed859467868fc0f6488279e3ee634f6d408872cb866ad086a037e8ace76599a05b7e9c07768adf5016b0ae782d153196b9c030db4c34a5
|
|
No changes in behavior
|
|
extend logging
deb52711a17236d0fca302701b5af585341ab42a Remove header checks out of net_processing (Troy Giorshev)
52d4ae46ab822d0f54e246a6f2364415cda149bd Give V1TransportDeserializer CChainParams& member (Troy Giorshev)
5bceef6b12fa16d20287693be377dace3dfec3e5 Change CMessageHeader Constructor (Troy Giorshev)
1ca20c1af8f08f07c407c3183c37b467ddf0f413 Add doxygen comment for ReceiveMsgBytes (Troy Giorshev)
890b1d7c2b8312d41d048d2db124586c5dbc8a49 Move checksum check from net_processing to net (Troy Giorshev)
2716647ebf60cea05fc9edce6a18dcce4e7727ad Give V1TransportDeserializer an m_node_id member (Troy Giorshev)
Pull request description:
Inspired by #15206 and #15197, this PR moves all message header verification from the message processing layer and into the network/transport layer.
In the previous PRs there is a change in behavior, where we would disconnect from peers upon a single failed checksum check. In various discussions there was concern over whether this was the right choice, and some expressed a desire to see how this would look if it was made to be a pure refactor.
For more context, see https://bitcoincore.reviews/15206.html#l-81.
This PR improves the separation between the p2p layers, helping improvements like [BIP324](https://github.com/bitcoin/bitcoin/pull/18242) and #18989.
ACKs for top commit:
ryanofsky:
Code review ACK deb52711a17236d0fca302701b5af585341ab42a just rebase due to conflict on adjacent line
jnewbery:
Code review ACK deb52711a17236d0fca302701b5af585341ab42a.
Tree-SHA512: 1a3b7ae883b020cfee1bef968813e04df651ffdad9dd961a826bd80654f2c98676ce7f4721038a1b78d8790e4cebe8060419e3d8affc97ce2b9b4e4b72e6fa9f
|
|
f7b331ea85d45c7337e527b6e77a45da7a689b7d rpc: add brackets to ConstructTransaction (Sjors Provoost)
d813d26f06248aaa7be3c698c87939cc777fafd0 [rpc] send: various touch-ups (Sjors Provoost)
0fc1c685e1ca68ca8ed2b35f623bbe6a9fc36d66 [rpc] send: fix parsing replaceable option (Sjors Provoost)
efc9b85e6f4aa431d308089874a18f0bbdcdd0fd Mark send RPC experimental (Sjors Provoost)
Pull request description:
Followup based on #16378 nits. It also fixes an argument parsing error (uncaught because the test wasn't sufficiently thorough).
I marked the RPC as experimental so we can tweak it a bit over the next release cycle.
ACKs for top commit:
meshcollider:
utACK f7b331ea85d45c7337e527b6e77a45da7a689b7d
fjahr:
utACK f7b331ea85d45c7337e527b6e77a45da7a689b7d
kallewoof:
ACK f7b331ea85d45c7337e527b6e77a45da7a689b7d
Tree-SHA512: 82dd8ac76a6558872db3f5249d4d6440469400aaa339153bc627d1ee673a91ecfadecb486bc1939ba87ebbd80e26ff29698e93e358599f3d26fde0e526892afe
|
|
|
|
|
|
|
|
|
|
|
|
92e28fa8b2590cce0e8f0adadae80e46cb63a9ef test: remove unused constants in functional tests (Sebastian Falbesoner)
Pull request description:
This mini-PR gets rid of constants in functional tests that are not used anymore. Found by [vulture ](https://pypi.org/project/vulture/)via the following script that has been lying around here locally for quite some time (I think it was once proposed by practicalswift, but I don't remember the concrete topic/PR):
```
#!/bin/sh
for F in $(git ls-files -- "*.py"); do vulture "$F" | grep "unused variable"; done
```
ACKs for top commit:
practicalswift:
ACK 92e28fa8b2590cce0e8f0adadae80e46cb63a9ef: patch looks correct.
Tree-SHA512: 16516abc8014207bcefdf0545dffaecff1fbba66f45b54c02371dcfd1f18194855c6b72598c11b5407009561eafe8048d47af3471f0efb1795d52477d5a0232e
|
|
a512925e19a70d7f6b80ac530a169f45ffaafa1c [doc] Release notes (Amiti Uttarwar)
50f94b34a33c954f6e207f509c93d33267a5c3e2 [rpc] Deprecate getpeerinfo addnode field (Amiti Uttarwar)
df091b9b509f0b10e4315c0bfa2da0cc0c31c22f [refactor] Rename test file to allow any getpeerinfo deprecations. (Amiti Uttarwar)
395acfa83a5436790c1a722a5609ac9d48df235f [rpc] Add connection type to getpeerinfo RPC, update tests (Amiti Uttarwar)
49c10a9ca40967d28ae16dfea9cccc6f3a6624a1 [log] Add connection type to log statement (Amiti Uttarwar)
Pull request description:
After #19316, we can more directly expose information about the connection type on the `getpeerinfo` RPC. Doing so also makes the existing addnode field redundant, so this PR begins the process of deprecating this field.
This PR also includes one commit that improves a log message, as both use a shared function to return the connection type as a string.
Suggested by sdaftuar- https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468001604 & https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468018093
ACKs for top commit:
jnewbery:
Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c.
sipa:
utACK a512925e19a70d7f6b80ac530a169f45ffaafa1c
guggero:
Tested and code review ACK a512925e.
MarcoFalke:
cr ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c 🌇
promag:
Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c.
Tree-SHA512: 601a7a38aee235ee59aca690784f886dc2ae4e418b2e6422c4b58cd597376c00f74910f66920b08a08a0bec28bf8022e71a1435785ff6ba8a188954261aba78e
|
|
|
|
To make the intent of the tests easier to understand, we reference the
p2p connection objects by their explicit names instead of the p2ps array.
|
|
confusing Test_Node.p2p property
10d61505fe77880d6989115defa5e08417f3de2d [test] remove confusing p2p property (gzhao408)
549d30faf04612d9589c81edf9770c99e3221885 scripted-diff: replace p2p with p2ps[0] in p2p_invalid_tx (gzhao408)
7a0de46aeafb351cffa3410e1aae9809fd4698ad [doc] sample code for test framework p2p objects (gzhao408)
784f757994c1306bb6584b14c0c78617d6248432 [refactor] clarify tests by referencing p2p objects directly (gzhao408)
Pull request description:
The `TestNode` has a `p2p` property which is an alias for `p2ps[0]`.
I think this should be removed because it can be confusing and misleading (to both the test writer and reviewer), especially if a TestNode has multiple p2ps connected (which is the case for many tests).
Another example is when a test has multiple subtests that connect 1 p2p and use the `p2p` property to reference it. If the subtests don't completely clean up after themselves, the subtests may affect one another.
The best way to refer to a connected p2p is use the object returned by `add_p2p_connection` like this:
```py
p2p_conn = node.add_p2p_connection(P2PInterface())
```
A good example is [p2p_invalid_locator.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_invalid_locator.py), which cleans up after itself (waits in both `wait_for_disconnect` and in `disconnect_p2ps`) but wouldn't need so much complexity if it just referenced the connections directly.
If there is only one connected, it's not really that tedious to just use `node.p2ps[0]` instead of `node.p2p` (and it can always be aliased inside the test itself).
ACKs for top commit:
robot-dreams:
utACK 10d61505fe77880d6989115defa5e08417f3de2d
jnewbery:
utACK 10d61505fe77880d6989115defa5e08417f3de2d
guggero:
Concept ACK 10d61505.
Tree-SHA512: 5965548929794ec660dae03467640cb2156d7d826cefd26d3a126472cbc2494b855c1d26bbb7b412281fbdc92b9798b9765a85c27bc1a97f7798f27f64db6f13
|
|
of continuing without proxy server)
|
|
tracking
759d94e70f6844443106404882c7b105f3a4dba7 Update zmq notification documentation and sample consumer (Gregory Sanders)
68c3c7e1bdd00bbe7d70592a8eb39520fa3f87f1 Add functional tests for zmq sequence topic and mempool sequence logic (Gregory Sanders)
e76fc2b84d065c9d06010d0a10b316f1f9d36fb9 Add 'sequence' zmq publisher to track all block (dis)connects, mempool deltas (Gregory Sanders)
1b615e61bfc464f215a1b48e6e27d1e8fc16b2d1 zmq test: Actually make reorg occur (Gregory Sanders)
Pull request description:
This PR creates a new ZMQ notifier that gives a "total hash history" of block (dis)connection, mempool addition/substraction, all in one pipeline. It also exposes a "mempool sequence number" to both this notifier and `getrawmempool` results, which allows the consumer to use the results together without confusion about ordering of results and without excessive `getrawmempool` polling.
See the functional test `interfaces_zmq.py::test_mempool_sync` which shows the proposed user flow for the client-side tracking of mempool contents and confirmations.
Inspired by https://github.com/bitcoin/bitcoin/pull/19462#issuecomment-656140421
Alternative to https://github.com/bitcoin/bitcoin/pull/19462 due to noted deficiencies in current zmq notification streams.
Also fixes a legacy zmq test that didn't actually trigger a reorg because of identical blocks being generated on each side of the split(oops)
ACKs for top commit:
laanwj:
Code review ACK 759d94e70f6844443106404882c7b105f3a4dba7
Tree-SHA512: 9daf0d7d996190f3a68ff40340a687519323d7a6c51dcb26be457fbc013217ea7b62fbd0700b74b654433d2e370704feb61e5584399290692464fcfcb72ce3b7
|
|
facaf9e61f4b9ea91fab554d495ebea1043d08fb doc: Document signet BIP (MarcoFalke)
faf0a26711eed9264113463e56b988cf9fe549fd doc: Update comments for new chain settings (-signet and -chain=signet) (MarcoFalke)
fae0548686cb3d095086d3f0fef38dcfcd31d8ca fuzz: Remove needless guard (MarcoFalke)
77771a03df6c5d940b340d15eb88f2ac9a29c13a refactor: Remove SignetTxs::m_valid and use optional instead (MarcoFalke)
fa2ad5dae17b237641b8ece0e68ffcdd79d543bf test: Run signet test even when wallet was not compiled (MarcoFalke)
Pull request description:
Some doc and test fixups for #18267
ACKs for top commit:
ajtowns:
ACK facaf9e61f4b9ea91fab554d495ebea1043d08fb -- code review only
dr-orlovsky:
Reviewed & ACK https://github.com/bitcoin/bitcoin/pull/19993/commits/facaf9e61f4b9ea91fab554d495ebea1043d08fb
kallewoof:
Code review ACK facaf9e61f4b9ea91fab554d495ebea1043d08fb
Tree-SHA512: 8085027c488d84bb4bddccba78bd2d4c5af0d8e2644ee72265f1f30fa8c83f61a961d9da2c796f2940e69682291cbee7b1028b6a6ce123ad9134c0ebbf4723b0
|
|
This moves header size and netmagic checking out of net_processing and
into net. This check now runs in ReadHeader, so that net can exit early
out of receiving bytes from the peer. IsValid is now slimmed down, so
it no longer needs a MessageStartChars& parameter.
Additionally this removes the rest of the m_valid_* members from
CNetMessage.
|
|
e15344889aac50aadee9211ac34e466867d5862b Clarify blocksonly whitelistforcerelay test (t-bast)
Pull request description:
As discussed in https://github.com/bitcoin/bitcoin/issues/19943, this test may be a bit misleading to newcomers.
We underscore the fact that our peer needs to run a modified version of Bitcoin Core to actually relay transactions to a `blocksonly` node and benefit from the `whitelistforcerelay` parameter.
ACKs for top commit:
naumenkogs:
ACK e15344889aac50aadee9211ac34e466867d5862b
Tree-SHA512: cc3526ac26c40a2d878b0ad863008663040683fd21092fcdc93866c2b0a79db8c2d29767d1f70bf56195092fca2aa2961cdbee52b5f0b1ae757cece9cd206301
|
|
|
