-
Notifications
You must be signed in to change notification settings - Fork 2.2k
chore(payment): Implement silent payments #2269
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.
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
base: master
Are you sure you want to change the base?
Conversation
This PR implements bip352 and exposes the following functions: - scanForSilentPayments: to find if given outputs belong to our sp address - deriveOutput: to create a new silent payment output TODO: - [] Decide how we expose scanning and deriving to the developer i guess via the p2sp object ? - [] Decide whether we return the found hex endcoded or Uint8Array output - [] Figure out of we can remove `modN32` and `subBE` as they might exist? - [] Add derivation logic - [] Replace `getPubkeyFromInputTS` from test with something that exists?
Tags were missing from the TAGS
Some outputs wernt matching what we expected them to be.
|
I've been in discussions on the best way to handle this, and I think it would be a better idea to pass in a cached key. Instead of calculating the sum of the inputs in real time every time, The payment should only take a special key ie. This is primarily because our friends over at BlueWallet see a future where apps like electrs or mempool etc will pre-calculate the aggregate keys and index them for us.... But... yeah... ballooning Payments API into something that encompasses a whole transaction seems like a bit much. At that point we might as well turn it into a method on the Psbt. |
|
@Overtorment if you'd like to chime in, feel free. |
ye maybe use the following logic:
|
|
ill take a look. btw we have a TS implementation of SP here: https://github.com/BlueWallet/SilentPayments/ |
Shouldent something have failed here?
Added lazy prop for outputs.
Allow various cases for deriving of outputs : bitcoinjs-lib/ts_src/payments/p2sp.ts Line 89 in 4f52dde
|
ts_src/payments/p2sp.ts
Outdated
| network, | ||
| ); | ||
| }); | ||
| lazy.prop(o, 'outputs', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something I am unsure about.
- Should outputs be the ones create to send?
- Where and how do we store resulting UTXO when we scanned?
Ye I mentioned on the top :) (thought I referred to to something inside the core bluewallet app). |
I would like to repeat what I said before:
It seems like your reply is ignoring this and talking about placing all of that inside the p2sp Payment API still... am I misunderstanding your response? Could you please clarify? |
For the inputs/outputs i was thinking of a bare minimal set of data, the parsing of the transactions into receiving inputs would be done elsewhere. I think the lazy output property kinda reflects this now right? or do you still think even that is too much here? |
|
i feel a bit out of element here, every time i get distracted from SP i completely forget how they work and getting familiar again means re-reading all the specs again, and it takes time. are there any specific questions for me? if yes, dumb it down for me. |
no worries thanks for making me aware of scure-btc-signer also mentioned the blue wallet repo there as there was an issue open for silent payments too. |
We dont want to drag in transactions into the base of p2sp.
Some basic renames hopefully more readable.
Added typings for SilentOutput and removed usage of Output. This SilentOutput is information used to create (or find) an actual UTXO. Its a building block instead of a final output.
Documented more params.
Added the type for more readability.
Forgot to remove the import.
…puts This is not the final name but hopefully makes this should be used as a building block to create a UTXO instead of final output.
We did not want to drag in inputs in payments so instead created a class that extends Transaction. This makes more sense for detecting (scanning) which UTXOs belong to our keys and as well for creating outputs who depend on the given inputs.
|
This PR is getting to be way out of scope for this library. Wipe everything and start from the beginning. What is your goal? Don't be vague. "To support BIP352" is vague. Describing a list of all the things you did before planning anything is also vague... it doesn't tell me your intentions.
Let's start there. |
|
Stop coding. Discussions only for now. |
Note that the silent payment address looks one way when you share it and different on chain as the addresses are derived from the silent payment address. |
I think that perhaps instead of using the existing Payments API, we should create a new SilentPayment class that doesn't extend Payment, it just exists on its own. It contains either a pub or priv of both spend and scan keys. It should have a static constructor method fromStealthAddress(s: string): SilentPayment that creates one with both public keys. Starting from this baseline, we could try to make this class interact with Transaction and Psbt. I don't think any code in bitcoinjs-lib should try to connect to RPC or some external service. It should be a simple data in data out API. ie. pass in a Transaction and Uint8Array[] (where each scriptPubkey Uint8Array corresponds to the same index input of the Transaction) Or maybe as simple as inputs, outputs (the outputs spent by the inputs) This might even be better suited for its own library that depends on bitcoinjs-lib... you could do that and just upstream the minimal changes you need from us to better support your library. I'm still open to including it in bitcoinjs-lib, but one thing I'm a bit unsure about is the Psbt support. For sending TO a stealth address, there should really be stealth payment info embedded in the Psbt so that we can update the output addresses if someone adds an input to a partially signed Psbt. I believe the BIP352 authors are currently discussing how to deal with Psbt... However, we DID release P2TR support without Psbt support initially (and it was very hacky and no one liked it) so I'm open to all ideas. Please discuss a concrete plan for an API (ie. like a d.ts style pseudo-type-declaration just to give a rough idea) before you start coding though so we don't waste your time. If you decide to go the separate library route, feel free to mention me on any issues you have over there and I'll be happy to join the discussion. |
|
I think its good if the base is this in library as I assume it will get more eyeballs and checks from more people. I tried to come with a very basic layout but its tougher than I thought, I dont want to expose the developer to too much of the internals. Should be as simple as I want to send 0.2 to silent payment address sp213123... initial spending and receiving layoutclass SilentPayment{
/**
* Encode a Silent payment address for others to spend to
* Encodes spend and scan public keys into a Bech32m Silent Payment address.
* @param {Uint8Array} [B_spend] - spend public key.
* @param {Uint8Array} [B_scan] - public scan key.
* @param {number} [version] - Optional version number of the silent payment scheme.
* @param network - testing, regtest or prod
* @returns bech32m encoded string
*/
export function encodeSilentPaymentAddress(
B_spend: Uint8Array,
B_scan: Uint8Array,
version = 0,
network: Network = BITCOIN_NETWORK,
): string
// TODO what kind of output do we want here ?
fuction createOutput(inputs : Input[], recipients : {silentAddr: string, value : number}[]) : ?
/**
* Convenience function to create outputs from a transaction
* Note: Silent payment depend on inputs, so if any input changes the output also changes
* This function removes all other outputs already set on the transaction to prevent outputs created on different input
* set.
* @param recipients - list of silent payment recipients
* @param tx - Partially build transactions already containing the required inputs
* @returns a transaction with
*/
export function createSilentPaymentTransaction(recipients : {silentAddr: string, value : number}[], tx: Transaction) : Transaction
/**
Storage interface for silent address and generated in/output addresses
Used to display to the user which on chain addresses belong to which silent payment addresses
*/
interface SilentPaymentOutputGroup{
silentPaymentAddress : string
sent : {txid : string, vout: number, pub_key : string, destSilentPaymentAddress : string}[],
received : {txid : string, vout: number, pub_key : string, originSilentPaymentAddress?: string}[]
}
}ScanningI think we may want to move scanning to a different PR and first focus on spending and sending. class SilentPayment{
/**
verify if given outputs really belong to us
@param potentialOutputs - List of outputs potentially to be for us
@param S - shared secret
@param B_spend - public spend key
@param b_scan - private scan key
@returns a list of SilentPaymentOutputs that can be used to construct an actual p2tr output or Pbst
*/
verifyScanResult(potentialOutputs : Output[], S: Uint8Array, B_spend : Uint8Array, b_scan : Uint8Array) : SilentPaymentOutput []
/**
scan if the given outputs belong to us
@param tk - tweaked hash
@param potentialOutputs - List of outputs potentially to be for us
@param B_spend - public spend key ( to check if
@param b_scan - private scan key
@returns a list of SilentPaymentOutputs that can be used to construct an actual bip341 output
*/
scan(tk: Uint8Array, potentialOutputs : Output[], B_spend : Uint8Array, b_scan : Uint8Array) : SilentPaymentOutput []
/**
check if a given transaction contains UTXO for our key public key B_scan
@param decodedTx - decoded transaction
@param B_spend - public spend key
@param b_scan - private scan key
*/
scan(decodedTx: Transaction, B_spend : Uint8Array, b_scan : Uint8Array) : SilentPaymentOutput []
/**
check if a given transaction contains UTXO for our key public key B_scan
@param rawTx - raw transaction bytes
@param B_spend - public spend key
@param b_scan - private scan key
@returns a list of SilentPaymentOutputs that can be used to construct an actual p2tr output or Pbst
*/
scan( rawTx: Uint8Array, B_spend : Uint8Array, b_scan : Uint8Array) : SilentPaymentOutput []
} |
I didn't understand the use case for this, nor how a hypothetical function that output this interface would work.
Without the previous outputs (pointed at by the input) it's impossible to know the public key we need for creating the aggregate pubkey. I am thinking more along the lines of:
I think this would be a good minimal interface to start with. |
Draft for now looking for a bit of feedback on how we can expose the functionality to the developer
Implements bip352 and exposes the following functions:
scanForSilentPayments: to find if given outputs belong to our sp addressderiveOutput: to create a new silent payment outputencodeSilentPaymentAddressdecodeSilentPaymentAddressNote it tests against vectors taken from: https://github.com/bitcoin/bips/blob/master/bip-0352/send_and_receive_test_vectors.json
TODO:
modN32andsubBEas they might exist?getPubkeyFromInputTSfrom test with something that exists?Do note that when this bitcoin-core/secp256k1#1698 gets merged and we make it available in https://github.com/bitcoinjs/tiny-secp256k1 part of this code becomes redundant but at least we already have the interfaces in place.
Bluewallet also has a TS implementation : https://github.com/BlueWallet/SilentPayments/blob/master/src/index.ts