Silent Payments: Implement BIP352 (wallet)

Host: josibake  -  PR author: josibake


This PR is the first step towards implementing BIP352: Silent Payments in the Bitcoin Core wallet. BIP352 is “a protocol for static payment addresses in Bitcoin without on-chain linkability of payments or a need for on-chain notifications.”

Before reviewing the PR, it is strongly recommended that you read the BIP, as well as the reference implementation and test vectors. This PR implements the logic from the BIP and is a child PR of Silent Payments: send and receive.

  • CPubKey and CKey are Bitcoin Core’s classes for public and private keys. These classes handle serializing and deserializing key data and encapsulate libsecp256k1 cryptographic operations. This PR adds new methods to these classes to support the cryptographic operations needed for BIP352.

  • CTxDestination is a std::variant class used to represent a transaction output (or “destination”). This variant represents standard scriptPubKey templates and handles encoding and decoding outputs via the DestinationEncoder class and DecodeDestination function. BIP352 introduces a new output type which does not represent a single scriptPubKey, but instructions for generating a scriptPubKey using ECDH. This PR introduces new methods for encoding and decoding a silent payment address.

  • BIP352 has a sending section and a receiving section. This allows wallets to implement either half of the protocol without needing to implement the other. The logic for sending is implemented in the Sender class, with the bulk of the logic in the GenerateRecipientScriptPubKeys() method. Receiving is implented in the Recipient class, with the bulk of the logic in the ScanTxOutputs method. This PR only implements the BIP352 primitives and leaves the wallet implementation for a follow-up PR. As such, the interface for these classes is abstracted away from transactions and descriptors and instead works directly with CPubKeys and CKeys.


  1. Did you read the BIP? In a few sentences, can you summarize it in your own words?

  2. Did you review the reference implementation and test vectors? Can you think of additional test cases?

  3. Did you review the PR? Concept ACK, approach ACK, tested ACK, or NACK? What was your review approach?

  4. In 5688262, why do we need a custom hash function? Why not just use the default hash function from libsecp256k1? (hint: see this discussion on the BIP)

  5. ae6019b and 439e57a add functions for decoding and encoding silent payment addresses. Why can’t we just add silent payment addresses as a new CTxDestination variant and use the existing encoder class and decoder function?

  6. BIP352 talks about about versioning and forward compatibility. Why is this important? Do you see any issues with the way the PR is written with respect to forwards compatibility (hint: look at the decoding logic in ae6019b)

  7. In DecodeSilentAddress there is a check on the version and data size. In your own words, can you explain what this check is doing? Why is it important?

  8. The new Silent Payments code is under the wallet directory in src/wallet/silentpayments.cpp. Is this a good place for the code to live? Why or why not? Can you think of use case where it would be better to have the code outside the wallet?

  9. Recipient is a class which is initialized with two private keys, the scan key and the spend key. Are both of these keys necessary for scanning? Can you propose a better implementation? Which functions would you need to re-write? (hint: take a look at Recipient::ScanTxOutputs

  10. Based on your answer above, what’s one benefit for how the Recipient class is currently written?

  11. In a few sentences, can you summarize what Recipient::ScanTxOutputs is doing?

  12. In Sender::CreateOutput, we hash the ECDH shared secret with an integer. Why do we need the integer counter?

  13. Bitcoin Core has a HashWriter class, which we use. Can you describe what this class does? For our usecase, we need to hash a public key (the shared secret) and an integer. The HashWriter provides a template function for hashing two objects: why not use this? Even better, why not hash the pubkey with the << operator? (hint: look at how a CPubKey is serialized)

  14. For the unit tests, we define an equality comparator as a “friendly” function. What is a “friendly” function? Does it make sense to use one here?

Meeting Log

Meeting 1

  117:00 <josie> #startmeeting
  217:00 <stickies-v> hi
  317:00 <josie> hi!
  417:00 <Pins> hi
  517:00 <Murch> Hi
  617:00 <BrandonOdiwuor> Hi
  717:00 <abubakarsadiq> hello
  817:00 <emjshrx> hey
  917:00 <glozow> hi
 1017:00 <larryruane> hi
 1117:01 <josie> we are reviewing "Silent Payments: Implement BIP352" today, with the notes here:
 1217:01 <josie> any first timers here?
 1317:01 <emjshrx> me
 1417:01 <josie> emjshrx: welcome!
 1517:01 <glozow> it's my first time at monthly pr review club :3
 1617:01 <josie> haha was about to say, for old-timers and newcomers alike, this is our first PR review club in the new format!
 1717:02 <josie> as a reminder, we are meeting today for an hour and then meeting again tomorrow at the same time to discuss the same PR
 1817:02 <Pins> Nice
 1917:03 <abubakarsadiq> \o/
 2017:03 <josie> lets get started! did you get a chance to read the BIP? if so, can you summarize it in a sentence or two, or mention a part that stood out to you?
 2117:04 <Murch> Silent Payments describes a static address scheme where the transaction itself provides sufficient information for the recipient to recognize that they got paid without revealing to any uninvolved parties that a silent payment has occurred. This is achieved by creating a unique recipient output script from the recipient’s static address and the public keys used in the spending inputs.
 2217:04 <BrandonOdiwuor> BIP 352 Provides a solution for making private payments using static addresses eliminating the need for interaction or on-chain overheads i.e notifications while preserving the privacy of both the sender and the receiver
 2317:05 <josie> Murch, BrandonOdiwour: nice summaries :)
 2417:05 <Pins> And no interaction between the participants is needed
 2517:06 <Murch> And it’s basically impossible to reuse addresses :)
 2617:06 <larryruane> I loved how the BIP adds complexity in stages, first simple, then creating > 1 output, etc. (I got a bit lost on the scan versus spend, and the labels)
 2717:06 <Murch> Well, unless someone just sends to a prior address
 2817:07 <emjshrx> larryruane : agree
 2917:07 <Murch> Yeah, the BIP is very well written. Easy to digest
 3017:07 <sosthene> Hi, just wanted to mention that the BIP made a lot of progress as far as I'm concern, and that the way labels work today lifted the most serious worry I had about the first iteration
 3117:07 <josie> Much: yep! so long as the sender is following the protocol, there is no way to accidentally reuse an address
 3217:08 <Murch> By the way, is it possible that the wallet would not even realize that it got paid another time to a prior output script?
 3317:08 <stickies-v> larryruane: the scan vs send means (I think) that, similar to the xpub setup, you want to delegate scanning for received payments to a hot machine that only has your spending pubkey and your scanning private key, which is enough to recognize payments, but not to spend them (for which you need the spending privkey too)
 3417:08 <sosthene> I think that to reuse an address you would need to make try very hard, almost impossible indeed
 3517:09 <Murch> Does the wallet keep track of the output scripts after it has found an output corresponding to one? Otherwise, it would not match the protocol, and the search process would not find it
 3617:09 <larryruane> stickies-v: thanks, yes, I understood the motivation, just not how it's implemented :)
 3717:09 <josie> sosthene: thanks for joining! the BIP has indeed changed quite a bit since the original proposal from march 2022
 3817:09 <sosthene> I think if an address got reused a sp wallet will just miss it
 3917:09 <Murch> sosthene: Yeah, that’s my suspicion as well. josie have you considered that?
 4017:09 <sosthene> except if you have some kind of hybrid wallets that's doing sp on top of a most classic wallet
 4117:10 <Murch> That would certainly make it easy to avoid forced address reuse. You simply never notice ^^
 4217:10 <josie> stickies-v, larry: that's correct regarding spend and scan key. as an extra benefit, the spend and scan key also makes using labels easier
 4317:11 <josie> for those that read the BIP, did you also look at the python reference implementation and test vectors? Can you think of any additional test cases?
 4417:11 <larryruane> yes I love the python code, I've been running it in the debugger, great way to look around and see what's going on
 4517:11 <josie> Murch: not sure I understand your question regarding tracking an output script after it found it. Can you give an example of what you mean?
 4617:11 <stickies-v> it seems like the test vectors only test happy paths, are there any unhappy paths/exceptions we need to test for too? (maybe i missed it)
 4717:12 <dergoegge> hi
 4817:13 <sosthene> it means you received some funds on a regular sp scriptpubkey, and the sender keep it and send to the same script pubkey again in another transaction
 4917:13 <emjshrx> stickies-v : Is it that in the BIP we only need happy path testing, wheras in the PR itself you can add more coverage?
 5017:14 <josie> sosthene, Murch: since we are using a sha256 to ensure a unique address is created, address reuse will not happen if the sender is following the protocol. of course, nothing stops a sender from maliciously sending to an output that they sent to previously, but this is true of bitcoin in general
 5117:14 <ottosch> I guess I'd rather have an address reused then miss a payment
 5217:14 <sosthene> afaiu silent payment would miss it since it won't match at scanning time
 5317:15 <abubakarsadiq> Silent payments enable using single address to receive multiple payments
 5417:15 <brunoerg> hi
 5517:16 <emjshrx> sosthene: are you referring to the indexes? They are for labels if i understood correctly
 5617:16 <josie> sosthene, ottosch: the receiving wallet would find the initial payment when scanning. continuing to monitor that output script for additional payments can be done with any other wallet protocol, because at that point the scriptPubKey is known
 5717:16 <josie> stickies-v: great observation! there are a few unhappy paths I can think of that are not currently covered in the tests
 5817:17 <larryruane> A basic question, `P = B + hash(a·B)·G` ... that addition, does this depend on Schnorr signatures being used? I'm unclear about how Bob creates the unlock script to spend a SP later (since the pubkey is this weird thing)
 5917:17 <sosthene> emjshrx I'm talking about labels :) I'm not sure about what you call indexes
 6017:19 <sosthene> larryruane that's just adding points on the curve, `a*B` is your ECDH, hashing it gives you something you can use as a Scalar, multiplying by G gives you another point on the curve, nothing to do with Schnorr
 6117:20 <Murch> @josie: sorry, got called away for a moment. sosthene already explained, but I meant that tx_A sent a regular SP to Bob, and then Mallory sends to the same output script as the SP to Bob went to. Would Bob’s wallet even notice that it got paid again to a prior SP output script, when in this second transaction the output script is not a valid SP
 6217:20 <josie> larryruane: addition here refers to addition of points on the elliptic curve, known as the elliptic curve group operation
 6317:21 <larryruane> sosthene: josie: thanks .. I need to study more, i don't understand how Bob (the receipient) later spends these silent payments, but that's okay
 6417:22 <larryruane> one more minor comment on the python (, really cool how all the typing is done, that really helps the readability!
 6517:22 <Murch> @ottosch: If it’s the same sender, they should just make a new SP if they want to pay me again. If it’s someone sending a tiny amount to force address reuse, I’d rather not even see it so I don’t have to think about it
 6617:22 <josie> Murch: if the question is "can a silent payments wallet detect payments not sent via the silent payments protocol" I think the correct answer is "it depends" :)
 6717:23 <Murch> Right, let’s not get hung up on this
 6817:23 <ottosch> larryruane, a.B = b.A; Bob will multiply his private key by Alice's pubkey. She did the opposite
 6917:23 <josie> larryruane: agree! mypy is great for improving the readibility of the code. unfortunately, that's the only thing its good at ;)
 7017:25 <josie> going to move on to the next question, but feel free to keep discussing/asking questions about the BIP if we aren't ready to move on
 7117:25 <stickies-v> larryruane: bob would just do a keypath spend, and know how to construct the private key that's needed for the (schnorr, because we're in taproot world, but could be ECDSA in theory) signature based on his (scan, spend) private key as well as the public key of the input used in the tx
 7217:25 <josie> did you get a chance to review the PR for Bitcoin Core? Concept ACK, NACK, etc? what was your review approach?
 7317:26 <Murch> larryruane: The second part `hash(a·B)·G` is derived from the shared secret of Alice and Bob: Alice’s private key and Bob’s public key multiplied result in the same value as Bob’s private key multiplied with Alice’s public key. This is just Diffie-Hellman (a·B = b·A)
 7417:26 <josie> for context, this PR implements only the logic from the BIP. it doesn't actually implement sending and receiving in the wallet
 7517:27 <Murch> Then Alice adds Bob’s public key to it the key. This means that only Bob will be able to spend it, because he needs the corresponding private key to sign
 7617:27 <josie> stickies-v, larryruane: that's correct! basically, the final private key ends up being Bob's spend secret key + the shared secret from Alice doing ECDH with Bob's scan key
 7717:28 <Murch> josie: I just skimmed it. Huge concept ACK on Silent Payments, but don’t have an opinion on the implementation, yet. Hope to stare at it more in the coming ten days or so
 7817:29 <stickies-v> yeah concept ACK on SP too, and approach-wise i like that you're implementing this first bit without the wallet, that seems to make sense at first sight
 7917:29 <abubakarsadiq> Concept Ack
 8017:29 <emjshrx> concept ACK. just curious as to why we had to modify key_io could we bring that logic closer to silentPayments.cpp
 8117:30 <sosthene> I didn't reviewed it in details, I just built it and ran the tests. However I would like to ask why chose to keep the send/receive for another PR?
 8217:30 <BrandonOdiwuor> @josie: Tested ACK, not done manual testing
 8317:30 <josie> emjshrx: great question! I'm curious what everyone here thinks? does implementing stuff in key_io.cpp make sense? Why or why not?
 8417:31 <josie> stickies-v: yeah, breaking it up this way made it much easier to unit test
 8517:32 <emjshrx> josie: my reasoning was just behind reuse of this logic. This logic seems to be only needed by SP, so keeping it closer made sense.
 8617:33 <ottosch> A question: I see the original e-mail checked only the UTXO set to reduce scanning. Will transaction history be lost?
 8717:33 <josie> sosthene: great question! my main motivation was to break the original PR into smaller chunks to make it easier to review. Having a PR just for the BIP352 logic (independent of send and receive) made unit testing much easier
 8817:33 <brunoerg> Concept ACK, started reviewing the code, left some comments there
 8917:34 <josie> sosthene: also, sending and receiving can be implemented separately. In other words, a wallet can send without needing to be able to receive. It seemed logical to then open the PRs as separate since the code allows it
 9017:34 <stickies-v> ottosch: if wallets want to implement scanning the full blockchain for transaction history, i don't think anything's stopping them, scanning the UTXO set is just the minimum requirement (and probably more than enough for most use cases?)
 9117:35 <Murch> ottosch: If you only care to find spendable UTXOs, you can restrict your scanning to transactions with unspent P2TR outputs since you last scanned. If you want the full history, you have to scan all transactions with P2TR outputs since your wallet birthdate
 9217:36 <josie> emjshrx: yeah, its a good point. definitely in favor of keeping the new code contained to just silent payments, where it makes sense. for some of the cryptographic operations, though, I think it makes sense to implement on the CKey and CPubKey objects only because we don't want to pull all that cryptography code into the wallet
 9317:36 <BrandonOdiwuor> ottosch scanning the UTXO set will only show the unspent transactions, but you will loose history of coins already spent
 9417:37 <ottosch> the actual question is how it's being or will be implemented
 9517:37 <emjshrx> josie: thanks! It makes sense now
 9617:37 <larryruane> is this the first time ECDH (or any kind of DH) is used in bitcoin, that we know of?
 9717:38 <josie> ottosch, BrandonOdiwour: scanning the UTXO set is an optimization for mobile clients to limit the data they need to download. transaction history can always be recovered by scanning the full chain
 9817:38 <ottosch> larryruane: bip47 uses it and bip351 (not sure the latter was implemented)
 9917:39 <Murch> ottosch: There is no reason why not both can be implemented
10017:39 <ottosch> right
10117:39 <sosthene> ottosch last time I checked there was a reference implementation of BIP 351 in rust
10217:39 <josie> ottosch, larryruane: correct! and also "stealth addresses", the original static address proposal for bitcoin, used ECDH. unrelated to wallets, BIP324 uses ECDH, but a modified XOnlyECDH iirc
10317:40 <larryruane> josie: ottosch: +1 thanks
10417:42 <josie> great segue to the next question! for ECDH, why does the PR define a new custom hash function? why not use the default hash function from libsecp256k1?
10517:42 <josie> this is this the commit for the custom hash function ->
10617:42 <BrandonOdiwuor> This is due to the need for un-hashed ECDH result, the ‘custom_ecdh_hash’ function returns the serialized public key without hashing it
10717:45 <josie> BrandonOdiwuor: yep! can you think of a reason why we would want the result to stay unhashed?
10817:45 <stickies-v> which confused me. it's called a hashing function, but the one thing it doesn't seem to do is hashing?
10917:45 <abubakarsadiq> I don't understand why we need it unhashed
11017:45 <Murch> josie: Why is it called “custom hash function”?
11117:45 <stickies-v> (also, "custom" tends to go stale _really_ quickly, what if we need other bespoke behaviour too? custom2?)
11217:46 <Murch> 👍️ What stickies-v said
11317:46 <emjshrx> josie: is it so that we can check for labels later on?
11417:46 <josie> Murch, stickies-v: because I'm bad at naming things :P jokes aside, I was trying to match the language in libsecp, where they say you can use the "default" hash function or pass a "custom" hash function
11517:46 <abubakarsadiq> Why is it living in wallet Instead of secp256k1
11617:46 <josie> abubakarsadiq: great observation! ideally, this will be moved to libsecp at some point
11717:47 <sosthene> I think we don't hash it because we want to add the n counter before hashing
11817:47 <glozow> maybe `not_hash_function`
11917:47 <josie> glozow: haha nice
12017:48 <josie> emjshrx: not quite! labels can work with or without it being hashed in that step
12117:48 <sosthene> but frankly when I noticed it I thought we could just hash it anyway, add the counter and hash it again, so that we don't need the `this_is_not_a_hash_function` trick
12217:48 <josie> sosthene: actually in the original version of the BIP, we hashed during ECDH and then again with the counter
12317:49 <josie> here's a hint as to why we don't hash during the ECDH step:
12417:50 <josie> (click on "show resolved" for the actual discussion)
12517:52 <Murch> It’s not obvious to me why the hash inside of ECDH is a problem for MuSig2 users
12617:54 <josie> Murch: less specific to Musig2, but more generally any time the creator of the transaction does not control all the inputs. By not hashing the result during ECDH, this allows individual participants to do ECDH with just their private key, and then pass the partial ECDH along. the partial ECDH results can then be summed up , and the rest of the protocol performed (hashing with the
12717:55 <josie> counter, etc)
12817:55 <Murch> Aah, right
12917:55 <Murch> Otherwise people would need to exchange the aggregate the private keys in order to calculate the hash
13017:55 <josie> Murch: exactly!
13117:56 <Murch> which would very likely leak the private keys to the other participants (unless there is a really nifty way of aggregating them without revealing them or smth, but just not hashing is way easier)
13217:56 <josie> so we've got 4 minutes left, so I think I'll wait til tomorrow to dive into the next question, because it's a fairly big one. As a reminder, we are meeting tomorrow at the same time and are going to jump right in with question 5
13317:57 <josie> with the remaining 4 mins, any questions or comments about what we've discussed so far?
13417:57 <emjshrx> wouldnt it be easier to just use complex scripts on the Spend key instead of Scan key
13517:58 <josie> emjshrx: can you give an example of what you mean by "complex scripts" ?
13617:58 <emjshrx> musig2
13717:58 <Murch> emjshrx: The problem is not the receiver, but the sender
13817:59 <emjshrx> oh okay. I need to go through the PR discussion again
13917:59 <Murch> The problem occurs in the calculation of the shared secreet
14018:00 <josie> emjshrx: the spend and scan key is so that the receiver can always keep one key in cold storage. in fact, the spend key itself can be an aggregate key (musig2, FROST)
14118:00 <josie> regarding the hashing and ECDH, this is on the sender side
14218:00 <Murch> which is more difficult for the sender, because they need to use all private keys from the relevant inputs
14318:00 <josie> cool, we'll stop here and be back tomorrow! thanks for attending and really hope to see you all tomorrow!
14418:00 <josie> #endmeeting

