Merge pull request #548 from evoskuil/master

Remove chaser_header.is_current checkpoint, add milestone/minwork.

include/bitcoin/node/chasers/chaser_header.hpp

@@ -76,10 +76,8 @@ class BCN_API chaser_header
     virtual bool get_is_strong(bool& strong, const uint256_t& work,
         size_t point) const NOEXCEPT;
 
-    /// Header timestamp is within configured span from current time,
-    /// or current header is a checkpoint or milestone.
-    virtual bool is_current(const system::chain::header& header,
-        size_t height) const NOEXCEPT;
+    /// Header timestamp is within configured span from current time.
+    virtual bool is_current(const system::chain::header& header) const NOEXCEPT;
 
     /// Save header to tree with validation context.
     virtual void save(const system::chain::header::cptr& header,
@@ -105,6 +103,8 @@ class BCN_API chaser_header
     void do_handle_event(const code& ec, chase event_, link value) NOEXCEPT;
 
     // These are thread safe.
+    const uint256_t minimum_work_;
+    const system::chain::checkpoint& milestone_;
     const system::chain::checkpoints& checkpoints_;
     const network::wall_clock::duration currency_window_;
     const bool use_currency_window_;

include/bitcoin/node/configuration.hpp

@@ -19,7 +19,6 @@
 #ifndef LIBBITCOIN_NODE_CONFIGURATION_HPP
 #define LIBBITCOIN_NODE_CONFIGURATION_HPP
 
-////#include <bitcoin/blockchain.hpp>
 #include <bitcoin/database.hpp>
 #include <bitcoin/network.hpp>
 #include <bitcoin/node/define.hpp>
@@ -72,7 +71,6 @@ class BCN_API configuration
     /// Settings.
     log::settings log;
     node::settings node;
-    ////blockchain::settings chain;
     database::settings database;
     network::settings network;
     system::settings bitcoin;

src/chasers/chaser_block.cpp

@@ -99,7 +99,7 @@ void chaser_block::do_organize(const block::cptr& block_ptr,
     const auto& coin = config().bitcoin;
     const auto hash = header.hash();
 
-    // Skip existing, fail orphan.
+    // Skip existing, orphan.
     // ------------------------------------------------------------------------
 
     if (closed())
@@ -127,8 +127,8 @@ void chaser_block::do_organize(const block::cptr& block_ptr,
     // ------------------------------------------------------------------------
 
     // Rolling forward chain_state eliminates requery cost.
-    // Do not use block ref here as the block override is for tx pool.
-    state_.reset(new chain_state(*state_, header, coin));
+    // Do not use block parameter here as that override is for tx pool.
+    state_.reset(new chain_state{ *state_, header, coin });
     const auto context = state_->context();
     const auto height = state_->height();
 
@@ -143,18 +143,16 @@ void chaser_block::do_organize(const block::cptr& block_ptr,
     if (!checkpoint::is_under(coin.checkpoints, height))
     {
         // Requires no population.
-        auto error = block.check();
-        if (error)
+        if (const auto error = block.check())
         {
-            handler(network::error::protocol_violation);
+            handler(error);
             return;
         }
 
         // Requires no population.
-        error = block.check(context);
-        if (error)
+        if (const auto error = block.check(context))
         {
-            handler(network::error::protocol_violation);
+            handler(error);
             return;
         }
 
@@ -172,19 +170,17 @@ void chaser_block::do_organize(const block::cptr& block_ptr,
         }
 
         // Requires only prevout population.
-        error = block.accept(context, coin.subsidy_interval_blocks,
-            coin.initial_subsidy());
-        if (error)
+        if (const auto error = block.accept(context,
+            coin.subsidy_interval_blocks, coin.initial_subsidy()))
         {
-            handler(network::error::protocol_violation);
+            handler(error);
             return;
         }
 
         // Requires only prevout population.
-        error = block.connect(context);
-        if (error)
+        if (const auto error = block.connect(context))
         {
-            handler(network::error::protocol_violation);
+            handler(error);
             return;
         }
     }
@@ -267,8 +263,7 @@ void chaser_block::do_organize(const block::cptr& block_ptr,
     }
 
     // Push new block as top of candidate chain.
-    const auto link = push(block_ptr, context);
-    if (link.is_terminal())
+    if (push(block_ptr, context).is_terminal())
     {
         handler(error::store_integrity);
         return;
@@ -367,11 +362,11 @@ database::header_link chaser_block::push(const block::cptr& block,
 {
     auto& query = archive();
     const auto link = query.set_link(*block, database::context
-        {
-            possible_narrow_cast<flags_t>(context.forks),
-            possible_narrow_cast<height_t>(context.height),
-            context.median_time_past,
-        });
+    {
+        possible_narrow_cast<flags_t>(context.forks),
+        possible_narrow_cast<height_t>(context.height),
+        context.median_time_past,
+    });
 
     if (!query.push_candidate(link))
         return {};

src/chasers/chaser_header.cpp

@@ -38,6 +38,8 @@ BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
 
 chaser_header::chaser_header(full_node& node) NOEXCEPT
   : chaser(node),
+    minimum_work_(config().bitcoin.minimum_work),
+    milestone_(config().bitcoin.milestone),
     checkpoints_(config().bitcoin.checkpoints),
     currency_window_(config().node.currency_window()),
     use_currency_window_(to_bool(config().node.currency_window_minutes))
@@ -113,7 +115,7 @@ void chaser_header::do_organize(const header::cptr& header_ptr,
     const auto& coin = config().bitcoin;
     const auto hash = header.hash();
 
-    // Skip existing, fail orphan.
+    // Skip existing, orphan.
     // ------------------------------------------------------------------------
 
     if (closed())
@@ -138,9 +140,10 @@ void chaser_header::do_organize(const header::cptr& header_ptr,
 
     // Validate header.
     // ------------------------------------------------------------------------
+    // Header validations are not bypassed when under checkpoint/milestone.
 
     // Rolling forward chain_state eliminates requery cost.
-    state_.reset(new chain_state(*state_, header, coin));
+    state_.reset(new chain_state{ *state_, header, coin });
     const auto context = state_->context();
     const auto height = state_->height();
 
@@ -151,27 +154,27 @@ void chaser_header::do_organize(const header::cptr& header_ptr,
         return;
     }
 
-    // Header validations are not bypassed when under checkpoint/milestone.
-
-    auto error = header.check(coin.timestamp_limit_seconds,
-        coin.proof_of_work_limit, coin.scrypt_proof_of_work);
-    if (error)
+    if (const auto error = header.check(coin.timestamp_limit_seconds,
+            coin.proof_of_work_limit, coin.scrypt_proof_of_work))
     {
         handler(error);
         return;
     }
 
-    error = header.accept(context);
-    if (error)
+    if (const auto error = header.accept(context))
     {
         handler(error);
+        return;
     }
 
     // Compute relative work.
     // ------------------------------------------------------------------------
 
-    // Header is new top of stale branch (strength not computed).
-    if (!is_current(header, context.height))
+    // A checkpointed or milestoned branch always gets disk stored. Otherwise
+    // branch must be both current and of sufficient chain work to be stored.
+    if (!checkpoint::is_at(checkpoints_, height) &&
+        !milestone_.equals(hash, height) &&
+        !(is_current(header) && state_->cumulative_work() >= minimum_work_))
     {
         save(header_ptr, context);
         handler(error::success);
@@ -195,9 +198,9 @@ void chaser_header::do_organize(const header::cptr& header_ptr,
         return;
     }
 
-    // Header is new top of current weak branch.
     if (!strong)
     {
+        // Header is new top of current weak branch.
         save(header_ptr, context);
         handler(error::success);
         return;
@@ -245,8 +248,7 @@ void chaser_header::do_organize(const header::cptr& header_ptr,
     }
 
     // Push new header as top of candidate chain.
-    const auto link = push(header_ptr, context);
-    if (link.is_terminal())
+    if (push(header_ptr, context).is_terminal())
     {
         handler(error::store_integrity);
         return;
@@ -262,16 +264,11 @@ void chaser_header::do_organize(const header::cptr& header_ptr,
     handler(error::success);
 }
 
-bool chaser_header::is_current(const header& header,
-    size_t height) const NOEXCEPT
+bool chaser_header::is_current(const header& header) const NOEXCEPT
 {
     if (!use_currency_window())
         return true;
 
-    // Checkpoints are already validated. Current if at a checkpoint height.
-    if (checkpoint::is_at(checkpoints_, height))
-        return true;
-
     // en.wikipedia.org/wiki/Time_formatting_and_storage_bugs#Year_2106
     const auto time = wall_clock::from_time_t(header.timestamp());
     const auto current = wall_clock::now() - currency_window();

src/configuration.cpp

@@ -19,7 +19,6 @@
 #include <bitcoin/node/configuration.hpp>
 
 #include <cstddef>
-////#include <bitcoin/blockchain.hpp>
 #include <bitcoin/network.hpp>
 
 namespace libbitcoin {

src/parser.cpp

@@ -20,7 +20,6 @@
 
 #include <iostream>
 #include <bitcoin/system.hpp>
-////#include <bitcoin/blockchain.hpp>
 #include <bitcoin/network.hpp>
 #include <bitcoin/node/full_node.hpp>
 #include <bitcoin/node/settings.hpp>
@@ -365,7 +364,6 @@ options_metadata parser::load_settings() THROWS
         value<config::block>(&configured.bitcoin.genesis_block),
         "The genesis block, defaults to mainnet."
     )
-    // TODO: checkpoint/checkpoints fail to parse.
     (
         "bitcoin.checkpoint",
         value<chain::checkpoints>(&configured.bitcoin.checkpoints),
@@ -413,9 +411,14 @@ options_metadata parser::load_settings() THROWS
         "The hash:height checkpoint for bip9 bit0 activation, defaults to 000000000000000004a1b34462cb8aeebd5799177f7a29cf28f2d1961716b5b5:419328."
     )
     (
-        "bitcoin.bip9_bit1_active_checkpoint",
-        value<chain::checkpoint>(&configured.bitcoin.bip9_bit1_active_checkpoint),
-        "The hash:height checkpoint for bip9 bit1 activation, defaults to 0000000000000000001c8018d9cb3b742ef25114f27563e3fc4a1902167f9893:481824."
+        "bitcoin.milestone",
+        value<chain::checkpoint>(&configured.bitcoin.milestone),
+        "A block presumed to be valid but not required to be present, defaults to 00000000000000000001a0a448d6cf2546b06801389cc030b2b18c6491266815:804000."
+    )
+    (
+        "bitcoin.minimum_work",
+        value<config::hash256>(&configured.bitcoin.minimum_work),
+        "The minimum work for any branch to be considered valid, defaults to 000000000000000000000000000000000000000052b2559353df4117b7348b64."
     )
 
     /* [network] */

src/protocols/protocol_block_in.cpp

@@ -136,15 +136,17 @@ bool protocol_block_in::handle_receive_block(const code& ec,
 
     // Alias.
     const auto& block_ptr = message->block_ptr;
+    const auto& block = *block_ptr;
+    const auto hash = block.hash();
 
     // Unrequested block, may not have been announced via inventory.
-    if (tracker->hashes.back() != block_ptr->hash())
+    if (tracker->hashes.back() != hash)
         return true;
 
     // Out of order or invalid.
-    if (block_ptr->header().previous_block_hash() != top_.hash())
+    if (block.header().previous_block_hash() != top_.hash())
     {
-        LOGP("Orphan block [" << encode_hash(block_ptr->hash())
+        LOGP("Orphan block [" << encode_hash(hash)
             << "] from [" << authority() << "].");
         return false;
     }
@@ -159,7 +161,7 @@ bool protocol_block_in::handle_receive_block(const code& ec,
     organize(block_ptr, BIND3(handle_organize, _1, height, block_ptr));
 
     // Set the new top and continue. Organize error will stop the channel.
-    top_ = { block_ptr->hash(), height };
+    top_ = { hash, height };
 
     // Order is reversed, so next is at back.
     tracker->hashes.pop_back();
@@ -199,23 +201,22 @@ void protocol_block_in::complete() NOEXCEPT
 void protocol_block_in::handle_organize(const code& ec, size_t height,
     const chain::block::cptr& block_ptr) NOEXCEPT
 {
-    if (ec == network::error::service_stopped)
+    if (ec == network::error::service_stopped || ec == error::duplicate_block)
         return;
 
-    if (!ec || ec == error::duplicate_block)
+    if (ec)
     {
-        LOGP("Block [" << encode_hash(block_ptr->hash())
+        // Assuming no store failure this is a consensus failure.
+        LOGR("Block [" << encode_hash(block_ptr->hash())
             << "] at (" << height << ") from [" << authority() << "] "
             << ec.message());
+        stop(ec);
         return;
     }
 
-    // Assuming no store failure this is a consensus failure.
-    LOGR("Block [" << encode_hash(block_ptr->hash())
+    LOGP("Block [" << encode_hash(block_ptr->hash())
         << "] at (" << height << ") from [" << authority() << "] "
         << ec.message());
-
-    stop(ec);
 }
 
 // private
@@ -255,11 +256,11 @@ get_blocks protocol_block_in::create_get_inventory(
 get_data protocol_block_in::create_get_data(
     const inventory& message) const NOEXCEPT
 {
-    get_data getter{};
-    getter.items.reserve(message.count(type_id::block));
-
     // clang emplace_back bug (no matching constructor), using push_back.
     // bip144: get_data uses witness constant but inventory does not.
+
+    get_data getter{};
+    getter.items.reserve(message.count(type_id::block));
     for (const auto& item: message.items)
         if ((item.type == type_id::block) && !archive().is_block(item.hash))
             getter.items.push_back({ block_type_, item.hash });

src/protocols/protocol_block_in_31800.cpp

@@ -18,9 +18,7 @@
  */
 #include <bitcoin/node/protocols/protocol_block_in_31800.hpp>
 
-#include <chrono>
 #include <functional>
-#include <utility>
 #include <bitcoin/system.hpp>
 #include <bitcoin/database.hpp>
 #include <bitcoin/network.hpp>
@@ -238,11 +236,11 @@ bool protocol_block_in_31800::handle_receive_block(const code& ec,
 get_data protocol_block_in_31800::create_get_data(
     const chaser_check::map& map) const NOEXCEPT
 {
-    get_data getter{};
-    getter.items.reserve(map.size());
-
     // clang emplace_back bug (no matching constructor), using push_back.
     // bip144: get_data uses witness constant but inventory does not.
+
+    get_data getter{};
+    getter.items.reserve(map.size());
     for (const auto& item: map)
         getter.items.push_back({ block_type_, item.first });