Skip to content

#23 add contract release checklist fix - #170

Open
onakijames-droid wants to merge 2 commits into
Aegis-RWA:mainfrom
onakijames-droid:#23-Add-contract-release-checklist-FIX
Open

#23 add contract release checklist fix#170
onakijames-droid wants to merge 2 commits into
Aegis-RWA:mainfrom
onakijames-droid:#23-Add-contract-release-checklist-FIX

Conversation

@onakijames-droid

Copy link
Copy Markdown

CLOSE #23 Add contract release checklist

Description

Adds a repeatable release gate for contract changes, plus automation so the
mechanical parts cannot rot.

Finding. The repo had no release process. CONTRIBUTING.md covered PR
workflow but nothing covered cutting a release, so a contract change could ship
without anyone verifying compliance enforcement, storage compatibility, or event
compatibility.

The deeper problem is that this codebase has tight, invisible coupling across
a language boundary
. The monitoring/ service decodes contract events in
JavaScript. A single #[contractevent] change silently breaks four things —
the monitor's decoder (ACTION_SCHEMA), the dashboard tag style, the dashboard
details() renderer, and analytics volume math — with no compile error
anywhere
, because the seam is Rust → JS.

A prose-only checklist would not catch that, and would rot. So this PR ships
both halves:

  1. docs/release-checklist.md — 10 sections covering every area the issue
    asked for, grounded in this codebase (real test names, real DataKey
    variants, real panic strings), plus a versioning policy and a copy-paste
    sign-off record.
  2. scripts/check-release-compat.sh — enforces the mechanical items:
    storage layout, contract↔monitor event parity, namespace pinning, dashboard
    render coverage, and spec documentation coverage.

Standing risk register (new)

Writing the Roles and Storage sections required reading the contract
closely, which surfaced five genuine pre-existing properties. These are now
documented as R1–R5 in §9, each with an owner prompt and re-confirmation
requirement at every release:

# Risk Location
R1 initialize() has no require_auth() — any address can call it first on a fresh deployment and seize admin src/lib.rs
R2 No admin rotation (set_admin absent) — lost key ⇒ redeploy contract-wide
R3 Whitelist is append-only, no revocation — an address cannot be off-boarded for compliance src/compliance.rs
R4 No extend_ttl() anywhere, despite docs/architecture.md claiming entries are "rent-exempted appropriately" — archived balances become unreachable contract-wide
R5 distribute_yield is a mock: emits an event, moves no value src/asset.rs

These are documented, not fixed. This is a Documentation issue, and R1/R3
are design decisions for maintainers rather than something to patch silently.
R1 in particular deserves a follow-up issue.

Key files

File Change
docs/release-checklist.md (new, 314 lines) The checklist: 10 sections, risk register R1–R5, versioning policy, sign-off record
scripts/check-release-compat.sh (new, 212 lines) Drift detector across 4 categories, 10 assertions
Makefile New compat-check (L42) and release-check (L56) targets; verify now depends on compat-check (L47)
README.md New ## Releasing section linking the checklist
CONTRIBUTING.md New ## Releasing section for maintainers

New public interfaces

No contract code changed. No new functions, events, error codes, or roles.
New developer-facing entry points only:

  • make compat-check — compatibility drift detection (exit 1 on drift)
  • make release-check — full gate + WASM sha256 + manual sections to sign off
  • BASELINE_DATAKEYS in scripts/check-release-compat.sh:30 — editing this
    line is the storage-migration decision, deliberately made explicit

Related Issues

Fixes #

Completion Table

Acceptance Criterion Status Implementation Evidence Test Evidence Documentation Impact
AC 1: Release checklist is added Complete docs/release-checklist.md — 10 sections (L32–L259) covering all nine suggested areas: tests (§1), storage (§2), events (§3), errors (§4), roles (§5), compliance (§6), SDK (§7), dashboard (§8), audit (§9); plus versioning policy (L261) and sign-off record (L276) make verify exit 0. Section coverage audited against the issue's suggested list — 9/9 present New: docs/release-checklist.md
AC 2: Compliance enforcement checks are included Complete §6 (L162–L186): mint/transfer whitelist gating, sender and receiver checks, "re-read every is_whitelisted call site", insufficient-balance revert, WhitelistAdd audit trail, instant-drain alert rule. Ties to real tests test_mint_to_non_whitelisted_fails, test_transfer_insufficient_balance_fails. Scope boundary stated: ledger-level only Referenced tests pass — cargo test 9/9. Compliance risk R3 (no revocation) documented at L255 New: checklist §6. N/A for docs/contract-spec.md — enforcement semantics unchanged by this PR
AC 3: Storage and event compatibility checks are included Complete §2 (L66–L93) storage: DataKey variants, variant reordering (positional encoding hazard), value-type changes, durability per key, no-upgrade-entrypoint reality, TTL/archival (R4). §3 (L95–L120) events: handler parity, topic order/arity, data format, ≤4 topic limit, fixture regeneration. Automated by scripts/check-release-compat.sh §1 (L53) and §2 (L76) 4 negative tests, all detect drift: NT-1 added DataKey::FeeBps → 1 FAIL, exit 1. NT-2 new burn event → 4 FAILs (undecodable + unstyled + unrenderable + undocumented). NT-3 namespace → aegis2 → 1 FAIL. NT-4 removed spec entry → 1 FAIL. Clean tree restored → exit 0 after each New: checklist §2, §3. N/A for docs/architecture.md — storage model unchanged; R4 flags its existing rent claim as unimplemented
AC 4: SDK and dashboard compatibility checks are included Complete §7 (L188–L205) SDK: soroban-sdk pin (26.0.0), signature/arg-order stability, spec documentation, binding regeneration, i128 exactness, deprecated-API avoidance. §8 (L207–L225) dashboard: tag style + details() coverage, VALUE_ACTIONS analytics, defaults.js rule validity, field-name parity, historical replay decode, contract-ID update. Automated at scripts/check-release-compat.sh §3 (L136) and §4 (L173) make compat-check — 10/10 PASS on clean tree. NT-2 proves dashboard gaps are caught (tag style + details() both FAIL). Monitoring suite 106/106 New: checklist §7, §8. N/A for monitoring/README.md — service behaviour unchanged
AC 5: Security and audit checks are included Complete §9 (L227–L259): independent diff review, threat-model delta, re-entrancy, secret scanning, dependency review (cargo tree / npm audit), audit-status recording, testnet rehearsal + on-chain Init verification, standing risk register R1–R5 (L253–L257) requiring per-release re-confirmation make verify exit 0 (fmt + clippy -D warnings + tests + build + monitoring). R1 mitigation is operational and stated as a §5 checkbox New: checklist §9 + risk register
AC 6: README links to the checklist Complete README.md ## Releasing section links docs/release-checklist.md and documents the three make targets. Also linked from CONTRIBUTING.md for maintainers All 6 relative links inside the checklist verified to resolve (../README.md, ../CONTRIBUTING.md, ../monitoring/README.md, ../scripts/check-release-compat.sh, contract-spec.md, architecture.md) — 0 dead links Modified: README.md, CONTRIBUTING.md

No incomplete criteria.

Detailed Traceability Mapping

Acceptance Criteria Implementation Storage & State Changes Events Emitted Test Coverage Security/Safety Controls
AC 1: Checklist added docs/release-checklist.md None — documentation only. No contract source modified None — no contract code changed make verify exit 0; contract 9/9, monitoring 106/106 unchanged from baseline Establishes a gate where none existed; §9 requires diff review + audit-status recording before tagging
AC 2: Compliance enforcement Checklist §6 None None Binds to existing test_mint_to_non_whitelisted_fails (src/test.rs:L273), test_transfer_insufficient_balance_fails (src/test.rs:L287) Forces per-release re-verification that whitelist gating is unbypassable; R3 (no revocation) flagged as an open compliance limitation
AC 3: Storage & event compat Checklist §2/§3 + scripts/check-release-compat.sh L53, L76 No layout change. BASELINE_DATAKEYS = "Admin Balance TotalSupply Whitelist" (script L30) records the current layout; the script fails on any add/remove/rename None emitted; the script verifies the existing 5 (init, wl_add, mint, transfer, yield) still have off-chain handlers NT-1 (storage drift → exit 1), NT-2 (event drift → 4 FAILs), NT-3 (namespace → FAIL); monitoring/tests/onchain-compat.test.js 10/10 Catches the silent Rust→JS drift class; warns on variant reordering, which corrupts existing persistent data rather than erroring; R4 flags unmanaged TTL as a balance-loss risk
AC 4: SDK & dashboard compat Checklist §7/§8 + script L136, L173 None None make compat-check 10/10 PASS; NT-2 proves unstyled/unrenderable actions are caught; monitoring 106/106 Prevents an event silently rendering as ? on the compliance dashboard — a monitoring blind spot during an incident
AC 5: Security & audit Checklist §9 + risk register None None make verify exit 0 — fmt clean, clippy 0 warnings, 9/9 + 106/106 Secret scanning, dependency review, testnet rehearsal, on-chain Init verification (R1 mitigation), explicit audited/unaudited disclosure
AC 6: README link README.md, CONTRIBUTING.md None None 6/6 relative links resolve; 0 dead Ensures the gate is discoverable — an unreferenced checklist is not a control

