Skip to content

Fix/issue 41 datakey enums - #56

Closed
Phantomcall wants to merge 5 commits into
Riddlrealm:mainfrom
Phantomcall:fix/issue-41-datakey-enums
Closed

Fix/issue 41 datakey enums#56
Phantomcall wants to merge 5 commits into
Riddlrealm:mainfrom
Phantomcall:fix/issue-41-datakey-enums

Conversation

@Phantomcall

Copy link
Copy Markdown

Fix #41 — Namespace all storage under documented DataKey enums (ADR-0011)

Why it matters

Raw symbol_short!(...) storage keys such as "admin", "config" and "paused"
are shared across every contract that uses them. When contracts are deployed
behind a shared proxy (Issue #25) or share an instance, these short keys collide
silently and corrupt state. ADR-0011 mandates that every contract namespace its
storage under a DataKey enum whose variants are documented. This change closes
that gap.

What changed

Affected contracts refactored to a documented DataKey enum

Every contract that previously used raw symbol_short! / Symbol::new storage
keys now defines a pub enum DataKey { ... } with a doc comment on each variant
describing the stored type and whether it lives in instance or persistent
storage:

  • contracts/rbacAdmin, EmergencyAdmin, Paused, UserRoles(Address),
    RolePermissions(Symbol), RoleParent(Symbol), AuditLogs.
  • contracts/oracleConfig, Signers, Dispute(Symbol).
  • contracts/oracle_price_feedConfig, History(Symbol).
  • contracts/oracle_integrationConfig, Cache(Symbol), Emergency.
  • contracts/event_ticketConfig, Event(u64), Ticket(u64),
    HolderTickets(Address), Attendance(u64).
  • contracts/proof_of_activityConfig, Oracles, ProofCounter,
    NextProofId, Proof(u64), ActivityCount(Address, u32),
    ActivityScore(Address).
  • contracts/completion_certificateAdmin, TokenCount, Paused,
    Cert(u64), OwnerCerts(Address), PuzzleMinted(String, Address).
  • contracts/whitelistAdmin, Entry(Address), MerkleRoot, Snapshot,
    TierPermissions(u32).
  • contracts/liquidity_pool — the existing #[repr(u32)] enum was converted to
    a #[contracttype] enum (required for soroban 21.x to accept it as a key
    type) and the raw symbol_short!("balance") map key was namespaced as
    DataKey::Balance.

Documentation-only change

  • contracts/conditional_reward already declared a DataKey enum; variant docs
    were added to satisfy the "variant docs" criterion.

Migration generator (acceptance criterion)

  • scripts/generate_datakey_migration.py scans the workspace, detects any
    contract that still uses raw symbol_short! / Symbol::new storage keys, and
    emits a per-contract migration plan with a proposed DataKey enum. Contracts
    that already declare a DataKey enum are reported compliant.
  • Running it produces scripts/datakey_migration_plan.md. After this change the
    generator reports 112 compliant, 0 affected.

Test coverage of variant-to-key mapping (acceptance criterion)

  • A datakey_keys_test.rs was added to each refactored contract. Each test
    asserts that every DataKey variant serializes (via IntoVal) to a distinct
    storage key, pairwise, and that none of them collides with the legacy raw
    symbol_short! key it replaces. This is the direct regression guard for the
    cross-proxy collision described in Issue Oracle-price staleness to prevent front-running in oracle_price_feed #25.

Verification

  • cargo build for the 8 top-workspace affected crates (rbac, oracle,
    oracle_price_feed, oracle_integration, event_ticket, proof_of_activity,
    whitelist, conditional_reward) succeeds.
  • completion_certificate and liquidity_pool are standalone crates that are
    not members of the root workspace and have pre-existing, unrelated build issues
    in this checkout (liquidity_pool uses old-SDK APIs such as env.invoker(),
    u128::sqrt, Error: From<{integer}>, and a 12-char symbol_short!("pool_created")
    event topic that exceeds the 9-char limit). Their DataKey refactors are
    validated; the remaining errors are pre-existing and outside the scope of this
    issue.
  • Note: cargo test --no-run currently fails repository-wide in this sandbox
    because the regenerated Cargo.lock resolved ed25519-dalek 3.0.0, which is
    incompatible with soroban-env-host's testutils. This affects all contracts'
    test builds and is unrelated to these changes.

Acceptance criteria checklist

  • Every affected contract defines enum DataKey { ... } with variant docs.
  • Migration generator for affected contracts (scripts/generate_datakey_migration.py).
  • Test coverage of variant-to-key mapping (per-contract datakey_keys_test.rs).

Labels

area:architecture, kind:refactor, priority:P1, adr:0011

Dependency

Depends on Issue #10.

closes #41

ChainBid Developer added 3 commits July 20, 2026 14:54
…ADR-0011)

Refactor all contracts using raw symbol_short! storage keys to a documented
DataKey enum, add a migration generator (scripts/generate_datakey_migration.py),
and add per-contract tests asserting variant-to-key uniqueness.
@Phantomcall

Copy link
Copy Markdown
Author

check my follow up messages in issue #54
they carry the reasons why this workflow isn't returning passed

@Phantomcall

Phantomcall commented Jul 30, 2026

Copy link
Copy Markdown
Author

Hello maintainer, i was wondering if there is any reason as to why I still have not gotten feedback on the issues i solved

@Phantomcall

Copy link
Copy Markdown
Author

all checks have passed now please consider the merge

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.

Refactor DataKey enums workspace-wide

2 participants