From f14969f3199b46ec4ca9d5d87addac4214971580 Mon Sep 17 00:00:00 2001 From: BigMick03 Date: Wed, 27 May 2026 01:55:34 +0000 Subject: [PATCH] feat(escrow): add governed params setter wiring readiness --- PR_REQUEST.md | 41 +++++++++++++++ contracts/escrow/src/governance.rs | 52 +++++++++++++++++++ contracts/escrow/src/lib.rs | 5 +- .../escrow/src/test/mainnet_readiness.rs | 49 +++++++++++++---- contracts/escrow/src/test/mod.rs | 3 +- contracts/escrow/src/types.rs | 9 ++++ docs/escrow/mainnet-readiness.md | 8 +-- 7 files changed, 149 insertions(+), 18 deletions(-) create mode 100644 PR_REQUEST.md create mode 100644 contracts/escrow/src/governance.rs diff --git a/PR_REQUEST.md b/PR_REQUEST.md new file mode 100644 index 0000000..13fdcc4 --- /dev/null +++ b/PR_REQUEST.md @@ -0,0 +1,41 @@ +# Pull Request Request + +## Summary +This PR adds an admin-gated governance parameter setter to the escrow contract in `contracts/escrow`. It introduces `set_governed_params(admin, protocol_fee_bps, max_escrow_total_stroops)`, persists governed parameters, and flips the readiness checklist flag `ReadinessChecklist.governed_params_set` to `true`. + +## Files changed +- `contracts/escrow/src/governance.rs` +- `contracts/escrow/src/lib.rs` +- `contracts/escrow/src/types.rs` +- `contracts/escrow/src/test/mainnet_readiness.rs` +- `contracts/escrow/src/test/mod.rs` +- `docs/escrow/mainnet-readiness.md` + +## What this change does +- Adds `GovernedParameters` storage and `DataKey::GovernedParameters` +- Adds a secure admin-controlled setter for governed parameters +- Enforces validation of `protocol_fee_bps` and `max_escrow_total_stroops` +- Ensures `governed_params_set` becomes `true` only after a successful admin-set operation +- Exposes `get_governed_parameters()` for read-back +- Updates readiness documentation to match the new setter flow + +## Security and behavior notes +- `set_governed_params` requires `initialize()` to have been called and authenticated as the registered admin. +- Invalid parameter updates and unauthorized callers do not mutate readiness state. +- The readiness checklist update is atomic and persisted together with the governed parameter values. + +## Testing +Run the following commands from the repo root: + +```bash +cargo fmt --all -- --check +cargo test -p escrow +``` + +## Branch and commit guidance +- Branch name: `feature/governed-params-setter` +- Example commit message: `feat(escrow): add governed params setter wiring readiness` + +## Additional notes +- This PR is scoped to the TalentTrust escrow Soroban contract. +- Documentation in `docs/escrow/mainnet-readiness.md` has been aligned with the new contract behavior. diff --git a/contracts/escrow/src/governance.rs b/contracts/escrow/src/governance.rs new file mode 100644 index 0000000..b7d4788 --- /dev/null +++ b/contracts/escrow/src/governance.rs @@ -0,0 +1,52 @@ +use soroban_sdk::{contractimpl, Address, Env}; + +use crate::{DataKey, EscrowError, GovernedParameters}; + +#[contractimpl] +impl super::Escrow { + /// Set governance-controlled deployment parameters. + /// + /// This admin-only entrypoint persists the governed parameters and flips + /// `ReadinessChecklist.governed_params_set` to `true`. + pub fn set_governed_params( + env: Env, + admin: Address, + protocol_fee_bps: u32, + max_escrow_total_stroops: i128, + ) -> bool { + Self::require_initialized(&env); + Self::require_admin(&env, &admin); + + if protocol_fee_bps > 10_000 { + env.panic_with_error(EscrowError::InvalidProtocolParameters); + } + if max_escrow_total_stroops <= 0 { + env.panic_with_error(EscrowError::InvalidProtocolParameters); + } + if max_escrow_total_stroops > super::MAX_TOTAL_ESCROW_STROOPS { + env.panic_with_error(EscrowError::InvalidProtocolParameters); + } + + let params = GovernedParameters { + protocol_fee_bps, + max_escrow_total_stroops, + }; + + env.storage() + .persistent() + .set(&DataKey::GovernedParameters, ¶ms); + + let mut checklist = Self::load_checklist(&env); + checklist.governed_params_set = true; + env.storage() + .persistent() + .set(&DataKey::ReadinessChecklist, &checklist); + + true + } + + /// Returns the currently configured governed parameters, if any. + pub fn get_governed_parameters(env: Env) -> Option { + env.storage().persistent().get(&DataKey::GovernedParameters) + } +} diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index 696afbf..c62406f 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -30,14 +30,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, + ContractStatus, ContractSummary, DataKey, DepositMode, EscrowError, GovernedParameters, + Milestone, MilestoneSummary, ReadinessChecklist, CONTRACT_SUMMARY_SCHEMA_VERSION, }; mod amount_validation; pub use amount_validation::{safe_add_amounts, safe_subtract_amounts, AmountValidationError}; mod ttl; +mod governance; pub use ttl::{ LEDGERS_PER_DAY, PENDING_APPROVAL_BUMP_THRESHOLD, PENDING_APPROVAL_TTL_LEDGERS, PENDING_MIGRATION_BUMP_THRESHOLD, PENDING_MIGRATION_TTL_LEDGERS, diff --git a/contracts/escrow/src/test/mainnet_readiness.rs b/contracts/escrow/src/test/mainnet_readiness.rs index 2603cc5..a49a55d 100644 --- a/contracts/escrow/src/test/mainnet_readiness.rs +++ b/contracts/escrow/src/test/mainnet_readiness.rs @@ -59,35 +59,64 @@ fn initialize_sets_initialized_to_true() { } // ── 4.3 ───────────────────────────────────────────────────────────────────── -// After `initialize_protocol_governance`, `governed_params_set` is true. +// After `set_governed_params`, `governed_params_set` is true. #[test] -fn initialize_protocol_governance_sets_governed_params() { +fn set_governed_params_sets_governed_params() { let (env, contract_id) = setup(); let client = EscrowClient::new(&env, &contract_id); let admin = Address::generate(&env); - client.initialize_protocol_governance(&admin, &10_i128, &4_u32, &1_i128, &5_i128); + client.initialize(&admin); + assert!(client.set_governed_params(&admin, &1000_u32, &500_000_000_000_i128)); let info = client.get_mainnet_readiness_info(); assert!( info.governed_params_set, - "governed_params_set must be true after initialize_protocol_governance()" + "governed_params_set must be true after set_governed_params()" ); + + let params = client.get_governed_parameters().unwrap(); + assert_eq!(params.protocol_fee_bps, 1000); + assert_eq!(params.max_escrow_total_stroops, 500_000_000_000_i128); } // ── 4.4 ───────────────────────────────────────────────────────────────────── -// `update_protocol_parameters` also sets `governed_params_set` to true. +// `set_governed_params` can be called only by the admin and leaves the checklist +// unchanged on failure. #[test] -fn update_protocol_parameters_sets_governed_params() { +fn unauthorized_set_governed_params_does_not_set_flag() { let (env, contract_id) = setup(); let client = EscrowClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let fake_admin = Address::generate(&env); + + client.initialize(&admin); - client.update_protocol_parameters(&50_i128, &8_u32, &1_i128, &5_i128); + let result = client.try_set_governed_params(&fake_admin, &1000_u32, &500_000_000_000_i128); + super::assert_contract_error(result, EscrowError::UnauthorizedRole); let info = client.get_mainnet_readiness_info(); assert!( - info.governed_params_set, - "governed_params_set must be true after update_protocol_parameters()" + !info.governed_params_set, + "governed_params_set must remain false after an unauthorized set_governed_params()" + ); +} + +#[test] +fn invalid_set_governed_params_does_not_set_flag() { + let (env, contract_id) = setup(); + let client = EscrowClient::new(&env, &contract_id); + let admin = Address::generate(&env); + + client.initialize(&admin); + + let result = client.try_set_governed_params(&admin, &20_000_u32, &500_000_000_000_i128); + super::assert_contract_error(result, EscrowError::InvalidProtocolParameters); + + let info = client.get_mainnet_readiness_info(); + assert!( + !info.governed_params_set, + "governed_params_set must remain false after an invalid set_governed_params()" ); } @@ -172,7 +201,7 @@ fn get_mainnet_readiness_info_is_idempotent() { // Apply some lifecycle ops to create non-trivial state. client.initialize(&admin); - client.initialize_protocol_governance(&admin, &10_i128, &4_u32, &1_i128, &5_i128); + client.set_governed_params(&admin, &1000_u32, &500_000_000_000_i128); let first = client.get_mainnet_readiness_info(); let second = client.get_mainnet_readiness_info(); diff --git a/contracts/escrow/src/test/mod.rs b/contracts/escrow/src/test/mod.rs index b540ebe..abdd6ee 100644 --- a/contracts/escrow/src/test/mod.rs +++ b/contracts/escrow/src/test/mod.rs @@ -7,8 +7,7 @@ use crate::{Escrow, EscrowClient, EscrowError}; // ─── Submodules ─────────────────────────────────────────────────────────────── mod emergency_controls; -mod pause_controls; - +mod pause_controls;mod mainnet_readiness; // ─── Shared constants ───────────────────────────────────────────────────────── pub const MILESTONE_ONE: i128 = 200_0000000; diff --git a/contracts/escrow/src/types.rs b/contracts/escrow/src/types.rs index 1213d63..a516153 100644 --- a/contracts/escrow/src/types.rs +++ b/contracts/escrow/src/types.rs @@ -22,6 +22,7 @@ pub enum DataKey { // Protocol / governance ProtocolFeeBps, AccumulatedProtocolFees, + GovernedParameters, ReadinessChecklist, } @@ -74,6 +75,7 @@ pub enum EscrowError { ExactDepositRequired = 41, DepositWouldExceedTotal = 42, AccountingInvariantViolated = 43, + InvalidProtocolParameters = 44, } #[contracttype] @@ -122,6 +124,13 @@ impl Default for ReadinessChecklist { } } +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct GovernedParameters { + pub protocol_fee_bps: u32, + pub max_escrow_total_stroops: i128, +} + // ─── Indexer summary types ──────────────────────────────────────────────────── pub const CONTRACT_SUMMARY_SCHEMA_VERSION: u32 = 1; diff --git a/docs/escrow/mainnet-readiness.md b/docs/escrow/mainnet-readiness.md index 924f9f3..7f4200a 100644 --- a/docs/escrow/mainnet-readiness.md +++ b/docs/escrow/mainnet-readiness.md @@ -22,7 +22,7 @@ pub fn get_mainnet_readiness_info(env: Env) -> MainnetReadinessInfo | Field | Type | Meaning | Set by | |---|---|---|---| | `caps_set` | `bool` | `true` when `MAINNET_MAX_TOTAL_ESCROW_PER_CONTRACT_STROOPS > 0`. Derived inline from the compile-time constant — always `true` in any deployed build that has the constant defined. | Compile-time constant (not persisted) | -| `governed_params_set` | `bool` | `true` once protocol governance parameters have been configured on-chain. | `initialize_protocol_governance` or `update_protocol_parameters` | +| `governed_params_set` | `bool` | `true` once protocol governance parameters have been configured on-chain. | `set_governed_params` | | `emergency_controls_enabled` | `bool` | `true` once the emergency control mechanism has been exercised at least once (pause or resolve). | `activate_emergency_pause` or `resolve_emergency` | | `initialized` | `bool` | `true` once the contract has been initialized via `initialize`. | `initialize` | | `protocol_version` | `u32` | Always equals the `MAINNET_PROTOCOL_VERSION` compile-time constant (currently `1`). | Compile-time constant (not persisted) | @@ -31,7 +31,7 @@ pub fn get_mainnet_readiness_info(env: Env) -> MainnetReadinessInfo ### Field lifecycle details - **`caps_set`** — computed as `MAINNET_MAX_TOTAL_ESCROW_PER_CONTRACT_STROOPS > 0` every time the view function is called. It is never stored in the `ReadinessChecklist`; it reflects the WASM binary itself. -- **`governed_params_set`** — persisted in instance storage. Set to `true` atomically within the same transaction as a successful `initialize_protocol_governance` or `update_protocol_parameters` call. Once `true`, it stays `true` (governance parameters can be updated again, but the flag is never reset to `false`). +- **`governed_params_set`** — persisted in instance storage. Set to `true` atomically within the same transaction as a successful `set_governed_params` call. Once `true`, it stays `true` (governance parameters can be updated again, but the flag is never reset to `false`). - **`emergency_controls_enabled`** — persisted in instance storage. Set to `true` atomically within the same transaction as a successful `activate_emergency_pause` or `resolve_emergency` call. Indicates the emergency control path has been validated end-to-end. - **`initialized`** — persisted in instance storage. Set to `true` atomically within the same transaction as a successful `initialize` call. The `initialize` function panics on a second call, so this flag is set exactly once. - **`protocol_version`** — read directly from the `MAINNET_PROTOCOL_VERSION` constant at call time. Not affected by storage state. @@ -74,7 +74,7 @@ A freshly deployed contract with no lifecycle operations completed returns: Before directing production traffic to a contract instance, verify all of the following conditions using `get_mainnet_readiness_info`: 1. **`caps_set` must be `true`** — confirms the WASM binary was compiled with a non-zero escrow cap. If `false`, the binary is misconfigured and must not be used. -2. **`governed_params_set` must be `true`** — confirms that `initialize_protocol_governance` (or `update_protocol_parameters`) has been called and protocol parameters are active on-chain. +2. **`governed_params_set` must be `true`** — confirms that `set_governed_params` has been called and protocol parameters are active on-chain. 3. **`emergency_controls_enabled` must be `true`** — confirms the emergency pause/resolve path has been exercised at least once, validating that the admin key can operate the emergency controls. 4. **`initialized` must be `true`** — confirms `initialize` has been called and the contract admin is set. 5. **`protocol_version` must match the expected version** — compare against the version your deployment tooling expects (currently `1`). A mismatch indicates a WASM version mismatch. @@ -115,7 +115,7 @@ It is safe to call from preflight scripts, CI pipelines, monitoring dashboards, | Mechanism | What it does | |-----------|--------------| | `MAINNET_MAX_TOTAL_ESCROW_PER_CONTRACT_STROOPS` | Hard cap on the sum of milestone amounts per escrow. Not changeable via governance; requires a WASM upgrade to adjust. Reflected in `max_escrow_total_stroops`. | -| `ProtocolParameters` | Governed limits: minimum milestone size, maximum milestone count, reputation rating bounds. Configured via `initialize_protocol_governance` and `update_protocol_parameters`. Reflected in `governed_params_set`. | +| `GovernedParameters` | Governed limits: protocol fee and maximum escrow cap. Configured via `set_governed_params`. Reflected in `governed_params_set`. | ---