Silent payment index (for light wallets and consistency check) #28241

Sjors · 2023-08-09T09:13:55Z

This PR adds an index with the silent payment tweak for every transaction. It builds on top of #28122. Supersedes #24897.

Usage: bitcoind -bip352index

The index does not have wallet dependencies, so it can be built with --disable-wallet.

It also adds a getsilentpaymentblockdata RPC that returns an array of silent payment tweaked public keys, one for each qualifying transaction.

This index serves two purposes:

Light client support, see BIP 352
A more thorough check than the test vectors; e.g. by comparing a checksum of the index for all of mainnet.

TODO:

check correctness against another implementation
(maybe) implement cut-through (-bip352ctindex)

DrahtBot · 2023-08-09T09:13:58Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30562 (PayToAnchor(P2A) followups by instagibbs)
#30546 (util: Use consteval checked format string in FatalErrorf by maflcko)
#30409 (Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals by Sjors)
#30364 (logging: Replace LogError and LogWarning with LogAlert by ryanofsky)
#30352 (policy: Add PayToAnchor(P2A), OP_1 <0x4e73> as a standard output script for spending by instagibbs)
#30051 (crypto, refactor: add new KeyPair class by josibake)
#29468 (rpc: method removeprunedfunds should take an array of txids by araujo88)
#29432 (Stratum v2 Template Provider (take 3) by Sjors)
#29295 (CKey: add Serialize and Unserialize by Sjors)
#28201 (Silent Payments: sending by josibake)
#26966 (index: blockfilter initial sync speedup, parallelize process by furszy)
#24539 (Add a "tx output spender" index by sstone)
#24230 (indexes: Stop using node internal types and locking cs_main, improve sync logic by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Sjors · 2023-08-09T09:15:51Z

I'd like to keep this based on #28122 but that requires some refactoring there. See #28122 (comment).

josibake · 2023-08-09T09:39:12Z

I'd like to keep this based on #28122 but that requires some refactoring there. See #28122 (comment).

you can also rebase on #27827 , which has everything, or just the receiving PR. Might be easier then to reason about which functions belong in which PRs if we can see it all together. I think the functions you need are introduced in the receiving PR (#28202)

Sjors · 2023-08-09T11:40:04Z

I'll take a stab at rebasing on the bigger thing. It makes sense to do that and then unrebase / debase(?) it later.

Sjors · 2023-08-09T14:57:33Z

I ended up "borrowing" some code from the followup PRs, without having to rebase on them.

This code compiles and the index doesn't crash. That's about the extend of it though :-) For now.

Sosthene00 · 2023-09-04T12:23:29Z

Hi, I have a little question about the motivation for making this index in Core. Afaiu, an implementation of electrum server (or any light client backend) could construct this index as easily, and I think it would be better separation of concerns to let them take responsibility for this instead of Core. Or maybe it's necessary for the consistency check you're mentioning? I'm not sure of what it is.

Sjors · 2023-09-04T14:39:54Z

@Sosthene00 I find it useful to have this index for developing and reviewing the silent payment implementation. But as you say, it may not be worth merging eventually. I'm not sure about that yet.

There's also the possibility of building something like this on top of the Kernel, when that project is a bit further along.

Sjors · 2023-09-19T14:49:43Z

Rebased to use GetSilentPaymentsTweakDataFromTxInputs. Otherwise still WIP.

Sjors · 2023-09-19T16:20:50Z

Pushed a simple RPC method to get the tweaked public key for a given transaction. I'd like to sanity check it against some other implementation.

Signet: dacb55db5c506a34771bf8ac3664e2510c17e80ada81347300fb4223f2d6c545

getsilentpaymenttxdata result: 03fc95cfeac43e9179c002d32a042626e5a2b289a8d7a4a0c88fbf81d570cef987

DrahtBot · 2024-07-28T19:59:19Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/27526127118}

Hints

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

DrahtBot · 2024-08-02T17:58:55Z

🐙 This pull request conflicts with the target branch and needs rebase.

thisIsNotTheFoxUrLookingFor · 2024-08-30T08:06:34Z

Hi, I have a little question about the motivation for making this index in Core. Afaiu, an implementation of electrum server (or any light client backend) could construct this index as easily, and I think it would be better separation of concerns to let them take responsibility for this instead of Core. Or maybe it's necessary for the consistency check you're mentioning? I'm not sure of what it is.

Personally I think Electrum is for SPV wallet things. Silent Payments would be considered part of a full node and not just an SPV thing. That is why I would think it makes it worthy of being in bitcoind.

src/common/bip352.h

