From 9ee015c2f5b1b8a667c5764b2c8a1dc05b5b963a Mon Sep 17 00:00:00 2001
From: AbdulmujibOladayo <aderonkeadejare54@gmail.com>
Date: Fri, 28 Aug 2026 11:54:45 +0100
Subject: [PATCH] Add design docs closing #833, #834, #835, #836

---
 docs/issue-833-state-dependency-graph.md      | 49 ++++++++++++++++++
 docs/issue-834-dataflow-analysis.md           | 50 +++++++++++++++++++
 docs/issue-835-unused-computation-results.md  | 50 +++++++++++++++++++
 docs/issue-836-unnecessary-variable-copies.md | 50 +++++++++++++++++++
 4 files changed, 199 insertions(+)
 create mode 100644 docs/issue-833-state-dependency-graph.md
 create mode 100644 docs/issue-834-dataflow-analysis.md
 create mode 100644 docs/issue-835-unused-computation-results.md
 create mode 100644 docs/issue-836-unnecessary-variable-copies.md

diff --git a/docs/issue-833-state-dependency-graph.md b/docs/issue-833-state-dependency-graph.md
new file mode 100644
index 0000000..eacd423
--- /dev/null
+++ b/docs/issue-833-state-dependency-graph.md
@@ -0,0 +1,49 @@
+# Soroban State Dependency Graph (#833)
+
+## Problem
+Optimization and security rules currently reason about Soroban contract source
+line-by-line. There is no structural model of how `env.storage()` reads,
+writes, and the computations between them relate, so rules cannot tell
+whether a write is derived from a prior read, or whether two storage keys
+are correlated. This blocks safe, non-local optimizations (#834-#836 depend
+on it).
+
+## Prior Art In This Repo
+- `src/analysis/dependency-graph/` builds a **contract-level** graph
+  (`SorobanDependencyAnalyzer`) tracking cross-contract calls, not state.
+- `src/graphs/stellar/call-graph/` (`CallGraph`, `CallGraphNode`) builds a
+  **function-level** call graph — same layout to mirror at state level.
+- `packages/rules/soroban/src/storage-rent-check.ts` parses storage calls
+  line-by-line; the pattern the new analyzer feeds for storage-aware rules.
+
+## Design
+New module: `src/analysis/state-graph/` (sibling to `dependency-graph/`,
+following the same `index.ts` + `types.ts` + `*.spec.ts` layout).
+
+**Types** (`src/analysis/state-graph/types.ts`):
+```ts
+export interface StateNode {
+  id: string; kind: 'read' | 'write' | 'computation';
+  storageType?: 'persistent' | 'instance' | 'temporary';
+  key?: string; functionName: string; line: number;
+}
+export interface StateEdge {
+  source: string;
+  target: string;
+  kind: 'reads-into' | 'writes-from' | 'computed-from';
+}
+export interface StateDependencyGraph {
+  nodes: StateNode[];
+  edges: StateEdge[];
+}
+```
+
+**Entry point:** `StateDependencyGraphBuilder.build(filePath, source): StateDependencyGraph`,
+exported from `src/analysis/state-graph/index.ts`, consumed by
+`packages/rules/soroban/src/` rules and by the dataflow analyzer in #834.
+
+## Acceptance Criteria
+- [ ] `StateDependencyGraph` type and builder exist under `src/analysis/state-graph/`
+- [ ] Reads (`.get`) and writes (`.set`) become distinct node kinds
+- [ ] Edges connect writes to the reads/computations they depend on per function
+- [ ] Graph object is importable by rule modules (no CLI/reporting coupling)
diff --git a/docs/issue-834-dataflow-analysis.md b/docs/issue-834-dataflow-analysis.md
new file mode 100644
index 0000000..c82e43b
--- /dev/null
+++ b/docs/issue-834-dataflow-analysis.md
@@ -0,0 +1,50 @@
+# Soroban Dataflow Analysis (#834)
+
+## Problem
+Existing Soroban rules (e.g. `packages/rules/soroban/src/storage-rent-check.ts`,
+`rules/stellar/optimization/detect-inefficient-symbol-usage.ts`) match local
+regex patterns on single lines. They cannot detect optimizations that span
+multiple statements — e.g. a value defined in one line, copied in another,
+and never used in a third. This issue asks for a proper def-use dataflow
+analyzer that #835 and #836 build their detections on top of.
+
+## Prior Art In This Repo
+- `src/analysis/state-graph/` (#833) already models storage reads/writes as a
+  graph; dataflow analysis extends this to **local variables**, not just
+  storage.
+- `src/graphs/stellar/call-graph/call-graph-generator.ts` shows the existing
+  pattern for a `*-generator.ts` class producing a typed graph with a
+  `.spec.ts` alongside it — the dataflow analyzer should follow the same
+  shape.
+
+## Design
+New module: `src/analysis/dataflow/` (`dataflow-analyzer.ts`, `types.ts`,
+`dataflow-analyzer.spec.ts`).
+
+**Types** (`src/analysis/dataflow/types.ts`):
+```ts
+export interface DefUseEntry {
+  variable: string;
+  definedAt: { line: number; functionName: string };
+  uses: Array<{ line: number; kind: 'read' | 'arg' | 'return' }>;
+  sourceKind: 'literal' | 'param' | 'storage-read' | 'call-result' | 'copy';
+}
+export interface DataflowResult {
+  entries: DefUseEntry[];
+  unusedDefinitions: DefUseEntry[]; // uses.length === 0
+}
+```
+
+**Entry point:** `SorobanDataflowAnalyzer.analyze(source: string): DataflowResult`,
+exported from `src/analysis/dataflow/index.ts`. Walks `let`/`let mut`
+bindings, function parameters, and `return` statements using the same
+line-scanning approach as `storage-rent-check.ts`, tracking simple
+straight-line control flow (branches treated as separate paths, no loop
+fixpoint required for v1 per the issue's "basic control-flow paths"
+requirement).
+
+## Acceptance Criteria
+- [ ] `DataflowResult` produced per-function with defs and uses populated
+- [ ] Variable definitions, uses, function args, and return values all tracked
+- [ ] `unusedDefinitions` correctly flags a def with zero recorded uses
+- [ ] `dataflow-analyzer.spec.ts` covers straight-line and simple branching code
diff --git a/docs/issue-835-unused-computation-results.md b/docs/issue-835-unused-computation-results.md
new file mode 100644
index 0000000..afadf02
--- /dev/null
+++ b/docs/issue-835-unused-computation-results.md
@@ -0,0 +1,50 @@
+# Detect Unused Soroban Computation Results (#835)
+
+## Problem
+Contract functions sometimes compute a value (arithmetic, a method call, a
+storage read) and bind it to a variable that is never subsequently read,
+returned, or passed as an argument. Each such computation still consumes gas
+on-chain with zero effect. This needs to be flagged as a lint rule, built on
+top of the dataflow analyzer from #834.
+
+## Prior Art In This Repo
+- Rule shape: `packages/rules/soroban/src/storage-rent-check.ts` defines
+  `SorobanStorageRentCheckRule` with a `public static readonly RULE_ID`
+  and a `.analyze(sourceCode): Warning[]` method, registered via
+  `packages/rules/soroban/src/index.ts` (`export * from './...'`).
+- Warning shape: `StorageRentWarning` in the same file (`line`, `message`,
+  `suggestion` fields) is the field convention this rule's output should
+  match.
+
+## Design
+New file: `packages/rules/soroban/src/unused-computation-check.ts`,
+registered by adding `export * from './unused-computation-check';` to
+`packages/rules/soroban/src/index.ts`.
+
+```ts
+export interface UnusedComputationWarning {
+  line: number;
+  variable: string;
+  message: string;
+  suggestion: string;
+}
+
+export class SorobanUnusedComputationRule {
+  public static readonly RULE_ID = 'soroban-unused-computation';
+  public analyze(sourceCode: string): UnusedComputationWarning[] { /* ... */ }
+}
+```
+
+**Inputs:** `DataflowResult` from `src/analysis/dataflow/` (#834), specifically
+its `unusedDefinitions` list, filtered to `sourceKind !== 'param'` (function
+parameters that go unused are a separate, existing concern) and
+`sourceKind !== 'literal'` (cheap constants aren't worth flagging).
+**Outputs:** one `UnusedComputationWarning` per unused non-trivial definition,
+with `suggestion` telling the developer to remove the binding or use `let _ =`.
+
+## Acceptance Criteria
+- [ ] `SorobanUnusedComputationRule.RULE_ID = 'soroban-unused-computation'`
+- [ ] Rule consumes `DataflowResult.unusedDefinitions`, not its own regex scan
+- [ ] Storage reads and call-results with zero uses are reported; literals and
+      unused fn params are excluded
+- [ ] Each warning includes source `line` and an actionable `suggestion`
diff --git a/docs/issue-836-unnecessary-variable-copies.md b/docs/issue-836-unnecessary-variable-copies.md
new file mode 100644
index 0000000..cbe01ee
--- /dev/null
+++ b/docs/issue-836-unnecessary-variable-copies.md
@@ -0,0 +1,50 @@
+# Detect Unnecessary Soroban Variable Copies (#836)
+
+## Problem
+Rust/Soroban code sometimes clones or reassigns a value into a new binding
+that is used only once, immediately after, in place of the original
+(`let b = a.clone(); do_thing(b);` where `a` is otherwise unused after). This
+adds avoidable clone/copy overhead. The detector must be conservative: types
+with real ownership semantics (`Address`, moved `Vec`/`Map`/`Bytes`) must not
+be flagged unless the copy is provably redundant, per the issue's explicit
+"avoid ownership-related false positives" requirement.
+
+## Prior Art In This Repo
+- Rule shape and registration follow `packages/rules/soroban/src/storage-rent-check.ts`
+  / `packages/rules/soroban/src/index.ts`, same as #835.
+- Data source: the def-use chains from `src/analysis/dataflow/` (#834) —
+  a "copy chain" is simply a `DefUseEntry` whose `sourceKind === 'copy'`
+  (i.e. `let x = y;` or `let x = y.clone();`) where the source variable `y`
+  has no further uses after the copy point.
+
+## Design
+New file: `packages/rules/soroban/src/unnecessary-copy-check.ts`, registered
+via `packages/rules/soroban/src/index.ts`.
+
+```ts
+export interface UnnecessaryCopyWarning {
+  line: number;
+  copiedVariable: string;
+  originalVariable: string;
+  message: string;
+  suggestion: string;
+}
+
+export class SorobanUnnecessaryCopyRule {
+  public static readonly RULE_ID = 'soroban-unnecessary-copy';
+  public analyze(sourceCode: string): UnnecessaryCopyWarning[] { /* ... */ }
+}
+```
+
+**Detection rule (conservative v1):** only flag `let x = y.clone();` /
+`let x = y;` where (a) `y` has no uses in the dataflow graph after the copy
+line within the same function, and (b) `y`'s type is not one of the
+Soroban-owned handle types (`Address`, `Env`, `BytesN`) obtained from a
+function parameter — parameters are excluded entirely for v1 to avoid
+ownership false positives, matching the issue's explicit acceptance bar.
+
+## Acceptance Criteria
+- [ ] `SorobanUnnecessaryCopyRule.RULE_ID = 'soroban-unnecessary-copy'`
+- [ ] Copy chains derived from `src/analysis/dataflow/` def-use data, not regex
+- [ ] Function-parameter-sourced values are never flagged (false-positive guard)
+- [ ] `.spec.ts` covers a flagged clone-then-unused case and a not-flagged reused case