From 4fafd7ff92c1483517fca862a44987ae4cdfc8f0 Mon Sep 17 00:00:00 2001 From: CleanDev-Fix <219162456+CleanDev-Fix@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:29:46 -0400 Subject: [PATCH] docs: expand acceptance criteria audit evidence --- .github/PULL_REQUEST_TEMPLATE.md | 13 ++++---- CONTRIBUTING.md | 4 +++ README.md | 4 +-- docs/self-review-template.md | 8 ++--- docs/traceability-table.md | 55 +++++++++++++++++--------------- 5 files changed, 47 insertions(+), 37 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index cbf4629..972e29e 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -87,17 +87,18 @@ make verify ## Traceability Table - -| Criterion ID | Criterion Text | Changed Function(s) | Test(s) | Edge Cases Covered | Status | -|---|---|---|---|---|---| -| | | | | | | +| Criterion ID | Criterion Text | Implementation Evidence | Test Evidence | Documentation Impact | Edge Cases Covered | Status | +|---|---|---|---|---|---|---| +| | | | | | | | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 780c641..9bac4ed 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -109,6 +109,10 @@ Every pull request must fill in the **[PR template](.github/PULL_REQUEST_TEMPLAT - **Security considerations** — a plain-language description of security impact plus the per-section security checklist for any PR that touches contract logic (see `docs/security-checklist.md`). - **Commands run** — confirmation that `make verify` passes (format, Clippy, workspace tests, and release WASM build), or equivalently that `cargo fmt --check`, `cargo clippy --tests -- -D warnings`, and `cargo test --workspace` all pass locally. - **CI status** — all CI checks green before requesting review. +- **Acceptance criteria audit** — the canonical + [Traceability Table](docs/traceability-table.md) maps every criterion to + implementation evidence, test evidence, documentation impact, edge cases, + and status. Additional guidance: diff --git a/README.md b/README.md index f911e64..9d20134 100644 --- a/README.md +++ b/README.md @@ -214,7 +214,7 @@ stellar-pocketpay-contracts/ - [Admin & Emergency Mechanism Threat Model](docs/admin-pause-threat-model.md) – Security analysis of malicious admin, compromised admin, accidental pause, and blocked-withdrawal scenarios. - [Failure Mode Catalogue](docs/failure-mode-catalogue.md) – Summary of safe-failure behavior, expected errors, affected functions, and related tests for vault operations. - [Contributor Self-Review Template](docs/self-review-template.md) – Copy-paste checklist covering behaviour, tests, CI, security, edge cases, and docs impact — fill it in before opening a PR. -- [Traceability Table Guide](docs/traceability-table.md) – Standard format for mapping PR changes to issue acceptance criteria, with worked examples. +- [Traceability Table Guide](docs/traceability-table.md) – Canonical acceptance criteria audit template mapping implementation, tests, documentation impact, edge cases, and status, with worked examples. - [Payment-Period Conduct Guidance](docs/payment-period-conduct.md) – Expectations for how contributors raise payment-status questions, and how GrantFox's evaluation process relates to this repository's review process. --- @@ -278,7 +278,7 @@ Every pull request must use the **[PR template](.github/PULL_REQUEST_TEMPLATE.md - A reference to the issue being fixed (`Closes #N`) - A list of contract functions added, modified, or removed - A description of tests added or updated -- A **[traceability table](docs/traceability-table.md)** mapping each acceptance criterion to changed functions, tests, and edge cases +- A **[traceability table](docs/traceability-table.md)** mapping each acceptance criterion to implementation evidence, test evidence, documentation impact, edge cases, and status - A security considerations section (with checklist for contract changes) - Confirmation that `make verify` passes (or equivalently `cargo fmt --check`, `cargo clippy --tests -- -D warnings`, and `cargo test --workspace`) - CI green before requesting review diff --git a/docs/self-review-template.md b/docs/self-review-template.md index 8bbce7c..d36eae7 100644 --- a/docs/self-review-template.md +++ b/docs/self-review-template.md @@ -42,8 +42,8 @@ reviewer. - [ ] Every acceptance criterion in the issue is addressed by this PR (or I've explained in the PR description which ones are deferred and why). - [ ] I've filled in the [traceability table](traceability-table.md) mapping - each acceptance criterion to the function(s), test(s), and edge cases - that satisfy it. + each acceptance criterion to implementation evidence, test evidence, + documentation impact, edge cases, and status. - [ ] No `TODO`, `FIXME`, or `HACK` markers remain in the code this PR touches. - [ ] The change is scoped to one issue — unrelated refactors or fixes are split into a separate PR. @@ -129,8 +129,8 @@ table get filled in for reviewers. incomplete work looks like. This template is the condensed, fill-in-the-blank version of that gate. - [Traceability Table Guide](traceability-table.md) — the standard format for - mapping acceptance criteria to functions and tests; fill this in as part of - the Behaviour section above. + the canonical acceptance criteria audit; fill this in as part of the + Behaviour section above. - [Contract Contributor Security Checklist](security-checklist.md) and [Invariant Test Checklist](invariant-test-checklist.md) — the detailed checklists the Security and Edge Cases sections above summarize. diff --git a/docs/traceability-table.md b/docs/traceability-table.md index aeac910..a10863d 100644 --- a/docs/traceability-table.md +++ b/docs/traceability-table.md @@ -1,8 +1,9 @@ # Traceability Table Guide -This document defines a standard traceability table format for PocketPay -contract pull requests. The table maps every acceptance criterion from the -linked issue to the contract functions, tests, and edge cases that satisfy it. +This document defines the canonical acceptance criteria audit template for +PocketPay contract pull requests. The table maps every acceptance criterion +from the linked issue to implementation, tests, documentation impact, and edge +cases. ## Why traceability matters @@ -23,10 +24,10 @@ confirm that each deliverable maps cleanly to the original requirements. ## Table format Every contract PR that references an issue with acceptance criteria must include -a **Traceability Table** in the PR description. The table uses six columns: +a **Traceability Table** in the PR description. The table uses seven columns: -| Criterion ID | Criterion Text | Changed Function(s) | Test(s) | Edge Cases Covered | Status | -|---|---|---|---|---|---| +| Criterion ID | Criterion Text | Implementation Evidence | Test Evidence | Documentation Impact | Edge Cases Covered | Status | +|---|---|---|---|---|---|---| ### Column definitions @@ -34,8 +35,9 @@ a **Traceability Table** in the PR description. The table uses six columns: |---|---| | **Criterion ID** | A short label matching the issue's acceptance criteria, e.g. `AC-1`, `AC-2`. Number them in the order they appear in the issue. | | **Criterion Text** | The exact wording of the acceptance criterion copied from the issue. Keep it verbatim so reviewers can cross-reference without switching tabs. | -| **Changed Function(s)** | The contract function(s) added, modified, or removed to satisfy this criterion. Use the Rust function name (e.g. `extend_lock`, `lock_funds`). Write `—` for documentation-only criteria. | -| **Test(s)** | The test function name(s) and file path(s) that prove this criterion is met. Use the format `test_name` (`path/to/file.rs`). List multiple tests on separate lines within the cell. | +| **Implementation Evidence** | The function, logic, configuration, or documentation section that satisfies the criterion. Use precise names and file paths, such as `extend_lock` in `contracts/savings_vault/src/lib.rs` or the updated heading in `docs/page.md`. | +| **Test Evidence** | The test function name(s) and file path(s) that prove this criterion is met. Use the format `test_name` (`path/to/file.rs`). List multiple tests on separate lines within the cell. Write `N/A` with a reason for documentation-only criteria. | +| **Documentation Impact** | Name every README or `docs/` page updated for this criterion. Write `N/A` with a short reason when documentation impact was assessed and no change is needed. | | **Edge Cases Covered** | Boundary conditions, failure paths, and adversarial scenarios explicitly tested for this criterion. Examples: "zero amount rejected", "expired pause auto-clears", "unauthorized caller blocked". Write `—` if no edge cases apply (e.g. pure documentation criteria). | | **Status** | One of: ✅ Met, ⚠️ Partial (with explanation), ❌ Not Met (with justification for deferral). Every row should be ✅ before the PR is marked ready for review. | @@ -57,13 +59,13 @@ during `extend_lock`"*. ### Traceability table -| Criterion ID | Criterion Text | Changed Function(s) | Test(s) | Edge Cases Covered | Status | -|---|---|---|---|---|---| -| AC-1 | `extend_lock` rejects extensions exceeding `MaxLockDurationSecs` | `extend_lock` | `test_extend_lock_exceeds_max_duration` (`src/test/lock_extension.rs`) | Extension exactly at max (accepted); extension 1 second over max (rejected); extension when no max is configured (accepted) | ✅ Met | -| AC-2 | `extend_lock` rejects durations shorter than `MinLockDurationSecs` | `extend_lock` | `test_extend_lock_below_min_duration` (`src/test/lock_extension.rs`) | Extension exactly at min (accepted); extension 1 second below min (rejected) | ✅ Met | -| AC-3 | Existing `extend_lock` tests continue to pass | — | All existing tests in `src/test/lock_extension.rs` | No regressions in happy-path or unauthorized-caller tests | ✅ Met | -| AC-4 | Error codes use `ContractError`, not bare panics | `extend_lock` | `test_extend_lock_exceeds_max_duration`, `test_extend_lock_below_min_duration` | Verify error variant matches `ContractError::LockDurationOutOfRange` | ✅ Met | -| AC-5 | `docs/lock-extension-design.md` updated | — | — | — | ✅ Met | +| Criterion ID | Criterion Text | Implementation Evidence | Test Evidence | Documentation Impact | Edge Cases Covered | Status | +|---|---|---|---|---|---|---| +| AC-1 | `extend_lock` rejects extensions exceeding `MaxLockDurationSecs` | `extend_lock` in `contracts/savings_vault/src/lib.rs` | `test_extend_lock_exceeds_max_duration` (`src/test/lock_extension.rs`) | N/A — public behavior was already documented | Extension exactly at max (accepted); extension 1 second over max (rejected); extension when no max is configured (accepted) | ✅ Met | +| AC-2 | `extend_lock` rejects durations shorter than `MinLockDurationSecs` | `extend_lock` in `contracts/savings_vault/src/lib.rs` | `test_extend_lock_below_min_duration` (`src/test/lock_extension.rs`) | N/A — public behavior was already documented | Extension exactly at min (accepted); extension 1 second below min (rejected) | ✅ Met | +| AC-3 | Existing `extend_lock` tests continue to pass | Existing `extend_lock` behavior remains unchanged | All existing tests in `src/test/lock_extension.rs` | N/A — regression verification only | No regressions in happy-path or unauthorized-caller tests | ✅ Met | +| AC-4 | Error codes use `ContractError`, not bare panics | `extend_lock` returns `ContractError::LockDurationOutOfRange` | `test_extend_lock_exceeds_max_duration`, `test_extend_lock_below_min_duration` | N/A — error reference was already current | Verify error variant matches `ContractError::LockDurationOutOfRange` | ✅ Met | +| AC-5 | `docs/lock-extension-design.md` updated | Updated enforcement-rules section in `docs/lock-extension-design.md` | N/A — documentation-only criterion | `docs/lock-extension-design.md` updated | — | ✅ Met | --- @@ -81,33 +83,36 @@ during `extend_lock`"*. ### Documentation-only criteria -Use `—` in the Changed Function(s), Test(s), and Edge Cases columns. Set -Status to ✅ and note the doc file path in the Criterion Text or a brief note. +Point Implementation Evidence to the exact file and section, put the changed +file in Documentation Impact, and use `N/A — documentation-only criterion` for +Test Evidence. Use `—` for Edge Cases Covered when none apply. ### Multiple tests per criterion -List each test on a separate line within the same table cell using `
` or a -line break inside the markdown cell: +List each test on a separate line within the same Test Evidence cell using +`
` or a line break inside the markdown cell: ```markdown -| AC-1 | ... | `deposit` | `test_deposit_happy_path` (`src/test/deposit.rs`)
`test_deposit_minimum_amount` (`src/test/deposit.rs`) | ... | ✅ Met | +| AC-1 | ... | `deposit` in `contracts/savings_vault/src/lib.rs` | `test_deposit_happy_path` (`src/test/deposit.rs`)
`test_deposit_minimum_amount` (`src/test/deposit.rs`) | N/A — behavior already documented | ... | ✅ Met | ``` ### Multiple functions per criterion -Similarly, list each function name separated by commas or line breaks: +Similarly, list each function name and file in Implementation Evidence, +separated by commas or line breaks: ```markdown -| AC-2 | ... | `lock_funds`, `extend_lock` | ... | ... | ✅ Met | +| AC-2 | ... | `lock_funds`, `extend_lock` in `contracts/savings_vault/src/lib.rs` | ... | N/A — behavior already documented | ... | ✅ Met | ``` ### Partial or deferred criteria -If a criterion cannot be fully met in the current PR, set Status to ⚠️ Partial -and explain what remains. Link to a follow-up issue if one has been created: +If a criterion cannot be fully met in the current PR, set Status to ⚠️ Partial, +explain what remains in Implementation Evidence, record any deferred docs in +Documentation Impact, and link to a follow-up issue if one has been created: ```markdown -| AC-3 | ... | ... | ... | ... | ⚠️ Partial — max-lock enforcement deferred to #42 | +| AC-3 | ... | Partial implementation; max-lock enforcement deferred to #42 | Existing tests only | Follow-up docs deferred with #42 | ... | ⚠️ Partial — follow-up required | ``` ### Relationship to other PR template sections