From 7c3ac598dd9a1f1a506c4931249ff6c9f1c949ba Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 7 Mar 2024 11:27:07 +0000 Subject: [PATCH 01/72] contrib: list other binaries in manpage output --- contrib/devtools/gen-manpages.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contrib/devtools/gen-manpages.py b/contrib/devtools/gen-manpages.py index 2860e7db99..92acd9a403 100755 --- a/contrib/devtools/gen-manpages.py +++ b/contrib/devtools/gen-manpages.py @@ -62,6 +62,10 @@ # Copyright is the same for all binaries, so just use the first. footer.write('[COPYRIGHT]\n') footer.write('\n'.join(versions[0][2]).strip()) + # Create SEE ALSO section + footer.write('\n[SEE ALSO]\n') + footer.write(', '.join(s.rpartition('/')[2] + '(1)' for s in BINARIES)) + footer.write('\n') footer.flush() # Call the binaries through help2man to produce a manual page for each of them. From 3e9c736a26724ffe3b70b387995fbf48c06300e2 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sun, 10 Mar 2024 20:18:19 +0100 Subject: [PATCH 02/72] test: fix accurate multisig sigop count (BIP16), add unit test --- test/functional/test_framework/script.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 3275517888..7b19d31e17 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -483,7 +483,7 @@ def raw_iter(self): i = 0 while i < len(self): sop_idx = i - opcode = self[i] + opcode = CScriptOp(self[i]) i += 1 if opcode > OP_PUSHDATA4: @@ -590,7 +590,7 @@ def GetSigOpCount(self, fAccurate): n += 1 elif opcode in (OP_CHECKMULTISIG, OP_CHECKMULTISIGVERIFY): if fAccurate and (OP_1 <= lastOpcode <= OP_16): - n += opcode.decode_op_n() + n += lastOpcode.decode_op_n() else: n += 20 lastOpcode = opcode @@ -782,6 +782,20 @@ def test_cscriptnum_encoding(self): for value in values: self.assertEqual(CScriptNum.decode(CScriptNum.encode(CScriptNum(value))), value) + def test_legacy_sigopcount(self): + # test repeated single sig ops + for n_ops in range(1, 100, 10): + for singlesig_op in (OP_CHECKSIG, OP_CHECKSIGVERIFY): + singlesigs_script = CScript([singlesig_op]*n_ops) + self.assertEqual(singlesigs_script.GetSigOpCount(fAccurate=False), n_ops) + self.assertEqual(singlesigs_script.GetSigOpCount(fAccurate=True), n_ops) + # test multisig op (including accurate counting, i.e. BIP16) + for n in range(1, 16+1): + for multisig_op in (OP_CHECKMULTISIG, OP_CHECKMULTISIGVERIFY): + multisig_script = CScript([CScriptOp.encode_op_n(n), multisig_op]) + self.assertEqual(multisig_script.GetSigOpCount(fAccurate=False), 20) + self.assertEqual(multisig_script.GetSigOpCount(fAccurate=True), n) + def BIP341_sha_prevouts(txTo): return sha256(b"".join(i.prevout.serialize() for i in txTo.vin)) From c3e632b44153e314ef946f342c68c2758b1cbc4d Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 14 Mar 2024 23:27:22 +0000 Subject: [PATCH 03/72] Bugfix: bitcoin-cli: Check length of peer.transport_protocol_type --- src/bitcoin-cli.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index f0e27cb675..eb94f253c7 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -551,7 +551,7 @@ class NetinfoRequestHandler : public BaseRequestHandler peer.is_outbound ? "out" : "in", ConnectionTypeForNetinfo(peer.conn_type), peer.network, - peer.transport_protocol_type.starts_with('v') ? peer.transport_protocol_type[1] : ' ', + (peer.transport_protocol_type.size() == 2 && peer.transport_protocol_type[0] == 'v') ? peer.transport_protocol_type[1] : ' ', PingTimeToString(peer.min_ping), PingTimeToString(peer.ping), peer.last_send ? ToString(time_now - peer.last_send) : "", From 3bf4f8db669e1e274ce2633cf84add2938b9914b Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 21 Mar 2024 13:12:19 +0100 Subject: [PATCH 04/72] lint: scripted-diff verification also requires GNU grep --- test/lint/commit-script-check.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/lint/commit-script-check.sh b/test/lint/commit-script-check.sh index 55c9528dea..fe845ed19e 100755 --- a/test/lint/commit-script-check.sh +++ b/test/lint/commit-script-check.sh @@ -22,6 +22,11 @@ if ! sed --help 2>&1 | grep -q 'GNU'; then exit 1; fi +if ! grep --help 2>&1 | grep -q 'GNU'; then + echo "Error: the installed grep package is not compatible. Please make sure you have GNU grep installed in your system."; + exit 1; +fi + RET=0 PREV_BRANCH=$(git name-rev --name-only HEAD) PREV_HEAD=$(git rev-parse HEAD) From e30e8625bbc42045b8b757a8d7e80c20cc61cebf Mon Sep 17 00:00:00 2001 From: brunoerg Date: Wed, 20 Mar 2024 18:52:26 -0300 Subject: [PATCH 05/72] test: remove duplicated ban test Test the ban list is preserved through restart has been done by both `rpc_setban` and `p2p_disconnect_ban`. Since `p2p_disconnect_ban` does it in a more elegant way, we can keep only it and remove the duplicated one. --- test/functional/p2p_disconnect_ban.py | 8 +++++--- test/functional/rpc_setban.py | 10 ---------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index c389ff732f..678b006886 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -77,6 +77,7 @@ def run_test(self): self.nodes[1].setmocktime(old_time) self.nodes[1].setban("127.0.0.0/32", "add") self.nodes[1].setban("127.0.0.0/24", "add") + self.nodes[1].setban("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", "add") self.nodes[1].setban("192.168.0.1", "add", 1) # ban for 1 seconds self.nodes[1].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) # ban for 1000 seconds listBeforeShutdown = self.nodes[1].listbanned() @@ -85,13 +86,13 @@ def run_test(self): self.log.info("setban: test banning with absolute timestamp") self.nodes[1].setban("192.168.0.2", "add", old_time + 120, True) - # Move time forward by 3 seconds so the third ban has expired + # Move time forward by 3 seconds so the fourth ban has expired self.nodes[1].setmocktime(old_time + 3) - assert_equal(len(self.nodes[1].listbanned()), 4) + assert_equal(len(self.nodes[1].listbanned()), 5) self.log.info("Test ban_duration and time_remaining") for ban in self.nodes[1].listbanned(): - if ban["address"] in ["127.0.0.0/32", "127.0.0.0/24"]: + if ban["address"] in ["127.0.0.0/32", "127.0.0.0/24", "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"]: assert_equal(ban["ban_duration"], 86400) assert_equal(ban["time_remaining"], 86397) elif ban["address"] == "2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19": @@ -108,6 +109,7 @@ def run_test(self): assert_equal("127.0.0.0/32", listAfterShutdown[1]['address']) assert_equal("192.168.0.2/32", listAfterShutdown[2]['address']) assert_equal("/19" in listAfterShutdown[3]['address'], True) + assert_equal("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", listAfterShutdown[4]['address']) # Clear ban lists self.nodes[1].clearbanned() diff --git a/test/functional/rpc_setban.py b/test/functional/rpc_setban.py index bc426d7371..ba86b278bd 100755 --- a/test/functional/rpc_setban.py +++ b/test/functional/rpc_setban.py @@ -64,20 +64,10 @@ def run_test(self): assert self.is_banned(node, tor_addr) assert not self.is_banned(node, ip_addr) - self.log.info("Test the ban list is preserved through restart") - - self.restart_node(1) - assert self.is_banned(node, tor_addr) - assert not self.is_banned(node, ip_addr) - node.setban(tor_addr, "remove") assert not self.is_banned(self.nodes[1], tor_addr) assert not self.is_banned(node, ip_addr) - self.restart_node(1) - assert not self.is_banned(node, tor_addr) - assert not self.is_banned(node, ip_addr) - self.log.info("Test -bantime") self.restart_node(1, ["-bantime=1234"]) self.nodes[1].setban("127.0.0.1", "add") From c4f857cc301d856f3c60acbe6271d3fe19441c7a Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Tue, 26 Mar 2024 16:03:41 +0100 Subject: [PATCH 06/72] test: Extends wait_for_getheaders so a specific block hash can be checked Previously, `wait_for_getheaders` would check whether a node had received **any** getheaders message. This implied that, if a test needed to check for a specific block hash within a headers message, it had to make sure that it was checking the desired message. This normally involved having to manually clear `last_message`. This method, apart from being too verbose, was error prone, given an undesired `getheaders` would make tests pass. This adds the ability to check for a specific block_hash within the last `getheaders` message. --- test/functional/mining_basic.py | 2 +- test/functional/p2p_compactblocks.py | 2 +- test/functional/p2p_initial_headers_sync.py | 15 +++++---------- test/functional/p2p_segwit.py | 3 +-- test/functional/p2p_sendheaders.py | 21 +++++++++++---------- test/functional/test_framework/p2p.py | 16 +++++++++------- 6 files changed, 28 insertions(+), 31 deletions(-) diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index da796d3f70..5f2dde8eac 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -308,7 +308,7 @@ def chain_tip(b_hash, *, status='headers-only', branchlen=1): # Should ask for the block from a p2p node, if they announce the header as well: peer = node.add_p2p_connection(P2PDataStore()) - peer.wait_for_getheaders(timeout=5) # Drop the first getheaders + peer.wait_for_getheaders(timeout=5, block_hash=block.hashPrevBlock) peer.send_blocks_and_test(blocks=[block], node=node) # Must be active now: assert chain_tip(block.hash, status='active', branchlen=0) in node.getchaintips() diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index 0950579580..9e314db110 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -387,7 +387,7 @@ def test_compactblock_requests(self, test_node): if announce == "inv": test_node.send_message(msg_inv([CInv(MSG_BLOCK, block.sha256)])) - test_node.wait_until(lambda: "getheaders" in test_node.last_message, timeout=30) + test_node.wait_for_getheaders(timeout=30) test_node.send_header_for_blocks([block]) else: test_node.send_header_for_blocks([block]) diff --git a/test/functional/p2p_initial_headers_sync.py b/test/functional/p2p_initial_headers_sync.py index e67c384da7..bc6e0fb355 100755 --- a/test/functional/p2p_initial_headers_sync.py +++ b/test/functional/p2p_initial_headers_sync.py @@ -38,9 +38,10 @@ def announce_random_block(self, peers): def run_test(self): self.log.info("Adding a peer to node0") peer1 = self.nodes[0].add_p2p_connection(P2PInterface()) + best_block_hash = int(self.nodes[0].getbestblockhash(), 16) # Wait for peer1 to receive a getheaders - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) # An empty reply will clear the outstanding getheaders request, # allowing additional getheaders requests to be sent to this peer in # the future. @@ -60,17 +61,12 @@ def run_test(self): assert "getheaders" not in peer2.last_message assert "getheaders" not in peer3.last_message - with p2p_lock: - peer1.last_message.pop("getheaders", None) - self.log.info("Have all peers announce a new block") self.announce_random_block(all_peers) self.log.info("Check that peer1 receives a getheaders in response") - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) peer1.send_message(msg_headers()) # Send empty response, see above - with p2p_lock: - peer1.last_message.pop("getheaders", None) self.log.info("Check that exactly 1 of {peer2, peer3} received a getheaders in response") count = 0 @@ -80,7 +76,6 @@ def run_test(self): if "getheaders" in p.last_message: count += 1 peer_receiving_getheaders = p - p.last_message.pop("getheaders", None) p.send_message(msg_headers()) # Send empty response, see above assert_equal(count, 1) @@ -89,14 +84,14 @@ def run_test(self): self.announce_random_block(all_peers) self.log.info("Check that peer1 receives a getheaders in response") - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) self.log.info("Check that the remaining peer received a getheaders as well") expected_peer = peer2 if peer2 == peer_receiving_getheaders: expected_peer = peer3 - expected_peer.wait_for_getheaders() + expected_peer.wait_for_getheaders(block_hash=best_block_hash) self.log.info("Success!") diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index af47c6d9f0..213c748eda 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -191,14 +191,13 @@ def announce_tx_and_wait_for_getdata(self, tx, success=True, use_wtxid=False): def announce_block_and_wait_for_getdata(self, block, use_header, timeout=60): with p2p_lock: self.last_message.pop("getdata", None) - self.last_message.pop("getheaders", None) msg = msg_headers() msg.headers = [CBlockHeader(block)] if use_header: self.send_message(msg) else: self.send_message(msg_inv(inv=[CInv(MSG_BLOCK, block.sha256)])) - self.wait_for_getheaders(timeout=timeout) + self.wait_for_getheaders(block_hash=block.hashPrevBlock, timeout=timeout) self.send_message(msg) self.wait_for_getdata([block.sha256], timeout=timeout) diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index 508d6fe403..27a3aa8fb9 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -311,6 +311,7 @@ def test_nonnull_locators(self, test_node, inv_node): # Now that we've synced headers, headers announcements should work tip = self.mine_blocks(1) + expected_hash = tip inv_node.check_last_inv_announcement(inv=[tip]) test_node.check_last_headers_announcement(headers=[tip]) @@ -334,7 +335,10 @@ def test_nonnull_locators(self, test_node, inv_node): if j == 0: # Announce via inv test_node.send_block_inv(tip) - test_node.wait_for_getheaders() + if i == 0: + test_node.wait_for_getheaders(block_hash=expected_hash) + else: + assert "getheaders" not in test_node.last_message # Should have received a getheaders now test_node.send_header_for_blocks(blocks) # Test that duplicate inv's won't result in duplicate @@ -521,6 +525,7 @@ def test_nonnull_locators(self, test_node, inv_node): self.log.info("Part 5: Testing handling of unconnecting headers") # First we test that receipt of an unconnecting header doesn't prevent # chain sync. + expected_hash = tip for i in range(10): self.log.debug("Part 5.{}: starting...".format(i)) test_node.last_message.pop("getdata", None) @@ -533,15 +538,14 @@ def test_nonnull_locators(self, test_node, inv_node): block_time += 1 height += 1 # Send the header of the second block -> this won't connect. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[1]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) test_node.send_header_for_blocks(blocks) test_node.wait_for_getdata([x.sha256 for x in blocks]) [test_node.send_message(msg_block(x)) for x in blocks] test_node.sync_with_ping() assert_equal(int(self.nodes[0].getbestblockhash(), 16), blocks[1].sha256) + expected_hash = blocks[1].sha256 blocks = [] # Now we test that if we repeatedly don't send connecting headers, we @@ -556,13 +560,12 @@ def test_nonnull_locators(self, test_node, inv_node): for i in range(1, MAX_NUM_UNCONNECTING_HEADERS_MSGS): # Send a header that doesn't connect, check that we get a getheaders. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[i]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) # Next header will connect, should re-set our count: test_node.send_header_for_blocks([blocks[0]]) + expected_hash = blocks[0].sha256 # Remove the first two entries (blocks[1] would connect): blocks = blocks[2:] @@ -571,10 +574,8 @@ def test_nonnull_locators(self, test_node, inv_node): # before we get disconnected. Should be 5*MAX_NUM_UNCONNECTING_HEADERS_MSGS for i in range(5 * MAX_NUM_UNCONNECTING_HEADERS_MSGS - 1): # Send a header that doesn't connect, check that we get a getheaders. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[i % len(blocks)]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) # Eventually this stops working. test_node.send_header_for_blocks([blocks[-1]]) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index ce76008c46..00bd1e4017 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -644,15 +644,17 @@ def test_function(): self.wait_until(test_function, timeout=timeout) - def wait_for_getheaders(self, *, timeout=60): - """Waits for a getheaders message. + def wait_for_getheaders(self, block_hash=None, *, timeout=60): + """Waits for a getheaders message containing a specific block hash. - Receiving any getheaders message will satisfy the predicate. the last_message["getheaders"] - value must be explicitly cleared before calling this method, or this will return - immediately with success. TODO: change this method to take a hash value and only - return true if the correct block header has been requested.""" + If no block hash is provided, checks whether any getheaders message has been received by the node.""" def test_function(): - return self.last_message.get("getheaders") + last_getheaders = self.last_message.pop("getheaders", None) + if block_hash is None: + return last_getheaders + if last_getheaders is None: + return False + return block_hash == last_getheaders.locator.vHave[0] self.wait_until(test_function, timeout=timeout) From f81fad5e0f3be1f7aed59f9da00396c75c2a6406 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Mon, 1 Apr 2024 14:03:35 +0200 Subject: [PATCH 07/72] test: introduce and use `calculate_input_weight` helper Rather than manually estimating an input's weight by adding up all the involved components (fixed-size skeleton, compact-serialized lengths, and the actual scriptSig / witness stack items) we can simply take use of the serialization classes `CTxIn` / `CTxInWitness` instead, to achieve the same with significantly less code. The new helper is used in the functional tests rpc_psbt.py and wallet_send.py, where the previous manual estimation code was duplicated. --- test/functional/rpc_psbt.py | 17 ++++---------- test/functional/test_framework/wallet_util.py | 18 +++++++++++++++ test/functional/wallet_send.py | 23 ++++++------------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 016aa3ba11..897cb51b18 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -16,8 +16,6 @@ CTxIn, CTxOut, MAX_BIP125_RBF_SEQUENCE, - WITNESS_SCALE_FACTOR, - ser_compact_size, ) from test_framework.psbt import ( PSBT, @@ -42,6 +40,7 @@ find_vout_for_address, ) from test_framework.wallet_util import ( + calculate_input_weight, generate_keypair, get_generate_key, ) @@ -752,17 +751,9 @@ def test_psbt_input_keys(psbt_input, keys): input_idx = i break psbt_in = dec["inputs"][input_idx] - # Calculate the input weight - # (prevout + sequence + length of scriptSig + scriptsig) * WITNESS_SCALE_FACTOR + len of num scriptWitness stack items + (length of stack item + stack item) * N stack items - # Note that occasionally this weight estimate may be slightly larger or smaller than the real weight - # as sometimes ECDSA signatures are one byte shorter than expected with a probability of 1/128 - len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0 - len_scriptsig += len(ser_compact_size(len_scriptsig)) - len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(ser_compact_size(len(psbt_in["final_scriptwitness"])))) if "final_scriptwitness" in psbt_in else 0 - len_prevout_txid = 32 - len_prevout_index = 4 - len_sequence = 4 - input_weight = ((len_prevout_txid + len_prevout_index + len_sequence + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness + scriptsig_hex = psbt_in["final_scriptSig"]["hex"] if "final_scriptSig" in psbt_in else "" + witness_stack_hex = psbt_in["final_scriptwitness"] if "final_scriptwitness" in psbt_in else None + input_weight = calculate_input_weight(scriptsig_hex, witness_stack_hex) low_input_weight = input_weight // 2 high_input_weight = input_weight * 2 diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index 44811918bf..8104701ff3 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -15,6 +15,11 @@ script_to_p2wsh, ) from test_framework.key import ECKey +from test_framework.messages import ( + CTxIn, + CTxInWitness, + WITNESS_SCALE_FACTOR, +) from test_framework.script_util import ( key_to_p2pkh_script, key_to_p2wpkh_script, @@ -123,6 +128,19 @@ def generate_keypair(compressed=True, wif=False): privkey = bytes_to_wif(privkey.get_bytes(), compressed) return privkey, pubkey +def calculate_input_weight(scriptsig_hex, witness_stack_hex=None): + """Given a scriptSig and a list of witness stack items for an input in hex format, + calculate the total input weight. If the input has no witness data, + `witness_stack_hex` can be set to None.""" + tx_in = CTxIn(scriptSig=bytes.fromhex(scriptsig_hex)) + witness_size = 0 + if witness_stack_hex is not None: + tx_inwit = CTxInWitness() + for witness_item_hex in witness_stack_hex: + tx_inwit.scriptWitness.stack.append(bytes.fromhex(witness_item_hex)) + witness_size = len(tx_inwit.serialize()) + return len(tx_in.serialize()) * WITNESS_SCALE_FACTOR + witness_size + class WalletUnlock(): """ A context manager for unlocking a wallet with a passphrase and automatically locking it afterward. diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index e4ca341b49..0a0a8dba0d 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -9,10 +9,6 @@ from test_framework.authproxy import JSONRPCException from test_framework.descriptors import descsum_create -from test_framework.messages import ( - ser_compact_size, - WITNESS_SCALE_FACTOR, -) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -21,7 +17,10 @@ assert_raises_rpc_error, count_bytes, ) -from test_framework.wallet_util import generate_keypair +from test_framework.wallet_util import ( + calculate_input_weight, + generate_keypair, +) class WalletSendTest(BitcoinTestFramework): @@ -543,17 +542,9 @@ def run_test(self): input_idx = i break psbt_in = dec["inputs"][input_idx] - # Calculate the input weight - # (prevout + sequence + length of scriptSig + scriptsig) * WITNESS_SCALE_FACTOR + len of num scriptWitness stack items + (length of stack item + stack item) * N stack items - # Note that occasionally this weight estimate may be slightly larger or smaller than the real weight - # as sometimes ECDSA signatures are one byte shorter than expected with a probability of 1/128 - len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0 - len_scriptsig += len(ser_compact_size(len_scriptsig)) - len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(ser_compact_size(len(psbt_in["final_scriptwitness"])))) if "final_scriptwitness" in psbt_in else 0 - len_prevout_txid = 32 - len_prevout_index = 4 - len_sequence = 4 - input_weight = ((len_prevout_txid + len_prevout_index + len_sequence + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness + scriptsig_hex = psbt_in["final_scriptSig"]["hex"] if "final_scriptSig" in psbt_in else "" + witness_stack_hex = psbt_in["final_scriptwitness"] if "final_scriptwitness" in psbt_in else None + input_weight = calculate_input_weight(scriptsig_hex, witness_stack_hex) # Input weight error conditions assert_raises_rpc_error( From 6d91cb781c30966963f28e7577c7aa3829fa9390 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Mon, 1 Apr 2024 15:01:38 +0200 Subject: [PATCH 08/72] test: add unit tests for `calculate_input_weight` --- test/functional/test_framework/wallet_util.py | 40 +++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 41 insertions(+) diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index 8104701ff3..d30b00f4a7 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Useful util functions for testing the wallet""" from collections import namedtuple +import unittest from test_framework.address import ( byte_to_base58, @@ -159,3 +160,42 @@ def __enter__(self): def __exit__(self, *args): _ = args self.wallet.walletlock() + + +class TestFrameworkWalletUtil(unittest.TestCase): + def test_calculate_input_weight(self): + SKELETON_BYTES = 32 + 4 + 4 # prevout-txid, prevout-index, sequence + SMALL_LEN_BYTES = 1 # bytes needed for encoding scriptSig / witness item lenghts < 253 + LARGE_LEN_BYTES = 3 # bytes needed for encoding scriptSig / witness item lengths >= 253 + + # empty scriptSig, no witness + self.assertEqual(calculate_input_weight(""), + (SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + self.assertEqual(calculate_input_weight("", None), + (SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + # small scriptSig, no witness + scriptSig_small = "00"*252 + self.assertEqual(calculate_input_weight(scriptSig_small, None), + (SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR) + # small scriptSig, empty witness stack + self.assertEqual(calculate_input_weight(scriptSig_small, []), + (SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR + SMALL_LEN_BYTES) + # large scriptSig, no witness + scriptSig_large = "00"*253 + self.assertEqual(calculate_input_weight(scriptSig_large, None), + (SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR) + # large scriptSig, empty witness stack + self.assertEqual(calculate_input_weight(scriptSig_large, []), + (SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR + SMALL_LEN_BYTES) + # empty scriptSig, 5 small witness stack items + self.assertEqual(calculate_input_weight("", ["00", "11", "22", "33", "44"]), + ((SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 5 * SMALL_LEN_BYTES + 5) + # empty scriptSig, 253 small witness stack items + self.assertEqual(calculate_input_weight("", ["00"]*253), + ((SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + LARGE_LEN_BYTES + 253 * SMALL_LEN_BYTES + 253) + # small scriptSig, 3 large witness stack items + self.assertEqual(calculate_input_weight(scriptSig_small, ["00"*253]*3), + ((SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 3 * LARGE_LEN_BYTES + 3*253) + # large scriptSig, 3 large witness stack items + self.assertEqual(calculate_input_weight(scriptSig_large, ["00"*253]*3), + ((SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 3 * LARGE_LEN_BYTES + 3*253) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 3f6e47d410..32b55813a8 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -85,6 +85,7 @@ "crypto.ripemd160", "script", "segwit_addr", + "wallet_util", ] EXTENDED_SCRIPTS = [ From 72ba7b5d263b6d909ae59040536a499a596237c2 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 22 Mar 2024 17:29:22 +0000 Subject: [PATCH 09/72] depends: libnatpmp f2433bec24ca3d3f22a8a7840728a3ac177f94ba This includes once CMake related change I upstreamed: https://github.com/miniupnp/libnatpmp/pull/43. --- depends/packages/libnatpmp.mk | 4 ++-- doc/dependencies.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/depends/packages/libnatpmp.mk b/depends/packages/libnatpmp.mk index caa809ec0b..e222f29eee 100644 --- a/depends/packages/libnatpmp.mk +++ b/depends/packages/libnatpmp.mk @@ -1,8 +1,8 @@ package=libnatpmp -$(package)_version=07004b97cf691774efebe70404cf22201e4d330d +$(package)_version=f2433bec24ca3d3f22a8a7840728a3ac177f94ba $(package)_download_path=https://github.com/miniupnp/libnatpmp/archive $(package)_file_name=$($(package)_version).tar.gz -$(package)_sha256_hash=9321953ceb39d07c25463e266e50d0ae7b64676bb3a986d932b18881ed94f1fb +$(package)_sha256_hash=ef84979950dfb3556705b63c9cd6c95501b75e887fba466234b187f3c9029669 $(package)_patches=no_libtool.patch define $(package)_set_vars diff --git a/doc/dependencies.md b/doc/dependencies.md index 1e48d225b0..4003f22b91 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -35,7 +35,7 @@ You can find installation instructions in the `build-*.md` file for your platfor ### Networking | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | -| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [07004b9...](https://github.com/bitcoin/bitcoin/pull/25917) | | No | +| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [f2433be...](https://github.com/bitcoin/bitcoin/pull/29708) | | No | | [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/20421) | 2.1 | No | ### Notifications From 3c1ae3ee33d4d9dbea046d5ab8ee924a12982759 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 19 Apr 2023 16:34:39 -0400 Subject: [PATCH 10/72] depends: switch libnatpmp to CMake Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> --- depends/packages/libnatpmp.mk | 18 ++++++------------ depends/patches/libnatpmp/no_libtool.patch | 15 --------------- 2 files changed, 6 insertions(+), 27 deletions(-) delete mode 100644 depends/patches/libnatpmp/no_libtool.patch diff --git a/depends/packages/libnatpmp.mk b/depends/packages/libnatpmp.mk index e222f29eee..5a573a18e7 100644 --- a/depends/packages/libnatpmp.mk +++ b/depends/packages/libnatpmp.mk @@ -3,24 +3,18 @@ $(package)_version=f2433bec24ca3d3f22a8a7840728a3ac177f94ba $(package)_download_path=https://github.com/miniupnp/libnatpmp/archive $(package)_file_name=$($(package)_version).tar.gz $(package)_sha256_hash=ef84979950dfb3556705b63c9cd6c95501b75e887fba466234b187f3c9029669 -$(package)_patches=no_libtool.patch +$(package)_build_subdir=build -define $(package)_set_vars - $(package)_build_opts=CC="$($(package)_cc)" - $(package)_build_opts_mingw32=CPPFLAGS=-DNATPMP_STATICLIB - $(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)" -endef - -define $(package)_preprocess_cmds - patch -p1 < $($(package)_patch_dir)/no_libtool.patch +define $(package)_config_cmds + $($(package)_cmake) -S .. -B . endef define $(package)_build_cmds - $(MAKE) libnatpmp.a $($(package)_build_opts) + $(MAKE) natpmp endef define $(package)_stage_cmds - mkdir -p $($(package)_staging_prefix_dir)/include $($(package)_staging_prefix_dir)/lib &&\ - install *.h $($(package)_staging_prefix_dir)/include &&\ + mkdir -p $($(package)_staging_prefix_dir)/include $($(package)_staging_prefix_dir)/lib && \ + install ../natpmp.h ../natpmp_declspec.h $($(package)_staging_prefix_dir)/include && \ install libnatpmp.a $($(package)_staging_prefix_dir)/lib endef diff --git a/depends/patches/libnatpmp/no_libtool.patch b/depends/patches/libnatpmp/no_libtool.patch deleted file mode 100644 index 2b9f01f6eb..0000000000 --- a/depends/patches/libnatpmp/no_libtool.patch +++ /dev/null @@ -1,15 +0,0 @@ -diff -ruN libnatpmp-07004b97cf691774efebe70404cf22201e4d330d/Makefile libnatpmp-07004b97cf691774efebe70404cf22201e4d330d.new/Makefile ---- libnatpmp-07004b97cf691774efebe70404cf22201e4d330d/Makefile 2022-07-05 07:49:50.000000000 +0000 -+++ libnatpmp-07004b97cf691774efebe70404cf22201e4d330d.new/Makefile 2024-01-23 20:59:35.674821779 +0000 -@@ -197,11 +197,7 @@ - $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS) - - $(STATICLIB): $(LIBOBJS) --ifneq (, $(findstring darwin, $(OS))) -- $(LIBTOOL) -static -o $@ $? --else - $(AR) crs $@ $? --endif - - $(SHAREDLIB): $(LIBOBJS) - ifneq (, $(findstring darwin, $(OS))) From 08ff17d1420a3d1c14c6b1a5436678fbb1dd9cbc Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 Apr 2024 15:58:05 +0200 Subject: [PATCH 11/72] ci: disable _FORTIFY_SOURCE with MSAN --- ci/test/00_setup_env_native_fuzz_with_msan.sh | 3 ++- ci/test/00_setup_env_native_msan.sh | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh index 36a172e42c..f1c358082d 100755 --- a/ci/test/00_setup_env_native_fuzz_with_msan.sh +++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh @@ -17,7 +17,8 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'" +# _FORTIFY_SOURCE is not compatible with MSAN. +export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE -U_FORTIFY_SOURCE'" export USE_MEMORY_SANITIZER="true" export RUN_UNIT_TESTS="false" export RUN_FUNCTIONAL_TESTS="false" diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index 431e4aba49..dd465cac2e 100755 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -17,7 +17,8 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BITCOIN_CONFIG="--with-sanitizers=memory --disable-hardening" +# _FORTIFY_SOURCE is not compatible with MSAN. +export BITCOIN_CONFIG="--with-sanitizers=memory CPPFLAGS='-U_FORTIFY_SOURCE'" export USE_MEMORY_SANITIZER="true" export RUN_FUNCTIONAL_TESTS="false" export CCACHE_MAXSIZE=250M From f2e3662e57eca1330962faf38ff428a564d50a11 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Thu, 11 Apr 2024 14:38:32 +0200 Subject: [PATCH 12/72] net: Decrease nMaxIPs when learning from DNS seeds Limit number of IPs learned from a single DNS seed to 32, to prevent the results from one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is bounded to 33 already, but it is possible for it to use TCP where a potentially enormous number of results can be returned. Closes #16070. --- src/net.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index e388f05b03..3e959c187c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2256,7 +2256,11 @@ void CConnman::ThreadDNSAddressSeed() if (!resolveSource.SetInternal(host)) { continue; } - unsigned int nMaxIPs = 256; // Limits number of IPs learned from a DNS seed + // Limit number of IPs learned from a single DNS seed. This limit exists to prevent the results from + // one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is + // bounded to 33 already, but it is possible for it to use TCP where a larger number of results can be + // returned. + unsigned int nMaxIPs = 32; const auto addresses{LookupHost(host, nMaxIPs, true)}; if (!addresses.empty()) { for (const CNetAddr& ip : addresses) { From fa6ab0d020d0b1492203f7eb2ccb8051812de086 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 15 Apr 2024 09:57:28 +0200 Subject: [PATCH 13/72] rpc: Reword SighashFromStr error message --- src/core_read.cpp | 2 +- test/functional/rpc_psbt.py | 4 ++-- test/functional/rpc_signrawtransactionwithkey.py | 2 +- test/functional/wallet_signrawtransactionwithwallet.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core_read.cpp b/src/core_read.cpp index e32e46d1b9..5956d9df5f 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -256,6 +256,6 @@ util::Result SighashFromStr(const std::string& sighash) if (it != map_sighash_values.end()) { return it->second; } else { - return util::Error{Untranslated(sighash + " is not a valid sighash parameter.")}; + return util::Error{Untranslated("'" + sighash + "' is not a valid sighash parameter.")}; } } diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 016aa3ba11..117a65121d 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -886,7 +886,7 @@ def test_psbt_input_keys(psbt_input, keys): assert_equal(comb_psbt, psbt) self.log.info("Test walletprocesspsbt raises if an invalid sighashtype is passed") - assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].walletprocesspsbt, psbt, sighashtype="all") + assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].walletprocesspsbt, psbt, sighashtype="all") self.log.info("Test decoding PSBT with per-input preimage types") # note that the decodepsbt RPC doesn't check whether preimages and hashes match @@ -992,7 +992,7 @@ def test_psbt_input_keys(psbt_input, keys): self.nodes[2].sendrawtransaction(processed_psbt['hex']) self.log.info("Test descriptorprocesspsbt raises if an invalid sighashtype is passed") - assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt, [descriptor], sighashtype="all") + assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt, [descriptor], sighashtype="all") if __name__ == '__main__': diff --git a/test/functional/rpc_signrawtransactionwithkey.py b/test/functional/rpc_signrawtransactionwithkey.py index 0913f5057e..268584331e 100755 --- a/test/functional/rpc_signrawtransactionwithkey.py +++ b/test/functional/rpc_signrawtransactionwithkey.py @@ -124,7 +124,7 @@ def invalid_sighashtype_test(self): self.log.info("Test signing transaction with invalid sighashtype") tx = self.nodes[0].createrawtransaction(INPUTS, OUTPUTS) privkeys = [self.nodes[0].get_deterministic_priv_key().key] - assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithkey, tx, privkeys, sighashtype="all") + assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithkey, tx, privkeys, sighashtype="all") def run_test(self): self.successful_signing_test() diff --git a/test/functional/wallet_signrawtransactionwithwallet.py b/test/functional/wallet_signrawtransactionwithwallet.py index b0517f951d..612a2542e7 100755 --- a/test/functional/wallet_signrawtransactionwithwallet.py +++ b/test/functional/wallet_signrawtransactionwithwallet.py @@ -55,7 +55,7 @@ def test_with_lock_outputs(self): def test_with_invalid_sighashtype(self): self.log.info("Test signrawtransactionwithwallet raises if an invalid sighashtype is passed") - assert_raises_rpc_error(-8, "all is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithwallet, hexstring=RAW_TX, sighashtype="all") + assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[0].signrawtransactionwithwallet, hexstring=RAW_TX, sighashtype="all") def script_verification_error_test(self): """Create and sign a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script. From 633e45b2e2728efcb0637afa94fcbd5756dfbe76 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 12 Apr 2024 01:42:57 +0200 Subject: [PATCH 14/72] remove unneeded shell option from cpp-subprocess We don't use this option and it's not supported on Windows anyways, so we can get as well rid of it. --- src/util/subprocess.hpp | 43 ----------------------------------------- 1 file changed, 43 deletions(-) diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp index 0fcc9397ea..96444ad1e7 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.hpp @@ -431,26 +431,6 @@ namespace util } - /*! - * Function: join - * Parameters: - * [in] vec : Vector of strings which needs to be joined to form - * a single string with words separated by a separator char. - * [in] sep : String used to separate 2 words in the joined string. - * Default constructed to ' ' (space). - * [out] string: Joined string. - */ - static inline - std::string join(const std::vector& vec, - const std::string& sep = " ") - { - std::string res; - for (auto& elem : vec) res.append(elem + sep); - res.erase(--res.end()); - return res; - } - - #ifndef __USING_WINDOWS__ /*! * Function: set_clo_on_exec @@ -696,11 +676,6 @@ struct session_leader { bool leader_ = false; }; -struct shell { - explicit shell(bool s): shell_(s) {} - bool shell_ = false; -}; - /*! * Base class for all arguments involving string value. */ @@ -1011,7 +986,6 @@ struct ArgumentDeducer void set_option(bufsize&& bsiz); void set_option(environment&& env); void set_option(defer_spawn&& defer); - void set_option(shell&& sh); void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); @@ -1350,7 +1324,6 @@ class Popen bool defer_process_start_ = false; bool close_fds_ = false; bool has_preexec_fn_ = false; - bool shell_ = false; bool session_leader_ = false; std::string exe_name_; @@ -1492,10 +1465,6 @@ inline void Popen::kill(int sig_num) inline void Popen::execute_process() noexcept(false) { #ifdef __USING_WINDOWS__ - if (this->shell_) { - throw OSError("shell not currently supported on windows", 0); - } - void* environment_string_table_ptr = nullptr; env_vector_t environment_string_vector; if(this->env_.size()){ @@ -1588,14 +1557,6 @@ inline void Popen::execute_process() noexcept(false) int err_rd_pipe, err_wr_pipe; std::tie(err_rd_pipe, err_wr_pipe) = util::pipe_cloexec(); - if (shell_) { - auto new_cmd = util::join(vargs_); - vargs_.clear(); - vargs_.insert(vargs_.begin(), {"/bin/sh", "-c"}); - vargs_.push_back(new_cmd); - populate_c_argv(); - } - if (exe_name_.length()) { vargs_.insert(vargs_.begin(), exe_name_); populate_c_argv(); @@ -1678,10 +1639,6 @@ namespace detail { popen_->defer_process_start_ = defer.defer; } - inline void ArgumentDeducer::set_option(shell&& sh) { - popen_->shell_ = sh.shell_; - } - inline void ArgumentDeducer::set_option(session_leader&& sleader) { popen_->session_leader_ = sleader.leader_; } From cececad7b29e2ca3de1216db1c541dba6dc81bfa Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Fri, 12 Apr 2024 01:48:08 +0200 Subject: [PATCH 15/72] remove unneeded preexec function option from cpp-subprocess We don't seem to ever need this, so remove it. --- src/util/subprocess.hpp | 50 ----------------------------------------- 1 file changed, 50 deletions(-) diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp index 96444ad1e7..1a98be3e1d 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.hpp @@ -845,44 +845,6 @@ struct error int wr_ch_ = -1; }; -// Impoverished, meager, needy, truly needy -// version of type erasure to store function pointers -// needed to provide the functionality of preexec_func -// ATTN: Can be used only to execute functions with no -// arguments and returning void. -// Could have used more efficient methods, ofcourse, but -// that won't yield me the consistent syntax which I am -// aiming for. If you know, then please do let me know. - -class preexec_func -{ -public: - preexec_func() {} - - template - explicit preexec_func(Func f): holder_(new FuncHolder(std::move(f))) - {} - - void operator()() { - (*holder_)(); - } - -private: - struct HolderBase { - virtual void operator()() const = 0; - virtual ~HolderBase(){}; - }; - template - struct FuncHolder: HolderBase { - FuncHolder(T func): func_(std::move(func)) {} - void operator()() const override { func_(); } - // The function pointer/reference - T func_; - }; - - std::unique_ptr holder_ = nullptr; -}; - // ~~~~ End Popen Args ~~~~ @@ -990,7 +952,6 @@ struct ArgumentDeducer void set_option(output&& out); void set_option(error&& err); void set_option(close_fds&& cfds); - void set_option(preexec_func&& prefunc); void set_option(session_leader&& sleader); private: @@ -1323,13 +1284,11 @@ class Popen bool defer_process_start_ = false; bool close_fds_ = false; - bool has_preexec_fn_ = false; bool session_leader_ = false; std::string exe_name_; std::string cwd_; env_map_t env_; - preexec_func preexec_fn_; // Command in string format std::string args_; @@ -1669,11 +1628,6 @@ namespace detail { popen_->close_fds_ = cfds.close_all; } - inline void ArgumentDeducer::set_option(preexec_func&& prefunc) { - popen_->preexec_fn_ = std::move(prefunc); - popen_->has_preexec_fn_ = true; - } - inline void Child::execute_child() { #ifndef __USING_WINDOWS__ @@ -1737,10 +1691,6 @@ namespace detail { if (sys_ret == -1) throw OSError("chdir failed", errno); } - if (parent_->has_preexec_fn_) { - parent_->preexec_fn_(); - } - if (parent_->session_leader_) { sys_ret = setsid(); if (sys_ret == -1) throw OSError("setsid failed", errno); From 80d008c66d00d3496cd8549daee6775cf2c6b782 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 13 Apr 2024 11:43:02 +0200 Subject: [PATCH 16/72] remove unneeded defer_spawn option from cpp-subprocess For our use-case, there doesn't seem to be any need for this. --- src/util/subprocess.hpp | 40 +++------------------------------------- 1 file changed, 3 insertions(+), 37 deletions(-) diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp index 1a98be3e1d..dd0789b72c 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.hpp @@ -641,16 +641,6 @@ struct bufsize { int bufsiz = 0; }; -/*! - * Option to defer spawning of the child process - * till `Popen::start_process` API is called. - * Default value is false. - */ -struct defer_spawn { - explicit defer_spawn(bool d): defer(d) {} - bool defer = false; -}; - /*! * Option to close all file descriptors * when the child process is spawned. @@ -947,7 +937,6 @@ struct ArgumentDeducer void set_option(cwd&& cwdir); void set_option(bufsize&& bsiz); void set_option(environment&& env); - void set_option(defer_spawn&& defer); void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); @@ -1153,8 +1142,6 @@ class Streams in case of redirection. See piping examples. *12. error() - Get the error channel/File pointer. Usually used in case of redirection. - *13. start_process() - Start the child process. Only to be used when - * `defer_spawn` option was provided in Popen constructor. */ class Popen { @@ -1172,7 +1159,7 @@ class Popen // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } template @@ -1184,7 +1171,7 @@ class Popen // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } template @@ -1195,7 +1182,7 @@ class Popen // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } /* @@ -1207,8 +1194,6 @@ class Popen } */ - void start_process() noexcept(false); - int pid() const noexcept { return child_pid_; } int retcode() const noexcept { return retcode_; } @@ -1282,7 +1267,6 @@ class Popen std::future cleanup_future_; #endif - bool defer_process_start_ = false; bool close_fds_ = false; bool session_leader_ = false; @@ -1323,20 +1307,6 @@ inline void Popen::populate_c_argv() cargv_.push_back(nullptr); } -inline void Popen::start_process() noexcept(false) -{ - // The process was started/tried to be started - // in the constructor itself. - // For explicitly calling this API to start the - // process, 'defer_spawn' argument must be set to - // true in the constructor. - if (!defer_process_start_) { - assert (0); - return; - } - execute_process(); -} - inline int Popen::wait() noexcept(false) { #ifdef __USING_WINDOWS__ @@ -1594,10 +1564,6 @@ namespace detail { popen_->env_ = std::move(env.env_); } - inline void ArgumentDeducer::set_option(defer_spawn&& defer) { - popen_->defer_process_start_ = defer.defer; - } - inline void ArgumentDeducer::set_option(session_leader&& sleader) { popen_->session_leader_ = sleader.leader_; } From 62db8f8e5a6cfe19d905afc91731d6bc8a665f61 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 16 Apr 2024 12:53:07 +0200 Subject: [PATCH 17/72] remove unneeded session_leader option from cpp-subprocess --- src/util/subprocess.hpp | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp index dd0789b72c..56f17c771f 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.hpp @@ -655,17 +655,6 @@ struct close_fds { bool close_all = false; }; -/*! - * Option to make the child process as the - * session leader and thus the process - * group leader. - * Default value is false. - */ -struct session_leader { - explicit session_leader(bool sl): leader_(sl) {} - bool leader_ = false; -}; - /*! * Base class for all arguments involving string value. */ @@ -941,7 +930,6 @@ struct ArgumentDeducer void set_option(output&& out); void set_option(error&& err); void set_option(close_fds&& cfds); - void set_option(session_leader&& sleader); private: Popen* popen_ = nullptr; @@ -1268,7 +1256,6 @@ class Popen #endif bool close_fds_ = false; - bool session_leader_ = false; std::string exe_name_; std::string cwd_; @@ -1385,8 +1372,7 @@ inline void Popen::kill(int sig_num) throw OSError("TerminateProcess", 0); } #else - if (session_leader_) killpg(child_pid_, sig_num); - else ::kill(child_pid_, sig_num); + ::kill(child_pid_, sig_num); #endif } @@ -1564,10 +1550,6 @@ namespace detail { popen_->env_ = std::move(env.env_); } - inline void ArgumentDeducer::set_option(session_leader&& sleader) { - popen_->session_leader_ = sleader.leader_; - } - inline void ArgumentDeducer::set_option(input&& inp) { if (inp.rd_ch_ != -1) popen_->stream_.read_from_parent_ = inp.rd_ch_; if (inp.wr_ch_ != -1) popen_->stream_.write_to_child_ = inp.wr_ch_; @@ -1657,11 +1639,6 @@ namespace detail { if (sys_ret == -1) throw OSError("chdir failed", errno); } - if (parent_->session_leader_) { - sys_ret = setsid(); - if (sys_ret == -1) throw OSError("setsid failed", errno); - } - // Replace the current image with the executable if (parent_->env_.size()) { for (auto& kv : parent_->env_) { From 79c30363733503a1fb7d4c98aa0d56ced0be6e32 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 16 Apr 2024 12:55:55 +0200 Subject: [PATCH 18/72] remove unneeded close_fds option from cpp-subprocess --- src/util/subprocess.hpp | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp index 56f17c771f..ea555c58a4 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.hpp @@ -641,20 +641,6 @@ struct bufsize { int bufsiz = 0; }; -/*! - * Option to close all file descriptors - * when the child process is spawned. - * The close fd list does not include - * input/output/error if they are explicitly - * set as part of the Popen arguments. - * - * Default value is false. - */ -struct close_fds { - explicit close_fds(bool c): close_all(c) {} - bool close_all = false; -}; - /*! * Base class for all arguments involving string value. */ @@ -929,7 +915,6 @@ struct ArgumentDeducer void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); - void set_option(close_fds&& cfds); private: Popen* popen_ = nullptr; @@ -1255,8 +1240,6 @@ class Popen std::future cleanup_future_; #endif - bool close_fds_ = false; - std::string exe_name_; std::string cwd_; env_map_t env_; @@ -1572,10 +1555,6 @@ namespace detail { if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_; } - inline void ArgumentDeducer::set_option(close_fds&& cfds) { - popen_->close_fds_ = cfds.close_all; - } - inline void Child::execute_child() { #ifndef __USING_WINDOWS__ @@ -1622,17 +1601,6 @@ namespace detail { if (stream.err_write_ != -1 && stream.err_write_ > 2) close(stream.err_write_); - // Close all the inherited fd's except the error write pipe - if (parent_->close_fds_) { - int max_fd = sysconf(_SC_OPEN_MAX); - if (max_fd == -1) throw OSError("sysconf failed", errno); - - for (int i = 3; i < max_fd; i++) { - if (i == err_wr_pipe_) continue; - close(i); - } - } - // Change the working directory if provided if (parent_->cwd_.length()) { sys_ret = chdir(parent_->cwd_.c_str()); From 03ffb09c31aa04cc296c0ce10d07109e22a8dd75 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 16 Apr 2024 14:11:21 +0200 Subject: [PATCH 19/72] remove unneeded bufsize option from cpp-subprocess --- src/util/subprocess.hpp | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp index ea555c58a4..f54223855a 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.hpp @@ -631,16 +631,6 @@ namespace util * ------------------------------- */ -/*! - * The buffer size of the stdin/stdout/stderr - * streams of the child process. - * Default value is 0. - */ -struct bufsize { - explicit bufsize(int sz): bufsiz(sz) {} - int bufsiz = 0; -}; - /*! * Base class for all arguments involving string value. */ @@ -910,7 +900,6 @@ struct ArgumentDeducer void set_option(executable&& exe); void set_option(cwd&& cwdir); - void set_option(bufsize&& bsiz); void set_option(environment&& env); void set_option(input&& inp); void set_option(output&& out); @@ -1065,9 +1054,6 @@ class Streams HANDLE g_hChildStd_ERR_Wr = nullptr; #endif - // Buffer size for the IO streams - int bufsiz_ = 0; - // Pipes for communicating with child // Emulates stdin @@ -1525,10 +1511,6 @@ namespace detail { popen_->cwd_ = std::move(cwdir.arg_value); } - inline void ArgumentDeducer::set_option(bufsize&& bsiz) { - popen_->stream_.bufsiz_ = bsiz.bufsiz; - } - inline void ArgumentDeducer::set_option(environment&& env) { popen_->env_ = std::move(env.env_); } @@ -1658,16 +1640,7 @@ namespace detail { for (auto& h : handles) { if (h == nullptr) continue; - switch (bufsiz_) { - case 0: - setvbuf(h, nullptr, _IONBF, BUFSIZ); - break; - case 1: - setvbuf(h, nullptr, _IONBF, BUFSIZ); - break; - default: - setvbuf(h, nullptr, _IOFBF, bufsiz_); - }; + setvbuf(h, nullptr, _IONBF, BUFSIZ); } #endif } From 2088777ba0f9ad3f6d4ab8b0b6ff8aad71117307 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 16 Apr 2024 14:13:08 +0200 Subject: [PATCH 20/72] remove unneeded cwd option from cpp-subprocess --- src/util/subprocess.hpp | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp index f54223855a..3591c65cc7 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.hpp @@ -656,18 +656,6 @@ struct executable: string_arg executable(T&& arg): string_arg(std::forward(arg)) {} }; -/*! - * Option to set the current working directory - * of the spawned process. - * - * Eg: cwd{"/some/path/x"} - */ -struct cwd: string_arg -{ - template - cwd(T&& arg): string_arg(std::forward(arg)) {} -}; - /*! * Option to specify environment variables required by * the spawned process. @@ -899,7 +887,6 @@ struct ArgumentDeducer ArgumentDeducer(Popen* p): popen_(p) {} void set_option(executable&& exe); - void set_option(cwd&& cwdir); void set_option(environment&& env); void set_option(input&& inp); void set_option(output&& out); @@ -1083,9 +1070,9 @@ class Streams * interface to the client. * * API's provided by the class: - * 1. Popen({"cmd"}, output{..}, error{..}, cwd{..}, ....) + * 1. Popen({"cmd"}, output{..}, error{..}, ....) * Command provided as a sequence. - * 2. Popen("cmd arg1"m output{..}, error{..}, cwd{..}, ....) + * 2. Popen("cmd arg1"m output{..}, error{..}, ....) * Command provided in a single string. * 3. wait() - Wait for the child to exit. * 4. retcode() - The return code of the exited child. @@ -1227,7 +1214,6 @@ class Popen #endif std::string exe_name_; - std::string cwd_; env_map_t env_; // Command in string format @@ -1507,10 +1493,6 @@ namespace detail { popen_->exe_name_ = std::move(exe.arg_value); } - inline void ArgumentDeducer::set_option(cwd&& cwdir) { - popen_->cwd_ = std::move(cwdir.arg_value); - } - inline void ArgumentDeducer::set_option(environment&& env) { popen_->env_ = std::move(env.env_); } @@ -1583,12 +1565,6 @@ namespace detail { if (stream.err_write_ != -1 && stream.err_write_ > 2) close(stream.err_write_); - // Change the working directory if provided - if (parent_->cwd_.length()) { - sys_ret = chdir(parent_->cwd_.c_str()); - if (sys_ret == -1) throw OSError("chdir failed", errno); - } - // Replace the current image with the executable if (parent_->env_.size()) { for (auto& kv : parent_->env_) { From fb4cc5f423ce587c1e97377e8afdf92fb4850f59 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Mon, 15 Apr 2024 14:14:49 -0400 Subject: [PATCH 21/72] netbase: clean up Proxy logging --- src/netbase.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/netbase.cpp b/src/netbase.cpp index 3ca1a5227a..f0fa298378 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -632,10 +632,7 @@ std::unique_ptr ConnectDirectly(const CService& dest, bool manual_connecti std::unique_ptr Proxy::Connect() const { - if (!IsValid()) { - LogPrintf("Cannot connect to invalid Proxy\n"); - return {}; - } + if (!IsValid()) return {}; if (!m_is_unix_socket) return ConnectDirectly(proxy, /*manual_connection=*/true); @@ -656,7 +653,6 @@ std::unique_ptr Proxy::Connect() const socklen_t len = sizeof(addrun); if(!ConnectToSocket(*sock, (struct sockaddr*)&addrun, len, path, /*manual_connection=*/true)) { - LogPrintf("Cannot connect to socket for %s\n", path); return {}; } From 016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Mon, 15 Apr 2024 16:27:21 +0200 Subject: [PATCH 22/72] fuzz: explicitly cap the vsize of RBFs for diagram checks --- src/test/fuzz/rbf.cpp | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 754aff4e70..30ed148c37 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -107,6 +107,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) std::vector mempool_txs; size_t iter{0}; + int64_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange(1, 1000000); + + // Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow + // Add replacement_vsize since this is added to new diagram during RBF check + int64_t running_vsize_total{replacement_vsize}; + LOCK2(cs_main, pool.cs); LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), NUM_ITERS) @@ -114,19 +120,33 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) // Make sure txns only have one input, and that a unique input is given to avoid circular references std::optional parent = ConsumeDeserializable(fuzzed_data_provider, TX_WITH_WITNESS); if (!parent) { - continue; + return; } assert(iter <= g_outpoints.size()); parent->vin.resize(1); parent->vin[0].prevout = g_outpoints[iter++]; mempool_txs.emplace_back(*parent); - pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back())); + const auto parent_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()); + running_vsize_total += parent_entry.GetTxSize(); + if (running_vsize_total > std::numeric_limits::max()) { + // We aren't adding this final tx to mempool, so we don't want to conflict with it + mempool_txs.pop_back(); + break; + } + pool.addUnchecked(parent_entry); if (fuzzed_data_provider.ConsumeBool() && !child->vin.empty()) { child->vin[0].prevout = COutPoint{mempool_txs.back().GetHash(), 0}; } mempool_txs.emplace_back(*child); - pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back())); + const auto child_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()); + running_vsize_total += child_entry.GetTxSize(); + if (running_vsize_total > std::numeric_limits::max()) { + // We aren't adding this final tx to mempool, so we don't want to conflict with it + mempool_txs.pop_back(); + break; + } + pool.addUnchecked(child_entry); if (fuzzed_data_provider.ConsumeBool()) { pool.PrioritiseTransaction(mempool_txs.back().GetHash().ToUint256(), fuzzed_data_provider.ConsumeIntegralInRange(-100000, 100000)); @@ -149,7 +169,6 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) // Calculate the feerate diagrams for a replacement. CAmount replacement_fees = ConsumeMoney(fuzzed_data_provider); - int64_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange(1, 1000000); auto calc_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; if (calc_results.has_value()) { From 6c1a2cc09a00baa6ff3ff34455c2243b43067fb5 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 19 May 2023 14:57:22 +0200 Subject: [PATCH 23/72] test: use h marker for external signer mock Consistent with #26076 --- test/functional/mocks/signer.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py index 5f4fad6380..64572ddda5 100755 --- a/test/functional/mocks/signer.py +++ b/test/functional/mocks/signer.py @@ -25,16 +25,16 @@ def getdescriptors(args): sys.stdout.write(json.dumps({ "receive": [ - "pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/0/*)#vt6w3l3j", - "sh(wpkh([00000001/49'/1'/" + args.account + "']" + xpub + "/0/*))#r0grqw5x", - "wpkh([00000001/84'/1'/" + args.account + "']" + xpub + "/0/*)#x30uthjs", - "tr([00000001/86'/1'/" + args.account + "']" + xpub + "/0/*)#sng9rd4t" + "pkh([00000001/44h/1h/" + args.account + "']" + xpub + "/0/*)#aqllu46s", + "sh(wpkh([00000001/49h/1h/" + args.account + "']" + xpub + "/0/*))#5dh56mgg", + "wpkh([00000001/84h/1h/" + args.account + "']" + xpub + "/0/*)#h62dxaej", + "tr([00000001/86h/1h/" + args.account + "']" + xpub + "/0/*)#pcd5w87f" ], "internal": [ - "pkh([00000001/44'/1'/" + args.account + "']" + xpub + "/1/*)#all0v2p2", - "sh(wpkh([00000001/49'/1'/" + args.account + "']" + xpub + "/1/*))#kwx4c3pe", - "wpkh([00000001/84'/1'/" + args.account + "']" + xpub + "/1/*)#h92akzzg", - "tr([00000001/86'/1'/" + args.account + "']" + xpub + "/1/*)#p8dy7c9n" + "pkh([00000001/44h/1h/" + args.account + "']" + xpub + "/1/*)#v567pq2g", + "sh(wpkh([00000001/49h/1h/" + args.account + "']" + xpub + "/1/*))#pvezzyah", + "wpkh([00000001/84h/1h/" + args.account + "']" + xpub + "/1/*)#xw0vmgf2", + "tr([00000001/86h/1h/" + args.account + "']" + xpub + "/1/*)#svg4njw3" ] })) @@ -47,8 +47,8 @@ def displayaddress(args): return sys.stdout.write(json.dumps({"error": "Unexpected fingerprint", "fingerprint": args.fingerprint})) expected_desc = [ - "wpkh([00000001/84'/1'/0'/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#0yneg42r", - "tr([00000001/86'/1'/0'/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#4vdj9jqk", + "wpkh([00000001/84h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#3te6hhy7", + "tr([00000001/86h/1h/0h/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#puqqa90m", ] if args.desc not in expected_desc: return sys.stdout.write(json.dumps({"error": "Unexpected descriptor", "desc": args.desc})) From dc55531087478d01fbde4f5fbb75375b672960c3 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 23 Nov 2021 16:12:55 +0100 Subject: [PATCH 24/72] wallet: compare address returned by displayaddress Update external signer documentation to reflect this requirement, which HWI already implements. --- doc/external-signer.md | 3 +++ src/wallet/external_signer_scriptpubkeyman.cpp | 13 +++++++++---- src/wallet/external_signer_scriptpubkeyman.h | 2 +- src/wallet/wallet.cpp | 2 +- src/wallet/wallet.h | 5 ++++- test/functional/mocks/signer.py | 14 +++++++------- test/functional/wallet_signer.py | 5 +++-- 7 files changed, 28 insertions(+), 16 deletions(-) diff --git a/doc/external-signer.md b/doc/external-signer.md index de44cdd880..1468e852ef 100644 --- a/doc/external-signer.md +++ b/doc/external-signer.md @@ -150,6 +150,9 @@ Example, display the first native SegWit receive address on Testnet: The command MUST be able to figure out the address type from the descriptor. +The command MUST return an object containing `{"address": "[the address]"}`. +As a sanity check, for devices that support this, it SHOULD ask the device to derive the address. + If contains a master key fingerprint, the command MUST fail if it does not match the fingerprint known by the device. If contains an xpub, the command MUST fail if it does not match the xpub known by the device. diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index a71f8f9fbc..ce668539e6 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -51,15 +52,19 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() { return signers[0]; } -bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CScript scriptPubKey, const ExternalSigner &signer) const +bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const { // TODO: avoid the need to infer a descriptor from inside a descriptor wallet + const CScript& scriptPubKey = GetScriptForDestination(dest); auto provider = GetSolvingProvider(scriptPubKey); auto descriptor = InferDescriptor(scriptPubKey, *provider); - signer.DisplayAddress(descriptor->ToString()); - // TODO inspect result - return true; + const UniValue& result = signer.DisplayAddress(descriptor->ToString()); + + const UniValue& ret_address = result.find_value("address"); + if (!ret_address.isStr()) return false; + + return ret_address.getValStr() == EncodeDestination(dest); } // If sign is true, transaction must previously have been filled diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index c052ce6129..b249833271 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -27,7 +27,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan static ExternalSigner GetExternalSigner(); - bool DisplayAddress(const CScript scriptPubKey, const ExternalSigner &signer) const; + bool DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const; TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override; }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 96c4397504..675d8f3985 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2676,7 +2676,7 @@ bool CWallet::DisplayAddress(const CTxDestination& dest) continue; } ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); - return signer_spk_man->DisplayAddress(scriptPubKey, signer); + return signer_spk_man->DisplayAddress(dest, signer); } return false; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b49b5a7d0d..74c0c5ed08 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -537,7 +537,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool IsSpentKey(const CScript& scriptPubKey) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - /** Display address on an external signer. Returns false if external signer support is not compiled */ + /** Display address on an external signer. + * Returns false if the signer does not respond with a matching address. + * Returns false if external signer support is not compiled. + */ bool DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsLockedCoin(const COutPoint& output) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py index 64572ddda5..dc3701fae2 100755 --- a/test/functional/mocks/signer.py +++ b/test/functional/mocks/signer.py @@ -41,19 +41,19 @@ def getdescriptors(args): def displayaddress(args): - # Several descriptor formats are acceptable, so allowing for potential - # changes to InferDescriptor: if args.fingerprint != "00000001": return sys.stdout.write(json.dumps({"error": "Unexpected fingerprint", "fingerprint": args.fingerprint})) - expected_desc = [ - "wpkh([00000001/84h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#3te6hhy7", - "tr([00000001/86h/1h/0h/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#puqqa90m", - ] + expected_desc = { + "wpkh([00000001/84h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#3te6hhy7": "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g", + "sh(wpkh([00000001/49h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7))#kz9y5w82": "2N2gQKzjUe47gM8p1JZxaAkTcoHPXV6YyVp", + "pkh([00000001/44h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#q3pqd8wh": "n1LKejAadN6hg2FrBXoU1KrwX4uK16mco9", + "tr([00000001/86h/1h/0h/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#puqqa90m": "tb1phw4cgpt6cd30kz9k4wkpwm872cdvhss29jga2xpmftelhqll62mscq0k4g", + } if args.desc not in expected_desc: return sys.stdout.write(json.dumps({"error": "Unexpected descriptor", "desc": args.desc})) - return sys.stdout.write(json.dumps({"address": "bcrt1qm90ugl4d48jv8n6e5t9ln6t9zlpm5th68x4f8g"})) + return sys.stdout.write(json.dumps({"address": expected_desc[args.desc]})) def signtx(args): if args.fingerprint != "00000001": diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 32a1887153..425f535b51 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -130,8 +130,9 @@ def test_valid_signer(self): assert_equal(address_info['hdkeypath'], "m/86h/1h/0h/0/0") self.log.info('Test walletdisplayaddress') - result = hww.walletdisplayaddress(address1) - assert_equal(result, {"address": address1}) + for address in [address1, address2, address3]: + result = hww.walletdisplayaddress(address) + assert_equal(result, {"address": address}) # Handle error thrown by script self.set_mock_result(self.nodes[1], "2") From 4357158c4712d479522d5cd441ad4dd1693fdd05 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 13 Feb 2024 13:25:59 +0100 Subject: [PATCH 25/72] wallet: return and display signer error Both RPC and GUI now render a useful error message instead of (silently) failing. Replace bool with util::Result to clarify that this either succeeds or returns an error message. --- src/interfaces/wallet.h | 2 +- src/qt/walletmodel.cpp | 9 +++++---- src/qt/walletmodel.h | 2 +- src/wallet/external_signer_scriptpubkeyman.cpp | 14 +++++++++++--- src/wallet/external_signer_scriptpubkeyman.h | 8 +++++++- src/wallet/interfaces.cpp | 2 +- src/wallet/rpc/addresses.cpp | 5 ++--- src/wallet/scriptpubkeyman.h | 1 - src/wallet/wallet.cpp | 4 ++-- src/wallet/wallet.h | 7 ++----- test/functional/mocks/signer.py | 1 + test/functional/wallet_signer.py | 7 +++++++ 12 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 6114236623..c41f35829d 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -127,7 +127,7 @@ class Wallet virtual bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) = 0; //! Display address on external signer - virtual bool displayAddress(const CTxDestination& dest) = 0; + virtual util::Result displayAddress(const CTxDestination& dest) = 0; //! Lock coin. virtual bool lockCoin(const COutPoint& output, const bool write_to_db) = 0; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 1bdf94d3b5..fe000bcbb8 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -569,16 +569,17 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) return true; } -bool WalletModel::displayAddress(std::string sAddress) const +void WalletModel::displayAddress(std::string sAddress) const { CTxDestination dest = DecodeDestination(sAddress); - bool res = false; try { - res = m_wallet->displayAddress(dest); + util::Result result = m_wallet->displayAddress(dest); + if (!result) { + QMessageBox::warning(nullptr, tr("Signer error"), QString::fromStdString(util::ErrorString(result).translated)); + } } catch (const std::runtime_error& e) { QMessageBox::critical(nullptr, tr("Can't display address"), e.what()); } - return res; } bool WalletModel::isWalletEnabled() diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 503ee16823..ab2096c1fe 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -130,7 +130,7 @@ class WalletModel : public QObject UnlockContext requestUnlock(); bool bumpFee(uint256 hash, uint256& new_hash); - bool displayAddress(std::string sAddress) const; + void displayAddress(std::string sAddress) const; static bool isWalletEnabled(); diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index ce668539e6..b5703fa54a 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -52,7 +53,7 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() { return signers[0]; } -bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const +util::Result ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const { // TODO: avoid the need to infer a descriptor from inside a descriptor wallet const CScript& scriptPubKey = GetScriptForDestination(dest); @@ -61,10 +62,17 @@ bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CTxDestination& dest, c const UniValue& result = signer.DisplayAddress(descriptor->ToString()); + const UniValue& error = result.find_value("error"); + if (error.isStr()) return util::Error{strprintf(_("Signer returned error: %s"), error.getValStr())}; + const UniValue& ret_address = result.find_value("address"); - if (!ret_address.isStr()) return false; + if (!ret_address.isStr()) return util::Error{_("Signer did not echo address")}; + + if (ret_address.getValStr() != EncodeDestination(dest)) { + return util::Error{strprintf(_("Signer echoed unexpected address %s"), ret_address.getValStr())}; + } - return ret_address.getValStr() == EncodeDestination(dest); + return util::Result(); } // If sign is true, transaction must previously have been filled diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index b249833271..44286456b6 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -9,6 +9,8 @@ #include +struct bilingual_str; + namespace wallet { class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan { @@ -27,7 +29,11 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan static ExternalSigner GetExternalSigner(); - bool DisplayAddress(const CTxDestination& dest, const ExternalSigner &signer) const; + /** + * Display address on the device and verify that the returned value matches. + * @returns nothing or an error message + */ + util::Result DisplayAddress(const CTxDestination& dest, const ExternalSigner& signer) const; TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr, bool finalize = true) const override; }; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index d33e6f3873..e17e6c332d 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -247,7 +247,7 @@ class WalletImpl : public Wallet return value.empty() ? m_wallet->EraseAddressReceiveRequest(batch, dest, id) : m_wallet->SetAddressReceiveRequest(batch, dest, id, value); } - bool displayAddress(const CTxDestination& dest) override + util::Result displayAddress(const CTxDestination& dest) override { LOCK(m_wallet->cs_wallet); return m_wallet->DisplayAddress(dest); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 7f068c05ef..bed9ec029a 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -789,9 +789,8 @@ RPCHelpMan walletdisplayaddress() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); } - if (!pwallet->DisplayAddress(dest)) { - throw JSONRPCError(RPC_MISC_ERROR, "Failed to display address"); - } + util::Result res = pwallet->DisplayAddress(dest); + if (!res) throw JSONRPCError(RPC_MISC_ERROR, util::ErrorString(res).original); UniValue result(UniValue::VOBJ); result.pushKV("address", request.params[0].get_str()); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 4575881d96..2c1ab8d44a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -27,7 +27,6 @@ #include enum class OutputType; -struct bilingual_str; namespace wallet { struct MigrationData; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 675d8f3985..8f4171eb15 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2667,7 +2667,7 @@ void ReserveDestination::ReturnDestination() address = CNoDestination(); } -bool CWallet::DisplayAddress(const CTxDestination& dest) +util::Result CWallet::DisplayAddress(const CTxDestination& dest) { CScript scriptPubKey = GetScriptForDestination(dest); for (const auto& spk_man : GetScriptPubKeyMans(scriptPubKey)) { @@ -2678,7 +2678,7 @@ bool CWallet::DisplayAddress(const CTxDestination& dest) ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); return signer_spk_man->DisplayAddress(dest, signer); } - return false; + return util::Error{_("There is no ScriptPubKeyManager for this address")}; } bool CWallet::LockCoin(const COutPoint& output, WalletBatch* batch) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 74c0c5ed08..6a998fa398 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -537,11 +537,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool IsSpentKey(const CScript& scriptPubKey) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - /** Display address on an external signer. - * Returns false if the signer does not respond with a matching address. - * Returns false if external signer support is not compiled. - */ - bool DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + /** Display address on an external signer. */ + util::Result DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsLockedCoin(const COutPoint& output) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool LockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py index dc3701fae2..23d163aac3 100755 --- a/test/functional/mocks/signer.py +++ b/test/functional/mocks/signer.py @@ -49,6 +49,7 @@ def displayaddress(args): "sh(wpkh([00000001/49h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7))#kz9y5w82": "2N2gQKzjUe47gM8p1JZxaAkTcoHPXV6YyVp", "pkh([00000001/44h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#q3pqd8wh": "n1LKejAadN6hg2FrBXoU1KrwX4uK16mco9", "tr([00000001/86h/1h/0h/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#puqqa90m": "tb1phw4cgpt6cd30kz9k4wkpwm872cdvhss29jga2xpmftelhqll62mscq0k4g", + "wpkh([00000001/84h/1h/0h/0/1]03a20a46308be0b8ded6dff0a22b10b4245c587ccf23f3b4a303885be3a524f172)#aqpjv5xr": "wrong_address", } if args.desc not in expected_desc: return sys.stdout.write(json.dumps({"error": "Unexpected descriptor", "desc": args.desc})) diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 425f535b51..abfc3c1ba1 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -141,6 +141,13 @@ def test_valid_signer(self): ) self.clear_mock_result(self.nodes[1]) + # Returned address MUST match: + address_fail = hww.getnewaddress(address_type="bech32") + assert_equal(address_fail, "bcrt1ql7zg7ukh3dwr25ex2zn9jse926f27xy2jz58tm") + assert_raises_rpc_error(-1, 'Signer echoed unexpected address wrong_address', + hww.walletdisplayaddress, address_fail + ) + self.log.info('Prepare mock PSBT') self.nodes[0].sendtoaddress(address4, 1) self.generate(self.nodes[0], 1) From c87b0a0ff4cb6d83bb59360ac4453f6daa871177 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Wed, 13 Mar 2024 11:41:04 -0400 Subject: [PATCH 26/72] zmq: accept unix domain socket address for notifier --- src/init.cpp | 37 +++++++++++++++------------- src/zmq/zmqnotificationinterface.cpp | 8 +++++- src/zmq/zmqutil.h | 3 +++ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index e8e6391af7..c19d596c7f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1301,30 +1301,33 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } - for (const std::string port_option : { - "-i2psam", - "-onion", - "-proxy", - "-rpcbind", - "-torcontrol", - "-whitebind", - "-zmqpubhashblock", - "-zmqpubhashtx", - "-zmqpubrawblock", - "-zmqpubrawtx", - "-zmqpubsequence", + for (const auto &port_option : std::vector>{ + // arg name UNIX socket support + {"-i2psam", false}, + {"-onion", true}, + {"-proxy", true}, + {"-rpcbind", false}, + {"-torcontrol", false}, + {"-whitebind", false}, + {"-zmqpubhashblock", true}, + {"-zmqpubhashtx", true}, + {"-zmqpubrawblock", true}, + {"-zmqpubrawtx", true}, + {"-zmqpubsequence", true} }) { - for (const std::string& socket_addr : args.GetArgs(port_option)) { + const std::string arg{port_option.first}; + const bool unix{port_option.second}; + for (const std::string& socket_addr : args.GetArgs(arg)) { std::string host_out; uint16_t port_out{0}; if (!SplitHostPort(socket_addr, port_out, host_out)) { #if HAVE_SOCKADDR_UN - // Allow unix domain sockets for -proxy and -onion e.g. unix:/some/file/path - if ((port_option != "-proxy" && port_option != "-onion") || socket_addr.find(ADDR_PREFIX_UNIX) != 0) { - return InitError(InvalidPortErrMsg(port_option, socket_addr)); + // Allow unix domain sockets for some options e.g. unix:/some/file/path + if (!unix || socket_addr.find(ADDR_PREFIX_UNIX) != 0) { + return InitError(InvalidPortErrMsg(arg, socket_addr)); } #else - return InitError(InvalidPortErrMsg(port_option, socket_addr)); + return InitError(InvalidPortErrMsg(arg, socket_addr)); #endif } } diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index d10db046f5..536e471053 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -57,7 +58,12 @@ std::unique_ptr CZMQNotificationInterface::Create(std { std::string arg("-zmq" + entry.first); const auto& factory = entry.second; - for (const std::string& address : gArgs.GetArgs(arg)) { + for (std::string& address : gArgs.GetArgs(arg)) { + // libzmq uses prefix "ipc://" for UNIX domain sockets + if (address.substr(0, ADDR_PREFIX_UNIX.length()) == ADDR_PREFIX_UNIX) { + address.replace(0, ADDR_PREFIX_UNIX.length(), ADDR_PREFIX_IPC); + } + std::unique_ptr notifier = factory(); notifier->SetType(entry.first); notifier->SetAddress(address); diff --git a/src/zmq/zmqutil.h b/src/zmq/zmqutil.h index 334b51aa91..bec48c0a56 100644 --- a/src/zmq/zmqutil.h +++ b/src/zmq/zmqutil.h @@ -9,4 +9,7 @@ void zmqError(const std::string& str); +/** Prefix for unix domain socket addresses (which are local filesystem paths) */ +const std::string ADDR_PREFIX_IPC = "ipc://"; // used by libzmq, example "ipc:///root/path/to/file" + #endif // BITCOIN_ZMQ_ZMQUTIL_H From 791dea204ecde9b500ec243b4e16fc601998ec84 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Wed, 13 Mar 2024 12:00:25 -0400 Subject: [PATCH 27/72] test: cover unix sockets in zmq interface --- test/functional/interface_zmq.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index 3c3ff1e4a0..9f6f8919de 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -3,7 +3,9 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the ZMQ notification interface.""" +import os import struct +import tempfile from time import sleep from io import BytesIO @@ -30,7 +32,7 @@ from test_framework.wallet import ( MiniWallet, ) -from test_framework.netutil import test_ipv6_local +from test_framework.netutil import test_ipv6_local, test_unix_socket # Test may be skipped and not have zmq installed @@ -119,6 +121,10 @@ def run_test(self): self.ctx = zmq.Context() try: self.test_basic() + if test_unix_socket(): + self.test_basic(unix=True) + else: + self.log.info("Skipping ipc test, because UNIX sockets are not supported.") self.test_sequence() self.test_mempool_sync() self.test_reorg() @@ -139,7 +145,7 @@ def setup_zmq_test(self, services, *, recv_timeout=60, sync_blocks=True, ipv6=Fa socket.setsockopt(zmq.IPV6, 1) subscribers.append(ZMQSubscriber(socket, topic.encode())) - self.restart_node(0, [f"-zmqpub{topic}={address}" for topic, address in services]) + self.restart_node(0, [f"-zmqpub{topic}={address.replace('ipc://', 'unix:')}" for topic, address in services]) for i, sub in enumerate(subscribers): sub.socket.connect(services[i][1]) @@ -176,12 +182,19 @@ def setup_zmq_test(self, services, *, recv_timeout=60, sync_blocks=True, ipv6=Fa return subscribers - def test_basic(self): + def test_basic(self, unix = False): + self.log.info(f"Running basic test with {'ipc' if unix else 'tcp'} protocol") # Invalid zmq arguments don't take down the node, see #17185. self.restart_node(0, ["-zmqpubrawtx=foo", "-zmqpubhashtx=bar"]) address = f"tcp://127.0.0.1:{self.zmq_port_base}" + + if unix: + # Use the shortest temp path possible since paths may have as little as 92-char limit + socket_path = tempfile.NamedTemporaryFile().name + address = f"ipc://{socket_path}" + subs = self.setup_zmq_test([(topic, address) for topic in ["hashblock", "hashtx", "rawblock", "rawtx"]]) hashblock = subs[0] @@ -247,6 +260,8 @@ def test_basic(self): ]) assert_equal(self.nodes[1].getzmqnotifications(), []) + if unix: + os.unlink(socket_path) def test_reorg(self): From 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Wed, 13 Mar 2024 12:43:57 -0400 Subject: [PATCH 28/72] doc: release notes for PR 27679 --- doc/release-notes-27679.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 doc/release-notes-27679.md diff --git a/doc/release-notes-27679.md b/doc/release-notes-27679.md new file mode 100644 index 0000000000..dbbb30428c --- /dev/null +++ b/doc/release-notes-27679.md @@ -0,0 +1,2 @@ +- unix socket paths are now accepted for `-zmqpubrawblock` and `-zmqpubrawtx` with +the format `-zmqpubrawtx=unix:/path/to/file` \ No newline at end of file From 78b6b5c485191b85ae201df9d5ef0bcdaaa9c190 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 15 Apr 2024 11:27:15 +0100 Subject: [PATCH 29/72] build: don't pass strip to macOS deploy if cross-compiling This could only be called in code paths that cannot be hit. --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index cd1509a0ff..a78ce61ba7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -126,7 +126,7 @@ $(OSX_ZIP): deploydir cd $(APP_DIST_DIR) && find . | sort | $(ZIP) -X@ $@ $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt: $(OSX_APP_BUILT) $(OSX_PACKAGING) - INSTALL_NAME_TOOL=$(INSTALL_NAME_TOOL) OTOOL=$(OTOOL) STRIP=$(STRIP) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR) + INSTALL_NAME_TOOL=$(INSTALL_NAME_TOOL) OTOOL=$(OTOOL) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR) deploydir: $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt endif !BUILD_DARWIN From 3bee51427a05075150721f0a05ead8f92e1ba019 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 15 Apr 2024 11:28:44 +0100 Subject: [PATCH 30/72] build: don't use install_name_tool for macOS deploy when cross-compiling This is only needed when compiling on macOS. This means we can also better scope the usage of `-headerpad_max_install_names`. --- Makefile.am | 2 +- configure.ac | 1 - contrib/macdeploy/README.md | 5 ++--- depends/Makefile | 1 - depends/builders/darwin.mk | 2 -- depends/builders/default.mk | 2 +- depends/config.site.in | 5 ----- depends/hosts/darwin.mk | 2 +- depends/hosts/default.mk | 2 +- 9 files changed, 6 insertions(+), 16 deletions(-) diff --git a/Makefile.am b/Makefile.am index a78ce61ba7..e79dc4e21b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -126,7 +126,7 @@ $(OSX_ZIP): deploydir cd $(APP_DIST_DIR) && find . | sort | $(ZIP) -X@ $@ $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt: $(OSX_APP_BUILT) $(OSX_PACKAGING) - INSTALL_NAME_TOOL=$(INSTALL_NAME_TOOL) OTOOL=$(OTOOL) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR) + OTOOL=$(OTOOL) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR) deploydir: $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt endif !BUILD_DARWIN diff --git a/configure.ac b/configure.ac index febb352cdb..17e1fb938c 100644 --- a/configure.ac +++ b/configure.ac @@ -759,7 +759,6 @@ case $host in ;; *) AC_PATH_TOOL([DSYMUTIL], [dsymutil], [dsymutil]) - AC_PATH_TOOL([INSTALL_NAME_TOOL], [install_name_tool], [install_name_tool]) AC_PATH_TOOL([OTOOL], [otool], [otool]) AC_PATH_PROG([ZIP], [zip], [zip]) diff --git a/contrib/macdeploy/README.md b/contrib/macdeploy/README.md index ea599df3d8..d1df3062f8 100644 --- a/contrib/macdeploy/README.md +++ b/contrib/macdeploy/README.md @@ -66,9 +66,8 @@ building for macOS. Apple's version of `binutils` (called `cctools`) contains lots of functionality missing in the FSF's `binutils`. In addition to extra linker options for frameworks and sysroots, several -other tools are needed as well such as `install_name_tool`, `lipo`, and `nmedit`. These -do not build under Linux, so they have been patched to do so. The work here was used as -a starting point: [mingwandroid/toolchain4](https://github.com/mingwandroid/toolchain4). +other tools are needed as well. These do not build under Linux, so they have been patched to +do so. The work here was used as a starting point: [mingwandroid/toolchain4](https://github.com/mingwandroid/toolchain4). In order to build a working toolchain, the following source packages are needed from Apple: `cctools`, `dyld`, and `ld64`. diff --git a/depends/Makefile b/depends/Makefile index 88aae7ad81..005d9696fb 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -234,7 +234,6 @@ $(host_prefix)/share/config.site : config.site.in $(host_prefix)/.stamp_$(final_ -e 's|@NM@|$(host_NM)|' \ -e 's|@STRIP@|$(host_STRIP)|' \ -e 's|@OTOOL@|$(host_OTOOL)|' \ - -e 's|@INSTALL_NAME_TOOL@|$(host_INSTALL_NAME_TOOL)|' \ -e 's|@DSYMUTIL@|$(host_DSYMUTIL)|' \ -e 's|@build_os@|$(build_os)|' \ -e 's|@host_os@|$(host_os)|' \ diff --git a/depends/builders/darwin.mk b/depends/builders/darwin.mk index a5f07643de..554bfd2c3e 100644 --- a/depends/builders/darwin.mk +++ b/depends/builders/darwin.mk @@ -5,7 +5,6 @@ build_darwin_RANLIB:=$(shell xcrun -f ranlib) build_darwin_STRIP:=$(shell xcrun -f strip) build_darwin_OTOOL:=$(shell xcrun -f otool) build_darwin_NM:=$(shell xcrun -f nm) -build_darwin_INSTALL_NAME_TOOL:=$(shell xcrun -f install_name_tool) build_darwin_DSYMUTIL:=$(shell xcrun -f dsymutil) build_darwin_SHA256SUM=shasum -a 256 build_darwin_DOWNLOAD=curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o @@ -18,7 +17,6 @@ darwin_RANLIB:=$(shell xcrun -f ranlib) darwin_STRIP:=$(shell xcrun -f strip) darwin_OTOOL:=$(shell xcrun -f otool) darwin_NM:=$(shell xcrun -f nm) -darwin_INSTALL_NAME_TOOL:=$(shell xcrun -f install_name_tool) darwin_DSYMUTIL:=$(shell xcrun -f dsymutil) darwin_native_binutils= darwin_native_toolchain= diff --git a/depends/builders/default.mk b/depends/builders/default.mk index 806d6e7c50..50869cd8a2 100644 --- a/depends/builders/default.mk +++ b/depends/builders/default.mk @@ -12,7 +12,7 @@ build_$(build_os)_$1 ?= $$(default_build_$1) build_$(build_arch)_$(build_os)_$1 ?= $$(build_$(build_os)_$1) build_$1=$$(build_$(build_arch)_$(build_os)_$1) endef -$(foreach var,CC CXX AR TAR RANLIB NM STRIP SHA256SUM DOWNLOAD OTOOL INSTALL_NAME_TOOL DSYMUTIL TOUCH,$(eval $(call add_build_tool_func,$(var)))) +$(foreach var,CC CXX AR TAR RANLIB NM STRIP SHA256SUM DOWNLOAD OTOOL DSYMUTIL TOUCH,$(eval $(call add_build_tool_func,$(var)))) define add_build_flags_func build_$(build_arch)_$(build_os)_$1 += $(build_$(build_os)_$1) build_$1=$$(build_$(build_arch)_$(build_os)_$1) diff --git a/depends/config.site.in b/depends/config.site.in index 29b2a67ed0..81975f02b9 100644 --- a/depends/config.site.in +++ b/depends/config.site.in @@ -123,11 +123,6 @@ if test "@host_os@" = darwin; then ac_cv_path_OTOOL="${OTOOL}" fi - if test -n "@INSTALL_NAME_TOOL@"; then - INSTALL_NAME_TOOL="@INSTALL_NAME_TOOL@" - ac_cv_path_INSTALL_NAME_TOOL="${INSTALL_NAME_TOOL}" - fi - if test -n "@DSYMUTIL@"; then DSYMUTIL="@DSYMUTIL@" ac_cv_path_DSYMUTIL="${DSYMUTIL}" diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index 8beedcc98a..639259ace3 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -39,7 +39,7 @@ llvm_config_prog=$(shell $(SHELL) $(.SHELLFLAGS) "command -v llvm-config") llvm_lib_dir=$(shell $(llvm_config_prog) --libdir) endif -cctools_TOOLS=AR RANLIB STRIP NM OTOOL INSTALL_NAME_TOOL DSYMUTIL +cctools_TOOLS=AR RANLIB STRIP NM OTOOL DSYMUTIL # Make-only lowercase function lc = $(subst A,a,$(subst B,b,$(subst C,c,$(subst D,d,$(subst E,e,$(subst F,f,$(subst G,g,$(subst H,h,$(subst I,i,$(subst J,j,$(subst K,k,$(subst L,l,$(subst M,m,$(subst N,n,$(subst O,o,$(subst P,p,$(subst Q,q,$(subst R,r,$(subst S,s,$(subst T,t,$(subst U,u,$(subst V,v,$(subst W,w,$(subst X,x,$(subst Y,y,$(subst Z,z,$1)))))))))))))))))))))))))) diff --git a/depends/hosts/default.mk b/depends/hosts/default.mk index cf3c90441d..6a6cab6cc6 100644 --- a/depends/hosts/default.mk +++ b/depends/hosts/default.mk @@ -38,5 +38,5 @@ host_$1 = $$($(host_arch)_$(host_os)_$1) host_$(release_type)_$1 = $$($(host_arch)_$(host_os)_$(release_type)_$1) endef -$(foreach tool,CC CXX AR RANLIB STRIP NM OBJCOPY OTOOL INSTALL_NAME_TOOL DSYMUTIL,$(eval $(call add_host_tool_func,$(tool)))) +$(foreach tool,CC CXX AR RANLIB STRIP NM OBJCOPY OTOOL DSYMUTIL,$(eval $(call add_host_tool_func,$(tool)))) $(foreach flags,CFLAGS CXXFLAGS CPPFLAGS LDFLAGS, $(eval $(call add_host_flags_func,$(flags)))) From 1a9aa8d4eedff3788c792799328ad599132e0da1 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 16 Apr 2024 16:03:22 +0100 Subject: [PATCH 31/72] build: better scope usage of -Wl,-headerpad_max_install_names If we aren't using install_name_tool when cross-compiling, we don't need to test for / add it to LDFLAGS when that is the case. --- configure.ac | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 17e1fb938c..49fe3a22b0 100644 --- a/configure.ac +++ b/configure.ac @@ -696,6 +696,9 @@ case $host in TARGET_OS=darwin if test $cross_compiling != "yes"; then BUILD_OS=darwin + + AX_CHECK_LINK_FLAG([-Wl,-headerpad_max_install_names], [CORE_LDFLAGS="$CORE_LDFLAGS -Wl,-headerpad_max_install_names"], [], [$LDFLAG_WERROR]) + AC_CHECK_PROG([BREW], [brew], [brew]) if test "$BREW" = "brew"; then dnl These Homebrew packages may be keg-only, meaning that they won't be found @@ -771,7 +774,6 @@ case $host in esac fi - AX_CHECK_LINK_FLAG([-Wl,-headerpad_max_install_names], [CORE_LDFLAGS="$CORE_LDFLAGS -Wl,-headerpad_max_install_names"], [], [$LDFLAG_WERROR]) CORE_CPPFLAGS="$CORE_CPPFLAGS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0" dnl ignore deprecated-declarations warnings coming from objcxx code From 65951e0418c53cbbf30b9ee85e24ccaf729088a1 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 17 Apr 2024 17:24:05 -0300 Subject: [PATCH 32/72] index: race fix, lock cs_main while 'm_synced' is subject to change This ensures that the index does not miss any 'new block' signals occurring in-between reading the 'next block' and setting 'm_synced'. Because, if this were to happen, the ignored blocks would never be indexed, thus stalling the index forever. --- src/index/base.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index a203ce4a9f..e66c89f9e4 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -160,12 +160,24 @@ void BaseIndex::Sync() } const CBlockIndex* pindex_next = WITH_LOCK(cs_main, return NextSyncBlock(pindex, m_chainstate->m_chain)); + // If pindex_next is null, it means pindex is the chain tip, so + // commit data indexed so far. if (!pindex_next) { SetBestBlockIndex(pindex); // No need to handle errors in Commit. See rationale above. Commit(); - m_synced = true; - break; + + // If pindex is still the chain tip after committing, exit the + // sync loop. It is important for cs_main to be locked while + // setting m_synced = true, otherwise a new block could be + // attached while m_synced is still false, and it would not be + // indexed. + LOCK(::cs_main); + pindex_next = NextSyncBlock(pindex, m_chainstate->m_chain); + if (!pindex_next) { + m_synced = true; + break; + } } if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) { FatalErrorf("%s: Failed to rewind index %s to a previous chain tip", __func__, GetName()); From 09f5a74198c328c80539c17d951a70558e6b361e Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:18:44 +0100 Subject: [PATCH 33/72] fuzz: Re-implement `read_stdin` in portable way --- src/test/fuzz/fuzz.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index a8e490b459..f9915187bd 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include @@ -135,9 +134,9 @@ void initialize() #if defined(PROVIDE_FUZZ_MAIN_FUNCTION) static bool read_stdin(std::vector& data) { - uint8_t buffer[1024]; - ssize_t length = 0; - while ((length = read(STDIN_FILENO, buffer, 1024)) > 0) { + std::istream::char_type buffer[1024]; + std::streamsize length; + while ((std::cin.read(buffer, 1024), length = std::cin.gcount()) > 0) { data.insert(data.end(), buffer, buffer + length); } return length == 0; From 4c078d7bd278fa8b4db6e1da7b9b747f49a8ac4c Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:27:12 +0100 Subject: [PATCH 34/72] build, msvc: Enable preprocessor conformance mode See: - https://learn.microsoft.com/en-us/cpp/build/reference/zc-preprocessor - https://learn.microsoft.com/en-us/cpp/preprocessor/preprocessor-experimental-overview Otherwise, the "traditional" MSVC preprocessor fails to parse the `FUZZ_TARGET` and `DETAIL_FUZZ` macros because of behavior changes highlighted in the docs mentioned above. --- build_msvc/common.init.vcxproj.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build_msvc/common.init.vcxproj.in b/build_msvc/common.init.vcxproj.in index 950cc37f0a..8f2cfbfe15 100644 --- a/build_msvc/common.init.vcxproj.in +++ b/build_msvc/common.init.vcxproj.in @@ -87,7 +87,7 @@ Level3 NotUsing - /utf-8 /Zc:__cplusplus /std:c++20 %(AdditionalOptions) + /utf-8 /Zc:preprocessor /Zc:__cplusplus /std:c++20 %(AdditionalOptions) 4018;4244;4267;4715;4805 true _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;%(PreprocessorDefinitions) From 19dceddf4bcdb74e84cf27229039a239b873d41b Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:27:25 +0100 Subject: [PATCH 35/72] build, msvc: Build `fuzz.exe` binary --- build_msvc/bitcoin.sln | 8 +- build_msvc/bitcoind/bitcoind.vcxproj | 2 +- build_msvc/common.init.vcxproj.in | 2 +- build_msvc/fuzz/fuzz.vcxproj | 91 +++++++++++++++++++++++ src/test/fuzz/addition_overflow.cpp | 2 + src/test/fuzz/deserialize.cpp | 1 - src/test/fuzz/multiplication_overflow.cpp | 6 ++ 7 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 build_msvc/fuzz/fuzz.vcxproj diff --git a/build_msvc/bitcoin.sln b/build_msvc/bitcoin.sln index 0931bf5dfe..9fd6395f59 100644 --- a/build_msvc/bitcoin.sln +++ b/build_msvc/bitcoin.sln @@ -48,7 +48,9 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "libtest_util", "libtest_uti EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "test_bitcoin-qt", "test_bitcoin-qt\test_bitcoin-qt.vcxproj", "{51201D5E-D939-4854-AE9D-008F03FF518E}" EndProject -Project("{542007E3-BE0D-4B0D-A6B0-AA8813E2558D}") = "libminisketch", "libminisketch\libminisketch.vcxproj", "{542007E3-BE0D-4B0D-A6B0-AA8813E2558D}" +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "libminisketch", "libminisketch\libminisketch.vcxproj", "{542007E3-BE0D-4B0D-A6B0-AA8813E2558D}" +EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "fuzz", "fuzz\fuzz.vcxproj", "{AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -152,6 +154,10 @@ Global {542007E3-BE0D-4B0D-A6B0-AA8813E2558D}.Debug|x64.Build.0 = Debug|x64 {542007E3-BE0D-4B0D-A6B0-AA8813E2558D}.Release|x64.ActiveCfg = Release|x64 {542007E3-BE0D-4B0D-A6B0-AA8813E2558D}.Release|x64.Build.0 = Release|x64 + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}.Debug|x64.ActiveCfg = Debug|x64 + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}.Debug|x64.Build.0 = Debug|x64 + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}.Release|x64.ActiveCfg = Release|x64 + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8}.Release|x64.Build.0 = Release|x64 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/build_msvc/bitcoind/bitcoind.vcxproj b/build_msvc/bitcoind/bitcoind.vcxproj index a93723d8be..63337ca6a7 100644 --- a/build_msvc/bitcoind/bitcoind.vcxproj +++ b/build_msvc/bitcoind/bitcoind.vcxproj @@ -80,7 +80,7 @@ + Replace="@ENABLE_FUZZ_BINARY_TRUE@" By=""> /utf-8 /Zc:preprocessor /Zc:__cplusplus /std:c++20 %(AdditionalOptions) 4018;4244;4267;4715;4805 true - _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;%(PreprocessorDefinitions) + _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;PROVIDE_FUZZ_MAIN_FUNCTION;%(PreprocessorDefinitions) ..\..\src;..\..\src\minisketch\include;..\..\src\univalue\include;..\..\src\secp256k1\include;..\..\src\leveldb\include;..\..\src\leveldb\helpers\memenv;%(AdditionalIncludeDirectories) diff --git a/build_msvc/fuzz/fuzz.vcxproj b/build_msvc/fuzz/fuzz.vcxproj new file mode 100644 index 0000000000..fb77251a17 --- /dev/null +++ b/build_msvc/fuzz/fuzz.vcxproj @@ -0,0 +1,91 @@ + + + + + {AFCEE6C1-89FB-49AB-A694-BA580A59E2D8} + + + Application + $(SolutionDir)$(Platform)\$(Configuration)\ + + + + + + $(IntDir)test_fuzz_util_descriptor.obj + + + $(IntDir)test_fuzz_util_mempool.obj + + + $(IntDir)test_fuzz_util_net.obj + + + $(IntDir)wallet_test_fuzz_coincontrol.obj + + + $(IntDir)wallet_test_fuzz_coinselection.obj + + + $(IntDir)wallet_test_fuzz_fees.obj + + + $(IntDir)wallet_test_fuzz_notifications.obj + + + $(IntDir)wallet_test_fuzz_parse_iso8601.obj + + + $(IntDir)wallet_test_fuzz_scriptpubkeyman.obj + + + + + {542007e3-be0d-4b0d-a6b0-aa8813e2558d} + + + {2b384fa8-9ee1-4544-93cb-0d733c25e8ce} + + + {0667528c-d734-4009-adf9-c0d6c4a5a5a6} + + + {7c87e378-df58-482e-aa2f-1bc129bc19ce} + + + {6190199c-6cf4-4dad-bfbd-93fa72a760c1} + + + {460fee33-1fe1-483f-b3bf-931ff8e969a5} + + + {b53a5535-ee9d-4c6f-9a26-f79ee3bc3754} + + + {93b86837-b543-48a5-a89b-7c87abb77df2} + + + {792d487f-f14c-49fc-a9de-3fc150f31c3f} + + + {1e065f03-3566-47d0-8fa9-daa72b084e7d} + + + {5724ba7d-a09a-4ba8-800b-c4c1561b3d69} + + + {bb493552-3b8c-4a8c-bf69-a6e7a51d2ea6} + + + {18430fef-6b61-4c53-b396-718e02850f1b} + + + + + 4018;4244;4267;4334;4715;4805 + + + + + + diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp index 5100b6f438..071e5fb029 100644 --- a/src/test/fuzz/addition_overflow.cpp +++ b/src/test/fuzz/addition_overflow.cpp @@ -24,12 +24,14 @@ void TestAdditionOverflow(FuzzedDataProvider& fuzzed_data_provider) assert(is_addition_overflow_custom == AdditionOverflow(j, i)); assert(maybe_add == CheckedAdd(j, i)); assert(sat_add == SaturatingAdd(j, i)); +#ifndef _MSC_VER T result_builtin; const bool is_addition_overflow_builtin = __builtin_add_overflow(i, j, &result_builtin); assert(is_addition_overflow_custom == is_addition_overflow_builtin); if (!is_addition_overflow_custom) { assert(i + j == result_builtin); } +#endif if (is_addition_overflow_custom) { assert(sat_add == std::numeric_limits::min() || sat_add == std::numeric_limits::max()); } else { diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index ebc5673e71..c9a3bc86ac 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -33,7 +33,6 @@ #include #include #include -#include using node::SnapshotMetadata; diff --git a/src/test/fuzz/multiplication_overflow.cpp b/src/test/fuzz/multiplication_overflow.cpp index aeef4f24b7..a762a4dfe3 100644 --- a/src/test/fuzz/multiplication_overflow.cpp +++ b/src/test/fuzz/multiplication_overflow.cpp @@ -17,12 +17,18 @@ void TestMultiplicationOverflow(FuzzedDataProvider& fuzzed_data_provider) const T i = fuzzed_data_provider.ConsumeIntegral(); const T j = fuzzed_data_provider.ConsumeIntegral(); const bool is_multiplication_overflow_custom = MultiplicationOverflow(i, j); +#ifndef _MSC_VER T result_builtin; const bool is_multiplication_overflow_builtin = __builtin_mul_overflow(i, j, &result_builtin); assert(is_multiplication_overflow_custom == is_multiplication_overflow_builtin); if (!is_multiplication_overflow_custom) { assert(i * j == result_builtin); } +#else + if (!is_multiplication_overflow_custom) { + (void)(i * j); + } +#endif } } // namespace From 23cb8207cdd6c674480840b76626039cdfe7cb13 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:27:34 +0100 Subject: [PATCH 36/72] ci, msvc: Add "Clone fuzz corpus" step --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b92e3c66d0..8cf7a55111 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -295,3 +295,10 @@ jobs: TEST_RUNNER_EXTRA: ${{ github.event_name != 'pull_request' && '--extended' || '' }} shell: cmd run: py -3 test\functional\test_runner.py --jobs %NUMBER_OF_PROCESSORS% --ci --quiet --tmpdirprefix=%RUNNER_TEMP% --combinedlogslen=99999999 --timeout-factor=%TEST_RUNNER_TIMEOUT_FACTOR% %TEST_RUNNER_EXTRA% + + - name: Clone fuzz corpus + run: | + git clone --depth=1 https://github.com/bitcoin-core/qa-assets "$env:RUNNER_TEMP\qa-assets" + Set-Location "$env:RUNNER_TEMP\qa-assets" + Write-Host "Using qa-assets repo from commit ..." + git log -1 From 52933d7283736fe3ae15e7ac44c02ca3bd95fe6d Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:27:46 +0100 Subject: [PATCH 37/72] fuzz: Pass `SystemRoot` environment variable to subprocess See https://docs.python.org/3/library/subprocess.html --- test/fuzz/test_runner.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index 558d63e85c..a635175e7c 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -11,6 +11,7 @@ import configparser import logging import os +import platform import random import subprocess import sys @@ -18,7 +19,7 @@ def get_fuzz_env(*, target, source_dir): symbolizer = os.environ.get('LLVM_SYMBOLIZER_PATH', "/usr/bin/llvm-symbolizer") - return { + fuzz_env = { 'FUZZ': target, 'UBSAN_OPTIONS': f'suppressions={source_dir}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1', @@ -27,6 +28,10 @@ def get_fuzz_env(*, target, source_dir): 'ASAN_SYMBOLIZER_PATH':symbolizer, 'MSAN_SYMBOLIZER_PATH':symbolizer, } + if platform.system() == "Windows": + # On Windows, `env` option must include valid `SystemRoot`. + fuzz_env = {**fuzz_env, 'SystemRoot': os.environ.get('SystemRoot')} + return fuzz_env def main(): From 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:27:55 +0100 Subject: [PATCH 38/72] ci, msvc: Add "Run fuzz binaries" step --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8cf7a55111..44521c1af3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -302,3 +302,9 @@ jobs: Set-Location "$env:RUNNER_TEMP\qa-assets" Write-Host "Using qa-assets repo from commit ..." git log -1 + + - name: Run fuzz binaries + env: + BITCOINFUZZ: "${{ github.workspace}}\\src\\fuzz.exe" + shell: cmd + run: py -3 test\fuzz\test_runner.py --par %NUMBER_OF_PROCESSORS% --loglevel DEBUG %RUNNER_TEMP%\qa-assets\fuzz_seed_corpus From fd81a37239541d0d508402cd4eeb28f38128c1f2 Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Wed, 18 Oct 2023 15:44:24 -0400 Subject: [PATCH 39/72] net: attempts to connect to all resolved addresses when connecting to a node Prior to this, when establishing a network connection via CConnman::ConnectNode, if the connection needed address resolution, a single address would be picked at random from the resolved addresses and our node will try to connect to it. However, this would lead to the behavior of ConnectNode being unpredictable when the address was resolved to various ips (e.g. the address resolving to IPv4 and IPv6, but we only support one of them). This patches the aforementioned behavior by going over all resolved IPs until we find one we can connect to or until we exhaust them. --- src/net.cpp | 166 ++++++++++++++++++++++++++++------------------------ 1 file changed, 91 insertions(+), 75 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index e388f05b03..f8ed487ccc 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -412,6 +412,9 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo // Resolve const uint16_t default_port{pszDest != nullptr ? GetDefaultPort(pszDest) : m_params.GetDefaultPort()}; + + // Collection of addresses to try to connect to: either all dns resolved addresses if a domain name (pszDest) is provided, or addrConnect otherwise. + std::vector connect_to{}; if (pszDest) { std::vector resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)}; if (!resolved.empty()) { @@ -432,8 +435,16 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort()); return nullptr; } + // Add the address to the resolved addresses vector so we can try to connect to it later on + connect_to.push_back(addrConnect); } + } else { + // For resolution via proxy + connect_to.push_back(addrConnect); } + } else { + // Connect via addrConnect directly + connect_to.push_back(addrConnect); } // Connect @@ -443,94 +454,99 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo assert(!addr_bind.IsValid()); std::unique_ptr i2p_transient_session; - if (addrConnect.IsValid()) { - const bool use_proxy{GetProxy(addrConnect.GetNetwork(), proxy)}; - bool proxyConnectionFailed = false; + for (auto& target_addr: connect_to) { + if (target_addr.IsValid()) { + const bool use_proxy{GetProxy(target_addr.GetNetwork(), proxy)}; + bool proxyConnectionFailed = false; - if (addrConnect.IsI2P() && use_proxy) { - i2p::Connection conn; - bool connected{false}; + if (target_addr.IsI2P() && use_proxy) { + i2p::Connection conn; + bool connected{false}; - if (m_i2p_sam_session) { - connected = m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed); - } else { - { - LOCK(m_unused_i2p_sessions_mutex); - if (m_unused_i2p_sessions.empty()) { - i2p_transient_session = - std::make_unique(proxy, &interruptNet); - } else { - i2p_transient_session.swap(m_unused_i2p_sessions.front()); - m_unused_i2p_sessions.pop(); + if (m_i2p_sam_session) { + connected = m_i2p_sam_session->Connect(target_addr, conn, proxyConnectionFailed); + } else { + { + LOCK(m_unused_i2p_sessions_mutex); + if (m_unused_i2p_sessions.empty()) { + i2p_transient_session = + std::make_unique(proxy, &interruptNet); + } else { + i2p_transient_session.swap(m_unused_i2p_sessions.front()); + m_unused_i2p_sessions.pop(); + } } - } - connected = i2p_transient_session->Connect(addrConnect, conn, proxyConnectionFailed); - if (!connected) { - LOCK(m_unused_i2p_sessions_mutex); - if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) { - m_unused_i2p_sessions.emplace(i2p_transient_session.release()); + connected = i2p_transient_session->Connect(target_addr, conn, proxyConnectionFailed); + if (!connected) { + LOCK(m_unused_i2p_sessions_mutex); + if (m_unused_i2p_sessions.size() < MAX_UNUSED_I2P_SESSIONS_SIZE) { + m_unused_i2p_sessions.emplace(i2p_transient_session.release()); + } } } - } - if (connected) { - sock = std::move(conn.sock); - addr_bind = CAddress{conn.me, NODE_NONE}; + if (connected) { + sock = std::move(conn.sock); + addr_bind = CAddress{conn.me, NODE_NONE}; + } + } else if (use_proxy) { + LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s\n", proxy.ToString(), target_addr.ToStringAddrPort()); + sock = ConnectThroughProxy(proxy, target_addr.ToStringAddr(), target_addr.GetPort(), proxyConnectionFailed); + } else { + // no proxy needed (none set for target network) + sock = ConnectDirectly(target_addr, conn_type == ConnectionType::MANUAL); } - } else if (use_proxy) { - LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s:%s\n", proxy.ToString(), addrConnect.ToStringAddr(), addrConnect.GetPort()); - sock = ConnectThroughProxy(proxy, addrConnect.ToStringAddr(), addrConnect.GetPort(), proxyConnectionFailed); - } else { - // no proxy needed (none set for target network) - sock = ConnectDirectly(addrConnect, conn_type == ConnectionType::MANUAL); + if (!proxyConnectionFailed) { + // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to + // the proxy, mark this as an attempt. + addrman.Attempt(target_addr, fCountFailure); + } + } else if (pszDest && GetNameProxy(proxy)) { + std::string host; + uint16_t port{default_port}; + SplitHostPort(std::string(pszDest), port, host); + bool proxyConnectionFailed; + sock = ConnectThroughProxy(proxy, host, port, proxyConnectionFailed); + } + // Check any other resolved address (if any) if we fail to connect + if (!sock) { + continue; } - if (!proxyConnectionFailed) { - // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to - // the proxy, mark this as an attempt. - addrman.Attempt(addrConnect, fCountFailure); + + NetPermissionFlags permission_flags = NetPermissionFlags::None; + std::vector whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector{}; + AddWhitelistPermissionFlags(permission_flags, target_addr, whitelist_permissions); + + // Add node + NodeId id = GetNewNodeId(); + uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); + if (!addr_bind.IsValid()) { + addr_bind = GetBindAddress(*sock); } - } else if (pszDest && GetNameProxy(proxy)) { - std::string host; - uint16_t port{default_port}; - SplitHostPort(std::string(pszDest), port, host); - bool proxyConnectionFailed; - sock = ConnectThroughProxy(proxy, host, port, proxyConnectionFailed); - } - if (!sock) { - return nullptr; - } + CNode* pnode = new CNode(id, + std::move(sock), + target_addr, + CalculateKeyedNetGroup(target_addr), + nonce, + addr_bind, + pszDest ? pszDest : "", + conn_type, + /*inbound_onion=*/false, + CNodeOptions{ + .permission_flags = permission_flags, + .i2p_sam_session = std::move(i2p_transient_session), + .recv_flood_size = nReceiveFloodSize, + .use_v2transport = use_v2transport, + }); + pnode->AddRef(); - NetPermissionFlags permission_flags = NetPermissionFlags::None; - std::vector whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector{}; - AddWhitelistPermissionFlags(permission_flags, addrConnect, whitelist_permissions); + // We're making a new connection, harvest entropy from the time (and our peer count) + RandAddEvent((uint32_t)id); - // Add node - NodeId id = GetNewNodeId(); - uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); - if (!addr_bind.IsValid()) { - addr_bind = GetBindAddress(*sock); + return pnode; } - CNode* pnode = new CNode(id, - std::move(sock), - addrConnect, - CalculateKeyedNetGroup(addrConnect), - nonce, - addr_bind, - pszDest ? pszDest : "", - conn_type, - /*inbound_onion=*/false, - CNodeOptions{ - .permission_flags = permission_flags, - .i2p_sam_session = std::move(i2p_transient_session), - .recv_flood_size = nReceiveFloodSize, - .use_v2transport = use_v2transport, - }); - pnode->AddRef(); - - // We're making a new connection, harvest entropy from the time (and our peer count) - RandAddEvent((uint32_t)id); - return pnode; + return nullptr; } void CNode::CloseSocketDisconnect() From 6b02c11d667adff24daf611f9b14815d27963674 Mon Sep 17 00:00:00 2001 From: stratospher <44024636+stratospher@users.noreply.github.com> Date: Wed, 17 Apr 2024 13:05:47 +0530 Subject: [PATCH 40/72] test: Fix intermittent issue in p2p_handshake.py If we reuse the same port when disconnecting and establishing connections again, we might hit this scenario: - disconnection is done on python side for P2PConnection - disconnection is not complete on c++ side for TestNode - we're trying to establish a new connection on same port again Prevent this scenario from happening by ensuring disconnection on c++ side for TestNode as well. --- test/functional/p2p_handshake.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/p2p_handshake.py b/test/functional/p2p_handshake.py index f0b62e291d..dd19fe9333 100755 --- a/test/functional/p2p_handshake.py +++ b/test/functional/p2p_handshake.py @@ -41,6 +41,7 @@ def add_outbound_connection(self, node, connection_type, services, wait_for_disc peer.sync_with_ping() peer.peer_disconnect() peer.wait_for_disconnect() + self.wait_until(lambda: len(node.getpeerinfo()) == 0) def test_desirable_service_flags(self, node, service_flag_tests, desirable_service_flags, expect_disconnect): """Check that connecting to a peer either fails or succeeds depending on its offered From 58c423def3d71892d60b973f2d86c94de7134648 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:36:55 +0100 Subject: [PATCH 41/72] depends: switch boost to .tar.gz --- depends/packages/boost.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index ebc097d686..7f0389b30d 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -1,8 +1,8 @@ package=boost $(package)_version=1.81.0 $(package)_download_path=https://boostorg.jfrog.io/artifactory/main/release/$($(package)_version)/source/ -$(package)_file_name=boost_$(subst .,_,$($(package)_version)).tar.bz2 -$(package)_sha256_hash=71feeed900fbccca04a3b4f2f84a7c217186f28a940ed8b7ed4725986baf99fa +$(package)_file_name=boost_$(subst .,_,$($(package)_version)).tar.gz +$(package)_sha256_hash=205666dea9f6a7cfed87c7a6dfbeb52a2c1b9de55712c9c1a87735d7181452b6 define $(package)_stage_cmds mkdir -p $($(package)_staging_prefix_dir)/include && \ From e7a8dd5931c165b5aac34fcfce467bc14cd727e5 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:38:15 +0100 Subject: [PATCH 42/72] depends: switch fontconfig to .tar.gz --- depends/packages/fontconfig.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/fontconfig.mk b/depends/packages/fontconfig.mk index 444acfe36d..6baaecc55a 100644 --- a/depends/packages/fontconfig.mk +++ b/depends/packages/fontconfig.mk @@ -1,8 +1,8 @@ package=fontconfig $(package)_version=2.12.6 $(package)_download_path=https://www.freedesktop.org/software/fontconfig/release/ -$(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=cf0c30807d08f6a28ab46c61b8dbd55c97d2f292cf88f3a07d3384687f31f017 +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=064b9ebf060c9e77011733ac9dc0e2ce92870b574cca2405e11f5353a683c334 $(package)_dependencies=freetype expat $(package)_patches=gperf_header_regen.patch From 5996c30384b0b2af1994751611cdeb81ee2a97d9 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:39:51 +0100 Subject: [PATCH 43/72] depends: switch libXau to .tar.gz --- depends/packages/libXau.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/libXau.mk b/depends/packages/libXau.mk index aeb14dcd6e..6bafc4f41a 100644 --- a/depends/packages/libXau.mk +++ b/depends/packages/libXau.mk @@ -1,8 +1,8 @@ package=libXau $(package)_version=1.0.9 $(package)_download_path=https://xorg.freedesktop.org/releases/individual/lib/ -$(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=ccf8cbf0dbf676faa2ea0a6d64bcc3b6746064722b606c8c52917ed00dcb73ec +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=1f123d8304b082ad63a9e89376400a3b1d4c29e67e3ea07b3f659cccca690eea $(package)_dependencies=xproto # When updating this package, check the default value of From b845029d4693a0c1ed21754e15a382cd522c95a5 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:41:16 +0100 Subject: [PATCH 44/72] depends: switch xproto to .tar.gz --- depends/packages/xproto.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/xproto.mk b/depends/packages/xproto.mk index 7a43c52faf..29c349a21b 100644 --- a/depends/packages/xproto.mk +++ b/depends/packages/xproto.mk @@ -1,8 +1,8 @@ package=xproto $(package)_version=7.0.31 $(package)_download_path=https://xorg.freedesktop.org/releases/individual/proto -$(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=c6f9747da0bd3a95f86b17fb8dd5e717c8f3ab7f0ece3ba1b247899ec1ef7747 +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=6d755eaae27b45c5cc75529a12855fed5de5969b367ed05003944cf901ed43c7 define $(package)_set_vars $(package)_config_opts=--without-fop --without-xmlto --without-xsltproc --disable-specs From 8e9190c6aae1e47f2a37d4f5f6ff4c28604e708b Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:42:27 +0100 Subject: [PATCH 45/72] depends: switch libxcb_util to .tar.gz --- depends/packages/libxcb_util.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/libxcb_util.mk b/depends/packages/libxcb_util.mk index 6e4c7359b2..dc4456f85c 100644 --- a/depends/packages/libxcb_util.mk +++ b/depends/packages/libxcb_util.mk @@ -1,8 +1,8 @@ package=libxcb_util $(package)_version=0.4.0 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-$($(package)_version).tar.bz2 -$(package)_sha256_hash=46e49469cb3b594af1d33176cd7565def2be3fa8be4371d62271fabb5eae50e9 +$(package)_file_name=xcb-util-$($(package)_version).tar.gz +$(package)_sha256_hash=0ed0934e2ef4ddff53fcc70fc64fb16fe766cd41ee00330312e20a985fd927a7 $(package)_dependencies=libxcb define $(package)_set_vars From 00a68963468cf77218bdd1158ccb9c83b5ded689 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:43:59 +0100 Subject: [PATCH 46/72] depends: switch libxcb_util_image to .tar.gz --- depends/packages/libxcb_util_image.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/libxcb_util_image.mk b/depends/packages/libxcb_util_image.mk index d12d67e8e8..2228250fec 100644 --- a/depends/packages/libxcb_util_image.mk +++ b/depends/packages/libxcb_util_image.mk @@ -1,8 +1,8 @@ package=libxcb_util_image $(package)_version=0.4.0 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-image-$($(package)_version).tar.bz2 -$(package)_sha256_hash=2db96a37d78831d643538dd1b595d7d712e04bdccf8896a5e18ce0f398ea2ffc +$(package)_file_name=xcb-util-image-$($(package)_version).tar.gz +$(package)_sha256_hash=cb2c86190cf6216260b7357a57d9100811bb6f78c24576a3a5bfef6ad3740a42 $(package)_dependencies=libxcb libxcb_util define $(package)_set_vars From ce28cb31b4ed7da9065128eb4bc9f0640e025dad Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:44:49 +0100 Subject: [PATCH 47/72] depends: switch libxcb_util_keysyms to .tar.gz --- depends/packages/libxcb_util_keysyms.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/libxcb_util_keysyms.mk b/depends/packages/libxcb_util_keysyms.mk index d4f72dedbe..56bc33d258 100644 --- a/depends/packages/libxcb_util_keysyms.mk +++ b/depends/packages/libxcb_util_keysyms.mk @@ -1,8 +1,8 @@ package=libxcb_util_keysyms $(package)_version=0.4.0 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-keysyms-$($(package)_version).tar.bz2 -$(package)_sha256_hash=0ef8490ff1dede52b7de533158547f8b454b241aa3e4dcca369507f66f216dd9 +$(package)_file_name=xcb-util-keysyms-$($(package)_version).tar.gz +$(package)_sha256_hash=0807cf078fbe38489a41d755095c58239e1b67299f14460dec2ec811e96caa96 $(package)_dependencies=libxcb xproto define $(package)_set_vars From fad989852d4e3a0723f1f7030b21fb6ac3f8ac1d Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:45:54 +0100 Subject: [PATCH 48/72] depends: switch libxcb_util_render to .tar.gz --- depends/packages/libxcb_util_render.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/libxcb_util_render.mk b/depends/packages/libxcb_util_render.mk index 28f1fb073c..ee2883feda 100644 --- a/depends/packages/libxcb_util_render.mk +++ b/depends/packages/libxcb_util_render.mk @@ -1,8 +1,8 @@ package=libxcb_util_render $(package)_version=0.3.9 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-renderutil-$($(package)_version).tar.bz2 -$(package)_sha256_hash=c6e97e48fb1286d6394dddb1c1732f00227c70bd1bedb7d1acabefdd340bea5b +$(package)_file_name=xcb-util-renderutil-$($(package)_version).tar.gz +$(package)_sha256_hash=55eee797e3214fe39d0f3f4d9448cc53cffe06706d108824ea37bb79fcedcad5 $(package)_dependencies=libxcb define $(package)_set_vars From 4a9b71b9006fc1d7069295c394baa74149576f2f Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:46:40 +0100 Subject: [PATCH 49/72] depends: switch libxcb_util_wm to .tar.gz --- depends/packages/libxcb_util_wm.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/libxcb_util_wm.mk b/depends/packages/libxcb_util_wm.mk index 3b905ba4ec..a68fd23f8a 100644 --- a/depends/packages/libxcb_util_wm.mk +++ b/depends/packages/libxcb_util_wm.mk @@ -1,8 +1,8 @@ package=libxcb_util_wm $(package)_version=0.4.1 $(package)_download_path=https://xcb.freedesktop.org/dist -$(package)_file_name=xcb-util-wm-$($(package)_version).tar.bz2 -$(package)_sha256_hash=28bf8179640eaa89276d2b0f1ce4285103d136be6c98262b6151aaee1d3c2a3f +$(package)_file_name=xcb-util-wm-$($(package)_version).tar.gz +$(package)_sha256_hash=038b39c4bdc04a792d62d163ba7908f4bb3373057208c07110be73c1b04b8334 $(package)_dependencies=libxcb define $(package)_set_vars From bd6e1d6718c8de8aa7b5bb173a201678b88d3da4 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:47:16 +0100 Subject: [PATCH 50/72] depends: switch qrencode to .tar.gz --- depends/packages/qrencode.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/packages/qrencode.mk b/depends/packages/qrencode.mk index 4d852d833d..4216646063 100644 --- a/depends/packages/qrencode.mk +++ b/depends/packages/qrencode.mk @@ -1,8 +1,8 @@ package=qrencode $(package)_version=4.1.1 $(package)_download_path=https://fukuchi.org/works/qrencode/ -$(package)_file_name=$(package)-$($(package)_version).tar.bz2 -$(package)_sha256_hash=e455d9732f8041cf5b9c388e345a641fd15707860f928e94507b1961256a6923 +$(package)_file_name=$(package)-$($(package)_version).tar.gz +$(package)_sha256_hash=da448ed4f52aba6bcb0cd48cac0dd51b8692bccc4cd127431402fca6f8171e8e $(package)_patches=cmake_fixups.patch define $(package)_set_vars From b8e084b9781eaa4d624a3c1d58b39c07005a0e13 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 17 Apr 2024 12:48:33 +0100 Subject: [PATCH 51/72] guix: remove no-longer-used bzip2 --- contrib/guix/manifest.scm | 1 - 1 file changed, 1 deletion(-) diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index 8f13c642d3..96818c7748 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -499,7 +499,6 @@ inspecting signatures in Mach-O binaries.") moreutils ;; Compression and archiving tar - bzip2 gzip xz ;; Build tools From 6f5954acac2ced22dae7088e2b679bf663507d4c Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 21 Apr 2024 14:30:38 +0100 Subject: [PATCH 52/72] ci: Drop no longer needed `-I` flag in "tidy" task --- ci/test/00_setup_env_native_tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/00_setup_env_native_tidy.sh b/ci/test/00_setup_env_native_tidy.sh index 4c8658479b..5f422bbdb6 100755 --- a/ci/test/00_setup_env_native_tidy.sh +++ b/ci/test/00_setup_env_native_tidy.sh @@ -16,5 +16,5 @@ export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=false export RUN_TIDY=true export GOAL="install" -export BITCOIN_CONFIG="CC=clang-${TIDY_LLVM_V} CXX=clang++-${TIDY_LLVM_V} --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -I/usr/lib/llvm-${TIDY_LLVM_V}/lib/clang/${TIDY_LLVM_V}/include'" +export BITCOIN_CONFIG="CC=clang-${TIDY_LLVM_V} CXX=clang++-${TIDY_LLVM_V} --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0'" export CCACHE_MAXSIZE=200M From fa6c300a9926a1d35fdd0a80f59ea39769bd2596 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 22 Apr 2024 15:24:47 +0200 Subject: [PATCH 53/72] test: Fix intermittent timeout in p2p_tx_download.py --- test/functional/p2p_tx_download.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index ae1edec2f1..7a50f1e605 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -284,17 +284,22 @@ def run_test(self): # Run each test against new bitcoind instances, as setting mocktimes has long-term effects on when # the next trickle relay event happens. - for test in [self.test_in_flight_max, self.test_inv_block, self.test_tx_requests, - self.test_rejects_filter_reset]: + for test, with_inbounds in [ + (self.test_in_flight_max, True), + (self.test_inv_block, True), + (self.test_tx_requests, True), + (self.test_rejects_filter_reset, False), + ]: self.stop_nodes() self.start_nodes() self.connect_nodes(1, 0) # Setup the p2p connections self.peers = [] - for node in self.nodes: - for _ in range(NUM_INBOUND): - self.peers.append(node.add_p2p_connection(TestP2PConn())) - self.log.info("Nodes are setup with {} incoming connections each".format(NUM_INBOUND)) + if with_inbounds: + for node in self.nodes: + for _ in range(NUM_INBOUND): + self.peers.append(node.add_p2p_connection(TestP2PConn())) + self.log.info("Nodes are setup with {} incoming connections each".format(NUM_INBOUND)) test() From b22901dfa9cc3af94bf13163a28300eb1ab25b22 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 17 Mar 2024 09:42:12 -0400 Subject: [PATCH 54/72] Avoid explicitly computing diagram; compare based on chunks --- src/policy/rbf.cpp | 8 +- src/test/feefrac_tests.cpp | 39 --------- src/test/fuzz/feeratediagram.cpp | 16 +++- src/test/fuzz/rbf.cpp | 58 ++++++------- src/test/rbf_tests.cpp | 144 +++++++++++++++---------------- src/txmempool.cpp | 6 +- src/txmempool.h | 4 +- src/util/feefrac.cpp | 40 +++------ src/util/feefrac.h | 15 ++-- 9 files changed, 140 insertions(+), 190 deletions(-) diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 84c3352b9d..2ad79b6f99 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -191,13 +191,13 @@ std::optional> ImprovesFeerateDiagram( int64_t replacement_vsize) { // Require that the replacement strictly improves the mempool's feerate diagram. - const auto diagram_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; + const auto chunk_results{pool.CalculateChunksForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; - if (!diagram_results.has_value()) { - return std::make_pair(DiagramCheckError::UNCALCULABLE, util::ErrorString(diagram_results).original); + if (!chunk_results.has_value()) { + return std::make_pair(DiagramCheckError::UNCALCULABLE, util::ErrorString(chunk_results).original); } - if (!std::is_gt(CompareFeerateDiagram(diagram_results.value().second, diagram_results.value().first))) { + if (!std::is_gt(CompareChunks(chunk_results.value().second, chunk_results.value().first))) { return std::make_pair(DiagramCheckError::FAILURE, "insufficient feerate: does not improve feerate diagram"); } return std::nullopt; diff --git a/src/test/feefrac_tests.cpp b/src/test/feefrac_tests.cpp index 2e015b382e..5af3c3d7ed 100644 --- a/src/test/feefrac_tests.cpp +++ b/src/test/feefrac_tests.cpp @@ -82,43 +82,4 @@ BOOST_AUTO_TEST_CASE(feefrac_operators) } -BOOST_AUTO_TEST_CASE(build_diagram_test) -{ - FeeFrac p1{1000, 100}; - FeeFrac empty{0, 0}; - FeeFrac zero_fee{0, 1}; - FeeFrac oversized_1{4611686000000, 4000000}; - FeeFrac oversized_2{184467440000000, 100000}; - - // Diagram-building will reorder chunks - std::vector chunks; - chunks.push_back(p1); - chunks.push_back(zero_fee); - chunks.push_back(empty); - chunks.push_back(oversized_1); - chunks.push_back(oversized_2); - - // Caller in charge of sorting chunks in case chunk size limit - // differs from cluster size limit - std::sort(chunks.begin(), chunks.end(), [](const FeeFrac& a, const FeeFrac& b) { return a > b; }); - - // Chunks are now sorted in reverse order (largest first) - BOOST_CHECK(chunks[0] == empty); // empty is considered "highest" fee - BOOST_CHECK(chunks[1] == oversized_2); - BOOST_CHECK(chunks[2] == oversized_1); - BOOST_CHECK(chunks[3] == p1); - BOOST_CHECK(chunks[4] == zero_fee); - - std::vector generated_diagram{BuildDiagramFromChunks(chunks)}; - BOOST_CHECK(generated_diagram.size() == 1 + chunks.size()); - - // Prepended with an empty, then the chunks summed in order as above - BOOST_CHECK(generated_diagram[0] == empty); - BOOST_CHECK(generated_diagram[1] == empty); - BOOST_CHECK(generated_diagram[2] == oversized_2); - BOOST_CHECK(generated_diagram[3] == oversized_2 + oversized_1); - BOOST_CHECK(generated_diagram[4] == oversized_2 + oversized_1 + p1); - BOOST_CHECK(generated_diagram[5] == oversized_2 + oversized_1 + p1 + zero_fee); -} - BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/feeratediagram.cpp b/src/test/fuzz/feeratediagram.cpp index 6d710093cb..1a9c5ee946 100644 --- a/src/test/fuzz/feeratediagram.cpp +++ b/src/test/fuzz/feeratediagram.cpp @@ -16,6 +16,20 @@ namespace { +/** Takes the pre-computed and topologically-valid chunks and generates a fee diagram which starts at FeeFrac of (0, 0) */ +std::vector BuildDiagramFromChunks(const Span chunks) +{ + std::vector diagram; + diagram.reserve(chunks.size() + 1); + + diagram.emplace_back(0, 0); + for (auto& chunk : chunks) { + diagram.emplace_back(diagram.back() + chunk); + } + return diagram; +} + + /** Evaluate a diagram at a specific size, returning the fee as a fraction. * * Fees in diagram cannot exceed 2^32, as the returned evaluation could overflow @@ -103,7 +117,7 @@ FUZZ_TARGET(build_and_compare_feerate_diagram) assert(diagram1.front() == empty); assert(diagram2.front() == empty); - auto real = CompareFeerateDiagram(diagram1, diagram2); + auto real = CompareChunks(chunks1, chunks2); auto sim = CompareDiagrams(diagram1, diagram2); assert(real == sim); diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 30ed148c37..64785948f6 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -82,17 +82,6 @@ FUZZ_TARGET(rbf, .init = initialize_rbf) } } -void CheckDiagramConcave(std::vector& diagram) -{ - // Diagrams are in monotonically-decreasing feerate order. - FeeFrac last_chunk = diagram.front(); - for (size_t i = 1; i mempool_txs; size_t iter{0}; - int64_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange(1, 1000000); + int32_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange(1, 1000000); // Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow // Add replacement_vsize since this is added to new diagram during RBF check @@ -167,32 +156,33 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) pool.CalculateDescendants(txiter, all_conflicts); } - // Calculate the feerate diagrams for a replacement. + // Calculate the chunks for a replacement. CAmount replacement_fees = ConsumeMoney(fuzzed_data_provider); - auto calc_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; + auto calc_results{pool.CalculateChunksForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)}; if (calc_results.has_value()) { - // Sanity checks on the diagrams. + // Sanity checks on the chunks. - // Diagrams start at 0. - assert(calc_results->first.front().size == 0); - assert(calc_results->first.front().fee == 0); - assert(calc_results->second.front().size == 0); - assert(calc_results->second.front().fee == 0); - - CheckDiagramConcave(calc_results->first); - CheckDiagramConcave(calc_results->second); + // Feerates are monotonically decreasing. + FeeFrac first_sum; + for (size_t i = 0; i < calc_results->first.size(); ++i) { + first_sum += calc_results->first[i]; + if (i) assert(!(calc_results->first[i - 1] << calc_results->first[i])); + } + FeeFrac second_sum; + for (size_t i = 0; i < calc_results->second.size(); ++i) { + second_sum += calc_results->second[i]; + if (i) assert(!(calc_results->second[i - 1] << calc_results->second[i])); + } - CAmount replaced_fee{0}; - int64_t replaced_size{0}; + FeeFrac replaced; for (auto txiter : all_conflicts) { - replaced_fee += txiter->GetModifiedFee(); - replaced_size += txiter->GetTxSize(); + replaced.fee += txiter->GetModifiedFee(); + replaced.size += txiter->GetTxSize(); } - // The total fee of the new diagram should be the total fee of the old - // diagram - replaced_fee + replacement_fees - assert(calc_results->first.back().fee - replaced_fee + replacement_fees == calc_results->second.back().fee); - assert(calc_results->first.back().size - replaced_size + replacement_vsize == calc_results->second.back().size); + // The total fee & size of the new diagram minus replaced fee & size should be the total + // fee & size of the old diagram minus replacement fee & size. + assert((first_sum - replaced) == (second_sum - FeeFrac{replacement_fees, replacement_vsize})); } // If internals report error, wrapper should too @@ -201,10 +191,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) assert(err_tuple.value().first == DiagramCheckError::UNCALCULABLE); } else { // Diagram check succeeded + auto old_sum = std::accumulate(calc_results->first.begin(), calc_results->first.end(), FeeFrac{}); + auto new_sum = std::accumulate(calc_results->second.begin(), calc_results->second.end(), FeeFrac{}); if (!err_tuple.has_value()) { // New diagram's final fee should always match or exceed old diagram's - assert(calc_results->first.back().fee <= calc_results->second.back().fee); - } else if (calc_results->first.back().fee > calc_results->second.back().fee) { + assert(old_sum.fee <= new_sum.fee); + } else if (old_sum.fee > new_sum.fee) { // Or it failed, and if old diagram had higher fees, it should be a failure assert(err_tuple.value().first == DiagramCheckError::FAILURE); } diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp index ed33969710..19e45c550a 100644 --- a/src/test/rbf_tests.cpp +++ b/src/test/rbf_tests.cpp @@ -426,21 +426,21 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) // Replacement of size 1 { - const auto replace_one{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})}; + const auto replace_one{pool.CalculateChunksForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})}; BOOST_CHECK(replace_one.has_value()); - std::vector expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)}; - BOOST_CHECK(replace_one->first == expected_old_diagram); - std::vector expected_new_diagram{FeeFrac(0, 0), FeeFrac(0, 1)}; - BOOST_CHECK(replace_one->second == expected_new_diagram); + std::vector expected_old_chunks{{low_fee, low_size}}; + BOOST_CHECK(replace_one->first == expected_old_chunks); + std::vector expected_new_chunks{{0, 1}}; + BOOST_CHECK(replace_one->second == expected_new_chunks); } // Non-zero replacement fee/size { - const auto replace_one_fee{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})}; + const auto replace_one_fee{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})}; BOOST_CHECK(replace_one_fee.has_value()); - std::vector expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)}; + std::vector expected_old_diagram{{low_fee, low_size}}; BOOST_CHECK(replace_one_fee->first == expected_old_diagram); - std::vector expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)}; + std::vector expected_new_diagram{{high_fee, low_size}}; BOOST_CHECK(replace_one_fee->second == expected_new_diagram); } @@ -451,22 +451,22 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto high_size = entry_high->GetTxSize(); { - const auto replace_single_chunk{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})}; + const auto replace_single_chunk{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})}; BOOST_CHECK(replace_single_chunk.has_value()); - std::vector expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)}; - BOOST_CHECK(replace_single_chunk->first == expected_old_diagram); - std::vector expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)}; - BOOST_CHECK(replace_single_chunk->second == expected_new_diagram); + std::vector expected_old_chunks{{low_fee + high_fee, low_size + high_size}}; + BOOST_CHECK(replace_single_chunk->first == expected_old_chunks); + std::vector expected_new_chunks{{high_fee, low_size}}; + BOOST_CHECK(replace_single_chunk->second == expected_new_chunks); } // Conflict with the 2nd tx, resulting in new diagram with three entries { - const auto replace_cpfp_child{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})}; + const auto replace_cpfp_child{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})}; BOOST_CHECK(replace_cpfp_child.has_value()); - std::vector expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)}; - BOOST_CHECK(replace_cpfp_child->first == expected_old_diagram); - std::vector expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size), FeeFrac(low_fee + high_fee, low_size + low_size)}; - BOOST_CHECK(replace_cpfp_child->second == expected_new_diagram); + std::vector expected_old_chunks{{low_fee + high_fee, low_size + high_size}}; + BOOST_CHECK(replace_cpfp_child->first == expected_old_chunks); + std::vector expected_new_chunks{{high_fee, low_size}, {low_fee, low_size}}; + BOOST_CHECK(replace_cpfp_child->second == expected_new_chunks); } // third transaction causes the topology check to fail @@ -476,7 +476,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto normal_size = entry_normal->GetTxSize(); { - const auto replace_too_large{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})}; + const auto replace_too_large{pool.CalculateChunksForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})}; BOOST_CHECK(!replace_too_large.has_value()); BOOST_CHECK_EQUAL(util::ErrorString(replace_too_large).original, strprintf("%s has 2 descendants, max 1 allowed", low_tx->GetHash().GetHex())); } @@ -493,12 +493,12 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto low_size_2 = entry_low_2->GetTxSize(); { - const auto replace_two_chunks_single_cluster{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})}; + const auto replace_two_chunks_single_cluster{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})}; BOOST_CHECK(replace_two_chunks_single_cluster.has_value()); - std::vector expected_old_diagram{FeeFrac(0, 0), FeeFrac(high_fee, high_size_2), FeeFrac(low_fee + high_fee, low_size_2 + high_size_2)}; - BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_diagram); - std::vector expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size_2)}; - BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_diagram); + std::vector expected_old_chunks{{high_fee, high_size_2}, {low_fee, low_size_2}}; + BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_chunks); + std::vector expected_new_chunks{{high_fee, low_size_2}}; + BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_chunks); } // You can have more than two direct conflicts if the there are multiple affected clusters, all of size 2 or less @@ -515,10 +515,10 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_3_entry = pool.GetIter(conflict_3->GetHash()).value(); { - const auto replace_multiple_clusters{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})}; + const auto replace_multiple_clusters{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})}; BOOST_CHECK(replace_multiple_clusters.has_value()); - BOOST_CHECK(replace_multiple_clusters->first.size() == 4); - BOOST_CHECK(replace_multiple_clusters->second.size() == 2); + BOOST_CHECK(replace_multiple_clusters->first.size() == 3); + BOOST_CHECK(replace_multiple_clusters->second.size() == 1); } // Add a child transaction to conflict_1 and make it cluster size 2, two chunks due to same feerate @@ -527,11 +527,11 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_1_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); { - const auto replace_multiple_clusters_2{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})}; + const auto replace_multiple_clusters_2{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})}; BOOST_CHECK(replace_multiple_clusters_2.has_value()); - BOOST_CHECK(replace_multiple_clusters_2->first.size() == 5); - BOOST_CHECK(replace_multiple_clusters_2->second.size() == 2); + BOOST_CHECK(replace_multiple_clusters_2->first.size() == 4); + BOOST_CHECK(replace_multiple_clusters_2->second.size() == 1); } // Add another descendant to conflict_1, making the cluster size > 2 should fail at this point. @@ -540,86 +540,86 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup) const auto conflict_1_grand_child_entry = pool.GetIter(conflict_1_child->GetHash()).value(); { - const auto replace_cluster_size_3{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})}; + const auto replace_cluster_size_3{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})}; BOOST_CHECK(!replace_cluster_size_3.has_value()); BOOST_CHECK_EQUAL(util::ErrorString(replace_cluster_size_3).original, strprintf("%s has 2 descendants, max 1 allowed", conflict_1->GetHash().GetHex())); } } -BOOST_AUTO_TEST_CASE(feerate_diagram_utilities) +BOOST_AUTO_TEST_CASE(feerate_chunks_utilities) { - // Sanity check the correctness of the feerate diagram comparison. + // Sanity check the correctness of the feerate chunks comparison. // A strictly better case. - std::vector old_diagram{{FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}}; - std::vector new_diagram{{FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1050, 400}}}; + std::vector old_chunks{{{950, 300}, {100, 100}}}; + std::vector new_chunks{{{1000, 300}, {50, 100}}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Incomparable diagrams - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1000, 400}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1000, 300}, {0, 100}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); // Strictly better but smaller size. - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 300}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1100, 300}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // New diagram is strictly better due to the first chunk, even though // second chunk contributes no fees - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 100}, FeeFrac{1100, 200}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{1100, 100}, {0, 100}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Feerate of first new chunk is better with, but second chunk is worse - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{999, 350}, FeeFrac{1150, 1000}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{750, 100}, {249, 250}, {151, 650}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); // If we make the second chunk slightly better, the new diagram now wins. - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{1000, 350}, FeeFrac{1150, 500}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{750, 100}, {250, 250}, {150, 150}}; - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks))); // Identical diagrams, cannot be strictly better - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; + old_chunks = {{950, 300}, {100, 100}}; + new_chunks = {{950, 300}, {100, 100}}; - BOOST_CHECK(std::is_eq(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_eq(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_eq(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_eq(CompareChunks(new_chunks, old_chunks))); // Same aggregate fee, but different total size (trigger single tail fee check step) - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}; + old_chunks = {{950, 300}, {100, 99}}; + new_chunks = {{950, 300}, {100, 100}}; // No change in evaluation when tail check needed. - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_gt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_lt(CompareChunks(new_chunks, old_chunks))); // Trigger multiple tail fee check steps - old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}}; - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}}; + old_chunks = {{950, 300}, {100, 99}}; + new_chunks = {{950, 300}, {100, 100}, {0, 1}, {0, 1}}; - BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram))); - BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram))); + BOOST_CHECK(std::is_gt(CompareChunks(old_chunks, new_chunks))); + BOOST_CHECK(std::is_lt(CompareChunks(new_chunks, old_chunks))); // Multiple tail fee check steps, unordered result - new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}, FeeFrac{1051, 403}}; - BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered); - BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered); + new_chunks = {{950, 300}, {100, 100}, {0, 1}, {0, 1}, {1, 1}}; + BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered); + BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 82eec6241f..06066e38b2 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1280,7 +1280,7 @@ std::optional CTxMemPool::CheckConflictTopology(const setEntries& d return std::nullopt; } -util::Result, std::vector>> CTxMemPool::CalculateFeerateDiagramsForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) +util::Result, std::vector>> CTxMemPool::CalculateChunksForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) { Assume(replacement_vsize > 0); @@ -1335,7 +1335,6 @@ util::Result, std::vector>> CTxMemPool:: // No topology restrictions post-chunking; sort std::sort(old_chunks.begin(), old_chunks.end(), std::greater()); - std::vector old_diagram = BuildDiagramFromChunks(old_chunks); std::vector new_chunks; @@ -1365,6 +1364,5 @@ util::Result, std::vector>> CTxMemPool:: // No topology restrictions post-chunking; sort std::sort(new_chunks.begin(), new_chunks.end(), std::greater()); - std::vector new_diagram = BuildDiagramFromChunks(new_chunks); - return std::make_pair(old_diagram, new_diagram); + return std::make_pair(old_chunks, new_chunks); } diff --git a/src/txmempool.h b/src/txmempool.h index 9dd4d56da7..52a3dc2d7d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -738,7 +738,7 @@ class CTxMemPool } /** - * Calculate the old and new mempool feerate diagrams relating to the + * Calculate the sorted chunks for the old and new mempool relating to the * clusters that would be affected by a potential replacement transaction. * (replacement_fees, replacement_vsize) values are gathered from a * proposed set of replacement transactions that are considered as a single @@ -752,7 +752,7 @@ class CTxMemPool * @param[in] all_conflicts All transactions that would be removed * @return old and new diagram pair respectively, or an error string if the conflicts don't match a calculable topology */ - util::Result, std::vector>> CalculateFeerateDiagramsForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) EXCLUSIVE_LOCKS_REQUIRED(cs); + util::Result, std::vector>> CalculateChunksForRBF(CAmount replacement_fees, int64_t replacement_vsize, const setEntries& direct_conflicts, const setEntries& all_conflicts) EXCLUSIVE_LOCKS_REQUIRED(cs); /* Check that all direct conflicts are in a cluster size of two or less. Each * direct conflict may be in a separate cluster. diff --git a/src/util/feefrac.cpp b/src/util/feefrac.cpp index 68fb836936..5b6173835c 100644 --- a/src/util/feefrac.cpp +++ b/src/util/feefrac.cpp @@ -7,39 +7,26 @@ #include #include -std::vector BuildDiagramFromChunks(const Span chunks) -{ - std::vector diagram; - diagram.reserve(chunks.size() + 1); - - diagram.emplace_back(0, 0); - for (auto& chunk : chunks) { - diagram.emplace_back(diagram.back() + chunk); - } - return diagram; -} - -std::partial_ordering CompareFeerateDiagram(Span dia0, Span dia1) +std::partial_ordering CompareChunks(Span chunks0, Span chunks1) { /** Array to allow indexed access to input diagrams. */ - const std::array, 2> dias = {dia0, dia1}; + const std::array, 2> chunk = {chunks0, chunks1}; /** How many elements we have processed in each input. */ - size_t next_index[2] = {1, 1}; + size_t next_index[2] = {0, 0}; + /** Accumulated fee/sizes in diagrams, up to next_index[i] - 1. */ + FeeFrac accum[2]; /** Whether the corresponding input is strictly better than the other at least in one place. */ bool better_somewhere[2] = {false, false}; /** Get the first unprocessed point in diagram number dia. */ - const auto next_point = [&](int dia) { return dias[dia][next_index[dia]]; }; + const auto next_point = [&](int dia) { return chunk[dia][next_index[dia]] + accum[dia]; }; /** Get the last processed point in diagram number dia. */ - const auto prev_point = [&](int dia) { return dias[dia][next_index[dia] - 1]; }; - - // Diagrams should be non-empty, and first elements zero in size and fee - Assert(!dia0.empty() && !dia1.empty()); - Assert(prev_point(0).IsEmpty()); - Assert(prev_point(1).IsEmpty()); + const auto prev_point = [&](int dia) { return accum[dia]; }; + /** Move to the next point in diagram number dia. */ + const auto advance = [&](int dia) { accum[dia] += chunk[dia][next_index[dia]++]; }; do { - bool done_0 = next_index[0] == dias[0].size(); - bool done_1 = next_index[1] == dias[1].size(); + bool done_0 = next_index[0] == chunk[0].size(); + bool done_1 = next_index[1] == chunk[1].size(); if (done_0 && done_1) break; // Determine which diagram has the first unprocessed point. If a single side is finished, use the @@ -69,17 +56,16 @@ std::partial_ordering CompareFeerateDiagram(Span dia0, Span BuildDiagramFromChunks(Span chunks); - -/** Compares two feerate diagrams. The shorter one is implicitly - * extended with a horizontal straight line. +/** Compare the feerate diagrams implied by the provided sorted chunks data. + * + * The implied diagram for each starts at (0, 0), then contains for each chunk the cumulative fee + * and size up to that chunk, and then extends infinitely to the right with a horizontal line. * - * A feerate diagram consists of a list of (fee, size) points with the property that size - * is strictly increasing and that the first entry is (0, 0). + * The caller must guarantee that the sum of the FeeFracs in either of the chunks' data set do not + * overflow (so sum fees < 2^63, and sum sizes < 2^31). */ -std::partial_ordering CompareFeerateDiagram(Span dia0, Span dia1); +std::partial_ordering CompareChunks(Span chunks0, Span chunks1); #endif // BITCOIN_UTIL_FEEFRAC_H From 55b13ecd2e00ad2dbfd44c34d7de6f616590adf8 Mon Sep 17 00:00:00 2001 From: Brandon Odiwuor Date: Thu, 30 Nov 2023 19:53:19 +0300 Subject: [PATCH 55/72] doc: explain what the wallet password does --- doc/managing-wallets.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/doc/managing-wallets.md b/doc/managing-wallets.md index 22e006c963..b99d88877b 100644 --- a/doc/managing-wallets.md +++ b/doc/managing-wallets.md @@ -122,6 +122,22 @@ $ bitcoin-cli -rpcwallet="restored-wallet" getwalletinfo The restored wallet can also be loaded in the GUI via `File` ->`Open wallet`. +## Wallet Passphrase + +Understanding wallet security is crucial for safely storing your Bitcoin. A key aspect is the wallet passphrase, used for encryption. Let's explore its nuances, role, encryption process, and limitations. + +- **Not the Seed:** +The wallet passphrase and the seed are two separate components in wallet security. The seed, or HD seed, functions as a master key for deriving private and public keys in a hierarchical deterministic (HD) wallet. In contrast, the passphrase serves as an additional layer of security specifically designed to secure the private keys within the wallet. The passphrase serves as a safeguard, demanding an additional layer of authentication to access funds in the wallet. + +- **Protection Against Unauthorized Access:** +The passphrase serves as a protective measure, securing your funds in situations where an unauthorized user gains access to your unlocked computer or device while your wallet application is active. Without the passphrase, they would be unable to access your wallet's funds or execute transactions. However, it's essential to be aware that someone with access can potentially compromise the security of your passphrase by installing a keylogger. + +- **Doesn't Encrypt Metadata or Public Keys:** +It's important to note that the passphrase primarily secures the private keys and access to funds within the wallet. It does not encrypt metadata associated with transactions or public keys. Information about your transaction history and the public keys involved may still be visible. + +- **Risk of Fund Loss if Forgotten or Lost:** +If the wallet passphrase is too complex and is subsequently forgotten or lost, there is a risk of losing access to the funds permanently. A forgotten passphrase will result in the inability to unlock the wallet and access the funds. + ## Migrating Legacy Wallets to Descriptor Wallets Legacy wallets (traditional non-descriptor wallets) can be migrated to become Descriptor wallets From 4d8d21320eba54571ff63931509cd515c3e20339 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 11 Apr 2024 16:08:01 +0200 Subject: [PATCH 56/72] sign: don't assume we are parsing a sane Miniscript The script provided for signature might be externally provided, for instance by way of 'finalizepsbt'. Therefore the script might be ill-crafted, so don't assume pubkeys are always 32 bytes. Thanks to Niklas for finding this. --- src/script/sign.cpp | 2 +- src/test/script_tests.cpp | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/script/sign.cpp b/src/script/sign.cpp index be4b357568..22ac062a63 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -295,7 +295,7 @@ struct TapSatisfier: Satisfier { //! Conversion from a raw xonly public key. template std::optional FromPKBytes(I first, I last) const { - CHECK_NONFATAL(last - first == 32); + if (last - first != 32) return {}; XOnlyPubKey pubkey; std::copy(first, last, pubkey.begin()); return pubkey; diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 0af2fdce08..e4142e203c 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1254,6 +1254,30 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) BOOST_CHECK(combined.scriptSig == partial3c); } +/** + * Reproduction of an exception incorrectly raised when parsing a public key inside a TapMiniscript. + */ +BOOST_AUTO_TEST_CASE(sign_invalid_miniscript) +{ + FillableSigningProvider keystore; + SignatureData sig_data; + CMutableTransaction prev, curr; + + // Create a Taproot output which contains a leaf in which a non-32 bytes push is used where a public key is expected + // by the Miniscript parser. This offending Script was found by the RPC fuzzer. + const auto invalid_pubkey{ParseHex("173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292")}; + TaprootBuilder builder; + builder.Add(0, {invalid_pubkey}, 0xc0); + XOnlyPubKey nums{ParseHex("50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0")}; + builder.Finalize(nums); + prev.vout.emplace_back(0, GetScriptForDestination(builder.GetOutput())); + curr.vin.emplace_back(COutPoint{prev.GetHash(), 0}); + sig_data.tr_spenddata = builder.GetSpendData(); + + // SignSignature can fail but it shouldn't raise an exception (nor crash). + BOOST_CHECK(!SignSignature(keystore, CTransaction(prev), curr, 0, SIGHASH_ALL, sig_data)); +} + BOOST_AUTO_TEST_CASE(script_standard_push) { ScriptError err; From 13adbf733f09c73c3cf0025d94c52f9cec5dba3b Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 16 Apr 2024 14:22:30 +0200 Subject: [PATCH 57/72] remove unneeded environment option from cpp-subprocess --- src/util/subprocess.hpp | 147 +--------------------------------------- 1 file changed, 2 insertions(+), 145 deletions(-) diff --git a/src/util/subprocess.hpp b/src/util/subprocess.hpp index 3591c65cc7..7a5ed5f3d6 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.hpp @@ -154,19 +154,6 @@ class OSError: public std::runtime_error {} }; -//-------------------------------------------------------------------- - -//Environment Variable types -#ifndef _MSC_VER - using env_string_t = std::string; - using env_char_t = char; -#else - using env_string_t = std::wstring; - using env_char_t = wchar_t; -#endif -using env_map_t = std::map; -using env_vector_t = std::vector; - //-------------------------------------------------------------------- namespace util { @@ -305,100 +292,6 @@ namespace util if (!SetHandleInformation(*child_handle, HANDLE_FLAG_INHERIT, 0)) throw OSError("SetHandleInformation", 0); } - - // env_map_t MapFromWindowsEnvironment() - // * Imports current Environment in a C-string table - // * Parses the strings by splitting on the first "=" per line - // * Creates a map of the variables - // * Returns the map - inline env_map_t MapFromWindowsEnvironment(){ - wchar_t *variable_strings_ptr; - wchar_t *environment_strings_ptr; - std::wstring delimiter(L"="); - int del_len = delimiter.length(); - env_map_t mapped_environment; - - // Get a pointer to the environment block. - environment_strings_ptr = GetEnvironmentStringsW(); - // If the returned pointer is NULL, exit. - if (environment_strings_ptr == NULL) - { - throw OSError("GetEnvironmentStringsW", 0); - } - - // Variable strings are separated by NULL byte, and the block is - // terminated by a NULL byte. - - variable_strings_ptr = (wchar_t *) environment_strings_ptr; - - //Since the environment map ends with a null, we can loop until we find it. - while (*variable_strings_ptr) - { - // Create a string from Variable String - env_string_t current_line(variable_strings_ptr); - // Find the first "equals" sign. - auto pos = current_line.find(delimiter); - // Assuming it's not missing ... - if(pos!=std::wstring::npos){ - // ... parse the key and value. - env_string_t key = current_line.substr(0, pos); - env_string_t value = current_line.substr(pos + del_len); - // Map the entry. - mapped_environment[key] = value; - } - // Jump to next line in the environment map. - variable_strings_ptr += std::wcslen(variable_strings_ptr) + 1; - } - // We're done with the old environment map buffer. - FreeEnvironmentStringsW(environment_strings_ptr); - - // Return the map. - return mapped_environment; - } - - // env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) - // * Creates a vector buffer for the new environment string table - // * Copies in the mapped variables - // * Returns the vector - inline env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) - { - // Make a new environment map buffer. - env_vector_t environment_map_buffer; - // Give it some space. - environment_map_buffer.reserve(4096); - - // And fill'er up. - for(auto kv: source_map){ - // Create the line - env_string_t current_line(kv.first); current_line += L"="; current_line += kv.second; - // Add the line to the buffer. - std::copy(current_line.begin(), current_line.end(), std::back_inserter(environment_map_buffer)); - // Append a null - environment_map_buffer.push_back(0); - } - // Append one last null because of how Windows does it's environment maps. - environment_map_buffer.push_back(0); - - return environment_map_buffer; - } - - // env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map) - // * Merges host environment with new mapped variables - // * Creates and returns string vector based on map - inline env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map){ - // Import the environment map - env_map_t environment_map = MapFromWindowsEnvironment(); - // Merge in the changes with overwrite - for(auto& it: changes_map) - { - environment_map[it.first] = it.second; - } - // Create a Windows-usable Environment Map Buffer - env_vector_t environment_map_strings_vector = WindowsEnvironmentVectorFromMap(environment_map); - - return environment_map_strings_vector; - } - #endif /*! @@ -656,22 +549,6 @@ struct executable: string_arg executable(T&& arg): string_arg(std::forward(arg)) {} }; -/*! - * Option to specify environment variables required by - * the spawned process. - * - * Eg: environment{{ {"K1", "V1"}, {"K2", "V2"},... }} - */ -struct environment -{ - environment(env_map_t&& env): - env_(std::move(env)) {} - explicit environment(const env_map_t& env): - env_(env) {} - env_map_t env_; -}; - - /*! * Used for redirecting input/output/error */ @@ -887,7 +764,6 @@ struct ArgumentDeducer ArgumentDeducer(Popen* p): popen_(p) {} void set_option(executable&& exe); - void set_option(environment&& env); void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); @@ -1214,7 +1090,6 @@ class Popen #endif std::string exe_name_; - env_map_t env_; // Command in string format std::string args_; @@ -1335,13 +1210,6 @@ inline void Popen::kill(int sig_num) inline void Popen::execute_process() noexcept(false) { #ifdef __USING_WINDOWS__ - void* environment_string_table_ptr = nullptr; - env_vector_t environment_string_vector; - if(this->env_.size()){ - environment_string_vector = util::CreateUpdatedWindowsEnvironmentVector(env_); - environment_string_table_ptr = (void*)environment_string_vector.data(); - } - if (exe_name_.length()) { this->vargs_.insert(this->vargs_.begin(), this->exe_name_); this->populate_c_argv(); @@ -1388,7 +1256,7 @@ inline void Popen::execute_process() noexcept(false) NULL, // primary thread security attributes TRUE, // handles are inherited creation_flags, // creation flags - environment_string_table_ptr, // use provided environment + NULL, // use parent's environment NULL, // use parent's current directory &siStartInfo, // STARTUPINFOW pointer &piProcInfo); // receives PROCESS_INFORMATION @@ -1493,10 +1361,6 @@ namespace detail { popen_->exe_name_ = std::move(exe.arg_value); } - inline void ArgumentDeducer::set_option(environment&& env) { - popen_->env_ = std::move(env.env_); - } - inline void ArgumentDeducer::set_option(input&& inp) { if (inp.rd_ch_ != -1) popen_->stream_.read_from_parent_ = inp.rd_ch_; if (inp.wr_ch_ != -1) popen_->stream_.write_to_child_ = inp.wr_ch_; @@ -1566,14 +1430,7 @@ namespace detail { close(stream.err_write_); // Replace the current image with the executable - if (parent_->env_.size()) { - for (auto& kv : parent_->env_) { - setenv(kv.first.c_str(), kv.second.c_str(), 1); - } - sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); - } else { - sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); - } + sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); if (sys_ret == -1) throw OSError("execve failed", errno); From d8e4ba4d0568769782b8c19c82e955c4aee73477 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 19 Apr 2024 11:02:01 +0100 Subject: [PATCH 58/72] refactor: Rename `subprocess.hpp` to follow our header name conventions --- src/Makefile.am | 2 +- src/common/run_command.cpp | 2 +- src/test/system_tests.cpp | 2 +- src/util/{subprocess.hpp => subprocess.h} | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) rename src/util/{subprocess.hpp => subprocess.h} (99%) diff --git a/src/Makefile.am b/src/Makefile.am index 7673d2f545..f48ed4d442 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -320,7 +320,7 @@ BITCOIN_CORE_H = \ util/sock.h \ util/spanparsing.h \ util/string.h \ - util/subprocess.hpp \ + util/subprocess.h \ util/syserror.h \ util/task_runner.h \ util/thread.h \ diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index e5356490ef..347b486095 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -12,7 +12,7 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER -#include +#include #endif // ENABLE_EXTERNAL_SIGNER UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 8aab2b565c..2de147deea 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -11,7 +11,7 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER -#include +#include #endif // ENABLE_EXTERNAL_SIGNER #include diff --git a/src/util/subprocess.hpp b/src/util/subprocess.h similarity index 99% rename from src/util/subprocess.hpp rename to src/util/subprocess.h index 701eec8c10..4a85ae9268 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.h @@ -33,8 +33,8 @@ Documentation for C++ subprocessing library. @version 1.0.0 */ -#ifndef SUBPROCESS_HPP -#define SUBPROCESS_HPP +#ifndef BITCOIN_UTIL_SUBPROCESS_H +#define BITCOIN_UTIL_SUBPROCESS_H #include #include @@ -1609,4 +1609,4 @@ namespace detail { } -#endif // SUBPROCESS_HPP +#endif // BITCOIN_UTIL_SUBPROCESS_H From 08f756bd370c3e100b186c7e24bef6a033575b29 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 19 Apr 2024 11:19:45 +0100 Subject: [PATCH 59/72] Replace locale-dependent `std::strerror` with `SysErrorString` --- src/util/subprocess.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/subprocess.h b/src/util/subprocess.h index 4a85ae9268..4acfa8ff83 100644 --- a/src/util/subprocess.h +++ b/src/util/subprocess.h @@ -36,6 +36,8 @@ Documentation for C++ subprocessing library. #ifndef BITCOIN_UTIL_SUBPROCESS_H #define BITCOIN_UTIL_SUBPROCESS_H +#include + #include #include #include @@ -150,7 +152,7 @@ class OSError: public std::runtime_error { public: OSError(const std::string& err_msg, int err_code): - std::runtime_error( err_msg + ": " + std::strerror(err_code) ) + std::runtime_error(err_msg + ": " + SysErrorString(err_code)) {} }; From f19f0a2e5af6c2a64900f1f229e21b6f1668bd3d Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Tue, 23 Apr 2024 20:26:42 -0400 Subject: [PATCH 60/72] test: Run framework unit tests in parallel Reorganize functional test framework unit tests to run in parallel with other functional tests. The option `skipunit` is removed, since unit tests no longer delay functional test execution. Unit tests are run by default when running all tests, and can be run explicitly with `feature_framework_unit_tests.py` when running a subset of tests. --- .../feature_framework_unit_tests.py | 50 +++++++++++++++++++ test/functional/test_runner.py | 33 ++---------- 2 files changed, 53 insertions(+), 30 deletions(-) create mode 100755 test/functional/feature_framework_unit_tests.py diff --git a/test/functional/feature_framework_unit_tests.py b/test/functional/feature_framework_unit_tests.py new file mode 100755 index 0000000000..c9754e083c --- /dev/null +++ b/test/functional/feature_framework_unit_tests.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Framework unit tests + +Unit tests for the test framework. +""" + +import sys +import unittest + +from test_framework.test_framework import TEST_EXIT_PASSED, TEST_EXIT_FAILED + +# List of framework modules containing unit tests. Should be kept in sync with +# the output of `git grep unittest.TestCase ./test/functional/test_framework` +TEST_FRAMEWORK_MODULES = [ + "address", + "crypto.bip324_cipher", + "blocktools", + "crypto.chacha20", + "crypto.ellswift", + "key", + "messages", + "crypto.muhash", + "crypto.poly1305", + "crypto.ripemd160", + "script", + "segwit_addr", + "wallet_util", +] + + +def run_unit_tests(): + test_framework_tests = unittest.TestSuite() + for module in TEST_FRAMEWORK_MODULES: + test_framework_tests.addTest( + unittest.TestLoader().loadTestsFromName(f"test_framework.{module}") + ) + result = unittest.TextTestRunner(stream=sys.stdout, verbosity=1, failfast=True).run( + test_framework_tests + ) + if not result.wasSuccessful(): + sys.exit(TEST_EXIT_FAILED) + sys.exit(TEST_EXIT_PASSED) + + +if __name__ == "__main__": + run_unit_tests() + diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 32b55813a8..f6eccab2b3 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -26,7 +26,6 @@ import tempfile import re import logging -import unittest os.environ["REQUIRE_WALLET_TYPE_SET"] = "1" @@ -70,23 +69,7 @@ TEST_EXIT_PASSED = 0 TEST_EXIT_SKIPPED = 77 -# List of framework modules containing unit tests. Should be kept in sync with -# the output of `git grep unittest.TestCase ./test/functional/test_framework` -TEST_FRAMEWORK_MODULES = [ - "address", - "crypto.bip324_cipher", - "blocktools", - "crypto.chacha20", - "crypto.ellswift", - "key", - "messages", - "crypto.muhash", - "crypto.poly1305", - "crypto.ripemd160", - "script", - "segwit_addr", - "wallet_util", -] +TEST_FRAMEWORK_UNIT_TESTS = 'feature_framework_unit_tests.py' EXTENDED_SCRIPTS = [ # These tests are not run by default. @@ -255,6 +238,7 @@ 'wallet_keypool.py --descriptors', 'wallet_descriptor.py --descriptors', 'p2p_nobloomfilter_messages.py', + TEST_FRAMEWORK_UNIT_TESTS, 'p2p_filter.py', 'rpc_setban.py --v1transport', 'rpc_setban.py --v2transport', @@ -440,7 +424,6 @@ def main(): parser.add_argument('--tmpdirprefix', '-t', default=tempfile.gettempdir(), help="Root directory for datadirs") parser.add_argument('--failfast', '-F', action='store_true', help='stop execution after the first test failure') parser.add_argument('--filter', help='filter scripts to run by regular expression') - parser.add_argument('--skipunit', '-u', action='store_true', help='skip unit tests for the test framework') args, unknown_args = parser.parse_known_args() @@ -552,10 +535,9 @@ def main(): combined_logs_len=args.combinedlogslen, failfast=args.failfast, use_term_control=args.ansi, - skipunit=args.skipunit, ) -def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, use_term_control, skipunit=False): +def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=False, args=None, combined_logs_len=0, failfast=False, use_term_control): args = args or [] # Warn if bitcoind is already running @@ -578,15 +560,6 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage= # a hard link or a copy on any platform. See https://github.com/bitcoin/bitcoin/pull/27561. sys.path.append(tests_dir) - if not skipunit: - print("Running Unit Tests for Test Framework Modules") - test_framework_tests = unittest.TestSuite() - for module in TEST_FRAMEWORK_MODULES: - test_framework_tests.addTest(unittest.TestLoader().loadTestsFromName("test_framework.{}".format(module))) - result = unittest.TextTestRunner(verbosity=1, failfast=True).run(test_framework_tests) - if not result.wasSuccessful(): - sys.exit("Early exiting after failure in TestFramework unit tests") - flags = ['--cachedir={}'.format(cache_dir)] + args if enable_coverage: From 970cbc3172b73e1faf6bdb429400d3497cbb9d33 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 23 Apr 2024 23:56:18 +0100 Subject: [PATCH 61/72] doc: Suggest only necessary Qt packages for installation on OpenBSD The currently suggested `qt5` installs many unneeded dependencies, for example, `qtsensors`, `qtspeech` etc. --- doc/build-openbsd.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/build-openbsd.md b/doc/build-openbsd.md index 7ed83853a8..264d5f2d08 100644 --- a/doc/build-openbsd.md +++ b/doc/build-openbsd.md @@ -1,6 +1,6 @@ # OpenBSD Build Guide -**Updated for OpenBSD [7.4](https://www.openbsd.org/74.html)** +**Updated for OpenBSD [7.5](https://www.openbsd.org/75.html)** This guide describes how to build bitcoind, command-line utilities, and GUI on OpenBSD. @@ -63,7 +63,7 @@ export BDB_PREFIX="/path/to/bitcoin/depends/x86_64-unknown-openbsd" Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, Qt 5 is required. ```bash -pkg_add qt5 +pkg_add qtbase qttools ``` ## Building Bitcoin Core From dace02f99d4a8785567732c0d687517175765bfd Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Mon, 22 Apr 2024 10:59:53 +0200 Subject: [PATCH 62/72] doc: suggest only necessary Qt packages for installation on FreeBSD The previously suggested `qt5` package is a meta package that does not install anything itself but depends on a bunch of others and is used as a convenience to install "everything" Qt5 related: 270 packages / 3 GiB. We only need a subset of those which amounts to 79 packages / 381 MiB, so suggest just that. For comparison: ``` pkg install qt5 Updating local repository catalogue... local repository is up to date. All repositories are up to date. Checking integrity... done (0 conflicting) The following 270 package(s) will be affected (of 0 checked): New packages to be INSTALLED: Imath: 3.1.11 abseil: 20230125.3 alsa-lib: 1.2.11 alsa-plugins: 1.2.7.1_3 aom: 3.8.2 assimp: 5.4.0 avahi-app: 0.8_2 brotli: 1.1.0,1 consolekit2: 1.2.6_3 cups: 2.4.7_2 curl: 8.7.1 cyrus-sasl: 2.1.28_4 dav1d: 1.4.1 dbus: 1.14.10_5,1 dbus-glib: 0.112_1 dejavu: 2.37_3 dotconf: 1.3_1 double-conversion: 3.3.0 duktape-lib: 2.7.0 encodings: 1.1.0,1 espeak-ng: 1.51.1_5 expat: 2.6.2 ffmpeg: 6.1.1_5,1 fftw3: 3.3.10_5 fftw3-float: 3.3.10_5 flac: 1.4.3_1,1 font-bh-ttf: 1.0.3_5 font-misc-ethiopic: 1.0.4 font-misc-meltho: 1.0.3_5 fontconfig: 2.15.0_2,1 freetds: 1.4.12,1 freetype2: 2.13.2 fribidi: 1.0.13_1 gdbm: 1.23 gdk-pixbuf2: 2.42.10_2 gettext-runtime: 0.22.5 giflib: 5.2.1_1 glib: 2.80.0,2 gmp: 6.3.0 gnome_subr: 1.0 gnutls: 3.8.5_1 graphite2: 1.3.14 groff: 1.23.0_3 gstreamer1: 1.22.10 gstreamer1-plugins: 1.22.10_1 gstreamer1-plugins-bad: 1.22.10_2 harfbuzz: 8.4.0 hicolor-icon-theme: 0.17 hidapi: 0.14.0 highway: 1.1.0 hunspell: 1.7.2_1 icu: 74.2_1,1 indexinfo: 0.3.1 iso-codes: 4.15.0 jasper: 4.2.3 jbigkit: 2.1_2 jpeg-turbo: 3.0.2 jsoncpp: 1.9.5 lame: 3.100_5 lcms2: 2.16_1 libICE: 1.1.0_2,1 libSM: 1.2.3_1,1 libX11: 1.8.9,1 libXScrnSaver: 1.2.4_1 libXau: 1.0.9_1 libXcomposite: 0.4.6_1,1 libXcursor: 1.2.2 libXdamage: 1.1.6 libXdmcp: 1.1.5 libXext: 1.3.6,1 libXfixes: 6.0.0_1 libXi: 1.8_1,1 libXmu: 1.1.4,1 libXrandr: 1.5.2_1 libXrender: 0.9.10_2 libXt: 1.3.0,1 libXtst: 1.2.3_3 libXv: 1.0.12_1,1 libass: 0.17.1_2 libcbor: 0.11.0 libcjson: 1.7.17 libdaemon: 0.14_1 libdeflate: 1.20 libdrm: 2.4.120_1,1 libedit: 3.1.20230828_1,1 libepoll-shim: 0.0.20230411 libevdev: 1.13.1 libevent: 2.1.12 libffi: 3.4.4_1 libfido2: 1.14.0 libfontenc: 1.1.8 libgcrypt: 1.10.3_1 libglvnd: 1.7.0 libgpg-error: 1.48 libgudev: 237 libiconv: 1.17_1 libidn2: 2.3.7 libinput: 1.25.0 libjxl: 0.10.2 libltdl: 2.4.7 liblz4: 1.9.4_1,1 libmng: 2.0.3_1 libmtdev: 1.1.6_1 libmysofa: 1.3.2 libnghttp2: 1.61.0 libnice: 0.1.21_2 libogg: 1.3.5,4 libpaper: 1.1.28_1 libpci: 3.12.0 libpciaccess: 0.18 libplacebo: 6.338.2 libpsl: 0.21.5 libsndfile: 1.2.2_1 libsoxr: 0.1.3_3 libssh2: 1.11.0_1,3 libtasn1: 4.19.0_1 libudev-devd: 0.5.2 libunibreak: 6.1,1 libunistring: 1.2 libunwind: 20240221 libv4l: 1.23.0_4 libva: 2.21.0 libvdpau: 1.5 libvorbis: 1.3.7_2,3 libvpx: 1.14.0 libwacom: 1.5_1 libx264: 0.164.3095 libxcb: 1.17.0 libxkbcommon: 1.6.0_2 libxkbfile: 1.1.3 libxml2: 2.11.7 libxslt: 1.1.37_1 llvm15: 15.0.7_10 lua53: 5.3.6_1 minizip: 1.2.11_1 mkfontscale: 1.2.3 mpdecimal: 4.0.0 mpg123: 1.32.5 mysql80-client: 8.0.35 nettle: 3.9.1 nspr: 4.35 nss: 3.99 openal-soft: 1.21.1_4 openexr: 3.2.4 openh264: 2.3.0,2 openldap26-client: 2.6.7 opus: 1.5.2 orc: 0.4.36 p11-kit: 0.25.3_2 pcaudiolib: 1.2_1 pciids: 20240331 pcre2: 10.43 perl5: 5.36.3_1 png: 1.6.43 polkit: 124_3 postgresql15-client: 15.6 psutils: 1.17_6 pulseaudio: 16.1_4 py39-evdev: 1.6.0 py39-packaging: 24.0 py39-pyudev: 0.22.0 py39-setuptools: 63.1.0_1 py39-six: 1.16.0 python39: 3.9.18_2 qt5: 5.15.13 qt5-3d: 5.15.13p0 qt5-assistant: 5.15.13p4 qt5-buildtools: 5.15.13p142 qt5-charts: 5.15.13p0 qt5-concurrent: 5.15.13p142 qt5-connectivity: 5.15.13p4 qt5-core: 5.15.13p142 qt5-datavis3d: 5.15.13p0 qt5-dbus: 5.15.13p142 qt5-declarative: 5.15.13p30 qt5-declarative-test: 5.15.13p30 qt5-designer: 5.15.13p4 qt5-doc: 5.12.2 qt5-examples: 5.15.13 qt5-gamepad: 5.15.13p0 qt5-graphicaleffects: 5.15.13p0 qt5-gui: 5.15.13p142 qt5-help: 5.15.13p4 qt5-imageformats: 5.15.13p7 qt5-l10n: 5.15.13p0 qt5-linguist: 5.15.13p4 qt5-linguisttools: 5.15.13p4 qt5-location: 5.15.13p6 qt5-multimedia: 5.15.13p2 qt5-network: 5.15.13p142 qt5-networkauth: 5.15.13p0 qt5-opengl: 5.15.13p142 qt5-pixeltool: 5.15.13p4 qt5-printsupport: 5.15.13p142 qt5-qdbus: 5.15.13p4 qt5-qdbusviewer: 5.15.13p4 qt5-qdoc: 5.15.13p4 qt5-qdoc-data: 5.15.13 qt5-qev: 5.15.13p4 qt5-qmake: 5.15.13p142 qt5-qtdiag: 5.15.13p4 qt5-qtpaths: 5.15.13p4 qt5-qtplugininfo: 5.15.13p4 qt5-quick3d: 5.15.13p1 qt5-quickcontrols: 5.15.13p0 qt5-quickcontrols2: 5.15.13p5 qt5-quicktimeline: 5.15.13p0 qt5-remoteobjects: 5.15.13p0 qt5-script: 5.15.16p0_2 qt5-scripttools: 5.15.16p0_1 qt5-scxml: 5.15.13p0 qt5-sensors: 5.15.13p0 qt5-serialbus: 5.15.13p0 qt5-serialport: 5.15.13p0 qt5-speech: 5.15.13p1 qt5-sql: 5.15.13p142 qt5-sqldrivers-mysql: 5.15.13p142 qt5-sqldrivers-odbc: 5.15.13p142 qt5-sqldrivers-pgsql: 5.15.13p142 qt5-sqldrivers-sqlite2: 5.15.13p142 qt5-sqldrivers-sqlite3: 5.15.13p142 qt5-sqldrivers-tds: 5.15.13p142 qt5-svg: 5.15.13p6 qt5-testlib: 5.15.13p142 qt5-uiplugin: 5.15.13p4 qt5-uitools: 5.15.13p4 qt5-virtualkeyboard: 5.15.13p0 qt5-webchannel: 5.15.13p3 qt5-webengine: 5.15.16.p9 qt5-webglplugin: 5.15.13p0 qt5-websockets: 5.15.13p2 qt5-websockets-qml: 5.15.13p2 qt5-webview: 5.15.13p0 qt5-widgets: 5.15.13p142 qt5-x11extras: 5.15.13p0 qt5-xml: 5.15.13p142 qt5-xmlpatterns: 5.15.13p0 re2: 20240401 readline: 8.2.10 shaderc: 2024.0 shared-mime-info: 2.2_2 snappy: 1.2.0 speech-dispatcher: 0.11.2_4 speexdsp: 1.2.1 sqlite: 2.8.17_5 sqlite3: 3.45.1,1 svt-av1: 2.0.0 tiff: 4.4.0_3 uchardet: 0.0.8_1 unixODBC: 2.3.12_1 vmaf: 3.0.0 vulkan-headers: 1.3.283 vulkan-loader: 1.3.283 wayland: 1.22.0 webp: 1.4.0 webrtc-audio-processing0: 0.3.1_3 x265: 3.5_1 xcb-util: 0.4.1,1 xcb-util-image: 0.4.1 xcb-util-keysyms: 0.4.1 xcb-util-renderutil: 0.3.10 xcb-util-wm: 0.4.2 xdg-utils: 1.1.3_4 xkeyboard-config: 2.41_4 xorg-fonts-truetype: 7.7_1 xorgproto: 2023.2 xprop: 1.2.7 xset: 1.2.5_1 xxhash: 0.8.2_1 zstd: 1.5.6 Number of packages to be installed: 270 The process will require 3 GiB more space. Proceed with this action? [y/N]: ``` ``` pkg install qt5-buildtools qt5-core qt5-gui qt5-linguisttools qt5-testlib qt5-widgets Updating local repository catalogue... local repository is up to date. All repositories are up to date. Checking integrity... done (0 conflicting) The following 79 package(s) will be affected (of 0 checked): New packages to be INSTALLED: brotli: 1.1.0,1 dbus: 1.14.10_5,1 dejavu: 2.37_3 double-conversion: 3.3.0 encodings: 1.1.0,1 expat: 2.6.2 font-bh-ttf: 1.0.3_5 font-misc-ethiopic: 1.0.4 font-misc-meltho: 1.0.3_5 fontconfig: 2.15.0_2,1 freetype2: 2.13.2 gettext-runtime: 0.22.5 glib: 2.80.0,2 graphite2: 1.3.14 harfbuzz: 8.4.0 hicolor-icon-theme: 0.17 icu: 74.2_1,1 indexinfo: 0.3.1 jpeg-turbo: 3.0.2 libICE: 1.1.0_2,1 libSM: 1.2.3_1,1 libX11: 1.8.9,1 libXau: 1.0.9_1 libXdmcp: 1.1.5 libXext: 1.3.6,1 libXfixes: 6.0.0_1 libXi: 1.8_1,1 libXmu: 1.1.4,1 libXrender: 0.9.10_2 libXt: 1.3.0,1 libepoll-shim: 0.0.20230411 libevdev: 1.13.1 libffi: 3.4.4_1 libfontenc: 1.1.8 libglvnd: 1.7.0 libgudev: 237 libiconv: 1.17_1 libinput: 1.25.0 liblz4: 1.9.4_1,1 libmtdev: 1.1.6_1 libudev-devd: 0.5.2 libwacom: 1.5_1 libxcb: 1.17.0 libxkbcommon: 1.6.0_2 libxml2: 2.11.7 mkfontscale: 1.2.3 mpdecimal: 4.0.0 pcre2: 10.43 png: 1.6.43 py39-evdev: 1.6.0 py39-packaging: 24.0 py39-pyudev: 0.22.0 py39-setuptools: 63.1.0_1 py39-six: 1.16.0 python39: 3.9.18_2 qt5-buildtools: 5.15.13p142 qt5-core: 5.15.13p142 qt5-dbus: 5.15.13p142 qt5-gui: 5.15.13p142 qt5-linguisttools: 5.15.13p4 qt5-network: 5.15.13p142 qt5-testlib: 5.15.13p142 qt5-widgets: 5.15.13p142 qt5-xml: 5.15.13p142 readline: 8.2.10 vulkan-headers: 1.3.283 wayland: 1.22.0 xcb-util: 0.4.1,1 xcb-util-image: 0.4.1 xcb-util-keysyms: 0.4.1 xcb-util-renderutil: 0.3.10 xcb-util-wm: 0.4.2 xdg-utils: 1.1.3_4 xkeyboard-config: 2.41_4 xorg-fonts-truetype: 7.7_1 xorgproto: 2023.2 xprop: 1.2.7 xset: 1.2.5_1 zstd: 1.5.6 Number of packages to be installed: 79 The process will require 381 MiB more space. Proceed with this action? [y/N]: ``` --- doc/build-freebsd.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/build-freebsd.md b/doc/build-freebsd.md index aa10e4a891..bf86a0ee4b 100644 --- a/doc/build-freebsd.md +++ b/doc/build-freebsd.md @@ -1,6 +1,6 @@ # FreeBSD Build Guide -**Updated for FreeBSD [12.3](https://www.freebsd.org/releases/12.3R/announce/)** +**Updated for FreeBSD [14.0](https://www.freebsd.org/releases/14.0R/announce/)** This guide describes how to build bitcoind, command-line utilities, and GUI on FreeBSD. @@ -64,10 +64,11 @@ sh/bash: export BDB_PREFIX=[path displayed above] #### GUI Dependencies ###### Qt5 -Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, we need to install `qt5`. Skip if you don't intend to use the GUI. +Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, we need to install the necessary parts of Qt. Skip if you don't intend to use the GUI. ```bash -pkg install qt5 +pkg install qt5-buildtools qt5-core qt5-gui qt5-linguisttools qt5-testlib qt5-widgets ``` + ###### libqrencode The GUI can encode addresses in a QR Code. To build in QR support for the GUI, install `libqrencode`. Skip if not using the GUI or don't want QR code functionality. From 9381052194a78024b3994cc6ad906858c477b88f Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 24 Apr 2024 15:22:12 +0100 Subject: [PATCH 63/72] doc: Bash is needed in gen_id and is not installed on FreeBSD by default --- depends/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/depends/README.md b/depends/README.md index a8dfc83e3b..10e0985cf4 100644 --- a/depends/README.md +++ b/depends/README.md @@ -85,6 +85,10 @@ For linux S390X cross compilation: sudo apt-get install g++-s390x-linux-gnu binutils-s390x-linux-gnu +### Install the required dependencies: FreeBSD + + pkg install bash + ### Install the required dependencies: OpenBSD pkg_add bash gtar From 46bc6c2aaa613eef526b21a06bf21e8edde31a88 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sat, 20 Apr 2024 16:39:16 +0200 Subject: [PATCH 64/72] test: Add unit tests for urlDecode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lőrinc --- src/Makefile.test.include | 1 + src/test/common_url_tests.cpp | 70 +++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 src/test/common_url_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index cf88a02b95..942e0bf69b 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -85,6 +85,7 @@ BITCOIN_TESTS =\ test/checkqueue_tests.cpp \ test/coins_tests.cpp \ test/coinstatsindex_tests.cpp \ + test/common_url_tests.cpp \ test/compilerbug_tests.cpp \ test/compress_tests.cpp \ test/crypto_tests.cpp \ diff --git a/src/test/common_url_tests.cpp b/src/test/common_url_tests.cpp new file mode 100644 index 0000000000..88bc17f22a --- /dev/null +++ b/src/test/common_url_tests.cpp @@ -0,0 +1,70 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://opensource.org/license/mit/. + +#include + +#include + +#include + +BOOST_AUTO_TEST_SUITE(common_url_tests) + +// These test vectors were ported from test/regress.c in the libevent library +// which used to be a dependency of the urlDecode function. + +BOOST_AUTO_TEST_CASE(encode_decode_test) { + BOOST_CHECK_EQUAL(urlDecode("Hello"), "Hello"); + BOOST_CHECK_EQUAL(urlDecode("99"), "99"); + BOOST_CHECK_EQUAL(urlDecode(""), ""); + BOOST_CHECK_EQUAL(urlDecode("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"), + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"); + BOOST_CHECK_EQUAL(urlDecode("%20"), " "); + BOOST_CHECK_EQUAL(urlDecode("%FF%F0%E0"), "\xff\xf0\xe0"); + BOOST_CHECK_EQUAL(urlDecode("%01%19"), "\x01\x19"); + BOOST_CHECK_EQUAL(urlDecode("http%3A%2F%2Fwww.ietf.org%2Frfc%2Frfc3986.txt"), + "http://www.ietf.org/rfc/rfc3986.txt"); + BOOST_CHECK_EQUAL(urlDecode("1%2B2%3D3"), "1+2=3"); +} + +BOOST_AUTO_TEST_CASE(decode_malformed_test) { + BOOST_CHECK_EQUAL(urlDecode("%%xhello th+ere \xff"), "%%xhello th+ere \xff"); + + BOOST_CHECK_EQUAL(urlDecode("%"), "%"); + BOOST_CHECK_EQUAL(urlDecode("%%"), "%%"); + BOOST_CHECK_EQUAL(urlDecode("%%%"), "%%%"); + BOOST_CHECK_EQUAL(urlDecode("%%%%"), "%%%%"); + + BOOST_CHECK_EQUAL(urlDecode("+"), "+"); + BOOST_CHECK_EQUAL(urlDecode("++"), "++"); + + BOOST_CHECK_EQUAL(urlDecode("?"), "?"); + BOOST_CHECK_EQUAL(urlDecode("??"), "??"); + + BOOST_CHECK_EQUAL(urlDecode("%G1"), "%G1"); + BOOST_CHECK_EQUAL(urlDecode("%2"), "%2"); + BOOST_CHECK_EQUAL(urlDecode("%ZX"), "%ZX"); + + BOOST_CHECK_EQUAL(urlDecode("valid%20string%G1"), "valid string%G1"); + BOOST_CHECK_EQUAL(urlDecode("%20invalid%ZX"), " invalid%ZX"); + BOOST_CHECK_EQUAL(urlDecode("%20%G1%ZX"), " %G1%ZX"); + + BOOST_CHECK_EQUAL(urlDecode("%1 "), "%1 "); + BOOST_CHECK_EQUAL(urlDecode("% 9"), "% 9"); + BOOST_CHECK_EQUAL(urlDecode(" %Z "), " %Z "); + BOOST_CHECK_EQUAL(urlDecode(" % X"), " % X"); + + BOOST_CHECK_EQUAL(urlDecode("%-1"), "%-1"); + BOOST_CHECK_EQUAL(urlDecode("%1-"), "%1-"); +} + +BOOST_AUTO_TEST_CASE(decode_lowercase_hex_test) { + BOOST_CHECK_EQUAL(urlDecode("%f0%a0%b0"), "\xf0\xa0\xb0"); +} + +BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) { + BOOST_CHECK_EQUAL(urlDecode("%00%00x%00%00"), ""); + BOOST_CHECK_EQUAL(urlDecode("abc%00%00"), "abc"); +} + +BOOST_AUTO_TEST_SUITE_END() From 650d43ec15f7a3ae38126f65ef8fa0b1fd3ee936 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sat, 20 Apr 2024 16:35:39 +0200 Subject: [PATCH 65/72] refactor: Replace libevent use in urlDecode with our own code --- configure.ac | 1 - src/Makefile.am | 6 +----- src/common/url.cpp | 40 +++++++++++++++++++++++++++++++--------- src/common/url.h | 3 ++- src/wallet/rpc/util.cpp | 5 +++-- 5 files changed, 37 insertions(+), 18 deletions(-) diff --git a/configure.ac b/configure.ac index febb352cdb..494cb3ce50 100644 --- a/configure.ac +++ b/configure.ac @@ -1706,7 +1706,6 @@ AM_CONDITIONAL([ENABLE_QT_TESTS], [test "$BUILD_TEST_QT" = "yes"]) AM_CONDITIONAL([ENABLE_BENCH], [test "$use_bench" = "yes"]) AM_CONDITIONAL([USE_QRCODE], [test "$use_qr" = "yes"]) AM_CONDITIONAL([USE_LCOV], [test "$use_lcov" = "yes"]) -AM_CONDITIONAL([USE_LIBEVENT], [test "$use_libevent" = "yes"]) AM_CONDITIONAL([HARDEN], [test "$use_hardening" = "yes"]) AM_CONDITIONAL([ENABLE_SSE42], [test "$enable_sse42" = "yes"]) AM_CONDITIONAL([ENABLE_SSE41], [test "$enable_sse41" = "yes"]) diff --git a/src/Makefile.am b/src/Makefile.am index 7673d2f545..7387eb1eaa 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -679,6 +679,7 @@ libbitcoin_common_a_SOURCES = \ common/run_command.cpp \ common/settings.cpp \ common/system.cpp \ + common/url.cpp \ compressor.cpp \ core_read.cpp \ core_write.cpp \ @@ -711,11 +712,6 @@ libbitcoin_common_a_SOURCES = \ script/solver.cpp \ warnings.cpp \ $(BITCOIN_CORE_H) - -if USE_LIBEVENT -libbitcoin_common_a_CPPFLAGS += $(EVENT_CFLAGS) -libbitcoin_common_a_SOURCES += common/url.cpp -endif # # util # diff --git a/src/common/url.cpp b/src/common/url.cpp index 053e1a825c..d0bf4e0cbe 100644 --- a/src/common/url.cpp +++ b/src/common/url.cpp @@ -4,19 +4,41 @@ #include -#include - -#include +#include #include +#include +#include -std::string urlDecode(const std::string &urlEncoded) { +std::string urlDecode(std::string_view urlEncoded) +{ std::string res; - if (!urlEncoded.empty()) { - char *decoded = evhttp_uridecode(urlEncoded.c_str(), false, nullptr); - if (decoded) { - res = std::string(decoded); - free(decoded); + res.reserve(urlEncoded.size()); + + for (size_t i = 0; i < urlEncoded.size(); ++i) { + char c = urlEncoded[i]; + // Special handling for percent which should be followed by two hex digits + // representing an octet values, see RFC 3986, Section 2.1 Percent-Encoding + if (c == '%' && i + 2 < urlEncoded.size()) { + unsigned int decoded_value{0}; + auto [p, ec] = std::from_chars(urlEncoded.data() + i + 1, urlEncoded.data() + i + 3, decoded_value, 16); + + // Only if there is no error and the pointer is set to the end of + // the string, we can be sure both characters were valid hex + if (ec == std::errc{} && p == urlEncoded.data() + i + 3) { + // A null character terminates the string + if (decoded_value == 0) { + return res; + } + + res += static_cast(decoded_value); + // Next two characters are part of the percent encoding + i += 2; + continue; + } + // In case of invalid percent encoding, add the '%' and continue } + res += c; } + return res; } diff --git a/src/common/url.h b/src/common/url.h index b16b8241af..7b0a944776 100644 --- a/src/common/url.h +++ b/src/common/url.h @@ -6,8 +6,9 @@ #define BITCOIN_COMMON_URL_H #include +#include -using UrlDecodeFn = std::string(const std::string& url_encoded); +using UrlDecodeFn = std::string(std::string_view url_encoded); UrlDecodeFn urlDecode; extern UrlDecodeFn* const URL_DECODE; diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 06ec7db1bc..f44ee7696c 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -61,9 +62,9 @@ bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wal bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) { - if (URL_DECODE && request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) { + if (URL_DECODE && request.URI.starts_with(WALLET_ENDPOINT_BASE)) { // wallet endpoint was used - wallet_name = URL_DECODE(request.URI.substr(WALLET_ENDPOINT_BASE.size())); + wallet_name = URL_DECODE(std::string_view{request.URI}.substr(WALLET_ENDPOINT_BASE.size())); return true; } return false; From 8f39aaae417c33490e0e41fb97620eb23ced3d05 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sat, 20 Apr 2024 16:49:02 +0200 Subject: [PATCH 66/72] refactor: Remove hooking code for urlDecode The point of this was to be able to build bitcoin-tx and bitcoin-wallet without libevent, see #18504. Now that we use our own implementation of urlDecode this is not needed anymore. Co-authored-by: stickies-v --- src/bitcoin-cli.cpp | 2 -- src/bitcoin-wallet.cpp | 2 -- src/bitcoind.cpp | 2 -- src/common/url.h | 8 +++++--- src/qt/main.cpp | 2 -- src/test/util/setup_common.cpp | 2 -- src/wallet/rpc/util.cpp | 4 ++-- 7 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 129deeec60..830171f63a 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -51,7 +50,6 @@ using CliClock = std::chrono::system_clock; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = urlDecode; static const char DEFAULT_RPCCONNECT[] = "127.0.0.1"; static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index d5dfbbec27..fe90958a5f 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -28,7 +27,6 @@ #include const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = nullptr; static void SetupWalletToolArgs(ArgsManager& argsman) { diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 4f0a816388..54796c5abb 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include @@ -35,7 +34,6 @@ using node::NodeContext; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = urlDecode; #if HAVE_DECL_FORK diff --git a/src/common/url.h b/src/common/url.h index 7b0a944776..42e982fb94 100644 --- a/src/common/url.h +++ b/src/common/url.h @@ -8,8 +8,10 @@ #include #include -using UrlDecodeFn = std::string(std::string_view url_encoded); -UrlDecodeFn urlDecode; -extern UrlDecodeFn* const URL_DECODE; +/* Decode a URL. + * + * Notably this implementation does not decode a '+' to a ' '. + */ +std::string urlDecode(std::string_view url_encoded); #endif // BITCOIN_COMMON_URL_H diff --git a/src/qt/main.cpp b/src/qt/main.cpp index ded057dbfa..16befd99e8 100644 --- a/src/qt/main.cpp +++ b/src/qt/main.cpp @@ -4,7 +4,6 @@ #include -#include #include #include @@ -17,7 +16,6 @@ extern const std::function G_TRANSLATION_FUN = [](const char* psz) { return QCoreApplication::translate("bitcoin-core", psz).toStdString(); }; -UrlDecodeFn* const URL_DECODE = urlDecode; const std::function G_TEST_GET_FULL_NAME{}; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2c18184261..38350b33cc 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -81,7 +80,6 @@ using node::RegenerateCommitments; using node::VerifyLoadedChainstate; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = nullptr; /** Random context to get unique temp data dirs. Separate from g_insecure_rand_ctx, which can be seeded from a const env var */ static FastRandomContext g_insecure_rand_ctx_temp_path; diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index f44ee7696c..eb081e765a 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -62,9 +62,9 @@ bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wal bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) { - if (URL_DECODE && request.URI.starts_with(WALLET_ENDPOINT_BASE)) { + if (request.URI.starts_with(WALLET_ENDPOINT_BASE)) { // wallet endpoint was used - wallet_name = URL_DECODE(std::string_view{request.URI}.substr(WALLET_ENDPOINT_BASE.size())); + wallet_name = urlDecode(std::string_view{request.URI}.substr(WALLET_ENDPOINT_BASE.size())); return true; } return false; From 099fa571511f113e0056d4bc27b3153a42f9dc65 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sat, 20 Apr 2024 17:05:18 +0200 Subject: [PATCH 67/72] scripted-diff: Modernize name of urlDecode function and param -BEGIN VERIFY SCRIPT- sed -i 's/urlDecode/UrlDecode/g' $(git grep -l 'urlDecode' ./src) sed -i 's/urlEncoded/url_encoded/g' $(git grep -l 'urlEncoded' ./src) -END VERIFY SCRIPT- --- src/common/url.cpp | 14 ++++---- src/common/url.h | 2 +- src/test/common_url_tests.cpp | 68 +++++++++++++++++------------------ src/test/fuzz/string.cpp | 2 +- src/wallet/rpc/util.cpp | 2 +- 5 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/common/url.cpp b/src/common/url.cpp index d0bf4e0cbe..f27bb2b73f 100644 --- a/src/common/url.cpp +++ b/src/common/url.cpp @@ -9,22 +9,22 @@ #include #include -std::string urlDecode(std::string_view urlEncoded) +std::string UrlDecode(std::string_view url_encoded) { std::string res; - res.reserve(urlEncoded.size()); + res.reserve(url_encoded.size()); - for (size_t i = 0; i < urlEncoded.size(); ++i) { - char c = urlEncoded[i]; + for (size_t i = 0; i < url_encoded.size(); ++i) { + char c = url_encoded[i]; // Special handling for percent which should be followed by two hex digits // representing an octet values, see RFC 3986, Section 2.1 Percent-Encoding - if (c == '%' && i + 2 < urlEncoded.size()) { + if (c == '%' && i + 2 < url_encoded.size()) { unsigned int decoded_value{0}; - auto [p, ec] = std::from_chars(urlEncoded.data() + i + 1, urlEncoded.data() + i + 3, decoded_value, 16); + auto [p, ec] = std::from_chars(url_encoded.data() + i + 1, url_encoded.data() + i + 3, decoded_value, 16); // Only if there is no error and the pointer is set to the end of // the string, we can be sure both characters were valid hex - if (ec == std::errc{} && p == urlEncoded.data() + i + 3) { + if (ec == std::errc{} && p == url_encoded.data() + i + 3) { // A null character terminates the string if (decoded_value == 0) { return res; diff --git a/src/common/url.h b/src/common/url.h index 42e982fb94..203f41c70f 100644 --- a/src/common/url.h +++ b/src/common/url.h @@ -12,6 +12,6 @@ * * Notably this implementation does not decode a '+' to a ' '. */ -std::string urlDecode(std::string_view url_encoded); +std::string UrlDecode(std::string_view url_encoded); #endif // BITCOIN_COMMON_URL_H diff --git a/src/test/common_url_tests.cpp b/src/test/common_url_tests.cpp index 88bc17f22a..eb92c2ceee 100644 --- a/src/test/common_url_tests.cpp +++ b/src/test/common_url_tests.cpp @@ -11,60 +11,60 @@ BOOST_AUTO_TEST_SUITE(common_url_tests) // These test vectors were ported from test/regress.c in the libevent library -// which used to be a dependency of the urlDecode function. +// which used to be a dependency of the UrlDecode function. BOOST_AUTO_TEST_CASE(encode_decode_test) { - BOOST_CHECK_EQUAL(urlDecode("Hello"), "Hello"); - BOOST_CHECK_EQUAL(urlDecode("99"), "99"); - BOOST_CHECK_EQUAL(urlDecode(""), ""); - BOOST_CHECK_EQUAL(urlDecode("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"), + BOOST_CHECK_EQUAL(UrlDecode("Hello"), "Hello"); + BOOST_CHECK_EQUAL(UrlDecode("99"), "99"); + BOOST_CHECK_EQUAL(UrlDecode(""), ""); + BOOST_CHECK_EQUAL(UrlDecode("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"), "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"); - BOOST_CHECK_EQUAL(urlDecode("%20"), " "); - BOOST_CHECK_EQUAL(urlDecode("%FF%F0%E0"), "\xff\xf0\xe0"); - BOOST_CHECK_EQUAL(urlDecode("%01%19"), "\x01\x19"); - BOOST_CHECK_EQUAL(urlDecode("http%3A%2F%2Fwww.ietf.org%2Frfc%2Frfc3986.txt"), + BOOST_CHECK_EQUAL(UrlDecode("%20"), " "); + BOOST_CHECK_EQUAL(UrlDecode("%FF%F0%E0"), "\xff\xf0\xe0"); + BOOST_CHECK_EQUAL(UrlDecode("%01%19"), "\x01\x19"); + BOOST_CHECK_EQUAL(UrlDecode("http%3A%2F%2Fwww.ietf.org%2Frfc%2Frfc3986.txt"), "http://www.ietf.org/rfc/rfc3986.txt"); - BOOST_CHECK_EQUAL(urlDecode("1%2B2%3D3"), "1+2=3"); + BOOST_CHECK_EQUAL(UrlDecode("1%2B2%3D3"), "1+2=3"); } BOOST_AUTO_TEST_CASE(decode_malformed_test) { - BOOST_CHECK_EQUAL(urlDecode("%%xhello th+ere \xff"), "%%xhello th+ere \xff"); + BOOST_CHECK_EQUAL(UrlDecode("%%xhello th+ere \xff"), "%%xhello th+ere \xff"); - BOOST_CHECK_EQUAL(urlDecode("%"), "%"); - BOOST_CHECK_EQUAL(urlDecode("%%"), "%%"); - BOOST_CHECK_EQUAL(urlDecode("%%%"), "%%%"); - BOOST_CHECK_EQUAL(urlDecode("%%%%"), "%%%%"); + BOOST_CHECK_EQUAL(UrlDecode("%"), "%"); + BOOST_CHECK_EQUAL(UrlDecode("%%"), "%%"); + BOOST_CHECK_EQUAL(UrlDecode("%%%"), "%%%"); + BOOST_CHECK_EQUAL(UrlDecode("%%%%"), "%%%%"); - BOOST_CHECK_EQUAL(urlDecode("+"), "+"); - BOOST_CHECK_EQUAL(urlDecode("++"), "++"); + BOOST_CHECK_EQUAL(UrlDecode("+"), "+"); + BOOST_CHECK_EQUAL(UrlDecode("++"), "++"); - BOOST_CHECK_EQUAL(urlDecode("?"), "?"); - BOOST_CHECK_EQUAL(urlDecode("??"), "??"); + BOOST_CHECK_EQUAL(UrlDecode("?"), "?"); + BOOST_CHECK_EQUAL(UrlDecode("??"), "??"); - BOOST_CHECK_EQUAL(urlDecode("%G1"), "%G1"); - BOOST_CHECK_EQUAL(urlDecode("%2"), "%2"); - BOOST_CHECK_EQUAL(urlDecode("%ZX"), "%ZX"); + BOOST_CHECK_EQUAL(UrlDecode("%G1"), "%G1"); + BOOST_CHECK_EQUAL(UrlDecode("%2"), "%2"); + BOOST_CHECK_EQUAL(UrlDecode("%ZX"), "%ZX"); - BOOST_CHECK_EQUAL(urlDecode("valid%20string%G1"), "valid string%G1"); - BOOST_CHECK_EQUAL(urlDecode("%20invalid%ZX"), " invalid%ZX"); - BOOST_CHECK_EQUAL(urlDecode("%20%G1%ZX"), " %G1%ZX"); + BOOST_CHECK_EQUAL(UrlDecode("valid%20string%G1"), "valid string%G1"); + BOOST_CHECK_EQUAL(UrlDecode("%20invalid%ZX"), " invalid%ZX"); + BOOST_CHECK_EQUAL(UrlDecode("%20%G1%ZX"), " %G1%ZX"); - BOOST_CHECK_EQUAL(urlDecode("%1 "), "%1 "); - BOOST_CHECK_EQUAL(urlDecode("% 9"), "% 9"); - BOOST_CHECK_EQUAL(urlDecode(" %Z "), " %Z "); - BOOST_CHECK_EQUAL(urlDecode(" % X"), " % X"); + BOOST_CHECK_EQUAL(UrlDecode("%1 "), "%1 "); + BOOST_CHECK_EQUAL(UrlDecode("% 9"), "% 9"); + BOOST_CHECK_EQUAL(UrlDecode(" %Z "), " %Z "); + BOOST_CHECK_EQUAL(UrlDecode(" % X"), " % X"); - BOOST_CHECK_EQUAL(urlDecode("%-1"), "%-1"); - BOOST_CHECK_EQUAL(urlDecode("%1-"), "%1-"); + BOOST_CHECK_EQUAL(UrlDecode("%-1"), "%-1"); + BOOST_CHECK_EQUAL(UrlDecode("%1-"), "%1-"); } BOOST_AUTO_TEST_CASE(decode_lowercase_hex_test) { - BOOST_CHECK_EQUAL(urlDecode("%f0%a0%b0"), "\xf0\xa0\xb0"); + BOOST_CHECK_EQUAL(UrlDecode("%f0%a0%b0"), "\xf0\xa0\xb0"); } BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) { - BOOST_CHECK_EQUAL(urlDecode("%00%00x%00%00"), ""); - BOOST_CHECK_EQUAL(urlDecode("abc%00%00"), "abc"); + BOOST_CHECK_EQUAL(UrlDecode("%00%00x%00%00"), ""); + BOOST_CHECK_EQUAL(UrlDecode("abc%00%00"), "abc"); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index e81efac6e0..631da13803 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -90,7 +90,7 @@ FUZZ_TARGET(string) (void)ToUpper(random_string_1); (void)TrimString(random_string_1); (void)TrimString(random_string_1, random_string_2); - (void)urlDecode(random_string_1); + (void)UrlDecode(random_string_1); (void)ContainsNoNUL(random_string_1); (void)_(random_string_1.c_str()); try { diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index eb081e765a..1252843e9d 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -64,7 +64,7 @@ bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& { if (request.URI.starts_with(WALLET_ENDPOINT_BASE)) { // wallet endpoint was used - wallet_name = urlDecode(std::string_view{request.URI}.substr(WALLET_ENDPOINT_BASE.size())); + wallet_name = UrlDecode(std::string_view{request.URI}.substr(WALLET_ENDPOINT_BASE.size())); return true; } return false; From 992c714451676cee33d3dff49f36329423270c1c Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sun, 21 Apr 2024 21:48:05 +0200 Subject: [PATCH 68/72] common: Don't terminate on null character in UrlDecode The previous behavior was the result of casting the result returned from the libevent function evhttp_uridecode to std:string but this was probably not intended. --- src/common/url.cpp | 5 ----- src/test/common_url_tests.cpp | 6 ++++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/common/url.cpp b/src/common/url.cpp index f27bb2b73f..ecf88d07ea 100644 --- a/src/common/url.cpp +++ b/src/common/url.cpp @@ -25,11 +25,6 @@ std::string UrlDecode(std::string_view url_encoded) // Only if there is no error and the pointer is set to the end of // the string, we can be sure both characters were valid hex if (ec == std::errc{} && p == url_encoded.data() + i + 3) { - // A null character terminates the string - if (decoded_value == 0) { - return res; - } - res += static_cast(decoded_value); // Next two characters are part of the percent encoding i += 2; diff --git a/src/test/common_url_tests.cpp b/src/test/common_url_tests.cpp index eb92c2ceee..cc893cbed7 100644 --- a/src/test/common_url_tests.cpp +++ b/src/test/common_url_tests.cpp @@ -63,8 +63,10 @@ BOOST_AUTO_TEST_CASE(decode_lowercase_hex_test) { } BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) { - BOOST_CHECK_EQUAL(UrlDecode("%00%00x%00%00"), ""); - BOOST_CHECK_EQUAL(UrlDecode("abc%00%00"), "abc"); + std::string result1{"\0\0x\0\0", 5}; + BOOST_CHECK_EQUAL(UrlDecode("%00%00x%00%00"), result1); + std::string result2{"abc\0\0", 5}; + BOOST_CHECK_EQUAL(UrlDecode("abc%00%00"), result2); } BOOST_AUTO_TEST_SUITE_END() From 03e36b3da093e2c23cf51b46f6901cb84ddbf867 Mon Sep 17 00:00:00 2001 From: hanmz Date: Tue, 23 Apr 2024 17:24:38 +0800 Subject: [PATCH 69/72] Fix typos in description.md and wallet_util.py Signed-off-by: hanmz --- depends/description.md | 2 +- test/functional/test_framework/wallet_util.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/depends/description.md b/depends/description.md index 69ee5bd36c..fa345bfe85 100644 --- a/depends/description.md +++ b/depends/description.md @@ -11,7 +11,7 @@ on new hosts. ### No reliance on timestamps File presence is used to determine what needs to be built. This makes the -results distributable and easily digestable by automated builders. +results distributable and easily digestible by automated builders. ### Each build only has its specified dependencies available at build-time. diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index d30b00f4a7..2168e607b2 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -165,7 +165,7 @@ def __exit__(self, *args): class TestFrameworkWalletUtil(unittest.TestCase): def test_calculate_input_weight(self): SKELETON_BYTES = 32 + 4 + 4 # prevout-txid, prevout-index, sequence - SMALL_LEN_BYTES = 1 # bytes needed for encoding scriptSig / witness item lenghts < 253 + SMALL_LEN_BYTES = 1 # bytes needed for encoding scriptSig / witness item lengths < 253 LARGE_LEN_BYTES = 3 # bytes needed for encoding scriptSig / witness item lengths >= 253 # empty scriptSig, no witness From 9adf949d2aa6d199b85295b18c08967395b5570a Mon Sep 17 00:00:00 2001 From: bstin Date: Wed, 14 Feb 2024 06:20:32 -0600 Subject: [PATCH 70/72] contrib: rpcauth.py - Add new option (-j/--json) to output text in json format -j/--json update and remove un-needed parens Update README to reflect new -j/--json options --- share/rpcauth/README.md | 1 + share/rpcauth/rpcauth.py | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/share/rpcauth/README.md b/share/rpcauth/README.md index 6f627b867b..1b3acb1dac 100644 --- a/share/rpcauth/README.md +++ b/share/rpcauth/README.md @@ -15,4 +15,5 @@ positional arguments: optional arguments: -h, --help show this help message and exit + -j, --json output data in json format ``` diff --git a/share/rpcauth/rpcauth.py b/share/rpcauth/rpcauth.py index cc7bba1f8b..506fcf9d91 100755 --- a/share/rpcauth/rpcauth.py +++ b/share/rpcauth/rpcauth.py @@ -7,6 +7,7 @@ from getpass import getpass from secrets import token_hex, token_urlsafe import hmac +import json def generate_salt(size): """Create size byte hex salt""" @@ -24,6 +25,7 @@ def main(): parser = ArgumentParser(description='Create login credentials for a JSON-RPC user') parser.add_argument('username', help='the username for authentication') parser.add_argument('password', help='leave empty to generate a random password or specify "-" to prompt for password', nargs='?') + parser.add_argument("-j", "--json", help="output to json instead of plain-text", action='store_true') args = parser.parse_args() if not args.password: @@ -35,9 +37,13 @@ def main(): salt = generate_salt(16) password_hmac = password_to_hmac(salt, args.password) - print('String to be appended to bitcoin.conf:') - print(f'rpcauth={args.username}:{salt}${password_hmac}') - print(f'Your password:\n{args.password}') + if args.json: + odict={'username':args.username, 'password':args.password, 'rpcauth':f'{args.username}:{salt}${password_hmac}'} + print(json.dumps(odict)) + else: + print('String to be appended to bitcoin.conf:') + print(f'rpcauth={args.username}:{salt}${password_hmac}') + print(f'Your password:\n{args.password}') if __name__ == '__main__': main() From fa55972a758865a6bd0114afe72e51877896d495 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 26 Apr 2024 08:26:52 +0200 Subject: [PATCH 71/72] test: Add two more urlDecode tests --- src/test/common_url_tests.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/common_url_tests.cpp b/src/test/common_url_tests.cpp index cc893cbed7..065c7d97bc 100644 --- a/src/test/common_url_tests.cpp +++ b/src/test/common_url_tests.cpp @@ -54,6 +54,9 @@ BOOST_AUTO_TEST_CASE(decode_malformed_test) { BOOST_CHECK_EQUAL(UrlDecode(" %Z "), " %Z "); BOOST_CHECK_EQUAL(UrlDecode(" % X"), " % X"); + BOOST_CHECK_EQUAL(UrlDecode("%%ffg"), "%\xffg"); + BOOST_CHECK_EQUAL(UrlDecode("%fg"), "%fg"); + BOOST_CHECK_EQUAL(UrlDecode("%-1"), "%-1"); BOOST_CHECK_EQUAL(UrlDecode("%1-"), "%1-"); } From 97a4ad5713853f51c729cced73f133fafa735ba2 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 26 Apr 2024 22:47:00 +0100 Subject: [PATCH 72/72] build, msvc: Drop duplicated `common\url.cpp` source file It has been a part of SOURCE_FILES since https://github.com/bitcoin/bitcoin/pull/29904 --- build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in | 1 - 1 file changed, 1 deletion(-) diff --git a/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in b/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in index 482e4333f7..b47d62b295 100644 --- a/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in +++ b/build_msvc/libbitcoin_common/libbitcoin_common.vcxproj.in @@ -8,7 +8,6 @@ StaticLibrary - @SOURCE_FILES@