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
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
d76925478efd35e6fd835370639f2139b28381e4 [doc] Clarify semantic of peer's m_protect w.r.t to outbound eviction logics (Antoine Riard)
ac71fe936da290adf5a3155fe8db5f78b485f1f1 [doc] Clarify scope of eviction protection of outbound block-relay peers (Antoine Riard)
Pull request description:
Block-relay-only peers were introduced by #15759. According to its
author, it was intented to make them only immune to outbound peer
rotation-based eviction and not from all eviction as modified comment
leans to think of.
Clearly indicate that outbound block-relay peers aren't protected
from eviction by the bad/lagging chain logic.
Fix#19863
ACKs for top commit:
naumenkogs:
ACK d76925478efd35e6fd835370639f2139b28381e4
jonatack:
ACK d76925478efd35e6fd835370639f2139b28381e4
Tree-SHA512: 597fbd62838a6e39276024165b11514cad20a2e9d33cf9202d261cbadcb62b2df427c858e0cb57e585840d4c1d4600104aa53916bb868541f2580e4eed9b4b52
96571b3d4cb4cda0fd3d5a457ae4a12f615de82b doc: Update onion service target port numbers in tor.md (Hennadii Stepanov)
bb145c9050203b3f3d8bff10fb3bba31da51adb1 net: Extend -bind config option with optional network type (Hennadii Stepanov)
92bd3c1da48d17c8ba20349e18ad19051614bc1a net, refactor: Move AddLocal call one level up (Hennadii Stepanov)
57f17e57c8c410e10c16a46f7372c0ea8b7dd467 net: Pass onion service target to Tor controller (Hennadii Stepanov)
e3f07851f02857b4844fccb2e91070c5cd3aad4d refactor: Rename TorController::target to m_tor_control_center (Hennadii Stepanov)
fdd3ae4d264f26f87009879838dec035db5a7aed net, refactor: Refactor CBaseChainParams::RPCPort function (Hennadii Stepanov)
a5266d4546c444cfd6d36cb63d2df52ce9e689e2 net: Add alternative port for onion service (Hennadii Stepanov)
b3273cf4039d26e66ae58a8acb9d865461618d54 net: Use network byte order for in_addr.s_addr (Hennadii Stepanov)
Pull request description:
This PR adds ability to label incoming Tor connections as different from normal localhost connections.
Closes#8973.
Closes#16693.
Default onion service target ports are:
- 8334 on mainnnet
- 18334 on testnet
- 38334 on signet
- 18445 on regtest
To set the onion service target socket manually the extended `-bind` config option could be used:
```
$ src/bitcoind -help | grep -A 6 -e '-bind'
-bind=<addr>[:<port>][=onion]
Bind to given address and always listen on it (default: 0.0.0.0). Use
[host]:port notation for IPv6. Append =onion to tag any incoming
connections to that address and port as incoming Tor connections
(default: 127.0.0.1:8334=onion, testnet: 127.0.0.1:18334=onion,
signet: 127.0.0.1:38334=onion, regtest: 127.0.0.1:18445=onion)
```
Since [pr19991.02 update](https://github.com/bitcoin/bitcoin/pull/19991#issuecomment-698882284) this PR is an alternative to #19043.
ACKs for top commit:
Sjors:
re-utACK 96571b3d4cb4cda0fd3d5a457ae4a12f615de82b
vasild:
ACK 96571b3d4
laanwj:
Re-ACK 96571b3d4cb4cda0fd3d5a457ae4a12f615de82b
Tree-SHA512: cb0eade80f4b3395f405f775e1b89c086a1f09d5a4464df6cb4faf808d9c2245474e1720b2b538f203f6c1996507f69b09f5a6e35ea42633c10e22bd733d4438
7eab781a145a35d0373c4ab4d237a82b4919e88d rpc: Set HTTP Content-Type in bitcoin-cli (Wladimir J. van der Laan)
Pull request description:
We don't set any `Content-Type` in the client. It is more consistent with our other JSON-RPC use to set it to `application/json`.
Note that our server doesn't enforce content types, so it doesn't make a difference in practice. But it is fairly strange HTTP behavior to not set it at all for a POST request.
This came up in #18950.
ACKs for top commit:
promag:
ACK 7eab781a145a35d0373c4ab4d237a82b4919e88d.
jonatack:
Tested ACK 7eab781a145a35d0373c4ab4d237a82b4919e88d
practicalswift:
ACK 7eab781a145a35d0373c4ab4d237a82b4919e88d: patch looks correct
fanquake:
ACK 7eab781a145a35d0373c4ab4d237a82b4919e88d - Looks fine to me.
Tree-SHA512: a9fa155324d0f7bff955585a336ead6bb60b721039f424521a435e4bb0fad3f4532e5cc7b7a9acc4e93585e8d3db3082c010138810f22c0e92b8f749b86ef653
7532c9a2216d5188ce55adc989892b1e4031dee3 qt: Periodic translations update (Wladimir J. van der Laan)
598e3b1ed3aa7c15a66948694300f86ed7733ef9 tx: Update transifex slug for 0.21 (Wladimir J. van der Laan)
Pull request description:
Update transifex slug for 0.21 (as part of #18947), and do a periodic translations update.
ACKs for top commit:
fanquake:
ACK 7532c9a2216d5188ce55adc989892b1e4031dee3
Tree-SHA512: 7e396c9a7bcf69088ec89b604332b44cfc2f1949665a7d464ad7cd1c6156cd32ca4a6f7e0b70e4782966f0904c82c4c9286b25ada98052adeaf97e1f408489f6
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 e66870c5a4
Tree-SHA512: f38ab4a6ff00dc821e5f4842508cefadb701e70bb3893992c1b32049be20247c8aa9476a1f886050c5f17fe7f2ce99ee30193ce2c81a7482a5a51f8fc22300c7
We don't set any `Content-Type` in the client. It is more
consistent with our other JSON-RPC use to set it to `application/json`.
Note that our server doesn't enforce content types, so it doesn't make a
difference in practice. But it is fairly strange HTTP behavior to not set it.
This came up in #18950.
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
-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-
fa710a6d67b2de64bde90def77c70d0a052f9030 doc: Add 19501 release notes (MarcoFalke)
faf60dee34ae3dbe8e103a2c1b0679f13df6a921 doc: Remove double-whitespace from help string, other whitespace fixups (MarcoFalke)
Pull request description:
Adds release notes and fixes up some whitespace nits for the touched RPCs
ACKs for top commit:
fanquake:
ACK fa710a6d67b2de64bde90def77c70d0a052f9030
laanwj:
Code review ACK fa710a6d67b2de64bde90def77c70d0a052f9030
Tree-SHA512: b84a96386a9a8ed69f464c7dffdd600cf9a8b33a06120798b141b300991baed369ab91ae48df6446e89e1d62534ccd8ae721454e7a19b48900b317e9192afc47
af57766182013e17c23245671a33463f754ccd28 Fix misleading error message: Clean stack rule (sanket1729)
Pull request description:
Error messages in clean stack is misleading as it lets the user believe that there are extra
elements on the stack which is incorrect if the stack is empty.
Let me know if this requires additional test.
ACKs for top commit:
instagibbs:
re-ACK af57766182
gzhao408:
reACK af57766182
theStack:
re-ACK af57766182013e17c23245671a33463f754ccd28
darosior:
re ACK af57766182013e17c23245671a33463f754ccd28
Tree-SHA512: 88e77416e220b080246fec368f5552a891d102d072b7bee62ac560d5e31c4a8c2ee9cbe569740b253e9df177d21dc788d10d856b2a542ab47761bb81698e4082
fac966142e00f6838cfd666ff8905078204d014e signet: Add assumed values for default signet (MarcoFalke)
Pull request description:
Doesn't matter much right now, but when the default signet is bigger, this might come in handy
ACKs for top commit:
jsarenik:
Tested ACK fac966142e00f6838cfd666ff8905078204d014e
laanwj:
Tested ACK fac966142e00f6838cfd666ff8905078204d014e (did a new re-sync)
kallewoof:
utACK fac966142e00f6838cfd666ff8905078204d014e
Tree-SHA512: ed2692f5896350f8dc81f9bc5d79fbf1a4544b8f724c5c667fcadec3a37e26e9833ac189a3067a0731fd7b17a0c94f6b44a641fffe448e42259f7b7b44910db1
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 69cf5d4eeb
meshcollider:
utACK 69cf5d4eeb73f7d685e915fc17af64634d88a4a2
Tree-SHA512: 2e3af32dcfbd5511ba95f8bc8edca7acfe709a8430ff03e43172e5d0af3dfa4b2f57906978e7f272d878043b9ed8c6004674cf47d7496b005d5f612e9a58aa0e
712f95d3324d02310dd468e7bfd1e1b0df432e77 Update msvc build to use new vcpkg manifest (Aaron Clauson)
Pull request description:
The vcpkg tool has introduced a proper way to use [manifests](https://devblogs.microsoft.com/cppblog/vcpkg-accelerate-your-team-development-environment-with-binary-caching-and-manifests/). This PR replaces the custom text file mechanism with the new manifest approach.
It is planned that vckpg manifests will include the ability to version dependencies in the future. Dependency versions would solve a number of issues that currently require workarounds with the appveyor CI.
ACKs for top commit:
fanquake:
ACK 712f95d3324d02310dd468e7bfd1e1b0df432e77 - This is a nice simplification. I tested this in a Windows VM; packages were downloaded and installed automatically as required:
hebasto:
Approach ACK 712f95d3324d02310dd468e7bfd1e1b0df432e77, I've verified that changes comply MS docs:
Tree-SHA512: ff9b3d6ad3cacabcbec6566fd289b179af163dc0c4545f8ba666fc14ba07527557f72bc84ba8abfa3bdffb22e2b8ff0b180f41d909c6de76894ac50ddcf8646b
2ea62cae483b764e30f61c06d8ac65755bbd864c Improve docs about feeler connections (Gleb Naumenko)
Pull request description:
"feeler" and "test-before-evict" are two different strategies suggest in [Eclipse Attacks on Bitcoin’s Peer-to-Peer Network](https://www.usenix.org/system/files/conference/usenixsecurity15/sec15-paper-heilman.pdf). In our codebase, we use `ConnType::FEELER` to implement both.
It is confusing, up to the point that our documentation was just incorrect.
This PR:
- ~clarifies this aspect by renaming "ConnType::FEELER" to "ConnType::PROBE", meaning that this connections only probes that the node is operational, and then disconnects.~
- fixes the documentation
ACKs for top commit:
amitiuttarwar:
ACK 2ea62cae48. thank you!
practicalswift:
ACK 2ea62cae483b764e30f61c06d8ac65755bbd864c
Tree-SHA512: c9c03c09eefeacec28ea199cc3f697b0a98723f2f849f7a8115edc43791f8165e296e0e25a82f0b5a4a781a7de38c8954b48bf74c714eba02cdc21f7460673e5
8a4dcda414fcf8281d8b3b79039b735c0a7d0d4c doc: Added default signet config for linearize script (gr0kchain)
Pull request description:
Updated the example-linearize.cfg file to include support for the signet chain network config which is used by the ./linearize-data.py
Problem:
Without the signet magic, genesis hash and path config, the linearize-data.py script cannot generate a bootstrap.dat file.
Example:
```
./linearize-data.py ./linearize.cfg
Read 4776 hashes
Genesis block not found in hashlist
```
Solution:
Added netmagic, genesis and input example parameters to file. Netmagic in terms of signet is derived from the signet-challenge and not static as with other networks. The provided signet magic is based on the default public signet.
Resolution
```
./linearize-hashes.py ./linearize.cfg > ./hashlist.txt
./linearize-data.py ./linearize.cfg
$ ./linearize-data.py ./linearize.cfg
Read 4776 hashes
Input file /Users/gr0kchain/.bitcoin/signet/blocks/blk00000.dat
Output file /Users/gr0kchain/Downloads/bootstrap.dat
1000 blocks scanned, 1000 blocks written (of 4776, 20.9% complete)
2000 blocks scanned, 2000 blocks written (of 4776, 41.9% complete)
3011 blocks scanned, 3000 blocks written (of 4776, 62.8% complete)
4010 blocks scanned, 4000 blocks written (of 4776, 83.8% complete)
Done (4776 blocks written)
```
ACKs for top commit:
laanwj:
Tested ACK 8a4dcda414fcf8281d8b3b79039b735c0a7d0d4c
Tree-SHA512: ad4d330358cf67e7424fb1d97ca828c28ca2758102e45747f4059c11a8acce801162da024a20cfb892f997fd4c3f4d5af988d1ca67c74efa78bf7d4ed27dd421
f22d6a11423a4462196de24cd68e7f45513cc001 log: Remove static log message "Initializing chainstate Chainstate [ibd] @ height -1 (null)" (practicalswift)
Pull request description:
Remove static log message `Initializing chainstate Chainstate [ibd] @ height -1 (null)`.
AFAICT `chainstate->ToString()` will always equal `"Chainstate [ibd] @ height -1 (null)"` here which makes the log message neither relevant nor interesting :)
ACKs for top commit:
laanwj:
ACK f22d6a11423a4462196de24cd68e7f45513cc001
promag:
ACK f22d6a11423a4462196de24cd68e7f45513cc001, just get rid of it.
hebasto:
ACK f22d6a11423a4462196de24cd68e7f45513cc001, I agree that the removed log message in its current state is cryptic and useless.
Tree-SHA512: 1a65c0d14c9a433afcdaadef9bfcdd5d63276d5d2caee1bf3c48ac477e54fa28138f64020e6e26ca5e67872954a1e7d93fa24a12accc7c7211bc6e7a6039051d
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
The vcpkg tool has introduced a proper way to use manifests, https://devblogs.microsoft.com/cppblog/vcpkg-accelerate-your-team-development-environment-with-binary-caching-and-manifests/. This PR replaces the custom text file mechanism with the new manifest approach.
It is planned that vckpg manifests will include the ability to version dependencies in the future. Dependency versions would solve a number of issues that currently require workarounds with the appveyor CI.
Set vcpkg manifest version to 1 to avoid any perception it's related to any release or other version numbering.
62dba9628d2532dca0c44934600f5aac3b21e275 log: print unexpected version warning in validation log category (nthumann)
Pull request description:
Fixes#19603: As suggested by practicalswift, instead of always printing `<n> of the last 100 blocks have unexpected version` as a warning appended to UpdateTip, it is now printed in the validation log category and therefore only visible with `-debug=validation` enabled.
Before:
`2020-09-06T15:56:00Z UpdateTip: new best=00000000000000000001b2872e107a98b57913120e5c6c87ce2715a34c40adf8 height=646969 version=0x20400000 log2_work=92.261571 tx=565651941 date='2020-09-06T10:35:36Z' progress=0.999888 cache=32.2MiB(237417txo) warning='72 of last 100 blocks have unexpected version'`
After:
`2020-09-06T16:31:26Z UpdateTip: new best=0000000000000000000b3bd786dc42745dd7be4a8c695500a04518cb9e2f4dc1 height=646971 version=0x20000000 log2_work=92.261607 tx=565655901 date='2020-09-06T10:57:19Z' progress=0.999883 cache=3.8MiB(27550txo)`
`2020-09-06T16:31:26Z 71 of last 100 blocks have unexpected version`
Ran unit & functional tests, confirmed that the warning is now only printed when validation category is enabled.
ACKs for top commit:
theStack:
ACK 62dba9628d2532dca0c44934600f5aac3b21e275
MarcoFalke:
re-ACK 62dba96
practicalswift:
ACK 62dba9628d2532dca0c44934600f5aac3b21e275 -- only change since last ACK is `s/nUpgraded/num_unexpected_version/`
hebasto:
re-ACK 62dba9628d2532dca0c44934600f5aac3b21e275, https://github.com/bitcoin/bitcoin/pull/19898#pullrequestreview-483158708 is resolved now.
Tree-SHA512: 2100ca7d6d3fd67c92e81d75162d2506d6f1ecf1761d5180d76663fac06771b35e5c4235ebe1a00731b5f7db82db3cd19328627929c8f22912df592686ba51d3
- 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
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