Storage summary: this PR changes no storage, no events, and no contract
code.
DataKey::{Admin, Whitelist, Balance, TotalSupply} and all five events
are untouched. The only contract-adjacent artifact is the recorded baseline
the drift checker compares against.

Type of Change

  • Documentation update — the primary deliverable
  • Chore (build tools)Makefile targets + scripts/check-release-compat.sh
  • Bug fix
  • New feature
  • Breaking change

⚠️ One behavioural change reviewers should weigh: verify now depends on
compat-check (Makefile:L47), so every PR — not just releases — fails on
storage or event drift. That is stricter than the issue required. It is the
right default (drift is cheapest to catch at PR time), but if you prefer PRs
lenient and gating only at release, deleting compat-check from the verify:
line is a one-word change. Flagging explicitly rather than slipping it in.

PR Evidence Checklist

1. Issue Reference

  • Links to the issue above (Fixes #<!-- number --> — fill in on open).

2. Implementation Summary

  • Summary provided in Description.
  • Key files listed with per-file descriptions.
  • No new contract functions, events, error codes, or roles (documentation +
    tooling only). New developer-facing make targets documented.

3. Tests Added or Justification

  • Covered. This is a documentation/tooling change, so the "tests" are the
    script's own negative tests — verifying it detects drift, not merely
    that it passes:

    Test Injected drift Result
    NT-1 Added DataKey::FeeBps to src/lib.rs 1 FAIL, exit 1 — baseline diff + migration hint
    NT-2 Added a burn #[contractevent] 4 FAILs — undecodable, unstyled, unrenderable, undocumented
    NT-3 NAMESPACE'aegis2' 1 FAIL — "RPC filters will break"
    NT-4 Removed distribute_yield from the spec 1 FAIL — undocumented function
    Clean tree (after each restore) exit 0, 10/10 PASS
  • Existing suites unchanged and passing: contract 9/9 (src/test.rs),
    monitoring 106/106 (monitoring/tests/).

  • Script robustness verified: bash -n clean, runs correctly from any cwd,
    emits no ANSI escapes when piped (CI-safe).

  • No-Test Justification — not needed.

4. Commands Run

  • make verify passes locally — exit 0. Output in Additional Context.

5. CI Status

  • Cannot self-certify. The repo's two workflows
    (auto-trigger.yml, trigger-auto-assign.yml) only dispatch to an
    external automation repo; there is no build/test CI to observe. If CI is
    added, make verify is the single command to wire in.

6. Acceptance Criteria Coverage

  • All six criteria in the Completion Table; all Complete.
  • Documentation impact recorded for every criterion, with explicit N/A +
    reason where no doc change was needed (AC 2, AC 3, AC 4).
  • No incomplete criteria.

Policy & Standards

  • Contributor Evaluation Policydocs/contributor-evaluation-policy.md
    is not present in this repo; could not read it.
  • Read CONTRIBUTING.md and followed it (and extended it with a
    maintainer-facing Releasing section).
  • cargo fmt --all --check — clean.
  • cargo clippy --all-targets -- -D warnings0 warnings.
  • cargo test9 passed, 0 failed, 1 ignored. No contract logic
    changed, so no new contract tests were required; the new tooling is
    covered by the negative tests in §3.
  • Traceability Mapping — completed above (storage/events/security all
    explicitly "None", with the recorded baseline noted).
  • Reviewer Checklistdocs/reviewer-checklist.md not present.
  • Compliance & Legal Checkdocs/legal-boundary-disclaimer.md
    not present; see note below.

Why four boxes are unticked. All six documents referenced by this template
(traceability-mapping.md, pr-evidence-checklist.md, testing-standards.md,
contributor-evaluation-policy.md, reviewer-checklist.md,
legal-boundary-disclaimer.md) are absent from the repository. I verified
each path. I have left these unticked rather than claim compliance with
documents I could not read — happy to complete them once added.

On the compliance/legal point specifically, and in the spirit of that
disclaimer, the new checklist states its own scope boundary explicitly (§6):

"These checks verify ledger-level enforcement only. Whitelisting a key is
an operational KYC decision made off-chain; the contract enforces the
resulting allow-list and makes no claim of regulatory completeness."

The checklist deliberately avoids implying that passing it makes a release
regulatorily compliant. It also records R3 (no whitelist revocation) as an
open limitation precisely so nobody mistakes the current contract for a
complete compliance solution.

Additional Context

make verify output (exit 0)

$ make verify

1. Storage compatibility (DataKey layout)
  PASS  DataKey variants unchanged: Admin Balance TotalSupply Whitelist

2. Event compatibility (contract -> monitoring/SDK)
  PASS  contract events: init mint transfer wl_add yield
  PASS  every contract event has a monitoring ACTION_SCHEMA handler
  PASS  no orphaned handlers in ACTION_SCHEMA
  PASS  event namespace pinned to "aegis" on both sides

3. Dashboard compatibility (rendering coverage)
  PASS  every action has a dashboard tag style
  PASS  every action has a details() renderer
  PASS  analytics tracks value actions: mint transfer yield

4. Specification documentation coverage
  PASS  every exported contract function is documented
  PASS  every emitted event is documented in the spec

Summary
  All release compatibility checks passed.

cargo fmt --all --check
cargo clippy --all-targets -- -D warnings
cargo test
running 10 tests
test test::dump_event_xdr ... ignored
test test::test_distribute_yield_emits_event ... ok
...
test result: ok. 9 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out

Build successful. WASM located at target/wasm32v1-none/release/aegis_contracts.wasm

# tests 106
# pass 106
# fail 0

MAKE_VERIFY_EXIT: 0

WASM sha256 c1ae3186984a151e2853d5da3f7510767e466350e19c1b009ad0369c86537666
(15,470 bytes) — unchanged from before this PR, confirming no contract impact.

Two issues found and fixed during verification

  1. The script lost its executable bit in transit, which would have broken
    make compat-check on a fresh clone with Permission denied. Fixed by
    invoking it as bash ./scripts/check-release-compat.sh in the Makefile, so
    the gate works regardless of file mode (common with zip downloads and some
    Windows checkouts). Verified by deliberately running chmod -x and
    confirming the target still passes.

  2. A full cargo build can be OOM-killed on small runners
    (signal: 9, SIGKILL while compiling stellar-xdr). This is an environment
    limit, not a code error. Documented in checklist §1 with the workaround
    (cargo test -j 1) so a maintainer does not mistake it for a broken release.

Reviewer note on BASELINE_DATAKEYS

scripts/check-release-compat.sh:30 hardcodes the current storage layout. This
is intentional: it makes changing storage an explicit, reviewable line in the
diff rather than something that happens silently. If you'd prefer it generated
or moved to a separate manifest, that's an easy follow-up — but the property
worth preserving is that a storage change must be consciously acknowledged.

@El-swaggerito

Copy link
Copy Markdown
Contributor
\nThis PR is currently blocked by merge conflicts.\n\nPlease update the branch with the latest main branch and resolve the conflicts before it can be merged.

@El-swaggerito

Copy link
Copy Markdown
Contributor
\nGitHub has not finished calculating whether this PR can be merged cleanly.\n\nThe auto-merge automation will skip this PR for now. Re-run the automation later.

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.

Add contract release checklist

2 participants