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

Create ecdsa::LegacySignature and ecdsa::SegwitV0Signature #1989

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Aug 11, 2023

Draft because on top of #2152, this PR is only the last patch.

This is a re-spin of #1606 since the sighash types were split already since then.

In an effort to further spit up the ECDSA type into legacy and segwit v0 add two new sig types. Currently unused, as it is this does not buy us much but if we would like to re-design our keys with legacy/segwit in mind then this will be needed.

@tcharding tcharding force-pushed the 08-11-ecdsa-sig-split branch 3 times, most recently from e1dde68 to 59fa30b Compare August 14, 2023 04:38
@tcharding
Copy link
Member Author

tcharding commented Aug 14, 2023

Note that this PR just duplicates the ecda::Signature API and makes the inner signatures pub(crate) - do we want the pub?

@tcharding tcharding marked this pull request as ready for review August 14, 2023 23:49
@tcharding tcharding marked this pull request as draft September 25, 2023 23:09
@tcharding
Copy link
Member Author

Not sure now if this is going in the right direction.

@apoelstra
Copy link
Member

I think this is going in the right direction. The main difficulty as always is that PSBT (the protocol) commingles these types so we'll need a PSBT-specific enum.

Eventually the legacy signature will use legacy keys and segwit signatures will use (compressed only) secp keys, which should also simplify our public key story.

@apoelstra
Copy link
Member

One observation I have is that these signature types seem to line up with the scriptpubkey/witness/txout-type table that you mentiend in #2084

I wonder if we want to try to create some "output type" trait which uses associated types and constants to capture the workflow here. And we could maybe allow upcasting/downcasting to a "generic" version of the output type and its associated types, for applications like PSBT where things are just byte-serialized.

@tcharding
Copy link
Member Author

Man, I like that idea.

@tcharding tcharding marked this pull request as ready for review October 2, 2023 02:34
@tcharding
Copy link
Member Author

On ice until next release.

@tcharding tcharding marked this pull request as draft October 10, 2023 21:37
Create a subdirectory `crytpo/ecdsa` and move `ecdsa.rs` to
`ecdsa/mod.rs`.

Done in preparation for further improvements to the `ecdsa` module.

No other changes.
Move the error code out of the `ecdsa/mod.rs` file and into a private
`error` submodule. Add public re-export so this not a breaking change.

Code move only plus import/export changes to build cleanly. No other
changes.
Create a subdirectory `crytpo/sighash` and move `sighash.rs` to
`sighash/mod.rs`.

Done in preparation for further improvements to the `sighash` module.

No other changes.
Move the error code out of the `sighash/mod.rs` file and into a private
`error` submodule. Add public re-export so this not a breaking change.

Code move only plus import/export changes to build cleanly. No other
changes.
Move the two sighash types (`TapSighashType` and `EcdsaSighashType`) to
a `sighash_type` submodule.

While we are at it lay the code out with impl blocks below the type
definitions as is customary.

Code move, re-order, and fix import/export paths only. No other changes.
Add two types

- `ecdsa::legacy::Signature`
- `ecdsa::segwit_v0::Signature`.

Both are just thin wrappers around `ecdsa::Signature`.

Do not use the new types anywhere.
@tcharding
Copy link
Member Author

I wonder if we want to try to create some "output type" trait which uses associated types and constants to capture the workflow here.

Started in #2154

@tcharding tcharding deleted the 08-11-ecdsa-sig-split branch May 22, 2024 23:21
@tcharding tcharding restored the 08-11-ecdsa-sig-split branch June 1, 2024 21:15
@tcharding tcharding deleted the 08-11-ecdsa-sig-split branch June 1, 2024 21:15
@tcharding tcharding restored the 08-11-ecdsa-sig-split branch June 1, 2024 21:16
@tcharding
Copy link
Member Author

Mad, github is not Linux, I can un-delete things :)

@tcharding tcharding reopened this Jun 1, 2024
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.

None yet

2 participants