rust-bitcoin / rust-bitcoin / 7732315706
83%

DEFAULT BRANCH: master
Ran
Jobs 1
Files 87
Run time 30s
Badge
Embed ▾
README BADGES
x

If you need to use a raster PNG badge, change the '.svg' to '.png' in the link

Markdown

Textile

RDoc

HTML

Rst
coverage: 84.148% (-0.2%) from 84.309%
7732315706

push

github

apoelstra 
Merge rust-bitcoin/rust-bitcoin#2417: Improve lock time errors

93dba898c Improve lock time errors (Martin Habovstiak)

Pull request description:

  The errors returned from various lock time functions had several issues. Among the obvious - `Error` being returned from all operations even when some of its variants were unreachable, there were subtle issues around error messages:

  * `ParseIntError` didn't contain information whether the parsed object is `Height` or `Time`.
  * Logically overflow and out-of-bounds should be the same thing but produced different error messages.
  * Mentioning integers is too technical for a user, talking about upper and lower bound is easier to understand.
  * When minus sign is present `std` reports it as invalid digit which is less helpful than saying negative numbers are not allowed.

  It is also possible that `ParseIntError` will need to be removed from public API during crate smashing or stabilization, so avoiding it may be better.

  This commit significantly refactors the errors. It adds separate types for parsing `Height` and `Time`. Notice that we don't compose them from `ParseIntError` and `ConversionError` - that's not helpful because they carry information that wouldn't be used when displaying which is wasteful. Keeping errors small can be important.

  It's also worth noting that exposing the inner representation could cause confusion since the same thing: out of bounds can be represented as an overflow or as a conversion error. So for now we conservatively hide the details and even pretend there's no `source` in case of overflow. This can be expanded in the future if needed.

  The returned errors are now minimal. `LockTime` parsing errors are currentlly unchanged.

  I can add `LockTime` changes in the same commit or separate within this PR if you want. Just wanted to push something for review before I go to sleep.

ACKs for top commit:
  apoelstra:
    ACK 93dba898c273179... (continued)

19328 of 22969 relevant lines covered (84.15%)

16834.91 hits per line

Jobs
ID Job ID Ran Files Coverage
1 7732315706.1 0
84.15
 GitHub Action Run
Source Files on build 7732315706
Detailed source file information is not available for this build.