add silent payments prevout summaries table#804
Conversation
return a terminal height when a prevout has no confirmed spender, instead of zero. previously, unspent outputs could get a height of zero which implies they were spent by the genesis block. more specifically, outputs that are unspent because the spending transaction was not confirmed would get marked as already spent by the genesis block when attempting to broadcast the transaction.
define the schema for the silent payments prevout summaries index. this naming was chosen to match the naming in the libsecp module PR. the name is meant to accurately represent what is stored in the table, but i am open to changing it if others feel strongly. the table is heavily inspired by the compact filter table as these two tables are conceptually similar.
add optional-table settings and store lifecycle plumbing for the silent payment prevout summary table. the uncompressed setting is stored beside the table configuration because the row format is selected when the index is built and must be consistent when rows are read. the reason for adding this is a compressed table is half the storage, but there is a query time cost of doing the decompression. if a user has ample storage and wishes to optimise for speed, they can store decompressionsed prevout summaries for a roughly ~11% speedup during scanning.
add query methods that build and read per-block silent payment prevout summaries. the block path mirrors compact filter construction: it uses populated prevouts, writes the optional table by block link, and indexes genesis during store initialization when the table is enabled. the query path rejects mismatched compressed/uncompressed table formats so configuration changes require an index rebuild instead of silently reading incompatible rows.
extend validated fork selection with an optional silent-payment-index constraint. when enabled, confirmation will not advance into taproot-active candidate blocks until their prevout summaries have been written. this keeps the sp index in the same confirmation workflow as compact filters and avoids publishing confirmed blocks whose scan data is not ready.
| header_states CLASS::get_validated_fork(size_t& fork_point, | ||
| size_t top_checkpoint, bool filter) const NOEXCEPT | ||
| size_t top_checkpoint, bool filter, bool sp_prevout_summaries, | ||
| size_t sp_prevout_summaries_activation) const NOEXCEPT |
There was a problem hiding this comment.
IIUC these parameters are configuration, so shouldn't be passed here. This applies to filter as well, its retention and passage by node::chaser_confirm is redundant/unnecessary. I've removed it from node/database in upcoming PRs.
| while (is_block_validated(ec, link, height, top_checkpoint) && | ||
| (!filter || is_filtered_body(link))) | ||
| (!filter || is_filtered_body(link)) && | ||
| (!sp_prevout_summaries || height < sp_prevout_summaries_activation || |
There was a problem hiding this comment.
The link-based state of activation should be reduced to a config-driven method, similar to those for the other optional tables (address/filters).
| size_t out{}; | ||
| if (const auto tx = to_input_tx(in); get_tx_height(out, tx)) | ||
| break; | ||
| return { system::possible_narrow_cast<uint32_t>(out) }; |
There was a problem hiding this comment.
This (original code) cast is incorrect, should be:
system::possible_narrow_cast<height_link::integer>(out)
I've fixed this in an independent PR.
There was a problem hiding this comment.
Also, yes, needs default return for terminal value. Prefer to keep out declaration outside the loop to reduce the clutter.
There was a problem hiding this comment.
There is also one small fix unrelated to the silent payments table...
Yes, please split out.
There was a problem hiding this comment.
This table name is way too long, it's crappin up stuff everywhere. This isn't lower case Java. ;)
Change to silent.hpp.
| { | ||
| auto copy = summaries; | ||
| copy.format = sp_prevout_summaries::uncompressed; | ||
| for (auto& record: copy.records) |
There was a problem hiding this comment.
Ideally this should be implemented where the records are created, or by the caller, not in the table writer. The values should really just be stored everywhere as compressed. Def don't want to initially overallocate and then copy the full set.
There was a problem hiding this comment.
Requires artifact regeneration.
| "3e9fce73d4e77a4809908e3c3a2e54ee147b9312dc5044a193d1fc85de46e3c1" | ||
| }; | ||
|
|
||
| static data_chunk chunk(const std::string_view& text) |
There was a problem hiding this comment.
I would discard all of these small helpers. The object constructions that the chain utils are facilitating just increase complexity, as the chain objects already have the necessary constructors.
| } | ||
|
|
||
| template <size_t Size> | ||
| static data_array<Size> array(const std::string_view& text) |
There was a problem hiding this comment.
Use these system utils for test literals:
// Literal decodings of hex string, errors reflected as zero-filled data.
// ----------------------------------------------------------------------------
template <size_t Size, if_odd<Size>>
constexpr std::string base16_string(const char(&string)[Size]) NOEXCEPT
{
return to_string(base16_chunk(string));
}
template <size_t Size, if_odd<Size>>
data_chunk base16_chunk(const char(&string)[Size]) NOEXCEPT
{
data_chunk out{};
decode_base16(out, string);
return out;
}
template <size_t Size, if_odd<Size>>
constexpr data_array<to_half(sub1(Size))>
base16_array(const char(&string)[Size]) NOEXCEPT
{
data_array<to_half(sub1(Size))> out{};
if (!decode_base16(out, string))
out.fill(0);
return out;
}
template <size_t Size, if_odd<Size>>
constexpr data_array<to_half(sub1(Size))>
base16_hash(const char(&string)[Size]) NOEXCEPT
{
data_array<to_half(sub1(Size))> out{};
if (!decode_hash(out, string))
out.fill(0);
return out;
}| return out; | ||
| } | ||
|
|
||
| static script raw_script(const std::string_view& text) |
There was a problem hiding this comment.
Try to avoid overuse of the term "raw", which can be ambiguous and overloaded. If you mean a byte buffer, then we typically use chunk or data by convention if it's clear from context (e.g. to_data()/from_data()).
| 0u | ||
| }; | ||
|
|
||
| const auto header = to_shared<chain::header>( |
There was a problem hiding this comment.
const auto header = to_shared(chain::header
{
...
});or
const auto header = emplace_shared<chain::header>(a, b, c, d, e, f);These are just utils to reduce code bloat and to suppress the exception warnings that come from the implied allocation.
2 participants
This adds a table for silent payments prevout summary objects. I'm using the terminology from the libsecp module as I feel its the best description for what this data is.
I mostly copied the design from the compact filters table as they seemed conceptually similar.
One thing that may stand out to reviewers is the extra knob for controlling the prevout summary object size, i.e., compressed vs uncompressed. Turning a compressed object into a full EC point requires a square root calculation for getting the y-value. When the prevout summaries are stored as compressed key, this sqrt cost is paid for every scan.
To avoid this, the prevout summaries can be stored as uncompressed keys. This gets you a roughly 11% speed up at query time for 2x the storage. I left the knob in because some users may have plenty of storage and prefer the faster scan. I also strongly suspect this query time speed up will be much higher if we use the batch affine optimisations in ultrafastsecp.
I've tried to add good tests where reasonable, but might add more/move some to other layers if it makes sense.
There is also one small fix unrelated to the silent payments table, which is needed for electrum protocol PRs in node/server. Its small, so I rolled it in, but can also open it as a separate PR.