Merge pull request #440 from evoskuil/master

Refactor confirm and comments.

include/bitcoin/database/impl/query/confirm.ipp

@@ -242,8 +242,6 @@ inline error::error_t CLASS::spent_prevout(const foreign_point& point,
             continue;
 
         // If strong spender exists then prevout is confirmed double spent.
-        // Since all spends are traversed, and strength is distinct by tx-block
-        // link association, this is safe for duplicate txs.
         if (!to_block(spend.parent_fk).is_terminal())
             return error::confirmed_double_spend;
     }
@@ -256,14 +254,6 @@ TEMPLATE
 inline error::error_t CLASS::unspendable_prevout(const point_link& link,
     uint32_t sequence, uint32_t version, const context& ctx) const NOEXCEPT
 {
-    // Due to the natural key coupling between tx-spend and spent-tx, it is
-    // possible to associate a not strong tx when a strong tx exists. So must
-    // iterate over the tx set associated by the transaction hash (vs. link).
-    // A strong/not-strong association only affects the associated instances
-    // and is there definitive for a given tx-block tuple. Therefore if one
-    // instance is negative and another is positive, it implies a reorganized
-    // block (negative) and a strong block (positive). There may be multiple
-    // positive and/or negative, but one positive is sufficient here.
     const auto strong = to_strong(get_point_key(link));
     if (strong.block.is_terminal())
         return strong.tx.is_terminal() ? error::missing_previous_output :
@@ -293,45 +283,25 @@ inline error::error_t CLASS::unspent_duplicates(const tx_link& link,
     if (!ctx.is_enabled(system::chain::flags::bip30_rule))
         return error::success;
 
-    // Self is not a spender in this case.
-    constexpr auto self = tx_link::terminal;
+    // Self should be strong but was not identified.
     const auto coinbases = to_strongs(get_tx_key(link));
-
-    // self should be strong but was not found (fault).
     if (coinbases.empty())
         return error::integrity;
 
-    // assuming it is strong only self was found (optimization).
+    // Only self was found (optimization).
     if (is_one(coinbases.size()))
         return error::success;
 
-    // All that are found must be confirmed spent except self.
-    size_t strong_unspent_coinbase_count{};
+    // All but one (self) must be confirmed spent or coinbase is unspent.
+    size_t strong_unspent{};
     for (const auto& coinbase: coinbases)
-    {
-        // All outputs must be spent or the coinbase is unspent.
         for (spend::pt::integer out{}; out < output_count(coinbase.tx); ++out)
-        {
-            // Could stop at two but very rare so condition is more costly.
-            if (!spent_prevout(spend::compose(coinbase.tx, out), self))
-            {
-                ++strong_unspent_coinbase_count;
-                continue;
-            }
-        }
-    }
+            if (!spent_prevout(spend::compose(coinbase.tx, out)) &&
+                is_one(strong_unspent++))
+                return error::unspent_coinbase_collision;
 
-    switch (strong_unspent_coinbase_count)
-    {
-        // self should be unspent (fault).
-        case zero: return error::integrity;
-
-        // only self is unspent.
-        case one: return error::success;
-
-        // at least one instance other than self is unspent.
-        default: return error::unspent_coinbase_collision;
-    }
+    // Only self should/must be unspent.
+    return is_zero(strong_unspent) ? error::integrity : error::success;
 }
 
 TEMPLATE

include/bitcoin/database/impl/query/translate.ipp

@@ -174,6 +174,7 @@ output_link CLASS::to_prevout(const spend_link& link) const NOEXCEPT
 
 // block/tx to block (reverse navigation)
 // ----------------------------------------------------------------------------
+// Required for confirmation processing.
 
 TEMPLATE
 header_link CLASS::to_parent(const header_link& link) const NOEXCEPT
@@ -186,22 +187,21 @@ header_link CLASS::to_parent(const header_link& link) const NOEXCEPT
     return header.parent_fk;
 }
 
-// The block of a strong block-tx association.
 TEMPLATE
 header_link CLASS::to_block(const tx_link& link) const NOEXCEPT
 {
     table::strong_tx::record strong{};
     if (!store_.strong_tx.get(store_.strong_tx.first(link), strong))
         return {};
 
-    // Terminal implies not strong.
+    // Terminal implies not strong (false).
     return strong.positive ? strong.header_fk : header_link::terminal;
 }
 
 // protected
-// The first block-tx tuple where the tx is strong by the block.
 // If there are no associations the link of the first tx by hash is returned,
 // which is an optimization to prevent requery to determine tx existence.
+// Return the first block-tx tuple where the tx is strong by the block.
 TEMPLATE
 inline strong_pair CLASS::to_strong(const hash_digest& tx_hash) const NOEXCEPT
 {
@@ -218,15 +218,15 @@ inline strong_pair CLASS::to_strong(const hash_digest& tx_hash) const NOEXCEPT
 }
 
 // protected
