feat(escrow): add two-step admin transfer

PR_REQUEST.md

@@ -0,0 +1,52 @@
+# PR Request: Add Two-Step Governance Admin Transfer for Escrow
+
+## Summary
+This PR adds a secure two-step admin transfer mechanism to the TalentTrust escrow governance layer in `contracts/escrow`.
+
+## What changed
+- Implemented governance storage and management in `contracts/escrow/src/governance.rs`
+- Added protocol parameter support and governance state keys in `contracts/escrow/src/types.rs`
+- Updated `contracts/escrow/src/lib.rs` to expose governance functions and types
+- Added/updated governance tests in `contracts/escrow/src/test/governance.rs`
+- Clarified governance documentation in `docs/escrow/governance-security.md`
+
+## New public governance behavior
+- `initialize_protocol_governance(admin, ...) -> bool`
+- `initialize_governance(admin) -> bool`
+- `update_protocol_parameters(...) -> bool`
+- `propose_governance_admin(new_admin) -> bool`
+- `accept_governance_admin() -> bool`
+- `get_protocol_parameters() -> ProtocolParameters`
+- `get_governance_admin() -> Option<Address>`
+- `get_pending_governance_admin() -> Option<Address>`
+
+## Security model enforced
+- Current governance admin must authenticate to propose a new admin
+- Proposed admin must authenticate to accept transfer
+- Pending admin state is cleared on acceptance
+- A new proposal overwrites any existing pending admin
+- Current admin retains control until the transfer is accepted
+
+## Files changed
+- `contracts/escrow/src/types.rs`
+- `contracts/escrow/src/lib.rs`
+- `contracts/escrow/src/governance.rs`
+- `contracts/escrow/src/test/governance.rs`
+- `contracts/escrow/src/test/mainnet_readiness.rs`
+- `docs/escrow/governance-security.md`
+
+## Test plan
+Run locally in the repo root:
+
+```bash
+cargo fmt --all
+cargo test -p escrow
+cargo test -p escrow governance
+cargo test test::performance -p escrow
+cargo fmt --all -- --check
+```
+
+## Notes
+- This PR targets the escrow governance layer only.
+- The implementation is intentionally scoped to the governance admin transfer flow and its supporting protocol parameter state.
+- The document is written for reviewers and release notes.

contracts/escrow/src/governance.rs

