Merge pull request #643 from evoskuil/master

Use new set_code() methods, refactor/fix bypass behavior.

include/bitcoin/node/chasers/chaser_block.hpp

@@ -48,10 +48,6 @@ class BCN_API chaser_block
     virtual bool get_block(system::chain::block::cptr& out,
         size_t height) const NOEXCEPT;
 
-    /// True if Block should bypass validation, given its candidate height.
-    virtual bool get_bypass(const system::chain::block& block,
-        size_t height) const NOEXCEPT;
-
     /// Determine if Block is valid.
     virtual code validate(const system::chain::block& block,
         const chain_state& state) const NOEXCEPT;
@@ -62,6 +58,9 @@ class BCN_API chaser_block
     /// Determine if state is top of a storable branch (always true).
     virtual bool is_storable(const chain_state& state) const NOEXCEPT;
 
+    /// True if Block is on a milestone-covered branch.
+    virtual bool is_under_milestone(size_t height) const NOEXCEPT;
+
     /// Milestone tracking.
     virtual void update_milestone(const system::chain::header& header,
         size_t height, size_t branch_point) NOEXCEPT;

include/bitcoin/node/chasers/chaser_confirm.hpp

@@ -48,11 +48,16 @@ class BCN_API chaser_confirm
     virtual void do_validated(height_t height) NOEXCEPT;
 
 private:
-    bool set_organized(header_t link, height_t height) NOEXCEPT;
-    bool reset_organized(header_t link, height_t height) NOEXCEPT;
-    bool set_reorganized(header_t link, height_t height) NOEXCEPT;
+    bool set_organized(const database::header_link& link,
+        height_t height) NOEXCEPT;
+    bool reset_organized(const database::header_link& link,
+        height_t height) NOEXCEPT;
+    bool set_reorganized(const database::header_link& link,
+        height_t height) NOEXCEPT;
     bool roll_back(const header_links& popped,
-        size_t fork_point, size_t top) NOEXCEPT;
+        const database::header_link& link, size_t fork_point,
+        size_t top) NOEXCEPT;
+
     bool get_fork_work(uint256_t& fork_work, header_links& fork,
         height_t fork_top) const NOEXCEPT;
     bool get_is_strong(bool& strong, const uint256_t& fork_work,

include/bitcoin/node/chasers/chaser_header.hpp

@@ -51,10 +51,6 @@ class BCN_API chaser_header
     virtual bool get_block(system::chain::header::cptr& out,
         size_t height) const NOEXCEPT;
 
-    /// True if Block should bypass validation, given its candidate height.
-    virtual bool get_bypass(const system::chain::header& header,
-        size_t height) const NOEXCEPT;
-
     /// Determine if Block is valid.
     virtual code validate(const system::chain::header& header,
         const chain_state& state) const NOEXCEPT;
@@ -65,20 +61,19 @@ class BCN_API chaser_header
     /// Determine if state is top of a storable branch.
     virtual bool is_storable(const chain_state& state) const NOEXCEPT;
 
+    /// True if Block is on a milestone-covered branch.
+    virtual bool is_under_milestone(size_t height) const NOEXCEPT;
+
     /// Milestone tracking.
     virtual void update_milestone(const system::chain::header& header,
         size_t height, size_t branch_point) NOEXCEPT;
 
-    /// Milestone methods.
-    bool initialize_milestone() NOEXCEPT;
-    bool is_under_milestone(size_t height) const NOEXCEPT;
-
 private:
-    // Storable methods.
     bool is_checkpoint(const chain_state& state) const NOEXCEPT;
     bool is_milestone(const chain_state& state) const NOEXCEPT;
     bool is_current(const chain_state& state) const NOEXCEPT;
     bool is_hard(const chain_state& state) const NOEXCEPT;
+    bool initialize_milestone() NOEXCEPT;
 
     // This is thread safe.
     const system::chain::checkpoint& milestone_;

include/bitcoin/node/chasers/chaser_organize.hpp

@@ -70,10 +70,6 @@ class chaser_organize
     virtual bool get_block(typename Block::cptr& out,
         size_t height) const NOEXCEPT = 0;
 
-    /// True if Block should bypass validation, given its candidate height.
-    virtual bool get_bypass(const Block& block,
-        size_t height) const NOEXCEPT = 0;
-
     /// Determine if Block is valid.
     virtual code validate(const Block& block,
         const chain_state& state) const NOEXCEPT = 0;
@@ -84,6 +80,9 @@ class chaser_organize
     /// Determine if state is top of a storable branch.
     virtual bool is_storable(const chain_state& state) const NOEXCEPT = 0;
 
+    /// True if Block is on a milestone-covered branch.
+    virtual bool is_under_milestone(size_t height) const NOEXCEPT = 0;
+
     /// Milestone tracking.
     virtual void update_milestone(const system::chain::header& header,
         size_t height, size_t branch_point) NOEXCEPT = 0;
@@ -153,10 +152,10 @@ class chaser_organize
         size_t branch_point) const NOEXCEPT;
 
     // Move tree Block to database and push to top of candidate chain.
