From 4f0dfc9e894052f39e1b70e13ef6176cb585507c Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 23 Jan 2024 15:07:43 -0500 Subject: [PATCH 1/5] Merge bitcoin/bitcoin#29144: init: handle empty settings file gracefully e9014042a6bed8c16cc9a31fc35cb709d4b3c766 settings: add auto-generated warning msg for editing the file manually (furszy) 966f5de99a9f5da05c91378ad1e8ea8ed37ac3b3 init: improve corrupted/empty settings file error msg (furszy) Pull request description: Small and simple issue reported [here](https://community.umbrel.com/t/bitcoin-docker-container-keeps-restarting/2144). Improving a confusing situation reported by users who did not understand why a settings parsing error occurred when the file was empty and did not know how to solve it. Empty setting file could be due (1) corruption or (2) an user manually cleaning up the file content. In both scenarios, the 'Unable to parse settings file' error does not help the user move forward. ACKs for top commit: achow101: ACK e9014042a6bed8c16cc9a31fc35cb709d4b3c766 hebasto: re-ACK e9014042a6bed8c16cc9a31fc35cb709d4b3c766. ryanofsky: Code review ACK e9014042a6bed8c16cc9a31fc35cb709d4b3c766. Just whitespace formatting changes and shortening a test string literal since last review shaavan: Code review ACK e9014042a6bed8c16cc9a31fc35cb709d4b3c766 Tree-SHA512: 2910654c6b9e9112de391eedb8e46980280f822fa3059724dd278db7436804dd27fae628d2003f2c6ac1599b07ac5c589af016be693486e949f558515e662bec --- src/qt/test/optiontests.cpp | 4 ++++ src/test/settings_tests.cpp | 4 +++- src/util/settings.cpp | 15 ++++++++++++++- test/functional/feature_settings.py | 9 +++++---- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/qt/test/optiontests.cpp b/src/qt/test/optiontests.cpp index 17ffeb220b69..067cfaba04b3 100644 --- a/src/qt/test/optiontests.cpp +++ b/src/qt/test/optiontests.cpp @@ -61,7 +61,11 @@ void OptionTests::migrateSettings() QVERIFY(!settings.contains("addrSeparateProxyTor")); std::ifstream file(gArgs.GetDataDirNet() / "settings.json"); + std::string default_warning = strprintf("This file is automatically generated and updated by %s. Please do not edit this file while the node " + "is running, as any changes might be ignored or overwritten.", + PACKAGE_NAME); QCOMPARE(std::string(std::istreambuf_iterator(file), std::istreambuf_iterator()).c_str(), "{\n" + " \"_warning_\": \""+ default_warning+"\",\n" " \"dbcache\": \"600\",\n" " \"listen\": false,\n" " \"onion\": \"onion:234\",\n" diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp index 9bd9c7942043..0f57b3c9e2c6 100644 --- a/src/test/settings_tests.cpp +++ b/src/test/settings_tests.cpp @@ -99,7 +99,9 @@ BOOST_AUTO_TEST_CASE(ReadWrite) // Check invalid json not allowed WriteText(path, R"(invalid json)"); BOOST_CHECK(!util::ReadSettings(path, values, errors)); - std::vector fail_parse = {strprintf("Unable to parse settings file %s", fs::PathToString(path))}; + std::vector fail_parse = {strprintf("Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, " + "and can be fixed by removing the file, which will reset settings to default values.", + fs::PathToString(path))}; BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), fail_parse.begin(), fail_parse.end()); } diff --git a/src/util/settings.cpp b/src/util/settings.cpp index 421b2be4623c..c0423ea023ff 100644 --- a/src/util/settings.cpp +++ b/src/util/settings.cpp @@ -5,6 +5,10 @@ #include #include +#if defined(HAVE_CONFIG_H) +#include +#endif + #include #include @@ -90,7 +94,9 @@ bool ReadSettings(const fs::path& path, std::map& va SettingsValue in; if (!in.read(std::string{std::istreambuf_iterator(file), std::istreambuf_iterator()})) { - errors.emplace_back(strprintf("Unable to parse settings file %s", fs::PathToString(path))); + errors.emplace_back(strprintf("Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, " + "and can be fixed by removing the file, which will reset settings to default values.", + fs::PathToString(path))); return false; } @@ -123,6 +129,13 @@ bool WriteSettings(const fs::path& path, std::vector& errors) { SettingsValue out(SettingsValue::VOBJ); + // Add auto-generated warning comment only if it does not exist + if (!values.contains("_warning_")) { + out.pushKV("_warning_", strprintf("This file is automatically generated and updated by %s. Please do not edit this file while the node " + "is running, as any changes might be ignored or overwritten.", + PACKAGE_NAME)); + } + // Push settings values for (const auto& value : values) { out.__pushKV(value.first, value.second); } diff --git a/test/functional/feature_settings.py b/test/functional/feature_settings.py index 7b295e854d6a..835cb5df5f58 100755 --- a/test/functional/feature_settings.py +++ b/test/functional/feature_settings.py @@ -24,10 +24,11 @@ def run_test(self): settings = Path(node.datadir, self.chain, "settings.json") conf = Path(node.datadir, "dash.conf") - # Assert empty settings file was created + # Assert default settings file was created self.stop_node(0) + default_settings = {"_warning_": "This file is automatically generated and updated by Dash Core. Please do not edit this file while the node is running, as any changes might be ignored or overwritten."} with settings.open() as fp: - assert_equal(json.load(fp), {}) + assert_equal(json.load(fp), default_settings) # Assert settings are parsed and logged with settings.open("w") as fp: @@ -49,12 +50,12 @@ def run_test(self): # Assert settings are unchanged after shutdown with settings.open() as fp: - assert_equal(json.load(fp), {"string": "string", "num": 5, "bool": True, "null": None, "list": [6, 7]}) + assert_equal(json.load(fp), {**default_settings, **{"string": "string", "num": 5, "bool": True, "null": None, "list": [6, 7]}}) # Test invalid json with settings.open("w") as fp: fp.write("invalid json") - node.assert_start_raises_init_error(expected_msg='Unable to parse settings file', match=ErrorMatch.PARTIAL_REGEX) + node.assert_start_raises_init_error(expected_msg='does not contain valid JSON. This is probably caused by disk corruption or a crash', match=ErrorMatch.PARTIAL_REGEX) # Test invalid json object with settings.open("w") as fp: From 4cc2103be2963c6b6ec02b47b74b432e3c603f86 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 4 Dec 2023 14:27:47 -0500 Subject: [PATCH 2/5] Merge bitcoin/bitcoin#28946: init: don't delete PID file if it was not generated 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b init: don't delete PID file if it was not generated (willcl-ark) Pull request description: In a similar vein to #28784, if a second `bitcoind` is started using the same datadir it will fail to start up, but during shutdown remove the PID file from the first `bitcoind` instance. ACKs for top commit: achow101: ACK 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b andrewtoth: ACK 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b romanz: ACK https://github.com/bitcoin/bitcoin/commit/8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b Tree-SHA512: c9af703cbfa179d33ef9580a51e86c1b0acbd28daa18c8d2e5e5ff796ab4d3e2009a962a47e6046a0e5ece936f8a06ee8af5fdf8ff4ae1e52cbcdbec4b942271 --- src/init.cpp | 25 ++++++++++++++++++------- test/functional/feature_filelock.py | 5 ++++- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 8d516f2cdb9a..c99f384abfbd 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -184,6 +184,11 @@ static const char* DEFAULT_ASMAP_FILENAME="ip_asn.map"; * The PID file facilities. */ static const char* BITCOIN_PID_FILENAME = "dashd.pid"; +/** + * True if this process has created a PID file. + * Used to determine whether we should remove the PID file on shutdown. + */ +static bool g_generated_pid{false}; static fs::path GetPidFile(const ArgsManager& args) { @@ -199,12 +204,24 @@ static fs::path GetPidFile(const ArgsManager& args) #else tfm::format(file, "%d\n", getpid()); #endif + g_generated_pid = true; return true; } else { return InitError(strprintf(_("Unable to create the PID file '%s': %s"), fs::PathToString(GetPidFile(args)), SysErrorString(errno))); } } +static void RemovePidFile(const ArgsManager& args) +{ + if (!g_generated_pid) return; + const auto pid_path{GetPidFile(args)}; + if (std::error_code error; !fs::remove(pid_path, error)) { + std::string msg{error ? error.message() : "File does not exist"}; + LogPrintf("Unable to remove PID file (%s): %s\n", fs::PathToString(pid_path), msg); + } +} + + ////////////////////////////////////////////////////////////////////////////// // // Shutdown @@ -476,13 +493,7 @@ void Shutdown(NodeContext& node) node.chainman.reset(); node.scheduler.reset(); - try { - if (!fs::remove(GetPidFile(*node.args))) { - LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__); - } - } catch (const fs::filesystem_error& e) { - LogPrintf("%s: Unable to remove PID file: %s\n", __func__, fsbridge::get_filesystem_error_message(e)); - } + RemovePidFile(*node.args); LogPrintf("%s: done\n", __func__); } diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py index 31f5225ae19c..2aa384ced7f5 100755 --- a/test/functional/feature_filelock.py +++ b/test/functional/feature_filelock.py @@ -33,8 +33,11 @@ def run_test(self): expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_NAME']} is probably already running." self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir}', '-noserver'], expected_msg=expected_msg) + self.log.info("Check that cookie and PID file are not deleted when attempting to start a second dashd using the same datadir") cookie_file = Path(datadir) / ".cookie" - assert cookie_file.exists() # should not be deleted during the second bitcoind instance shutdown + assert cookie_file.exists() # should not be deleted during the second dashd instance shutdown + pid_file = Path(datadir) / "dashd.pid" + assert pid_file.exists() if self.is_wallet_compiled(): def check_wallet_filelock(descriptors): From e6524067ec498efeef256a02b3a17e01d8188e18 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 6 Dec 2023 10:34:18 -0500 Subject: [PATCH 3/5] Merge bitcoin/bitcoin#28980: rpc: encryptwallet help, mention HD seed rotation and backup requirement ca09415e630f0f7de9160cab234bd5ba6968ff2d rpc, doc: encryptwallet, mention HD seed rotation and new backup (furszy) Pull request description: Small and simple PR, updating the `encryptwallet` help message. Better to notify users about the HD seed rotation and the new backup requirement before executing the encryption process. Ensuring they are prepared to update previous backups and securely safeguard the updated wallet file. ACKs for top commit: S3RK: ACK ca09415e630f0f7de9160cab234bd5ba6968ff2d achow101: ACK ca09415e630f0f7de9160cab234bd5ba6968ff2d Tree-SHA512: f0ee65f5cea66450566e3a85e066d4c06b3293dd0e0b2ed5fafdb7fb11da0a2cd94407299a3c57a0706c2ed782f8eabb73443e85d8099a62a3fb10a02636ab46 --- src/wallet/rpc/encrypt.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpc/encrypt.cpp b/src/wallet/rpc/encrypt.cpp index 9a0b6896a152..8ea9ed030bff 100644 --- a/src/wallet/rpc/encrypt.cpp +++ b/src/wallet/rpc/encrypt.cpp @@ -220,7 +220,11 @@ RPCHelpMan encryptwallet() "After this, any calls that interact with private keys such as sending or signing \n" "will require the passphrase to be set prior the making these calls.\n" "Use the walletpassphrase call for this, and then walletlock call.\n" - "If the wallet is already encrypted, use the walletpassphrasechange call.\n", + "If the wallet is already encrypted, use the walletpassphrasechange call.\n" + "** IMPORTANT **\n" + "For security reasons, the encryption process will generate a new HD seed, resulting\n" + "in the creation of a fresh set of active descriptors. Therefore, it is crucial to\n" + "securely back up the newly generated wallet file using the backupwallet RPC.\n", { {"passphrase", RPCArg::Type::STR, RPCArg::Optional::NO, "The pass phrase to encrypt the wallet with. It must be at least 1 character, but should be long."}, }, @@ -266,7 +270,7 @@ RPCHelpMan encryptwallet() if (pwallet->IsHDEnabled()) { return "wallet encrypted; If you forget the passphrase, you will lose access to your funds. Make sure that you have backup of your seed or mnemonic."; } - return "wallet encrypted; The keypool has been flushed. You need to make a new backup."; + return "wallet encrypted; The keypool has been flushed. You need to make a new backup with the backupwallet RPC."; }, }; } From cfcbc8d48c2d0d6f59adc31ebf3dbc746e9b7bbe Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 28 Nov 2023 16:20:20 -0500 Subject: [PATCH 4/5] Merge bitcoin/bitcoin#28554: bugfix: throw an error if an invalid parameter is passed to getnetworkhashps RPC 9ac114e5cd9d8ade3a1d9f3d76a08ff59a3f1658 Throw error if invalid parameters passed to getnetworkhashps RPC endpoint (Jameson Lopp) Pull request description: When writing some scripts that iterated over many blocks to generate hashrate estimates I realized that my script was going out of range of the current chain tip height but was not encountering any errors. I believe that passing an invalid block height to this function but receiving the hashrate estimate for the chain tip instead should be considered unexpected behavior. ACKs for top commit: Sjors: re-utACK 9ac114e5cd9d8ade3a1d9f3d76a08ff59a3f1658 kevkevinpal: reACK [9ac114e](https://github.com/bitcoin/bitcoin/pull/28554/commits/9ac114e5cd9d8ade3a1d9f3d76a08ff59a3f1658) achow101: ACK 9ac114e5cd9d8ade3a1d9f3d76a08ff59a3f1658 Tree-SHA512: eefb465c2dd654fc48267f444e1809597ec5363cdd131ea9ec812458fed1e4bffbbbb0617d74687c9f7bb16274b598d8292f5eeb7953421e5d2a8dc2cc081f2b --- src/rpc/mining.cpp | 19 ++++++++++++----- test/functional/rpc_blockchain.py | 34 ++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 24b63fe83eec..aab76c8131f2 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -54,13 +54,22 @@ using node::UpdateTime; /** * Return average network hashes per second based on the last 'lookup' blocks, - * or from the last difficulty change if 'lookup' is nonpositive. - * If 'height' is nonnegative, compute the estimate at the time when a given block was found. + * or from the last difficulty change if 'lookup' is -1. + * If 'height' is -1, compute the estimate from current chain tip. + * If 'height' is a valid block height, compute the estimate at the time when a given block was found. */ static UniValue GetNetworkHashPS(int lookup, int height, const CChain& active_chain) { + if (lookup < -1 || lookup == 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid nblocks. Must be a positive number or -1."); + } + + if (height < -1 || height > active_chain.Height()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Block does not exist at specified height"); + } + const CBlockIndex* pb = active_chain.Tip(); - if (height >= 0 && height < active_chain.Height()) { + if (height >= 0) { pb = active_chain[height]; } @@ -68,7 +77,7 @@ static UniValue GetNetworkHashPS(int lookup, int height, const CChain& active_ch return 0; // If lookup is -1, then use blocks since last difficulty change. - if (lookup <= 0) + if (lookup == -1) lookup = pb->nHeight % Params().GetConsensus().DifficultyAdjustmentInterval() + 1; // If lookup is larger than chain, then set it to chain length. @@ -102,7 +111,7 @@ static RPCHelpMan getnetworkhashps() "Pass in [blocks] to override # of blocks, -1 specifies since last difficulty change.\n" "Pass in [height] to estimate the network speed at the time when a certain block was found.\n", { - {"nblocks", RPCArg::Type::NUM, RPCArg::Default{120}, "The number of blocks, or -1 for blocks since last difficulty change."}, + {"nblocks", RPCArg::Type::NUM, RPCArg::Default{120}, "The number of previous blocks to calculate estimate from, or -1 for blocks since last difficulty change."}, {"height", RPCArg::Type::NUM, RPCArg::Default{-1}, "To estimate at the time of the given height."}, }, RPCResult{ diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 2f74a60003ba..f436cccbe1d3 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -428,18 +428,46 @@ def _test_getdifficulty(self): def _test_getnetworkhashps(self): self.log.info("Test getnetworkhashps") - hashes_per_second = self.nodes[0].getnetworkhashps() + assert_raises_rpc_error( + -8, + "Block does not exist at specified height", + lambda: self.nodes[0].getnetworkhashps(100, self.nodes[0].getblockcount() + 1), + ) + assert_raises_rpc_error( + -8, + "Block does not exist at specified height", + lambda: self.nodes[0].getnetworkhashps(100, -10), + ) + assert_raises_rpc_error( + -8, + "Invalid nblocks. Must be a positive number or -1.", + lambda: self.nodes[0].getnetworkhashps(-100), + ) + assert_raises_rpc_error( + -8, + "Invalid nblocks. Must be a positive number or -1.", + lambda: self.nodes[0].getnetworkhashps(0), + ) + + # Genesis block height estimate should return 0 + hashes_per_second = self.nodes[0].getnetworkhashps(100, 0) + assert_equal(hashes_per_second, 0) + # This should be 2 hashes every 2.6 minutes (156 seconds) or 1/78 + hashes_per_second = self.nodes[0].getnetworkhashps() assert abs(hashes_per_second * 78 - 1) < 0.0001 - # Test setting the first param of getnetworkhashps to negative value returns the average network + # Test setting the first param of getnetworkhashps to -1 returns the average network # hashes per second from the last difficulty change. current_block_height = self.nodes[0].getmininginfo()['blocks'] blocks_since_last_diff_change = current_block_height % DIFFICULTY_ADJUSTMENT_INTERVAL + 1 expected_hashes_per_second_since_diff_change = self.nodes[0].getnetworkhashps(blocks_since_last_diff_change) assert_equal(self.nodes[0].getnetworkhashps(-1), expected_hashes_per_second_since_diff_change) - assert_equal(self.nodes[0].getnetworkhashps(-2), expected_hashes_per_second_since_diff_change) + + # Ensure long lookups get truncated to chain length + hashes_per_second = self.nodes[0].getnetworkhashps(self.nodes[0].getblockcount() + 1000) + assert hashes_per_second > 0.003 def _test_stopatheight(self): assert_equal(self.nodes[0].getblockcount(), HEIGHT) From d2ce6669dc81f5d3f261e4a048ab4177288a3f70 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 12 Sep 2023 12:18:52 -0400 Subject: [PATCH 5/5] Merge bitcoin/bitcoin#28414: wallet rpc: return final tx hex from walletprocesspsbt if complete 2e249b922762f19d6ae61edaad062f31bc2849f3 doc: add release note for PR #28414 (Matthew Zipkin) 4614332fc4514f63fcbe9e6de507f7bb9b7e87e9 test: remove unnecessary finalizepsbt rpc calls (ismaelsadeeq) e3d484b603abff69c6ebfca5cfb78cf82743d090 wallet rpc: return final tx hex from walletprocesspsbt if complete (Matthew Zipkin) Pull request description: See https://github.com/bitcoin/bitcoin/pull/28363#discussion_r1315753887 `walletprocesspsbt` currently returns a base64-encoded PSBT and a boolean indicating if the tx is "complete". If it is complete, the base64 PSBT can be finalized with `finalizepsbt` which returns the hex-encoded transaction suitable for `sendrawtransaction`. With this patch, `walletprocesspsbt` return object will ALSO include the broadcast-able hex string if the tx is already final. This saves users the extra step of calling `finalizepsbt` assuming they have already inspected and approve the transaction from earlier steps. ACKs for top commit: ismaelsadeeq: re ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 BrandonOdiwuor: re ACK 2e249b9 Randy808: Tested ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 achow101: ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 ishaanam: ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 Tree-SHA512: 229c1103265a9b4248f080935a7ad5607c3be3f9a096a9ab6554093b2cd8aa8b4d1fa55b1b97d3925ba208dbc3ccba4e4d37c40e1491db0d27ba3d9fe98f931e --- doc/release-notes-28414.md | 5 +++ src/wallet/rpc/spend.cpp | 9 ++++++ test/functional/rpc_psbt.py | 32 ++++++++++++------- test/functional/wallet_fundrawtransaction.py | 3 +- .../wallet_multisig_descriptor_psbt.py | 3 +- test/functional/wallet_send.py | 5 +-- test/functional/wallet_signer.py | 3 +- 7 files changed, 38 insertions(+), 22 deletions(-) create mode 100644 doc/release-notes-28414.md diff --git a/doc/release-notes-28414.md b/doc/release-notes-28414.md new file mode 100644 index 000000000000..7fca11f8220b --- /dev/null +++ b/doc/release-notes-28414.md @@ -0,0 +1,5 @@ +RPC Wallet +---------- + +- RPC `walletprocesspsbt` return object now includes field `hex` (if the transaction +is complete) containing the serialized transaction suitable for RPC `sendrawtransaction`. (#28414) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index f44eedff2bc8..c1b040a0e8cd 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1278,6 +1278,7 @@ RPCHelpMan walletprocesspsbt() { {RPCResult::Type::STR, "psbt", "The base64-encoded partially signed transaction"}, {RPCResult::Type::BOOL, "complete", "If the transaction has a complete set of signatures"}, + {RPCResult::Type::STR_HEX, "hex", /*optional=*/true, "The hex-encoded network transaction if complete"}, } }, RPCExamples{ @@ -1327,6 +1328,14 @@ RPCHelpMan walletprocesspsbt() ssTx << psbtx; result.pushKV("psbt", EncodeBase64(ssTx.str())); result.pushKV("complete", complete); + if (complete) { + CMutableTransaction mtx; + // Returns true if complete, which we already think it is. + CHECK_NONFATAL(FinalizeAndExtractPSBT(psbtx, mtx)); + CDataStream ssTx_final(SER_NETWORK, PROTOCOL_VERSION); + ssTx_final << mtx; + result.pushKV("hex", HexStr(ssTx_final)); + } return result; }, diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 5788ff158336..688f4a117de7 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -77,12 +77,21 @@ def run_test(self): self.nodes[0].walletpassphrase(passphrase="password", timeout=1000000) - # Sign the transaction and send - signed_tx = self.nodes[0].walletprocesspsbt(psbt=psbtx, finalize=False)['psbt'] - finalized_tx = self.nodes[0].walletprocesspsbt(psbt=psbtx, finalize=True)['psbt'] - assert signed_tx != finalized_tx - final_tx = self.nodes[0].finalizepsbt(signed_tx)['hex'] - self.nodes[0].sendrawtransaction(final_tx) + # Sign the transaction but don't finalize + processed_psbt = self.nodes[0].walletprocesspsbt(psbt=psbtx, finalize=False) + assert "hex" not in processed_psbt + signed_psbt = processed_psbt['psbt'] + + # Finalize and send + finalized_hex = self.nodes[0].finalizepsbt(signed_psbt)['hex'] + self.nodes[0].sendrawtransaction(finalized_hex) + + # Alternative method: sign AND finalize in one command + processed_finalized_psbt = self.nodes[0].walletprocesspsbt(psbt=psbtx, finalize=True) + finalized_psbt = processed_finalized_psbt['psbt'] + finalized_psbt_hex = processed_finalized_psbt['hex'] + assert signed_psbt != finalized_psbt + assert finalized_psbt_hex == finalized_hex # Manually selected inputs can be locked: assert_equal(len(self.nodes[0].listlockunspent()), 0) @@ -139,7 +148,7 @@ def run_test(self): # Check decodepsbt fee calculation (input values shall only be counted once per UTXO) assert_equal(decoded['fee'], created_psbt['fee']) assert_equal(walletprocesspsbt_out['complete'], True) - self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex']) + self.nodes[1].sendrawtransaction(walletprocesspsbt_out['hex']) self.log.info("Test walletcreatefundedpsbt fee rate of 10000 sat/vB and 0.1 BTC/kvB produces a total fee at or slightly below -maxtxfee (~0.05290000)") res1 = self.nodes[1].walletcreatefundedpsbt(inputs, outputs, 0, {"fee_rate": 10000, "add_inputs": True}) @@ -230,7 +239,7 @@ def run_test(self): # partially sign with node 2. This should be complete and sendable walletprocesspsbt_out = self.nodes[2].walletprocesspsbt(psbtx) assert_equal(walletprocesspsbt_out['complete'], True) - self.nodes[2].sendrawtransaction(self.nodes[2].finalizepsbt(walletprocesspsbt_out['psbt'])['hex']) + self.nodes[2].sendrawtransaction(walletprocesspsbt_out['hex']) # check that walletprocesspsbt fails to decode a non-psbt rawtx = self.nodes[1].createrawtransaction([{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():9.99}) @@ -514,14 +523,13 @@ def test_psbt_input_keys(psbt_input, keys): assert not signed['complete'] signed = self.nodes[0].walletprocesspsbt(signed['psbt']) assert signed['complete'] - self.nodes[0].finalizepsbt(signed['psbt']) psbt = wallet.walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {"add_inputs": True, "solving_data":{"descriptors": [desc]}}) signed = wallet.walletprocesspsbt(psbt['psbt']) assert not signed['complete'] signed = self.nodes[0].walletprocesspsbt(signed['psbt']) assert signed['complete'] - final = self.nodes[0].finalizepsbt(signed['psbt'], False) + final = signed['hex'] dec = self.nodes[0].decodepsbt(signed["psbt"]) for i, txin in enumerate(dec["tx"]["vin"]): @@ -555,8 +563,8 @@ def test_psbt_input_keys(psbt_input, keys): ) signed = wallet.walletprocesspsbt(psbt["psbt"]) signed = self.nodes[0].walletprocesspsbt(signed["psbt"]) - final = self.nodes[0].finalizepsbt(signed["psbt"]) - assert self.nodes[0].testmempoolaccept([final["hex"]])[0]["allowed"] + final = signed["hex"] + assert self.nodes[0].testmempoolaccept([final])[0]["allowed"] # Reducing the weight should have a lower fee psbt2 = wallet.walletcreatefundedpsbt( inputs=[{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "size": low_input_weight}], diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 2d54742337c4..a1c0aed0b1e3 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -530,8 +530,7 @@ def test_spend_2of2(self): funded_psbt = wmulti.walletcreatefundedpsbt(inputs=inputs, outputs=outputs, options={'changeAddress': w2.getrawchangeaddress()})['psbt'] signed_psbt = w2.walletprocesspsbt(funded_psbt) - final_psbt = w2.finalizepsbt(signed_psbt['psbt']) - self.nodes[2].sendrawtransaction(final_psbt['hex']) + self.nodes[2].sendrawtransaction(signed_psbt['hex']) self.generate(self.nodes[2], 1) # Make sure funds are received at node1. diff --git a/test/functional/wallet_multisig_descriptor_psbt.py b/test/functional/wallet_multisig_descriptor_psbt.py index ccd89379a6de..40b291212e3d 100755 --- a/test/functional/wallet_multisig_descriptor_psbt.py +++ b/test/functional/wallet_multisig_descriptor_psbt.py @@ -150,8 +150,7 @@ def run_test(self): signing_wallet = participants["signers"][m] psbt = signing_wallet.walletprocesspsbt(psbt["psbt"]) assert_equal(psbt["complete"], m == self.M - 1) - finalized = coordinator_wallet.finalizepsbt(psbt["psbt"]) - coordinator_wallet.sendrawtransaction(finalized["hex"]) + coordinator_wallet.sendrawtransaction(psbt["hex"]) self.log.info("Check that balances are correct after the transaction has been included in a block.") self.generate(self.nodes[0], 1) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 289fa5468fe1..d7479267c26a 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -498,13 +498,11 @@ def run_test(self): signed = ext_wallet.walletprocesspsbt(res["psbt"]) signed = ext_fund.walletprocesspsbt(res["psbt"]) assert signed["complete"] - self.nodes[0].finalizepsbt(signed["psbt"]) res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"descriptors": [desc]}) signed = ext_wallet.walletprocesspsbt(res["psbt"]) signed = ext_fund.walletprocesspsbt(res["psbt"]) assert signed["complete"] - self.nodes[0].finalizepsbt(signed["psbt"]) dec = self.nodes[0].decodepsbt(signed["psbt"]) for i, txin in enumerate(dec["tx"]["vin"]): @@ -539,8 +537,7 @@ def run_test(self): signed = ext_wallet.walletprocesspsbt(res["psbt"]) signed = ext_fund.walletprocesspsbt(res["psbt"]) assert signed["complete"] - tx = self.nodes[0].finalizepsbt(signed["psbt"]) - testres = self.nodes[0].testmempoolaccept([tx["hex"]])[0] + testres = self.nodes[0].testmempoolaccept([signed["hex"]])[0] assert_equal(testres["allowed"], True) assert_fee_amount(testres["fees"]["base"], testres["vsize"], Decimal(0.0001)) diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index eb0610cb256c..b77ab9097c7a 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -148,8 +148,7 @@ def test_valid_signer(self): dest = self.nodes[0].getnewaddress() mock_psbt = mock_wallet.walletcreatefundedpsbt([], {dest:0.5}, 0, {}, True)['psbt'] mock_psbt_signed = mock_wallet.walletprocesspsbt(psbt=mock_psbt, sign=True, sighashtype="ALL", bip32derivs=True) - mock_psbt_final = mock_wallet.finalizepsbt(mock_psbt_signed["psbt"]) - mock_tx = mock_psbt_final["hex"] + mock_tx = mock_psbt_signed["hex"] assert mock_wallet.testmempoolaccept([mock_tx])[0]["allowed"] # # Create a new wallet and populate with specific public keys, in order