-// This is required for bip30 processing.
-// The distinct set of block-tx tuples where the tx is strong by the block.
+// Required for bip30 processing.
+// Each it.self() is a unique link to a tx instance with tx_hash.
+// Duplicate tx instances with the same hash result from a write race.
+// It is possible that one tx instance is strong by distinct blocks, but it
+// is not possible that two tx instances are both strong by the same block.
+// Return the distinct set of block-tx tuples where tx is strong by block.
 TEMPLATE
 inline strong_pairs CLASS::to_strongs(const hash_digest& tx_hash) const NOEXCEPT
 {
-    // Each it.self() is a unique link to a tx instance with tx_hash.
-    // Duplicate tx instances with the same hash result from a write race.
-    // It is possible that one tx instance is strong by distinct blocks, but it
-    // is not possible that two tx instances are both strong by the same block.
     auto it = store_.tx.it(tx_hash);
     strong_pairs strongs{};
     do
@@ -240,42 +240,46 @@ inline strong_pairs CLASS::to_strongs(const hash_digest& tx_hash) const NOEXCEPT
 }
 
 // protected
-// This is required for bip30 processing.
+// Required for bip30 processing.
 // A single tx.link may be associated to multiple blocks (see bip30). But the
 // top of the strong_tx table will reflect the current state of only one block
 // association. This scans the multimap for the first instance of each block
 // that is associated by the tx.link and returns that set of block links.
+// Return the distinct set of block/header links where tx is strong by block.
 TEMPLATE
 inline header_links CLASS::to_blocks(const tx_link& link) const NOEXCEPT
 {
-    using record = table::strong_tx::record;
-    using records = std::vector<record>;
-    const auto contains = [](const records& items, const record& item) NOEXCEPT
-    {
-        return std::any_of(items.begin(), items.end(), [&](const record& it)
-        {
-            return it.header_fk == item.header_fk;
-        });
-    };
-
     auto it = store_.strong_tx.it(link);
-    if (it.self().is_terminal())
-        return {};
-
-    records strongs{};
+    block_tx strong{};
+    block_txs strongs{};
     do
     {
-        record strong{};
         if (!store_.strong_tx.get(it.self(), strong))
             return {};
 
-        // Retain only the first record for each block, strong or weak.
         if (!contains(strongs, strong))
             strongs.push_back(strong);
     }
     while(it.advance());
+    return strong_only(strongs);
+}
 
-    // Return just the block links of the strong associations.
+// private/static
+TEMPLATE
+inline bool CLASS::contains(const block_txs& blocks,
+    const block_tx& block) NOEXCEPT
+{
+    return std::any_of(blocks.begin(), blocks.end(),
+        [&block](const auto& it) NOEXCEPT
+        {
+            return it.header_fk == block.header_fk;
+        });
+}
+
+// private/static
+TEMPLATE
+inline header_links CLASS::strong_only(const block_txs& strongs) NOEXCEPT
+{
     header_links blocks{};
     for (const auto& strong: strongs)
         if (strong.positive)

include/bitcoin/database/query.hpp

@@ -377,9 +377,6 @@ class query
 protected:
     /// Translate.
     /// -----------------------------------------------------------------------
-    inline header_links to_blocks(const tx_link& link) const NOEXCEPT;
-    inline strong_pair to_strong(const hash_digest& tx_hash) const NOEXCEPT;
-    inline strong_pairs to_strongs(const hash_digest& tx_hash) const NOEXCEPT;
     uint32_t to_spend_index(const tx_link& parent_fk,
         const spend_link& input_fk) const NOEXCEPT;
     uint32_t to_output_index(const tx_link& parent_fk,
@@ -389,6 +386,11 @@ class query
     spend_links to_tx_spends(uint32_t& version,
         const tx_link& link) const NOEXCEPT;
 
+    // Critical path
+    inline header_links to_blocks(const tx_link& link) const NOEXCEPT;
+    inline strong_pair to_strong(const hash_digest& tx_hash) const NOEXCEPT;
+    inline strong_pairs to_strongs(const hash_digest& tx_hash) const NOEXCEPT;
+
     /// Archival
     /// -----------------------------------------------------------------------
     point_link set_link_(const hash_digest& point_hash) NOEXCEPT;
@@ -412,7 +414,7 @@ class query
 
     // Critical path
     inline error::error_t spent_prevout(const foreign_point& point,
-        const tx_link& self) const NOEXCEPT;
+        const tx_link& self = tx_link::terminal) const NOEXCEPT;
     inline error::error_t unspendable_prevout(const point_link& link,
         uint32_t sequence, uint32_t version, const context& ctx) const NOEXCEPT;
     inline error::error_t unspent_duplicates(const tx_link& link,
@@ -454,6 +456,13 @@ class query
         const header_link& link, size_t height) const NOEXCEPT;
 
 private:
+    // for to_blocks
+    using block_tx = table::strong_tx::record;
+    using block_txs = std::vector<block_tx>;
+    static inline header_links strong_only(const block_txs& strongs) NOEXCEPT;
+    static inline bool contains(const block_txs& blocks,
+        const block_tx& block) NOEXCEPT;
+
     Store& store_;
 };