Age | Commit message (Collapse) | Author |
|
Divided tests in rpc_signmessage.py into 2 files wallet_signmessagewithaddress.py and
rpc_signmessagewithprivkey.py, latter one can run even when wallet is disabled.
|
|
|
|
82b6f89819e55af26f5264678e0f93052934bcb3 [style] Small style improvements to DNS parameters (Amiti Uttarwar)
4c89e24f64c1dc1a56a3bcb6b5e2b4fb95e8b29f [test] Test the delay before querying DNS seeds (Amiti Uttarwar)
6395c8ed5689ea72e9a1618f14551775246f6361 [test] Test the interactions between -forcednsseed and -dnsseed (Amiti Uttarwar)
6f6b7df6bdcf863af160c0426e3a22028ca8259a [init] Disallow starting up with conflicting paramters for -dnsseed and -forcednsseed (Amiti Uttarwar)
26d0ffe4f2573e0297c9b0e095c2a0868929b08b [test] Test -forcednsseed causes querying DNS seeds (Amiti Uttarwar)
35851450a928ffacca240fadbf1747a42d5ba256 [test] Test the interactions between -connect and -dnsseed (Amiti Uttarwar)
75c05af361552eeecd100cee8cc40d4cd5a3aa27 [test] Test logic to query DNS seeds with block-relay-only connections (Amiti Uttarwar)
9c0871977839c28636eff975748182888299cd2b [test] Introduce test logic to query DNS seeds (Amiti Uttarwar)
Pull request description:
This PR adds a DNS seed to the regtest chain params to enable testing the DNS seed querying logic of `CConnman::ThreadDNSAddressSeed` and relevant startup parameters. Adds coverage for the changes in #22013 (and then some).
The main behavioral change to bitcoind is that this PR disallows starting up with conflicting parameters for `-dnsseed` and `-forcednsseed`.
The tests include:
* parameter interactions of different combinations of `-connect`, `-dnsseed` and `-forcednsseed`
* the delay before querying DNS seeds depending on how many addresses are in the addrman
* the behavior of `-forcednsseed`
* skipping DNS querying if we have outbound full relay connections & not block-relay-only connections
Huge props to mzumsande for identifying the timing technique for testing successful connections before running `ThreadDNSAddressSeed` 🙌🏽
ACKs for top commit:
mzumsande:
ACK 82b6f89819e55af26f5264678e0f93052934bcb3
jnewbery:
reACK 82b6f89819e55af26f5264678e0f93052934bcb3
Tree-SHA512: 9f0c29bfbf99426727e79c0a25606ae09deab91a92e3c5cee7f84c3ca7503a8ac9ab85a85c51841d40b164ef8c991326070f0b2f41d075fb7985df26f6e95d6d
|
|
This commit introduces a DNS seed to the regest chain params in order to add
coverage to the DNS querying logic.
The first test checks that we do not query DNS seeds if we are able to
succesfully connect to 2 outbound connections. Since we participate in ADDR
relay with those connections, including sending a GETADDR message during the
VERSION handshake, querying the DNS seeds is unnecessary.
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
|
|
(speedup, cleanup, logging)
a006d7d73019b8cf4d68626c019c3d69729dda69 test: add logging to wallet_listtransactions (Sebastian Falbesoner)
47915b118720c6e2b2ec9f599f25848041b42b99 test: remove unneeded/redundant code in wallet_listtransactions (Sebastian Falbesoner)
fb6c6a7938cb7c4808ad88d23bfc2b7408407b12 test: speedup wallet_listtransactions by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)
Pull request description:
This PR improves the test `wallet_listtransactions.py` in three ways:
* speeds up runtime by a factor of 2-3x by using the good ol' immediate tx relay trick (`-whitelist=noban@127.0.0.1`)
* removes unneeded/redundant code
* adds log messages, mostly by turning comments into `self.log.info(...)` calls
ACKs for top commit:
jonatack:
ACK a006d7d73019b8cf4d68626c019c3d69729dda69
kristapsk:
ACK a006d7d73019b8cf4d68626c019c3d69729dda69
Tree-SHA512: a91a19f5ebc4d05f0b96c5419683c4c57ac0ef44b64eeb8dd550bd72296fd3a2857a3ba83f755fe4b0b3bd06439973f226070b5d0ce2dee58344dae78cb50290
|
|
a806647d260132a00cd633160040625c7dd17803 [validation] Always include merkle root in coinbase commitment (Dhruv Mehta)
189128c220190a588500b8e74ee7ae47671b9558 [validation] Set witness script flag with p2sh for blocks (Dhruv Mehta)
ac82b99db77ec843af82dcdf040dfdbc98c8ff26 [p2p] remove redundant NODE_WITNESS checks (Dhruv Mehta)
6f8b198b8256a6703a6f5e592dfa77fa024a7035 [p2p] remove unused segwitheight=-1 option (Dhruv Mehta)
eba5b1cd6460c98e75d0422bd394e12af7f11e4c [test] remove or move tests using `-segwitheight=-1` (Dhruv Mehta)
Pull request description:
Builds on #21009 and makes progress on remaining items in #17862
Removing `RewindBlockIndex()` in #21009 allows the following:
- removal of tests using `segwitheight=-1` in `p2p_segwit.py`.
- move `test_upgrade_after_activation()` out of `p2p_segwit.py` reducing runtime
- in turn, that allows us to drop support for `-segwitheight=-1`, which is only supported for that test.
- that allows us to always set `NODE_WITNESS` in our local services. The only reason we don't do that is to support `-segwitheight=-1`.
- that in turn allows us to drop all of the `GetLocalServices() & NODE_WITNESS` checks inside `net_processing.cpp`, since our local services would always include `NODE_WITNESS`
ACKs for top commit:
mzumsande:
Code-Review ACK a806647d260132a00cd633160040625c7dd17803
laanwj:
Code review ACK a806647d260132a00cd633160040625c7dd17803, nice cleanup
jnewbery:
utACK a806647d260132a00cd633160040625c7dd17803
theStack:
ACK a806647d260132a00cd633160040625c7dd17803
Tree-SHA512: 73e1a69d1d7eca1f5c38558ec6672decd0b60b16c2ef6134df6f6af71bb159e6eea160f9bb5ab0eb6723c6632d29509811e29469d0d87abbe9b69a2890fbc73e
|
|
self-announcements
5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 test: Add functional test for AddrFetch connections (Martin Zumsande)
c34ad3309f93979b274a37de013502b05d25fad8 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande)
533500d9072b7d5a36a6491784bdeb9247e91fb0 p2p: Add timeout for AddrFetch peers (Martin Zumsande)
b6c5d1e450dde6a54bd785504c923adfb45c7060 p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande)
Pull request description:
AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them.
This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement.
So we'll disconnect before the peer gets a chance to answer our `getaddr`.
I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us.
The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers.
Fix this by only disconnecting after receiving an `addr` message of size > 1.
[Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test.
ACKs for top commit:
amitiuttarwar:
reACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6
naumenkogs:
ACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6
jnewbery:
ACK 5730a43703
Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
|
|
By whitelisting the peers via -whitelist, the inventory is transmissioned
immediately rather than on average every 5 seconds, speeding up the test by at
least a factor of two:
before:
$ time ./wallet_listtransactions.py
...
0m40.25s real 0m01.74s user 0m01.70s system
with this PR:
$ time ./wallet_listtransactions.py
...
0m14.93s real 0m01.68s user 0m01.87s system
This commit also moves the wallet_listtransactions tests into the < 30s group.
|
|
|
|
fa80e10d94dbf86da84fc761b09fb631155a5b25 test: Add feature_taproot.py --previous_release (MarcoFalke)
85ccffa26686c6c9adbd18bdde37fc1747281bab test: move releases download incantation to README (Sjors Provoost)
29d6b1da2a862bfbb14e7821979c97416c5400e8 test: previous releases: add v0.20.1 (Sjors Provoost)
Pull request description:
Disabling the new consensus code at runtime is fine, but potentially fragile and incomplete. Fix that by giving the option to run with a version that has been compiled without any taproot code.
ACKs for top commit:
Sjors:
tACK fa80e10
NelsonGaldeman:
tACK fa80e10d94dbf86da84fc761b09fb631155a5b25
Tree-SHA512: 1a1feef823f08c05268759645a8974e1b2d39a024258f5e6acecbe25097aae3fa9302c27262978b40f1aa8e7b525b60c0047199010f2a5d6017dd6434b4066f0
|
|
4101ec9d2e05a35c35f587a28f1feee6cebcc61b doc: mention that we enforce port=0 in I2P (Vasil Dimov)
e0a2b390c144e123e2fc8a289fdff36815476964 addrman: reset I2P ports to 0 when loading from disk (Vasil Dimov)
41cda9d075ebcab1dbb950160ebe9d0ba7b5745e test: ensure I2P ports are handled as expected (Vasil Dimov)
4f432bd738c420512a86a51ab3e00323f396b89e net: do not connect to I2P hosts on port!=0 (Vasil Dimov)
1f096f091ebd88efb18154b8894a38122c39624f net: distinguish default port per network (Vasil Dimov)
aeac3bce3ead1f24ca782079ef0defa86fd8cb98 net: change I2P seeds' ports to 0 (Vasil Dimov)
38f900290cc3a839e99bef13474d35e1c02e6b0d net: change assumed I2P port to 0 (Vasil Dimov)
Pull request description:
_This is an alternative to https://github.com/bitcoin/bitcoin/pull/21514, inspired by https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-815049933. They are mutually exclusive. Just one of them should be merged._
Change assumed ports for I2P to 0 (instead of the default 8333) as this is closer to what actually happens underneath with SAM 3.1 (https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-812632520, https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-816564719).
Don't connect to I2P peers with advertised port != 0 (we don't specify a port to our SAM 3.1 proxy and it always connects to port = 0).
Note, this change:
* Keeps I2P addresses with port != 0 in addrman and relays them to others via P2P gossip. There may be non-bitcoin-core-22.0 peers using SAM 3.2 and for them such addresses may be useful.
* Silently refuses to connect to I2P hosts with port != 0. This is ok for automatically chosen peers from addrman. Not so ok for peers provided via `-addnode` or `-connect` - a user who specifies `foo.b32.i2p:1234` (non zero port) may wonder why "nothing is happening".
Fixes #21389
ACKs for top commit:
laanwj:
Code review ACK 4101ec9d2e05a35c35f587a28f1feee6cebcc61b
jonatack:
re-ACK 4101ec9d2e05a35c35f587a28f1feee6cebcc61b per `git range-diff efff9c3 0b0ee03 4101ec9`, built with DDEBUG_ADDRMAN, did fairly extensive testing on mainnet both with and without a peers.dat / -dnsseeds=0 to test boostrapping.
Tree-SHA512: 0e3c019e1dc05e54f559275859d3450e0c735596d179e30b66811aad9d5b5fabe3dcc44571e8f7b99f9fe16453eee393d6e153454dd873b9ff14907d4e6354fe
|
|
restricted to Tor
2feec3ce3130961f98ceb030951d0e46d3b9096c net: don't bind on 0.0.0.0 if binds are restricted to Tor (Vasil Dimov)
Pull request description:
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.
Before this change, if no `-bind` is given then we would bind to
`0.0.0.0:8333` and to `127.0.0.1:8334` (incoming Tor) which is ok -
the user does not care to restrict the binding.
However, if only `-bind=addr:port=onion` is given (without ordinary
`-bind=`) then we would bind to `addr:port` _and_ to `0.0.0.0:8333` in
addition.
Change the above to not do the additional bind: if only
`-bind=addr:port=onion` is given (without ordinary `-bind=`) then bind
to `addr:port` (only) and consider incoming connections to that as Tor
and do not advertise it. I.e. a Tor-only node.
ACKs for top commit:
laanwj:
Code review ACK 2feec3ce3130961f98ceb030951d0e46d3b9096c
jonatack:
utACK 2feec3ce3130961f98ceb030951d0e46d3b9096c per `git diff a004833 2feec3c`
hebasto:
ACK 2feec3ce3130961f98ceb030951d0e46d3b9096c, tested on Linux Mint 20.1 (x86_64):
Tree-SHA512: a04483af601706da928958b92dc560f9cfcc78ab0bb9d74414636eed1c6f29ed538ce1fb5a17d41ed82c9c9a45ca94899d0966e7ef93da809c9bcdcdb1d1f040
|
|
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
|
|
|
|
Co-authored-by: Antoine Riard <ariard@student.42.fr>
Co-authored-by: Antoine Riard <antoine.riard@gmail.com>
|
|
|
|
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.
Before this change, if no `-bind` is given then we would bind to
`0.0.0.0:8333` and to `127.0.0.1:8334` (incoming Tor) which is ok -
the user does not care to restrict the binding.
However, if only `-bind=addr:port=onion` is given (without ordinary
`-bind=`) then we would bind to `addr:port` _and_ to `0.0.0.0:8333` in
addition.
Change the above to not do the additional bind: if only
`-bind=addr:port=onion` is given (without ordinary `-bind=`) then bind
to `addr:port` (only) and consider incoming connections to that as Tor
and do not advertise it. I.e. a Tor-only node.
|
|
30aee2dfe671b347438c1c327c6f79edfacff1ce tests: Add test for compact block HB selection (Pieter Wuille)
6efbcec4ded6116a42d2783c96c60ef0f255a1b2 Protect last outbound HB compact block peer (Suhas Daftuar)
Pull request description:
If all our high-bandwidth compact block serving peers (BIP 152) stall block
download, then we can be denied a block for (potentially) a long time. As
inbound connections are much more likely to be adversarial than outbound
connections, mitigate this risk by never removing our last outbound HB peer if
it would be replaced by an inbound.
ACKs for top commit:
achow101:
ACK 30aee2dfe671b347438c1c327c6f79edfacff1ce
ariard:
Code ACK 30aee2dfe
jonatack:
ACK 30aee2dfe671b347438c1c327c6f79edfacff1ce
Tree-SHA512: 5c6c9326e3667b97e0864c371ae2174d2be9054dad479f4366127b9cd3ac60ffa01ec9707b16ef29cac122db6916cf56fd9985733390017134ace483278921d5
|
|
|
|
|
|
e4356f6a6c18e5027a064a4d3a5deda27985f584 Testcase for wallet issue with orphaned rewards. (Daniel Kraft)
Pull request description:
This adds a new test case demonstrating the wallet issue when block rewards are orphaned (#14148).
ACKs for top commit:
LarryRuane:
ACK e4356f6a6c18e5027a064a4d3a5deda27985f584
leonardojobim:
reACK https://github.com/bitcoin/bitcoin/pull/18795/commits/e4356f6a6c18e5027a064a4d3a5deda27985f584 .
Tree-SHA512: e9a2310ee1b3d52cfa302f431ed3d272bbc1b9195439ff318d9eb1006c0b28968dbe840e1600b6ff185e5d7ea57e4dcc837cef16051b5537445e10bc363b8c22
|
|
2667366aaa69447a9de4d819669d254a5ebd4d4b tests: check derivation of P2TR (Pieter Wuille)
7cedafc5412857404e9a6c3450b100cb8ee4081a Add tr() descriptor (derivation only, no signing) (Pieter Wuille)
90fcac365e1616779b40a69736428435df75fdf2 Add TaprootBuilder class (Pieter Wuille)
5f6cc8daa83700d1c949d968a5cf0d935be337b7 Add XOnlyPubKey::CreateTapTweak (Pieter Wuille)
2fbfb1becb3c0c109cd7c30b245b51da22039932 Make consensus checking of tweaks in pubkey.* Taproot-specific (Pieter Wuille)
a4bf84039c00b196b87f969acf6369d72c56ab46 Separate WitnessV1Taproot variant in CTxDestination (Pieter Wuille)
41839bdb89b3777ece2318877b9c7921ecca2472 Avoid dependence on CTxDestination index order (Pieter Wuille)
31df02a07091dbd5e0b315c8e5695e808f3a5505 Change Solver() output for WITNESS_V1_TAPROOT (Pieter Wuille)
4b1cc08f9f94a1e6e1ecba6b97f99b73fb513872 Make XOnlyPubKey act like byte container (Pieter Wuille)
Pull request description:
This is a subset of #21365, to aide review.
This adds support `tr(KEY)` or `tr(KEY,SCRIPT)` or `tr(KEY,{{S1,{{S2,S3},...}},...})` descriptors, describing Taproot outputs with specified internal key, and optionally any number of scripts, in nested groups of 2 inside `{`/`}` if there are more than one. While it permits importing `tr(KEY)`, anything beyond that is just laying foundations for more features later.
Missing:
* Signing support (see #21365)
* Support for more interesting scripts inside the tree (only `pk(KEY)` is supported for now). In particular, a multisig policy based on the new `OP_CHECKSIGADD` opcode would be very useful.
* Inferring `tr()` descriptors from outputs (given sufficient information).
* `getaddressinfo` support.
* MuSig support. Standardizing that is still an ongoing effort, and is generally kind of useless without corresponding PSBT support.
* Convenient ways of constructing descriptors without spendable internal key (especially ones that arent't trivially recognizable as such).
ACKs for top commit:
Sjors:
utACK 2667366 (based on https://github.com/bitcoin/bitcoin/pull/21365#issuecomment-846945215 review, plus the new functional test)
achow101:
Code Review ACK 2667366aaa69447a9de4d819669d254a5ebd4d4b
lsilva01:
Tested ACK https://github.com/bitcoin/bitcoin/pull/22051/commits/2667366aaa69447a9de4d819669d254a5ebd4d4b
meshcollider:
utACK 2667366aaa69447a9de4d819669d254a5ebd4d4b
Tree-SHA512: 61046fef22c561228338cb178422f0b782ef6587ec8208d3ce2bd07afcff29a664b54b35c6b01226eb70b6540b43f6dd245043d09aa6cb6db1381b6042667e75
|
|
This adds a new test case demonstrating the wallet issue when block
rewards are orphaned (https://github.com/bitcoin/bitcoin/issues/14148).
|
|
|
|
|
|
|
|
This commit moves the ExternalSigner class and RPC methods out of the wallet module.
The enumeratesigners RPC can be used without a wallet since #21417.
With additional modifications external signers could be used without a wallet in general, e.g. via signrawtransaction.
The signerdisplayaddress RPC is ranamed to walletdisplayaddress because it requires wallet context.
A future displayaddress RPC call without wallet context could take a descriptor argument.
This commit fixes a rpc_help.py failure when configured with --disable-wallet.
|
|
90ae3d8ca68334ec712d67b21a8d4721c6d79788 doc: Add release notes for -deprecatedrpc=addresses and bitcoin-tx (Michael Dietz)
085b3a729952896ccd0e40c17df569f4421f5493 rpc: deprecate `addresses` and `reqSigs` from rpc outputs (Michael Dietz)
Pull request description:
Considering the limited applicability of `reqSigs` and the confusing output of `1` in all cases except bare multisig, the `addresses` and `reqSigs` outputs are removed for all rpc commands.
1) add a new sane "address" field (for outputs that have an identifiable address, which doesn't include bare multisig)
2) with -deprecatedrpc: leave "reqSigs" and "addresses" intact (with all weird/wrong behavior they have now)
3) without -deprecatedrpc: drop "reqSigs" and "addresses" entirely always.
Note: Some light refactoring done to allow us to very easily delete a few chunks of code (marked with TODOs) when we remove this deprecated behavior.
Using `IsDeprecatedRPCEnabled` in core_write.cpp caused some circular dependencies involving core_io
Circular dependencies were caused by rpc/util unnecessarily importing node/coinstats and node/transaction. Really what rpc/util needs are some fundamental type/helper-function definitions. So this was cleaned up to make more sense.
This fixes #20102.
ACKs for top commit:
MarcoFalke:
re-ACK 90ae3d8ca68334ec712d67b21a8d4721c6d79788 📢
Tree-SHA512: 8ffb617053b5f4a8b055da17c06711fd19632e0037d71c4c8135e50c8cd7a19163989484e4e0f17a6cc48bd597f04ecbfd609aef54b7d1d1e76a784214fcf72a
|
|
1) add a new sane "address" field (for outputs that have an
identifiable address, which doesn't include bare multisig)
2) with -deprecatedrpc: leave "reqSigs" and "addresses" intact
(with all weird/wrong behavior they have now)
3) without -deprecatedrpc: drop "reqSigs" and "addresses" entirely,
always.
|
|
|
|
|
|
Includes a mock to mimick the HWI interace.
|
|
84716b134e9afca2fba7731de4af1f22fa1b1518 Add "index/blockfilterindex -> validation -> index/blockfilterindex" to expected circular dependencies (Jonas Schnelli)
ab3a0a2fb915d8b8384c30a8b38b4b5cc35236fd Add functional test for blockfilterindex in prune-mode (Jonas Schnelli)
c286a22f7b63a8bd336d5d7606c339053ee0054b Add debug startup parameter -fastprune for more effective pruning tests (Jonas Schnelli)
5e112269c311a559bfded814d3c3c438349a1986 Avoid pruning below the blockfilterindex sync height (Jonas Schnelli)
00d57ff76854938ead800767fb673a8af46eac8e Avoid accessing nullpointer in BaseIndex::GetSummary() (Jonas Schnelli)
6abe9f5b11cd4a5ecb6caca8443fe2950a417842 Allow blockfilter in conjunction with prune (Jonas Schnelli)
Pull request description:
Maintaining the blockfilterindexes in prune mode is possible and may lead to efficient p2p based rescans of wallets (restore backups, import/sweep keys) beyond the prune height (rescans not part of that PR).
This PR allows running the blockfilterindex(es) in conjunction with pruning.
* Bitcoind/Qt will shutdown during startup when missing block data has been detected ([re]enable `-blockfilterindex` when we already have pruned)
* manual block pruning is disabled during blockfilterindex sync
* auto-pruning is delayed during blockfilterindex sync
ToDos:
* [x] Functional tests
ACKs for top commit:
fjahr:
Code review ACK 84716b1
ryanofsky:
Code review ACK 84716b134e9afca2fba7731de4af1f22fa1b1518. Only changes since last review were suggested new FindFilesToPrune argument and test.
benthecarman:
tACK 84716b134e9afca2fba7731de4af1f22fa1b1518
Tree-SHA512: 91d832c6c562c463f7ec7655c08956385413a99a896640b9737bda0183607fac530435d03d87c3c0e70c61ccdfe73fe8f3639bc7d26d33ca7e60925ebb97d77a
|
|
|
|
e987ae5a554c9952812746c29f2766bacea4b727 test: Add test for deterministic UTXO set hash results (Fabian Jahr)
6ccc8fc067bf516cda7bc5d7d721945be5ac2003 test: Add test for gettxoutsetinfo RPC with MuHash (Fabian Jahr)
0d3b2f643d7da3202c0a0e757539208c4aa7c450 rpc: Add hash_type MUHASH to gettxoutsetinfo (Fabian Jahr)
2474645f3b15687e7f196b89eb935d6e6a98a9da refactor: Separate hash and stats calculation in coinstats (Fabian Jahr)
a1fcceac69097a8e6540a6fd8121a5d53022528f refactor: Improve encapsulation between MuHash3072 and Num3072 (Fabian Jahr)
Pull request description:
This is another Pr in the series PRs for Coinstatsindex (see overview in #18000). This PR adds the `hash_type` option `muhash` to `gettxoutsetinfo` through which the user can calculate the serialized muhash of the utxo set. This PR does not use the index yet.
ACKs for top commit:
Sjors:
tACK e987ae5
achow101:
ACK e987ae5a554c9952812746c29f2766bacea4b727
jonatack:
Tested re-ACK e987ae5a554c9952812746c29f2766bacea4b727 per `git diff 3506d90 e987ae5`, reviewed diff, debug built, ran gettxoutsetinfo -signet and help on this branch vs master, at height 23127 both returned `hash_serialized_2` of `2b72d65f3b6efb2311f58374ea2b939abf49684d44f4bafda45faa3b5452a454` and this branch returned `muhash` of `c9f1ff12d345ccf9939c6bbf087e6f7399b6115adee1569287e9c5c43dbb475c`
ryanofsky:
Code review ACK e987ae5a554c9952812746c29f2766bacea4b727. Looks very good. I left one suggestion to simplify code, but feel free to ignore it here and maybe consider it for later since PR has already had a lot of review.
Tree-SHA512: 9a739ce375e73749fa69a467262b60d3e5314ef384e2d7150b3bbc8e4125cd9fd1db95306623bb9a632fcbaf5d9d2bf2f5cc43bf717d4ff5e2c9c4b52dd9296c
|
|
49797c3ccfbb9f7ac9c1fbb574d35b315c103805 tests: Disable bdb dump test when no bdb (Andrew Chow)
1194cf9269e6c4bf67b09c4766f42bf173d12c0a Fix wallet_send.py wallet setup to work with descriptors (Andrew Chow)
fbaea7bfe44822710a36601c6b0febbd5e33dfbd Require legacy wallet for wallet_upgradewallet.py (Andrew Chow)
b1b679e0ab7a9981e3e78424fe8836edd59abf6f Explicitly mark legacy wallet tests as such (Andrew Chow)
09514e1bef46444a67cde9ff13e76bd4b9f8c7ac Setup wallets for interface_zmq.py (Andrew Chow)
4d03ef9a73ceb5ef4e9d184a135bca6bdeb8c311 Use MiniWallet in rpc_net.py (Andrew Chow)
4de23824b0c711ece68f9fc007ffac12126710aa Setup wallets for interface_bitcoin_cli.py (Andrew Chow)
7c71c627d28f0cddaf2349a55336278a681c27c2 Setup wallets with descriptors for feature_notifications (Andrew Chow)
1f1bef8dbab7225884d769a45477ee11d0ebf654 Have feature_filelock.py test both bdb and sqlite, depending on compiled (Andrew Chow)
c77975abc0123b29b0eb3481b8916e7c025b7c4c Disable upgrades tests that require BDB if BDB is not compiled (Andrew Chow)
1f20cac9d41e507901a2811d6db7147d7ab0321b Disable wallet_descriptor.py bdb format check if BDB is not compiled (Andrew Chow)
3641597d7ef6f5097a9e93cab3ef7e0f9c820296 tests: Don't make any wallets unless wallet is required (Andrew Chow)
b9b88f57a9b9a28e0f0614c12ae3012cf5050b10 Skip legacy wallet reliant tests if BDB is not compiled (Andrew Chow)
6f36242389bd3e7eacf594ce90491e8ccca70f3a tests: Set descriptors default based on compilation (Andrew Chow)
Pull request description:
This PR fixes tests for when BDB is not compiled. Tests which rely on or test legacy wallet behavior are disabled and skipped when BDB is not compiled. For the components of some tests that are for legacy wallet things, those parts of the tests are skipped.
For the majority of tests, changes are made so that they can be run with either legacy wallets or descriptor wallets without materially effecting the test. Most tests only need the wallet for balance and transactions, so the type of wallet is not an important part of those tests. Additionally, some tests are wallet agnostic and modified to instead use the test framework's MiniWallet.
ACKs for top commit:
laanwj:
ACK 49797c3ccfbb9f7ac9c1fbb574d35b315c103805
ryanofsky:
Code review ACK 49797c3ccfbb9f7ac9c1fbb574d35b315c103805. Only change since last review is dropping last commit. Previous review w/ suggestions for future followup is https://github.com/bitcoin/bitcoin/pull/20267#pullrequestreview-581508843
Tree-SHA512: 69659f8a81fb437ecbca962f4082c12835282dbf1fba7d9952f727a49e01981d749af9b09feda1c8ca737516c7d7a08ef17e782795df3fa69892d5021b41c1ed
|
|
bff7c66e67aa2f18ef70139338643656a54444fe Add documentation to contrib folder (Troy Giorshev)
381f77be858d7417209b6de0b7cd23cb7eb99261 Add Message Capture Test (Troy Giorshev)
e4f378a505922c0f544b4cfbfdb169e884e02be9 Add capture parser (Troy Giorshev)
4d1a582549bc982d55e24585b0ba06f92f21e9da Call CaptureMessage at appropriate locations (Troy Giorshev)
f2a77ff97bec09dd5fcc043d8659d8ec5dfb87c2 Add CaptureMessage (Troy Giorshev)
dbf779d5deb04f55c6e8493ce4e12ed4628638f3 Clean PushMessage and ProcessMessages (Troy Giorshev)
Pull request description:
This PR introduces per-peer message capture into Bitcoin Core. 📓
## Purpose
The purpose and scope of this feature is intentionally limited. It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?".
## Functionality
When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured. The capture occurs in the MessageHandler thread. When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue. When sending, the message is captured just before the message is pushed onto the vSendMsg queue.
The message capture is as minimal as possible to reduce the performance impact on the node. Messages are captured to a new `message_capture` folder in the datadir. Each node has their own subfolder named with their IP address and port. Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so:
```
message_capture/203.0.113.7:56072/msgs_recv.dat
message_capture/203.0.113.7:56072/msgs_sent.dat
```
Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON. This script has been placed on its own and out of the way in the new `contrib/message-capture` folder. Its usage is simple and easily discovered by the autogenerated `-h` option.
## Future Maintenance
I sympathize greatly with anyone who says "the best code is no code".
The future maintenance of this feature will be minimal. The logic to deserialize the payload of the p2p messages exists in our testing framework. As long as our testing framework works, so will this tool.
Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small.
## FAQ
"Why not just use Wireshark"
Yes, Wireshark has the ability to filter and decode Bitcoin messages. However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol. This drives the design in a different direction than Wireshark, in two different ways. First, this tool must be convenient and simple to use. Using an external tool, like Wireshark, requires setup and interpretation of the results. To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty. This tool, on the other hand, "just works". Turn on the command line flag, run your node, run the script, read the JSON. Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible. A lot can happen in the SocketHandler thread that would be missed by Wireshark.
Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent. As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done.
Lastly, I truly believe that this tool will be used significantly more by being included in the codebase. It's just that much more discoverable.
ACKs for top commit:
MarcoFalke:
re-ACK bff7c66e67aa2f18ef70139338643656a54444fe only some minor changes: 👚
jnewbery:
utACK bff7c66e67aa2f18ef70139338643656a54444fe
theStack:
re-ACK bff7c66e67aa2f18ef70139338643656a54444fe
Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47
|
|
|
|
647b81b70938dc4dbcf32399c56f78be395c721a wallet, rpc: add listdescriptors command (Ivan Metlushko)
Pull request description:
Looking for concept ACKs
**Rationale**: allow users to inspect the contents of their newly created descriptor wallets.
Currently the command only returns xpubs which is not very useful in itself, but there are multiples ways to extend it:
* add an option to export xprv
* with #19136 it'll be possible to return normalised descriptors suitable for a watch-only purposes
The output is compatible with `importdescriptors` command so it could be easily used for backup/recover purposes.
**Output example:**
```json
[
{
"desc": "wpkh(tpubD6NzVbkrYhZ4WW6E2ZETFyNfq2hfF23SKxqSGFvUpPAY58jmmuBybwqwFihAyQPk9KnwTt5516NDZRJ7k5QPeKjy7wuVd5WvXNxwwAs5tUD/*)#nhavpr5h",
"timestamp": 1296688602,
"active": false,
"range": [
0,
999
],
"next": 0
}
]
```
ACKs for top commit:
jonatack:
re-ACK 647b81b70938dc4dbcf32399c56f78be395c721a rebased to master, debug builds cleanly, reviewed diff since last review, tested with a descriptor wallet (and with a legacy wallet)
achow101:
re-ACK 647b81b
Tree-SHA512: 51a3620bb17c836c52cecb066d4fa9d5ff418af56809046eaee0528c4dc240a4e90fff5711ba96e399c6664e00b9ee8194e33852b1b9e75af18061296e19a8a7
|
|
|
|
Fixes the wallet setup so this test works with descriptor wallets. Also
enabled explicit descriptor and legacy wallet testing in the test
runner.
|
|
|
|
Some tests are intended to test only legacy wallet behavior. With
automatic switching of wallet type, we need to make them explicit
|
|
This commit addresses #20809.
We add an additional 'error' property in the result of 'validateaddress' in case the address is not valid that gives a short description of why the address in invalid. We also change the error message returned by 'getaddressinfo' in case the address is invalid.
|
|
Add a functional test for CaptureMessage. This connects and then
disconnects a peer so that the handshake can be used to check if capture
is being done correctly.
Included in a docstring in the test is the following:
From the data file we'll only check the structure.
We won't care about things like:
- Deserializing the payload of the message
- This is managed by the deserialize methods in
test_framework.messages
- The order of the messages
- There's no reason why we can't, say, change the order of the
messages in the handshake
- Message Type
- We can add new message types
We're ignoring these because they're simply too brittle to test here.
|
|
Open max number of full-relay and block-relay-only connections from a
functional test with different sorts of behaviors to ensure it behaves as
expected.
|
|
|
|
|
|
Although legacy wallet is still the default, for future use, add a
--legacy-wallet option to the test framework. Additional tests for
descriptor wallets have been enabled with the --descriptors option.
Tests that must be legacy wallet only are being started with
--legacy-wallet. Even though this option does not currently do anything,
this will be helpful in the future when descriptor wallets become the
default.
|
|
|