Skip to content

Summaries for inner_test_validate_owner#1001

Open
mariaKt wants to merge 11 commits intofeature/p-tokenfrom
mk/lemmas-inner_test_validate_owner
Open

Summaries for inner_test_validate_owner#1001
mariaKt wants to merge 11 commits intofeature/p-tokenfrom
mk/lemmas-inner_test_validate_owner

Conversation

@mariaKt
Copy link
Copy Markdown
Collaborator

@mariaKt mariaKt commented Mar 24, 2026

Summary

This PR adds lemma rules that summarize the execution of expected_validate_owner_result
and inner_test_validate_owner for the validate_owner proof. Common helpers are extracted
in VALIDATE-OWNER-COMMON, and intercept rules and case handlers are in dedicated
modules (EXPECTED-VALIDATE-OWNER-RESULT-P-TOKEN-LEMMA and
INNER-TEST-VALIDATE-OWNER-P-TOKEN-LEMMA). The new file includes detailed documentation
of the case analysis, module structure, and design decisions, so I chose not to repeat them here.

Development process

Cases and their requirements were identified through proof inspection with Claude, e.g.
examining split constraints, node functions, and branching structure to map each proof path to
a lemma rule. This provided a large amount of automation in generating the rules.

Manual effort was needed to guide the process in the right direction:

  • how to make rules match properly (use of seqstrict, sorts, priorities, when to pattern match a constructor to identify what to match and which projections to use)
  • how to reduce duplication and code size blowup from the N (registered signers) × tx_signer count dimensions
    (list-based helpers, manual heating/cooling instead of per-count dispatch).

Fail rules

The inner_test_validate_owner lemma includes fail rules for each error case, i.e.
rules that match when the result argument does not match the expected error
value. These produce a #ValidateOwnerAssertFailed error that causes execution
to get stuck immediately with a diagnostic message. These fail
paths were not observed in the passing proofs, I had to pass a mismatched/unexpected
Result in order to observe them. However, they are included so that if
they do occur, the proof gets stuck at the lemma level rather than needing to
proceed through the small-step semantics to eventually get stuck on a
missing core::panicking::assert_failed_inner body. Both outcomes are stuck,
but the lemma version is faster and produces a clearer error.

@mariaKt
Copy link
Copy Markdown
Collaborator Author

mariaKt commented Mar 24, 2026

I want to let CI run once, but then I think I should have the requires and imports commented out before we merge it so this doesn't use the new module every time. It makes sense to me, since these do not affect the mir-semantics tests in any way, and the code increase might lead to more time per test. That should become clear after CI is done.

To use them, we can uncomment in the solana-token mir-semantics dependency, create a new branch feature/p-token-lemmas on top of feature/p-token with this as the only difference, and branch proofs-with-lemmas branch with feature/p-token-lemmas to run the proofs. We would need to do the opposite anyway if they are left enabled and we want to run the proofs without the lemmas, so I don't see this as a big issue. Let me know what you prefer.

@Stevengre
Copy link
Copy Markdown
Contributor

I like the proofs-with-lemmas idea. One quick question about CI: do you mean waiting to see whether test_validate_owner_multisig passes?

@mariaKt
Copy link
Copy Markdown
Collaborator Author

mariaKt commented Mar 25, 2026

I like the proofs-with-lemmas idea. One quick question about CI: do you mean waiting to see whether test_validate_owner_multisig passes?

I meant the CI with commit 5653ced, just to have a record that the lemmas do not cause any issues with any test in this test suite (as expected, since none would match).

Stevengre added a commit that referenced this pull request Apr 1, 2026
Adapted from the p-token version (PR #1001). Key differences from
the p-token lemma:

- spl-token AccountInfo uses raw Aggregate values instead of p-token's
  PAccountAccount/PAcc wrappers, so the intercept rules extract
  individual fields (key, is_signer, owner) from AccountInfo using
  field projections (field 0, 5, 3 respectively)
- Case rules match pre-extracted Values directly instead of
  destructuring PAccount wrappers
- Uses _IS_SIGNER wildcard for symbolic bool handling (spl-token's
  is_signer may be a symbolic BoolVal)
- Case 1b handles keys-match + Err(Custom(4)) path via LHS pattern
  match (avoids booster sort injection errors from requires ==K)
- Cases 8-10 (multisig signer-checking) fall through to small-step
  execution (sufficient for n==1)

Also fixes #forceSetLocal -> #setLocalValue in spl-token.md for
compatibility with origin/master which removed the redundant helper.

Proof result: test_validate_owner_multisig PASSED with 27 nodes
in 2886s (vs 123 nodes / 4877s without lemma, 41% speedup).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stevengre added a commit that referenced this pull request Apr 2, 2026
Adapted from the p-token version (PR #1001). Key differences from
the p-token lemma:

- spl-token AccountInfo uses raw Aggregate values instead of p-token's
  PAccountAccount/PAcc wrappers, so the intercept rules extract
  individual fields (key, is_signer, owner) from AccountInfo using
  field projections (field 0, 5, 3 respectively)
- Case rules match pre-extracted Values directly instead of
  destructuring PAccount wrappers
- Uses _IS_SIGNER wildcard for symbolic bool handling (spl-token's
  is_signer may be a symbolic BoolVal)
- Case 1b handles keys-match + Err(Custom(4)) path via LHS pattern
  match (avoids booster sort injection errors from requires ==K)
- Cases 8-10 (multisig signer-checking) fall through to small-step
  execution (sufficient for n==1)

Also fixes #forceSetLocal -> #setLocalValue in spl-token.md for
compatibility with origin/master which removed the redundant helper.

Proof result: test_validate_owner_multisig PASSED with 27 nodes
in 2886s (vs 123 nodes / 4877s without lemma, 41% speedup).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Stevengre Stevengre marked this pull request as ready for review April 3, 2026 14:17
This PR includes Jost's feedback for the issue I faced with the proof of
test_process_approve_multisig when enabling the lemmas for
inner_test_validate_owner.

The proof would be stuck on a path that could not be revolved by the
booster and reverted to the old backend. The reasons were:
- some functions needed to be total (explicit, even though their
conditions were mutually exclusive) or perserve-definedness.
- the Range constructor needed to be removed, so that the constraints
about its values could be resolved by the SMT solver.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 518ed56ee4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +723 to +724
// imports EXPECTED-VALIDATE-OWNER-RESULT-P-TOKEN-LEMMA
// imports INNER-TEST-VALIDATE-OWNER-P-TOKEN-LEMMA
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Import the new validate-owner lemma modules

These imports are commented out, so KMIR never includes the newly added EXPECTED-VALIDATE-OWNER-RESULT-P-TOKEN-LEMMA and INNER-TEST-VALIDATE-OWNER-P-TOKEN-LEMMA rules (and the file is also left commented in the requires section). In practice the commit has no effect on execution/proofs: calls still use the original small-step function bodies instead of the new summaries, so the intended proof acceleration and early assert-failure diagnostics are not applied.

Useful? React with 👍 / 👎.

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.

3 participants