Feature/zero address validation 747

contracts/admin/src/lib.rs

@@ -687,13 +687,74 @@ pub struct UpgradeProposal {
 /// whose 32-byte payload is all zeros. No private key can ever produce a
 /// signature for it, so it is used as the canonical zero-address sentinel
 /// that must never be allowed to hold a role.
-const ZERO_ADDRESS_STRKEY: &str = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF";
+///
+/// @title ZERO_ADDRESS_STRKEY
+/// @notice The Stellar zero address constant ("GAAAA...WHF") used for zero-address validation.
+/// @dev This is the canonical zero-address sentinel; no private key can ever produce a signature for it.
+pub const ZERO_ADDRESS_STRKEY: &str = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF";
 
-fn is_zero_address(env: &Env, address: &Address) -> bool {
+/// Returns `true` if `address` is the canonical zero-address sentinel.
+///
+/// The zero address ("GAAAA…WHF") is an ed25519 public key whose 32-byte
+/// payload is all zeros. No private key can ever produce a signature for it,
+/// so holding a role there would be unrecoverable. This helper is used
+/// throughout the admin module to reject zero addresses before any storage
+/// writes.
+///
+/// # Arguments
+///
+/// * `env` - The Soroban environment
+/// * `address` - The address to check
+///
+/// # Returns
+///
+/// `true` if `address` equals the zero-address sentinel, `false` otherwise.
+///
+/// # Examples
+///
+/// ```rust,ignore
+/// // Check if an address is the zero address
+/// if is_zero_address(env, &some_address) {
+///     // Reject the address
+/// }
+/// ```
+///
+/// @notice Checks whether `address` is the canonical zero-address sentinel.
+/// @dev Compares `address` against `Address::from_str(env, ZERO_ADDRESS_STRKEY).
+/// @param env The Soroban environment.
+/// @param address The address to check.
+/// @return `true` if `address` is the zero address, `false` otherwise.
+pub fn is_zero_address(env: &Env, address: &Address) -> bool {
     *address == Address::from_str(env, ZERO_ADDRESS_STRKEY)
 }
 
-fn require_non_zero_address(env: &Env, address: &Address) {
+/// Requires that `address` is not the zero-address sentinel.
+///
+/// Panics with [`AdminError::InvalidAddress`] if `address` equals the
+/// canonical zero address ("GAAAA…WHF"). Use this guard before any storage
+/// write that associates an address with a role or administrative privilege.
+///
+/// # Arguments
+///
+/// * `env` - The Soroban environment
+/// * `address` - The address to validate
+///
+/// # Panics
+///
+/// Panics with [`AdminError::InvalidAddress`] if `address` is the zero address.
+///
+/// # Examples
+///
+/// ```rust,ignore
+/// // Reject zero address before granting a role
+/// require_non_zero_address(env, &address);
+/// ```
+///
+/// @notice Reverts if `address` is the canonical zero-address sentinel.
+/// @dev Panics with `AdminError::InvalidAddress` when `address` is the zero address.
+/// @param env The Soroban environment.
+/// @param address The address to validate.
+pub fn require_non_zero_address(env: &Env, address: &Address) {
     if is_zero_address(env, address) {
         soroban_sdk::panic_with_error!(env, AdminError::InvalidAddress);
     }
@@ -2208,6 +2269,14 @@ mod tests {
             super::require_pauser(&env, &address);
         }
 
+        pub fn is_zero_address(env: Env, address: Address) -> bool {
+            super::is_zero_address(&env, &address)
+        }
+
+        pub fn require_non_zero_address(env: Env, address: Address) {
+            super::require_non_zero_address(&env, &address);
+        }
+
         pub fn require_deployer(env: Env) {
             super::require_deployer(&env);
         }

docs/ACCESS_CONTROL.md

@@ -254,7 +254,8 @@ contract's `Role` enum.
 
 - **Admin is a superset.** Any address holding `Admin` passes every role check.
 - **Zero-address rejection.** `GAAAA…WHF` can never hold a role; all guards
-  reject it before storage writes.
+  reject it before storage writes. Use [`is_zero_address`] and
+  [`require_non_zero_address`] for validation in consuming contracts.
 - **Storage slot isolation.** Each `AdminKey` variant uses a unique enum
   discriminant. Domain separation (`instance` vs `persistent`) provides an
   additional layer.
@@ -263,6 +264,51 @@ contract's `Role` enum.
 - **Idempotent proposals.** Duplicate approvals and double-execution are
   rejected at the contract level.
 
+## Zero-address validation helpers
+
+The admin module exports two public helpers for zero-address validation:
+
+| Function | Signature | Description |
+| --- | --- | --- |
+| `is_zero_address` | `pub fn is_zero_address(env: &Env, address: &Address) -> bool` | Returns `true` if `address` is the zero-address sentinel |
+| `require_non_zero_address` | `pub fn require_non_zero_address(env: &Env, address: &Address)` | Panics with `InvalidAddress` if `address` is the zero address |
+| `ZERO_ADDRESS_STRKEY` | `pub const ZERO_ADDRESS_STRKEY: &str` | The Stellar zero address constant |
+
+These are used throughout the admin module in:
+- `set_admin` — rejects zero address before storing
+- `grant_role` — rejects zero address before role assignment
+- `_grant_role` — rejects zero address before storage write
+- `revoke_role` — rejects zero address before role removal
+- `_revoke_role` — rejects zero address before storage mutation
+- `has_role` — short-circuits to `false` for zero address
+- `set_admin_pool` — rejects zero addresses in the pool
+
+### Usage in consuming contracts
+
+```rust,ignore
+use bc_forge_admin::{is_zero_address, require_non_zero_address};
+
+// Check without panicking
+if is_zero_address(env, &some_address) {
+    // Handle the zero address case
+}
+
+// Guard before a storage write
+require_non_zero_address(env, &new_address);
+```
+
+### TypeScript SDK
+
+The SDK exports a client-side `isZeroAddress` helper:
+
+```typescript
+import { isZeroAddress, ZERO_ADDRESS } from '@bc-forge/sdk';
+
+if (isZeroAddress(someAddress)) {
+    throw new Error('Invalid address: zero address is not allowed');
+}
+```
+
 ## Source of truth
 
 - Role definitions and guards:

docs/UPGRADE_GUIDE.md

@@ -255,6 +255,8 @@ If you have an existing contract that was deployed before the `SuperAdmin`
 role was introduced, use `migrate_admin` to enable `SuperAdmin`-based guards
 without resetting state.
 
+### Option 1: CLI
+
 ```bash
 stellar contract invoke \
   --id <CONTRACT_ID> \
@@ -263,8 +265,55 @@ stellar contract invoke \
   migrate_admin
 ```
 
+### Option 2: TypeScript SDK
+
+```typescript
+import { bcForgeClient } from '@bc-forge/sdk';
+import { Keypair } from '@stellar/stellar-sdk';
+
+const client = new bcForgeClient({
+  rpcUrl: 'https://soroban-testnet.stellar.org',
+  networkPassphrase: 'Test SDF Network ; September 2015',
+  contractId: '<CONTRACT_ID>',
+});
+
+const adminKeypair = Keypair.fromSecret(process.env.ADMIN_SECRET!);
+const result = await client.migrateAdmin(adminKeypair);
+console.log('Migration TX:', result.hash);
+```
+
+### Option 3: Standalone migration script
+
+A standalone migration script is available at `migrations/rbac-migration.ts`.
+It provides a complete migration workflow with verification:
+
+```bash
+# Dry-run (simulate without submitting)
+npx ts-node migrations/rbac-migration.ts \
+  --rpc-url https://soroban-testnet.stellar.org \
+  --network-passphrase "Test SDF Network ; September 2015" \
+  --contract-id <CONTRACT_ID> \
+  --admin-secret <ADMIN_SECRET> \
+  --dry-run
+
+# Execute migration
+npx ts-node migrations/rbac-migration.ts \
+  --rpc-url https://soroban-testnet.stellar.org \
+  --network-passphrase "Test SDF Network ; September 2015" \
+  --contract-id <CONTRACT_ID> \
+  --admin-secret <ADMIN_SECRET>
+```
+
+The script performs the following steps:
+1. Verifies the contract has an admin set
+2. Checks if migration is already complete (idempotent)
+3. Executes the migration transaction
+4. Verifies the admin now has the SuperAdmin role
+
+### Storage migration process
+
 This is a one-shot, idempotent operation:
-- Reads the current admin from instance storage.
+- Reads the current admin from instance storage (`AdminKey::Admin`).
 - Creates a persistent `SuperAdmin(admin)` entry.
 - Safe to call multiple times (no-op on subsequent calls).