diff options
author | Andrew Chow <github@achow101.com> | 2022-10-13 13:24:37 -0400 |
---|---|---|
committer | Andrew Chow <github@achow101.com> | 2022-10-13 13:31:54 -0400 |
commit | 885366c67addd86bf06629919aaa6f74a76f44f4 (patch) | |
tree | 33a4783ec25bd9754bf28bcfb1fa737ca3d2d20f | |
parent | 3f385c912ea351ab90ba09f8793b499e91a36d74 (diff) | |
parent | e2e4c2969ba753a94587985582123a596f57067b (diff) |
Merge bitcoin/bitcoin#26133: [24.x] Backports for rc2
e2e4c2969ba753a94587985582123a596f57067b tests: Test that PSBT_OUT_TAP_TREE is included correctly (Andrew Chow)
4d42c3a2401c63084bf94433609240daa366753e psbt: Only include m_tap_tree if it has scripts (Andrew Chow)
d810fde8ea64b71567f8b50895ac76bcb7afbfbc psbt: Change m_tap_tree to store just the tuples (Andrew Chow)
a9419eff0cc21d21755165e66cc0e496aab65650 tests: Test that PSBT_OUT_TAP_TREE is combined correctly (Andrew Chow)
4abd2ab18e26999e2dafcb15a58a7979de90af34 psbt: Fix merging of m_tap_tree (Andrew Chow)
1390c96c8e9c56e0d8348ef056f18e3e04f0f63f [BugFix]: Do not allow deserializing PSBT with empty PSBT_OUT_TAP_TREE (Jeremy Rubin)
9b438f06ecfc3fb21d2c5219b71fb4aa77875b8c refactor: revert m_next_resend to not be std::atomic (stickies-v)
43ced0b436b05ed12489a99bbac89f3b4c9ac035 wallet: only update m_next_resend when actually resending (stickies-v)
fc8f2bfa3abc284ae3c1127fcf36535603ecc891 refactor: carve out tx resend timer logic into ShouldResend (stickies-v)
a6fb674f966df27c09dc3d2b81040ce2965b2d7e refactor: remove unused locks for ResubmitWalletTransactions (stickies-v)
5ad82a09b409d416236092062a4201e238dfd68b index: Improve BaseIndex::BlockUntilSyncedToCurrentChain reliability (Ryan Ofsky)
997faf6b6c774dc87ae730f2f08d7f4f08bdfd04 contrib: Fix capture_output in getcoins.py (willcl-ark)
7e0bcfbfef61cb688bc92a96003c1219cad67935 p2p: ProcessHeadersMessage(): fix received_new_header (Larry Ruane)
c97d924880eaad136c5f7776f05bf887657ccca7 Correct sanity-checking script_size calculation (Pieter Wuille)
da6fba6fe785ba2c54f9b88dd5b1b4ceb02c18c9 docs: Add 371 to bips.md (Andrew Chow)
Pull request description:
Will collect backports for rc2 as they become available. Currently:
* https://github.com/bitcoin/bitcoin/pull/25858
* https://github.com/bitcoin/bitcoin/pull/26124
* https://github.com/bitcoin/bitcoin/pull/26149
* https://github.com/bitcoin/bitcoin/pull/26172
* https://github.com/bitcoin/bitcoin/pull/26205
* https://github.com/bitcoin/bitcoin/pull/26212
* https://github.com/bitcoin/bitcoin/pull/26215
ACKs for top commit:
dergoegge:
ACK e2e4c2969ba753a94587985582123a596f57067b
achow101:
ACK e2e4c2969ba753a94587985582123a596f57067b
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/26133/commits/e2e4c2969ba753a94587985582123a596f57067b
Tree-SHA512: b6374fe202561057dbe1430d4c40f06f721eb568f91e7275ae1ee7747edf780ce64620382d13ecc4b9571d931dc25d226af8284987cf35ff6a6182c5f64eb10c
-rwxr-xr-x | contrib/signet/getcoins.py | 2 | ||||
-rw-r--r-- | doc/bips.md | 1 | ||||
-rw-r--r-- | src/index/base.cpp | 13 | ||||
-rw-r--r-- | src/net_processing.cpp | 2 | ||||
-rw-r--r-- | src/psbt.cpp | 16 | ||||
-rw-r--r-- | src/psbt.h | 25 | ||||
-rw-r--r-- | src/rpc/rawtransaction.cpp | 8 | ||||
-rw-r--r-- | src/script/miniscript.h | 2 | ||||
-rw-r--r-- | src/script/standard.h | 2 | ||||
-rw-r--r-- | src/wallet/rpc/backup.cpp | 20 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 40 | ||||
-rw-r--r-- | src/wallet/wallet.h | 8 | ||||
-rw-r--r-- | test/functional/data/rpc_psbt.json | 4 | ||||
-rwxr-xr-x | test/functional/rpc_psbt.py | 22 | ||||
-rw-r--r-- | test/functional/test_framework/psbt.py | 9 |
15 files changed, 109 insertions, 65 deletions
diff --git a/contrib/signet/getcoins.py b/contrib/signet/getcoins.py index 147d12600d..a069f5fad3 100755 --- a/contrib/signet/getcoins.py +++ b/contrib/signet/getcoins.py @@ -129,7 +129,7 @@ if args.captcha != '': # Retrieve a captcha # Convert SVG image to PPM, and load it try: - rv = subprocess.run([args.imagemagick, 'svg:-', '-depth', '8', 'ppm:-'], input=res.content, check=True, capture_output=True) + rv = subprocess.run([args.imagemagick, 'svg:-', '-depth', '8', 'ppm:-'], input=res.content, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) except FileNotFoundError: raise SystemExit(f"The binary {args.imagemagick} could not be found. Please make sure ImageMagick (or a compatible fork) is installed and that the correct path is specified.") diff --git a/doc/bips.md b/doc/bips.md index 4119d86aaa..25381818e4 100644 --- a/doc/bips.md +++ b/doc/bips.md @@ -58,6 +58,7 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v24.0**): with mainnet activation as of **v0.21.1** ([PR 21377](https://github.com/bitcoin/bitcoin/pull/21377), [PR 21686](https://github.com/bitcoin/bitcoin/pull/21686)). * [`BIP 350`](https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki): Addresses for native v1+ segregated Witness outputs use Bech32m instead of Bech32 as of **v22.0** ([PR 20861](https://github.com/bitcoin/bitcoin/pull/20861)). +* [`BIP 371`](https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki): Taproot fields for PSBT as of **v24.0** ([PR 22558](https://github.com/bitcoin/bitcoin/pull/22558)). * [`BIP 380`](https://github.com/bitcoin/bips/blob/master/bip-0380.mediawiki) [`381`](https://github.com/bitcoin/bips/blob/master/bip-0381.mediawiki) [`382`](https://github.com/bitcoin/bips/blob/master/bip-0382.mediawiki) diff --git a/src/index/base.cpp b/src/index/base.cpp index 88c2ce98fa..3eea09b17d 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -298,6 +298,10 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const } interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex, block.get()); if (CustomAppend(block_info)) { + // Setting the best block index is intentionally the last step of this + // function, so BlockUntilSyncedToCurrentChain callers waiting for the + // best block index to be updated can rely on the block being fully + // processed, and the index object being safe to delete. SetBestBlockIndex(pindex); } else { FatalError("%s: Failed to write block %s to index", @@ -414,10 +418,17 @@ IndexSummary BaseIndex::GetSummary() const void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) { assert(!node::fPruneMode || AllowPrune()); - m_best_block_index = block; if (AllowPrune() && block) { node::PruneLockInfo prune_lock; prune_lock.height_first = block->nHeight; WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock)); } + + // Intentionally set m_best_block_index as the last step in this function, + // after updating prune locks above, and after making any other references + // to *this, so the BlockUntilSyncedToCurrentChain function (which checks + // m_best_block_index as an optimization) can be used to wait for the last + // BlockConnected notification and safely assume that prune locks are + // updated and that the index object is safe to delete. + m_best_block_index = block; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 74700580ad..1c44ec9903 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2843,7 +2843,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, // If we don't have the last header, then this peer will have given us // something new (if these headers are valid). - bool received_new_header{last_received_header != nullptr}; + bool received_new_header{last_received_header == nullptr}; // Now process all the headers. BlockValidationState state; diff --git a/src/psbt.cpp b/src/psbt.cpp index 36fec74bc9..cbf2f88788 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -218,8 +218,14 @@ void PSBTOutput::FillSignatureData(SignatureData& sigdata) const for (const auto& key_pair : hd_keypaths) { sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair); } - if (m_tap_tree.has_value() && m_tap_internal_key.IsFullyValid()) { - TaprootSpendData spenddata = m_tap_tree->GetSpendData(); + if (!m_tap_tree.empty() && m_tap_internal_key.IsFullyValid()) { + TaprootBuilder builder; + for (const auto& [depth, leaf_ver, script] : m_tap_tree) { + builder.Add((int)depth, script, (int)leaf_ver, /*track=*/true); + } + assert(builder.IsComplete()); + builder.Finalize(m_tap_internal_key); + TaprootSpendData spenddata = builder.GetSpendData(); sigdata.tr_spenddata.internal_key = m_tap_internal_key; sigdata.tr_spenddata.Merge(spenddata); @@ -243,8 +249,8 @@ void PSBTOutput::FromSignatureData(const SignatureData& sigdata) if (!sigdata.tr_spenddata.internal_key.IsNull()) { m_tap_internal_key = sigdata.tr_spenddata.internal_key; } - if (sigdata.tr_builder.has_value()) { - m_tap_tree = sigdata.tr_builder; + if (sigdata.tr_builder.has_value() && sigdata.tr_builder->HasScripts()) { + m_tap_tree = sigdata.tr_builder->GetTreeTuples(); } for (const auto& [pubkey, leaf_origin] : sigdata.taproot_misc_pubkeys) { m_tap_bip32_paths.emplace(pubkey, leaf_origin); @@ -265,7 +271,7 @@ void PSBTOutput::Merge(const PSBTOutput& output) if (redeem_script.empty() && !output.redeem_script.empty()) redeem_script = output.redeem_script; if (witness_script.empty() && !output.witness_script.empty()) witness_script = output.witness_script; if (m_tap_internal_key.IsNull() && !output.m_tap_internal_key.IsNull()) m_tap_internal_key = output.m_tap_internal_key; - if (m_tap_tree.has_value() && !output.m_tap_tree.has_value()) m_tap_tree = output.m_tap_tree; + if (m_tap_tree.empty() && !output.m_tap_tree.empty()) m_tap_tree = output.m_tap_tree; } bool PSBTInputSigned(const PSBTInput& input) { diff --git a/src/psbt.h b/src/psbt.h index d5c67802c7..ddcdb8c68d 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -713,7 +713,7 @@ struct PSBTOutput CScript witness_script; std::map<CPubKey, KeyOriginInfo> hd_keypaths; XOnlyPubKey m_tap_internal_key; - std::optional<TaprootBuilder> m_tap_tree; + std::vector<std::tuple<uint8_t, uint8_t, CScript>> m_tap_tree; std::map<XOnlyPubKey, std::pair<std::set<uint256>, KeyOriginInfo>> m_tap_bip32_paths; std::map<std::vector<unsigned char>, std::vector<unsigned char>> unknown; std::set<PSBTProprietary> m_proprietary; @@ -754,15 +754,11 @@ struct PSBTOutput } // Write taproot tree - if (m_tap_tree.has_value()) { + if (!m_tap_tree.empty()) { SerializeToVector(s, PSBT_OUT_TAP_TREE); std::vector<unsigned char> value; CVectorWriter s_value(s.GetType(), s.GetVersion(), value, 0); - const auto& tuples = m_tap_tree->GetTreeTuples(); - for (const auto& tuple : tuples) { - uint8_t depth = std::get<0>(tuple); - uint8_t leaf_ver = std::get<1>(tuple); - CScript script = std::get<2>(tuple); + for (const auto& [depth, leaf_ver, script] : m_tap_tree) { s_value << depth; s_value << leaf_ver; s_value << script; @@ -858,10 +854,13 @@ struct PSBTOutput } else if (key.size() != 1) { throw std::ios_base::failure("Output Taproot tree key is more than one byte type"); } - m_tap_tree.emplace(); std::vector<unsigned char> tree_v; s >> tree_v; SpanReader s_tree(s.GetType(), s.GetVersion(), tree_v); + if (s_tree.empty()) { + throw std::ios_base::failure("Output Taproot tree must not be empty"); + } + TaprootBuilder builder; while (!s_tree.empty()) { uint8_t depth; uint8_t leaf_ver; @@ -875,9 +874,10 @@ struct PSBTOutput if ((leaf_ver & ~TAPROOT_LEAF_MASK) != 0) { throw std::ios_base::failure("Output Taproot tree has a leaf with an invalid leaf version"); } - m_tap_tree->Add((int)depth, script, (int)leaf_ver, true /* track */); + m_tap_tree.push_back(std::make_tuple(depth, leaf_ver, script)); + builder.Add((int)depth, script, (int)leaf_ver, true /* track */); } - if (!m_tap_tree->IsComplete()) { + if (!builder.IsComplete()) { throw std::ios_base::failure("Output Taproot tree is malformed"); } break; @@ -931,11 +931,6 @@ struct PSBTOutput } } - // Finalize m_tap_tree so that all of the computed things are computed - if (m_tap_tree.has_value() && m_tap_tree->IsComplete() && m_tap_internal_key.IsFullyValid()) { - m_tap_tree->Finalize(m_tap_internal_key); - } - if (!found_sep) { throw std::ios_base::failure("Separator is missing at the end of an output map"); } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index f365de7d0c..d654de1862 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1241,13 +1241,9 @@ static RPCHelpMan decodepsbt() } // Taproot tree - if (output.m_tap_tree.has_value()) { + if (!output.m_tap_tree.empty()) { UniValue tree(UniValue::VARR); - const auto& tuples = output.m_tap_tree->GetTreeTuples(); - for (const auto& tuple : tuples) { - uint8_t depth = std::get<0>(tuple); - uint8_t leaf_ver = std::get<1>(tuple); - CScript script = std::get<2>(tuple); + for (const auto& [depth, leaf_ver, script] : output.m_tap_tree) { UniValue elem(UniValue::VOBJ); elem.pushKV("depth", (int)depth); elem.pushKV("leaf_ver", (int)leaf_ver); diff --git a/src/script/miniscript.h b/src/script/miniscript.h index ab25fa67b7..c4f41e0adf 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -1221,7 +1221,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) // n = 1 here because we read the first WRAPPED_EXPR before reaching THRESH to_parse.emplace_back(ParseContext::THRESH, 1, k); to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); - script_size += 2 + (k > 16); + script_size += 2 + (k > 16) + (k > 0x7f) + (k > 0x7fff) + (k > 0x7fffff); } else if (Const("andor(", in)) { to_parse.emplace_back(ParseContext::ANDOR, -1, -1); to_parse.emplace_back(ParseContext::CLOSE_BRACKET, -1, -1); diff --git a/src/script/standard.h b/src/script/standard.h index 1e6769782a..966a52b2c7 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -315,6 +315,8 @@ public: TaprootSpendData GetSpendData() const; /** Returns a vector of tuples representing the depth, leaf version, and script */ std::vector<std::tuple<uint8_t, uint8_t, CScript>> GetTreeTuples() const; + /** Returns true if there are any tapscripts */ + bool HasScripts() const { return !m_branch.empty(); } }; /** Given a TaprootSpendData and the output key, reconstruct its script tree. diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 35df151e84..a971331a70 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -293,10 +293,7 @@ RPCHelpMan importaddress() if (fRescan) { RescanWallet(*pwallet, reserver); - { - LOCK(pwallet->cs_wallet); - pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); - } + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); } return UniValue::VNULL; @@ -474,10 +471,7 @@ RPCHelpMan importpubkey() if (fRescan) { RescanWallet(*pwallet, reserver); - { - LOCK(pwallet->cs_wallet); - pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); - } + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); } return UniValue::VNULL; @@ -1395,10 +1389,7 @@ RPCHelpMan importmulti() } if (fRescan && fRunScan && requests.size()) { int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, reserver, true /* update */); - { - LOCK(pwallet->cs_wallet); - pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); - } + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); if (pwallet->IsAbortingRescan()) { throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted by user."); @@ -1689,10 +1680,7 @@ RPCHelpMan importdescriptors() // Rescan the blockchain using the lowest timestamp if (rescan) { int64_t scanned_time = pwallet->RescanFromTime(lowest_timestamp, reserver, true /* update */); - { - LOCK(pwallet->cs_wallet); - pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); - } + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); if (pwallet->IsAbortingRescan()) { throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted by user."); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5889de2e36..ac7bf46a14 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -21,6 +21,7 @@ #include <primitives/block.h> #include <primitives/transaction.h> #include <psbt.h> +#include <random.h> #include <script/descriptor.h> #include <script/script.h> #include <script/signingprovider.h> @@ -1903,11 +1904,29 @@ std::set<uint256> CWallet::GetTxConflicts(const CWalletTx& wtx) const return result; } +bool CWallet::ShouldResend() const +{ + // Don't attempt to resubmit if the wallet is configured to not broadcast + if (!fBroadcastTransactions) return false; + + // During reindex, importing and IBD, old wallet transactions become + // unconfirmed. Don't resend them as that would spam other nodes. + // We only allow forcing mempool submission when not relaying to avoid this spam. + if (!chain().isReadyToBroadcast()) return false; + + // Do this infrequently and randomly to avoid giving away + // that these are our transactions. + if (GetTime() < m_next_resend) return false; + + return true; +} + +int64_t CWallet::GetDefaultNextResend() { return GetTime() + (12 * 60 * 60) + GetRand(24 * 60 * 60); } + // Resubmit transactions from the wallet to the mempool, optionally asking the // mempool to relay them. On startup, we will do this for all unconfirmed // transactions but will not ask the mempool to relay them. We do this on startup -// to ensure that our own mempool is aware of our transactions, and to also -// initialize m_next_resend so that the actual rebroadcast is scheduled. There +// to ensure that our own mempool is aware of our transactions. There // is a privacy side effect here as not broadcasting on startup also means that we won't // inform the world of our wallet's state, particularly if the wallet (or node) is not // yet synced. @@ -1934,17 +1953,6 @@ void CWallet::ResubmitWalletTransactions(bool relay, bool force) // even if forcing. if (!fBroadcastTransactions) return; - // During reindex, importing and IBD, old wallet transactions become - // unconfirmed. Don't resend them as that would spam other nodes. - // We only allow forcing mempool submission when not relaying to avoid this spam. - if (!force && relay && !chain().isReadyToBroadcast()) return; - - // Do this infrequently and randomly to avoid giving away - // that these are our transactions. - if (!force && GetTime() < m_next_resend) return; - // resend 12-36 hours from now, ~1 day on average. - m_next_resend = GetTime() + (12 * 60 * 60) + GetRand(24 * 60 * 60); - int submitted_tx_count = 0; { // cs_wallet scope @@ -1979,7 +1987,9 @@ void CWallet::ResubmitWalletTransactions(bool relay, bool force) void MaybeResendWalletTxs(WalletContext& context) { for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) { + if (!pwallet->ShouldResend()) continue; pwallet->ResubmitWalletTransactions(/*relay=*/true, /*force=*/false); + pwallet->SetNextResend(); } } @@ -3196,14 +3206,12 @@ bool CWallet::UpgradeWallet(int version, bilingual_str& error) void CWallet::postInitProcess() { - LOCK(cs_wallet); - // Add wallet transactions that aren't already in a block to mempool // Do this here as mempool requires genesis block to be loaded ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); // Update wallet transactions with current mempool transactions. - chain().requestMempoolTransactions(*this); + WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this)); } bool CWallet::BackupWallet(const std::string& strDest) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6a1b76505c..2f10c598b5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -250,7 +250,7 @@ private: int nWalletVersion GUARDED_BY(cs_wallet){FEATURE_BASE}; /** The next scheduled rebroadcast of wallet transactions. */ - std::atomic<int64_t> m_next_resend{}; + int64_t m_next_resend{GetDefaultNextResend()}; /** Whether this wallet will submit newly created transactions to the node's mempool and * prompt rebroadcasts (see ResendWalletTransactions()). */ bool fBroadcastTransactions = false; @@ -348,6 +348,8 @@ private: */ static bool AttachChain(const std::shared_ptr<CWallet>& wallet, interfaces::Chain& chain, const bool rescan_required, bilingual_str& error, std::vector<bilingual_str>& warnings); + static int64_t GetDefaultNextResend(); + public: /** * Main wallet lock. @@ -537,6 +539,10 @@ public: }; ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress); void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override; + /** Set the next time this wallet should resend transactions to 12-36 hours from now, ~1 day on average. */ + void SetNextResend() { m_next_resend = GetDefaultNextResend(); } + /** Return true if all conditions for periodically resending transactions are met. */ + bool ShouldResend() const; void ResubmitWalletTransactions(bool relay, bool force); OutputType TransactionChangeType(const std::optional<OutputType>& change_type, const std::vector<CRecipient>& vecSend) const; diff --git a/test/functional/data/rpc_psbt.json b/test/functional/data/rpc_psbt.json index 657faebffc..3127350872 100644 --- a/test/functional/data/rpc_psbt.json +++ b/test/functional/data/rpc_psbt.json @@ -44,6 +44,10 @@ [ "cHNidP8BAKOro2MDAwMDA5ggCAAA////CQAtAAD+///1AAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAJAAAAAAAAAAAAAAAAAAAAAAAAAD+///1Zm9ybmV3nWx1Y2vmelLmegAAAAAAAAAAAAAAAAAAAAMKAwMDAwMDAwMDAwMACvMBA3FkAAAAAAAAAAAABAAlAAAAAAAAACEWDQ0zDQ0NDQ0NDQ0NCwEAAH9/f39/fwMAAABNo6P///kAAA==", "Input Taproot BIP32 keypath has an invalid length" + ], + [ + "cHNidP8BAIkCAAAAAapfm08b0MipBvW9thL06f8rMbeazW7TIa0W9plHj4WoAAAAAAD9////AoCWmAAAAAAAIlEgC+blBlIP1iijRWxqjw1u9H02sqr7y8fno6/LdnvGqPl895x2AAAAACJRIM5wyjSexMbADl4K+AI1/68zyaDlE7guKvrEDUAjwqU1AAAAAAABASsAlDV3AAAAACJRIDfCpO/CIAqc0JKgBhsCfaPGdyroYtmH+4gQK/Mnn72UIRZGOixxmh9h2gqDIecYHcQHRa8w+Sokc//iDiqXz7uMGRkAHzYIzlYAAIABAACAAAAAgAAAAABhAAAAARcgRjoscZofYdoKgyHnGB3EB0WvMPkqJHP/4g4ql8+7jBkAAQUg1YCB33LpmkGemw3ncz7fcnjhL/bBG/PjH8vpgr2L3cUBBgAhB9WAgd9y6ZpBnpsN53M+33J44S/2wRvz4x/L6YK9i93FGQAfNgjOVgAAgAEAAIAAAACAAAAAAGIAAAAAAQUg9jMNus8cd+GAosBk9wn+pNP9wn7A+jy2Vq0cy+siJ8wBBgAhB/YzDbrPHHfhgKLAZPcJ/qTT/cJ+wPo8tlatHMvrIifMGQAfNgjOVgAAgAEAAIAAAACAAQAAAFEBAAAA", + "Output Taproot tree must not be empty" ] ], "valid" : [ diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 4583ca25cf..1fe3b21542 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -27,6 +27,7 @@ from test_framework.psbt import ( PSBT_IN_SHA256, PSBT_IN_HASH160, PSBT_IN_HASH256, + PSBT_OUT_TAP_TREE, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -779,9 +780,18 @@ class PSBTTest(BitcoinTestFramework): self.generate(self.nodes[0], 1) self.nodes[0].importdescriptors([{"desc": descsum_create("tr({})".format(privkey)), "timestamp":"now"}]) - psbt = watchonly.sendall([wallet.getnewaddress()])["psbt"] + psbt = watchonly.sendall([wallet.getnewaddress(), addr])["psbt"] psbt = self.nodes[0].walletprocesspsbt(psbt)["psbt"] - self.nodes[0].sendrawtransaction(self.nodes[0].finalizepsbt(psbt)["hex"]) + txid = self.nodes[0].sendrawtransaction(self.nodes[0].finalizepsbt(psbt)["hex"]) + vout = find_vout_for_address(self.nodes[0], txid, addr) + + # Make sure tap tree is in psbt + parsed_psbt = PSBT.from_base64(psbt) + assert_greater_than(len(parsed_psbt.o[vout].map[PSBT_OUT_TAP_TREE]), 0) + assert "taproot_tree" in self.nodes[0].decodepsbt(psbt)["outputs"][vout] + parsed_psbt.make_blank() + comb_psbt = self.nodes[0].combinepsbt([psbt, parsed_psbt.to_base64()]) + assert_equal(comb_psbt, psbt) self.log.info("Test that walletprocesspsbt both updates and signs a non-updated psbt containing Taproot inputs") addr = self.nodes[0].getnewaddress("", "bech32m") @@ -793,6 +803,14 @@ class PSBTTest(BitcoinTestFramework): self.nodes[0].sendrawtransaction(rawtx) self.generate(self.nodes[0], 1) + # Make sure tap tree is not in psbt + parsed_psbt = PSBT.from_base64(psbt) + assert PSBT_OUT_TAP_TREE not in parsed_psbt.o[0].map + assert "taproot_tree" not in self.nodes[0].decodepsbt(psbt)["outputs"][0] + parsed_psbt.make_blank() + comb_psbt = self.nodes[0].combinepsbt([psbt, parsed_psbt.to_base64()]) + assert_equal(comb_psbt, psbt) + self.log.info("Test decoding PSBT with per-input preimage types") # note that the decodepsbt RPC doesn't check whether preimages and hashes match hash_ripemd160, preimage_ripemd160 = random_bytes(20), random_bytes(50) diff --git a/test/functional/test_framework/psbt.py b/test/functional/test_framework/psbt.py index 68945e7e84..3a5b4ec74d 100644 --- a/test/functional/test_framework/psbt.py +++ b/test/functional/test_framework/psbt.py @@ -123,6 +123,15 @@ class PSBT: psbt = [x.serialize() for x in [self.g] + self.i + self.o] return b"psbt\xff" + b"".join(psbt) + def make_blank(self): + """ + Remove all fields except for PSBT_GLOBAL_UNSIGNED_TX + """ + for m in self.i + self.o: + m.map.clear() + + self.g = PSBTMap(map={0: self.g.map[0]}) + def to_base64(self): return base64.b64encode(self.serialize()).decode("utf8") |