Files
nym/openspec/changes/archive/2026-05-21-ecash-contract-spec/tasks.md
T
Jędrzej Stuczyński d2833c76c0 experiment: attempt to retroactively generate specs for node families and ecash contracts (#6813)
* experiment: add openspec details for node families contract

* add openspec for the ecash contract

* fix(ecash): correct latest_deposit off-by-one

DepositStorage::latest_deposit() returned the counter value, but the
counter holds the *next* free id (after next_id() saves counter+1). The
GetLatestDeposit handler then tried try_load_by_id(counter), which
always returned None — meaning the query yielded { deposit: None }
both on a fresh contract and after every successful deposit.

Fix: return counter.checked_sub(1) so latest_deposit() yields the most
recently assigned id (or None on a fresh contract). The
getting_latest_deposit unit test is updated to assert Some(0) and
Some(1) after one and two next_id() calls respectively.

No downstream consumer was relying on the buggy semantics
(validator-client exposes the query as a passthrough trait method that
nothing currently calls).

* experiment: add openspec details for ecash contract

Reverse-engineered openspec change `ecash-contract-spec` documenting
the existing CosmWasm contract at `contracts/ecash/`. Mirrors the
node-families workflow: docs-only deliverable, no migration, no
dependency changes. Archived as
openspec/changes/archive/2026-05-21-ecash-contract-spec/ and promoted
to openspec/specs/ecash-contract/spec.md as the canonical reference.

The spec captures 25 normative requirements with 64 scenarios covering
instantiation, migration, deposit submission (default + reduced tier),
RequestRedemption + redemption-proposal reply, legacy RedeemTickets
(dead code retained), stubbed blacklist surface, the ticketbook-size
invariant tripwire, the full query surface, and the public storage /
event / error surface.

Key documented points the source-of-truth phrasing pins down:
- The contract stores claimed ed25519 pubkeys opaquely; ownership is
  enforced off-chain by nym-api signers via `validate_deposit`.
- Per-signer-local de-duplication via `state.already_issued`; no
  on-chain "issued" state.
- Raw 32-byte deposit storage under the `"deposit"` namespace; deposit
  ids are sequential `u32` starting at 0.
- Statistics invariant: default_count + sum(custom_count) = total.
- `cw_controllers::Admin` is used as a generic address-equality helper
  for the `multisig` slot (the wrapper's full admin semantics are not
  exercised on that slot).
- `RedeemTickets` is dead code retained on the public surface; flagged
  as a candidate for removal.

Stubbed-blacklist final disposition is the only Open Question left for
the redesign change owner.

* docs(ecash): add rustdoc derived from archived ecash-contract spec

Drop short doc-comments on the ecash contract surface — handlers,
storage slots, message variants, error variants, event constants,
shared types — derived from the canonical spec at
openspec/specs/ecash-contract/spec.md (archived 2026-05-21).

Coverage:
- contracts/ecash/src/*.rs: crate-root summary, both DepositStorage
  and DepositStatsStorage with their invariants called out, every
  #[sv::msg(...)] handler in contract/mod.rs, reply id constants,
  Config + invariants snapshot, migration entry point.
- common/cosmwasm-smart-contracts/ecash-contract/src/*.rs: every
  ExecuteMsg / QueryMsg variant, every reachable EcashContractError
  variant (with unreachable-but-preserved variants flagged), every
  event constant, every response type, Deposit + DepositId.

Explicitly out of scope (separate concerns):
- Removing event_attributes::BANDWIDTH_PROPOSAL_ID (dead constant,
  documented as such for now).
- Removing ExecuteMsg::RedeemTickets (dead handler, documented as such;
  removal is a breaking-schema change).
- contracts/ecash/Cargo.toml version bump (docs-only).

No behaviour change; all 38 contract tests pass and cargo doc emits
no warnings on the touched crates.
2026-05-22 15:30:08 +01:00

9.6 KiB
Raw Blame History

1. Verify spec coverage against the live contract

  • 1.1 Compare each ExecuteMsg variant in common/cosmwasm-smart-contracts/ecash-contract/src/msg.rs against the spec to confirm every execute path has a corresponding requirement with scenarios (Deposit, RequestRedemption, RedeemTickets, UpdateAdmin, UpdateDefaultDepositValue, SetReducedDepositPrice, RemoveReducedDepositPrice, ProposeToBlacklist, AddToBlacklist).
  • 1.2 Compare each QueryMsg variant against the spec to confirm every read path is covered (GetBlacklistedAccount, GetBlacklistPaged, GetDefaultDepositAmount, GetRequiredDepositAmount alias, GetReducedDepositAmount, GetAllWhitelistedAccounts, GetDeposit, GetLatestDeposit, GetDepositsPaged, GetDepositsStatistics).
  • 1.3 Compare each EcashContractError variant against the spec to confirm every reachable error has a scenario that triggers it, and that the error name in the scenario matches the enum variant exactly. Annotate the unreachable variants (NotEnoughFunds, MaximumDepositTypesReached, UnknownCompressedDepositInfoType, UnknownDepositInfoType, Unauthorized, SemVerFailure) as preserved-but-unreachable. Finding: added InvalidDeposit(PaymentError) to the public-error requirement and a triggering scenario under the Deposit requirement; multitest::invalid_deposit exercises NoFunds, MultipleDenoms, MissingDenom.
  • 1.4 Confirm every event name and attribute key in nym_ecash_contract_common::events (DEPOSITED_FUNDS_EVENT_TYPE, DEPOSIT_ID, WASM_EVENT_NAME, PROPOSAL_ID_ATTRIBUTE_NAME) is named verbatim in the events requirement. Cross-check the "ticket_redemption" event and its "moved_to_holding_account" attribute (defined inline, not via a constant) and verify the "updated_deposit", "action", "address", "deposit" attributes from the admin handlers. Finding: event_attributes::BANDWIDTH_PROPOSAL_ID = "proposal_id" is dead code (no in-tree references); duplicates events::PROPOSAL_ID_ATTRIBUTE_NAME. Worth removing in the rustdoc follow-on; no spec impact.
  • 1.5 Confirm every storage key (raw string literal in Item::new(...) / Map::new(...) / StoredDeposits::NAMESPACE) is named verbatim in the storage-layout requirement. Specifically: "contract_admin", "multisig", "config", "pool_counters", "expected_invariants", "deposit_ids", "deposit", "reduced_deposits", "blacklist", "deposits_with_default_price", "deposits_with_default_price_amounts", "deposits_with_custom_price", "deposits_with_custom_price_amounts".
  • 1.6 Confirm both reply-id constants (BLACKLIST_PROPOSAL_REPLY_ID = 7759, REDEMPTION_PROPOSAL_REPLY_ID = 2137) are listed verbatim in the storage-layout requirement.

2. Cross-check against the contract unit tests

  • 2.1 For each #[test] in contracts/ecash/src/deposit.rs, identify the requirement(s) and scenario(s) in the spec that it exercises. Flag any test that asserts behaviour not yet captured in the spec. Tests: getting_latest_deposit, total_deposits_made_tracks_count, iterating_over_deposits — all covered by "Deposit ids are sequential u32" + "Deposit-by-id and latest-deposit queries" + "Paginated deposits query" requirements.
  • 2.2 Do the same for contracts/ecash/src/deposit_stats.rs tests (statistics invariant and per-account bookkeeping). Six tests on default/reduced accumulation and per-address independence — all covered by the "Statistics invariant" requirement and the Deposit classification scenarios.
  • 2.3 Do the same for contracts/ecash/src/contract/test.rs (handler-level integration tests). ~15 tests covering query handlers, set/remove reduced price (admin gating, validations, overwrite), and deposit_stats_invariant_holds_after_mixed_deposits — all map to existing requirements.
  • 2.4 Do the same for contracts/ecash/src/contract/queued_migrations.rs tests (migration backfill + whitelist seeding). Six tests covering empty-stats init, deposit-count backfill, whitelist seeding, denom/amount/ticketbook-size rejections — all map to the Migration requirement.
  • 2.5 Do the same for contracts/ecash/src/multitest.rs tests (multi-contract integration with the multisig harness). Four tests: invalid_deposit (drove the spec edit in 1.3), wrong_deposit_amount, correct_default_deposit_succeeds, reduced_price_deposit_end_to_end — all covered.
  • 2.6 For each spec scenario that has no corresponding contract unit test, decide whether to add a test or annotate the scenario as covered indirectly (e.g. by the sylvia-generated dispatcher). Annotation: the MalformedEd25519Identity scenario is reachable only when funds are correct AND identity_key is a malformed bs58 string — currently no test exercises this directly (the must_pay check fires first in the invalid-deposit test). Documented in the rustdoc follow-on as a candidate for a new test. The TicketBookSizeChanged tripwire is also indirectly covered (no test redeploys with a different TICKETBOOK_SIZE); acceptable since the assertion guards a deployment-time invariant.

3. Validate via openspec tooling

  • 3.1 Run openspec validate ecash-contract-spec and confirm it reports "valid".
  • 3.2 Run openspec show ecash-contract-spec and review the rendered output for readability and section ordering.
  • 3.3 Run openspec status --change ecash-contract-spec and confirm applyRequires is satisfied (tasks artifact present, all dependencies done).

4. Reviewer pass

  • 4.1 Walk through proposal.md with a reviewer to confirm the "Why" section accurately captures the design intent of the ecash contract (deposit anchor for the blind-signature pipeline) and that the "Known limitation — blacklist is stubbed" note matches operational understanding. Resolved 2026-05-21: contract role confirmed accurate; blacklist limitation note confirmed OK. Two changes landed: (a) the "two-admin model" framing was wrong — the team uses cw_controllers::Admin only as a generic address-equality helper for the multisig slot, not as an admin in the operational sense. Reworded the bullet, mirrored in design.md "What to document" + Decision 1 + spec.md authorisation requirement. (b) The "off-chain ownership/double-issue checks" wording was vague — replaced with a precise bullet citing state.already_issued(deposit_id) for the per-signer cache and the specific ed25519 verification path in validate_deposit (signs the plaintext IssuanceTicketBook::request_plaintext(&request.inner_sign_request, request.deposit_id)), plus the surrounding signer-eligibility / expiration / DKG / off-chain-blacklist gates.
  • 4.2 Walk through design.md Decisions 112 with a reviewer to confirm each rationale matches the team's reasoning at the time the contract was built. Pay particular attention to Decision 2 (off-chain ed25519 ownership), Decision 3 (per-signer-local de-duplication), Decision 4 (raw-bytes storage), and Decision 11 (legacy RedeemTickets retention). Resolved 2026-05-21: Decision 1 already rewritten in 4.1 (Admin-wrapper misuse). Decisions 3 (adversarial framing), 4 (storage-gas framing), 6 (graceful-degradation framing) confirmed as-is. Decisions 2, 5, 7, 8, 9, 10, 12 confirmed without changes. Decision 11 substantively rewritten: RedeemTickets is dead code that hasn't been cleaned up — not retained for indexer compatibility or held multisig grants. Removed the "load-bearing for chain scrapers" framing from design.md Decision 11, proposal.md non-obvious-choices bullet, and spec.md (events requirement + the legacy-RedeemTickets requirement title and scenario name). Spec still validates at 25 reqs / 63 scenarios.
  • 4.3 Walk through specs/ecash-contract/spec.md requirement by requirement; for each disagreement, decide whether the spec is wrong (update the spec) or the implementation is wrong (open a follow-on change). Specifically confirm the client-vs-gateway role split (clients deposit, gateways redeem) is correctly worded throughout. Resolved 2026-05-21: all 25 requirements walked in six batches (13, 47, 811, 1216, 1722, 2325). One real bug found in Req 19: DepositStorage::latest_deposit() returned the counter (the next free id), so GetLatestDeposit always returned { deposit: None } even after deposits. Fixed in contracts/ecash/src/deposit.rs (counter.checked_sub(1) semantics) and the existing getting_latest_deposit test updated; all 38 contract tests pass. Spec Req 19 rewritten to describe the corrected semantics and gained a new "After deposits exist..." scenario. Final shape: 25 requirements, 64 scenarios, valid.
  • 4.4 Resolve the four Open Questions in design.md (holding_account updatability, multisig address updatability, admin renunciation, stubbed-blacklist final disposition) or move them to follow-on changes. Resolved 2026-05-21: three kept current behaviour (locked holding_account, locked multisig address, no admin renunciation) and folded into a new "Resolved Questions" section in design.md; stubbed-blacklist final disposition deferred to the blacklist redesign change owner, kept as the sole entry in "Open Questions".

5. Archive the change

  • 5.1 Once reviewed and accepted, run openspec archive ecash-contract-spec to promote specs/ecash-contract/spec.md into openspec/specs/ecash-contract/spec.md as the canonical spec.
  • 5.2 Open the follow-on change ecash-contract-rustdoc once the canonical spec is archived, so that the rustdoc pass references the archived spec for normative phrasing. Plan: deferred to a fresh session (per the apply-loop decision on 2026-05-21). The rustdoc change will start with /opsx:propose ecash-contract-rustdoc and reference openspec/specs/ecash-contract/spec.md for normative phrasing.