From d9bbc938f48fa804acca2265860a644d71a979dc Mon Sep 17 00:00:00 2001 From: evoskuil Date: Thu, 2 Jan 2025 03:10:24 -0500 Subject: [PATCH 1/8] Style, comments. --- include/bitcoin/database/impl/query/translate.ipp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/bitcoin/database/impl/query/translate.ipp b/include/bitcoin/database/impl/query/translate.ipp index 092c9c35..febad9f3 100644 --- a/include/bitcoin/database/impl/query/translate.ipp +++ b/include/bitcoin/database/impl/query/translate.ipp @@ -218,7 +218,6 @@ TEMPLATE inline strong_pair CLASS::to_strong(const hash_digest& tx_hash) const NOEXCEPT { // Iteration of tx is necessary because there may be duplicates. - // Only top block (strong) association for given tx instance is considered. auto it = store_.tx.it(tx_hash); strong_pair strong{ {}, it.self() }; if (!it) @@ -226,10 +225,13 @@ inline strong_pair CLASS::to_strong(const hash_digest& tx_hash) const NOEXCEPT do { - strong.tx = it.self(); + // Only top block (strong) association for given tx is considered. strong.block = to_block(strong.tx); if (!strong.block.is_terminal()) + { + strong.tx = it.self(); return strong; + } } while (it.advance()); return strong; From 1306da2211093d4f5bbe9929135ab1529ecd300a Mon Sep 17 00:00:00 2001 From: evoskuil Date: Thu, 2 Jan 2025 03:11:32 -0500 Subject: [PATCH 2/8] Iterate for duplicate points for spent checks, fix populate_with_metadata. --- .../bitcoin/database/impl/query/archive.ipp | 5 +- .../bitcoin/database/impl/query/confirm.ipp | 53 ++++++++++--------- include/bitcoin/database/query.hpp | 9 ++-- 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/include/bitcoin/database/impl/query/archive.ipp b/include/bitcoin/database/impl/query/archive.ipp index a705145e..54c3738d 100644 --- a/include/bitcoin/database/impl/query/archive.ipp +++ b/include/bitcoin/database/impl/query/archive.ipp @@ -213,8 +213,11 @@ bool CLASS::populate_with_metadata(const input& input, if (!get_context(ctx, block)) return false; + const auto point_fk = to_point(input.point().hash); + const auto point_index = input.point().index; + input.metadata.coinbase = is_coinbase(tx); - input.metadata.spent = is_spent_prevout(input.point(), link); + input.metadata.spent = is_spent_prevout(point_fk, point_index, link); input.metadata.median_time_past = ctx.mtp; input.metadata.height = ctx.height; return true; diff --git a/include/bitcoin/database/impl/query/confirm.ipp b/include/bitcoin/database/impl/query/confirm.ipp index d3b99b59..96c9017d 100644 --- a/include/bitcoin/database/impl/query/confirm.ipp +++ b/include/bitcoin/database/impl/query/confirm.ipp @@ -135,7 +135,8 @@ bool CLASS::is_spent(const spend_link& link) const NOEXCEPT if (spend.is_null()) return false; - return is_spent_prevout(spend.prevout(), spend.parent_fk); + return is_spent_prevout(spend.point_fk, spend.point_index, + spend.parent_fk); } // unused @@ -234,39 +235,41 @@ code CLASS::locked_prevout(const point_link& link, uint32_t sequence, // protected TEMPLATE -bool CLASS::is_spent_prevout(const tx_link& link, index index) const NOEXCEPT -{ - const auto fp = table::spend::compose(link, index); - return is_spent_prevout(fp, tx_link::terminal); -} - -// protected -TEMPLATE -bool CLASS::is_spent_prevout(const foreign_point& point, +bool CLASS::is_spent_prevout(const point_link& link, index index, const tx_link& self) const NOEXCEPT { - return spent_prevout(point, self) != error::success; + return spent_prevout(link, index, self) != error::success; } // protected TEMPLATE -error::error_t CLASS::spent_prevout(const foreign_point& point, +error::error_t CLASS::spent_prevout(const point_link& link, index index, const tx_link& self) const NOEXCEPT { - auto it = store_.spend.it(point); - if (!it) - return error::success; + // Iteration of points by tx hash because there may be duplicates. + auto point = store_.point.it(get_point_key(link)); + if (!point) + return error::integrity; - table::spend::get_parent spend{}; do { - if (!store_.spend.get(it, spend)) - return error::integrity; + // Iterate all spends of each instance of the point. + auto it = store_.spend.it(table::spend::compose(point.self(), index)); + if (!it) + return error::success; - if ((spend.parent_fk != self) && is_strong_tx(spend.parent_fk)) - return error::confirmed_double_spend; + table::spend::get_parent spend{}; + do + { + if (!store_.spend.get(it, spend)) + return error::integrity; + + if ((spend.parent_fk != self) && is_strong_tx(spend.parent_fk)) + return error::confirmed_double_spend; + } + while (it.advance()); } - while (it.advance()); + while (point.advance()); return error::success; } @@ -379,7 +382,7 @@ code CLASS::tx_confirmable(const tx_link& link, // This query goes away. // If utxo exists then it is not spent (push own block first). - if (is_spent_prevout(spend.prevout(), link)) + if (is_spent_prevout(spend.point_fk, spend.point_index, link)) return error::confirmed_double_spend; } @@ -471,7 +474,7 @@ code CLASS::block_confirmable(const header_link& link) const NOEXCEPT { error::error_t ec{}; for (const auto& spend: set.spends) - if ((ec = spent_prevout(spend.prevout(), set.tx))) + if ((ec = spent_prevout(spend.point_fk, spend.point_index, set.tx))) { fault.store(ec); return true; @@ -514,7 +517,7 @@ code CLASS::block_confirmable(const header_link& link) const NOEXCEPT set.version, ctx))) return ec; - if (is_spent_prevout(spend.prevout(), set.tx)) + if (is_spent_prevout(spend.point_fk, spend.point_index, set.tx)) return error::confirmed_double_spend; } } @@ -545,7 +548,7 @@ code CLASS::block_confirmable(const header_link& link) const NOEXCEPT for (const auto& set: sets) for (const auto& spend: set.spends) - if (is_spent_prevout(spend.prevout(), set.tx)) + if (is_spent_prevout(spend.point_fk, spend.point_index, set.tx)) return error::confirmed_double_spend; return ec; diff --git a/include/bitcoin/database/query.hpp b/include/bitcoin/database/query.hpp index 7bd10bb1..dced62e2 100644 --- a/include/bitcoin/database/query.hpp +++ b/include/bitcoin/database/query.hpp @@ -577,11 +577,10 @@ class query const context& ctx) const NOEXCEPT; // Critical path - bool is_spent_prevout(const tx_link& link, index index) const NOEXCEPT; - bool is_spent_prevout(const foreign_point& point, - const tx_link& self) const NOEXCEPT; - error::error_t spent_prevout(const foreign_point& point, - const tx_link& self) const NOEXCEPT; + bool is_spent_prevout(const point_link& link, index index, + const tx_link& self=tx_link::terminal) const NOEXCEPT; + error::error_t spent_prevout(const point_link& link, index index, + const tx_link& self=tx_link::terminal) const NOEXCEPT; error::error_t unspendable_prevout(const point_link& link, uint32_t sequence, uint32_t version, const context& ctx) const NOEXCEPT; From e9fe60c5a32e786e2271899e8690ec2bf0da50ba Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 3 Jan 2025 13:15:34 -0500 Subject: [PATCH 3/8] Add is_spent_coinbase. --- .../bitcoin/database/impl/query/archive.ipp | 4 +- .../bitcoin/database/impl/query/confirm.ipp | 37 +++++++++++++------ .../bitcoin/database/impl/query/translate.ipp | 17 +++------ include/bitcoin/database/query.hpp | 1 + 4 files changed, 35 insertions(+), 24 deletions(-) diff --git a/include/bitcoin/database/impl/query/archive.ipp b/include/bitcoin/database/impl/query/archive.ipp index 54c3738d..9b311ab9 100644 --- a/include/bitcoin/database/impl/query/archive.ipp +++ b/include/bitcoin/database/impl/query/archive.ipp @@ -213,8 +213,8 @@ bool CLASS::populate_with_metadata(const input& input, if (!get_context(ctx, block)) return false; - const auto point_fk = to_point(input.point().hash); - const auto point_index = input.point().index; + const auto point_fk = to_point(input.point().hash()); + const auto point_index = input.point().index(); input.metadata.coinbase = is_coinbase(tx); input.metadata.spent = is_spent_prevout(point_fk, point_index, link); diff --git a/include/bitcoin/database/impl/query/confirm.ipp b/include/bitcoin/database/impl/query/confirm.ipp index 96c9017d..b9ac82c5 100644 --- a/include/bitcoin/database/impl/query/confirm.ipp +++ b/include/bitcoin/database/impl/query/confirm.ipp @@ -305,6 +305,9 @@ error::error_t CLASS::unspendable_prevout(const point_link& link, return error::success; } +// Duplicate tx instances (with the same hash) may result from a write race. +// Duplicate cb tx instances are allowed by consensus. Apart from two bip30 +// exceptions, duplicate cb txs are allowed only if previous are fully spent. TEMPLATE code CLASS::unspent_duplicates(const header_link& link, const context& ctx) const NOEXCEPT @@ -312,23 +315,25 @@ code CLASS::unspent_duplicates(const header_link& link, if (!ctx.is_enabled(system::chain::flags::bip30_rule)) return error::success; - // This will be empty if current block is not set_strong. - const auto coinbases = to_strong_txs(get_tx_key(to_coinbase(link))); + auto coinbases = to_strong_txs(get_tx_key(to_coinbase(link))); + // Found only this block's coinbase instance, no duplicates. if (is_one(coinbases.size())) return error::success; - if (coinbases.empty()) + // Remove self (will be not found if current block is not set_strong). + const auto self = std::find(coinbases.begin(), coinbases.end(), link); + if (self == coinbases.end() || coinbases.erase(self) == coinbases.end()) return error::integrity; - // bip30: all (but self) must be confirmed spent or dup invalid (cb only). - size_t unspent{}; - for (const auto& tx: coinbases) - for (index out{}; out < output_count(tx); ++out) - if (!is_spent_prevout(tx, out) && is_one(unspent++)) - return error::unspent_coinbase_collision; + const auto spent = [this](const auto& tx) NOEXCEPT + { + return is_spent_coinbase(tx); + }; - return is_zero(unspent) ? error::integrity : error::success; + // bip30: all outputs of all previous duplicate coinbases must be spent. + return std::all_of(coinbases.begin(), coinbases.end(), spent) ? + error::success : error::unspent_coinbase_collision; } #if defined(UNDEFINED) @@ -415,7 +420,6 @@ code CLASS::block_confirmable(const header_link& link) const NOEXCEPT #endif - // protected TEMPLATE spend_sets CLASS::to_spend_sets(const header_link& link) const NOEXCEPT @@ -556,6 +560,17 @@ code CLASS::block_confirmable(const header_link& link) const NOEXCEPT #endif // DISABLED +TEMPLATE +bool CLASS::is_spent_coinbase(const tx_link& link) const NOEXCEPT +{ + const auto point_fk = to_point(get_tx_key(link)); + for (index index{}; index < output_count(link); ++index) + if (!is_spent_prevout(point_fk, index)) + return false; + + return true; +} + TEMPLATE bool CLASS::is_strong_tx(const tx_link& link) const NOEXCEPT { diff --git a/include/bitcoin/database/impl/query/translate.ipp b/include/bitcoin/database/impl/query/translate.ipp index febad9f3..b102a0ff 100644 --- a/include/bitcoin/database/impl/query/translate.ipp +++ b/include/bitcoin/database/impl/query/translate.ipp @@ -239,11 +239,7 @@ inline strong_pair CLASS::to_strong(const hash_digest& tx_hash) const NOEXCEPT // protected // 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. +// Return distinct set of txs by link for hash where each is strong by block. TEMPLATE inline tx_links CLASS::to_strong_txs(const hash_digest& tx_hash) const NOEXCEPT { @@ -263,11 +259,10 @@ inline tx_links CLASS::to_strong_txs(const hash_digest& tx_hash) const NOEXCEPT // protected // 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 tx links where each tx is strong by block. +// 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 distinct set of txs by link where each is strong by block. TEMPLATE inline tx_links CLASS::to_strong_txs(const tx_link& link) const NOEXCEPT { @@ -275,7 +270,7 @@ inline tx_links CLASS::to_strong_txs(const tx_link& link) const NOEXCEPT if (!it) return {}; - // Obtain all first (by block) duplicate (by hash) tx records. + // Obtain all first (by block) duplicate (by link) tx records. maybe_strongs pairs{}; do { diff --git a/include/bitcoin/database/query.hpp b/include/bitcoin/database/query.hpp index dced62e2..3a33e22b 100644 --- a/include/bitcoin/database/query.hpp +++ b/include/bitcoin/database/query.hpp @@ -498,6 +498,7 @@ class query /// These are not used in confirmation. /// These rely on strong (use only for confirmation process). bool is_spent(const spend_link& link) const NOEXCEPT; + bool is_spent_coinbase(const tx_link& link) const NOEXCEPT; bool is_strong_tx(const tx_link& link) const NOEXCEPT; bool is_strong_block(const header_link& link) const NOEXCEPT; bool is_strong_spend(const spend_link& link) const NOEXCEPT; From 785434334508c8b0db432fa42a8fbd657be98012 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 3 Jan 2025 02:56:44 -0500 Subject: [PATCH 4/8] Confirmation optimization comments, style. --- .../bitcoin/database/impl/query/confirm.ipp | 94 ++++++++++++++++--- 1 file changed, 80 insertions(+), 14 deletions(-) diff --git a/include/bitcoin/database/impl/query/confirm.ipp b/include/bitcoin/database/impl/query/confirm.ipp index b9ac82c5..9ac51971 100644 --- a/include/bitcoin/database/impl/query/confirm.ipp +++ b/include/bitcoin/database/impl/query/confirm.ipp @@ -246,14 +246,47 @@ TEMPLATE error::error_t CLASS::spent_prevout(const point_link& link, index index, const tx_link& self) const NOEXCEPT { - // Iteration of points by tx hash because there may be duplicates. + // TODO: get_point_key(link) is redundant with unspendable_prevout(). + // searches [point.iterate {new} x (spend.iterate + strong_tx.find)]. + + // The search for spenders must be exhaustive. + // This is walking the full conflict list for the hash, but there is only + // one match possible (self) unless there are duplicates/conflicts. + // Conflicts here are both likely tx pool conflicts and rare duplicate txs, + // since the points table is written for each spend (unless compressed and + // that is still not a guarantee. So all must be checked. This holds one + // instance of a tx for ***all spends of all outputs*** of that tx. + + // TODO: evaluate. + // If point hash was in spend table key there would be just as many but the + // key would be the hash and index combined, resulting in no unnecessary + // point hash searches over irrelevant point indexes. Would save some space + // in table compression, and simplify some code, but would eliminate store + // compression and might increase paging cost due to spend table increase. + // There would be only one search unless duplicates, and this would be self + // so would not result in calling the is_strong_tx search. Spenders of outs + // of the same prevout.tx would not result in search hits. Table no-hash + // algorithm would require definition. This would eliminate spend.point_fk + // and a point.pk link per record, or 8 bytes per spend. This is the amount + // to be added by the new array cache table, maybe just repurpose point. + // Because cache can be removed this is a 19GB reduction, for the loss of + // ability to reduce 50GB, which we don't generally do. So also a win on + // nominal store size. All we need from the cache is the spend.pk/index. + // spend.pk size does not change because it's an array (count unchanged). + // So this is a reduction from plan, 4+3 bytes per cache row vs. 5+3. + // But if we hold the spend.pk/prevout.tx we can just read the + // spend.hash/index, so we don't need to store the index, and we need to + // read the spend.hash anyway, so index is free (no paging). So that's now + // just spend[4] + tx[4], back to 8 bytes (19GB). + + // Iterate points by point hash (of output tx) because may be conflicts. auto point = store_.point.it(get_point_key(link)); if (!point) return error::integrity; do { - // Iterate all spends of each instance of the point. + // Iterate all spends of the point to find double spends. auto it = store_.spend.it(table::spend::compose(point.self(), index)); if (!it) return error::success; @@ -264,6 +297,8 @@ error::error_t CLASS::spent_prevout(const point_link& link, index index, if (!store_.spend.get(it, spend)) return error::integrity; + // is_strong_tx (search) only called in the case of duplicate. + // Other parent tx of spend is strong (confirmed spent prevout). if ((spend.parent_fk != self) && is_strong_tx(spend.parent_fk)) return error::confirmed_double_spend; } @@ -273,27 +308,53 @@ error::error_t CLASS::spent_prevout(const point_link& link, index index, return error::success; } +// Low cost. +// header_link +// header_link.ctx.mtp +// header_link.ctx.flags +// header_link.ctx.height +// header_link:txs.tx.pk +// header_link:txs.tx.version + +// unspendable_prevout +// Given that these use the same txs association, there is no way for the +// header.txs.tx to change, and it is only ever this pk that is set strong. +// If unconfirmed_spend is encountered, perform a search (free). It's not +// possible for a confirmed spend to be the wrong tx instance. +// +// There is no strong (prevout->tx->block) association at this time. +// strong_tx is interrogated for each spend except self (0) and each prevout (2.6B). +// to_tx(get_point_key(header_link:txs.tx.puts.spend.point_fk)):block.ctx.height|mtp +// This is done in populate, except for to_strong, ***so save prevout tx [4]*** +// This would increase the cache to 11 bytes per row (27GB) less 19GB savings (above), +// and the cache can be dropped (for less performant query). +// If the cached prevout tx is not strong, perform duplicate search (free). +// +// is_coinbase_mature(is_coinbase(header_link:txs.tx), ...block.ctx.height), is_locked +// is_locked(header_link:txs.tx.puts.spend.sequence, ...block.ctx.height|mtp) + +// spent_prevout (see notes in fn). +// header_link:txs.tx.puts.spend.point_index + // protected TEMPLATE error::error_t CLASS::unspendable_prevout(const point_link& link, uint32_t sequence, uint32_t version, const context& ctx) const NOEXCEPT { - // utxo.find(spend.prevout()) no iteration or hash conversion. - // Read utxo => is_coinbase, header_link => ctx (height/mtp). - const auto strong = to_strong(get_point_key(link)); + // TODO: get_point_key(link) is redundant with spent_prevout(). + // to_strong has the only searches [tx.iterate, strong.find]. + const auto strong_prevout = to_strong(get_point_key(link)); - // utxo is strong if present. - if (strong.block.is_terminal()) - return strong.tx.is_terminal() ? error::missing_previous_output : - error::unconfirmed_spend; + // prevout is strong if present. + if (strong_prevout.block.is_terminal()) + return strong_prevout.tx.is_terminal() ? + error::missing_previous_output : error::unconfirmed_spend; - // utxo get context is still required. context out{}; - if (!get_context(out, strong.block)) + if (!get_context(out, strong_prevout.block)) return error::integrity; - // utxo.is_coinbase (is known). - if (is_coinbase(strong.tx) && + if (is_coinbase(strong_prevout.tx) && !transaction::is_coinbase_mature(out.height, ctx.height)) return error::coinbase_maturity; @@ -312,9 +373,11 @@ TEMPLATE code CLASS::unspent_duplicates(const header_link& link, const context& ctx) const NOEXCEPT { + // This is generally going to be disabled. if (!ctx.is_enabled(system::chain::flags::bip30_rule)) return error::success; + // [txs.find, {tx.iterate}, strong_tx.it] auto coinbases = to_strong_txs(get_tx_key(to_coinbase(link))); // Found only this block's coinbase instance, no duplicates. @@ -326,6 +389,7 @@ code CLASS::unspent_duplicates(const header_link& link, if (self == coinbases.end() || coinbases.erase(self) == coinbases.end()) return error::integrity; + // [point.first, is_spent_prevout()] const auto spent = [this](const auto& tx) NOEXCEPT { return is_spent_coinbase(tx); @@ -424,7 +488,8 @@ code CLASS::block_confirmable(const header_link& link) const NOEXCEPT TEMPLATE spend_sets CLASS::to_spend_sets(const header_link& link) const NOEXCEPT { - // Coinbase tx does not spend. + // This is the only search [txs.find]. + // Coinbase tx does not spend so is not retrieved. const auto txs = to_spending_transactions(link); if (txs.empty()) @@ -454,6 +519,7 @@ code CLASS::block_confirmable(const header_link& link) const NOEXCEPT if ((ec = unspent_duplicates(link, ctx))) return ec; + // This is also eliminated by caching, since we cache each spend. const auto sets = to_spend_sets(link); if (sets.empty()) return ec; From 090290ff1fa82ddca4114cc05014c0072201296c Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 3 Jan 2025 13:40:26 -0500 Subject: [PATCH 5/8] Overload input.metadata.height to populate prevout's tx_link. --- include/bitcoin/database/impl/query/archive.ipp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/include/bitcoin/database/impl/query/archive.ipp b/include/bitcoin/database/impl/query/archive.ipp index 9b311ab9..78732af7 100644 --- a/include/bitcoin/database/impl/query/archive.ipp +++ b/include/bitcoin/database/impl/query/archive.ipp @@ -142,9 +142,15 @@ bool CLASS::populate(const input& input) const NOEXCEPT if (input.prevout) return true; + // HACK: overloading input.metadata.height with tx link. + static_assert(sizeof(size_t) >= sizeof(tx_link::integer)); + const auto tx = to_tx(input.point().hash()); + input.metadata.height = tx; + input.prevout = get_output(tx, input.point().index()); + // input.metadata is not populated. // Null point would return nullptr and be interpreted as missing. - input.prevout = get_output(input.point()); + ////input.prevout = get_output(input.point()); return !is_null(input.prevout); } From 66b15ffc237585516735d3c828b35e266ccd707d Mon Sep 17 00:00:00 2001 From: evoskuil Date: Fri, 3 Jan 2025 18:17:09 -0500 Subject: [PATCH 6/8] Populate: input.metadata.coinbase = is_coinbase(tx). --- include/bitcoin/database/impl/query/archive.ipp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/bitcoin/database/impl/query/archive.ipp b/include/bitcoin/database/impl/query/archive.ipp index 78732af7..3e97f8d7 100644 --- a/include/bitcoin/database/impl/query/archive.ipp +++ b/include/bitcoin/database/impl/query/archive.ipp @@ -146,6 +146,7 @@ bool CLASS::populate(const input& input) const NOEXCEPT static_assert(sizeof(size_t) >= sizeof(tx_link::integer)); const auto tx = to_tx(input.point().hash()); input.metadata.height = tx; + input.metadata.coinbase = is_coinbase(tx); input.prevout = get_output(tx, input.point().index()); // input.metadata is not populated. From c8504e17c10f27f615b938a1066a5a768b03eae7 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Sat, 4 Jan 2025 01:18:11 -0500 Subject: [PATCH 7/8] Add/update populate tests with metadata, style. --- .../bitcoin/database/impl/query/archive.ipp | 6 +- test/query/archive.cpp | 78 ++++++++++++------- 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/include/bitcoin/database/impl/query/archive.ipp b/include/bitcoin/database/impl/query/archive.ipp index 3e97f8d7..cb91e2f1 100644 --- a/include/bitcoin/database/impl/query/archive.ipp +++ b/include/bitcoin/database/impl/query/archive.ipp @@ -142,12 +142,10 @@ bool CLASS::populate(const input& input) const NOEXCEPT if (input.prevout) return true; - // HACK: overloading input.metadata.height with tx link. - static_assert(sizeof(size_t) >= sizeof(tx_link::integer)); const auto tx = to_tx(input.point().hash()); - input.metadata.height = tx; - input.metadata.coinbase = is_coinbase(tx); input.prevout = get_output(tx, input.point().index()); + input.metadata.coinbase = is_coinbase(tx); + input.metadata.parent = tx; // input.metadata is not populated. // Null point would return nullptr and be interpreted as missing. diff --git a/test/query/archive.cpp b/test/query/archive.cpp index a1c59fa8..4731aca4 100644 --- a/test/query/archive.cpp +++ b/test/query/archive.cpp @@ -773,24 +773,14 @@ BOOST_AUTO_TEST_CASE(query_archive__populate__null_prevouts__true) BOOST_REQUIRE(query.set(test::block3, test::context, false, false)); system::chain::block copy{ test::genesis }; - BOOST_REQUIRE(query.populate(copy)); - ////BOOST_REQUIRE(query.populate(*test::genesis.transactions_ptr()->front())); - ////BOOST_REQUIRE(query.populate(*test::genesis.inputs_ptr()->front())); - system::chain::block copy1{ test::block1 }; - BOOST_REQUIRE(query.populate(copy1)); - ////BOOST_REQUIRE(query.populate(*test::block1.transactions_ptr()->front())); - ////BOOST_REQUIRE(query.populate(*test::block1.inputs_ptr()->front())); - system::chain::block copy2{ test::block2 }; - BOOST_REQUIRE(query.populate(copy2)); - ////BOOST_REQUIRE(query.populate(*test::block2.transactions_ptr()->front())); - ///BOOST_REQUIRE(query.populate(*test::block2.inputs_ptr()->front())); - system::chain::block copy3{ test::block3 }; + + BOOST_REQUIRE(query.populate(copy)); + BOOST_REQUIRE(query.populate(copy1)); + BOOST_REQUIRE(query.populate(copy2)); BOOST_REQUIRE(query.populate(copy3)); - ////BOOST_REQUIRE(query.populate(*test::block3.transactions_ptr()->front())); - ////BOOST_REQUIRE(query.populate(*test::block3.inputs_ptr()->front())); } BOOST_AUTO_TEST_CASE(query_archive__populate__partial_prevouts__false) @@ -809,22 +799,58 @@ BOOST_AUTO_TEST_CASE(query_archive__populate__partial_prevouts__false) // Block populate treates first tx as null point. BOOST_REQUIRE( query.populate(copy1)); - BOOST_REQUIRE(!query.populate(*test::block1a.transactions_ptr()->front())); - BOOST_REQUIRE(!query.populate(*test::block1a.inputs_ptr()->front())); - BOOST_REQUIRE(!query.populate(*test::block1a.inputs_ptr()->back())); + BOOST_REQUIRE(!query.populate(*copy1.transactions_ptr()->at(0))); + BOOST_REQUIRE(!query.populate(*copy1.inputs_ptr()->at(0))); + BOOST_REQUIRE(!query.populate(*copy1.inputs_ptr()->at(2))); // Block populate treates first tx as null point and other has missing prevouts. system::chain::block copy2{ test::block2a }; BOOST_REQUIRE(!query.populate(copy2)); - BOOST_REQUIRE( query.populate(*test::block2a.transactions_ptr()->front())); - BOOST_REQUIRE(!query.populate(*test::block2a.transactions_ptr()->back())); - BOOST_REQUIRE( query.populate(*test::block2a.inputs_ptr()->front())); - BOOST_REQUIRE(!query.populate(*test::block2a.inputs_ptr()->back())); - - // Block populate treates first tx as null point and other has found prevouts. - BOOST_REQUIRE(query.populate(test::tx4)); - BOOST_REQUIRE(query.populate(*test::tx4.inputs_ptr()->front())); - BOOST_REQUIRE(query.populate(*test::tx4.inputs_ptr()->back())); + BOOST_REQUIRE( query.populate(*copy2.transactions_ptr()->at(0))); + BOOST_REQUIRE(!query.populate(*copy2.transactions_ptr()->at(1))); + BOOST_REQUIRE( query.populate(*copy2.inputs_ptr()->at(0))); + BOOST_REQUIRE(!query.populate(*copy2.inputs_ptr()->at(3))); + + system::chain::transaction copy4{ test::tx4 }; + BOOST_REQUIRE(query.populate(copy4)); + BOOST_REQUIRE(query.populate(*copy4.inputs_ptr()->at(0))); + BOOST_REQUIRE(query.populate(*copy4.inputs_ptr()->at(1))); +} + +BOOST_AUTO_TEST_CASE(query_archive__populate__metadata__expected) +{ + settings settings{}; + settings.path = TEST_DIRECTORY; + test::chunk_store store{ settings }; + test::query_accessor query{ store }; + BOOST_REQUIRE(!store.create(events_handler)); + BOOST_REQUIRE(query.initialize(test::genesis)); + BOOST_REQUIRE(query.set(test::block1a, test::context, false, false)); + BOOST_REQUIRE(query.set(test::block2a, test::context, false, false)); + BOOST_REQUIRE(query.set(test::tx4)); + BOOST_REQUIRE(!query.is_coinbase(1)); + + // Genesis only has coinbase, which does not spend. + system::chain::block copy{ test::genesis }; + BOOST_REQUIRE(query.populate(copy)); + BOOST_REQUIRE(!copy.inputs_ptr()->at(0)->prevout); + + // Transaction population. + system::chain::transaction copy4{ test::tx4 }; + BOOST_REQUIRE(query.populate(copy4)); + + // TODO: test non-coinbase and other parent. + // spent/mtp are defaults, coinbase/parent are set (to non-default values). + BOOST_REQUIRE( copy4.inputs_ptr()->at(0)->prevout); + BOOST_REQUIRE( copy4.inputs_ptr()->at(0)->metadata.spent); + BOOST_REQUIRE(!copy4.inputs_ptr()->at(0)->metadata.coinbase); + BOOST_REQUIRE_EQUAL(copy4.inputs_ptr()->at(0)->metadata.parent, 1u); + BOOST_REQUIRE_EQUAL(copy4.inputs_ptr()->at(0)->metadata.median_time_past, max_uint32); + BOOST_REQUIRE( copy4.inputs_ptr()->at(1)->prevout); + BOOST_REQUIRE( copy4.inputs_ptr()->at(1)->metadata.spent); + BOOST_REQUIRE(!copy4.inputs_ptr()->at(1)->metadata.coinbase); + BOOST_REQUIRE_EQUAL(copy4.inputs_ptr()->at(1)->metadata.parent, 1u); + BOOST_REQUIRE_EQUAL(copy4.inputs_ptr()->at(1)->metadata.median_time_past, max_uint32); } // archive (foreign-keyed) From 6e8a6a53dbfa1d72393a666cc621b6920dce95c8 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Sat, 4 Jan 2025 01:18:36 -0500 Subject: [PATCH 8/8] Comments. --- include/bitcoin/database/impl/query/confirm.ipp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/include/bitcoin/database/impl/query/confirm.ipp b/include/bitcoin/database/impl/query/confirm.ipp index 9ac51971..ce63f284 100644 --- a/include/bitcoin/database/impl/query/confirm.ipp +++ b/include/bitcoin/database/impl/query/confirm.ipp @@ -322,13 +322,10 @@ error::error_t CLASS::spent_prevout(const point_link& link, index index, // If unconfirmed_spend is encountered, perform a search (free). It's not // possible for a confirmed spend to be the wrong tx instance. // -// There is no strong (prevout->tx->block) association at this time. +// There is no strong (prevout->tx->block) association at this point in validation. // strong_tx is interrogated for each spend except self (0) and each prevout (2.6B). // to_tx(get_point_key(header_link:txs.tx.puts.spend.point_fk)):block.ctx.height|mtp // This is done in populate, except for to_strong, ***so save prevout tx [4]*** -// This would increase the cache to 11 bytes per row (27GB) less 19GB savings (above), -// and the cache can be dropped (for less performant query). -// If the cached prevout tx is not strong, perform duplicate search (free). // // is_coinbase_mature(is_coinbase(header_link:txs.tx), ...block.ctx.height), is_locked // is_locked(header_link:txs.tx.puts.spend.sequence, ...block.ctx.height|mtp) @@ -341,7 +338,12 @@ TEMPLATE error::error_t CLASS::unspendable_prevout(const point_link& link, uint32_t sequence, uint32_t version, const context& ctx) const NOEXCEPT { - // TODO: get_point_key(link) is redundant with spent_prevout(). + // TODO: If unconfirmed_spend is encountered, perform a search (free). + // It's not possible for a confirmed spend to be the wrong tx instance. + // This eliminates the hash lookup and to_strong(hash) iteration. + + // TODO: don't need to return tx link here, just the block (for strong/context). + // MOOT: get_point_key(link) is redundant with spent_prevout(). // to_strong has the only searches [tx.iterate, strong.find]. const auto strong_prevout = to_strong(get_point_key(link)); @@ -354,6 +356,8 @@ error::error_t CLASS::unspendable_prevout(const point_link& link, if (!get_context(out, strong_prevout.block)) return error::integrity; + // All txs with same hash must be coinbase or not, so this query is redundant. + // TODO: Just use the cached value for the prevout, obtained in validation. if (is_coinbase(strong_prevout.tx) && !transaction::is_coinbase_mature(out.height, ctx.height)) return error::coinbase_maturity; @@ -515,11 +519,12 @@ code CLASS::block_confirmable(const header_link& link) const NOEXCEPT if (!get_context(ctx, link)) return error::integrity; + // This is never invoked (bip30). code ec{}; if ((ec = unspent_duplicates(link, ctx))) return ec; - // This is also eliminated by caching, since we cache each spend. + // This is eliminated by caching, since each non-internal spend is cached. const auto sets = to_spend_sets(link); if (sets.empty()) return ec;