Skip to content

add BIP341 to the threshold Schnorr rust example - #1071

Merged
altkdf merged 14 commits into
masterfrom
alex/bip341-in-tschnorr
Jan 3, 2025
Merged

add BIP341 to the threshold Schnorr rust example#1071
altkdf merged 14 commits into
masterfrom
alex/bip341-in-tschnorr

Conversation

@altkdf

@altkdf altkdf commented Dec 19, 2024

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread rust/threshold-schnorr/README.md Outdated
Comment thread rust/threshold-schnorr/README.md Outdated
Comment thread rust/threshold-schnorr/README.md Outdated
Comment thread .github/workflows/rust-threshold-schnorr-example.yml
Comment thread rust/threshold-schnorr/src/schnorr_example_rust/build.sh
Comment thread rust/threshold-schnorr/src/schnorr_example_rust/build.sh
Comment thread rust/threshold-schnorr/src/schnorr_example_rust/schnorr_example.did
Comment thread rust/threshold-schnorr/src/schnorr_example_rust/tests/tests.rs Outdated
Comment thread rust/threshold-schnorr/src/schnorr_example_rust/tests/tests.rs
Comment thread rust/threshold-schnorr/src/schnorr_example_rust/tests/tests.rs
fspreiss
fspreiss previously approved these changes Dec 20, 2024
@altkdf
altkdf marked this pull request as ready for review December 20, 2024 17:33
@altkdf
altkdf requested a review from a team December 20, 2024 17:33
@altkdf
altkdf requested a review from a team as a code owner December 20, 2024 17:33
@altkdf
altkdf requested a review from randombit December 20, 2024 17:38
k256::schnorr::Signature::try_from(sig_bytes).expect("failed to deserialize signature");

let vk = k256::schnorr::VerifyingKey::from_bytes(&tweaked_pk_bytes)
.expect("failed to deserialize BIP340 encoding into public key");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIRC we can do the entire verification using secp256k1 here rather than having to use both k256 and secp256k.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alternately, with a bit more effort (copying the code from https://github.com/dfinity/ic/blob/master/rs/crypto/secp256k1/src/lib.rs#L352) we could do the verification using only k256 and avoid the bitcoin dependency. Given the apparent complications in the build that this brings in, it might be worthwhile.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed k256.

Re removing bitcoin, there's also another aspect, which is complexity. I guess here, among other things, we want to show how simple it is to use threshold Schnorr. It's useful to show that it's perfectly fine to use the existing bitcoin crate with threshold Schnorr in a canister.

fspreiss
fspreiss previously approved these changes Dec 25, 2024

@fspreiss fspreiss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-approving because review was automatically dismissed

fspreiss
fspreiss previously approved these changes Jan 3, 2025

@fspreiss fspreiss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-approving because review was automatically dismissed

Comment thread rust/threshold-schnorr/README.md Outdated
fspreiss
fspreiss previously approved these changes Jan 3, 2025

@fspreiss fspreiss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-approving because review was automatically dismissed

jessiemongeon1
jessiemongeon1 previously approved these changes Jan 3, 2025
Comment thread rust/threshold-schnorr/README.md Outdated
Comment thread rust/threshold-schnorr/README.md Outdated
Comment thread rust/threshold-schnorr/README.md Outdated
Co-authored-by: Jessie Mongeon <133128541+jessiemongeon1@users.noreply.github.com>
@altkdf
altkdf dismissed stale reviews from jessiemongeon1 and fspreiss via 225cdd3 January 3, 2025 15:40
@altkdf

altkdf commented Jan 3, 2025

Copy link
Copy Markdown
Contributor Author

Oh, approvals are reset even if I only apply suggestions from the PR via the github UI. That's quite extreme...

@fspreiss fspreiss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-approving because review was automatically dismissed

@altkdf
altkdf merged commit 34ec579 into master Jan 3, 2025
@altkdf
altkdf deleted the alex/bip341-in-tschnorr branch January 3, 2025 16:13
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