Repository navigation
Test/admin boundary recovery coverage - #1599
Open
otobongdev wants to merge 10 commits into
Open
otobongdev wants to merge 10 commits into
otobongdev wants to merge 10 commits into
Conversation
The committed lockfile resolved two incompatible major versions of the
ed25519-dalek / curve25519-dalek / rand_core family at once. The test
build therefore failed before any workspace code was compiled:
error[E0277]: the trait bound `ChaCha20Rng: ed25519_dalek::rand_core::CryptoRng`
is not satisfied
error: could not compile `soroban-env-host` (lib)
Reproduce on the previous lockfile with:
cargo test -p admin --locked --no-run
Re-resolving collapses the duplicates onto one version of each crate
(-150/+30 lines) and the admin test target builds and runs again.
… table
`cargo clippy --all-targets -- -D warnings` failed with 11 `unreachable_pattern`
errors: `category()` and `description()` each listed arms that had already been
matched above them (e.g. `UnsupportedDecimals` twice, `InvalidStringifiedBytes`
and `SnapshotGenerationMismatch` twice, `StaleAdminEpoch` / `StaleSignerEpoch`
twice). A duplicated arm is unreachable, so the first occurrence silently won —
if the two ever disagreed the second would be dead code with no warning.
`BytesTooLarge` is retained by the Bond arm so coverage is unchanged.
The parallel variant inventories had drifted apart and none matched the enum:
src/test_errors.rs::all_variants() 110 rows, 5 duplicated, 11 missing
variant_table.rs 104 rows, 12 missing
discriminant_uniqueness.rs 107 rows, 9 missing
enum 116 variants
Rather than re-introduce three hand-maintained lists, the two test binaries now
`include!("../variant_table.rs")` — the file that already documents itself as
the single source of truth — and `all_variants()` derives from it, so a new
variant cannot be added to one list and missed in another. Wire codes that moved
when colliding discriminants were resolved are updated (ZeroBytes32 109 -> 127,
RoleRequired 127 -> 128) and pinned with `const _: () = assert!(...)` guards.
cargo test -p credence_errors -> 118 passed, 0 failed
cargo clippy -p credence_errors --all-targets -- -D warnings -> clean
…Governable tests compile `crates/interfaces/src/consts.rs` was committed as a single line of base64 (14569 bytes, `md5` identical to `git show HEAD:...`), so the crate did not parse at all: `cargo check -p interfaces` failed with "expected one of `!` or `::`, found `Ly8vIGxvbmcga2V5cy...`". Decoding restores the file the commit intended to add; the paste had also mangled six tokens inside it, all repaired here (`Ok(()` -> `Ok(())`, `#[config(test)]` / `use supek::*` / `$AX_KEY_LEN` / a stray `}` in a fn signature, and two `&static str` lifetimes that needed `&'static str`). `governable.rs` carried `#[config(test)]]` and a test module that could never compile: typo'd storage keys (`ADDIN_KEY`, `ADFIN_KEY`, `nEw_admin`), an undefined `catch_unwind`/`assert_uneq`, a `Governable` trait that nothing implemented, and calls to SDK APIs that do not exist in the pinned soroban-sdk 22.0.11 (`Address::generate` without `testutils`). Rewritten as a real `#[contract]` mock implementing the trait and driven through its generated client, matching the pattern already used in `credence_treasury::test_flash_loan`. It now pins the documented invariants: uninitialized reads fail deterministically, transfer replaces the admin atomically, an unauthorized caller is rejected with state intact, self-transfer is a silent no-op, the previous admin loses control immediately, and a rejected transfer leaves nothing observable. testutils is now a dev-dependency only, so the release build is unchanged. Two clippy findings in the restored code are fixed at the same time (`(MIN..=MAX).contains(&len)`, module-level `//!` doc, deprecated `register_contract` -> `register`). cargo test -p interfaces -> 26 passed, 0 failed cargo clippy -p interfaces --all-targets --no-deps -- -D warnings -> clean
The file did not parse, so the whole `credence_bond` lib — and therefore every one of its test targets — failed to compile before a single test could run: error: mismatched closing delimiter / unexpected closing delimiter error: unknown start of token: \ (a literal `#[test\n]`) error: expected one of `!` or `::`, found keyword `enum` (`public enum`) Four more corruptions only surface once the file parses, and are fixed here: `Ok(()` -> `Ok(())`, `#[cfg](test)]` -> `#[cfg(test)]`, `...Default::default()` -> `..Default::default()` (and because `IdentityBond` derives no `Default`, the test helper now builds the struct field by field), and `oka_or` -> `ok_or`. The last one also had a real type error: `ok_or` was being handed an already-wrapped `Err`, so `period_end` returned `Result<Result<u64, _>, _>`; it now takes the error value directly. `cargo check -p credence_bond` is clean for this module. The rest of the crate's test build is still broken by unrelated pre-existing work (see the PR description).
…ninitialized reads, MaxAdmins capacity, paused gate, suspension expiry and stale role checks Adds `contracts/admin/src/test_lib_boundary_recovery.rs` (6 tests) and one `#[cfg(test)] mod` line. No production code, signature, validation or safeguard is changed. What had no coverage at all, and is now pinned: - reads against absent storage (the loading state) — each one is either a stable error or a documented default, none mutates the epoch, and two rejected `initialize` calls leave the contract fully uninitialized so the corrected retry commits exactly once; - the `MaxAdmins` capacity boundary — `MaxAdmins` fits, `MaxAdmins + 1` is rejected without disturbing membership, role lists or the epoch, and the identical retry succeeds once a slot frees; - the paused gate across all 8 privileged mutations — every one fails with `ContractPaused` before authorization or state validation, emits nothing and moves no state, while reads keep working, a duplicate `pause` stays a silent no-op, and `unpause` restores normal operation; - `get_effective_active_admin_count` (previously untested) — it drops a suspended admin without deleting the record, counts it again exactly at `suspended_until` (inclusive `>=`, no second transaction, no epoch bump), and skips a dangling `AdminList` entry without panicking; - `check_role_at_ledger` (previously untested) — side-effect free and rejects a stale ledger with `RoleNotHeldAtLedger`; - deactivation removes authority only: the `AdminInfo` record survives and `reactivate_admin` restores both counts exactly. Also fixes the 30 failing tests this crate already had, all pre-existing and none caused by the new module (a control run with the module removed fails the same 30): - `test_role_events` (21) took an event count "before" a call and subtracted it from the count "after". `env.events().all()` is scoped to the frame of the most recent invocation, so the two numbers came from different frames and the delta was always 0. Each assertion now reads the log immediately after the call it describes, and multi-step ordering is captured per invocation with a new `record_role_events` helper. - `test_ownership_transfer` (5): three mocked contract calls shared one `as_contract` frame, and a mocked authorization frame is consumed by the first `require_auth` (Error(Auth, ExistingValue)); the frame is split per call. Two `#[should_panic]` expectations named wire codes that no longer exist (CredenceOrg#107 InvalidPauseAction, CredenceOrg#109 unused) where the contract raises CredenceOrg#111 AdminUnchanged and CredenceOrg#115 NoPendingAdmin. `const _: () = assert!(..)` guards now pin those literals to `credence_errors::ContractError`. - `test_pause_failure_boundaries` (2) and `test_atomic_rollback` (1): the same cross-frame event-count bug. - `test_suspension`: `test_suspend_below_min_admins_rejected` suspended an admin on itself, so it tripped the earlier `AdminUnchanged` (CredenceOrg#111) guard and never reached the `MinAdmins` guard it was written for. It now uses MinAdmins = 2 with a peer admin, so the guard under test is the one that fires. `lib.rs` also carries the pre-existing prerequisite repair of a duplicated `is_admin` definition, and `test_basic.rs` / `test_emergency.rs` carry the matching call-site updates; without them this crate does not compile, since HEAD defines `is_admin` twice and `test_basic.rs` calls a set of entry points that do not exist. cargo test -p admin -> 258 passed, 0 failed (was 219 passed, 30 failed) cargo clippy -p admin --all-targets --no-deps -- -D warnings -> clean
…space compile errors Co-Authored-By: Freebuff <noreply@freebuff.com>
Co-Authored-By: Freebuff <noreply@freebuff.com>
Co-Authored-By: Freebuff <noreply@freebuff.com>
…s-control matrix Restore the treasury source tree, resolve the duplicate `get_admin` export (inherent `#[contractimpl]` vs the `Governable` trait impl), call `pausable::require_not_paused` instead of the non-existent `Self::` method, and add `#[cfg(test)] extern crate std/alloc` shims for the `#![no_std]` crate. Update the access-control matrix to the current 1-arg admin entrypoints. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
Resolve the PR CredenceOrg#1599 conflicts with the main branch (7973a84) by taking the base version of every conflicted file. The branch keeps only the new admin boundary/recovery test module, registered in contracts/admin/src/lib.rs. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1307