Skip to content

Discharge implies-check consequent obligations under the antecedent via SMT#4150

Closed
ehildenb wants to merge 4 commits into
masterfrom
implies-smt-discharge
Closed

Discharge implies-check consequent obligations under the antecedent via SMT#4150
ehildenb wants to merge 4 commits into
masterfrom
implies-smt-discharge

Conversation

@ehildenb
Copy link
Copy Markdown
Member

@ehildenb ehildenb commented Jun 2, 2026

When booster's implies check matches the consequent against the antecedent and finds leftover consequent obligations, it previously passed them to evaluateConstraints with no path-condition context — effectively asking SMT whether each obligation holds "under the assumption of itself". Anything depending on the antecedent's constraints couldn't be closed, and unresolved obligations aborted the RPC with Aborted "unknown constraints" (the source of 57 kore-implies handoffs observed in KEVM recover-mode). This change discharges those obligations under the antecedent, mirroring the rewrite path's two-stage approach (Booster.Pattern.Rewrite.checkRequires): each obligation is first simplified with the antecedent in knownPredicates (so K-level vocabulary like #rangeAddress unfolds), then any non-TrueBool residue is SMT-closed against the antecedent's predicates and substitution. Unresolvable obligations now return an indeterminate valid: false verdict instead of an RPC error, so a recover-mode client escalates to kore rather than failing.

Changes:

  • Booster.Pattern.Implies: replace the MatchSuccess FIXME path with simplify-under-antecedent + SMT.checkPredicates; map IsValid→implies, IsInvalid→does-not-imply, IsUnknown InconsistentGroundTruth→implies (vacuous antecedent), IsUnknown _→indeterminate.
  • kore-rpc-types: add an optional indeterminate :: Maybe Bool field to ImpliesResult (omitted when absent) so clients can distinguish a decisive valid: false from an indeterminate one.
  • kore/JsonRpc: set the new field to Nothing on kore-side results (always decisive).
  • Add test-implies-smt RPC integration tests (bound-weakening, does-not-imply, vacuous-antecedent, address-bound) and update test-implies2 goldens for the cases that previously aborted.
  • Some refactors to clean up the Implies check file a bit: mkResult helps build implication results in a standard way, and we pre-compute several terms that are used on several branches of the implication check.

Validation:

  • Builds clean under -Wall -Werror (stack, GHC 9.6.5); fourmolu-clean.
  • Booster unit tests pass (721 tests).
  • Tested on KEVM:
    • Does not cause any notable overall performance regressions.
    • Does not change any passing tests, and does not cause any tests to be passable with booster-dev that were not before.
    • Does clear some Implies checks directly in booster that were not cleared before and relied on Kore (collecting data on that now).

ehildenb and others added 3 commits June 2, 2026 16:44
…ate field on ImpliesResult

Plumbing-only change: introduces the new optional 'indeterminate ::
Maybe Bool' field on 'ImpliesResult' and sets it to 'Nothing' at every
existing construction site.

  * 'kore-rpc-types': field added with doc comment; 'OmitNothingFields'
    keeps the JSON output unchanged when absent.
  * 'Booster.Pattern.Implies': 'doesNotImply'' and 'implies'' record
    constructions extended with 'indeterminate = Nothing'.
  * 'Kore.JsonRpc': positional 'ImpliesResult' constructions extended
    with trailing 'Nothing' (kore checks are always decisive).

No call site sets the field to a non-'Nothing' value yet; that wiring
lands in subsequent commits.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…, scripts/booster-integration-tests: add failing RPC tests for implies SMT discharge

Pins booster's current `implies` behaviour on four cases where the
antecedent and consequent share an identical term skeleton (so the
matcher returns `MatchSuccess`) but differ in their attached
path-condition predicates. The leftover obligations are direct
consequences of the antecedent under linear-integer reasoning, so an
SMT-aware discharge step under the antecedent would close them as
`valid : true` (or `valid : false` for the doesNotImply case).

Cases:
  - 001-bound-weakening:   X<100 ⇒ X<1000
  - 002-does-not-imply:    X<100 ⇒ X<10        (counterexample exists)
  - 003-vacuous-antecedent: X<10 ∧ X>20 ⇒ anything
  - 004-address-bound:     0<=X<pow160 ⇒ X<pow256  (KEVM-faithful)

`response-N.booster-dev` documents today's `Aborted: unknown constraints`
on cases 001/002/004 (the FIXME on `Implies.hs:161`'s `MatchSuccess`
branch — antecedent constraints are not assumed when discharging the
leftover consequent predicates). Case 003 already passes on booster-dev
via earlier bottom-detection. `response-N.json` shows the
`kore-rpc-booster` ground truth that the fix should match.

The `.kore` definition is built on demand from `implies-smt.k` via the
shipped `.kompile` script (mirrors the `escalate-partial-match` pattern).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…implies2}: discharge consequent obligations under antecedent via SMT

