Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions docs/REGISTRY_INVARIANTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,49 @@ When adding a contract function that mutates state:
3. Add the resulting invariant to the table above.

A new mutating function with no fuzz arm is a review blocker.

---

## Hole Policy

The registry keeps two structures over the same username set: the legacy
flat index (`INDEX_KEY`) and the chunked index (`CHUNK_KEY` pages). This
section is the documented answer to "what does a hole mean here", so a test
or reviewer has something to check the code against instead of inferring it
from behavior.

**Membership holes are not allowed.** `remove_from_index` rewrites the flat
index and the one chunk that held the removed entry to exclude it
immediately — there is no tombstone state where a username remains
enumerable in the index but has no record. This is an unconditional
invariant:

> For every username in the flat index: it has a live record (`has_record`
> is `true`) and it appears in exactly one chunk. For every username in any
> chunk: it appears in the flat index. (I.e. index membership, chunk
> membership, and record existence are always kept in lockstep — an "iff",
> not just a one-directional guarantee.)

**Capacity holes (chunk under-fill) are allowed.** A chunk can shrink below
`CHUNK_SIZE` (50) entries after a removal, and a middle chunk can end up
smaller than a later chunk — `remove_from_index` only rewrites the one
chunk the removed entry was found in, it does not rebalance neighboring
chunks. This is *not* a membership violation; it is expected fragmentation.
`compact_index` (Issue #209) repacks the chunked index into dense,
contiguous, `CHUNK_SIZE`-full pages plus one partial tail, and reclaims
persistent entries for chunks it no longer needs — but compaction changes
only chunk *density*, never index membership.

This is checked directly — not through `get_all_registered` or a paginated
export, both of which silently skip any index entry with no backing record
and so cannot themselves detect a membership violation — by
`tests/registry_hole_policy.rs::test_index_membership_property_holds_across_register_remove_compact_sequence`
and
`...::test_index_membership_property_holds_at_chunk_boundary`, which read
the flat index and every chunk directly out of storage and assert the iff
property above across register/remove/compact sequences, including one that
straddles a chunk boundary.

| ID | Invariant | Enforced by |
|----|-----------|-------------|
| I10 | Flat-index membership, chunk membership, and record existence agree in both directions (no membership holes) | `tests/registry_hole_policy.rs` |
51 changes: 51 additions & 0 deletions docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -959,3 +959,54 @@ For production deployments, consider:
- Bug bounty program
- Staged rollout on testnet/futurenet first


---

### Index Repair (`repair_index`)

`count` and `verified` are maintained incrementally by every mutating call
(`register`, `remove`, `verify`, `revoke_verification`, batch operations,
…). Under normal operation they never need correcting — the property fuzz
suite (see [REGISTRY_INVARIANTS.md](REGISTRY_INVARIANTS.md)) exercises long
random operation sequences precisely to catch a code change that would make
them drift, before it ever reaches testnet or mainnet.

That fuzz coverage does not extend to state that reaches storage by some
path other than these public entry points — a bug in a future migration
step, a hand-crafted storage write during an incident, or an upgrade that
changes the counters' encoding. Nothing on-chain currently detects that
class of drift.

`repair_index(apply: bool)` (admin-only) recomputes `count` and `verified`
by walking the chunked username index and checking each entry's stored
record, independent of the counters themselves, and returns a
`RepairReport` with both the stored and recomputed values.

**When to use it:**

- After any incident that involved a manual/scripted storage write, a
migration, or a WASM upgrade you are not fully confident preserved the
counters.
- Whenever `get_stats()` or `get_health()` looks implausible relative to
what an off-chain indexer's own tally of `Registered`/`Removed`/`Verified`
events says it should be.
- As a routine post-upgrade sanity check, called once with `apply = false`.

**How to use it:**

1. Call with `apply = false` first. This is a pure read — it writes
nothing — and returns whether `drifted` is `true` along with the stored
vs. recomputed values for both counters.
2. Only if `drifted` is `true` and the recomputed values have been reviewed,
call again with `apply = true` to write the corrected values. A call
that finds no drift never writes, even with `apply = true`.

**What it deliberately does not do:** `repair_index` is never invoked
automatically by any other entry point — a silent repair on every call
would mask the very drift this operation exists to surface, and would let
an issue in one counter path go unnoticed behind an automatic fix on
another. Every repair is an explicit, auditable admin transaction.

Covered by `tests/repair_index.rs`, which drifts the counters against a
known-good fixture and checks both the dry-run report and the corrected
on-chain state after `apply = true`.
27 changes: 27 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub use storage::{
VerificationConfig, VerifierAllowEntry, WasmAttestation, WasmProvenance, PauseReason,
MAX_VERIFIERS,
};
pub use storage::RepairReport;
pub use version::Version;

use soroban_sdk::{contract, contractimpl, Address, BytesN, Env, String, Symbol, Vec};
Expand Down Expand Up @@ -3724,6 +3725,32 @@ impl TrustBridgeContract {
let chunks_written = crate::storage::compact_chunked_index(&env);
Ok(chunks_written)
}

// ── Index repair (admin) ────────────────────────────────────────────────

/// Recomputes `count` and `verified` from the chunked username index and
/// each entry's stored record, independent of the counters themselves.
/// Admin-only.
///
/// Pass `apply = false` for a dry run: nothing is written, only the
/// [`RepairReport`] comparing stored vs. recomputed values is returned.
/// Pass `apply = true` to have any drift corrected in the same call — a
/// call that finds no drift never writes, even with `apply = true`.
/// This is never run automatically on any other entry point; see
/// `docs/SECURITY.md#index-repair-repair_index` for when an operator
/// should reach for it.
///
/// # Errors
///
/// - [`ContractError::NotInitialized`] if `initialize` has not been called.
/// - [`ContractError::NotAuthorized`] if the caller is not the contract admin.
pub fn repair_index(env: Env, apply: bool) -> Result<RepairReport, ContractError> {
require_initialized(&env)?;
let admin = get_admin(&env)?;
admin.require_auth();

Ok(crate::storage::repair_index(&env, apply))
}
}

