Skip to content

Add psetv2 support#53

Merged
tiero merged 57 commits into
vulpemventures:masterfrom
altafan:psetv2
Sep 16, 2022
Merged

Add psetv2 support#53
tiero merged 57 commits into
vulpemventures:masterfrom
altafan:psetv2

Conversation

@altafan
Copy link
Copy Markdown
Collaborator

@altafan altafan commented Jun 7, 2022

This adds support for psetv2, which looks more similar to the go-elements implementation rather than the existing psbt (psetv0) package.

This is done on purpose in order to have a clear separation between the 2 packages, and also to have a clear separation of the "roles" as described in the spec.

  • Pset, Input, Output and Global are the classes for the inner representation of a partial transaction.
  • Creator, Updater, Blinder, Signer, Finalizer and Extractor are the classes to manipulate a partial transaction according to the spec.

NOTE: To prevent having secp256k1-zkp as a direct dependency, the Blinder role makes use of 2 interfaces BlindingGenerator and BlindingValidator (similarly to what is done in go-elements) to generate blinding data for the last output of the last blinder, and to validate all blinding data before adding to the partial transaction.
confidential.ts, instead, contains ZKPGenerator and ZKPValidator that are the implementations of such interfaces.

BONUS:

  • This fixes AssetHash and adds tests for it, and similarly, this introduces ElementsValue replacing the deprecated satoshiToConfidentialValue|confidentialValueToSatoshi.
  • Adds name to existing network interfaces.
  • Adds and exports new method to get type of output script.

TODO:

  • Taproot support (should be done here or in a next PR?)
  • Fix import path of secp256k1-zkp in package.json as soon as #27 is merged.

Closes #32.
Closes #51.

Please @tiero review this.

@tiero tiero requested a review from louisinger June 7, 2022 16:26
Comment thread ts_src/psetv2/extractor.ts
Comment thread ts_src/psetv2/fields.ts Outdated
Comment thread ts_src/psetv2/finalizer.ts Outdated
Comment thread ts_src/psetv2/globals.ts Outdated
Comment thread ts_src/psetv2/input.ts
Comment thread ts_src/psetv2/signer.ts
Comment thread ts_src/psetv2/utils.ts Outdated
Comment thread ts_src/psetv2/utils.ts Outdated
Comment thread ts_src/psetv2/interfaces.ts
Comment thread ts_src/index.ts
@tiero
Copy link
Copy Markdown
Member

tiero commented Jun 9, 2022

Taproot support (should be done here or in a next PR?)

I would do it here since we are at it. They are just extra fields

Fix import path of secp256k1-zkp in package.json as soon as vulpemventures/secp256k1-zkp#27 is merged.

I pushed to npm v2.1.0

Comment thread src/asset.js
@altafan
Copy link
Copy Markdown
Collaborator Author

altafan commented Jun 14, 2022

@tiero @louisinger I synced my branch with master and added the missing support for taproot. The "biggest" changes are the new taproot input/output fields (of course), and among the roles, I worked on the abstraction for signing and finalizing an input, whether it is taproot or non-taproot.

@louisinger Not sure why, but some of the tests of hashForWitnessV1 are now failing. Can you take a look?

I ask you guys if you could write a taproot test case in test/intergration/psetv2.spec.ts as the very first users of this new feature. Something very simple but with enough meaning to have decent coverage, or at least if you can suggest one.

Comment thread ts_src/psetv2/finalizer.ts
@tiero
Copy link
Copy Markdown
Member

tiero commented Jun 14, 2022

You can look in bitcoinJS PR for test vectors, I aaked them to make.simple calculator bside also CHECKSIG

Comment thread ts_src/psetv2/utils.ts
@tiero
Copy link
Copy Markdown
Member

tiero commented Jul 14, 2022

some problems with parsing the pset with elements CLI

Did you test with this branch? ElementsProject/elements#1121

But I guess the only thing we care about now is making sure GDK's sign_psbt works, so I would focus on making anything necessary to be able to test with that

@altafan
Copy link
Copy Markdown
Collaborator Author

altafan commented Jul 15, 2022

Did you test with this branch? ElementsProject/elements#1121

No, I didn't yet. I have to compile the binary for that version in order to test against it, there doesn't seem to be any available.

But I guess the only thing we care about now is making sure GDK's sign_psbt works, so I would focus on making anything necessary to be able to test with that

Right now, a pset in base64 format containing only unconfidential inputs made with this new package can be successfully decoded with the elements CLI's decodepsbt RPC.
This makes me think that at least for this kind of txs we're 100% spec-compliant and that GDK too should be able to parse the pset.

Comment thread package.json
"bip174-liquid": "^1.0.3",
"bip66": "^1.1.0",
"bitcoin-ops": "^1.4.0",
"bitcoinjs-lib": "^6.0.2",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats this used for

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to dev dependencies

Comment thread src/value.js
get bytes() {
return Buffer.concat([Buffer.of(this.prefix), this.value]);
}
get number() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename "satoshis"?

Comment thread ts_src/psetv2/creator.ts Outdated
import { OPS } from '../ops';
import BitSet from 'bitset';

export class Input {
Copy link
Copy Markdown
Collaborator

@louisinger louisinger Sep 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an export conflict with input.ts/Input class

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok my bad, exported as PsetInput -> maybe clearer to rename ?

Comment thread ts_src/psetv2/creator.ts Outdated
Comment thread ts_src/psetv2/blinder.ts Outdated
@vulpemventures vulpemventures deleted a comment from louisinger Sep 16, 2022
@tiero tiero merged commit 641178e into vulpemventures:master Sep 16, 2022
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

Successfully merging this pull request may close these issues.

AssetHash should implement a bytesWithoutPrefix getter Support PSET v2

3 participants