Resolves the FIXME in 'checkImpliesMatchTerms''s 'MatchSuccess' branch.
Previously the leftover consequent predicates were passed to
'evaluateConstraints' with no known context — i.e. SMT was asked
whether each obligation is valid 'under the assumption of itself',
which trivially holds for self-discharging predicates but cannot
close anything that depends on the antecedent's path condition (the
57 'kore-implies' handoffs observed in KEVM recover-mode).

The new flow mirrors the rewrite path's two-stage discharge
('Booster.Pattern.Rewrite.checkRequires'):

  1. Simplify each leftover obligation under the antecedent context
     ('patL.constraints' + 'asEquations patL.substitution') via
     'simplifyConstraint' — this unfolds K-level vocabulary
     ('#rangeAddress', boolean structure, ...) so SMT sees only
     SMT-tractable predicates.
  2. SMT-close any non-'TrueBool' residue with
     'SMT.checkPredicates solver antecedentPreds patL.substitution
     residue':

     - 'IsValid'                            → 'implies'
     - 'IsInvalid'                          → 'doesNotImply'
     - 'IsUnknown InconsistentGroundTruth'  → 'implies' (vacuous antecedent)
     - 'IsUnknown _'                        → 'doesNotImplyIndeterminate'

The previous hard 'Aborted "unknown constraints"' path is gone:
unresolvable obligations now return the 'Unsure' verdict
('valid: false, indeterminate: Just True'), so a recover-mode client
escalates to kore rather than seeing an RPC error.

Test response updates (booster-dev expected outputs, flipped from
the prior 'Aborted' verdict):

  - test-implies-smt/response-{001-bound-weakening,004-address-bound}.booster-dev:
      'Aborted' → 'valid: true' (SMT closes under antecedent).
  - test-implies-smt/response-002-does-not-imply.booster-dev:
      'Aborted' → 'valid: false, indeterminate: true' (SMT
      'ImplicationIndeterminate' — both polarities sat).
  - test-implies2/response-{consequent-constraint,refutation-1,
      refutation-3,refutation-4}.booster-dev:
      'Aborted' → 'valid: false, indeterminate: true' (same
      mechanism on the existing implies2 cases that triggered the
      bug pre-fix).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ehildenb ehildenb force-pushed the implies-smt-discharge branch from c894061 to 4662846 Compare June 2, 2026 18:10
…a single mkResult builder

The four result builders ('implies'', 'doesNotImply'',
'doesNotImplyIndeterminate', and the implied 'implies'/'doesNotImply'
wrappers) each spelled out the same 'pure . Right . Implies . ImpliesResult'
record, differing only in 'valid', 'condition', and 'indeterminate'. Collapse
the shared skeleton into one 'mkResult' helper so the record lives in a single
place — a future field addition (as 'haskellLogEntries' was upstream) becomes a
one-line change rather than four. No behavioural change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ehildenb
Copy link
Copy Markdown
Member Author

ehildenb commented Jun 3, 2026

Going to close this for now, will re-open when it has a measureable effect on KEVM (other issues are in the way).

@ehildenb ehildenb closed this Jun 3, 2026
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.

1 participant