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

Implement BIP322 #1224

Merged
merged 9 commits into from
Sep 6, 2024
Merged

Implement BIP322 #1224

merged 9 commits into from
Sep 6, 2024

Conversation

Kukks
Copy link
Contributor

@Kukks Kukks commented Sep 4, 2024

No description provided.

@Kukks Kukks force-pushed the bip322 branch 4 times, most recently from 3f1f0d5 to d5fb75d Compare September 4, 2024 14:00
@lontivero
Copy link
Contributor

ack. I only reviewed the Key, Address and Bip322Signature and they look good to me. I would suggest to avoid try{}catch{} with empty catch because it makes the developers' live miserable, any bug is hidden and you will never understand why.

On a side note, I am not sure we can use this in Wasabi. The Slip19 is based on a much earlier version of the bip322. Anyway, good job and thanks for this one @Kukks

@NicolasDorier
Copy link
Collaborator

I would suggest to avoid try{}catch{} with empty catch because it makes the developers' live miserable, any bug is hidden and you will never understand why.

Sadly not possible when parsing a witscript and a transaction without massive refactoring. The try/catch is however quite contained to parsing them.

@NicolasDorier NicolasDorier merged commit e3ad850 into MetacoSA:master Sep 6, 2024
4 of 6 checks passed
@knocte
Copy link
Contributor

knocte commented Sep 6, 2024

Sadly not possible when parsing a witscript and a transaction without massive refactoring. The try/catch is however quite contained to parsing them.

But cannot you know the type of the exception that would be thrown at least?

@Kukks Kukks deleted the bip322 branch September 6, 2024 09:11
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