Goals:
- All errors should be forward-compatible
- To avoid
unreachable!() calls functions should only return error variants that can actually happen; catch-all errors may be provided to help people who don't need this.
- Each error should contain all relevant information about the operation that failed
- Error information should not be lost in conversions except for explicit methods
- If the error information is large enough to slow down programs as demonstrated by benchmarks and motivated by real-life use cases we can put it on heap or omit it depending on circumstances. RC/beta versions should help people find out soon enough.
Motivation/Rationale
- Forward-compatibility will give us confidence to mark the crate (or its parts after split) 1.0. 1.0 is one of the green flags when picking a crate, a mark of professionalism and dedication to keep the API.
- Relying on compiler instead of humans scales better with size of the project and number of contributors. This is an often-mentioned reason to use Rust. Eliminating impossible branches also saves a bit of CPU time.
- From experience, lack of information makes debugging hard. We want to make debugging easy so the development is efficient, enjoyable and possibly saves people from losing satoshis.
- Same as above.
- Some crates put error information on heap to make moving around faster. Judging between heap allocation cost and memcpy cost is difficult without knowing the use cases and differs depending on the code. Avoiding heap is simpler (the default in Rust) thus doing anything else is premature optimization
Tasks
Ordered for efficiency, not strict.
Forward-compatibility decisions:
- If we're sure no variant can ever be removed from an enum-based error, keep it enum if we're unsure make it
struct Foo(InnerFoo)
- If we're sure no more variants will be added keep it as is, otherwise add
#[non_exhaustive]
- For each variant perform similar analysis on the contents of its variants and either hide them in struct or expose them. Hint: if the error type comes from non-stable dependency it MUST be encapsulated. There's a possible conversion trick involving
TryFrom and a wrapper to avoid branching for some dependency versions
- Check if important information is missing and add it in line with the above steps.
From<T> for Error or vice-versa may only be implemented it T is from a stable crate and our error representation is guaranteed to contain T or be contained in T respectively.
Unresolved questions
Should all errors be suffixed with Error? I guess no. (Moved to task list due to discussion below.)
- When should errors go to separate module? My suggestion: only if error-related code is large. "Large" is currently not defined.
- Naming of internal errors:
Inner/`Internal/other prefix/suffix (not affecting stability)
Goals:
unreachable!()calls functions should only return error variants that can actually happen; catch-all errors may be provided to help people who don't need this.Motivation/Rationale
Tasks
Ordered for efficiency, not strict.
secp256k1/bitcoin_hashesand use conversions from there - this is low-hanging fruit, obvious cleanup to be done first Use infallible conversions from hashes tosecp256k1::Message#824genioor NIH similar traits to avoid.expect()calls.ErrorAudit error types code base wide #2101Forward-compatibility decisions:
struct Foo(InnerFoo)#[non_exhaustive]TryFromand a wrapper to avoid branching for some dependency versionsFrom<T> for Erroror vice-versa may only be implemented itTis from a stable crate and our error representation is guaranteed to containTor be contained inTrespectively.Unresolved questions
Should all errors be suffixed with(Moved to task list due to discussion below.)Error? I guess no.Inner/`Internal/other prefix/suffix (not affecting stability)