-    bool push_block(const system::hash_digest& key) NOEXCEPT;
+    code push_block(const system::hash_digest& key) NOEXCEPT;
 
     /// Store Block to database and push to top of candidate chain.
-    bool push_block(const Block& block,
+    code push_block(const Block& block,
         const system::chain::context& context) const NOEXCEPT;
 
     // Logging.

include/bitcoin/node/impl/chasers/chaser_organize.ipp

@@ -262,6 +262,7 @@ void CLASS::do_organize(typename Block::cptr& block_ptr,
     // A milestone can only be set within a to-be-archived chain of candidate
     // headers/blocks. Once the milestone block is archived it is not useful.
     update_milestone(header, height, branch_point);
+    code ec{};
 
     const auto top_candidate = state_->height();
     if (branch_point > top_candidate)
@@ -303,9 +304,9 @@ void CLASS::do_organize(typename Block::cptr& block_ptr,
     // Store strong tree headers and push to candidate chain.
     for (const auto& key: views_reverse(tree_branch))
     {
-        if (!push_block(key))
+        if ((ec = push_block(key)))
         {
-            handler(fault(error::node_push), height);
+            handler(fault(ec), height);
             return;
         }
 
@@ -316,9 +317,9 @@ void CLASS::do_organize(typename Block::cptr& block_ptr,
 
     // Push new header as top of candidate chain.
     {
-        if (!push_block(*block_ptr, state->context()))
+        if ((ec = push_block(*block_ptr, state->context())))
         {
-            handler(fault(error::node_push), height);
+            handler(fault(ec), height);
             return;
         }
 
@@ -564,23 +565,45 @@ bool CLASS::get_is_strong(bool& strong, const uint256_t& branch_work,
 }
 
 TEMPLATE
-bool CLASS::push_block(const Block& block,
+code CLASS::push_block(const Block& block,
     const system::chain::context& context) const NOEXCEPT
 {
     auto& query = archive();
-    const auto bypass = get_bypass(block, context.height);
+    const auto milestone = is_under_milestone(context.height);
 
-    // TODO: change this to set_code() and return code.
-    // TODO: add confirm option to set_code() and pass bypass (to both).
-    // TODO: block bypass/confirm is checkpoints, headers is also milestone.
-    return query.push_candidate(query.set_link(block, context, bypass));
+    // headers-first sets milestone and set_strong ms or cp and not mealleable.
+    // blocks-first does not set milestone and set_strong cp not malleable.
+    const auto strong = [&]() NOEXCEPT
+    {
+        if constexpr (is_block())
+        {
+            return is_under_checkpoint(context.height) &&
+                !block.is_malleable64();
+        }
+        else
+        {
+            return false;
+        }
+    };
+
+    database::header_link link{};
+    const auto ec = query.set_code(link, block, context, milestone, strong());
+    if (ec)
+        return ec;
+
+    if (!query.push_candidate(link))
+        return error::push_candidate;
+
+    return ec;
 }
 
 TEMPLATE
-bool CLASS::push_block(const system::hash_digest& key) NOEXCEPT
+code CLASS::push_block(const system::hash_digest& key) NOEXCEPT
 {
     const auto handle = tree_.extract(key);
-    if (!handle) return false;
+    if (!handle)
+        return error::internal_error;
+
     const auto& value = handle.mapped();
     return push_block(*value.block, value.state->context());
 }

src/chasers/chaser_block.cpp

@@ -47,58 +47,54 @@ bool chaser_block::get_block(block::cptr& out, size_t height) const NOEXCEPT
     return !is_null(out);
 }
 
-bool chaser_block::get_bypass(const block& block, size_t height) const NOEXCEPT
-{
-    // Milestones are not relevant to block-first organization.
-    // TODO: Can a validated block be malleable64 (i.e. can we ignore here).
-    return is_under_checkpoint(height) && !block.is_malleable64();
-}
-
 code chaser_block::validate(const block& block,
     const chain_state& state) const NOEXCEPT
 {
     code ec{};
     const auto& header = block.header();
+    const auto& setting = settings();
+    const auto ctx = state.context();
 
     // header.check is never bypassed.
     // block.check does not invoke header.check.
     if ((ec = header.check(
-        settings().timestamp_limit_seconds,
-        settings().proof_of_work_limit,
-        settings().forks.scrypt_proof_of_work)))
+        setting.timestamp_limit_seconds,
+        setting.proof_of_work_limit,
+        setting.forks.scrypt_proof_of_work)))
         return ec;
 
     // header.accept is never bypassed.
     // block.accept does not invoke header.accept.
-    if ((ec = header.accept(state.context())))
+    if ((ec = header.accept(ctx)))
         return ec;
 
     // Transaction/witness commitments are required under checkpoint.
     // This ensures that the block/header hash represents expected txs.
-    const auto bypass = get_bypass(block, state.height());
+    const auto checked = is_under_checkpoint(state.height()) &&
+        !block.is_malleable64();
 
-    // Transaction commitments and malleated32 are checked under checkpoint.
-    if ((ec = block.check(bypass)))
+    // Transaction commitments and malleated32 are checked under bypass.
+    if ((ec = block.check(checked)))
         return ec;
 
-    // Witnessed tx commitments are checked under checkpoint (if bip141).
-    if ((ec = block.check(state.context(), bypass)))
+    // Witnessed tx commitments are checked under bypass (if bip141).
+    if ((ec = block.check(ctx, checked)))
         return ec;
 
-    if (bypass)
+    if (checked)
         return system::error::block_success;
 
     // Populate prevouts from self/tree/store (metadata not required).
     populate(block);
     if (!archive().populate(block))
         return network::error::protocol_violation;
 
-    if ((ec = block.accept(state.context(),
-        settings().subsidy_interval_blocks,
-        settings().initial_subsidy())))
+    if ((ec = block.accept(ctx,
+        setting.subsidy_interval_blocks,
+        setting.initial_subsidy())))
         return ec;
 
-    return block.connect(state.context());
+    return block.connect(ctx);
 }
 
 // The archived malleable block was found to be invalid (treat as malleated).
@@ -127,6 +123,15 @@ bool chaser_block::is_storable(const chain_state&) const NOEXCEPT
     return true;
 }
 
+
+// Milestone methods.
+// ----------------------------------------------------------------------------
+
+bool chaser_block::is_under_milestone(size_t) const NOEXCEPT
+{
+    return false;
+}
+
 void chaser_block::update_milestone(const header&, size_t, size_t) NOEXCEPT
 {
 }