Meeting 2

14517:00 <josie> #startmeeting
14617:00 <josie> hi, and welcome back to round 2!
14717:00 <abubakarsadiq> hi
14817:00 <brunoerg> hi
14917:00 <emjshrx> hi!
15017:00 <josie> we are continuing our discussion of #28122 (notes here:
15117:02 <josie> before we jump in with question 5, does anyone have any questions or comments related to the discussion yesterday?
15217:02 <glozow> hi
15317:04 <josie> let's jump in! question 5 pertains to these commits: and
15417:05 <josie> these commits add functions for decoding and encoding a silent payment address. why can't we just reuse the existing encoder and decoder functions and add silent payments as a new `CTxDestination`?
15517:06 <josie> maybe as a start, can someone summarize what a `CTxDestination` is?
15617:06 <abubakarsadiq> txout script template, basically represent a scriptPubKey
15717:09 <josie> abubakarsadiq: yep! it's a std::variant which holds all of the standard scriptPubKey types
15817:09 <josie> why not just add the silent payments address here as a new destination?
15917:09 <abubakarsadiq> I think because silent payment payment address is the same as `WitnessV1Taproot` output type?
16017:10 <emjshrx> SP addresess are longer since they have both scan and spend keys, so we need a new decode fn?
16117:11 <josie> abubakarsadiq: well, the silent payments protocol dictates that the outputs need to be `WitnessV1Taproot`, but that's different than the silent payment address itself
16217:11 <abubakarsadiq> yes we have to modify the decode and encode to support silentpayment addresses
16317:12 <josie> emjshrx: it is longer, yes. in the payload, it contains two pubkeys. but we could easily handle this in the existing Encoder and Decoder functions. any ideas as to why we are writing new functions just for a silent payment address?
16417:12 <vostrnad> Because a silent payment address doesn't actually encode a specific output script?
16517:13 <josie> vostrnad: correct! a silent payment address is not a scriptPubKey. it's more like.. instructions on how to create scriptPubKeys
16617:15 <abubakarsadiq> instructions on how to create scriptPubKey can you elaborate on that?
16717:16 <vostrnad> abubakarsadiq: The silent payment address encodes the public keys needed to derive the actual output script, which is also dependent on the inputs of your transaction, so the output script is different every time.
16817:16 <emjshrx> ah yes, got it now
16917:17 <josie> yep! instead of giving you a scriptPubKey to send to (which is what a traditional address does), a silent payment address gives you pubkeys to do ECDH with, and then the protocol dictates how to turn that shared secret into a scriptPubKey for the receiver
17017:18 <josie> on the subject of address, let's go to question 6: BIP352 talks about versioning and forward compatibility. why is forward compatibility important?
17117:19 <josie> do you see any issues in how the PR is currently written with respect to forward compatibility?
17217:19 <emjshrx> so that future nodes dont have to rescan the blockchain?
17317:20 <josie> emjshrx: not quite, this is more about just interpreting the address itself
17417:21 <josie> so if I give you a string of characters, encoded in bech32m, how do you know what to do with it?
17517:21 <josie> maybe lets take a step back: can someone explain what forward compatibility means in this context?
17617:21 <emjshrx> by its metadata
17717:23 <abubakarsadiq> forward compatible means silentpayments version 0 interprets silentpayments version 1 addresses
17817:23 <abubakarsadiq> in this context
17917:24 <josie> abubakarsadiq: can you explain what you mean by "interprets" a silent payments v1 address?
18017:25 <josie> (also, as a hint, the relevant code is in this commit:
18117:25 <emjshrx> a v1 SP address should also be a valid v0 SP address?
18217:25 <vostrnad> Forward compatibility in this context means that wallets that only implement Silent Payments v0 can still send to future SP version addresses. (I'm still a bit unsure what kinds of changes could be useful to add to new versions without breaking compatibility with v0.)
18317:26 <josie> vostrnad: exactly. if I am a v0 wallet, and someone gives me a v1, v2, v3, etc address, I should still be able to send them a silent payment
18417:27 <abubakarsadiq> not sure but I think, means decodes the address from version 1 as a valid silent payment address
18517:27 <josie> abubakarsadiq: that's correct! even if someone gives you a newer version, your wallet should be able to decode it and send to it, even if you haven't upgraded
18617:27 <vostrnad> josie: What if a new version wants to intentionally break compatibility? Is that what v31 is for? If so, shouldn't more than one version be allocated for this?
18717:28 <Murch> Yeah, I was also wondering about that
18817:29 <josie> vostrnad: great question. v31 is reserved for an upgrade which would break compatibility. allocating more "breaking change" versions ahead of time is one approach, but can you think of why its okay to just allocate one?
18917:29 <abubakarsadiq> but I think `DecodeSilentAddress` and `DecodeSilentData` should be `DecodeV0SilentAddress` and `DecodeSilentData` because thats what they do actually
19017:30 <abubakarsadiq> `DecodeV0SilentData`
19117:30 <vostrnad> josie: Because we can then add one more byte to be the new version byte?
19217:30 <josie> vostrnad: exactly! and we can define new rules for how versions after the breaking change version should be treated
19317:32 <josie> abubakarsadiq: its true, that's what they do now, but these same functions could be added to in the future to handle v1 data, v2 data, etc without us needing to create new functions
19417:33 <josie> moving along, in `DecodeSilentAddress` there is a check on the version and data size. can you explain in your own words what's happening here?
19517:34 <josie> (also, in a previous question I asked if there were any issues with forward compatibility with the way the PR is currently written. there is ;) and it might be more obvious after looking at `DecodeSilentAddress`)
19617:35 <glozow> if it interests anyone, example of adding a new address type to decode/encode within the existing functions:
19717:35 <Murch> Well, if a new version adds more data to the address, we might need to have a way of getting only the forward compatible parts
19817:35 <Murch> e.g. restrict ourselves to parsing the first 66 characters
19917:35 <Murch> s/characters/byets/
20017:37 <josie> Murch: definitely! lets say v1 silent payments adds a third pubkey (just for the sake of example). what would happen with how the code is currently written? (hint: look at the change in src/bech32.cpp)
20117:38 <Murch> Is this about extending the limit of what length bech32m addresses we allow?
20217:39 <josie> Murch: bingo. right now, it just extends the length to 117. but it should really just remove the limit for a silent payment address (or make it something much higher, like 1023)
20317:39 <josie> if we were to add 33 more bytes in a v1 address, a v0 wallet would fail to decode the address if we keep the 117 char limit
20417:40 <josie> also, to your earlier point, we should probably be more explicit and say the first 66 bytes MUST be reserved for the v0 payload
20517:41 <josie> so most of the new silent payments code is in `src/wallet/silentpayments.{h,cpp}`. is this a good place for the code to live? why / why not?
20617:41 <josie> can you think of a use case where we would want to use silent payments code outside of a wallet context?
20717:43 <vostrnad> In an RPC call maybe? Not sure whether that would be useful for anything.
20817:43 <emjshrx> since we need private keys for sending and receiving I see it not being used outside wallets
20917:44 <glozow> perhaps for implementing a server that detects sps or computes stuff for on behalf of a lighter sp wallet
21017:44 <josie> vostrnad: I actually ran into this working on the receiving PR for silent payments :) some of the RPCs for creating transactions don't need a wallet at all
21117:45 <josie> glozow: exactly! one can imagine a use case where a full node indexes the tweak data for transactions and stores it in an index for light clients to query, or serves data via some bip158 like filter
21217:45 <josie> for this, you wouldn't even need a wallet compiled with the node
21317:46 <josie> does someone have an argument for why it's best to keep the silent payments code in `src/wallet` ?
21417:48 <glozow> better code organization. unless the intention is for bitcoind to be such a server
21517:49 <Guest35> no
21617:50 <josie> glozow: yeah, better code organization and much easier to spot code smell. in the original version of the PR, the silent payments code was outside the wallet and was calling functions in the script interpreter during scanning. moving it into the wallet made me rethink some of those boundaries
21717:51 <josie> speaking of scanning, the `Recipient` class ( is initialized with two private keys in the PR, the spend and scan key. are both keys necessary for scanning?
21817:52 <josie> can you propose a better implementation? and if so, which functions would you need to re-write?
21917:52 <glozow> ^I'd also expect to not compile sp code if I --disable-wallet, assuming the code is only used for sending/receiving
22017:52 <Murch> Well, presumably it would be mostly used in conjuction with your own wallet sending or receiving.
22117:53 <Murch> Or with external wallets that have been added as watch-only wallets
22217:53 <Murch> It may also be the area where we’d add code to watch a SP wallet for which we only know the scan key
22317:54 <josie> Murch, glozow: definitely. the current use case is most clearly a node scanning for its own wallet. so it wouldn't make sense to compile this code for nodes that explicitly don't want a wallet.
22417:55 <josie> in the future, we might lift some of this code out to be available for more general scanning, but it seems better to only lift out what we need at that point
22517:55 <Guest35> 'scan key'?
22617:55 <josie> Murch: regarding "for which we only know the scan key," that's almost an answer to question 9 :)
22717:57 <josie> Guest35: yep! BIP352 defines a spend key and a scan key. the silent payment output is created in such a way where the output can be found with just the scan key, but can only be spent with the spend key + the scan key
22817:58 <Guest35> tnx
22917:58 <josie> we are almost at time and there are still a few questions left, so I'd encourage everyone to go through the remaining questions and leave your feedback on the PR!
23017:59 <Murch> So, you only need the scan key to discover the payments, but you do need the spend key to pay
23117:59 <josie> if these review clubs have piqued your interest in silent payments, there's
23217:59 <glozow> thank you josie :) great review club meetings!
23317:59 <Murch> This means that the scan key can be e.g. handed to a trusted server of slightly less secure setup to find the payments, but the sc
23417:59 <josie> this PR is a parent PR for the one we are reviewing today, one for sending, and one for receiving
23517:59 <Murch> spend key can be kept more secure
23618:00 <josie> Murch: exactly! by separating the keys, you can delegate scanning to a "hot" device
23718:00 <Murch> I was pondering a bit whether there would be a way to make the scan key dependent on the secret key so that the addresses could be shorter
23818:00 <josie> this hot device could be your own node, with the spend key kept securely tucked away in cold storage
23918:00 <josie> thanks everyone for attending, especially for two meetings in a row!
24018:00 <josie> #endmeeting