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

bip-0322: add another valid sig vector not to confuse #1598

Merged
merged 1 commit into from
May 31, 2024

Conversation

ChrisCho-H
Copy link
Contributor

Following this discussion, add another valid sig vector not to confuse developer.
It has been misleading some developers, as they think their implementation is wrong even if it's right.
This test vector is also included in ongoing implementation of BIP322(PR is closed but seems like still in development here)

@murchandamus
Copy link
Contributor

Pinging @kallewoof

@murchandamus murchandamus added Proposed BIP modification Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified labels May 15, 2024
@bitcoin bitcoin deleted a comment from Vahid12015595 May 15, 2024
@bitcoin bitcoin deleted a comment from Vahid12015595 May 15, 2024
@kallewoof
Copy link
Member

LGTM, atlhough it might be good if @luke-jr OK'd as well.

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

BIP champion approved this change, and it seems like an improvement to me. Any reservations on merging this, @luke-jr?

@murchandamus murchandamus removed the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label May 31, 2024
@luke-jr
Copy link
Member

luke-jr commented May 31, 2024

Sounds fine to me (though I didn't verify the crypto)

This test vector is also included in ongoing implementation of BIP322(PR is closed but seems like still in development here)

This was a branch I made for OCEAN to be compatible with BIP322 signatures. It isn't intended as further development of BIP322 nor support thereof for Core or Knots.

I did consider developing BIP322 further, but came to the conclusion I would need to break compatibility with the current draft, and unfortunately the current draft is already implemented.

IMO at this stage, it should be stripped down to the one case actually implemented (afaik just Simple) and marked Final (if someone wants to complete a new signature system that actually covers all the use cases correctly, it should be a new BIP).

@luke-jr luke-jr merged commit 70d9b07 into bitcoin:master May 31, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants