Skip to content
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

Define PSBT, Re-enable PSBT functional tests #504

Closed
stevenroose opened this issue Feb 11, 2019 · 8 comments
Closed

Define PSBT, Re-enable PSBT functional tests #504

stevenroose opened this issue Feb 11, 2019 · 8 comments
Assignees

Comments

@stevenroose
Copy link
Member

The PSBT functional tests are being disabled in #494.

@instagibbs instagibbs changed the title Re-enable PSBT functional tests when Elements PSBT format is defined Define PSBT, Re-enable PSBT functional tests for 0.17 Feb 13, 2019
@instagibbs
Copy link
Collaborator

renamed it so we can discuss format here.

@instagibbs
Copy link
Collaborator

instagibbs commented Feb 13, 2019

We can extend BIP174 with these new per-input/per-output fields:

  1. Amount
  2. Value blinding factor
  3. Asset type
  4. Asset blinding factor
  5. Blinding pubkeys (output only? Any reason to attach to input?)
  6. rangeproof(output only)
  7. surjectionproof(output only)

Each field in network serialization. These fields should be legal with or without the inputs/outputs being blinded already. I think that's about it.

@instagibbs
Copy link
Collaborator

instagibbs commented Feb 13, 2019

One * here is that we need to be able to collaboratively make a transaction, while not revealing the particulars of an output if you don't want to, but also having a PSBT be "equal" to another PSBT(BIP174 this means it's the same unsigned transaction).

In PSBT the "Creator" makes a transaction, so we potentially need a way of users to create blinded "slots" for outputs. I suppose this could be an output with both value and asset fields being a null value. It could also just come with blinded commitment in the output a priori, but it could not come with a surjection proof(would take an "Updater" step to know what the inputs are possibly). The initially-brought blinded commitments would also require recover later, which means a wallet step is already involved before this, and if the inputs aren't already filled in, there's no way to balance the blinding factors. It will likely interact poorly with deterministic blinding factor designs.

It may make more sense to allow value/asset commitments be directly included in the transaction when privacy between counter-parties is desired, and change the PSBT equality definition internally(or define blinded equality) to allow this to "change". We would also treat null asset/amount values as "same" for this expanded definition. For signing, of course we must use the original definition(for sighash reasons if nothing else).

In the case where privacy between signers isn't desired, leave the raw transaction alone, and fill in the new fields as needed.

@instagibbs
Copy link
Collaborator

Something to chew on:

Some counter-parties you're ok sharing all data with(say, hww), while others you are not(trading peer).

Is it enough to have a strip option that takes out everything except maybe input asset type(for surjection proof)?

@gwillen
Copy link
Contributor

gwillen commented Feb 14, 2019

FWIW the idea that having the same underlying transaction is "equality" for PSBTs was never semantically good, and I am killing the overloaded op== on PSBT object in bitcoin#14978, with no apparent objections, as being a terrible abuse of operator overloading. I think instead the right concept is "can be merged with" or "compatible with".

@instagibbs
Copy link
Collaborator

instagibbs commented Feb 14, 2019 via email

@instagibbs instagibbs added 0.17 and removed 0.17 labels Mar 21, 2019
@instagibbs instagibbs changed the title Define PSBT, Re-enable PSBT functional tests for 0.17 Define PSBT, Re-enable PSBT functional tests Mar 22, 2019
@instagibbs instagibbs mentioned this issue Apr 9, 2019
7 tasks
@gwillen
Copy link
Contributor

gwillen commented May 3, 2019

see #600

@instagibbs
Copy link
Collaborator

this was completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants