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

taproot: ensure check for expiry script spend correctly handles annex #393

Merged
merged 1 commit into from
Oct 7, 2022
Merged

taproot: ensure check for expiry script spend correctly handles annex #393

merged 1 commit into from
Oct 7, 2022

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Oct 6, 2022

This PR extends poolscript.IsTaprootExpirySpend such that Taproot witness annex elements are correctly handled (ignored).

@ffranr ffranr force-pushed the taproot-add-check-for-annex-in-taproot-expiry-script-spend-path branch 3 times, most recently from 3d84842 to 08bbd3e Compare October 6, 2022 16:16
@ffranr ffranr marked this pull request as draft October 6, 2022 16:18
@ffranr ffranr force-pushed the taproot-add-check-for-annex-in-taproot-expiry-script-spend-path branch 2 times, most recently from 0523537 to 8ff3a0e Compare October 6, 2022 16:26
@ffranr ffranr marked this pull request as ready for review October 6, 2022 16:27
@ffranr ffranr requested a review from guggero October 6, 2022 17:00
positiveblue
positiveblue approved these changes Oct 7, 2022
View reviewed changes
Copy link
Contributor

@positiveblue positiveblue left a comment

Choose a reason for hiding this comment

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

Nice work @ffranr 🚀

I added a couple of format related nits.

PS: for my future me, here is some info

poolscript/script_test.go Show resolved Hide resolved
poolscript/script_test.go Outdated Show resolved Hide resolved
poolscript/script_test.go Outdated Show resolved Hide resolved
poolscript/script.go Show resolved Hide resolved
guggero
guggero approved these changes Oct 7, 2022
View reviewed changes
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Nice work, LGTM 🎉

Looks good, pending the remaining few nits.
The formatting nits are a bit annoying at first, I'm sure. But setting up the editor to use 8 spaces for tabs and adding a visual line at 80 characters should help.

poolscript/script.go Outdated Show resolved Hide resolved
@ffranr ffranr force-pushed the taproot-add-check-for-annex-in-taproot-expiry-script-spend-path branch 3 times, most recently from 2d40cbd to df02893 Compare October 7, 2022 13:32
@ffranr
Copy link
Contributor Author

ffranr commented Oct 7, 2022

Thank you @positiveblue and @guggero for the reviews.

I'm happy to hear about any formatting issues you guys might find, no problem at all. I've added an 80 character visual line to my IDE and I've set tabs to 8 characters.

If you notice any further formatting nits please do let me know.

@ffranr ffranr force-pushed the taproot-add-check-for-annex-in-taproot-expiry-script-spend-path branch from df02893 to 340ff1a Compare October 7, 2022 13:39
@ffranr ffranr requested a review from positiveblue October 7, 2022 13:47
@ffranr ffranr merged commit da88f02 into lightninglabs:master Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guggero guggero guggero approved these changes

@positiveblue positiveblue Awaiting requested review from positiveblue

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ffranr @guggero @positiveblue