Skip to content

get_full_audit_state performs about ten redundant version checks and re-reads the same instance storage per delegated getter #507

Description

@usmanimamu17-create

Problem

get_full_audit_state (src/lib.rs) calls check_version once, then delegates to nine methods that each call it again and re-read the same instance storage:

pub fn get_full_audit_state(env: Env) -> Result<AuditState, SLAError> {
    Self::check_version(&env)?;
    let admin = Self::get_admin(env.clone())?;             // check_version + storage read
    let operator = Self::get_operator(env.clone())?;       // check_version + storage read
    let pending_admin = Self::get_pending_admin(env.clone())?;
    let pending_operator = Self::get_pending_operator(env.clone())?;
    let paused = Self::is_paused(env.clone())?;
    let pause_info = Self::get_pause_info(env.clone())?;
    let config_snapshot = Self::get_config_snapshot(env.clone())?;   // 4 more config map reads
    let stats = Self::get_stats(env.clone())?;
    let result_schema = Self::get_result_schema(env.clone())?;
    ...
}

Soroban instance storage is read whole per key access, so this "one-shot bootstrap" issues ~10 version checks and many full-key deserializations for data that could be read once.

Consequences:

  • The bootstrap envelope is not actually one-shot-cheap: the envelope exists so backends avoid N round-trips; internally it performs N reads + N version checks, so the RPC is cheap but the host work is the sum of all delegated reads.
  • The version check is 10x redundant: check_version is pure (reads VER); doing it ten times in one call is wasted work with no correctness benefit.
  • The cost grows with each added field: extending AuditState with another delegated getter adds another env.clone() + read + version check; nothing measures the envelope's cost (no read-budget test covers get_full_audit_state).

Root cause

The envelope was composed from existing getters rather than written as a single storage pass; the delegation pattern (clone env, delegate, check_version inside) was reused without considering the redundancy.

Why this is architecturally hard

  1. Rewriting the envelope as one storage pass requires reading each key directly (or adding internal helpers that skip check_version — e.g. pub(crate) unchecked getters) — a refactor that must preserve the exact response semantics (including pause_info as Vec, the history_len full read — companion issues).
  2. The env.clone() pattern is pervasive; the fix should establish whether internal composition should use unchecked helpers vs. public methods.
  3. Any read-budget enforcement (the offchain/readCostRegression.ts companion issue) should cover this envelope, so the fix should produce a measurable cost model.

Acceptance criteria

  • get_full_audit_state performs a single version check and reads each storage key once.
  • Response semantics are unchanged (all existing AuditState tests pass).
  • The envelope's read cost is documented (or measured by a read-budget check).

Out of scope

The history_len full-history read (companion issue) and the pause_info representation (companion issue).

Getting started

just test

Good first files to read: apexchainx_calculator/src/lib.rs (get_full_audit_state), apexchainx_calculator/src/audit_state.rs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardStellar WaveIssues in the Stellar wave programThird CampaignCampaign: Third Campaignarea/storageImported campaign issue labelpriority/mediumImported campaign issue label

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions