Skip to content

feat(contracts): optimize data schemas to leverage Soroban instance storage (Closes #271) - #486

Open
Fabluchy wants to merge 1 commit into
Sahara-Pay:mainfrom
Fabluchy:fix/instance-storage-optimization-271
Open

feat(contracts): optimize data schemas to leverage Soroban instance storage (Closes #271)#486
Fabluchy wants to merge 1 commit into
Sahara-Pay:mainfrom
Fabluchy:fix/instance-storage-optimization-271

Conversation

@Fabluchy

Copy link
Copy Markdown
Contributor

Summary\n\nCloses #271 — partitions the contract 's primary data models so that small, globally-scoped, bounded configuration lives in the contract's 64 KB instance storage while per-org / per-maintainer entries continue to scale in persistent storage.\n\n### Acceptance criteria coverage\n\n| AC | Status |\n|---|---|\n| Data schemas are partitioned effectively | Token / MultisigAdmin / ProtocolState / PendingAdmin → instance storage; Organization / OrgAdmin / OrgMaintainers / OrgBudget / MaintainerOrg / MaintainerBalance → persistent storage |\n| Instance storage stays well under 64 KB | Instance store is bounded to one Address + one small struct + one enum variant + one Address ≈ ~150 bytes |\n| Strict unit-test coverage for ledger state mutations | New assert_failed_with helper + 4 dedicated tests pin the exact PrinceError code for every instance-storage read/write path |\n| All tests pass in CI environment | cargo test and SNAPSHOT_TEST=1 cargo test both pass with 20/20 green; clippy -D warnings clean; rustfmt clean |\n\n### Data-schema partition\n\nMoved to instance storage (read once per invocation, single TTL covers them all):\n\n- DataKey::Token (Address)\n- DataKey::MultisigAdmin (struct { admins: Vec<Address>, threshold: u32 })\n- DataKey::ProtocolState (enum { Active, Paused })\n- DataKey::PendingAdmin (Address)\n\nRemains in persistent storage (per-entry TTL, scales linearly with usage):\n\n- Organization, OrgAdmin, OrgMaintainers, OrgBudget\n- MaintainerOrg, MaintainerBalance\n\n### TTL handling\n\n- Adds INSTANCE_LIFETIME_THRESHOLD / INSTANCE_BUMP_AMOUNT constants (~7 day threshold, ~30 day extension).\n- Adds Self::extend_instance_ttl(&env) helper called before every instance-storage read or write in init, get_token, get_multisig_admin, get_protocol_state, pause_protocol, unpause_protocol, propose_admin, and accept_admin.\n- Removes one redundant pre-init extend_ttl call (now only the trailing call remains, which preserves the existing 518 400-ledger TTL snapshot).\n\n### Strict unit-test coverage for ledger state mutations\n\nAdds a typed assert_failed_with helper that flattens the soroban-sdk 21 Result<T, Result<soroban_sdk::Error, InvokeError>> return shape and compares the embedded contract-error code against PrinceError as u32. This catches wrong-variant panics instead of just "an error happened".\n\nFour new tests in packages/contracts/src/tests.rs:\n\n- test_uninitialized_getters_panicPrinceError::ContractNotInitialized × 3.\n- test_init_cannot_be_called_twicePrinceError::AlreadyInitialized.\n- test_propose_and_accept_admin_instance_storage_flow — drives the two-step admin transfer through InsufficientMultisigAuth (1 signer), NotPendingAdmin (impostor accept), NoPendingAdmin (second accept after PendingAdmin removal).\n- test_paused_protocol_blocks_state_mutating_opsPrinceError::ProtocolPaused × 3 across fund_org, allocate_payout, claim_payout.\n\n### Snapshot churn (heads-up for reviewers)\n\nRunning cargo test regenerated the existing 16 snapshot files because the XDR shape of MultisigAdmin / ProtocolState / Token moves from standalone persistent ledger entries (key: vec![Symbol:"..."]) to nested entries under the contract's ledger_key_contract_instance. Each existing snapshot lost ~215 lines and picked up ~7 lines. This is the expected migration path; the new 4 test snapshots record the new state.\n\n### Notes for maintainers\n\n- The repo's .github/workflows/ directory was deleted in 7e012fc, so this PR cannot be auto-validated in CI. Local SNAPSHOT_TEST=1 cargo test is the verification step.\n- The front-end lib/contractTypes.ts and sorobanClient.ts need no changes — the contract's public method signatures are unchanged.\n\n## Validation\n\n- cargo test → 20 / 20 pass\n- SNAPSHOT_TEST=1 cargo test → 20 / 20 pass\n- cargo clippy --all-targets -- -D warnings → clean\n- cargo fmt --check → clean\n\nCo-authored-by: Buffy (Freebuff AI coding assistant)

…torage

This change finishes the migration of small, globally-scoped, bounded configuration
out of persistent storage and into the 64KB contract-instance store. The migration
keeps contract RAM and per-invocation CPU costs predictable while preserving all
public behaviour.

Data-schema partition
---------------------
Moved to instance storage (read once per invocation, single TTL covers them all):
  - DataKey::Token              (Address)
  - DataKey::MultisigAdmin      (struct { admins: Vec<Address>, threshold: u32 })
  - DataKey::ProtocolState      (enum { Active, Paused })
  - DataKey::PendingAdmin       (Address)

Remains in persistent storage (per-entry TTL, scales linearly with usage):
  - Organization, OrgAdmin, OrgMaintainers, OrgBudget
  - MaintainerOrg, MaintainerBalance

With this layout the contract-instance store stays well under 64KB even as the
org/maintainer set grows, because those entries are per-org/per-maintainer and
therefore unbounded.

TTL handling
------------
Adds a single INSTANCE_LIFETIME_THRESHOLD / INSTANCE_BUMP_AMOUNT pair and a
Self::extend_instance_ttl(&env) helper. The helper is invoked before every read
or write to instance storage (init, get_*, pause_protocol, unpause_protocol,
propose_admin, accept_admin). The redundant pre-init extend_ttl call was
removed for clarity.

Strict unit-test coverage for ledger state mutations
----------------------------------------------------
Adds four tests inside packages/contracts/src/tests.rs that cover every
instance-storage read/write end to end:

  * test_uninitialized_getters_panic
        Confirms get_token / get_multisig_admin / get_protocol_state panic with
        ContractNotInitialized before init is called.

  * test_init_cannot_be_called_twice
        Confirms the instance-storage Token guard rejects a second init.

  * test_propose_and_accept_admin_instance_storage_flow
        Drives the full PendingAdmin write/read/remove cycle and asserts the
        multisig replaces cleanly to a single-admin, threshold-1 configuration.

  * test_paused_protocol_blocks_state_mutating_ops
        Confirms pause_protocol instance-storage writes immediately guard
        fund_org, allocate_payout and claim_payout.

Test snapshot files were regenerated to reflect that Token / MultisigAdmin /
ProtocolState / PendingAdmin now live inside the contract-instance entry.

Closes Sahara-Pay#271
@Damola-Sodiq

Copy link
Copy Markdown
Contributor

@Fabluchy broooo will you be able to resolve this conflicts or just close this pr, create a new branch and make a fresh pr chief?

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.

2 participants