From 79e89f3479dfe2e650216f7d6d0a88a0ffec7100 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Sat, 25 May 2024 17:37:30 -0400 Subject: [PATCH 1/3] Replace !is_coinbase() assertions with runtime bypass. --- src/chain/transaction.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/chain/transaction.cpp b/src/chain/transaction.cpp index f14c6f1721..44b4a28a37 100644 --- a/src/chain/transaction.cpp +++ b/src/chain/transaction.cpp @@ -1281,14 +1281,14 @@ code transaction::check(const context& ctx) const NOEXCEPT return error::transaction_success; } -// Do NOT invoke on coinbase. +// Do not need to invoke on coinbase. // This assumes that prevout caching is completed on all inputs. code transaction::accept(const context&) const NOEXCEPT { - BC_ASSERT(!is_coinbase()); + ////BC_ASSERT(!is_coinbase()); - ////if (is_coinbase()) - //// return error::transaction_success; + if (is_coinbase()) + return error::transaction_success; if (is_missing_prevouts()) return error::missing_previous_output; if (is_overspent()) @@ -1301,16 +1301,16 @@ code transaction::accept(const context&) const NOEXCEPT // height // median_time_past -// Do NOT invoke on coinbase. +// Do not need to invoke on coinbase. // Node performs these checks through database query. // This assumes that prevout and metadata caching are completed on all inputs. code transaction::confirm(const context& ctx) const NOEXCEPT { - BC_ASSERT(!is_coinbase()); + ////BC_ASSERT(!is_coinbase()); const auto bip68 = ctx.is_enabled(bip68_rule); - ////if (is_coinbase()) - //// return error::transaction_success; + if (is_coinbase()) + return error::transaction_success; if (bip68 && is_locked(ctx.height, ctx.median_time_past)) return error::relative_time_locked; if (is_immature(ctx.height)) @@ -1328,13 +1328,13 @@ code transaction::confirm(const context& ctx) const NOEXCEPT // forks -// Do NOT invoke on coinbase. +// Do not need to invoke on coinbase. code transaction::connect(const context& ctx) const NOEXCEPT { - BC_ASSERT(!is_coinbase()); + ////BC_ASSERT(!is_coinbase()); - ////if (is_coinbase()) - //// return error::transaction_success; + if (is_coinbase()) + return error::transaction_success; code ec; using namespace machine; From a464b79cb37b484e4716073bcf46a5e45e0eec02 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Sat, 25 May 2024 22:40:48 -0400 Subject: [PATCH 2/3] Comment. --- src/wallet/neutrino_filter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/neutrino_filter.cpp b/src/wallet/neutrino_filter.cpp index 12b3a7ef36..1593bcee3e 100644 --- a/src/wallet/neutrino_filter.cpp +++ b/src/wallet/neutrino_filter.cpp @@ -64,7 +64,7 @@ bool compute_filter(data_chunk& out, const chain::block& block) NOEXCEPT const auto& script = output->script(); // bip138: any "nil" items MUST NOT be included. - // bip138:exclude all outputs that start with OP_RETURN. + // bip138: exclude all outputs that start with OP_RETURN. if (!script.ops().empty() && !chain::script::is_pay_op_return_pattern(script.ops())) scripts.push_back(script.to_data(false)); From 232e93412511f7775947ff74bb1919ca132c2340 Mon Sep 17 00:00:00 2001 From: evoskuil Date: Mon, 27 May 2024 18:10:00 -0400 Subject: [PATCH 3/3] Performance analysis comments. --- include/bitcoin/system/chain/header.hpp | 3 ++- include/bitcoin/system/chain/transaction.hpp | 5 +++-- src/chain/block.cpp | 2 ++ src/chain/input.cpp | 1 + src/chain/operation.cpp | 1 + src/chain/output.cpp | 1 + src/chain/script.cpp | 2 ++ src/chain/transaction.cpp | 1 + src/chain/witness.cpp | 1 + 9 files changed, 14 insertions(+), 3 deletions(-) diff --git a/include/bitcoin/system/chain/header.hpp b/include/bitcoin/system/chain/header.hpp index d4eb3572ef..538bdc0e50 100644 --- a/include/bitcoin/system/chain/header.hpp +++ b/include/bitcoin/system/chain/header.hpp @@ -148,7 +148,8 @@ class BC_API header uint32_t nonce_; bool valid_; - // Identity hash cashing. + // Identity hash caching. + // TODO: use std::optional to avoid this pointer allocation. mutable std::shared_ptr hash_{}; }; diff --git a/include/bitcoin/system/chain/transaction.hpp b/include/bitcoin/system/chain/transaction.hpp index 22f4a3526c..38658f65d8 100644 --- a/include/bitcoin/system/chain/transaction.hpp +++ b/include/bitcoin/system/chain/transaction.hpp @@ -261,8 +261,9 @@ class BC_API transaction void initialize_hash_cache() const NOEXCEPT; - // Signature and identity hash cashing (witness hash if witnessed). - mutable std::unique_ptr cache_{}; + // TODO: use std::optional to avoid these pointer allocations (0.16%). + // Signature and identity hash caching (witness hash if witnessed). + mutable std::unique_ptr cache_{}; mutable std::unique_ptr nominal_hash_{}; mutable std::unique_ptr witness_hash_{}; }; diff --git a/src/chain/block.cpp b/src/chain/block.cpp index c1ce8e65c2..b91f96b133 100644 --- a/src/chain/block.cpp +++ b/src/chain/block.cpp @@ -263,6 +263,7 @@ hash_digest block::hash() const NOEXCEPT return header_->hash(); } +// TODO: this is expensive. size_t block::serialized_size(bool witness) const NOEXCEPT { // Overflow returns max_size_t. @@ -350,6 +351,7 @@ size_t block::non_coinbase_inputs() const NOEXCEPT return std::accumulate(std::next(txs_->begin()), txs_->end(), zero, inputs); } +// TODO: this is expensive (1.85%). // This also precludes the block merkle calculation DoS exploit. // bitcointalk.org/?topic=102395 bool block::is_internal_double_spend() const NOEXCEPT diff --git a/src/chain/input.cpp b/src/chain/input.cpp index 2e634862d0..db9ca74d93 100644 --- a/src/chain/input.cpp +++ b/src/chain/input.cpp @@ -220,6 +220,7 @@ void input::to_data(writer& sink) const NOEXCEPT sink.write_4_bytes_little_endian(sequence_); } +// TODO: this is expensive. size_t input::serialized_size(bool witness) const NOEXCEPT { // input.serialized_size(witness) provides sizing for witness, however diff --git a/src/chain/operation.cpp b/src/chain/operation.cpp index a951b465b9..7d6c952757 100644 --- a/src/chain/operation.cpp +++ b/src/chain/operation.cpp @@ -459,6 +459,7 @@ const chunk_cptr& operation::data_ptr() const NOEXCEPT return data_; } +// TODO: this is expensive. size_t operation::serialized_size() const NOEXCEPT { static constexpr auto op_size = sizeof(uint8_t); diff --git a/src/chain/output.cpp b/src/chain/output.cpp index 02cbb79653..35bba72a49 100644 --- a/src/chain/output.cpp +++ b/src/chain/output.cpp @@ -157,6 +157,7 @@ void output::to_data(writer& sink) const NOEXCEPT script_->to_data(sink, true); } +// TODO: this is expensive. size_t output::serialized_size() const NOEXCEPT { return sizeof(value_) + script_->serialized_size(true); diff --git a/src/chain/script.cpp b/src/chain/script.cpp index 30f4ba7993..330c1dc26d 100644 --- a/src/chain/script.cpp +++ b/src/chain/script.cpp @@ -200,6 +200,7 @@ size_t script::op_count(reader& source) NOEXCEPT const auto start = source.get_read_position(); auto count = zero; + // TODO: this is expensive (0.83%). while (operation::count_op(source)) ++count; @@ -358,6 +359,7 @@ hash_digest script::hash() const NOEXCEPT return sha256; } +// TODO: this is expensive. size_t script::serialized_size(bool prefix) const NOEXCEPT { const auto op_size = [](size_t total, const operation& op) NOEXCEPT diff --git a/src/chain/transaction.cpp b/src/chain/transaction.cpp index 44b4a28a37..97c2693caa 100644 --- a/src/chain/transaction.cpp +++ b/src/chain/transaction.cpp @@ -338,6 +338,7 @@ void transaction::to_data(writer& sink, bool witness) const NOEXCEPT sink.write_4_bytes_little_endian(locktime_); } +// TODO: this is expensive. size_t transaction::serialized_size(bool witness) const NOEXCEPT { witness &= segregated_; diff --git a/src/chain/witness.cpp b/src/chain/witness.cpp index 963168b8a9..3cbe8a8c28 100644 --- a/src/chain/witness.cpp +++ b/src/chain/witness.cpp @@ -293,6 +293,7 @@ size_t witness::serialized_size() const NOEXCEPT return std::accumulate(stack_.begin(), stack_.end(), zero, sum); } +// TODO: this is expensive. size_t witness::serialized_size(bool prefix) const NOEXCEPT { // Witness prefix is an element count, not a byte length (unlike script).