-
Notifications
You must be signed in to change notification settings - Fork 3
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 padding for single txn blocks #17
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah looks good! This is something that I would not have caught on my own, so thanks for the PR.
// after the only txn of this block. | ||
// If there are no withdrawals, then the dummy proof will be prepended to the | ||
// actual txn. | ||
match has_withdrawals { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use if { … } else { … }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually find that using a match
on a bool
is usually more readable (there might be something wrong with me, idk 🙃) than an if / else
. I think using a match is a bit less common though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah not sure why I went for match, maybe an implicit preference as Brendan said, happy to change though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote for keeping match
We were still prepending to the actual txn the dummy payload for blocks containing only 1 txn, but we were then updating that payload if the block contained withdrawals.
We now do a case disjunction for single_txn blocks based on the presence of withdrawals: