Skip to content

build silent payment index during validation #1027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
josibake wants to merge 1 commit into
base: master
Choose a base branch
from
Open

build silent payment index during validation #1027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions data/bn.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ filter_tx_buckets = <value>
filter_tx_rate = <value>
# The minimum allocation of the filter_tx table body, defaults to '1'.
filter_tx_size = <value>
# The number of buckets in the silent table head, defaults to '0' (0 disables).
silent_buckets = <value>
# The percentage expansion of the silent table body, defaults to '5'.
silent_rate = <value>
# The minimum allocation of the silent table body, defaults to '1'.
silent_size = <value>
# The first height indexed by the silent table, defaults to the bip341 activation height.
silent_start_height = <value>
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This belongs somewhere else, application logic. Since it's node-written prob just node. Also we use the term threshold for a minimum in a few places.

[node]
# The minimum height indexed for silent payments, defaults to the bip341 activation height.
silent_index_threshold = 42

Though we should consider relationship to address indexing. We don't currently provide a threshold for that. Since that's written directly off of tx archive, it would require store config. That might drive this back into store config as well.

Don't forget to add to server parse once merged here.

# The number of buckets in the header table head, defaults to '386364'.
header_buckets = <value>
# The percentage expansion of the header table body, defaults to '5'.
Expand Down
4 changes: 4 additions & 0 deletions include/bitcoin/node/chase.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ enum class chase
/// Issued by 'confirm' and handled by 'transaction'.
reorganized,

/// Silent payment prevout summaries indexed for confirmed block (header_t).
/// Issued by 'confirm' and handled by protocol subscribers.
silent_indexed,

/// Mining.
/// -----------------------------------------------------------------------

Expand Down
1 change: 1 addition & 0 deletions include/bitcoin/node/chasers/chaser_validate.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class BCN_API chaser_validate
const bool node_witness_;
const bool defer_;
const bool filter_;
const bool silent_;
};

} // namespace node
Expand Down
1 change: 1 addition & 0 deletions include/bitcoin/node/error.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ enum error_t : uint8_t
validate6,
validate7,
validate8,
validate9,
confirm1,
confirm2,
confirm3,
Expand Down
8 changes: 8 additions & 0 deletions src/chasers/chaser_confirm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,11 @@ bool chaser_confirm::set_organized(const header_link& link,
}
#endif // !NDEBUG

const auto silent = query.silent_enabled() &&
confirmed_height >= query.silent_start_height();
if (silent && !query.is_silent_indexed(link))
return false;

// Checkpointed blocks are set strong by archiver.
if (!query.push_confirmed(link, !is_under_checkpoint(confirmed_height)))
return false;
Expand All @@ -396,6 +401,9 @@ bool chaser_confirm::set_organized(const header_link& link,
fire(events::block_organized, confirmed_height);
LOGV("Block organized: " << confirmed_height);

if (silent)
notify(error::success, chase::silent_indexed, link);
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would drop this chaser event. The same can be accomplished by subscribing to block/tx and filtering on feature enabled, which cuts event traffic in half. This is the approach with address and filter as well, so the event count could start becoming a performance and complexity issue at some point.


announce(link, confirmed_height);
return true;
}
Expand Down
23 changes: 17 additions & 6 deletions src/chasers/chaser_validate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ chaser_validate::chaser_validate(full_node& node) NOEXCEPT
maximum_backlog_(node.node_settings().maximum_concurrency_()),
node_witness_(node.network_settings().witness_node()),
defer_(node.node_settings().defer_validation),
filter_(!defer_ && node.archive().filter_enabled())
filter_(!defer_ && node.archive().filter_enabled()),
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that defer_ is an diagnostic tool.

silent_(!defer_ && node.network_settings().witness_node() &&
node.node_settings().headers_first &&
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why headers first?

node.system_settings().forks.bip341 &&
node.archive().silent_enabled())
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot going on here. Prob best to just set this to node.archive().silent_enabled()) and deal with the rest further down.

{
}

Expand Down Expand Up @@ -160,13 +164,14 @@ void chaser_validate::do_bumped(height_t height) NOEXCEPT

