Merge bitcoin/bitcoin#22512: Consolidate XOnlyPubKey lookup hack

d9d3ec0 Consolidate XOnlyPubKey lookup hack (Andrew Chow)

Pull request description:

  The places where we need to lookup information for a XOnlyPubKey
  currently implement a hack which makes both serializations of the full
  pubkey in order to try the CKeyIDs for the lookup functions. Instead of
  duplicating this everywhere it is needed, we can consolidate the CKeyID
  generation into a function, and then have wrappers around GetPubKey,
  GetKey, and GetKeyOrigin which takes the XOnlyPubKey, retrieves all of
  the CKeyIDs (using the new GetKeyIDs() function in XOnlyPubKey), and
  tries their respective underlying lookup function.

  Split from #22364

ACKs for top commit:
  S3RK:
    Code Review reACK d9d3ec0
  Zero-1729:
    re-crACK d9d3ec0
  theStack:
    re-ACK d9d3ec0
  meshcollider:
    Code review + functional test run ACK d9d3ec0

Tree-SHA512: 21a7f6d37fad74483a38006f82b3558337fe9ed30e0b4392e6fff82c22251a42ac996b43f06cdaa9289ee34a768e181d87aa4208b5538e36ae4977954e1fa6a0

Author: meshcollider

src/pubkey.cpp

@@ -180,6 +180,23 @@ XOnlyPubKey::XOnlyPubKey(Span<const unsigned char> bytes)
     std::copy(bytes.begin(), bytes.end(), m_keydata.begin());
 }
 
+std::vector<CKeyID> XOnlyPubKey::GetKeyIDs() const
+{
+    std::vector<CKeyID> out;
+    // For now, use the old full pubkey-based key derivation logic. As it is indexed by
+    // Hash160(full pubkey), we need to return both a version prefixed with 0x02, and one
+    // with 0x03.
+    unsigned char b[33] = {0x02};
+    std::copy(m_keydata.begin(), m_keydata.end(), b + 1);
+    CPubKey fullpubkey;
+    fullpubkey.Set(b, b + 33);
+    out.push_back(fullpubkey.GetID());
+    b[0] = 0x03;
+    fullpubkey.Set(b, b + 33);
+    out.push_back(fullpubkey.GetID());
+    return out;
+}
+
 bool XOnlyPubKey::IsFullyValid() const
 {
     secp256k1_xonly_pubkey pubkey;

src/pubkey.h

@@ -267,6 +267,11 @@ class XOnlyPubKey
     /** Construct a Taproot tweaked output point with this point as internal key. */
     std::optional<std::pair<XOnlyPubKey, bool>> CreateTapTweak(const uint256* merkle_root) const;
 
+    /** Returns a list of CKeyIDs for the CPubKeys that could have been used to create this XOnlyPubKey.
+     * This is needed for key lookups since keys are indexed by CKeyID.
+     */
+    std::vector<CKeyID> GetKeyIDs() const;
+
     const unsigned char& operator[](int pos) const { return *(m_keydata.begin() + pos); }
     const unsigned char* data() const { return m_keydata.begin(); }
     static constexpr size_t size() { return decltype(m_keydata)::size(); }

src/script/descriptor.cpp

@@ -1242,14 +1242,8 @@ std::unique_ptr<PubkeyProvider> InferXOnlyPubkey(const XOnlyPubKey& xkey, ParseS
     CPubKey pubkey(full_key);
     std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, true);
     KeyOriginInfo info;
-    if (provider.GetKeyOrigin(pubkey.GetID(), info)) {
+    if (provider.GetKeyOriginByXOnly(xkey, info)) {
         return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider));
-    } else {
-        full_key[0] = 0x03;
-        pubkey = CPubKey(full_key);
-        if (provider.GetKeyOrigin(pubkey.GetID(), info)) {
-            return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider));
-        }
     }
     return key_provider;
 }

src/script/sign.cpp

@@ -60,22 +60,7 @@ bool MutableTransactionSignatureCreator::CreateSchnorrSig(const SigningProvider&
     assert(sigversion == SigVersion::TAPROOT || sigversion == SigVersion::TAPSCRIPT);
 
     CKey key;
-    {
-        // For now, use the old full pubkey-based key derivation logic. As it is indexed by
-        // Hash160(full pubkey), we need to try both a version prefixed with 0x02, and one
-        // with 0x03.
-        unsigned char b[33] = {0x02};
-        std::copy(pubkey.begin(), pubkey.end(), b + 1);
-        CPubKey fullpubkey;
-        fullpubkey.Set(b, b + 33);
-        CKeyID keyid = fullpubkey.GetID();
-        if (!provider.GetKey(keyid, key)) {
-            b[0] = 0x03;
-            fullpubkey.Set(b, b + 33);
-            CKeyID keyid = fullpubkey.GetID();
-            if (!provider.GetKey(keyid, key)) return false;
-        }
-    }
+    if (!provider.GetKeyByXOnly(pubkey, key)) return false;
 
     // BIP341/BIP342 signing needs lots of precomputed transaction data. While some
     // (non-SIGHASH_DEFAULT) sighash modes exist that can work with just some subset

src/script/signingprovider.h

@@ -26,6 +26,30 @@ class SigningProvider
     virtual bool HaveKey(const CKeyID &address) const { return false; }
     virtual bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const { return false; }
     virtual bool GetTaprootSpendData(const XOnlyPubKey& output_key, TaprootSpendData& spenddata) const { return false; }
+
+    bool GetKeyByXOnly(const XOnlyPubKey& pubkey, CKey& key) const
+    {
+        for (const auto& id : pubkey.GetKeyIDs()) {
+            if (GetKey(id, key)) return true;
+        }
+        return false;
+    }
+
+    bool GetPubKeyByXOnly(const XOnlyPubKey& pubkey, CPubKey& out) const
+    {
+        for (const auto& id : pubkey.GetKeyIDs()) {
+            if (GetPubKey(id, out)) return true;
+        }
+        return false;
+    }
+
+    bool GetKeyOriginByXOnly(const XOnlyPubKey& pubkey, KeyOriginInfo& info) const
+    {
+        for (const auto& id : pubkey.GetKeyIDs()) {
+            if (GetKeyOrigin(id, info)) return true;
+        }
+        return false;
+    }
 };
 
 extern const SigningProvider& DUMMY_SIGNING_PROVIDER;