@@ -0,0 +1,205 @@
+use crate::{types::ProtocolParameters, DataKey, EscrowError};
+use soroban_sdk::{contractimpl, symbol_short, Address, Env, Symbol};
+
+impl Escrow {
+    fn validate_protocol_parameters(
+        env: &Env,
+        min_milestone_amount: i128,
+        max_milestones: u32,
+        min_reputation_rating: i128,
+        max_reputation_rating: i128,
+    ) -> ProtocolParameters {
+        if min_milestone_amount <= 0 {
+            env.panic_with_error(EscrowError::InvalidProtocolParameters);
+        }
+        if max_milestones == 0 {
+            env.panic_with_error(EscrowError::InvalidProtocolParameters);
+        }
+        if min_reputation_rating <= 0 {
+            env.panic_with_error(EscrowError::InvalidProtocolParameters);
+        }
+        if min_reputation_rating > max_reputation_rating {
+            env.panic_with_error(EscrowError::InvalidProtocolParameters);
+        }
+
+        ProtocolParameters {
+            min_milestone_amount,
+            max_milestones,
+            min_reputation_rating,
+            max_reputation_rating,
+        }
+    }
+
+    fn governance_admin(env: &Env) -> Address {
+        env.storage()
+            .persistent()
+            .get(&DataKey::GovernanceAdmin)
+            .unwrap_or_else(|| env.panic_with_error(EscrowError::GovernanceNotInitialized))
+    }
+
+    fn set_governance_readiness(env: &Env) {
+        let mut checklist = Self::load_checklist(env);
+        checklist.governed_params_set = true;
+        env.storage()
+            .persistent()
+            .set(&DataKey::ReadinessChecklist, &checklist);
+    }
+}
+
+#[contractimpl]
+impl Escrow {
+    /// One-time governance initialization.
+    /// Sets the governance admin and initial protocol parameters.
+    pub fn initialize_protocol_governance(
+        env: Env,
+        admin: Address,
+        min_milestone_amount: i128,
+        max_milestones: u32,
+        min_reputation_rating: i128,
+        max_reputation_rating: i128,
+    ) -> bool {
+        if env
+            .storage()
+            .persistent()
+            .get::<_, Address>(&DataKey::GovernanceAdmin)
+            .is_some()
+        {
+            panic!("protocol governance is already initialized");
+        }
+
+        admin.require_auth();
+        let params = Self::validate_protocol_parameters(
+            &env,
+            min_milestone_amount,
+            max_milestones,
+            min_reputation_rating,
+            max_reputation_rating,
+        );
+        env.storage().persistent().set(&DataKey::GovernanceAdmin, &admin);
+        env.storage()
+            .persistent()
+            .set(&DataKey::ProtocolParameters, &params);
+        Self::set_governance_readiness(&env);
+
+        env.events().publish(
+            (symbol_short!("governance"), Symbol::new(&env, "initialized")),
+            (admin.clone(), env.ledger().timestamp()),
+        );
+        true
+    }
+
+    /// Convenience governance initialization using default protocol parameters.
+    pub fn initialize_governance(env: Env, admin: Address) -> bool {
+        let defaults = ProtocolParameters::default();
+        Self::initialize_protocol_governance(
+            env,
+            admin,
+            defaults.min_milestone_amount,
+            defaults.max_milestones,
+            defaults.min_reputation_rating,
+            defaults.max_reputation_rating,
+        )
+    }
+
+    /// Update governance protocol parameters. Requires governance admin auth.
+    pub fn update_protocol_parameters(
+        env: Env,
+        min_milestone_amount: i128,
+        max_milestones: u32,
+        min_reputation_rating: i128,
+        max_reputation_rating: i128,
+    ) -> bool {
+        if env
+            .storage()
+            .persistent()
+            .get::<_, Address>(&DataKey::GovernanceAdmin)
+            .is_none()
+        {
+            panic!("protocol governance is not initialized");
+        }
+
+        let admin = Self::governance_admin(&env);
+        admin.require_auth();
+
+        let params = Self::validate_protocol_parameters(
+            &env,
+            min_milestone_amount,
+            max_milestones,
+            min_reputation_rating,
+            max_reputation_rating,
+        );
+        env.storage()
+            .persistent()
+            .set(&DataKey::ProtocolParameters, &params);
+        Self::set_governance_readiness(&env);
+
+        env.events().publish(
+            (symbol_short!("governance"), Symbol::new(&env, "parameters_updated")),
+            (admin.clone(), env.ledger().timestamp()),
+        );
+        true
+    }
+
+    /// Returns the currently active protocol parameters.
+    pub fn get_protocol_parameters(env: Env) -> ProtocolParameters {
+        env.storage()
+            .persistent()
+            .get(&DataKey::ProtocolParameters)
+            .unwrap_or_default()
+    }
+
+    /// Returns the current governance admin if initialized.
+    pub fn get_governance_admin(env: Env) -> Option<Address> {
+        env.storage().persistent().get(&DataKey::GovernanceAdmin)
+    }
+
+    /// Returns the pending governance admin, if one has been proposed.
+    pub fn get_pending_governance_admin(env: Env) -> Option<Address> {
+        env.storage()
+            .persistent()
+            .get(&DataKey::PendingGovernanceAdmin)
+    }
+
+    /// Propose a new governance admin. Current admin must authenticate.
+    pub fn propose_governance_admin(env: Env, new_admin: Address) -> bool {
+        let admin = Self::governance_admin(&env);
+        admin.require_auth();
+
+        env.storage()
+            .persistent()
+            .set(&DataKey::PendingGovernanceAdmin, &new_admin);
+        env.events().publish(
+            (
+                symbol_short!("governance"),
+                Symbol::new(&env, "admin_proposed"),
+            ),
+            (admin.clone(), new_admin.clone(), env.ledger().timestamp()),
+        );
+        true
+    }
+
+    /// Accept proposed governance admin privileges. Pending admin must authorize.
+    pub fn accept_governance_admin(env: Env) -> bool {
+        let pending_admin: Address = env
+            .storage()
+            .persistent()
+            .get(&DataKey::PendingGovernanceAdmin)
+            .unwrap_or_else(|| env.panic_with_error(EscrowError::InvalidState));
+        pending_admin.require_auth();
+
+        env.storage()
+            .persistent()
+            .set(&DataKey::GovernanceAdmin, &pending_admin);
+        env.storage()
+            .persistent()
+            .remove(&DataKey::PendingGovernanceAdmin);
+        env.events().publish(
+            (
+                symbol_short!("governance"),
+                Symbol::new(&env, "admin_accepted"),
+            ),
+            (pending_admin.clone(), env.ledger().timestamp()),
+        );
+        true
+    }
+}

contracts/escrow/src/lib.rs

