Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport: merge bitcoin#27452, #29347, #29356, #29353, #29452, #29483, #30545, #31383 (BIP324 backports: part 5) #6488

Merged
merged 9 commits into from
Dec 15, 2024
1 change: 1 addition & 0 deletions ci/test/00_setup_env_native_multiprocess.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ export CONTAINER_NAME=ci_native_multiprocess
export PACKAGES="cmake python3 llvm clang"
export DEP_OPTS="DEBUG=1 MULTIPROCESS=1"
export GOAL="install"
export TEST_RUNNER_EXTRA="--v2transport"
export BITCOIN_CONFIG="--with-boost-process --enable-debug CC=clang CXX=clang++" # Use clang to avoid OOM
export TEST_RUNNER_ENV="BITCOIND=dash-node"
2 changes: 1 addition & 1 deletion doc/bips.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@ Versions and PRs are relevant to Bitcoin's core if not mentioned other.
* [`BIP 158`](https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki): Compact Block Filters for Light Clients can be indexed as of **Dash Core v18.0** ([PR dash#4314](https://github.com/dashpay/dash/pull/4314), [PR #14121](https://github.com/bitcoin/bitcoin/pull/14121)).
* [`BIP 159`](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki): The `NODE_NETWORK_LIMITED` service bit is signalled as of **v0.16.0** ([PR 11740](https://github.com/bitcoin/bitcoin/pull/11740)), and such nodes are connected to as of **v0.17.0** ([PR 10387](https://github.com/bitcoin/bitcoin/pull/10387)).
* [`BIP 174`](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki): RPCs to operate on Partially Signed Bitcoin Transactions (PSBT) are present as of **v18.0** ([PR 13557](https://github.com/bitcoin/bitcoin/pull/13557)).
* [`BIP 324`](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki): The v2 transport protocol specified by BIP324 and the associated `NODE_P2P_V2` service bit are supported as of **v22.0**, but off by default ([PR 28331](https://github.com/bitcoin/bitcoin/pull/28331)).
* [`BIP 324`](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki): The v2 transport protocol specified by BIP324 and the associated `NODE_P2P_V2` service bit are supported as of **v22.0**, but off by default ([PR 28331](https://github.com/bitcoin/bitcoin/pull/28331)). On by default as of **v22.1** ([PR 29347](https://github.com/bitcoin/bitcoin/pull/29347)).
* [`BIP 339`](https://github.com/bitcoin/bips/blob/master/bip-0339.mediawiki): Relay of transactions by wtxid is supported as of **v0.21.0** ([PR 18044](https://github.com/bitcoin/bitcoin/pull/18044)).
2 changes: 1 addition & 1 deletion src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ static const bool DEFAULT_FIXEDSEEDS = true;
static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000;
static const size_t DEFAULT_MAXSENDBUFFER = 1 * 1000;

static constexpr bool DEFAULT_V2_TRANSPORT{false};
static constexpr bool DEFAULT_V2_TRANSPORT{true};

#if defined USE_KQUEUE
#define DEFAULT_SOCKETEVENTS "kqueue"
Expand Down
9 changes: 5 additions & 4 deletions src/rpc/masternode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ static RPCHelpMan masternode_connect()
"Connect to given masternode\n",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The address of the masternode to connect"},
{"v2transport", RPCArg::Type::BOOL, RPCArg::Default{false}, "Attempt to connect using BIP324 v2 transport protocol"},
{"v2transport", RPCArg::Type::BOOL, RPCArg::DefaultHint{"set by -v2transport"}, "Attempt to connect using BIP324 v2 transport protocol"},
},
RPCResults{},
RPCExamples{""},
Expand All @@ -51,12 +51,13 @@ static RPCHelpMan masternode_connect()
throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Incorrect masternode address %s", strAddress));
}

bool use_v2transport = !request.params[1].isNull() && ParseBoolV(request.params[1], "v2transport");

const NodeContext& node = EnsureAnyNodeContext(request.context);
CConnman& connman = EnsureConnman(node);

if (use_v2transport && !(connman.GetLocalServices() & NODE_P2P_V2)) {
bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2;
bool use_v2transport = request.params[1].isNull() ? node_v2transport : ParseBoolV(request.params[1], "v2transport");

if (use_v2transport && !node_v2transport) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: Adding v2transport connections requires -v2transport init flag to be set.");
}

Expand Down
2 changes: 1 addition & 1 deletion src/rpc/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ static RPCHelpMan addconnection()
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address and port to attempt connecting to."},
{"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\", \"addr-fetch\" or \"feeler\")."},
{"v2transport", RPCArg::Type::BOOL, RPCArg::Default{false}, "Attempt to connect using BIP324 v2 transport protocol"},
{"v2transport", RPCArg::Type::BOOL, RPCArg::Optional::NO, "Attempt to connect using BIP324 v2 transport protocol"},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

29356: nit: probably partial because of missing bool use_v2transport = self.Arg<bool>(2); part, not sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.Arg<bool>(2) is the equivalent of request.params[2].get_bool(), the newer syntax being introduced in bitcoin#28230 and fleshed out in bitcoin#29277, backporting them out of order brings little benefit but would make backports until that point annoying at best, so it was decided to skip that change but not mark it as partial as the behaviour should be identical.

Though the !request.params[2].isNull() half of use_v2transport is a no-op as v2transport is no longer optional and is a nit that can be addressed later on.

},
RPCResult{
RPCResult::Type::OBJ, "", "",
Expand Down
61 changes: 58 additions & 3 deletions test/functional/feature_anchors.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
import os

from test_framework.p2p import P2PInterface
kwvg marked this conversation as resolved.
Show resolved Hide resolved
from test_framework.socks5 import Socks5Configuration, Socks5Server
from test_framework.messages import CAddress, hash256, NODE_NETWORK
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import check_node_connections
from test_framework.util import check_node_connections, assert_equal, p2p_port

INBOUND_CONNECTIONS = 5
BLOCK_RELAY_CONNECTIONS = 2
ONION_ADDR = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion:8333"


class AnchorsTest(BitcoinTestFramework):
Expand Down Expand Up @@ -55,7 +58,7 @@ def run_test(self):
else:
inbound_nodes_port.append(hex(int(addr_split[1]))[2:])

self.log.info("Stop node 0")
self.log.debug("Stop node")
self.stop_node(0)

# It should contain only the block-relay-only addresses
Expand All @@ -79,12 +82,64 @@ def run_test(self):
tweaked_contents[20:20] = b'1'
out_file_handler.write(bytes(tweaked_contents))

self.log.info("Start node")
self.log.debug("Start node")
self.start_node(0)

self.log.info("When node starts, check if anchors.dat doesn't exist anymore")
assert not os.path.exists(node_anchors_path)

self.log.info("Ensure addrv2 support")
# Use proxies to catch outbound connections to networks with 256-bit addresses
onion_conf = Socks5Configuration()
onion_conf.auth = True
onion_conf.unauth = True
onion_conf.addr = ('127.0.0.1', p2p_port(self.num_nodes))
onion_conf.keep_alive = True
onion_proxy = Socks5Server(onion_conf)
onion_proxy.start()
self.restart_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"])

self.log.info("Add 256-bit-address block-relay-only connections to node")
self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only', v2transport=False)

self.log.debug("Stop node")
with self.nodes[0].assert_debug_log([f"DumpAnchors: Flush 1 outbound block-relay-only peer addresses to anchors.dat"]):
self.stop_node(0)
# Manually close keep_alive proxy connection
onion_proxy.stop()

self.log.info("Check for addrv2 addresses in anchors.dat")
caddr = CAddress()
caddr.net = CAddress.NET_TORV3
caddr.ip, port_str = ONION_ADDR.split(":")
caddr.port = int(port_str)
# TorV3 addrv2 serialization:
# time(4) | services(1) | networkID(1) | address length(1) | address(32)
expected_pubkey = caddr.serialize_v2()[7:39].hex()

# position of services byte of first addr in anchors.dat
# network magic, vector length, version, nTime
services_index = 4 + 1 + 4 + 4
data = bytes()
with open(node_anchors_path, "rb") as file_handler:
data = file_handler.read()
assert_equal(data[services_index], 0x00) # services == NONE
anchors2 = data.hex()
assert expected_pubkey in anchors2

with open(node_anchors_path, "wb") as file_handler:
# Modify service flags for this address even though we never connected to it.
# This is necessary because on restart we will not attempt an anchor connection
# to a host without our required services, even if its address is in the anchors.dat file
new_data = bytearray(data)[:-32]
new_data[services_index] = NODE_NETWORK
new_data_hash = hash256(new_data)
file_handler.write(new_data + new_data_hash)

self.log.info("Restarting node attempts to reconnect to anchors")
with self.nodes[0].assert_debug_log([f"Trying to make an anchor connection to {ONION_ADDR}"]):
self.start_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"])


if __name__ == "__main__":
AnchorsTest().main()
11 changes: 6 additions & 5 deletions test/functional/feature_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
rv = []
addr = "15.61.23.23:1234"
self.log.debug(f"Test: outgoing IPv4 connection through node for address {addr}")
node.addnode(addr, "onetry")
# v2transport=False is used to avoid reconnections with v1 being scheduled. These could interfere with later checks.
node.addnode(addr, "onetry", v2transport=False)
cmd = proxies[0].queue.get()
assert isinstance(cmd, Socks5Command)
# Note: dashd's SOCKS5 implementation only sends atyp DOMAINNAME, even if connecting directly to IPv4/IPv6
Expand All @@ -141,7 +142,7 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
if self.have_ipv6:
addr = "[1233:3432:2434:2343:3234:2345:6546:4534]:5443"
self.log.debug(f"Test: outgoing IPv6 connection through node for address {addr}")
node.addnode(addr, "onetry")
node.addnode(addr, "onetry", v2transport=False)
cmd = proxies[1].queue.get()
assert isinstance(cmd, Socks5Command)
# Note: dashd's SOCKS5 implementation only sends atyp DOMAINNAME, even if connecting directly to IPv4/IPv6
Expand All @@ -157,7 +158,7 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
if test_onion:
addr = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion:8333"
self.log.debug(f"Test: outgoing onion connection through node for address {addr}")
node.addnode(addr, "onetry")
node.addnode(addr, "onetry", v2transport=False)
cmd = proxies[2].queue.get()
assert isinstance(cmd, Socks5Command)
assert_equal(cmd.atyp, AddressType.DOMAINNAME)
Expand All @@ -172,7 +173,7 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
if test_cjdns:
addr = "[fc00:1:2:3:4:5:6:7]:8888"
self.log.debug(f"Test: outgoing CJDNS connection through node for address {addr}")
node.addnode(addr, "onetry")
node.addnode(addr, "onetry", v2transport=False)
cmd = proxies[1].queue.get()
assert isinstance(cmd, Socks5Command)
assert_equal(cmd.atyp, AddressType.DOMAINNAME)
Expand All @@ -186,7 +187,7 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):

addr = "node.noumenon:8333"
self.log.debug(f"Test: outgoing DNS name connection through node for address {addr}")
node.addnode(addr, "onetry")
node.addnode(addr, "onetry", v2transport=False)
cmd = proxies[3].queue.get()
assert isinstance(cmd, Socks5Command)
assert_equal(cmd.atyp, AddressType.DOMAINNAME)
Expand Down
2 changes: 1 addition & 1 deletion test/functional/p2p_add_connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def on_version(self, message):
# message is received from the test framework. Don't send any responses
# to the node's version message since the connection will already be
# closed.
pass
self.send_version()

class P2PAddConnections(BitcoinTestFramework):
def set_test_params(self):
Expand Down
1 change: 1 addition & 0 deletions test/functional/p2p_addr_relay.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def addr_received(self):
return self.num_ipv4_received != 0

def on_version(self, message):
self.send_version()
self.send_message(msg_verack())
if (self.send_getaddr):
self.send_message(msg_getaddr())
Expand Down
24 changes: 20 additions & 4 deletions test/functional/p2p_addrv2_relay.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from test_framework.util import assert_equal

I2P_ADDR = "c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p"
ONION_ADDR = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"

ADDRS: List[CAddress] = []

Expand All @@ -37,6 +38,16 @@ def on_addrv2(self, message):
def wait_for_addrv2(self):
self.wait_until(lambda: "addrv2" in self.last_message)

def calc_addrv2_msg_size(addrs):
size = 1 # vector length byte
for addr in addrs:
size += 4 # time
size += 1 # services, COMPACTSIZE(P2P_SERVICES)
size += 1 # network id
size += 1 # address length byte
size += addr.ADDRV2_ADDRESS_LENGTH[addr.net] # address
size += 2 # port
return size

class AddrTest(BitcoinTestFramework):
def set_test_params(self):
Expand All @@ -48,14 +59,18 @@ def run_test(self):
for i in range(10):
addr = CAddress()
addr.time = int(self.mocktime) + i
addr.port = 8333 + i
addr.nServices = NODE_NETWORK
# Add one I2P address at an arbitrary position.
# Add one I2P and one onion V3 address at an arbitrary position.
if i == 5:
addr.net = addr.NET_I2P
addr.ip = I2P_ADDR
addr.port = 0
elif i == 8:
addr.net = addr.NET_TORV3
addr.ip = ONION_ADDR
else:
addr.ip = f"123.123.123.{i % 256}"
addr.port = 8333 + i
ADDRS.append(addr)

self.log.info('Create connection that sends addrv2 messages')
Expand All @@ -73,14 +88,15 @@ def run_test(self):
addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver())
msg.addrs = ADDRS
msg_size = calc_addrv2_msg_size(ADDRS)
with self.nodes[0].assert_debug_log([
'received: addrv2 (159 bytes) peer=1',
f'received: addrv2 ({msg_size} bytes) peer=1',
]):
addr_source.send_and_ping(msg)

# Wait until "Added ..." before bumping mocktime to make sure addv2 is (almost) fully processed
with self.nodes[0].assert_debug_log([
'sending addrv2 (159 bytes) peer=2',
f'sending addrv2 ({msg_size} bytes) peer=2',
]):
self.bump_mocktime(30 * 60)
addr_receiver.wait_for_addrv2()
Expand Down
1 change: 1 addition & 0 deletions test/functional/p2p_ibd_stalling.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def run_test(self):

self.log.info("Check that a staller does not get disconnected if the 1024 block lookahead buffer is filled")
self.mocktime = int(time.time()) + 1
node.setmocktime(self.mocktime)
for id in range(NUM_PEERS):
peers.append(node.add_outbound_p2p_connection(P2PStaller(stall_block), services = NODE_NETWORK | NODE_BLOOM, p2p_idx=id, connection_type="outbound-full-relay"))
peers[-1].block_store = block_dict
Expand Down
5 changes: 3 additions & 2 deletions test/functional/p2p_sendtxrcncl.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def __init__(self,):

def on_version(self, message):
# Avoid sending verack in response to version.
pass
self.send_version()

class SendTxrcnclReceiver(P2PInterface):
def __init__(self):
Expand All @@ -38,7 +38,8 @@ def on_sendtxrcncl(self, message):

class P2PFeelerReceiver(SendTxrcnclReceiver):
def on_version(self, message):
pass # feeler connections can not send any message other than their own version
# feeler connections can not send any message other than their own version
self.send_version()


class PeerTrackMsgOrder(P2PInterface):
Expand Down
Loading
Loading