Skip to content

PSBT taproot spending signature type#671

Closed
dr-orlovsky wants to merge 2 commits intorust-bitcoin:masterfrom
BP-WG:psbt/taproot
Closed

PSBT taproot spending signature type#671
dr-orlovsky wants to merge 2 commits intorust-bitcoin:masterfrom
BP-WG:psbt/taproot

Conversation

@dr-orlovsky
Copy link
Copy Markdown
Collaborator

No description provided.

containing both signature and SigHash, serializing according 
to the consensus (for witness field) and PSBT (for taproot signature 
keys) rules
@dr-orlovsky dr-orlovsky added this to the 0.28.0 milestone Sep 29, 2021
@dr-orlovsky dr-orlovsky mentioned this pull request Sep 29, 2021
20 tasks
pub fn sighash_byte(self) -> u8 {
self.sighash_type as u8
}

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.

Why do these consume self?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Because self is Copy - and, according to rust API guidelines, copy-types should consume self, rather than taking it by ref.

The rationale is quite simple: Self here is 8 bit, reference is 64 bit.

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.

Because self is Copy - and, according to rust API guidelines, copy-types should consume self, rather than taking it by ref.

Interesting, thanks I did not know that.

The rationale is quite simple: Self here is 8 bit, reference is 64 bit.

self here is SpendingSignature not 8 bit, but it is Copy :)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh right. We had this discussion before, that pushing 32 bytes to stack in function call may be still cheaper than 64-bit pointer + memory access

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.

More importantly, the compiler can do these optimizations better than all of us combined. Passing via value or reference should be mostly semantics, not an optimization.

@dr-orlovsky
Copy link
Copy Markdown
Collaborator Author

Should be rebased in top of #681

@dr-orlovsky dr-orlovsky changed the title PSBT taproot implementation (BIP-371) PSBT taproot spending signature type Nov 6, 2021
@sanket1729
Copy link
Copy Markdown
Member

I will open a PR to address #670 first that should address this

@dr-orlovsky
Copy link
Copy Markdown
Collaborator Author

dr-orlovsky commented Nov 23, 2021

Superseded by #681 and #702

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.

4 participants