Merge pull request #1698 from evoskuil/master

Fix v1 sighash annex serialization.

Author: evoskuil

include/bitcoin/system/chain/annex.hpp

@@ -39,7 +39,7 @@ class BC_API annex
 
     inline size_t size() const NOEXCEPT;
     inline const data_chunk& data() const NOEXCEPT;
-    inline const hash_digest hash() const NOEXCEPT;
+    inline const hash_digest hash(bool prefix) const NOEXCEPT;
     inline operator bool() const NOEXCEPT;
 
     /// The stack adheres to the annex pattern [bip341].

include/bitcoin/system/impl/chain/annex.ipp

@@ -23,6 +23,7 @@
 #include <bitcoin/system/data/data.hpp>
 #include <bitcoin/system/define.hpp>
 #include <bitcoin/system/hash/hash.hpp>
+#include <bitcoin/system/stream/stream.hpp>
 
 namespace libbitcoin {
 namespace system {
@@ -72,9 +73,18 @@ inline const data_chunk& annex::data() const NOEXCEPT
     return data_ ? *data_ : empty_annex();
 }
 
-inline const hash_digest annex::hash() const NOEXCEPT
+inline const hash_digest annex::hash(bool prefix) const NOEXCEPT
 {
-    return sha256_hash(data());
+    if (!prefix)
+        return sha256_hash(data());
+
+    hash_digest out{};
+    stream::out::fast stream{ out };
+    hash::sha256::fast sink{ stream };
+    sink.write_variable(size());
+    sink.write_bytes(data());
+    sink.flush();
+    return out;
 }
 
 // static

src/chain/transaction_sighash_v0.cpp

@@ -72,10 +72,6 @@ bool transaction::version0_sighash(hash_digest& out,
     stream::out::fast stream{ out };
     hash::sha256x2::fast sink{ stream };
 
-    ///////////////////////////////////////////////////////////////////////////
-    // TODO: update cache calls.
-    ///////////////////////////////////////////////////////////////////////////
-
     sink.write_4_bytes_little_endian(version_);
     sink.write_bytes(!anyone ? double_hash_points() : null_hash);
     sink.write_bytes(!anyone && all ? double_hash_sequences() : null_hash);

src/chain/transaction_sighash_v1.cpp

@@ -57,14 +57,15 @@ uint32_t transaction::subscript_v1(const script& script) NOEXCEPT
 // ext_flags and annex flag are combined into one byte, who knows why.
 uint8_t transaction::spend_type_v1(bool annex, bool tapscript) const NOEXCEPT
 {
-    const auto ext_flags = to_value(tapscript ?
-        extension::tapscript : extension::taproot);
+    const auto ext_flag = to_value(tapscript ? extension::tapscript :
+        extension::taproot);
 
-    return set_right(shift_left(ext_flags), zero, annex);
+    return set_right(shift_left(ext_flag), zero, annex);
 }
 
 // NOT THREAD SAFE
 // Concurrent input validation for a tx unsafe due to on-demand hash caching.
+// TODO: may be more optimal to not cache single output hash as use is rare.
 bool transaction::version1_sighash(hash_digest& out,
     const input_iterator& input, const script& script, uint64_t value,
     const hash_cptr& tapleaf, uint8_t sighash_flags) const NOEXCEPT
@@ -80,6 +81,18 @@ bool transaction::version1_sighash(hash_digest& out,
     const auto single = (flag == coverage::hash_single);
     const auto all = (flag == coverage::hash_all);
 
+    // ************************************************************************
+    // CONSENSUS: Guards public interface only, node always populates prevout.
+    // ************************************************************************
+    if (anyone && is_null(in.prevout))
+        return false;
+
+    // ************************************************************************
+    // CONSENSUS: Taproot finally eliminates one_hash (null_hash in v0) return.
+    // ************************************************************************
+    if (single && output_overflow(input_index(input)))
+        return false;
+
     // Create tagged hash writer.
     stream::out::fast stream{ out };
     hash::sha256t::fast<"TapSighash"> sink{ stream };
@@ -106,10 +119,6 @@ bool transaction::version1_sighash(hash_digest& out,
 
     if (anyone)
     {
-        // This implies a parameterization error, and will fail the script.
-        if (is_null(in.prevout))
-            return false;
-
         in.point().to_data(sink);
         sink.write_8_bytes_little_endian(value);
         in.prevout->script().to_data(sink, true);
@@ -122,19 +131,13 @@ bool transaction::version1_sighash(hash_digest& out,
 
     if (annex)
     {
-        sink.write_variable(annex.size());
-        sink.write_bytes(annex.hash());
+        sink.write_bytes(annex.hash(true));
     }
 
     if (single)
     {
-        const auto index = input_index(input);
-        if (output_overflow(index))
-            return false;
-
         // Hash is cached for use with each single sigop in the same script.
-        // TODO: it may be more optimal to not cache, since benefit is rare.
-        sink.write_bytes(outputs_->at(index)->get_hash());
+        sink.write_bytes(outputs_->at(input_index(input))->get_hash());
     }
 
     // Additional for tapscript [bip342].
@@ -146,11 +149,6 @@ bool transaction::version1_sighash(hash_digest& out,
         sink.write_4_bytes_little_endian(subscript_v1(script));
     }
 
-    // TODO: write position pertains to the hash instance (written at flush).
-    // Total length at most 206 bytes (!anyone, no epoch/tapscript) [bip341].
-    ////BC_ASSERT(!anyone && sink.get_write_position() == (add1(206u) + 37));
-    ////BC_ASSERT( anyone && sink.get_write_position() == (add1(157u) + 37));
-
     sink.flush();
     return true;
 }