Compact blocks improvements.

include/bitcoin/node/protocols/protocol_header_in_70014.hpp

@@ -90,13 +90,15 @@ class BCN_API protocol_header_in_70014
         const database::header_link& link) NOEXCEPT;
     void request() NOEXCEPT;
     void identify() NOEXCEPT;
-    void resolve() NOEXCEPT;
+    bool resolve(const database::header_link& link) NOEXCEPT;
+    bool matched(const database::header_link& link) const NOEXCEPT;
 
     // These are protected by strand.
     bool compact_{};
     std::optional<fill> fill_{};
     database::header_link evidence_link_{};
-    system::hashes evidence_{};
+    system::hash_digest evidence_root_{};
+    size_t evidence_count_{};
 };
 
 } // namespace node

include/bitcoin/node/settings.hpp

@@ -57,6 +57,7 @@ class BCN_API settings
     uint32_t silent_start_height;
     uint16_t sample_period_seconds;
     uint16_t compact_timeout_seconds;
+    uint16_t compact_missing_percent;
     uint32_t currency_window_minutes;
     ////uint64_t snapshot_bytes;
     ////uint32_t snapshot_valid;

src/protocols/protocol_header_in_70014.cpp

@@ -80,22 +80,24 @@ bool protocol_header_in_70014::handle_receive_compact_block(const code& ec,
     if (stopped(ec))
         return false;
 
-    resolve();
+    const auto& header = message->header_ptr;
+    const auto& hash = header->get_hash();
+    const auto link = archive().to_header(hash);
+
+    if (!resolve(link))
+        return true;
+
     if (stopped())
         return false;
 
     // A compact block only extends a current chain, otherwise it is fetched.
     if (!is_current_chain(true))
         return true;
 
-    const auto& header = message->header_ptr;
-    const auto& hash = header->get_hash();
     set_announced(hash);
-
     if (fill_ && (fill_->hash == hash))
         return true;
 
-    const auto link = archive().to_header(hash);
     if (link.is_terminal())
     {
         organize_compact(header, BIND(handle_organize_compact, _1, _2,
@@ -157,12 +159,7 @@ void protocol_header_in_70014::collect(const compact_block& message,
 {
     BC_ASSERT(stranded());
     const auto& query = archive();
-
-    // Fill only a single block extension of the confirmed chain.
-    if (query.is_associated(link) ||
-        (query.to_candidate(query.get_top_candidate()) != link) ||
-        (query.to_confirmed(query.get_top_confirmed()) !=
-            query.to_parent(link)))
+    if (!query.is_candidate_extension(link))
         return;
 
     chain::context ctx{};
@@ -191,6 +188,11 @@ void protocol_header_in_70014::collect(const compact_block& message,
         return;
     }
 
+    // A block the pool cannot mostly fill is downloaded instead.
+    const auto percent = node_settings().compact_missing_percent;
+    if ((block.missing.size() * 100u) > (block.txs.size() * percent))
+        return;
+
     fill_.emplace(std::move(block));
     if (fill_->missing.empty())
         identify();
@@ -207,7 +209,7 @@ bool protocol_header_in_70014::decode(fill& block,
     const auto& items = message.transactions;
     const auto& ids = message.short_ids;
     const auto count = ids.size() + items.size();
-    if (is_zero(count) || (count > chain::max_block_weight))
+    if (is_zero(count) || (count > chain::max_block_size))
         return false;
 
     block.short_ids.resize(count);
@@ -365,16 +367,18 @@ void protocol_header_in_70014::identify() NOEXCEPT
     code ec{};
     const auto& first = *block.txs.front();
     const auto& root = block.header->merkle_root();
-    const auto segregated = !std::equal(txids.begin(), txids.end(),
-        wtxids.begin());
+    const auto segregated = !std::equal(txids.cbegin(), txids.cend(),
+        wtxids.cbegin());
+    const auto witness_root = sha256::merkle_root(std::move(wtxids));
     if ((ec = chain::block::identify(root, txids, is_malleated64(block))) ||
-        (ec = chain::block::identify(ctx, first, wtxids, segregated)))
+        (ec = chain::block::identify(ctx, first, witness_root, segregated)))
     {
         LOGR("Compact block [" << encode_hash(block.hash) << "] from ["
             << opposite() << "] " << ec.message());
 
         evidence_link_ = block.link;
-        evidence_ = std::move(wtxids);
+        evidence_root_ = witness_root;
+        evidence_count_ = block.txs.size();
         fill_.reset();
         return;
     }
@@ -479,17 +483,23 @@ void protocol_header_in_70014::do_submit_compact(const code& ec,
 // ----------------------------------------------------------------------------
 
 // Evidence of an unidentified compact block is held until the peer's next
-// compact block, when it is resolved against the block if then archived.
-void protocol_header_in_70014::resolve() NOEXCEPT
+// compact block, when it is resolved against the block if then archived. A
+// resend of the block in evidence is not filled while it remains unarchived.
+bool protocol_header_in_70014::resolve(
+    const database::header_link& link) NOEXCEPT
 {
     BC_ASSERT(stranded());
 
-    if (evidence_.empty())
-        return;
+    if (evidence_link_.is_terminal())
+        return true;
 
     const auto& query = archive();
-    if (query.is_associated(evidence_link_) &&
-        (query.get_wtxids(evidence_link_) != evidence_))
+    if (!query.is_associated(evidence_link_))
+    {
+        if (link == evidence_link_)
+            return false;
+    }
+    else if (!matched(evidence_link_))
     {
         LOGR("Invalid compact block ["
             << encode_hash(query.get_header_key(evidence_link_))
@@ -498,7 +508,16 @@ void protocol_header_in_70014::resolve() NOEXCEPT
     }
 
     evidence_link_ = {};
-    evidence_.clear();
+    return true;
+}
+
+bool protocol_header_in_70014::matched(
+    const database::header_link& link) const NOEXCEPT
+{
+    BC_ASSERT(stranded());
+    const auto& query = archive();
+    return (query.get_tx_count(link) == evidence_count_) &&
+        query.is_witness_committed(evidence_root_, link);
 }
 
 BC_POP_WARNING()

src/settings.cpp

@@ -55,6 +55,7 @@ settings::settings() NOEXCEPT
     maximum_concurrency{ 50'000 },
     sample_period_seconds{ 10 },
     compact_timeout_seconds{ 10 },
+    compact_missing_percent{ 50 },
     currency_window_minutes{ 1440 }
 {
 }

test/functional/p2p_compact.cpp

@@ -146,6 +146,17 @@ BOOST_AUTO_TEST_CASE(functional_p2p_compact__compact_block__short_id__requested_
     BOOST_REQUIRE(associated(block.hash()));
 }
 
+// The pool holds none of the block, so it is downloaded (not requested).
+BOOST_FIXTURE_TEST_CASE(functional_p2p_compact__compact_block__short_id_unpooled__not_requested, p2p_compact_pooled_setup_fixture)
+{
+    BOOST_REQUIRE(handshake(full_node));
+
+    send(shortened(block1()), node_version->value);
+    send(ping{ 42 }, node_version->value);
+
+    BOOST_REQUIRE(!received(get_compact_transactions::command, pong::command));
+}
+
 BOOST_AUTO_TEST_CASE(functional_p2p_compact__compact_block__short_id_mismatch__stopped)
 {
     BOOST_REQUIRE(handshake(full_node));
@@ -162,36 +173,32 @@ BOOST_AUTO_TEST_CASE(functional_p2p_compact__compact_block__short_id_mismatch__s
 }
 
 // An unidentified fill leaves evidence, resolved against the archived block.
-////BOOST_AUTO_TEST_CASE(functional_p2p_compact__compact_block__unidentified_then_archived__stopped)
-////{
-////    BOOST_REQUIRE(handshake(full_node));
-
-////    const auto& block = block1();
-////    const chain::block& genesis = config_.bitcoin.genesis_block;
-////    send(prefilled(block, genesis.transactions_ptr()->front()), node_version->value);
-////    BOOST_REQUIRE(await([&]() { return query_.get_top_candidate() == one; }));
-
-////    // Headers are proven, then requested again for archival. The deferred
-////    // compact top is then downloaded, as it is no longer the top.
-////    const headers announcement{ { block2().header_ptr() } };
-////    send(announcement, node_version->value);
-////    receive(get_headers::command);
-////    receive(get_headers::command);
-////    send(announcement, node_version->value);
-////    BOOST_REQUIRE(await([&]() { return query_.get_top_candidate() == two; }));
-
-////    const auto request = get_data::deserialize(node_version->value, receive(get_data::command));
-////    BOOST_REQUIRE(request);
-////    BOOST_REQUIRE(request->items.front().hash == block.hash());
-
-////    send(network::messages::peer::block::command, block.to_data(true));
-////    BOOST_REQUIRE(associated(block.hash()));
-
-////    send(prefilled(block, block.transactions_ptr()->front()), node_version->value);
-////    send(ping{ 42 }, node_version->value);
-
-////    BOOST_REQUIRE_THROW(receive(pong::command), boost::system::system_error);
-////}
+// Block 2 remains an unassociated candidate, so the chain never coalesces
+// (which would prune the store, suspending the network).
+BOOST_FIXTURE_TEST_CASE(functional_p2p_compact__compact_block__unidentified_then_archived__stopped, p2p_compact_candidate_setup_fixture)
+{
+    BOOST_REQUIRE(handshake(full_node));
+
+    // The unassociated candidates are requested on handshake.
+    const auto& block = block1();
+    const auto request = get_data::deserialize(node_version->value, receive(get_data::command));
+    BOOST_REQUIRE(request);
+    BOOST_REQUIRE(request->items.front().hash == block.hash());
+
+    // The fill is unidentified (wrong coinbase), the pong orders it.
+    const chain::block& genesis = config_.bitcoin.genesis_block;
+    send(prefilled(block, genesis.transactions_ptr()->front()), node_version->value);
+    send(ping{ 1 }, node_version->value);
+    receive(pong::command);
+
+    send(network::messages::peer::block::command, block.to_data(true));
+    BOOST_REQUIRE(associated(block.hash()));
+
+    send(prefilled(block, block.transactions_ptr()->front()), node_version->value);
+    send(ping{ 2 }, node_version->value);
+
+    BOOST_REQUIRE_THROW(receive(pong::command), boost::system::system_error);
+}
 
 // cmpctblock (announce)
 // ----------------------------------------------------------------------------

test/functional/p2p_setup_fixture.cpp

@@ -136,6 +136,20 @@ data_chunk p2p_setup_fixture::receive(const std::string& command)
     }
 }
 
+bool p2p_setup_fixture::received(const std::string& command,
+    const std::string& terminal)
+{
+    while (true)
+    {
+        const auto message = receive();
+        if (message.first == command)
+            return true;
+
+        if (message.first == terminal)
+            return false;
+    }
+}
+
 bool p2p_setup_fixture::handshake(uint64_t services, uint32_t value,
     bool relay)
 {

test/functional/p2p_setup_fixture.hpp

@@ -53,6 +53,9 @@ struct p2p_setup_fixture
     /// Read framed messages from the node until the command matches.
     system::data_chunk receive(const std::string& command);
 
+    /// Read framed messages until the command (true) or the terminal.
+    bool received(const std::string& command, const std::string& terminal);
+
     /// Perform the version handshake, retains the node's version message.
     bool handshake(uint64_t services=0,
         uint32_t version=network::messages::peer::level::maximum_protocol,
@@ -147,12 +150,13 @@ struct p2p_compact_setup_fixture
     static const system::chain::block& block2() NOEXCEPT;
 
     inline p2p_compact_setup_fixture(const initializer& setup={},
-        uint16_t compact_timeout=max_uint16)
+        uint16_t compact_timeout=max_uint16, uint16_t missing_percent=100)
       : p2p_setup_fixture(setup, [=](configuration& config)
         {
             config.network.enable_compact = true;
             config.node.currency_window_minutes = 0;
             config.node.compact_timeout_seconds = compact_timeout;
+            config.node.compact_missing_percent = missing_percent;
             config.bitcoin.minimum_work = {};
         })
     {
@@ -165,6 +169,16 @@ struct p2p_compact_setup_fixture
     bool associated(const system::hash_digest& hash);
 };
 
+// A compact node that fills only a block fully held by the pool.
+struct p2p_compact_pooled_setup_fixture
+  : p2p_compact_setup_fixture
+{
+    inline p2p_compact_pooled_setup_fixture()
+      : p2p_compact_setup_fixture({}, max_uint16, 0)
+    {
+    }
+};
+
 // A compact node with blocks 1 and 2 as unassociated candidates.
 struct p2p_compact_candidate_setup_fixture
   : p2p_compact_setup_fixture

test/settings.cpp

@@ -56,6 +56,7 @@ BOOST_AUTO_TEST_CASE(settings__node__default_context__expected)
     BOOST_REQUIRE_EQUAL(node.maximum_concurrency_(), 50000_size);
     BOOST_REQUIRE_EQUAL(node.sample_period_seconds, 10_u16);
     BOOST_REQUIRE_EQUAL(node.compact_timeout_seconds, 10_u16);
+    BOOST_REQUIRE_EQUAL(node.compact_missing_percent, 50_u16);
     BOOST_REQUIRE_EQUAL(node.currency_window_minutes, 1440_u32);
     ////BOOST_REQUIRE_EQUAL(node.snapshot_bytes, 200'000'000'000_u64);
     ////BOOST_REQUIRE_EQUAL(node.snapshot_valid, 250'000_u32);