const auto bypass = defer_ || is_under_checkpoint(height) ||
query.is_milestone(link);
const auto silent = silent_ && height >= query.silent_start_height();
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should make this a (efficient) query, similar to is_milestone(link). It can determine from the store if taproot is active at the block (from ctx.flags), if silent is active (based on the link's height, assuming store config), and if block is a witnessed block (from block sizes). Also should be moved into the case where it's used, since it won't be universal and has a cost.


switch (ec.value())
{
case database::error::unvalidated:
case database::error::unknown_state:
{
if (!bypass || filter_)
if (!bypass || filter_ || silent)
post_block(link, bypass);
else
complete_block(error::success, link, height, true);
Expand All @@ -175,7 +180,11 @@ void chaser_validate::do_bumped(height_t height) NOEXCEPT
case database::error::block_valid:
case database::error::block_confirmable:
{
complete_block(error::success, link, height, true);
if (silent && !query.is_silent_indexed(link))
post_block(link, true);
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed. The valid (or valid and confirmable) state implies this has already been done.

else
complete_block(error::success, link, height, true);

break;
}
case database::error::block_unconfirmable:
Expand Down Expand Up @@ -254,7 +263,7 @@ code chaser_validate::populate(bool bypass, const chain::block& block,

if (bypass)
{
// Populating for filters only (no validation metadata required).
// Populating optional indexes only (no validation metadata required).
block.populate(ctx);
if (!query.populate_without_metadata(block))
return system::error::missing_previous_output;
Expand Down Expand Up @@ -306,7 +315,10 @@ code chaser_validate::validate(bool bypass, const chain::block& block,
if (!query.set_filter_body(link, block))
return error::validate7;

// Valid must be set after set_prevouts and set_filter_body.
if (silent_ && !query.set_silent(link, block))
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can avoid the silent_ condition and just call the query, same as filter. The query can internally determine relevance using the same (efficient) checks noted above. Getting header metadata is almost as cheap as reading a local var. The heights are the most expensive, because that's a txs hashmap search, but still cheap.

return error::validate9;

// Valid must be set after optional block indexes.
if (!bypass && !query.set_block_valid(link))
return error::validate8;

Expand Down Expand Up @@ -334,7 +346,6 @@ void chaser_validate::complete_block(const code& ec, const header_link& link,
return;
}

// VALID BLOCK
// Under deferral there is no state change, but downloads will stall unless
// the window is closed out, so notify the check chaser of the increment.
notify(ec, chase::valid, possible_wide_cast<height_t>(height));
Expand Down
1 change: 1 addition & 0 deletions src/configuration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ configuration::configuration(system::chain::selection context) NOEXCEPT
network(context),
node(context)
{
database.silent_start_height = bitcoin.bip9_bit2_active_checkpoint.height();
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This belongs in the settings class that defines the property. The choice of value can be determined explicitly from the chain selection context above in that case, or any other value, such as the height at which the feature became usable or BIPped.

}

} // namespace node
Expand Down
1 change: 1 addition & 0 deletions src/error.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ DEFINE_ERROR_T_MESSAGE_MAP(error)
{ validate6, "validate6" },
{ validate7, "validate7" },
{ validate8, "validate8" },
{ validate9, "validate9" },
{ confirm1, "confirm1" },
{ confirm2, "confirm2" },
{ confirm3, "confirm3" },
Expand Down
8 changes: 8 additions & 0 deletions test/configuration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,12 @@ BOOST_AUTO_TEST_CASE(configuration__construct1__none_context__expected)
BOOST_REQUIRE_EQUAL(instance.bitcoin.first_version, 1_u32);
}

BOOST_AUTO_TEST_CASE(configuration__construct1__mainnet_silent_start__expected)
{
const node::configuration instance(chain::selection::mainnet);
const auto height = instance.bitcoin.bip9_bit2_active_checkpoint.height();

BOOST_REQUIRE_EQUAL(instance.database.silent_start_height, height);
}

BOOST_AUTO_TEST_SUITE_END()
2 changes: 1 addition & 1 deletion test/error.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(error_t__code__organize1__true_expected_message)
BOOST_REQUIRE_EQUAL(ec.message(), "organize1");
}

// TODO: validate2-validate6
// TODO: validate2-validate9
BOOST_AUTO_TEST_CASE(error_t__code__validate1__true_expected_message)
{
constexpr auto value = error::validate1;
Expand Down