Add support for taproot psbt fields BIP 371 by sanket1729 · Pull Request #681 · rust-bitcoin/rust-bitcoin

sanket1729 · 2021-10-27T13:15:30Z

Built on top of #677 . Will rebase and mark ready for review after #677 is merged.

RCasatta · 2021-10-27T13:26:23Z

Built on top of #666. Will rebase and mark ready for review after #666 is merged.

I think you mean #677

dr-orlovsky · 2021-11-06T08:38:25Z

You can grab SpendingSignature type implementation from my PR #671 after which I will close it. This type IMO simplifies serialization and readibility.

sanket1729 · 2021-11-06T12:42:59Z

I will do so after #677 is merged. I like creating a dedicated type for (signature, sighashtype). I already have a type for it in rust-miniscript and it is indeed handy.

dr-orlovsky · 2021-11-12T19:37:57Z

Now it can be rebased and finalized

sanket1729 · 2021-11-15T18:31:15Z

@dr-orlovsky, I think we should address this along with a resolution for #670. As implemented #671 is incorrect as it uses the wrong sighash type.

dr-orlovsky

tACK eaf1887

Compared against spec descriptions & test vectors and confirm that they do match (actually found a bug in the spec: bitcoin/bips#1241)

src/util/psbt/map/output.rs

src/util/psbt/serialize.rs

dr-orlovsky

utACK 0eb6730543dc39415b2ad082255505e54f578c9d

dr-orlovsky · 2021-12-02T19:17:16Z

@Kixunil pls if you could review this Taproot release blocker as well

Kixunil

Unfortunately, I'm not deeply familiar with byte-level details of Taproot, so I could only check structure of the code and don't feel confident acking. I found various potential improvements and suspicious code.

src/util/psbt/map/output.rs

src/util/psbt/serialize.rs

src/util/taproot.rs

src/util/psbt/serialize.rs

sanket1729 · 2021-12-02T23:51:15Z

Unfortunately, I'm not deeply familiar with byte-level details of Taproot, so I could only check structure of the code and don't feel confident acking. I found various potential improvements and suspicious code.

Thanks, this is super valuable. I will address your comments shortly. As for the correctness of the taproot code, there are exact test vectors from the BIP that we are testing against which should give some confidence.

sanket1729 · 2021-12-12T07:26:19Z

Sorry for the delay. I was traveling and could not get to this. I have addressed all your comments.

Kixunil · 2021-12-12T11:36:45Z

First light review looks good. Will try to do deeper later.

dr-orlovsky

tACK 4e79e4483f4eaa77a7e08d8a6245ce507f18b9e6

Excellent work! Tested all commits with git rebase -x using full test script.

src/util/psbt/map/output.rs

src/util/psbt/serialize.rs

dr-orlovsky · 2021-12-21T11:31:36Z

Needs a rebase again...

sanket1729 · 2021-12-23T18:29:26Z

Rebased

sanket1729 · 2021-12-24T10:39:25Z

Rebased again after #686

apoelstra

ACK 3bbd4465731011c8927bd56f0c1d565a136dc542

I also did not check the byte-for-byte compatibility with BIP 371. Hopefully the test vectors save us here. Also I did not check that the test vectors were correct.

sanket1729 · 2021-12-28T14:23:32Z

Pushed again to use schnorr::SchnorrSig struct from #702

apoelstra

ACK 7d982fa

Wow, having the sighashtype inside the signature type really makes things cleaner.

dr-orlovsky

re-tACK 7d982fa basing on git range-diff. The original PR before last re-base was tested commit-by-commit.

…e features ccc2815 Remove no-std and actual-serde features (Sexosexosexo) Pull request description: Closes rust-bitcoin#681 The `no-std` feature is no longer needed with `bitcoin` version 0.32.0 - Update README.md (in Building I have copy-pasted part of the same section from `rust-bitcoin`) - Disable `actual-serde` implicit feature I think `FEATURES_WITH_NO_STD` now could be removed from the maintainer tools. Let me know if there's something missing or to improve! ACKs for top commit: apoelstra: ACK ccc2815; successfully ran local tests; even nicer! Tree-SHA512: c1146176d65d1953b765273e55d28b8c6b926bcd091e46db66858b448c9435f90db703ae7ec11425a316276ce73c8ec6025be73afbf22ea88aa6ff27224abe9a

sanket1729 force-pushed the taproot_psbt branch from 7ce7d71 to 187234f Compare October 28, 2021 07:46

dr-orlovsky mentioned this pull request Nov 6, 2021

PSBT taproot spending signature type #671

Closed

dr-orlovsky mentioned this pull request Nov 12, 2021

Tracking issue for Taproot #503

Closed

20 tasks

sanket1729 force-pushed the taproot_psbt branch from 187234f to 0a158a7 Compare November 15, 2021 18:06

sanket1729 marked this pull request as ready for review November 15, 2021 18:31

sanket1729 force-pushed the taproot_psbt branch from 0a158a7 to eaf1887 Compare November 21, 2021 22:57

dr-orlovsky previously approved these changes Nov 23, 2021

View reviewed changes

src/util/psbt/map/output.rs Outdated Show resolved Hide resolved

src/util/psbt/serialize.rs Outdated Show resolved Hide resolved

sanket1729 dismissed dr-orlovsky’s stale review via 108ba73 November 24, 2021 18:07

sanket1729 force-pushed the taproot_psbt branch 2 times, most recently from 108ba73 to 0eb6730 Compare November 24, 2021 18:20

dr-orlovsky previously approved these changes Nov 24, 2021

View reviewed changes

Kixunil added the one ack PRs that have one ACK, so one more can progress them label Dec 2, 2021

Kixunil reviewed Dec 2, 2021

View reviewed changes

sanket1729 dismissed dr-orlovsky’s stale review via 0e0ba8e December 12, 2021 07:08

sanket1729 force-pushed the taproot_psbt branch 2 times, most recently from 0e0ba8e to 4e79e44 Compare December 12, 2021 07:22

dr-orlovsky previously approved these changes Dec 12, 2021

View reviewed changes

src/util/psbt/map/output.rs Outdated Show resolved Hide resolved

src/util/psbt/serialize.rs Outdated Show resolved Hide resolved

src/util/psbt/serialize.rs Outdated Show resolved Hide resolved

sanket1729 dismissed dr-orlovsky’s stale review via 7b2ccbe December 15, 2021 14:37

sanket1729 force-pushed the taproot_psbt branch 2 times, most recently from 7b2ccbe to 2f7e0a9 Compare December 15, 2021 14:42

sanket1729 force-pushed the taproot_psbt branch from 2f7e0a9 to 708f2da Compare December 23, 2021 18:26

sanket1729 force-pushed the taproot_psbt branch 2 times, most recently from 8387423 to 3bbd446 Compare December 24, 2021 10:36

apoelstra previously approved these changes Dec 24, 2021

View reviewed changes

sanket1729 dismissed apoelstra’s stale review via 7d8fc66 December 28, 2021 14:22

sanket1729 force-pushed the taproot_psbt branch from 3bbd446 to 7d8fc66 Compare December 28, 2021 14:22

sanket1729 removed the one ack PRs that have one ACK, so one more can progress them label Dec 28, 2021

sanket1729 added 4 commits December 28, 2021 20:40

Derive serde for taproot stuctures

c7478d8

Impl encodable traits for TapLeafhash

108fc3d

Taproot psbt impl BIP 371

d22e014

Add all tests from BIP 371

7d982fa

sanket1729 force-pushed the taproot_psbt branch from 7d8fc66 to 7d982fa Compare December 28, 2021 15:11

apoelstra approved these changes Dec 28, 2021

View reviewed changes

dr-orlovsky approved these changes Dec 30, 2021

View reviewed changes

dr-orlovsky merged commit 670e808 into rust-bitcoin:master Dec 30, 2021

dr-orlovsky mentioned this pull request Jan 15, 2022

Bump version to v0.28 #785

Closed

Conversation

sanket1729 commented Oct 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RCasatta commented Oct 27, 2021

Uh oh!

dr-orlovsky commented Nov 6, 2021

Uh oh!

sanket1729 commented Nov 6, 2021

Uh oh!

dr-orlovsky commented Nov 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanket1729 commented Nov 15, 2021

Uh oh!

dr-orlovsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dr-orlovsky left a comment

Choose a reason for hiding this comment

Uh oh!

dr-orlovsky commented Dec 2, 2021

Uh oh!

Kixunil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sanket1729 commented Dec 2, 2021

Uh oh!

sanket1729 commented Dec 12, 2021

Uh oh!

Kixunil commented Dec 12, 2021

Uh oh!

dr-orlovsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dr-orlovsky commented Dec 21, 2021

Uh oh!

sanket1729 commented Dec 23, 2021

Uh oh!

sanket1729 commented Dec 24, 2021

Uh oh!

apoelstra left a comment

Choose a reason for hiding this comment

Uh oh!

sanket1729 commented Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apoelstra left a comment

Choose a reason for hiding this comment

Uh oh!

dr-orlovsky left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sanket1729 commented Oct 27, 2021 •

edited

Loading

dr-orlovsky commented Nov 12, 2021 •

edited

Loading

sanket1729 commented Dec 28, 2021 •

edited

Loading