Improve config error message and add attester registry tests

CHANGELOG.md

@@ -9,6 +9,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- `attester-registry`: `add_attesters(Vec<Address>)` and
+  `remove_attesters(Vec<Address>)` — admin-gated batch operations re-landing the
+  feature from commit `5a93edd` that was silently dropped in the PR #94 merge
+  (PROC-01 / issue #103). Both functions enforce `BATCH_LIMIT = 40` to stay
+  within Soroban's per-transaction write-entry budget, skip already-present
+  (add) or already-absent (remove) addresses idempotently, and emit one
+  `AttesterAdded` / `AttesterRemoved` event per address actually changed.
+  `Error::BatchTooLarge` (code `8`) is added and documented in
+  `docs/error-codes.md`.
+- `docs/adr/0006-attestation-revocation-semantics.md`: Decision section filled
+  in and status moved to Accepted. Explicit choice: attestations are immutable
+  historical records; responders independently check current attester status via
+  `is_attester`; `revoke_attestation` is available for surgical per-record
+  admin removal (ARCH-02 / issue #105).
+
+### Fixed
+
+- `attester-registry`: added missing `extend_ttl` calls to all state-mutating
+  functions (`initialize`, `propose_admin`, `accept_admin`, `pause`, `unpause`,
+  `remove_attester`, `set_max_attesters`, `suspend_attester`,
+  `reinstate_attester`, `upgrade`, `migrate`) so that instance storage TTL is
+  bumped on every write path, not only on `add_attester` /
+  `add_attester_with_info` / `update_attester_info` (ARCH-01 / issue #104).
+- `attestation-registry`: `attest()` now also extends the TTL of the specific
+  `Attestation(record_hash, sequence)` persistent entry it writes, preventing
+  archival of individual attestation records independently of instance storage
+  (ARCH-01 / issue #104).
+
 - ADR-0009 and a prototype release manifest: `scripts/generate_release_manifest.py`
   binds contract wasm hashes, storage schema versions, generated bindings, event
   schemas, and per-network deployment state into one JSON document

contracts/attestation-registry/src/lib.rs

@@ -345,6 +345,14 @@ impl AttestationRegistry {
             .persistent()
             .set(&DataKey::AttestationCount(record_hash.clone()), &new_count);
 
+        // Extend TTL on the specific attestation entry just written, so it is
+        // not subject to state-archival independently of the instance storage.
+        env.storage().persistent().extend_ttl(
+            &DataKey::Attestation(record_hash.clone(), new_sequence),
+            INSTANCE_LIFETIME_THRESHOLD,
+            INSTANCE_BUMP_AMOUNT,
+        );
+
         env.storage()
             .instance()
             .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);

contracts/attester-registry/src/lib.rs

@@ -5,7 +5,7 @@
 
 use soroban_sdk::{
     contract, contracterror, contractevent, contractimpl, contracttype, Address, BytesN, Env,
-    Symbol,
+    Symbol, Vec,
 };
 
 const SCHEMA_VERSION: u32 = 1;
@@ -73,6 +73,17 @@ const INSTANCE_LIFETIME_THRESHOLD: u32 = 518_400;
 /// unboundedly.
 const DEFAULT_MAX_ATTESTERS: u32 = 50_000;
 
+/// Maximum number of addresses that may be processed in a single
+/// `add_attesters` / `remove_attesters` call.
+///
+/// Rationale: each address in the batch is one persistent-storage write entry.
+/// Soroban's per-transaction write-entry limit is 50, so a ceiling of 40 gives
+/// headroom for the instance-storage writes (AttesterCount, Paused, etc.) that
+/// happen in the same transaction. Batches larger than this are rejected with
+/// `Error::BatchTooLarge` — an early, deterministic error rather than a silent
+/// resource-limit abort at the network layer.
+pub const BATCH_LIMIT: u32 = 40;
+
 /// Errors returned by the attester registry's public entry points.
 #[contracterror]
 #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
@@ -93,6 +104,8 @@ pub enum Error {
     /// The referenced attester is not currently allowlisted (never added,
     /// or since removed).
     AttesterNotFound = 7,
+    /// The supplied batch exceeds `BATCH_LIMIT` addresses.
+    BatchTooLarge = 8,
 }
 
 /// Emitted when admin ownership finishes transferring to a new address.
@@ -196,6 +209,9 @@ impl AttesterRegistry {
         env.storage()
             .instance()
             .set(&DataKey::SchemaVersion, &SCHEMA_VERSION);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -211,6 +227,9 @@ impl AttesterRegistry {
         env.storage()
             .instance()
             .set(&DataKey::PendingAdmin, &new_admin);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -236,6 +255,10 @@ impl AttesterRegistry {
         }
         .publish(&env);
 
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
+
         Ok(())
     }
 
@@ -248,6 +271,9 @@ impl AttesterRegistry {
         admin.require_auth();
         env.storage().instance().set(&DataKey::Paused, &true);
         Paused { by: admin }.publish(&env);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -257,6 +283,9 @@ impl AttesterRegistry {
         admin.require_auth();
         env.storage().instance().set(&DataKey::Paused, &false);
         Unpaused { by: admin }.publish(&env);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -377,6 +406,101 @@ impl AttesterRegistry {
         Ok(())
     }
 
+    /// Add multiple attesters to the allowlist in a single transaction.
+    ///
+    /// Requires the admin's authorization. Blocked while the contract is paused.
+    /// Returns `Error::BatchTooLarge` if `attesters.len() > BATCH_LIMIT`.
+    /// Returns `Error::AllowlistFull` if adding the new (non-duplicate)
+    /// addresses would exceed the configured `max_attesters` cap. Addresses
+    /// that are already allowlisted are silently skipped (idempotent), so the
+    /// call never fails due to duplicates in the batch and no duplicate events
+    /// are emitted. Exactly one `AttesterAdded` event is emitted per newly
+    /// added address.
+    pub fn add_attesters(env: Env, attesters: Vec<Address>) -> Result<(), Error> {
+        Self::admin(&env)?.require_auth();
+        Self::require_not_paused(&env)?;
+
+        if attesters.len() > BATCH_LIMIT {
+            return Err(Error::BatchTooLarge);
+        }
+
+        let max = Self::max_attesters(&env);
+        let mut count = Self::attester_count(&env);
+
+        for attester in attesters.iter() {
+            let key = DataKey::Attester(attester.clone());
+            if !env.storage().persistent().has(&key) {
+                if count >= max {
+                    return Err(Error::AllowlistFull);
+                }
+                let info = AttesterInfo {
+                    license_hash: None,
+                    region: None,
+                };
+                env.storage().persistent().set(&key, &info);
+                count += 1;
+                AttesterAdded {
+                    attester: attester.clone(),
+                }
+                .publish(&env);
+            }
+        }
+
+        env.storage()
+            .instance()
+            .set(&DataKey::AttesterCount, &count);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
+
+        Ok(())
+    }
+
+    /// Remove multiple attesters from the allowlist in a single transaction.
+    ///
+    /// Requires the admin's authorization. Blocked while the contract is paused.
+    /// Returns `Error::BatchTooLarge` if `attesters.len() > BATCH_LIMIT`.
+    /// Addresses that are not currently allowlisted are silently skipped
+    /// (idempotent), so the call never fails if an address was already removed
+    /// and no spurious events are emitted. Exactly one `AttesterRemoved` event
+    /// is emitted per address that was actually removed.
+    pub fn remove_attesters(env: Env, attesters: Vec<Address>) -> Result<(), Error> {
+        Self::admin(&env)?.require_auth();
+        Self::require_not_paused(&env)?;
+
+        if attesters.len() > BATCH_LIMIT {
+            return Err(Error::BatchTooLarge);
+        }
+
+        let mut count = Self::attester_count(&env);
+
+        for attester in attesters.iter() {
+            let key = DataKey::Attester(attester.clone());
+            if env.storage().persistent().has(&key) {
+                env.storage().persistent().remove(&key);
+                env.storage()
+                    .persistent()
+                    .remove(&DataKey::Suspended(attester.clone()));
+                if count > 0 {
+                    count -= 1;
+                }
+                AttesterRemoved {
+                    attester: attester.clone(),
+                }
+                .publish(&env);
+            }
+        }
+
+        env.storage()
+            .instance()
+            .set(&DataKey::AttesterCount, &count);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
+
+        Ok(())
+    }
+
     /// Remove `attester` from the allowlist. Requires the admin's
     /// authorization. A no-op if the attester was never allowlisted.
     pub fn remove_attester(env: Env, attester: Address) -> Result<(), Error> {
@@ -401,6 +525,9 @@ impl AttesterRegistry {
             }
         }
         AttesterRemoved { attester }.publish(&env);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -412,6 +539,9 @@ impl AttesterRegistry {
         env.storage()
             .instance()
             .set(&DataKey::MaxAttesters, &max_attesters);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -433,6 +563,9 @@ impl AttesterRegistry {
             .persistent()
             .set(&DataKey::Suspended(attester.clone()), &true);
         AttesterSuspended { attester }.publish(&env);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -444,6 +577,9 @@ impl AttesterRegistry {
             .persistent()
             .remove(&DataKey::Suspended(attester.clone()));
         AttesterReinstated { attester }.publish(&env);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -505,6 +641,9 @@ impl AttesterRegistry {
         env.deployer()
             .update_current_contract_wasm(new_wasm_hash.clone());
         Upgraded { new_wasm_hash }.publish(&env);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }
 
@@ -537,6 +676,9 @@ impl AttesterRegistry {
         env.storage()
             .instance()
             .set(&DataKey::SchemaVersion, &SCHEMA_VERSION);
+        env.storage()
+            .instance()
+            .extend_ttl(INSTANCE_LIFETIME_THRESHOLD, INSTANCE_BUMP_AMOUNT);
         Ok(())
     }