@@ -31,12 +31,15 @@ use soroban_sdk::{contract, contractimpl, symbol_short, Address, Env, Symbol, Ve
 mod types;
 pub use types::{
     ContractStatus, ContractSummary, DataKey, DepositMode, EscrowError, Milestone,
-    MilestoneSummary, ReadinessChecklist, CONTRACT_SUMMARY_SCHEMA_VERSION,
+    MilestoneSummary, ProtocolParameters, ReadinessChecklist,
+    CONTRACT_SUMMARY_SCHEMA_VERSION,
 };
 
 mod amount_validation;
 pub use amount_validation::{safe_add_amounts, safe_subtract_amounts, AmountValidationError};
 
+mod governance;
+
 mod ttl;
 pub use ttl::{
     LEDGERS_PER_DAY, PENDING_APPROVAL_BUMP_THRESHOLD, PENDING_APPROVAL_TTL_LEDGERS,

contracts/escrow/src/test/governance.rs

@@ -1,6 +1,6 @@
 use super::register_client;
 use crate::{EscrowError, ProtocolParameters};
-use soroban_sdk::{testutils::Address as _, vec, Address, Env};
+use soroban_sdk::{testutils::Address as _, Address, Env};
 
 #[test]
 fn protocol_parameters_default_before_governance_is_initialized() {
@@ -45,7 +45,6 @@ fn initialize_governance_twice_panics() {
     let admin = Address::generate(&env);
     client.initialize_governance(&admin);
 
-    // Second initialization should panic
     let admin2 = Address::generate(&env);
     client.initialize_governance(&admin2);
 }
@@ -73,25 +72,19 @@ fn update_protocol_parameters_without_initialization_panics() {
     let (env, contract_id) = setup();
     let client = EscrowClient::new(&env, &contract_id);
 
-    // Try to update without initializing governance
     client.update_protocol_parameters(&100_i128, &10_u32, &1_i128, &10_i128);
 }
 
 #[test]
-#[should_panic(expected = "minimum milestone amount must be positive")]
+#[should_panic(expected = "invalid protocol parameters")]
 fn update_protocol_parameters_with_zero_min_milestone_panics() {
     let (env, contract_id) = setup();
     let client = EscrowClient::new(&env, &contract_id);
 
     let admin = Address::generate(&env);
     client.initialize_governance(&admin);
 
-    let contract_id = client.create_contract(&escrow_client, &freelancer, &milestones);
-    assert!(client.deposit_funds(&contract_id, &285_i128));
-    assert!(client.release_milestone(&contract_id, &0));
-    assert!(client.release_milestone(&contract_id, &1));
-    assert!(client.release_milestone(&contract_id, &2));
-    assert!(client.issue_reputation(&contract_id, &5_i128));
+    client.update_protocol_parameters(&0_i128, &4_u32, &1_i128, &5_i128);
 }
 
 #[test]
@@ -105,16 +98,31 @@ fn governance_admin_transfer_is_two_step() {
     assert!(client.initialize_protocol_governance(&admin, &10_i128, &4_u32, &1_i128, &5_i128));
 
     assert!(client.propose_governance_admin(&next_admin));
-    assert_eq!(
-        client.get_pending_governance_admin(),
-        Some(next_admin.clone())
-    );
+    assert_eq!(client.get_pending_governance_admin(), Some(next_admin.clone()));
 
     assert!(client.accept_governance_admin());
     assert_eq!(client.get_governance_admin(), Some(next_admin));
     assert_eq!(client.get_pending_governance_admin(), None);
 }
 
+#[test]
+fn governance_propose_overwrites_pending_admin() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let client = register_client(&env);
+
+    let admin = Address::generate(&env);
+    let first_pending = Address::generate(&env);
+    let second_pending = Address::generate(&env);
+    assert!(client.initialize_protocol_governance(&admin, &10_i128, &4_u32, &1_i128, &5_i128));
+
+    assert!(client.propose_governance_admin(&first_pending));
+    assert_eq!(client.get_pending_governance_admin(), Some(first_pending.clone()));
+
+    assert!(client.propose_governance_admin(&second_pending));
+    assert_eq!(client.get_pending_governance_admin(), Some(second_pending.clone()));
+}
+
 #[test]
 fn governance_rejects_invalid_parameter_updates() {
     let env = Env::default();

contracts/escrow/src/test/mainnet_readiness.rs

@@ -81,7 +81,9 @@ fn initialize_protocol_governance_sets_governed_params() {
 fn update_protocol_parameters_sets_governed_params() {
     let (env, contract_id) = setup();
     let client = EscrowClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
 
+    client.initialize_governance(&admin);
     client.update_protocol_parameters(&50_i128, &8_u32, &1_i128, &5_i128);
 
     let info = client.get_mainnet_readiness_info();