Skip to content

Conversation

@federico-stacks
Copy link
Contributor

@federico-stacks federico-stacks commented Feb 5, 2026

Description

This PR converts all RuntimeCheckErrorKind variants marked as functionally unreachable in runtime_analysis_tests.rs to ExpectsAcceptable, with the following exceptions:

  • ExecutionTimeExpired is kept as-is to maintain alignment with the CostErrors variants.
  • SupertypeTooLarge is converted to ExpectsRejectable to preserve the logic in RuntimeCheckErrorKind::rejectable().

NOTE: There are 56 affected variants. To simplify the review process, each variant conversion has been split into its own commit.

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • For new Clarity features or consensus changes, add property tests (see docs/property-testing.md)
  • Changelog is updated
  • Required documentation changes (e.g., rpc/openapi.yaml for RPC endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo

@federico-stacks federico-stacks force-pushed the aac/runtime-expect-acceptable branch from 1afca09 to 41c7347 Compare February 9, 2026 14:59
@federico-stacks federico-stacks changed the title [WIP] Aac/runtime expect acceptable [WIP] aac: runtime check error to expect acceptable Feb 9, 2026
@federico-stacks federico-stacks marked this pull request as ready for review February 9, 2026 16:03
@federico-stacks federico-stacks changed the title [WIP] aac: runtime check error to expect acceptable aac: runtime check error to expect acceptable Feb 9, 2026
jacinta-stacks
jacinta-stacks previously approved these changes Feb 9, 2026
Copy link
Contributor

@brice-stacks brice-stacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGMT 😄

Copy link
Contributor

@francesco-stacks francesco-stacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! Just before approving, I was wondering if we already thought about a mechanism we should have in place to immediately find out if an ExpectAcceptable error is ever committed to the chain. Should we discuss this with the devops team?

@federico-stacks
Copy link
Contributor Author

lgtm! Just before approving, I was wondering if we already thought about a mechanism we should have in place to immediately find out if an ExpectAcceptable error is ever committed to the chain. Should we discuss this with the devops team?

there is a discussion about this here: https://github.com/stx-labs/core-epics/issues/119 (that also mention the monitoring)

TLDR: these ExpectAcceptable are just here for convenience and will be converted to Unreachable before the fork.

Copy link
Contributor

@francesco-stacks francesco-stacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

@federico-stacks federico-stacks added this pull request to the merge queue Feb 12, 2026
Merged via the queue into stacks-network:develop with commit 7c95397 Feb 12, 2026
632 of 636 checks passed
@federico-stacks federico-stacks deleted the aac/runtime-expect-acceptable branch February 12, 2026 15:00
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