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

BIP345: fix OP_SUCCESS188 hex value #1745

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Conversation

shesek
Copy link
Contributor

@shesek shesek commented Jan 11, 2025

OP_SUCCESS188 is 0xbc, not 0xbb.

`OP_SUCCESS188` is `0xbc`, not `0xbb`.
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK c532b53, OP_SUCCESS188 is constrained with new rules in this BIP to implement OP_VAULT_RECOVER and 188 is 0xbc.

@jonatack
Copy link
Member

Pinging @jamesob and @instagibbs for approval.

@jonatack jonatack added the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label Jan 11, 2025
@jonatack jonatack changed the title Fix typo in BIP 345 Fix OP_SUCCESS188 hex vslue of typo in BIP 345 Jan 12, 2025
@jonatack jonatack changed the title Fix OP_SUCCESS188 hex vslue of typo in BIP 345 BIP345: fix OP_SUCCESS188 hex value Jan 12, 2025
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.

I was wondering whether it was obvious whether OP_SUCCESS187 was meant and 0xbb was correct, or whether OP_SUCCESS188 was correct and 0xbc was the typo.

As the line corrected in this PR follows another prior mention that introduces the two new opcodes, and the introductions of the paragraphs repeat except in the opcode name, it seems clear that the lower mention is a copy error from the previous header, and I agree that that this is correct.

The tapscript opcodes OP_SUCCESS187 (0xbb) and
OP_SUCCESS188 (0xbc) are constrained with new rules
to implement OP_VAULT and OP_VAULT_RECOVER,
respectively.

@murchandamus
Copy link
Contributor

The reference implementation also confirms this interpretation:
image

Merging.

@murchandamus murchandamus merged commit 1f41c17 into bitcoin:master Jan 13, 2025
4 checks passed
@jonatack jonatack removed the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants