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.

Sign up for GitHub

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

Jump to bottom

Bad enumeration of script errors #1285

Closed
roconnor-blockstream opened this issue Nov 9, 2023 · 2 comments · Fixed by #1286
Closed

Bad enumeration of script errors #1285

roconnor-blockstream opened this issue Nov 9, 2023 · 2 comments · Fixed by #1286

Comments

@roconnor-blockstream
Copy link
Contributor

elements/src/script/script_error.h

Line 87 in 20c5630

// ELEMENTS:

has incorrectly inserted new Elements errors into this enumeration. All the new elements errors need to go before SCRIPT_ERR_ERROR_COUNT, which isn't actually a real error. Then SCRIPT_ERR_LAST needs to be reset to SCRIPT_ERR_ERROR_COUNT, as it originally was.

AFAICT the only consequences of this error are for the fuzzer.

I'm not sure that having both SCRIPT_ERR_LAST and SCRIPT_ERR_ERROR_COUNT makes sense, but that is how it is in Bitcoin at the moment.

cc @uncomputable
@uncomputable
Copy link
Contributor

The error exists on master: https://github.com/ElementsProject/elements/blob/master/src/script/script_error.h#L85

@roconnor-blockstream roconnor-blockstream mentioned this issue Nov 9, 2023
Merged
@uncomputable
Copy link
Contributor

This fuzz test is the only place where SCRIPT_ERR_ERROR_COUNT is used. SCRIPT_ERR_LAST is not used at all.

I think it is safest to keep both errors in case Bitcoin Core starts using either in the future.

uncomputable added a commit to uncomputable/elements that referenced this issue Nov 10, 2023
@uncomputable
Script: Move SCRIPT_ERR_COUNT
651cc22 
SCRIPT_ERR_COUNT is a pseudo error that returns the number of errors.
It must always be last in the enum:
ElementsProject#1285
@uncomputable uncomputable mentioned this issue Nov 10, 2023
Merged
@delta1 delta1 closed this as completed in df8e572 Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Script: Move SCRIPT_ERR_COUNT uncomputable/elements
2 participants
@uncomputable @roconnor-blockstream