#[cfg(test)]
Expand Down
78 changes: 78 additions & 0 deletions src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2291,3 +2291,81 @@ pub fn is_batch_remove_proposal_expired(env: &Env, proposal: &PendingBatchRemove
.proposed_at
.saturating_add(BATCH_REMOVE_PROPOSAL_TTL_SECS)
}

// ── Index repair (admin) ──────────────────────────────────────────────────────

/// Report returned by `repair_index`: what `count` / `verified` currently say
/// on chain versus what a fresh walk of the chunked index and every stored
/// record says they should be.
#[derive(Clone, Debug, Eq, PartialEq)]
#[soroban_sdk::contracttype]
pub struct RepairReport {
/// `count` as stored on chain before this call.
pub stored_count: u32,
/// `count` recomputed by walking every chunk and counting entries that
/// still have a live record.
pub recomputed_count: u32,
/// `verified` as stored on chain before this call.
pub stored_verified: u32,
/// `verified` recomputed the same way, counting only entries whose
/// record has `verified == true`.
pub recomputed_verified: u32,
/// Whether either stored counter disagreed with the recomputed value.
pub drifted: bool,
/// Whether the stored counters were overwritten with the recomputed
/// values by this call. Always `false` when `apply` was `false` (dry
/// run) or when no drift was found — a clean report never writes, even
/// with `apply == true`.
pub applied: bool,
}

/// Recomputes `count` and `verified` from the chunked username index and
/// each entry's stored record, independent of the counters themselves — so
/// it reports correctly even if `count` or `verified` have already drifted.
///
/// With `apply == false` this is a dry run: nothing is written, only a
/// [`RepairReport`] is returned. With `apply == true`, the stored counters
/// are corrected to the recomputed values, but only if drift was actually
/// found. Never touches the flat index, individual records, or the chunked
/// index itself — only the two aggregate counters.
///
/// See `docs/SECURITY.md#index-repair-repair_index` for when an operator
/// should reach for this instead of trusting the live counters.
pub fn repair_index(env: &Env, apply: bool) -> RepairReport {
let chunk_count = get_chunk_count(env);
let mut recomputed_count: u32 = 0;
let mut recomputed_verified: u32 = 0;

for c in 0..chunk_count {
let chunk = get_chunk(env, c);
for i in 0..chunk.len() {
if let Some(username) = chunk.get(i) {
if let Some(record) = get_record(env, &username) {
recomputed_count = recomputed_count.saturating_add(1);
if record.verified {
recomputed_verified = recomputed_verified.saturating_add(1);
}
}
}
}
}

let stored_count = get_count(env);
let stored_verified = get_verified_count(env);
let drifted = stored_count != recomputed_count || stored_verified != recomputed_verified;
let applied = apply && drifted;

if applied {
set_count(env, recomputed_count);
set_verified_count(env, recomputed_verified);
}

RepairReport {
stored_count,
recomputed_count,
stored_verified,
recomputed_verified,
drifted,
applied,
}
}
Loading
Loading