From fdcfbd8694ded2db5c0d7924ce0a583e4f883943 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 12 Sep 2023 12:41:11 -0400 Subject: [PATCH 1/4] Merge bitcoin/bitcoin#28101: doc, refactor: changing -torcontrol help to specify that a default port is used 9a84200cfc994eebf38c46919b20e0c0261799ae doc, refactor: Changing -torcontrol help to specify that a default port is used (kevkevin) Pull request description: Right now when we get the help for -torcontrol it says that there is a default ip and port we dont specify if there is a specified ip that we would also use port 9051 as default Also I create a new const instead of using 9051 directly in the function linking this PR because this was discussed here https://github.com/bitcoin/bitcoin/pull/28018 ACKs for top commit: jonatack: re-ACK 9a84200cfc994eebf38c46919b20e0c0261799ae achow101: ACK 9a84200cfc994eebf38c46919b20e0c0261799ae MarnixCroes: utACK 9a84200cfc994eebf38c46919b20e0c0261799ae kristapsk: utACK 9a84200cfc994eebf38c46919b20e0c0261799ae Tree-SHA512: 21d9e65f3c280a2853a9cf60d4e93e8d72caccea106206d1862c19535bde7ea6ada7f55e6ea19a1fc0f59dbe791ec6fc4084fdbe7fa6d6991fa89c62070db637 --- src/init.cpp | 2 +- src/torcontrol.cpp | 6 +++--- src/torcontrol.h | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 364af98cb4d5..74aeb57ee816 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -652,7 +652,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-socketevents=", "Socket events mode, which must be one of 'select', 'poll', 'epoll' or 'kqueue', depending on your system (default: Linux - 'epoll', FreeBSD/Apple - 'kqueue', Windows - 'select')", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-networkactive", "Enable all P2P network activity (default: 1). Can be changed by the setnetworkactive RPC command", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-timeout=", strprintf("Specify socket connection timeout in milliseconds. If an initial attempt to connect is unsuccessful after this amount of time, drop it (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-torcontrol=:", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-torcontrol=:", strprintf("Tor control host and port to use if onion listening enabled (default: %s). If no port is specified, the default port of %i will be used.", DEFAULT_TOR_CONTROL, DEFAULT_TOR_CONTROL_PORT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torpassword=", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION); #ifdef USE_UPNP argsman.AddArg("-upnp", strprintf("Use UPnP to map the listening port (default: %u)", DEFAULT_UPNP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index f7c244687a96..8387f467a914 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -40,8 +40,8 @@ #include #include -/** Default control port */ -const std::string DEFAULT_TOR_CONTROL = "127.0.0.1:9051"; +/** Default control ip and port */ +const std::string DEFAULT_TOR_CONTROL = "127.0.0.1:" + ToString(DEFAULT_TOR_CONTROL_PORT); /** Tor cookie size (from control-spec.txt) */ static const int TOR_COOKIE_SIZE = 32; /** Size of client/server nonce for SAFECOOKIE */ @@ -142,7 +142,7 @@ bool TorControlConnection::Connect(const std::string& tor_control_center, const Disconnect(); } - const std::optional control_service{Lookup(tor_control_center, 9051, fNameLookup)}; + const std::optional control_service{Lookup(tor_control_center, DEFAULT_TOR_CONTROL_PORT, fNameLookup)}; if (!control_service.has_value()) { LogPrintf("tor: Failed to look up control center %s\n", tor_control_center); return false; diff --git a/src/torcontrol.h b/src/torcontrol.h index f97dcd82ef4f..a0603fb1d705 100644 --- a/src/torcontrol.h +++ b/src/torcontrol.h @@ -19,6 +19,7 @@ #include #include +constexpr int DEFAULT_TOR_CONTROL_PORT = 9051; extern const std::string DEFAULT_TOR_CONTROL; static const bool DEFAULT_LISTEN_ONION = true; From 91a8dc545b4e73a9bbc41086073f06b4e8c54f8f Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 8 Nov 2023 10:53:34 -0500 Subject: [PATCH 2/4] Merge bitcoin/bitcoin#28787: init: completely remove `-zapwallettxes` (remaining hidden option) 5039c346ca87d6112ea1eb124bdc622ba9e9a513 init: completely remove `-zapwallettxes` (remaining hidden option) (Sebastian Falbesoner) Pull request description: The `-zapwallettxes` functionality has been removed in v0.21.0 (see commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b / PR #19671), with the parameter being kept as hidden option, to inform users via an exit error that `abandontransaction` should be used instead. As any guides that still suggest to use `-zapwallettxes` would refer to a Bitcoin Core version that is EOL since many years (i.e. <= v0.20.x), it is highly unlikely that the error caused by the option is still relevant for any user, hence it seems fine to remove it now. ACKs for top commit: achow101: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 BrandonOdiwuor: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 fanquake: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 Tree-SHA512: e3ccc6918e0f8fa68dbd1a7ec4999cc2a44e28038711919fcddaf0727648c73a9ba0fb77674317147592a113fad20755d4e727f48176bc17b048fbdebad2d6c9 --- src/wallet/init.cpp | 6 ------ test/lint/check-doc.py | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 444e066b0021..e1be2e7eefb6 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -125,8 +125,6 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-walletrejectlongchains", strprintf("Wallet will not create transactions that violate mempool chain limits (default: %u)", DEFAULT_WALLET_REJECT_LONG_CHAINS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-walletcrosschain", strprintf("Allow reusing wallet files across chains (default: %u)", DEFAULT_WALLETCROSSCHAIN), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); - - argsman.AddHiddenArgs({"-zapwallettxes"}); } bool WalletInit::ParameterInteraction() const @@ -150,10 +148,6 @@ bool WalletInit::ParameterInteraction() const LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); } - if (gArgs.IsArgSet("-zapwallettxes")) { - return InitError(Untranslated("-zapwallettxes has been removed. If you are attempting to remove a stuck transaction from your wallet, please use abandontransaction instead.")); - } - int rescan_mode = gArgs.GetIntArg("-rescan", 0); if (rescan_mode < 0 || rescan_mode > 2) { LogPrintf("%s: Warning: incorrect -rescan mode, falling back to default value.\n", __func__); diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py index 7955dd4f3e65..dd7cb0306eb0 100755 --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -23,7 +23,7 @@ CMD_GREP_WALLET_HIDDEN_ARGS = r"git grep --function-context 'void DummyWalletInit::AddWalletOptions' -- {}".format(CMD_ROOT_DIR) CMD_GREP_DOCS = r"git grep --perl-regexp '{}' {}".format(REGEX_DOC, CMD_ROOT_DIR) # list unsupported, deprecated and duplicate args as they need no documentation -SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb', '-zapwallettxes']) +SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb']) def lint_missing_argument_documentation(): From e3bdacbcf973a0f639c7377b2add30b7ccce71fd Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 2 Aug 2023 11:39:48 +0100 Subject: [PATCH 3/4] Merge bitcoin/bitcoin#27572: test: dedup file hashing using `sha256sum_file` helper 2c0c6f44770403899bd8514ad7343356853bf38c test: dedup file hashing using `sha256sum_file` helper (Sebastian Falbesoner) Pull request description: Rather than doing the open/read/hash-steps manually in the affected functional tests, we can just use the `sha256sum_file` helper from the utils module instead. Note that for the tool_wallet.py test, the used hash is changed from sha1 to sha256, but as the only purpose is to detect file content changes, this doesn't matter. Also, the optimization using `memoryview` is overkill here, as the opened file has only a size of 24KiB and determining the hash via the helper doesn't take longer than a few hundred micro-seconds on my machine. ACKs for top commit: kristapsk: ACK 2c0c6f44770403899bd8514ad7343356853bf38c Tree-SHA512: 64fe21650b56a50e9f1a95f6ef27d88d8bfbb621e5be456f327ef8dbb5596b529d03976c200f3fd68da48cc427de9f257b403f3228e38cf1df918006674fac68 --- test/functional/rpc_dumptxoutset.py | 20 +++++++++++--------- test/functional/tool_wallet.py | 15 +++++---------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index 6214768034a8..690538997f0f 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -4,13 +4,15 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the generation of UTXO snapshots using `dumptxoutset`. """ +from pathlib import Path from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error - -import hashlib -from pathlib import Path +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + sha256sum_file, +) class DumptxoutsetTest(BitcoinTestFramework): @@ -39,11 +41,11 @@ def run_test(self): out['base_hash'], '38619d68335f21ded08d6e1f4ef9572eaeb0bb04b9070b3e6be33c7c908564b4') - with open(str(expected_path), 'rb') as f: - digest = hashlib.sha256(f.read()).hexdigest() - # UTXO snapshot hash should be deterministic based on mocked time. - assert_equal( - digest, '4a34cf865938252bf0bef702989955824d886f595afab596b1edac4dd31cd89f') + + # UTXO snapshot hash should be deterministic based on mocked time. + assert_equal( + sha256sum_file(str(expected_path)).hex(), + '4a34cf865938252bf0bef702989955824d886f595afab596b1edac4dd31cd89f') assert_equal( out['txoutset_hash'], 'b2d7429106c96f5ab831843d5c96ba131ca8793111d0a0e30e7d7d8b4841e6cc') diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 25b2bd5cdf88..1fcd2aa16e2a 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test dash-wallet.""" -import hashlib import os import stat import subprocess @@ -13,9 +12,10 @@ from collections import OrderedDict from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal - -BUFFER_SIZE = 16 * 1024 +from test_framework.util import ( + assert_equal, + sha256sum_file, +) class ToolWalletTest(BitcoinTestFramework): def set_test_params(self): @@ -49,12 +49,7 @@ def assert_tool_output(self, output, *args): assert_equal(p.poll(), 0) def wallet_shasum(self): - h = hashlib.sha1() - mv = memoryview(bytearray(BUFFER_SIZE)) - with open(self.wallet_path, 'rb', buffering=0) as f: - for n in iter(lambda : f.readinto(mv), 0): - h.update(mv[:n]) - return h.hexdigest() + return sha256sum_file(self.wallet_path).hex() def wallet_timestamp(self): return os.path.getmtime(self.wallet_path) From ee2c19645ffa16426984353f0ba9c83c0ed3c16f Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 11 Jul 2023 17:16:14 -0400 Subject: [PATCH 4/4] Merge bitcoin/bitcoin#28025: test: refactor: deduplicate legacy ECDSA signing for tx inputs 5cf44275c8ca8c32d238f37f717d78e9823f44c2 test: refactor: deduplicate legacy ECDSA signing for tx inputs (Sebastian Falbesoner) Pull request description: There are several instances in functional tests and the framework (MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy ECDSA signature for a certain transaction's input by doing the following steps: 1. calculate the `LegacySignatureHash` with the desired sighash type 2. create the actual digital signature by calling `ECKey.sign_ecdsa` on the signature message hash calculated above 3. put the DER-encoded result as CScript data push into tx input's scriptSig Create a new helper `sign_input_legacy` which hides those details and takes only the necessary parameters (tx, input index, relevant scriptPubKey, private key, sighash type [SIGHASH_ALL by default]). For further convenience, the signature is prepended to already existing data-pushes in scriptSig, in order to avoid rehashing the transaction after calling the new signing function. ACKs for top commit: dimitaracev: ACK `5cf4427` achow101: ACK 5cf44275c8ca8c32d238f37f717d78e9823f44c2 pinheadmz: ACK 5cf44275c8ca8c32d238f37f717d78e9823f44c2 Tree-SHA512: 8f0e4fb2c3e0f84fac5dbc4dda87973276242b0f628034272a7f3e45434c1e17dd1b26a37edfb302dcaf380dbfe98b0417391ace5e0ac9720155d8fba702031e --- test/functional/feature_block.py | 14 ++++---------- test/functional/test_framework/script.py | 11 +++++++++++ test/functional/test_framework/wallet.py | 17 +++++++---------- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index a56288176f31..12caa6340de0 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -42,8 +42,7 @@ OP_INVALIDOPCODE, OP_RETURN, OP_TRUE, - SIGHASH_ALL, - SignatureHash, + sign_input, ) from test_framework.script_util import ( script_to_p2sh_script, @@ -551,12 +550,8 @@ def run_test(self): # second input is corresponding P2SH output from b39 tx.vin.append(CTxIn(COutPoint(b39.vtx[i].sha256, 0), b'')) # Note: must pass the redeem_script (not p2sh_script) to the signature hash function - (sighash, err) = SignatureHash(redeem_script, tx, 1, SIGHASH_ALL) - sig = self.coinbase_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL])) - scriptSig = CScript([sig, redeem_script]) - - tx.vin[1].scriptSig = scriptSig - tx.rehash() + tx.vin[1].scriptSig = CScript([redeem_script]) + sign_input(tx, 1, redeem_script, self.coinbase_key) new_txs.append(tx) lastOutpoint = COutPoint(tx.sha256, 0) @@ -1354,8 +1349,7 @@ def sign_tx(self, tx, spend_tx): if (scriptPubKey[0] == OP_TRUE): # an anyone-can-spend tx.vin[0].scriptSig = CScript() return - (sighash, err) = SignatureHash(spend_tx.vout[0].scriptPubKey, tx, 0, SIGHASH_ALL) - tx.vin[0].scriptSig = CScript([self.coinbase_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL]))]) + sign_input(tx, 0, spend_tx.vout[0].scriptPubKey, self.coinbase_key) def create_and_sign_transaction(self, spend_tx, value, script=CScript([OP_TRUE])): tx = self.create_tx(spend_tx, 0, value, script) diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index fe4602213cdf..bb078591b9f4 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -666,6 +666,17 @@ def SignatureHash(script, txTo, inIdx, hashtype): return (hash, None) +def sign_input(tx, input_index, input_scriptpubkey, privkey, sighash_type=SIGHASH_ALL): + """Add legacy ECDSA signature for a given transaction input. Note that the signature + is prepended to the scriptSig field, i.e. additional data pushes necessary for more + complex spends than P2PK (e.g. pubkey for P2PKH) can be already set before.""" + (sighash, err) = SignatureHash(input_scriptpubkey, tx, input_index, sighash_type) + assert err is None + der_sig = privkey.sign_ecdsa(sighash) + tx.vin[input_index].scriptSig = bytes(CScript([der_sig + bytes([sighash_type])])) + tx.vin[input_index].scriptSig + tx.rehash() + + class TestFrameworkScript(unittest.TestCase): def test_bn2vch(self): self.assertEqual(bn2vch(0), bytes([])) diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index e350614b20cb..b551ebef238f 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -30,10 +30,9 @@ ) from test_framework.script import ( CScript, - SignatureHash, OP_TRUE, OP_NOP, - SIGHASH_ALL, + sign_input, ) from test_framework.script_util import ( key_to_p2pk_script, @@ -134,18 +133,16 @@ def scan_tx(self, tx): def sign_tx(self, tx, fixed_length=True): if self._mode == MiniWalletMode.RAW_P2PK: - (sighash, err) = SignatureHash(CScript(self._scriptPubKey), tx, 0, SIGHASH_ALL) - assert err is None # for exact fee calculation, create only signatures with fixed size by default (>49.89% probability): # 65 bytes: high-R val (33 bytes) + low-S val (32 bytes) - # with the DER header/skeleton data of 6 bytes added, this leads to a target size of 71 bytes - der_sig = b'' - while not len(der_sig) == 71: - der_sig = self._priv_key.sign_ecdsa(sighash) + # with the DER header/skeleton data of 6 bytes added, plus 2 bytes scriptSig overhead + # (OP_PUSHn and SIGHASH_ALL), this leads to a scriptSig target size of 73 bytes + tx.vin[0].scriptSig = b'' + while not len(tx.vin[0].scriptSig) == 73: + tx.vin[0].scriptSig = b'' + sign_input(tx, 0, self._scriptPubKey, self._priv_key) if not fixed_length: break - tx.vin[0].scriptSig = CScript([der_sig + bytes(bytearray([SIGHASH_ALL]))]) - tx.rehash() elif self._mode == MiniWalletMode.RAW_OP_TRUE: for i in range(len(tx.vin)): tx.vin[i].scriptSig = CScript([OP_NOP] * 24) # pad to identical size