Skip to content

Conversation

@t-bast
Copy link
Member

@t-bast t-bast commented Nov 5, 2025

While 73 bytes der-encoded signatures aren't standard (they're only possible with high-S signatures), miners could accept such signatures.

Most of our tests use 72 bytes der-encoded signatures because we will never generate 73 bytes der-encoded signatures ourselves. But it's more safe to use a 73 bytes signature in our weight estimation because:

  • it's what the BOLTs recommend
  • it's what other implementations use (e.g. LDK) which means they may reject our interactive-tx attempts by thinking we're under-paying fees

While 73 bytes der-encoded signatures aren't standard (they're only
possible with high-S signatures), miners could accept such signatures.

Most of our tests use 72 bytes der-encoded signatures because we will
never generate 73 bytes der-encoded signatures ourselves. But it's more
safe to use a 73 bytes signature in our weight estimation because:

- it's what the BOLTs recommend
- it's what other implementations use (e.g. LDK) which means they may
  reject our interactive-tx attempts by thinking we're under-paying
  fees
@t-bast t-bast requested a review from sstone November 5, 2025 13:53
@t-bast t-bast merged commit bfe34ab into master Nov 5, 2025
1 check passed
@t-bast t-bast deleted the der-encoding-size branch November 5, 2025 15:47
@morehouse
Copy link

FYI: LDK seems to be changing it the other direction: lightningdevkit/rust-lightning#4208

@t-bast
Copy link
Member Author

t-bast commented Nov 6, 2025

FYI: LDK seems to be changing it the other direction: lightningdevkit/rust-lightning#4208

Damn 😅, but it doesn't matter in our case since we allow the remote node to slightly underpay the estimated fees (to take into account cases where coin selection returns a solution that doesn't have a change output but slightly underpays), we won't reject splicing attempts because of this increased weight. We will slightly overpay the feerate compared to real signature size, but that's negligible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants