Description

Some input would be appreciated!

• StaticCheckErrorKind: 98 total variants
• CheckErrorKind: 87 total variants
• Shared Variants (in both): 66
  • Shared Variants used in common code paths (CommonCheckErrorKind): 26
• Unique to StaticCheckErrorKind: 34
• Unique to CheckErrorKind: 22

Why Separating the two?

Separating static from "execution-time" Check errors would make the code explicit about what the contract analysis guarantees vs what can only be checked at runtime. Currently, all errors seem possible at any time.

More importantly, this split enables a second phase of cleanup: many CheckErrorKind at runtime are actually safeguards for unreachable code that's already protected by contract analysis. Even the relative tests bypass the contract_analysis to stimulate the unreachable code paths. Once separated, we can identify these and replace them with CheckError::Expects (or a new variant that accepts the transaction but marks it as failed for consensus safety).

We currently have 79 CheckErrorKind variants (58 shared with static). After this cleanup, we'd likely reduce this to ~40 actual runtime errors, making the codebase significantly clearer about what can actually fail at runtime.

WIP: currently the CommonCheckErrorKind enum is public, but should become private and invisible to users of the clarity-types and clarity crates (can't be truly private because the clarity crate also needs it. However, this is solvable by making it public but ensuring all public functions only return StaticCheckError or CheckError)

While trying to achieve this clean public API, I've tried multiple approaches, but each has significant problems (uuugly):

Duplicate public functions

impl ListTypeData {
    pub fn new_list(...) -> Result<ListTypeData, CheckError> {
        Self::inner_new_list(...).map_err(CheckError::from)
    }

    pub fn new_list_static(...) -> Result<ListTypeData, StaticCheckError> {
        Self::inner_new_list(...).map_err(StaticCheckError::from)
    }
    
    fn inner_new_list(...) -> Result<ListTypeData, CommonCheckError> {
        // actual implementation
    }
} 

Problems:

• Ugly and awkward
• Duplicates signatures for tens of functions
• Higher-level shared functions force inner_* functions to become public. (e.g. a function in clarty has to return a CommonCheckError and use a function in clarity-types)

Generic error type

impl ListTypeData {
    pub fn new_list<E: From<CommonCheckError>>(
        entry_type: TypeSignature, 
        max_len: u32
    ) -> Result<ListTypeData, E> {
        // implementation
    }
}

Problems:

• Sometimes the user needs to explicitly state the type, even when not needed e.g. ListTypeData :::new_list::<CheckError>(...).unwrap()
• Functions returning CommonCheckError can't call these generic functions

Open questions:

• Is there a cleaner architectural pattern I'm missing?
• Or should I accept that mixing CommonCheckError, StaticCheckError, and CheckError in the public API is acceptable?
• Is this split actually providing enough value in terms of type safety and code clarity to justify the complexity?

Looking for feedback on whether to push through with a better approach, or abandon this split entirely.

Applicable issues

• fixes #

Additional info (benefits, drawbacks, caveats)

Checklist

• Test coverage for new or modified code paths
• Changelog is updated
• Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
• New clarity functions have corresponding PR in clarity-benchmarking repo