Add design docs for auth/storage change detection and state mutation analysis

docs/issue-829-optimization-induced-auth-changes.md

@@ -0,0 +1,45 @@
+# Detect Optimization-Induced Authorization Changes
+
+Closes #829
+
+## Problem
+
+Autofix transformations (e.g. loop unrolling, function inlining, storage
+consolidation) can restructure a Soroban function body enough that a
+`require_auth()` / `require_auth_for_args()` call is dropped, moved past an
+early return, or applied to the wrong `Address` parameter. Today nothing in
+the autofix pipeline verifies that the *set* of enforced authorization checks
+is unchanged before a suggested fix is accepted.
+
+## Design
+
+Add a new rule module `packages/rules/src/security/authorization/`, mirroring
+the existing per-category layout under `packages/rules/src/security/`
+(`constructors/`, `signatures/`, `emergency/`). It contributes an
+`AuthorizationDiffRule` implementing the `SorobanRule` trait consumed by
+`SorobanRuleEngine::add_rule` in `packages/rules/src/soroban/rule_engine.rs`.
+
+The rule runs twice: once against the pre-fix `SorobanContract` (from
+`packages/rules/src/soroban/parser.rs`) and once against the autofix
+candidate produced under `packages/autofix/validation/security/`. For each
+`SorobanFunction` in `SorobanImpl.functions` it extracts the ordered list of
+`require_auth`/`require_auth_for_args` call sites and the `Address` argument
+each guards, then diffs the two lists per function name:
+
+- an entry present before and absent after -> authorization removed
+- a guard whose argument changed -> authorization scope changed
+- a guard now reachable only after another branch/return -> ordering hazard
+
+Diff results are emitted as `RuleViolation`s with
+`ViolationSeverity::Critical` (via `crate::{RuleViolation, ViolationSeverity}`,
+same pattern as `packages/rules/src/soroban/analyzer.rs`), which the autofix
+validator in `packages/autofix/validation/security/` uses to reject the
+candidate fix outright rather than merely warning.
+
+## Acceptance Criteria
+
+- [ ] `AuthorizationDiffRule` compares pre/post authorization call sites per function
+- [ ] Removed, moved, or retargeted `require_auth*` calls are flagged
+- [ ] Violations use `ViolationSeverity::Critical` and block the autofix, not just warn
+- [ ] Rule is registered in `SorobanRuleEngine::with_default_rules`
+- [ ] Regression fixtures cover: dropped auth, auth after early return, auth on wrong `Address`

docs/issue-830-optimization-induced-storage-changes.md

@@ -0,0 +1,45 @@
+# Detect Optimization-Induced Storage Changes
+
+Closes #830
+
+## Problem
+
+Storage-focused optimizations (batching writes, hoisting reads out of loops,
+collapsing redundant `get`/`set` pairs — see
+`packages/rules/src/optimization/storage/multiple_storage_reads.rs`) can
+accidentally drop a required `env.storage().persistent().set(...)` call, or
+introduce a new write that was never in the original contract. Neither case
+is currently caught before a fix is applied to Soroban contracts, unlike the
+existing rent-cost check in `packages/rules/soroban/src/storage-rent-check.ts`,
+which only looks at TTL/rent, not write-set equivalence.
+
+## Design
+
+Add `packages/rules/src/soroban/storage_diff.rs`, registered in
+`packages/rules/src/soroban/mod.rs` next to `inefficient_storage.rs`. It
+walks each `SorobanFunction.raw_definition` (available on the AST types in
+`packages/rules/src/soroban/mod.rs`) for calls into
+`storage().persistent()`, `storage().temporary()`, and `storage().instance()`
+`set`/`remove`/`extend_ttl` invocations, recording, per function: the storage
+tier, the operation kind, and the key expression where it is a literal or a
+simple identifier (falling back to "unanalyzable" otherwise, consistent with
+requirement "track affected keys where analyzable").
+
+The pre-fix and post-fix operation lists are diffed the same way as the
+autofix validation flow used for `packages/autofix/validation/storage/`:
+
+- key present pre-fix, absent post-fix -> removed write (flagged)
+- key absent pre-fix, present post-fix -> new/unexpected write (flagged)
+- key present in both, tier or op kind changed -> persistence-behavior change
+
+Findings surface as `RuleViolation`s (`crate::RuleViolation`) with severity
+`Warning` for unanalyzable keys and `Error` for confirmed removed/added
+writes, consumed by `packages/autofix/validation/storage/`.
+
+## Acceptance Criteria
+
+- [ ] Storage operation lists extracted per function for persistent/temporary/instance tiers
+- [ ] Removed writes flagged as `ViolationSeverity::Error`
+- [ ] Newly introduced writes flagged as `ViolationSeverity::Error`
+- [ ] Non-literal/unanalyzable keys reported at `Warning` rather than silently skipped
+- [ ] Fixtures added under `packages/rules/soroban/tests/` mirroring `storage-rent-check.spec.ts`

docs/issue-831-state-mutation-analyzer.md

@@ -0,0 +1,47 @@
+# Implement Soroban State Mutation Analyzer
+
+Closes #831
+
+## Problem
+
+`packages/rules/src/soroban/analyzer.rs` (`SorobanAnalyzer`) currently
+checks struct/impl-level issues (unused state variables, inefficient field
+types, unbounded loops) but has no notion of *how often* or *along which
+paths* a contract mutates its own state. Frequent or redundant mutations
+increase Soroban resource-fee consumption without being visible in the
+existing checks. Issues #829/#830 need a shared mutation model to diff
+against; this issue provides that foundation.
+
+## Design
+
+Add a new module tree `packages/rules/src/soroban/state/` (`mod.rs` plus
+`mutation_analyzer.rs`), following the same layout as the existing
+`packages/rules/src/soroban/memory/` module
+(`mod.rs` re-exporting `InefficientBytesAllocationRule`).
+
+`mutation_analyzer.rs` defines `StateMutationAnalyzer`, invoked from
+`SorobanAnalyzer::analyze_implementation` in `analyzer.rs` alongside the
+existing `analyze_function` calls. For each `SorobanFunction` it walks
+`raw_definition` for `storage().*().set(...)` / `.update(...)` calls and
+builds a `MutationPath { function: String, key: String, tier: StorageTier,
+call_site_line: usize }` per occurrence, then groups paths by `(function,
+key)` to compute a mutation count.
+
+A companion `SorobanRule` impl, `StateMutationRule`, registered via
+`SorobanRuleEngine::add_rule` in `rule_engine.rs`, reports:
+
+- functions with more than N (configurable, default 3) mutations of the
+  same key on one execution path -> "repeated update" violation
+- the single most expensive mutation path per function (by tier: persistent
+  > instance > temporary) surfaced as an informational `RuleViolation`
+
+This `MutationPath` model is the shared input both issue #830 (storage
+diffing) and issue #832 (redundant-write detection) build on.
+
+## Acceptance Criteria
+
+- [ ] `StateMutationAnalyzer` collects per-function, per-key mutation paths across all storage tiers
+- [ ] Repeated updates to the same key on one path are counted and reported
+- [ ] Most expensive mutation path per function is identified and surfaced
+- [ ] `StateMutationRule` registered in `SorobanRuleEngine::with_default_rules`
+- [ ] Tests added under `packages/rules/soroban/tests/` covering single, repeated, and multi-tier mutations

docs/issue-832-redundant-state-mutations.md

@@ -0,0 +1,46 @@
+# Detect Redundant Soroban State Mutations
+
+Closes #832
+
+## Problem
+
+Writing a storage value that is identical to its current value still incurs
+a full `SSTORE`-equivalent Soroban resource-fee charge, with no state benefit.
+`packages/rules/src/soroban/redundant_clone.rs` already detects a related but
+distinct pattern (unnecessary `.clone()` calls); no equivalent rule exists
+for redundant storage writes, and conditional mutations (writes inside an
+`if`/`match` arm) are not handled by any existing rule.
+
+## Design
+
+Add `packages/rules/src/soroban/state/redundant_mutation.rs` alongside
+`mutation_analyzer.rs` from #831, reusing its `MutationPath` output rather
+than re-parsing the contract. `RedundantMutationRule` (another `SorobanRule`,
+registered next to `StateMutationRule` in `rule_engine.rs`) inspects each
+`MutationPath` and:
+
+- compares the value expression being written against the last known read or
+  write of the same key on the same path (literal/identifier comparison,
+  same "analyzable vs. not" fallback used in issue #830's `storage_diff.rs`)
+  and flags an exact match as a redundant write
+- for conditional mutations (call site inside an `if`/`else`/`match` arm,
+  detected from indentation/brace depth relative to the arm in
+  `raw_definition`), flags the write only when *every* branch writes the
+  same value, since a value-changing branch makes the write non-redundant
+- for confirmed redundant writes, generates an optimization suggestion
+  string (e.g. "skip write: value unchanged from prior write at line N"),
+  attached to the `RuleViolation.suggestion` field used elsewhere in
+  `packages/rules/src/optimization/`
+
+Violations use `ViolationSeverity::Info` (optimization, not correctness) so
+they surface as suggestions rather than blocking autofix, consistent with
+how `packages/rules/src/optimization/storage/multiple_storage_reads.rs`
+reports its findings.
+
+## Acceptance Criteria
+
+- [ ] Redundant writes (same key, same value as prior write/read) detected via `MutationPath` from #831
+- [ ] Conditional/branch-guarded writes analyzed per-branch, not flagged unless all branches redundant
+- [ ] Optimization suggestion string generated and attached per violation
+- [ ] `RedundantMutationRule` registered in `SorobanRuleEngine::with_default_rules`
+- [ ] Tests added under `packages/rules/soroban/tests/` covering unconditional and conditional redundant writes