src/index/bip352.h

src/common/bip352.h

cculianu · 2024-08-30T09:36:56Z

src/common/bip352.cpp

+    return std::nullopt;
+}
+
+bool MaybeSilentPayment(CTransactionRef &tx) {


Suggested change

bool MaybeSilentPayment(CTransactionRef &tx) {

bool MaybeSilentPayment(const CTransactionRef &tx) {

cculianu · 2024-08-30T09:37:31Z

src/index/bip352.cpp

+
+BIP352Index::~BIP352Index() = default;
+
+bool BIP352Index::GetSilentPaymentKeys(std::vector<CTransactionRef> txs, CBlockUndo& block_undo, tweak_index_entry& index_entry)


Suggested change

bool BIP352Index::GetSilentPaymentKeys(std::vector<CTransactionRef> txs, CBlockUndo& block_undo, tweak_index_entry& index_entry)

bool BIP352Index::GetSilentPaymentKeys(const std::vector<CTransactionRef>& txs, const CBlockUndo& block_undo, tweak_index_entry& index_entry) const

cculianu · 2024-08-30T09:38:43Z

src/index/bip352.cpp

+{
+    assert(txs.size() - 1 == block_undo.vtxundo.size());
+
+    for (uint32_t i=0; i < txs.size(); i++) {


Compare apples to apples and also use idiomatic pre-increment to avoid a temporary in unoptimized builds.

Suggested change

for (uint32_t i=0; i < txs.size(); i++) {

for (size_t i = 0; i < txs.size(); ++i) {

cculianu · 2024-08-30T09:39:42Z

src/index/bip352.cpp

+    assert(txs.size() - 1 == block_undo.vtxundo.size());
+
+    for (uint32_t i=0; i < txs.size(); i++) {
+        auto& tx = txs.at(i);


at unnecessary here (and in various places below).. since i is guaranteed to be in-range, since the loop is looping on txs.size().

cculianu · 2024-08-30T09:41:01Z

src/index/bip352.cpp

+        CTxUndo undoTX{block_undo.vtxundo.at(i - 1)};
+        std::map<COutPoint, Coin> coins;
+
+        for (uint32_t j = 0; j < tx->vin.size(); j++) {


Apples to apples, and also use pre-increment.

Suggested change

for (uint32_t j = 0; j < tx->vin.size(); j++) {

for (size_t j = 0; j < tx->vin.size(); ++j) {

cculianu · 2024-08-30T09:42:52Z

src/index/bip352.cpp

+                const CCoinsViewCache& coins_cache = m_chainstate->CoinsTip();
+
+                uint32_t spent{0};
+                for (uint32_t i{0}; i < tx->vout.size(); i++) {


This loop index shadows an outer variable i. Also should be using size_t here.

I'll fix this in Sjors#86

Sjors · 2024-08-30T10:01:32Z

@cculianu thanks for reviewing the code. However many of your comments refer to #28122 (which this pull request builds on top of), so it's better comment there. Paging @josibake to take a look though.

I'll look at the comments for src/index when I next rebase.

cculianu · 2024-08-30T10:11:33Z

@cculianu thanks for reviewing the code. However many of your comments refer to #28122 (which this pull request builds on top of), so it's better comment there. Paging @josibake to take a look though.

I'll look at the comments for src/index when I next rebase.

Ah ok I can review/comment there too. Thanks for the tip.

DrahtBot · 2024-11-27T01:13:05Z

⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

fanquake · 2025-02-20T18:57:56Z

Closing this for now, given after a year and a half it's still a draft, based on a draft, based on another PR that is still being worked on. Maybe a new PR can be opened in future.

Sjors · 2025-04-23T12:22:53Z

Opened a PR on my own fork for this: Sjors#86

It's a new branch based on this one, rebased on the latest #28122, and fixes conflicts with #28358, #31483, #31490 and the introduction of CMake.

I also applied most of @cculianu's suggestions there (not pushed yet).

Sjors mentioned this pull request Aug 9, 2023

Silent Payments: Implement BIP352 #28122

Draft

DrahtBot added the CI failed label Aug 9, 2023

Sjors force-pushed the 2023/08/silent-index branch from c23e4e7 to d589099 Compare August 9, 2023 12:40

Sjors force-pushed the 2023/08/silent-index branch from d589099 to 92897d6 Compare August 9, 2023 14:57

Sjors force-pushed the 2023/08/silent-index branch from 92897d6 to ea825ae Compare August 29, 2023 08:57

DrahtBot mentioned this pull request Sep 7, 2023

Enhanced error messages for invalid network prefix during address parsing. #27260

Open

DrahtBot added the Needs rebase label Sep 7, 2023

Sjors force-pushed the 2023/08/silent-index branch from ea825ae to 4a7dccb Compare September 19, 2023 14:37

DrahtBot removed the Needs rebase label Sep 19, 2023

This was referenced Jul 17, 2024

index: initial sync speedup, parallelize process #26966

Open

Stratum v2 Template Provider common functionality #30475

Closed

Sjors mentioned this pull request Jul 23, 2024

Allow ext keys josibake/bitcoin#3

Merged

DrahtBot added the CI failed label Jul 28, 2024

This was referenced Jul 31, 2024

util: Use consteval checked format string in FatalErrorf, LogConnectFailure #30546

Merged

PayToAnchor(P2A) followups #30562

Merged

DrahtBot added the Needs rebase label Aug 2, 2024

cculianu mentioned this pull request Aug 30, 2024

Silent Payment scanning [suggestion] cculianu/Fulcrum#257

Open

cculianu reviewed Aug 30, 2024

View reviewed changes

src/common/bip352.h Show resolved Hide resolved

cculianu reviewed Aug 30, 2024

View reviewed changes

src/index/bip352.h Show resolved Hide resolved

cculianu reviewed Aug 30, 2024

View reviewed changes

src/common/bip352.h Show resolved Hide resolved

cculianu reviewed Aug 30, 2024

View reviewed changes

fanquake closed this Feb 20, 2025

Sjors mentioned this pull request Apr 23, 2025

BIP352 index (for light wallets and consistency check) Sjors/bitcoin#86

Open

1 task

	bool MaybeSilentPayment(CTransactionRef &tx) {
	bool MaybeSilentPayment(const CTransactionRef &tx) {


		BIP352Index::~BIP352Index() = default;

		bool BIP352Index::GetSilentPaymentKeys(std::vector<CTransactionRef> txs, CBlockUndo& block_undo, tweak_index_entry& index_entry)

	bool BIP352Index::GetSilentPaymentKeys(std::vector<CTransactionRef> txs, CBlockUndo& block_undo, tweak_index_entry& index_entry)
	bool BIP352Index::GetSilentPaymentKeys(const std::vector<CTransactionRef>& txs, const CBlockUndo& block_undo, tweak_index_entry& index_entry) const

	for (uint32_t i=0; i < txs.size(); i++) {
	for (size_t i = 0; i < txs.size(); ++i) {

	for (uint32_t j = 0; j < tx->vin.size(); j++) {
	for (size_t j = 0; j < tx->vin.size(); ++j) {

Silent payment index (for light wallets and consistency check) #28241

Silent payment index (for light wallets and consistency check) #28241

Uh oh!

Conversation

Sjors commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

Sjors commented Aug 9, 2023

Uh oh!

josibake commented Aug 9, 2023

Uh oh!

Sjors commented Aug 9, 2023

Uh oh!

Sjors commented Aug 9, 2023

Uh oh!

Sosthene00 commented Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Sep 19, 2023

Uh oh!

Sjors commented Sep 19, 2023

Uh oh!

DrahtBot commented Jul 28, 2024

Uh oh!

DrahtBot commented Aug 2, 2024

Uh oh!

thisIsNotTheFoxUrLookingFor commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cculianu Aug 30, 2024

Choose a reason for hiding this comment

Uh oh!

cculianu Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cculianu Aug 30, 2024

Choose a reason for hiding this comment

Uh oh!

cculianu Aug 30, 2024

Choose a reason for hiding this comment

Uh oh!

cculianu Aug 30, 2024

Choose a reason for hiding this comment

Uh oh!

cculianu Aug 30, 2024

Choose a reason for hiding this comment

Uh oh!

Sjors Apr 23, 2025

Choose a reason for hiding this comment

Uh oh!

Sjors commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cculianu commented Aug 30, 2024

Uh oh!

DrahtBot commented Nov 27, 2024

Uh oh!

fanquake commented Feb 20, 2025

Uh oh!

Sjors commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Sjors commented Aug 9, 2023 •

edited

Loading

DrahtBot commented Aug 9, 2023 •

edited

Loading

Sosthene00 commented Sep 4, 2023 •

edited

Loading

Sjors commented Sep 4, 2023 •

edited

Loading

thisIsNotTheFoxUrLookingFor commented Aug 30, 2024 •

edited

Loading

cculianu Aug 30, 2024 •

edited

Loading

Sjors commented Aug 30, 2024 •

edited

Loading

Sjors commented Apr 23, 2025 •

edited

Loading