-
Notifications
You must be signed in to change notification settings - Fork 492
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
Correct htlc witness size calculations #815
Conversation
Regardless of the previous commit, this didn't add up
Is this an artifact of signature size variation? I mean there is no "right" answer after you sign things vary a byte or two, but is the limit wrong because we didn't account for low-S implying signatures are never max size? |
We always assume they are the worst case size (73). Are you saying signatures never actually get that large? Interesting. |
Wow good catch, I don't understand why we considered this |
03-transactions.md
Outdated
HTLC-success) results in weights of: | ||
|
||
663 (HTLC-timeout) (666 with with option_anchor_outputs)) | ||
703 (HTLC-success) (706 with with option_anchor_outputs)) | ||
703 (HTLC-success) (706 with with option_anchor_outputs)) | ||
- (reeeally 702 and 705, but we use these numbers for historical reasons) |
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 re-did the calculations and I agree with what you found. |
940ff6d
to
fc5f7c4
Compare
fc5f7c4
to
9fdee1e
Compare
03-transactions.md
Outdated
HTLC-success) results in weights of: | ||
|
||
663 (HTLC-timeout) (666 with with option_anchor_outputs)) | ||
703 (HTLC-success) (706 with with option_anchor_outputs)) | ||
703 (HTLC-success) (706 with with option_anchor_outputs)) |
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.
with with
is probably a typo, right?
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.
most certainly
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.
ACK 2a8da1a
I noticed a few inconsistencies in the weight constants.
I guess implementations just use the hardcoded constants for
HTLC-timeout
andHTLC-success
when doing tx constructions (this is what lnd does), so I added a note here that we should continue using this value even though it is off by one.