sipa
commented at 7:14 pm on January 29, 2024:
member
This enables BIP324’s v2 transport by default (see #27634):
Inbound connections will auto-sense whether v1 or v2 is in use.
Automatic outbound connections will use v2 if NODE_P2P_V2 was set in addr gossip, but retry with v1 if met with immediate failure.
Manual outbound connections will default to v2, but retry with v1 if met with immediate failure.
It remains possible to run with -v2transport=0 to disable all of these, and make all outbound and inbound connections v1. It also remains possible to specify the v2transport argument to the addnode RPC as false, to disable attempting a v2 connection for that particular added node.
DrahtBot
commented at 7:14 pm on January 29, 2024:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
pablomartin4btc
commented at 10:03 pm on January 29, 2024:
member
ACK292e716cde3325bef83e78f7804d2d0bddf03509
1440000bytes
commented at 10:10 pm on January 29, 2024:
none
Concept ACK
Not sure if it should be enabled for onion/i2p connections by default.
sipa
commented at 10:22 pm on January 29, 2024:
member
Hmm, no. The issue is that previous releases may not even support the -v2transport flag, so to determine whether to set it, we need to know what version we’re talking about.
DrahtBot added the label
CI failed
on Jan 29, 2024
pablomartin4btc
commented at 10:29 pm on January 29, 2024:
member
Hmm, no. The issue is that previous releases may not even support the -v2transport flag, so to determine whether to set it, we need to know what version we’re talking about.
Sorry I deleted my comment cos I realised it was wrong as other tests would have failed… originally was:
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot removed review request from josibake
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
in
test/functional/test_framework/test_node.py:133
in
0bef1042ce
129@@ -130,8 +130,15 @@ def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor,
130 # Default behavior from global -v2transport flag is added to args to persist it over restarts.
131 # May be overwritten in individual tests, using extra_args.
132 self.default_to_v2 = v2transport
133- if self.default_to_v2:
naumenkogs
commented at 9:19 am on January 30, 2024:
i’m not sure we want to keep default_to_v2. It’s kinda unused now, and I’m not sure future tests would ever find it useful.
naumenkogs
commented at 9:19 am on January 30, 2024:
member
ACK0bef1042ce6c459acb1de965cbccd98867a417f1
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
theStack approved
theStack
commented at 9:24 am on January 30, 2024:
contributor
ACK0bef1042ce6c459acb1de965cbccd98867a417f1
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
willcl-ark
commented at 11:58 am on January 30, 2024:
contributor
If we are now defaulting v2 to enabled, do we want to enable v2transport on a few more of the p2p functional tests, here or in a followup? I currently see the following enabled/disabled variants running the test suite with default options:
0p2p_add_connections.py |✓ Passed |7 s
1p2p_addr_relay.py |✓ Passed |15 s
2p2p_addrfetch.py |✓ Passed |1 s
3p2p_addrv2_relay.py |✓ Passed |1 s
4p2p_block_sync.py |✓ Passed |2 s
5p2p_block_sync.py --v2transport |✓ Passed |2 s
6p2p_blockfilters.py |✓ Passed |84 s
7p2p_blocksonly.py |✓ Passed |16 s
8p2p_compactblocks.py |✓ Passed |10 s
9p2p_compactblocks_blocksonly.py |✓ Passed |2 s
10p2p_compactblocks_hb.py |✓ Passed |18 s
11p2p_compactblocks_hb.py --v2transport |✓ Passed |18 s
12p2p_disconnect_ban.py |✓ Passed |2 s
13p2p_disconnect_ban.py --v2transport |✓ Passed |2 s
14p2p_dns_seeds.py |✓ Passed |27 s
15p2p_dos_header_tree.py |✓ Passed |2 s
16p2p_eviction.py |✓ Passed |5 s
17p2p_feefilter.py |✓ Passed |16 s
18p2p_filter.py |✓ Passed |3 s
19p2p_fingerprint.py |✓ Passed |2 s
20p2p_getaddr_caching.py |✓ Passed |8 s
21p2p_getdata.py |✓ Passed |1 s
22p2p_headers_sync_with_minchainwork.py |✓ Passed |50 s
23p2p_i2p_ports.py |✓ Passed |3 s
24p2p_i2p_sessions.py |✓ Passed |1 s
25p2p_ibd_stalling.py |✓ Passed |37 s
26p2p_ibd_stalling.py --v2transport |✓ Passed |37 s
27p2p_ibd_txrelay.py |✓ Passed |2 s
28p2p_initial_headers_sync.py |✓ Passed |1 s
29p2p_invalid_block.py |✓ Passed |2 s
30p2p_invalid_block.py --v2transport |✓ Passed |2 s
31p2p_invalid_locator.py |✓ Passed |1 s
32p2p_invalid_messages.py |✓ Passed |29 s
33p2p_invalid_tx.py |✓ Passed |8 s
34p2p_invalid_tx.py --v2transport |✓ Passed |7 s
35p2p_leak.py |✓ Passed |6 s
36p2p_leak_tx.py |✓ Passed |23 s
37p2p_leak_tx.py --v2transport |✓ Passed |8 s
38p2p_message_capture.py |✓ Passed |1 s
39p2p_net_deadlock.py |✓ Passed |2 s
40p2p_net_deadlock.py --v2transport |✓ Passed |2 s
41p2p_nobloomfilter_messages.py |✓ Passed |1 s
42p2p_node_network_limited.py |✓ Passed |10 s
43p2p_node_network_limited.py --v2transport |✓ Passed |11 s
44p2p_orphan_handling.py |✓ Passed |7 s
45p2p_permissions.py |✓ Passed |8 s
46p2p_ping.py |✓ Passed |1 s
47p2p_segwit.py |✓ Passed |104 s
48p2p_sendheaders.py |✓ Passed |23 s
49p2p_sendtxrcncl.py |✓ Passed |5 s
50p2p_timeouts.py |✓ Passed |1 s
51p2p_timeouts.py --v2transport |✓ Passed |1 s
52p2p_tx_download.py |✓ Passed |36 s
53p2p_tx_privacy.py |✓ Passed |8 s
54p2p_unrequested_blocks.py |✓ Passed |5 s
55p2p_v2_earlykeyresponse.py |✓ Passed |4 s
56p2p_v2_encrypted.py |✓ Passed |8 s
57p2p_v2_transport.py |✓ Passed |8 s
By adding some logging I measure that it is currently only enabled on 21 out of 102 p2p_*.py test nodes used running the suite. It can be enabled on all 102 test nodes with the global --v2transport option passed to test_runner, but I don’t think that’s used either by our CI or (routinely) by developers.
I appreciate that v2 should not directly affect many of these tests, but one of the benefits of the functional tests is to catch unforeseen interactions.
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
sipa
commented at 1:36 pm on January 30, 2024:
member
@willcl-ark I believe @mzumsande is planning to open a PR to enable it everywhere in the tests.
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
willcl-ark approved
willcl-ark
commented at 2:10 pm on January 30, 2024:
contributor
crACK0bef1042ce6c459acb1de965cbccd98867a417f1
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
BrandonOdiwuor approved
BrandonOdiwuor
commented at 2:27 pm on January 30, 2024:
contributor
utACK0bef1042ce6c459acb1de965cbccd98867a417f1
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
mzumsande
commented at 3:09 pm on January 30, 2024:
contributor
Concept ACK
@willcl-ark I believe @mzumsande is planning to open a PR to enable it everywhere in the tests.
If we are now defaulting v2 to enabled, do we want to enable v2transport on a few more of the p2p functional tests
It’s more than adding more tests to the list, the -v2transport option currently doesn’t make the python P2PConnection use v2 (even after #24748 added support), so the status quo is that specific tests need to enable that explicitly.
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
pablomartin4btc approved
pablomartin4btc
commented at 4:44 pm on January 30, 2024:
member
re ACK0bef1042ce6c459acb1de965cbccd98867a417f1
DrahtBot removed review request from kristapsk
on Jan 30, 2024
DrahtBot requested review from mzumsande
on Jan 30, 2024
DrahtBot requested review from kristapsk
on Jan 30, 2024
epiccurious
commented at 4:05 pm on January 31, 2024:
none
utACK
DrahtBot removed review request from kristapsk
on Jan 31, 2024
DrahtBot requested review from kristapsk
on Jan 31, 2024
kristapsk approved
kristapsk
commented at 7:32 pm on January 31, 2024:
contributor
utACK0bef1042ce6c459acb1de965cbccd98867a417f1
achow101 added the label
Needs release note
on Jan 31, 2024
achow101
commented at 8:24 pm on January 31, 2024:
member
ACK0bef1042ce6c459acb1de965cbccd98867a417f1
achow101 merged this
on Jan 31, 2024
achow101 closed this
on Jan 31, 2024
maflcko
commented at 12:20 pm on February 19, 2024:
member
Needs bips.md updated?
glozow referenced this in commit
ddf1d72cc2
on Feb 19, 2024
fanquake removed the label
Needs release note
on Mar 4, 2024
fanquake
commented at 11:57 am on March 4, 2024:
member
Added rel-note todo to the draft wiki.
luke-jr referenced this in commit
ff52aa4a22
on Mar 5, 2024
twofaktor
commented at 10:58 pm on April 1, 2024:
none
Will it still be neccesary to specify v2transport=1 parameter on
bitcoin.conf starting from version 27 to support v2 transport protocol?
mzumsande
commented at 0:17 am on April 2, 2024:
contributor
Will it still be neccesary to specify v2transport=1 parameter on bitcoin.conf starting from version 27 to support v2 transport protocol?
No, that won’t be necessary (thought it won’t hurt to leave it in).
The default was changed with this PR, so if you wanted to not support v2 for some reason you’d now have to include v2transport=0 in your bitcoin.conf.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2024-12-11 06:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me