[POC/WIP] Add optional Bip352 silentpayments index#1075
Open
jlest01 wants to merge 4 commits intoromanz:masterfrom
Open
[POC/WIP] Add optional Bip352 silentpayments index#1075jlest01 wants to merge 4 commits intoromanz:masterfrom
jlest01 wants to merge 4 commits intoromanz:masterfrom
Conversation
jlest01
force-pushed
the
bip352-silentpayments-module
branch
from
6df1529 to
c1e110c
Compare
but I can make silent payments to the node, from a client? |
antonilol
reviewed
Aug 22, 2025
Comment on lines
+442
to
+447
| if self | ||
| .index | ||
| .store | ||
| .iter_spending(SpendingPrefixRow::scan_prefix(outpoint)) | ||
| .next() | ||
| .is_none() |
Contributor
There was a problem hiding this comment.
If I understand correctly, this must make sure only transactions with (P2TR) unspent outputs are scanned.
Making an index dependent on data from another index seems odd to me, I have some questions about this:
- Does this handle collisions of
spendingentries in the database (correctly)? - Wouldn't a user want historic data after restoring a wallet?
- Looking at the code,
spendingis made sure to be updated to the current chain tip and thentweaksis indexed (do I get this correctly?). This will include spent transactions (imagine 1 P2TR output TXs) as blocks get propagated, but not on the initial sync or when spent in the same block. Is this intended?
antonilol
reviewed
Aug 22, 2025
Comment on lines
+538
to
+541
| let height = index.chain.get_block_height(&hash).expect("Unexpected non existing blockhash"); | ||
| let mut value: Vec<u8> = u64::try_from(height).expect("Unexpected invalid usize").to_be_bytes().to_vec(); | ||
| value.extend(tweaks.iter()); | ||
| batch.tweak_rows.push(value); |
Contributor
There was a problem hiding this comment.
If I understand correctly, the key of the stored key-value pair is the block height. Is RocksDB the right tool for the job then? I think this can easily be achieved using two files that only need appending when building/updating the index: one for the tweaks, and one for the offsets and lengths in the tweaks file, for every block (for random access).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR proposes a new
--silent-payments-indexoption that creates an index of Bip352 silent payments.Most of the code is based on @Sosthene00's fork, but I made the following changes:
. It uses bitcoin-core/secp256k1#1519 and rust-bitcoin/rust-secp256k1#721 instead of the
rust-silentpaymentscrate.. It adds a new
--silent-payments-indexconfiguration option. Without this option, the server does not index silent payments and works the same as it does today.. The code does not use
Box[u8]indb.rs.To run this, something like the following command can be used:
cargo run -- --log-filters=INFO --db-dir <db_dir> --daemon-dir ~/.bitcoin/ --network signet --electrum-rpc-addr="127.0.0.1:60001" --silent-payments-index