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

InvalidWitnessVersion error display should convert fe to u8 #162

Closed
apoelstra opened this issue Jan 3, 2024 · 0 comments · Fixed by #165
Closed

InvalidWitnessVersion error display should convert fe to u8 #162

apoelstra opened this issue Jan 3, 2024 · 0 comments · Fixed by #165

Comments

@apoelstra
Copy link
Member

Right now if we have an invalid witness version we print the field element directly, which will print it in the bech32 alphabet. This is confusing because, for example, the number 17 will be printed as "3".

We should convert it to a number by adding to_u8 before printing.
tcharding added a commit to tcharding/rust-bech32 that referenced this issue Jan 7, 2024
@tcharding
Improve error display for invalid witness version
d9bb03e 
We have two errors that cover invalid witness version, for one we print
the field element which can be confusing for some values and for the
other we omit the invalid version all together - we can do better.

Print the field element as well as the integer value when displaying the
two invalid witness version errors.

Fix: rust-bitcoin#162
@tcharding tcharding mentioned this issue Jan 7, 2024
Merged
apoelstra added a commit that referenced this issue Jan 9, 2024
@apoelstra
Merge #165: Improve error display for invalid witness version
84b2c50 
e988f34 Improve error display for invalid witness version (Tobin C. Harding)
d692972 Add rustfmt::skip to Display impl (Tobin C. Harding)

Pull request description:

  We have two errors that cover invalid witness version, for one we print the field element which can be confusing for some values and for the other we omit the invalid version all together - we can do better.

  Print the field element as well as the integer value when displaying the two invalid witness version errors.

  Fix: #162

ACKs for top commit:
  apoelstra:
    ACK e988f34

Tree-SHA512: 93d666f396c536d6348cf793b74c27363eeacec02c0a7a4ec53786778f7c64c393bad90fe10b718f872d61deb355de4b18f4fd8a66e35a1f53e09ced2cbe7c0b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve error display for invalid witness version tcharding/rust-bech32
1 participant
@apoelstra