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

fix: op_return predicate evaluation #416

Merged
merged 3 commits into from
Oct 2, 2023

Conversation

qustavo
Copy link
Contributor

@qustavo qustavo commented Sep 15, 2023

Description

Handles OP_RETURN predicates correctly.

Breaking change?

Potentially yes, but unlikely.
Currently there is a OP_RETURN handler, that matches the output.script_pubkey against the expression set in the predicate, although this implementation does not confirm with the documentation, someone might be relying on it

Checklist

  • All tests pass
  • Tests added in this PR (if applicable)

fixes #385 #388

@qustavo
Copy link
Contributor Author

qustavo commented Sep 15, 2023

Still a draft, since I'm not so sure if the tests on the PR are the right way to go.

@qustavo qustavo force-pushed the fix/opreturn_evaluation branch 3 times, most recently from 18484fd to 247c2d7 Compare September 19, 2023 09:19
@qustavo qustavo force-pushed the fix/opreturn_evaluation branch from 247c2d7 to 51eb307 Compare September 20, 2023 17:00
@qustavo qustavo marked this pull request as ready for review September 20, 2023 17:34
@qustavo
Copy link
Contributor Author

qustavo commented Sep 24, 2023

@lgalabru can you have a look at why CI is failing?

@lgalabru
Copy link
Contributor

CI is failing because it's being triggered from your fork, that does not have knowledges of the secrets required to push a build to Dockerhub. Safe to ignore!
Feel free to tag reviewers when you think the PR is ready!

@qustavo
Copy link
Contributor Author

qustavo commented Sep 25, 2023

For some reason I can't set reviewers, probably a permission issue.
image

Copy link
Contributor

@lgalabru lgalabru left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

Awesome work, thanks @qustavo!

@qustavo
Copy link
Contributor Author

qustavo commented Oct 2, 2023

@lgalabru @MicaiahReid is there anything else left to do for this PR to get merged?

@lgalabru lgalabru merged commit d9fe536 into hirosystems:develop Oct 2, 2023
1 check failed
@lgalabru
Copy link
Contributor

lgalabru commented Oct 2, 2023

Done! thank you @qustavo 🙏

@qustavo qustavo deleted the fix/opreturn_evaluation branch October 2, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

OP_RETURN starting with"!a" is not found when OP_RETURN !aĆ[a+UQ{d^½2 is present
3 participants