Merge pull request #1487 from evoskuil/master

Factor block.check "bypass" into block.identify overrides.

include/bitcoin/system/chain/block.hpp

@@ -110,12 +110,18 @@ class BC_API block
     bool is_malleable64() const NOEXCEPT;
     bool is_malleated64() const NOEXCEPT;
 
+    /// Identity.
+    /// -----------------------------------------------------------------------
+
+    code identify() const NOEXCEPT;
+    code identify(const context& ctx) const NOEXCEPT;
+
     /// Validation.
     /// -----------------------------------------------------------------------
 
     /// Consensus checks (no DoS guards for block sync without headers first).
-    code check(bool bypass=false) const NOEXCEPT;
-    code check(const context& ctx, bool bypass=false) const NOEXCEPT;
+    code check() const NOEXCEPT;
+    code check(const context& ctx) const NOEXCEPT;
     code accept(const context& ctx, size_t subsidy_interval,
         uint64_t initial_subsidy) const NOEXCEPT;
     code connect(const context& ctx) const NOEXCEPT;

include/bitcoin/system/chain/prevout.hpp

@@ -33,26 +33,29 @@ class BC_API prevout final
     /// A height of zero is immature (unspendable) despite unspent state.
     ///************************************************************************
     /// The confirmed chain height of the prevout (zero if not found).
+    /// Unused if the input owning this prevout is null (coinbase).
     size_t height{ zero };
 
     ///************************************************************************
     /// CONSENSUS: 
     /// A mtp of max_uint32 fails locktime maturity (until time overflow).
     ///************************************************************************
     /// The median time past at height (max_uint32 if not found/confirmed).
+    /// Unused if the input owning this prevout is null (coinbase).
     uint32_t median_time_past{ max_uint32 };
 
     ///************************************************************************
     /// CONSENSUS: 
     /// An unspent coinbase collision is immature (unspendable) and spent
-    /// collision is mature (bip30). Coinbase collision precluded by bip34.
+    /// collision is mature (bip30). CB collision presumed precluded by bip34.
     ///************************************************************************
-    /// For coinbase tx, implies fully spent at height or not found.
-    /// Coinbase value stored in first input of coinbase transaction.
-    /// For a non-coinbase input this indicates spent at height.
+    /// If the input owning this prevout is null (coinbase), this implies that
+    /// all outputs of any duplicate txs are fully spent at height.
+    /// If the input owning this prevout is not null (not coinbase), this
+    /// indicates whether the prevout is spent at height (double spend).
     bool spent{ true };
 
-    /// The output is of a coinbase transaction.
+    /// The previous output is of a coinbase transaction.
     bool coinbase{ false };
 };
 

include/bitcoin/system/chain/witness.hpp

@@ -110,7 +110,7 @@ class BC_API witness
     /// The (only) coinbase witness must be (arbitrary) 32-byte value (bip141).
     static VCONSTEXPR bool is_reserved_pattern(const chunk_cptrs& stack) NOEXCEPT
     {
-        return stack.size() == one && stack.front()->size() == hash_size;
+        return is_one(stack.size()) && stack.front()->size() == hash_size;
     }
 
     bool extract_sigop_script(script& out_script,

include/bitcoin/system/funclets.hpp

@@ -125,7 +125,7 @@ constexpr Type to_bits(Type bytes) noexcept
 
 constexpr uint8_t to_byte(char value) noexcept
 {
-    static_assert(sizeof(char) == 1u);
+    static_assert(is_one(sizeof(char)));
     return static_cast<uint8_t>(value);
 }
 
@@ -147,8 +147,8 @@ constexpr Type to_int(bool value) noexcept
 template <typename Type>
 constexpr bool to_bool(Type value) noexcept
 {
-    static_assert(static_cast<Type>(true) == 1);
-    static_assert(static_cast<Type>(false) == 0);
+    static_assert(is_one(static_cast<Type>(true)));
+    static_assert(is_zero(static_cast<Type>(false)));
     return is_nonzero(value);
 }
 

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

@@ -102,7 +102,7 @@ VCONSTEXPR opcode operation::minimal_opcode_from_data(
 {
     const auto size = data.size();
 
-    if (size == one)
+    if (is_one(size))
     {
         const auto value = data.front();
 

include/bitcoin/system/impl/machine/interpreter.ipp

@@ -291,7 +291,7 @@ op_dup2() NOEXCEPT
     if (state::stack_size() < 2)
         return error::op_dup2;
 
-    // [0,1,2,3] => 1, [0,1,2,3] =>  0,1,[0,1,2,3]
+    // [0,1,2,3] => 1,[0,1,2,3] =>  0,1,[0,1,2,3]
     state::push_variant(state::peek_(1));
     state::push_variant(state::peek_(1));
     return error::op_success;

include/bitcoin/system/impl/machine/program.ipp

@@ -547,7 +547,7 @@ template <typename Stack>
 INLINE bool program<Stack>::
 is_stack_clean() const NOEXCEPT
 {
-    return stack_size() == one;
+    return is_one(stack_size());
 }
 
 // Alternate stack.

include/bitcoin/system/impl/math/bytes.ipp

@@ -88,8 +88,8 @@ template <typename Integer, if_integer<Integer>>
 constexpr bool is_negated(Integer value) NOEXCEPT
 {
     // Guard precludes zero appearing negated: ((0 % 8) + 1) == 1.
-    return is_nonzero(value) && add1(bit_width(value) % byte_bits) == one;
-};
+    return is_nonzero(value) && is_one(add1(bit_width(value) % byte_bits));
+}
 
 template <typename Integer, if_integer<Integer>>
 constexpr Integer to_negated(Integer value) NOEXCEPT

include/bitcoin/system/intrinsics/haves.hpp

@@ -174,12 +174,12 @@ template <typename Integral, size_t Lanes,
     if_not_greater<safe_multiply(sizeof(Integral), Lanes),
         sizeof(xint512_t)> = true>
 using to_extended =
-    iif<capacity<uint8_t, Integral, Lanes> == one, uint8_t,
-        iif<capacity<uint16_t, Integral, Lanes> == one, uint16_t,
-            iif<capacity<uint32_t, Integral, Lanes> == one, uint32_t,
-                iif<capacity<uint64_t, Integral, Lanes> == one, uint64_t,
-                    iif<capacity<xint128_t, Integral, Lanes> == one, xint128_t,
-                        iif<capacity<xint256_t, Integral, Lanes> == one, xint256_t,
+    iif<is_one(capacity<uint8_t, Integral, Lanes>), uint8_t,
+        iif<is_one(capacity<uint16_t, Integral, Lanes>), uint16_t,
+            iif<is_one(capacity<uint32_t, Integral, Lanes>), uint32_t,
+                iif<is_one(capacity<uint64_t, Integral, Lanes>), uint64_t,
+                    iif<is_one(capacity<xint128_t, Integral, Lanes>), xint128_t,
+                        iif<is_one(capacity<xint256_t, Integral, Lanes>), xint256_t,
                             xint512_t>>>>>>;
 
 /// Runtime time availability of extended integer intrinsics, as a template

include/bitcoin/system/stream/streams.hpp

@@ -57,7 +57,7 @@ namespace stream
         using copy = make_stream<copy_sink<data_slab>>;
 
         /// A fast output stream that copies data to a data_slab.
-        using fast = system:: ostream<>;
+        using fast = system::ostream<>;
 
         /// A std::output stream that inserts data to a container.
         template <typename Container>

src/chain/block.cpp

@@ -682,10 +682,8 @@ void block::populate() const NOEXCEPT
 // DO invoke on coinbase.
 code block::check_transactions() const NOEXCEPT
 {
-    code ec;
-
     for (const auto& tx: *txs_)
-        if ((ec = tx->check()))
+        if (const auto ec = tx->check())
             return ec;
 
     return error::block_success;
@@ -694,10 +692,8 @@ code block::check_transactions() const NOEXCEPT
 // DO invoke on coinbase.
 code block::check_transactions(const context& ctx) const NOEXCEPT
 {
-    code ec;
-
     for (const auto& tx: *txs_)
-        if ((ec = tx->check(ctx)))
+        if (const auto ec = tx->check(ctx))
             return ec;
 
     return error::block_success;
@@ -706,11 +702,9 @@ code block::check_transactions(const context& ctx) const NOEXCEPT
 // Do NOT invoke on coinbase.
 code block::accept_transactions(const context& ctx) const NOEXCEPT
 {
-    code ec;
-
     if (!is_empty())
         for (auto tx = std::next(txs_->begin()); tx != txs_->end(); ++tx)
-            if ((ec = (*tx)->accept(ctx)))
+            if (const auto ec = (*tx)->accept(ctx))
                 return ec;
 
     return error::block_success;
@@ -719,11 +713,9 @@ code block::accept_transactions(const context& ctx) const NOEXCEPT
 // Do NOT invoke on coinbase.
 code block::connect_transactions(const context& ctx) const NOEXCEPT
 {
-    code ec;
-
     if (!is_empty())
         for (auto tx = std::next(txs_->begin()); tx != txs_->end(); ++tx)
-            if ((ec = (*tx)->connect(ctx)))
+            if (const auto ec = (*tx)->connect(ctx))
                 return ec;
 
     return error::block_success;
@@ -732,35 +724,49 @@ code block::connect_transactions(const context& ctx) const NOEXCEPT
 // Do NOT invoke on coinbase.
 code block::confirm_transactions(const context& ctx) const NOEXCEPT
 {
-    code ec;
-
     if (!is_empty())
         for (auto tx = std::next(txs_->begin()); tx != txs_->end(); ++tx)
-            if ((ec = (*tx)->confirm(ctx)))
+            if (const auto ec = (*tx)->confirm(ctx))
                 return ec;
 
     return error::block_success;
 }
 
-// Validation.
+// Identity.
 // ----------------------------------------------------------------------------
-// The block header is checked/accepted independently.
+// invalid_transaction_commitment, invalid_witness_commitment, block_malleated
+// codes specifically indicate lack of block hash tx identification (identity).
 
-// context free.
-// Node relies on error::block_malleated.
-// Node relies on error::invalid_transaction_commitment.
-// TODO: use of get_hash() in is_forward_reference makes this thread unsafe.
-code block::check(bool bypass) const NOEXCEPT
+code block::identify() const NOEXCEPT
 {
-    // type32 malleated is a subset of is_internal_double_spend.
     // type64 malleated is a subset of first_not_coinbase.
-    if (bypass && is_malleated())
+    // type32 malleated is a subset of is_internal_double_spend.
+    if (is_malleated())
         return error::block_malleated;
     if (is_invalid_merkle_root())
         return error::invalid_transaction_commitment;
-    if (bypass)
-        return error::block_success;
 
+    return error::block_success;
+}
+
+code block::identify(const context& ctx) const NOEXCEPT
+{
+    const auto bip141 = ctx.is_enabled(bip141_rule);
+
+    if (bip141 && is_invalid_witness_commitment())
+        return error::invalid_witness_commitment;
+
+    return error::block_success;
+}
+
+// Validation.
+// ----------------------------------------------------------------------------
+// In the case of validation failure
+// The block header is checked/accepted independently.
+
+// TODO: use of get_hash() in is_forward_reference makes this thread unsafe.
+code block::check() const NOEXCEPT
+{
     // empty_block is subset of first_not_coinbase.
     //if (is_empty())
     //    return error::empty_block;
@@ -774,6 +780,8 @@ code block::check(bool bypass) const NOEXCEPT
         return error::forward_reference;
     if (is_internal_double_spend())
         return error::block_internal_double_spend;
+    if (is_invalid_merkle_root())
+        return error::invalid_transaction_commitment;
 
     return check_transactions();
 }
@@ -783,18 +791,10 @@ code block::check(bool bypass) const NOEXCEPT
 // timestamp
 // median_time_past
 
-// context required.
-// Node relies on error::invalid_witness_commitment.
 // TODO: use of get_hash() in is_hash_limit_exceeded makes this thread unsafe.
-code block::check(const context& ctx, bool bypass) const NOEXCEPT
+code block::check(const context& ctx) const NOEXCEPT
 {
     const auto bip141 = ctx.is_enabled(bip141_rule);
-
-    if (bip141 && is_invalid_witness_commitment())
-        return error::invalid_witness_commitment;
-    if (bypass)        
-        return error::block_success;
-
     const auto bip34 = ctx.is_enabled(bip34_rule);
     const auto bip50 = ctx.is_enabled(bip50_rule);
 
@@ -804,6 +804,8 @@ code block::check(const context& ctx, bool bypass) const NOEXCEPT
         return error::coinbase_height_mismatch;
     if (bip50 && is_hash_limit_exceeded())
         return error::temporary_hash_limit;
+    if (bip141 && is_invalid_witness_commitment())
+        return error::invalid_witness_commitment;
 
     return check_transactions(ctx);
 }

src/chain/operation.cpp

@@ -292,13 +292,13 @@ operation operation::from_string(const std::string& mnemonic) NOEXCEPT
         // Data encoding uses single token with one or two parts.
         const auto parts = split_push_token(mnemonic);
 
-        if (parts.size() == 1)
+        if (parts.size() == one)
         {
             // Extract operation using nominal data size decoding.
             if ((valid = decode_base16(chunk, parts.front())))
                 code = nominal_opcode_from_data(chunk);
         }
-        else if (parts.size() == 2)
+        else if (parts.size() == two)
         {
             // Extract operation using explicit data size decoding.
 

src/chain/transaction.cpp

@@ -1016,7 +1016,7 @@ bool transaction::create_endorsement(endorsement& out, const ec_secret& secret,
 
 bool transaction::is_coinbase() const NOEXCEPT
 {
-    return inputs_->size() == one && inputs_->front()->point().is_null();
+    return is_one(inputs_->size()) && inputs_->front()->point().is_null();
 }
 
 bool transaction::is_internal_double_spend() const NOEXCEPT

src/config/printer.cpp

@@ -201,7 +201,7 @@ static std::string format_setting(const parameter& value,
     auto required = value.required();
 
     // In terms of formatting we also treat multivalued as not required.
-    auto optional = value.args_limit() == 1;
+    auto optional = is_one(value.args_limit());
 
     std::string formatter;
     if (required)
@@ -308,10 +308,10 @@ std::string printer::format_usage_parameters() NOEXCEPT
         const auto option = parameter.position() == parameter::not_positional;
 
         // In terms of formatting we also treat multivalued as not required.
-        const auto optional = parameter.args_limit() == 1;
+        const auto optional = is_one(parameter.args_limit());
 
         // This will capture only options set to zero_tokens().
-        const auto toggle = parameter.args_limit() == 0;
+        const auto toggle = is_zero(parameter.args_limit());
 
         // A toggle with a short name gets mashed up in group.
         const auto is_short = parameter.short_name() != parameter::no_short_name;

src/wallet/points_value.cpp

@@ -39,7 +39,7 @@ void points_value::greedy(points_value& out, const points_value& unspent,
         return;
 
     // Optimization for simple case not requiring search.
-    if (unspent.points.size() == 1)
+    if (is_one(unspent.points.size()))
     {
         out.points.push_back(unspent.points.front());
         return;

test/hash/hash.hpp

@@ -68,7 +68,7 @@ constexpr auto merkle_hash(const auto& expected) noexcept
     };
 
     Algorithm::merkle_hash(digests);
-    return (digests.size() == one) && (digests.front() == expected);
+    return is_one(digests.size()) && (digests.front() == expected);
 };
 
 // constexpr test helper for merkle_hash with four hashes.

test/math/addition.cpp

@@ -134,7 +134,7 @@ static_assert(subtract<int64_t>(pos_int32, min_int32) == (int64_t)pos_int32 - (i
 BC_POP_WARNING()
 
 // Unsigned underflow (signed underflow fails constexpr evaluation).
-static_assert(subtract(min_uint32, max_uint32) == one);
+static_assert(is_one(subtract(min_uint32, max_uint32)));
 static_assert(is_subtract_overflow(min_uint32, max_uint32));
 
 // TODO: value tests for default and explicit typing.

test/types.cpp

@@ -46,7 +46,7 @@ static_assert(is_same_type<argument<one, base, derived>, derived>);
 // ----------------------------------------------------------------------------
 
 // This are design limitations, and not a matter of C++ specification.
-static_assert(sizeof(char) == one);
+static_assert(is_one(sizeof(char)));
 static_assert(
     is_same_size<size_t, uint32_t> ||
     is_same_size<size_t, uint64_t>);