diff --git a/contracts/AUTHORIZATION.md b/contracts/AUTHORIZATION.md new file mode 100644 index 000000000..67e2abbe2 --- /dev/null +++ b/contracts/AUTHORIZATION.md @@ -0,0 +1,231 @@ +# Authorization audit + +Every public entrypoint across the five crates, and which principal it +authenticates ([SC-42]). + +Soroban authorization is **explicit**. An entrypoint that does not call +`require_auth()` on an address is callable by anyone, whatever else it checks. +Comparing a caller-supplied `Address` argument against an allowlist is *not* +authorization: the attacker chooses that argument. + +`env.mock_all_auths()` hides the difference entirely, which is why every claim +below is backed by a negative test that runs **without** it — +`assetsup/src/tests/auth.rs` and the `*_requires_*_authorization` tests in the +other crates. + +## Legend + +| Mark | Meaning | +|---|---| +| ✅ | Calls `require_auth()` on the named principal. | +| 🔓 | Deliberately permissionless. | +| 📖 | Read-only; no state change, no auth required. | + +--- + +## `assetsup` + +### Fixed in this change + +These four took a `caller: Address`, compared it against a registrar allowlist, +the asset owner, or the admin — and never authenticated it. Because `caller` is +supplied by whoever builds the transaction, the check was satisfied by naming a +privileged address. + +| Entrypoint | Was | Now | +|---|---|---| +| `register_asset` | registrar allowlist check only | ✅ `caller` | +| `update_asset_metadata` | owner/admin check only | ✅ `caller` | +| `transfer_asset_ownership` | owner check only | ✅ `caller` | +| `retire_asset` | owner/admin check only | ✅ `caller` | + +`transfer_asset_ownership` was the most serious: a direct path to taking any +registered asset by naming its owner. + +### Lifecycle and administration + +| Entrypoint | Principal | | +|---|---|---| +| `initialize` | `admin` | ✅ | +| `update_admin` | current admin | ✅ | +| `add_authorized_registrar` | current admin | ✅ | +| `remove_authorized_registrar` | current admin | ✅ | +| `pause_contract` | current admin | ✅ | +| `unpause_contract` | current admin | ✅ | + +`initialize` now authenticates the incoming admin, closing the front-running +window where whoever called it first on a freshly deployed contract became +admin. + +### Registry + +| Entrypoint | Principal | | +|---|---|---| +| `register_asset` | `caller`, must be an authorized registrar | ✅ | +| `update_asset_metadata` | `caller`, must be owner or admin | ✅ | +| `transfer_asset_ownership` | `caller`, must be the current owner | ✅ | +| `retire_asset` | `caller`, must be owner or admin | ✅ | +| `get_asset`, `get_asset_info`, `batch_get_asset_info`, `get_assets_by_owner`, `check_asset_exists`, `get_total_asset_count`, `get_admin`, `is_paused`, `is_authorized_registrar`, `get_contract_metadata`, `get_asset_audit_logs` | — | 📖 | + +### Tokenization, dividends, voting + +| Entrypoint | Principal | | +|---|---|---| +| `tokenize_asset` | owner | ✅ | +| `mint_tokens`, `burn_tokens` | issuer | ✅ | +| `transfer_tokens` | `from` | ✅ | +| `lock_tokens` | owner | ✅ | +| `claim_dividends` | `holder` | ✅ | +| `cast_vote` | `voter` | ✅ | +| `propose_detokenization` | `proposer` | ✅ | +| `unlock_tokens`, `update_valuation`, `distribute_dividends`, `enable_revenue_sharing`, `disable_revenue_sharing`, `execute_detokenization` | — | ⚠️ **no auth** | +| `set_transfer_restriction`, `add_to_whitelist`, `remove_from_whitelist` | — | ⚠️ **no auth** | +| `get_token_balance`, `get_token_holders`, `is_tokens_locked`, `get_ownership_percentage`, `get_tokenized_asset`, `get_unclaimed_dividends`, `get_vote_tally`, `has_voted`, `proposal_passed`, `is_whitelisted`, `get_whitelist`, `get_detokenization_proposal`, `is_detokenization_active` | — | 📖 | + +The `require_auth` for the ✅ rows lives in the `lib.rs` entrypoint wrapper, not +in the module function it delegates to. `tokenization.rs`, `dividends.rs`, +`voting.rs` and `detokenization.rs` contain no `require_auth` at all, so calling +one of those functions directly would bypass authorization. They are +`pub(crate)` and only reachable through the wrappers today, but that is a +property of the module layout rather than an enforced boundary. + +The ⚠️ rows are **not fixed here.** They are real gaps, but each needs a +decision about *which* principal should be required — the token issuer, the +asset owner, or the admin — and that is a design question rather than an +oversight. They are listed so the next change starts from a list rather than a +search. None is an ownership-transfer path, which is why the four registry +entrypoints took priority. + +--- + +## `contrib` + +`contrib` was already the reference implementation for authorization in this +workspace: every acting address is authenticated. + +| Entrypoint | Principal | | +|---|---|---| +| `initialize` | `admin` | ✅ (added here) | +| `register_asset` | `registrar` | ✅ | +| `transfer_asset` | `caller` | ✅ | +| `retire_asset` | `caller` | ✅ | +| `add_authorized_registrar` / `add_registrar` | `caller`, must be admin | ✅ | +| `remove_authorized_registrar` / `remove_registrar` | `caller`, must be admin | ✅ | +| `pause_contract`, `unpause_contract` | `caller`, must be admin | ✅ | +| `get_admin`, `get_asset`, `get_asset_info`, `get_assets_by_owner`, `get_total_count`, `get_total_asset_count`, `is_authorized_registrar`, `get_audit_logs`, `is_paused` | — | 📖 | + +Only `initialize` was unguarded, and it is now authenticated. + +> The escrow, KYC, staking, oracle, tokenization, detokenization and +> restrictions files in `contrib/src/` are **not declared as modules** and so +> are not compiled into the crate. They expose no entrypoints and are excluded +> from this audit. + +--- + +## `multisig-wallet` + +| Entrypoint | Principal | | +|---|---|---| +| `initialize` | `admin` | ✅ | +| `submit_transaction` | `initiator`, must be an owner | ✅ | +| `confirm_transaction` | `confirmer`, must be an owner | ✅ | +| `revoke_confirmation` | `revoker`, must be an owner | ✅ | +| `cancel_transaction` | `caller`, must be the initiator | ✅ | +| `propose_add_owner`, `propose_remove_owner`, `propose_change_threshold` | `proposer`, must be an owner | ✅ | +| `confirm_proposal` | `confirmer`, must be an owner | ✅ | +| `emergency_freeze`, `emergency_unfreeze` | `caller`, must be an owner | ✅ | +| `set_daily_limit` | `caller`, must be an owner | ✅ | +| `execute_transaction`, `execute_proposal` | — | 🔓 | +| `get_owners`, `get_threshold`, `get_transaction`, `is_frozen`, `get_required_confirmations`, `get_owner_profile`, `get_proposal` | — | 📖 | + +`execute_transaction` and `execute_proposal` are permissionless **by design**. +The authorization decision was already made by the confirming owners; requiring +one of them to also submit the execution adds no security while adding liveness +risk. This is the one place where "anyone can call it" is the correct answer. + +--- + +## `multisig-transfer` + +### Fixed in this change + +`multisig-transfer` had the **same gap as `assetsup`**, across every +entrypoint. Each compared a caller-supplied `caller` argument against the +stored admin, the asset's owner, or the approver set, and none authenticated +it — so approvals could be forged by naming an authorized approver, and +approval rules rewritten by naming the admin. + +| Entrypoint | Principal | | +|---|---|---| +| `initialize` | `admin` | ✅ (added here) | +| `configure_approval_rule` | `caller`, must be admin | ✅ (added here) | +| `create_transfer_request` | `caller`, ownership checked against the registry | ✅ (added here) | +| `approve_transfer_request` | `caller`, must be an authorized approver | ✅ (added here) | +| `reject_transfer_request` | `caller`, must be an authorized approver | ✅ (added here) | +| `cancel_transfer_request` | `caller`, requester or admin | ✅ (added here) | +| `execute_transfer` | — | 🔓 | +| `get_request`, `get_asset_history`, `get_pending_transfers_approver`, `get_required_approvers_category` | — | 📖 | + +`execute_transfer` is permissionless for the same reason as +`multisig-wallet`'s `execute_*`: the approvers already made the authorization +decision. It takes a `caller` argument for the audit trail, which is +deliberately not authenticated — a comment now says so, since an unused +authenticated-looking parameter is exactly what makes this class of bug hard to +spot. + +--- + +## `asset-maintenance` + +| Entrypoint | Principal | | +|---|---|---| +| `init` | `admin` | ✅ (added here) | +| `register_provider` | stored admin | ✅ | +| `deactivate_provider` | stored admin | ✅ | +| `add_maintenance_record` | `record.provider` | ✅ | +| `schedule_maintenance` | `owner` | ✅ | +| `update_maintenance_schedule` | `owner` | ✅ | +| `complete_scheduled_maintenance` | `record.provider`, via `add_maintenance_record` | ✅ | +| `acknowledge_maintenance_alert` | `by` | ✅ | +| `add_warranty_information` | stored admin | ✅ (added here) | +| `update_warranty_information` | stored admin | ✅ (added here) | +| `file_warranty_claim` | stored admin | ✅ (added here) | +| `create_maintenance_alert` | stored admin | ✅ (added here) | +| `get_maintenance_history`, `get_upcoming_maintenance`, `get_provider_details`, `get_warranty`, `get_alerts`, `calculate_total_maintenance_cost`, `calculate_asset_downtime`, `get_asset_health_score`, `get_asset_stats`, `is_maintenance_cost_excessive`, `get_overdue_maintenance` | — | 📖 | + +Four entrypoints wrote warranty terms, filed claims, and raised alerts with **no +authorization at all** — anyone could forge the audit evidence this contract +exists to provide. They now require the stored admin. + +Admin is used because these entrypoints take no caller argument, so the stored +admin is the only principal the contract can authenticate without an ABI change. +A finer-grained model — letting an asset owner manage their own warranty — needs +the asset-registry integration this contract does not have. `verify_asset_exists` +is currently a stub returning `true`. + +### Authenticating by delegation + +`update_maintenance_schedule` and `complete_scheduled_maintenance` do not call +`require_auth` directly; they delegate to `schedule_maintenance` and +`add_maintenance_record`, which do. The protection is real but easy to miss when +reading, and easy to break by refactoring the delegation away — which is why +both have their own negative test. + +--- + +## Confused-deputy check + +No contract calls `require_auth()` on its own address, and none passes its own +address where a caller's is expected. The one cross-contract call — +`multisig-transfer` invoking the registry to move ownership — is the intended +authority boundary: the multisig contract *is* the authorized party at that +point, having already collected the required approvals. + +## What is not covered + +- `require_auth_for_args` is not used anywhere. Authorization is currently + per-entrypoint, not bound to specific amounts or recipients, so a signature + authorizing a transfer does not pin *which* transfer. Binding auth to + arguments on the value-carrying paths is worth a follow-up. +- The ⚠️ rows under `assetsup` tokenization remain open. diff --git a/contracts/asset-maintenance/src/lib.rs b/contracts/asset-maintenance/src/lib.rs index f8b11a27f..54cfdc06d 100644 --- a/contracts/asset-maintenance/src/lib.rs +++ b/contracts/asset-maintenance/src/lib.rs @@ -2,6 +2,7 @@ use soroban_sdk::{contract, contractimpl, contracttype, symbol_short, Address, Env, String, Vec}; mod test; +mod tests_coverage; #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] @@ -162,7 +163,27 @@ pub struct AssetMaintenanceContract; #[contractimpl] impl AssetMaintenanceContract { + /// Loads the stored admin and requires its authorization. + /// + /// Several entrypoints mutate registry-level data without taking a caller + /// argument, so the stored admin is the only principal this contract can + /// authenticate against without an ABI change. A finer-grained model — + /// letting the asset owner manage their own warranty, for instance — needs + /// the asset-registry integration this contract does not have yet. + fn require_admin(env: &Env) { + let admin: Address = env + .storage() + .persistent() + .get(&DataKey::Admin) + .expect("not initialized"); + admin.require_auth(); + } + pub fn init(env: Env, admin: Address, registry: Address) { + // Without this, whoever calls init first becomes admin of a freshly + // deployed contract, regardless of who deployed it. + admin.require_auth(); + if env.storage().persistent().has(&DataKey::Admin) { panic!("already initialized"); } @@ -347,6 +368,8 @@ impl AssetMaintenanceContract { } pub fn add_warranty_information(env: Env, warranty: WarrantyInfo) { + Self::require_admin(&env); + if warranty.end_date <= warranty.start_date { panic!("warranty dates invalid"); } @@ -361,6 +384,8 @@ impl AssetMaintenanceContract { } pub fn update_warranty_information(env: Env, warranty: WarrantyInfo) { + Self::require_admin(&env); + if !env .storage() .persistent() @@ -382,6 +407,8 @@ impl AssetMaintenanceContract { } pub fn file_warranty_claim(env: Env, asset_id: u64, claim_amount: i128) { + Self::require_admin(&env); + let mut warranty: WarrantyInfo = env .storage() .persistent() @@ -410,6 +437,8 @@ impl AssetMaintenanceContract { } pub fn create_maintenance_alert(env: Env, alert: MaintenanceAlert) { + Self::require_admin(&env); + let mut alerts = env .storage() .persistent() diff --git a/contracts/asset-maintenance/src/test.rs b/contracts/asset-maintenance/src/test.rs index 74ab59c7b..bb3bd4376 100644 --- a/contracts/asset-maintenance/src/test.rs +++ b/contracts/asset-maintenance/src/test.rs @@ -14,6 +14,9 @@ fn test_init_and_provider_registration() { let admin = Address::generate(&env); let registry = Address::generate(&env); + // init now requires the admin's authorization, so auths must be mocked + // before it rather than after. + env.mock_all_auths(); client.init(&admin, ®istry); let provider_addr = Address::generate(&env); @@ -30,7 +33,6 @@ fn test_init_and_provider_registration() { service_area: String::from_str(&env, "Global"), }; - env.mock_all_auths(); client.register_provider(&provider); let fetched = client.get_provider_details(&provider_addr).unwrap(); diff --git a/contracts/asset-maintenance/src/tests_coverage.rs b/contracts/asset-maintenance/src/tests_coverage.rs new file mode 100644 index 000000000..a96d48a06 --- /dev/null +++ b/contracts/asset-maintenance/src/tests_coverage.rs @@ -0,0 +1,976 @@ +//! Expanded coverage for asset-maintenance ([SC-41]). +//! +//! `test.rs` covers four happy paths across ~730 lines. This module works +//! through every entrypoint with both happy and failure paths, the validation +//! boundaries, the authorization boundaries without `mock_all_auths`, and the +//! property the whole contract exists for: **maintenance history is audit +//! evidence and cannot be silently altered or deleted**. +#![cfg(test)] + +extern crate std; + +use soroban_sdk::testutils::{Address as _, Ledger as _, LedgerInfo}; +use soroban_sdk::{vec, Address, Env, String, Vec}; + +use super::*; + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +struct Ctx<'a> { + client: AssetMaintenanceContractClient<'a>, + admin: Address, + provider: Address, +} + +const NOW: u64 = 1_000_000; + +fn setup(env: &Env) -> Ctx<'_> { + env.ledger().with_mut(|li: &mut LedgerInfo| { + li.timestamp = NOW; + }); + + let contract_id = env.register(AssetMaintenanceContract, ()); + let client = AssetMaintenanceContractClient::new(env, &contract_id); + let admin = Address::generate(env); + let registry = Address::generate(env); + let provider = Address::generate(env); + + env.mock_all_auths(); + client.init(&admin, ®istry); + client.register_provider(&provider_profile(env, &provider, true)); + + Ctx { + client, + admin, + provider, + } +} + +fn provider_profile(env: &Env, address: &Address, active: bool) -> ProviderProfile { + ProviderProfile { + address: address.clone(), + name: String::from_str(env, "Service Corp"), + specialization: vec![env, String::from_str(env, "Engines")], + certification_details: String::from_str(env, "ISO9001"), + total_services: 0, + average_rating: 0, + registration_timestamp: env.ledger().timestamp(), + is_active: active, + contact_hash: String::from_str(env, "hash"), + service_area: String::from_str(env, "Global"), + } +} + +/// A valid record: costs balance, ratings in range, service date not in future. +fn record(env: &Env, provider: &Address, record_id: u64, asset_id: u64) -> MaintenanceRecord { + MaintenanceRecord { + record_id, + asset_id, + maintenance_type: MaintenanceType::Preventive, + provider: provider.clone(), + technician_id: String::from_str(env, "tech-01"), + service_date: NOW - 100, + duration_hours: 4, + description: String::from_str(env, "Routine service"), + parts_replaced: vec![env, String::from_str(env, "filter")], + labor_cost: 60, + parts_cost: 40, + total_cost: 100, + location: String::from_str(env, "Depot"), + condition_before: 5, + condition_after: 9, + issues_found: String::from_str(env, "none"), + issues_resolved: String::from_str(env, "none"), + next_recommendation: String::from_str(env, "6 months"), + documents_ipfs: vec![env, String::from_str(env, "ipfs://doc")], + quality_rating: 8, + timestamp: NOW, + } +} + +fn schedule(env: &Env, provider: &Address, asset_id: u64, due: u64) -> ScheduledMaintenance { + ScheduledMaintenance { + asset_id, + maintenance_type: MaintenanceType::Preventive, + frequency_days: 30, + last_service_date: NOW - 1000, + next_service_due: due, + provider_assigned: provider.clone(), + reminder_days: 7, + auto_schedule: true, + priority: PriorityLevel::Medium, + estimated_cost: 100, + estimated_duration: 4, + required_parts: vec![env, String::from_str(env, "filter")], + special_instructions: String::from_str(env, "none"), + } +} + +fn warranty(env: &Env, asset_id: u64, start: u64, end: u64) -> WarrantyInfo { + WarrantyInfo { + asset_id, + provider: String::from_str(env, "Maker Ltd"), + warranty_type: String::from_str(env, "Manufacturer"), + start_date: start, + end_date: end, + coverage_details: String::from_str(env, "Parts and labour"), + terms_hash: String::from_str(env, "hash"), + claim_count: 0, + max_claims: 2, + status: WarrantyStatus::Active, + is_transferable: true, + } +} + +fn alert(env: &Env, asset_id: u64) -> MaintenanceAlert { + MaintenanceAlert { + asset_id, + alert_type: AlertType::ServiceDue, + severity: AlertSeverity::Medium, + message: String::from_str(env, "Service due"), + due_date: NOW + 1000, + acknowledged: false, + acknowledged_by: Address::generate(env), + created_at: NOW, + } +} + +// --------------------------------------------------------------------------- +// init +// --------------------------------------------------------------------------- + +#[test] +fn init_stores_admin_and_registry() { + let env = Env::default(); + let ctx = setup(&env); + // Observable through an admin-gated call succeeding. + ctx.client + .register_provider(&provider_profile(&env, &Address::generate(&env), true)); +} + +#[test] +#[should_panic(expected = "already initialized")] +fn init_twice_panics() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client.init(&ctx.admin, &Address::generate(&env)); +} + +#[test] +fn init_requires_the_admins_authorization() { + // Without this, whoever calls init first owns a freshly deployed contract. + let env = Env::default(); + let contract_id = env.register(AssetMaintenanceContract, ()); + let client = AssetMaintenanceContractClient::new(&env, &contract_id); + + let res = client.try_init(&Address::generate(&env), &Address::generate(&env)); + assert!(res.is_err(), "init must require the admin's authorization"); +} + +// --------------------------------------------------------------------------- +// Providers +// --------------------------------------------------------------------------- + +#[test] +fn register_provider_stores_the_profile() { + let env = Env::default(); + let ctx = setup(&env); + + let fetched = ctx.client.get_provider_details(&ctx.provider).unwrap(); + assert_eq!(fetched.name, String::from_str(&env, "Service Corp")); + assert!(fetched.is_active); +} + +#[test] +fn get_provider_details_is_none_for_an_unknown_provider() { + let env = Env::default(); + let ctx = setup(&env); + assert!(ctx + .client + .get_provider_details(&Address::generate(&env)) + .is_none()); +} + +#[test] +fn deactivate_provider_clears_the_active_flag() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client.deactivate_provider(&ctx.provider); + + assert!( + !ctx.client + .get_provider_details(&ctx.provider) + .unwrap() + .is_active + ); +} + +#[test] +fn deactivating_an_unknown_provider_is_a_no_op() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client.deactivate_provider(&Address::generate(&env)); +} + +#[test] +fn register_provider_requires_the_admins_authorization() { + let env = Env::default(); + let ctx = setup(&env); + env.set_auths(&[]); + + let res = + ctx.client + .try_register_provider(&provider_profile(&env, &Address::generate(&env), true)); + assert!(res.is_err(), "only the admin may register providers"); +} + +#[test] +fn deactivate_provider_requires_the_admins_authorization() { + let env = Env::default(); + let ctx = setup(&env); + env.set_auths(&[]); + + let res = ctx.client.try_deactivate_provider(&ctx.provider); + assert!(res.is_err(), "only the admin may deactivate providers"); +} + +// --------------------------------------------------------------------------- +// Maintenance records — validation boundaries +// --------------------------------------------------------------------------- + +#[test] +fn add_maintenance_record_appends_to_history() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); + + let history = ctx.client.get_maintenance_history(&7); + assert_eq!(history.len(), 1); + assert_eq!(history.get(0).unwrap().record_id, 1); +} + +#[test] +fn maintenance_history_is_empty_for_an_unknown_asset() { + let env = Env::default(); + let ctx = setup(&env); + assert_eq!(ctx.client.get_maintenance_history(&999).len(), 0); +} + +#[test] +#[should_panic(expected = "provider not registered")] +fn a_record_from_an_unregistered_provider_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let stranger = Address::generate(&env); + + ctx.client + .add_maintenance_record(&record(&env, &stranger, 1, 7)); +} + +#[test] +#[should_panic(expected = "provider is inactive")] +fn a_record_from_a_deactivated_provider_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client.deactivate_provider(&ctx.provider); + + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); +} + +#[test] +#[should_panic(expected = "service date cannot be in future")] +fn a_record_dated_in_the_future_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.service_date = NOW + 1; + + ctx.client.add_maintenance_record(&r); +} + +#[test] +fn a_record_dated_exactly_now_is_accepted() { + // The boundary: `service_date > now` is rejected, so `== now` must pass. + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.service_date = NOW; + + ctx.client.add_maintenance_record(&r); + assert_eq!(ctx.client.get_maintenance_history(&7).len(), 1); +} + +#[test] +fn a_record_dated_far_in_the_past_is_accepted() { + // Backfilling historical service records is legitimate. + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.service_date = 1; + + ctx.client.add_maintenance_record(&r); + assert_eq!(ctx.client.get_maintenance_history(&7).len(), 1); +} + +#[test] +#[should_panic(expected = "cost values must be non-negative")] +fn a_record_with_a_negative_cost_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.labor_cost = -1; + r.total_cost = 39; + + ctx.client.add_maintenance_record(&r); +} + +#[test] +#[should_panic(expected = "labor + parts cost must equal total cost")] +fn a_record_whose_costs_do_not_balance_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.total_cost = 999; + + ctx.client.add_maintenance_record(&r); +} + +#[test] +fn a_zero_cost_record_is_accepted() { + // Boundary at zero: a warranty-covered service costs nothing. + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.labor_cost = 0; + r.parts_cost = 0; + r.total_cost = 0; + + ctx.client.add_maintenance_record(&r); + assert_eq!(ctx.client.calculate_total_maintenance_cost(&7), 0); +} + +#[test] +#[should_panic(expected = "condition ratings must be 1-10")] +fn a_condition_rating_below_the_range_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.condition_before = 0; + + ctx.client.add_maintenance_record(&r); +} + +#[test] +#[should_panic(expected = "condition ratings must be 1-10")] +fn a_condition_rating_above_the_range_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.condition_after = 11; + + ctx.client.add_maintenance_record(&r); +} + +#[test] +fn condition_ratings_at_both_ends_of_the_range_are_accepted() { + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.condition_before = 1; + r.condition_after = 10; + + ctx.client.add_maintenance_record(&r); + assert_eq!(ctx.client.get_maintenance_history(&7).len(), 1); +} + +#[test] +#[should_panic(expected = "quality rating must be 1-10")] +fn a_quality_rating_of_zero_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.quality_rating = 0; + + ctx.client.add_maintenance_record(&r); +} + +#[test] +#[should_panic(expected = "quality rating must be 1-10")] +fn a_quality_rating_above_ten_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut r = record(&env, &ctx.provider, 1, 7); + r.quality_rating = 11; + + ctx.client.add_maintenance_record(&r); +} + +#[test] +fn add_maintenance_record_requires_the_providers_authorization() { + let env = Env::default(); + let ctx = setup(&env); + env.set_auths(&[]); + + let res = ctx + .client + .try_add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); + assert!( + res.is_err(), + "naming a registered provider must not be enough to file a record" + ); +} + +// --------------------------------------------------------------------------- +// The audit property: history is append-only +// --------------------------------------------------------------------------- + +#[test] +fn history_is_append_only_and_earlier_records_are_never_rewritten() { + // The core reason this contract exists. If a later write could alter an + // earlier record, the on-chain history would be worthless as evidence. + let env = Env::default(); + let ctx = setup(&env); + + let first = record(&env, &ctx.provider, 1, 7); + ctx.client.add_maintenance_record(&first); + + let mut second = record(&env, &ctx.provider, 2, 7); + second.description = String::from_str(&env, "Second service"); + second.total_cost = 250; + second.labor_cost = 150; + second.parts_cost = 100; + ctx.client.add_maintenance_record(&second); + + let history = ctx.client.get_maintenance_history(&7); + assert_eq!(history.len(), 2, "both records must be retained"); + + let stored_first = history.get(0).unwrap(); + assert_eq!(stored_first.record_id, 1); + assert_eq!( + stored_first.total_cost, 100, + "the first record is unchanged" + ); + assert_eq!(stored_first.description, first.description); + assert_eq!(history.get(1).unwrap().record_id, 2); +} + +#[test] +fn reusing_a_record_id_appends_rather_than_overwriting() { + // There is no de-duplication on record_id. Filing the same id twice adds a + // second entry; it does not replace the first. Pinning the real behaviour: + // the audit trail keeps both, which is the safe direction, but consumers + // must not assume record_id is unique. + let env = Env::default(); + let ctx = setup(&env); + + let mut original = record(&env, &ctx.provider, 1, 7); + original.total_cost = 100; + ctx.client.add_maintenance_record(&original); + + let mut duplicate = record(&env, &ctx.provider, 1, 7); + duplicate.total_cost = 20; + duplicate.labor_cost = 10; + duplicate.parts_cost = 10; + ctx.client.add_maintenance_record(&duplicate); + + let history = ctx.client.get_maintenance_history(&7); + assert_eq!(history.len(), 2, "the original is retained, not replaced"); + assert_eq!(history.get(0).unwrap().total_cost, 100); +} + +#[test] +fn no_entrypoint_removes_a_maintenance_record() { + // Completing scheduled maintenance and deactivating the provider are the + // operations most likely to prune history. Neither may. + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); + ctx.client + .schedule_maintenance(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW + 500)); + ctx.client + .complete_scheduled_maintenance(&7, &record(&env, &ctx.provider, 2, 7)); + ctx.client.deactivate_provider(&ctx.provider); + + let history = ctx.client.get_maintenance_history(&7); + assert_eq!( + history.len(), + 2, + "history must survive every other operation" + ); + assert_eq!(history.get(0).unwrap().record_id, 1); +} + +// --------------------------------------------------------------------------- +// Scheduling +// --------------------------------------------------------------------------- + +#[test] +fn schedule_maintenance_stores_the_schedule() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .schedule_maintenance(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW + 500)); + + let upcoming = ctx.client.get_upcoming_maintenance(&7).unwrap(); + assert_eq!(upcoming.next_service_due, NOW + 500); +} + +#[test] +fn get_upcoming_maintenance_is_none_when_nothing_is_scheduled() { + let env = Env::default(); + let ctx = setup(&env); + assert!(ctx.client.get_upcoming_maintenance(&7).is_none()); +} + +#[test] +#[should_panic(expected = "frequency must be positive")] +fn a_schedule_with_zero_frequency_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut sched = schedule(&env, &ctx.provider, 7, NOW + 500); + sched.frequency_days = 0; + + ctx.client.schedule_maintenance(&ctx.admin, &sched); +} + +#[test] +fn a_schedule_due_in_the_past_is_accepted_and_reads_as_overdue() { + // Scheduling in the past is allowed — it is how a missed service is + // recorded — and must immediately register as overdue. + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .schedule_maintenance(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW - 1)); + + assert!(ctx.client.get_overdue_maintenance(&7)); +} + +#[test] +fn a_schedule_due_in_the_future_is_not_overdue() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .schedule_maintenance(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW + 500)); + + assert!(!ctx.client.get_overdue_maintenance(&7)); +} + +#[test] +fn a_schedule_due_exactly_now_is_not_yet_overdue() { + // Boundary: the check is `now > due`, so equality is not overdue. + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .schedule_maintenance(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW)); + + assert!(!ctx.client.get_overdue_maintenance(&7)); +} + +#[test] +fn get_overdue_maintenance_is_false_without_a_schedule() { + let env = Env::default(); + let ctx = setup(&env); + assert!(!ctx.client.get_overdue_maintenance(&7)); +} + +#[test] +#[should_panic(expected = "no schedule exists for asset")] +fn updating_a_schedule_that_does_not_exist_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .update_maintenance_schedule(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW + 500)); +} + +#[test] +fn update_maintenance_schedule_replaces_the_existing_schedule() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .schedule_maintenance(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW + 500)); + + ctx.client + .update_maintenance_schedule(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW + 9_000)); + + assert_eq!( + ctx.client + .get_upcoming_maintenance(&7) + .unwrap() + .next_service_due, + NOW + 9_000 + ); +} + +#[test] +fn schedule_maintenance_requires_the_owners_authorization() { + let env = Env::default(); + let ctx = setup(&env); + env.set_auths(&[]); + + let res = ctx + .client + .try_schedule_maintenance(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW + 500)); + assert!(res.is_err(), "scheduling must require authorization"); +} + +#[test] +fn completing_scheduled_maintenance_rolls_the_schedule_forward() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .schedule_maintenance(&ctx.admin, &schedule(&env, &ctx.provider, 7, NOW + 500)); + + let r = record(&env, &ctx.provider, 1, 7); + ctx.client.complete_scheduled_maintenance(&7, &r); + + // auto_schedule is on, so the next due date moves to service_date + 30 days. + let expected = r.service_date + 30 * 86400; + assert_eq!( + ctx.client + .get_upcoming_maintenance(&7) + .unwrap() + .next_service_due, + expected + ); + assert_eq!(ctx.client.get_maintenance_history(&7).len(), 1); +} + +#[test] +fn completing_maintenance_without_a_schedule_still_records_the_service() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .complete_scheduled_maintenance(&7, &record(&env, &ctx.provider, 1, 7)); + + assert_eq!(ctx.client.get_maintenance_history(&7).len(), 1); + assert!(ctx.client.get_upcoming_maintenance(&7).is_none()); +} + +// --------------------------------------------------------------------------- +// Warranties +// --------------------------------------------------------------------------- + +#[test] +fn add_warranty_information_stores_the_terms() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .add_warranty_information(&warranty(&env, 7, NOW, NOW + 10_000)); + + assert_eq!(ctx.client.get_warranty(&7).unwrap().end_date, NOW + 10_000); +} + +#[test] +fn get_warranty_is_none_for_an_asset_without_one() { + let env = Env::default(); + let ctx = setup(&env); + assert!(ctx.client.get_warranty(&7).is_none()); +} + +#[test] +#[should_panic(expected = "warranty dates invalid")] +fn a_warranty_ending_before_it_starts_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_warranty_information(&warranty(&env, 7, NOW + 100, NOW)); +} + +#[test] +#[should_panic(expected = "warranty dates invalid")] +fn a_zero_length_warranty_is_rejected() { + // Boundary: end must be strictly after start. + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_warranty_information(&warranty(&env, 7, NOW, NOW)); +} + +#[test] +#[should_panic(expected = "no warranty exists for asset")] +fn updating_a_warranty_that_does_not_exist_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .update_warranty_information(&warranty(&env, 7, NOW, NOW + 10_000)); +} + +#[test] +fn update_warranty_information_replaces_the_terms() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_warranty_information(&warranty(&env, 7, NOW, NOW + 10_000)); + + ctx.client + .update_warranty_information(&warranty(&env, 7, NOW, NOW + 50_000)); + + assert_eq!(ctx.client.get_warranty(&7).unwrap().end_date, NOW + 50_000); +} + +#[test] +fn add_warranty_information_requires_the_admins_authorization() { + let env = Env::default(); + let ctx = setup(&env); + env.set_auths(&[]); + + let res = ctx + .client + .try_add_warranty_information(&warranty(&env, 7, NOW, NOW + 10_000)); + assert!( + res.is_err(), + "warranty terms must not be writable by anyone" + ); +} + +// --------------------------------------------------------------------------- +// Warranty claims +// --------------------------------------------------------------------------- + +#[test] +fn filing_a_claim_increments_the_claim_count() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_warranty_information(&warranty(&env, 7, NOW, NOW + 10_000)); + + ctx.client.file_warranty_claim(&7, &500); + + assert_eq!(ctx.client.get_warranty(&7).unwrap().claim_count, 1); +} + +#[test] +#[should_panic(expected = "no warranty found")] +fn claiming_against_a_missing_warranty_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client.file_warranty_claim(&7, &500); +} + +#[test] +#[should_panic(expected = "max claims reached")] +fn claiming_beyond_the_maximum_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_warranty_information(&warranty(&env, 7, NOW, NOW + 10_000)); + + // max_claims is 2. + ctx.client.file_warranty_claim(&7, &100); + ctx.client.file_warranty_claim(&7, &100); + ctx.client.file_warranty_claim(&7, &100); +} + +#[test] +#[should_panic(expected = "warranty has expired")] +fn claiming_after_the_warranty_expires_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_warranty_information(&warranty(&env, 7, NOW, NOW + 100)); + + env.ledger().with_mut(|li: &mut LedgerInfo| { + li.timestamp = NOW + 101; + }); + + ctx.client.file_warranty_claim(&7, &100); +} + +#[test] +#[should_panic(expected = "warranty is not active")] +fn claiming_against_a_voided_warranty_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + let mut w = warranty(&env, 7, NOW, NOW + 10_000); + w.status = WarrantyStatus::Voided; + ctx.client.add_warranty_information(&w); + + ctx.client.file_warranty_claim(&7, &100); +} + +#[test] +fn file_warranty_claim_requires_the_admins_authorization() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_warranty_information(&warranty(&env, 7, NOW, NOW + 10_000)); + env.set_auths(&[]); + + let res = ctx.client.try_file_warranty_claim(&7, &100); + assert!(res.is_err(), "claims must not be forgeable by anyone"); +} + +// --------------------------------------------------------------------------- +// Alerts +// --------------------------------------------------------------------------- + +#[test] +fn create_maintenance_alert_appends_to_the_alert_list() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client.create_maintenance_alert(&alert(&env, 7)); + ctx.client.create_maintenance_alert(&alert(&env, 7)); + + assert_eq!(ctx.client.get_alerts(&7).len(), 2); +} + +#[test] +fn get_alerts_is_empty_for_an_asset_without_any() { + let env = Env::default(); + let ctx = setup(&env); + assert_eq!(ctx.client.get_alerts(&7).len(), 0); +} + +#[test] +fn acknowledging_an_alert_marks_it_and_records_who() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client.create_maintenance_alert(&alert(&env, 7)); + + let acker = Address::generate(&env); + ctx.client.acknowledge_maintenance_alert(&7, &0, &acker); + + let stored = ctx.client.get_alerts(&7).get(0).unwrap(); + assert!(stored.acknowledged); + assert_eq!(stored.acknowledged_by, acker); +} + +#[test] +fn acknowledging_an_out_of_range_alert_index_is_a_no_op() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client.create_maintenance_alert(&alert(&env, 7)); + + ctx.client + .acknowledge_maintenance_alert(&7, &99, &Address::generate(&env)); + + assert!(!ctx.client.get_alerts(&7).get(0).unwrap().acknowledged); +} + +#[test] +#[should_panic(expected = "no alerts found")] +fn acknowledging_an_alert_for_an_asset_without_any_is_rejected() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .acknowledge_maintenance_alert(&7, &0, &Address::generate(&env)); +} + +#[test] +fn create_maintenance_alert_requires_the_admins_authorization() { + let env = Env::default(); + let ctx = setup(&env); + env.set_auths(&[]); + + let res = ctx.client.try_create_maintenance_alert(&alert(&env, 7)); + assert!(res.is_err(), "alerts must not be forgeable by anyone"); +} + +// --------------------------------------------------------------------------- +// Derived statistics +// --------------------------------------------------------------------------- + +#[test] +fn stats_start_at_zero_for_an_unknown_asset() { + let env = Env::default(); + let ctx = setup(&env); + + assert_eq!(ctx.client.calculate_total_maintenance_cost(&7), 0); + assert_eq!(ctx.client.calculate_asset_downtime(&7), 0); +} + +#[test] +fn cost_and_downtime_accumulate_across_records() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 2, 7)); + + assert_eq!(ctx.client.calculate_total_maintenance_cost(&7), 200); + assert_eq!(ctx.client.calculate_asset_downtime(&7), 8); + assert_eq!(ctx.client.get_asset_stats(&7).service_count, 2); +} + +#[test] +fn health_score_is_within_the_documented_range() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); + + let score = ctx.client.get_asset_health_score(&7); + assert!(score <= 100, "health score must be on a 1-100 scale"); +} + +#[test] +fn is_maintenance_cost_excessive_compares_against_the_threshold() { + let env = Env::default(); + let ctx = setup(&env); + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); + + assert!(ctx.client.is_maintenance_cost_excessive(&7, &99)); + assert!(!ctx.client.is_maintenance_cost_excessive(&7, &100)); + assert!(!ctx.client.is_maintenance_cost_excessive(&7, &101)); +} + +#[test] +fn stats_are_tracked_per_asset() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 2, 8)); + + assert_eq!(ctx.client.calculate_total_maintenance_cost(&7), 100); + assert_eq!(ctx.client.calculate_total_maintenance_cost(&8), 100); + assert_eq!(ctx.client.get_maintenance_history(&7).len(), 1); + assert_eq!(ctx.client.get_maintenance_history(&8).len(), 1); +} + +#[test] +fn records_for_different_assets_do_not_share_history() { + let env = Env::default(); + let ctx = setup(&env); + + ctx.client + .add_maintenance_record(&record(&env, &ctx.provider, 1, 7)); + + assert_eq!(ctx.client.get_maintenance_history(&8).len(), 0); +} + +#[test] +fn parts_replaced_and_documents_round_trip() { + let env = Env::default(); + let ctx = setup(&env); + let r = record(&env, &ctx.provider, 1, 7); + + ctx.client.add_maintenance_record(&r); + + let stored = ctx.client.get_maintenance_history(&7).get(0).unwrap(); + let expected_parts: Vec = vec![&env, String::from_str(&env, "filter")]; + assert_eq!(stored.parts_replaced, expected_parts); + assert_eq!(stored.documents_ipfs.len(), 1); + assert_eq!(stored.technician_id, String::from_str(&env, "tech-01")); +} diff --git a/contracts/assetsup/src/dividends.rs b/contracts/assetsup/src/dividends.rs index 9ba0819ee..e24a31e1c 100644 --- a/contracts/assetsup/src/dividends.rs +++ b/contracts/assetsup/src/dividends.rs @@ -1,4 +1,5 @@ use crate::error::Error; +use crate::math; use crate::types::{OwnershipRecord, TokenDataKey, TokenizedAsset}; use soroban_sdk::{Address, Env, Vec}; @@ -27,11 +28,19 @@ pub fn distribute_dividends(env: &Env, asset_id: u64, total_amount: i128) -> Res let holder_key = TokenDataKey::TokenHolder(asset_id, holder.clone()); let mut ownership: OwnershipRecord = store.get(&holder_key).ok_or(Error::HolderNotFound)?; - // Calculate proportional dividend: (balance / total_supply) * total_amount - let proportion = (ownership.balance * total_amount) / tokenized_asset.total_supply; - - // Add to unclaimed dividends - ownership.unclaimed_dividends += proportion; + // Proportional dividend: balance/total_supply of total_amount. + // + // Written naively as (balance * total_amount) / total_supply this + // overflows whenever the intermediate product exceeds i128::MAX, even + // though the quotient is small. Rounds down; the remainder stays with + // the contract. See crate::math for the rounding rationale. + let proportion = math::mul_div( + ownership.balance, + total_amount, + tokenized_asset.total_supply, + )?; + + ownership.unclaimed_dividends = math::add(ownership.unclaimed_dividends, proportion)?; store.set(&holder_key, &ownership); } diff --git a/contracts/assetsup/src/lib.rs b/contracts/assetsup/src/lib.rs index 7c220273f..040e33bca 100644 --- a/contracts/assetsup/src/lib.rs +++ b/contracts/assetsup/src/lib.rs @@ -14,8 +14,10 @@ pub(crate) mod dividends; pub(crate) mod error; pub(crate) mod insurance; pub(crate) mod lease; +pub(crate) mod math; pub(crate) mod tokenization; pub(crate) mod transfer_restrictions; +pub(crate) mod ttl; pub(crate) mod types; pub(crate) mod voting; @@ -43,6 +45,7 @@ pub struct AssetUpContract; impl AssetUpContract { pub fn initialize(env: Env, admin: Address) -> Result<(), Error> { admin.require_auth(); + ttl::extend_instance(&env); if env.storage().persistent().has(&DataKey::Admin) { handle_error(&env, Error::AlreadyInitialized) @@ -73,20 +76,38 @@ impl AssetUpContract { .persistent() .set(&DataKey::AuthorizedRegistrar(admin.clone()), &true); + // Extend on write, not only on read. A freshly written entry gets the + // network's minimum lifetime, which is short; without this the whole + // contract configuration can be archived before anyone reads it, and a + // read cannot rescue an entry that is already gone. + ttl::extend_persistent(&env, &DataKey::Admin); + ttl::extend_persistent(&env, &DataKey::Paused); + ttl::extend_persistent(&env, &DataKey::TotalAssetCount); + ttl::extend_persistent(&env, &DataKey::ContractMetadata); + ttl::extend_persistent(&env, &DataKey::AuthorizedRegistrar(admin)); + Ok(()) } pub fn get_admin(env: Env) -> Result { + ttl::extend_instance(&env); + let key = DataKey::Admin; if !env.storage().persistent().has(&key) { handle_error(&env, Error::AdminNotFound) } + // Contract-level configuration lives in persistent storage, so it + // needs extending like any other persistent entry — the instance bump + // alone does not cover it. + ttl::extend_persistent(&env, &key); + let admin = env.storage().persistent().get(&key).unwrap(); Ok(admin) } pub fn is_paused(env: Env) -> Result { + ttl::extend_persistent(&env, &DataKey::Paused); Ok(env .storage() .persistent() @@ -95,6 +116,7 @@ impl AssetUpContract { } pub fn get_total_asset_count(env: Env) -> Result { + ttl::extend_persistent(&env, &DataKey::TotalAssetCount); Ok(env .storage() .persistent() @@ -103,6 +125,7 @@ impl AssetUpContract { } pub fn get_contract_metadata(env: Env) -> Result { + ttl::extend_persistent(&env, &DataKey::ContractMetadata); let metadata = env.storage().persistent().get(&DataKey::ContractMetadata); match metadata { Some(m) => Ok(m), @@ -111,15 +134,21 @@ impl AssetUpContract { } pub fn is_authorized_registrar(env: Env, address: Address) -> Result { - Ok(env - .storage() - .persistent() - .get(&DataKey::AuthorizedRegistrar(address)) - .unwrap_or(false)) + let key = DataKey::AuthorizedRegistrar(address); + ttl::extend_persistent(&env, &key); + Ok(env.storage().persistent().get(&key).unwrap_or(false)) } // Asset functions pub fn register_asset(env: Env, asset: asset::Asset, caller: Address) -> Result<(), Error> { + ttl::extend_instance(&env); + + // Authenticate the caller before trusting `caller` for anything else. + // The registrar check below compares against a caller-supplied address, + // so without this any account could name an authorized registrar and + // pass it. + caller.require_auth(); + // Check if contract is paused if Self::is_paused(env.clone())? { return Err(Error::ContractPaused); @@ -143,6 +172,7 @@ impl AssetUpContract { // Store asset store.set(&key, &asset); + ttl::extend_persistent(&env, &key); // Update owner registry let owner_key = asset::DataKey::OwnerRegistry(asset.owner.clone()); @@ -220,6 +250,12 @@ impl AssetUpContract { new_custom_attributes: Option>, caller: Address, ) -> Result<(), Error> { + ttl::extend_instance(&env); + + // Authenticate before the owner/admin comparison below, which would + // otherwise be satisfied by simply naming the owner's address. + caller.require_auth(); + // Check if contract is paused if Self::is_paused(env.clone())? { return Err(Error::ContractPaused); @@ -256,6 +292,7 @@ impl AssetUpContract { } store.set(&key, &asset); + ttl::extend_persistent(&env, &key); // Append audit log audit::append_audit_log( @@ -281,6 +318,13 @@ impl AssetUpContract { new_owner: Address, caller: Address, ) -> Result<(), Error> { + ttl::extend_instance(&env); + + // Authenticate before the ownership comparison below. Without this, + // anyone could pass the current owner's address and take the asset — + // a direct asset-theft path. + caller.require_auth(); + // Check if contract is paused if Self::is_paused(env.clone())? { return Err(Error::ContractPaused); @@ -331,6 +375,7 @@ impl AssetUpContract { asset.last_transfer_timestamp = env.ledger().timestamp(); asset.status = AssetStatus::Transferred; store.set(&key, &asset); + ttl::extend_persistent(&env, &key); // Append audit log audit::append_audit_log( @@ -351,6 +396,11 @@ impl AssetUpContract { } pub fn retire_asset(env: Env, asset_id: BytesN<32>, caller: Address) -> Result<(), Error> { + ttl::extend_instance(&env); + + // Authenticate before the owner/admin comparison below. + caller.require_auth(); + // Check if contract is paused if Self::is_paused(env.clone())? { return Err(Error::ContractPaused); @@ -372,6 +422,7 @@ impl AssetUpContract { asset.status = AssetStatus::Retired; store.set(&key, &asset); + ttl::extend_persistent(&env, &key); // Append audit log audit::append_audit_log( @@ -392,10 +443,18 @@ impl AssetUpContract { } pub fn get_asset(env: Env, asset_id: BytesN<32>) -> Result { + ttl::extend_instance(&env); + let key = asset::DataKey::Asset(asset_id); let store = env.storage().persistent(); match store.get::<_, asset::Asset>(&key) { - Some(a) => Ok(a), + Some(a) => { + // Extend on read too. An asset that is only ever queried and + // never modified would otherwise be archived despite being in + // active use. + ttl::extend_persistent(&env, &key); + Ok(a) + } None => Err(Error::AssetNotFound), } } @@ -410,6 +469,8 @@ impl AssetUpContract { } pub fn check_asset_exists(env: Env, asset_id: BytesN<32>) -> Result { + ttl::extend_instance(&env); + let key = asset::DataKey::Asset(asset_id); let store = env.storage().persistent(); Ok(store.has(&key)) diff --git a/contracts/assetsup/src/math.rs b/contracts/assetsup/src/math.rs new file mode 100644 index 000000000..0353460be --- /dev/null +++ b/contracts/assetsup/src/math.rs @@ -0,0 +1,181 @@ +//! Checked arithmetic for value-carrying paths ([SC-43]). +//! +//! `overflow-checks = true` is set on the release profile, which turns an +//! overflow into a **panic** — a trapped transaction the caller cannot +//! distinguish from any other trap. On paths that move asset value, share +//! counts, dividends, or percentages, an explicit typed error is far more +//! useful: the caller learns *what* went wrong. +//! +//! Every helper here maps failure to [`Error::MathOverflow`] or +//! [`Error::MathUnderflow`] rather than trapping. +//! +//! ## Rounding +//! +//! [`mul_div`] truncates toward zero — the same direction as Rust's `/` on +//! non-negative operands, so it rounds **down** for the non-negative values +//! these contracts deal in. +//! +//! The remainder is **not** distributed. For a dividend split this means the +//! contract retains up to `holder_count - 1` stroops per distribution rather +//! than paying them to an arbitrarily chosen holder. That is the deliberate +//! choice: rounding in favour of the contract can never overpay, and picking a +//! holder to absorb the remainder would silently advantage whoever happens to +//! be first in iteration order. +//! +//! For an ownership percentage in basis points, rounding down means the +//! reported percentages can sum to slightly less than 10000. They are a +//! derived display value; the authoritative figure is always `balance`. + +use crate::error::Error; + +/// `a + b`, or [`Error::MathOverflow`]. +pub fn add(a: i128, b: i128) -> Result { + a.checked_add(b).ok_or(Error::MathOverflow) +} + +/// `a - b`, or [`Error::MathUnderflow`]. +pub fn sub(a: i128, b: i128) -> Result { + a.checked_sub(b).ok_or(Error::MathUnderflow) +} + +/// `a * b`, or [`Error::MathOverflow`]. +/// +/// Not currently called — [`mul_div`] covers every multiplication on a +/// value-carrying path today. Kept so the checked-arithmetic surface is +/// complete and a future caller reaches for it rather than a bare `*`. +#[allow(dead_code)] +pub fn mul(a: i128, b: i128) -> Result { + a.checked_mul(b).ok_or(Error::MathOverflow) +} + +/// `(a * b) / d`, rounding toward zero. +/// +/// The naive spelling `(a * b) / d` overflows whenever the intermediate +/// product exceeds `i128::MAX`, even when the final result is small — which is +/// exactly the case for proportional splits, where `balance * total_amount` is +/// large but the quotient is not. This computes the same value without +/// trapping, and returns [`Error::MathOverflow`] when the product genuinely +/// cannot be represented. +/// +/// Returns [`Error::MathOverflow`] if `d` is zero, since a zero denominator +/// here always means a corrupt total supply rather than a caller mistake. +pub fn mul_div(a: i128, b: i128, d: i128) -> Result { + if d == 0 { + return Err(Error::MathOverflow); + } + + // The common case: the product fits, so compute it directly. + if let Some(product) = a.checked_mul(b) { + return Ok(product / d); + } + + // The product does not fit. Reduce the fraction before multiplying by + // cancelling the greatest common divisor of each operand with the + // denominator, then retry. This rescues the realistic case where the + // denominator shares factors with one of the operands — a holder balance + // against a total supply, for instance. + let g1 = gcd(a.unsigned_abs(), d.unsigned_abs()); + let (a, d) = (a / g1 as i128, d / g1 as i128); + + let g2 = gcd(b.unsigned_abs(), d.unsigned_abs()); + let (b, d) = (b / g2 as i128, d / g2 as i128); + + match a.checked_mul(b) { + Some(product) => Ok(product / d), + None => Err(Error::MathOverflow), + } +} + +fn gcd(mut a: u128, mut b: u128) -> u128 { + while b != 0 { + let t = b; + b = a % b; + a = t; + } + if a == 0 { + 1 + } else { + a + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn add_reports_overflow_instead_of_trapping() { + assert_eq!(add(1, 2), Ok(3)); + assert_eq!(add(i128::MAX, 0), Ok(i128::MAX)); + assert_eq!(add(i128::MAX, 1), Err(Error::MathOverflow)); + } + + #[test] + fn sub_reports_underflow_instead_of_trapping() { + assert_eq!(sub(3, 2), Ok(1)); + assert_eq!(sub(0, 0), Ok(0)); + assert_eq!(sub(i128::MIN, 1), Err(Error::MathUnderflow)); + } + + #[test] + fn mul_reports_overflow_instead_of_trapping() { + assert_eq!(mul(0, i128::MAX), Ok(0)); + assert_eq!(mul(1, i128::MAX), Ok(i128::MAX)); + assert_eq!(mul(2, i128::MAX), Err(Error::MathOverflow)); + } + + #[test] + fn mul_div_handles_the_ordinary_case() { + // A 25% holder of a 1000-token supply receives 25% of 400. + assert_eq!(mul_div(250, 400, 1000), Ok(100)); + } + + #[test] + fn mul_div_survives_an_intermediate_product_that_would_overflow() { + // balance * total_amount overflows i128, but the result is small. + // Written naively as (a * b) / d this traps. + let balance = i128::MAX / 2; + let total_amount = 1000i128; + let total_supply = i128::MAX / 2; + + assert!(balance.checked_mul(total_amount).is_none()); + assert_eq!(mul_div(balance, total_amount, total_supply), Ok(1000)); + } + + #[test] + fn mul_div_rounds_down() { + // 1 of 3 holders splitting 10: each gets 3, one unit is retained. + assert_eq!(mul_div(1, 10, 3), Ok(3)); + assert_eq!(mul_div(2, 10, 3), Ok(6)); + // The remainder stays with the contract rather than being handed to a + // holder chosen by iteration order. + assert_eq!(mul_div(1, 10, 3).unwrap() * 3, 9); + } + + #[test] + fn mul_div_boundaries_at_zero_and_one() { + assert_eq!(mul_div(0, 1000, 10), Ok(0)); + assert_eq!(mul_div(1, 0, 10), Ok(0)); + assert_eq!(mul_div(1, 1, 1), Ok(1)); + assert_eq!(mul_div(i128::MAX, 1, i128::MAX), Ok(1)); + } + + #[test] + fn mul_div_rejects_a_zero_denominator() { + assert_eq!(mul_div(1, 1, 0), Err(Error::MathOverflow)); + } + + #[test] + fn mul_div_reports_overflow_when_the_result_truly_cannot_fit() { + // Coprime operands with a denominator of 1: nothing can be cancelled + // and the product genuinely does not fit. + assert_eq!(mul_div(i128::MAX, 3, 1), Err(Error::MathOverflow)); + } + + #[test] + fn gcd_is_well_behaved_at_zero() { + assert_eq!(gcd(0, 0), 1); + assert_eq!(gcd(0, 5), 5); + assert_eq!(gcd(12, 18), 6); + } +} diff --git a/contracts/assetsup/src/tests/auth.rs b/contracts/assetsup/src/tests/auth.rs new file mode 100644 index 000000000..032a63fe3 --- /dev/null +++ b/contracts/assetsup/src/tests/auth.rs @@ -0,0 +1,332 @@ +//! Authorization tests ([SC-42]). +//! +//! Every test here runs **without** `mock_all_auths`, which is the only way to +//! prove an entrypoint actually authenticates. With auths mocked, an entrypoint +//! that never calls `require_auth` is indistinguishable from one that does. +//! +//! The regression these guard against: `register_asset`, +//! `update_asset_metadata`, `transfer_asset_ownership` and `retire_asset` each +//! take a `caller: Address` and compare it against a registrar allowlist, the +//! asset owner, or the admin — but originally never called +//! `caller.require_auth()`. Because `caller` is supplied by whoever builds the +//! transaction, the comparison could be satisfied by simply naming a privileged +//! address. `transfer_asset_ownership` was a direct asset-theft path. + +use soroban_sdk::testutils::Address as _; +use soroban_sdk::{Address, BytesN, Env, IntoVal, String, Vec}; + +use super::helpers::{create_env, create_test_asset}; +use crate::{AssetUpContract, AssetUpContractClient}; + +/// Registers the contract and initializes it, then clears the mocked auths so +/// every subsequent call must carry real authorization. +fn setup_unmocked(env: &Env) -> (AssetUpContractClient<'_>, Address) { + let admin = Address::generate(env); + let contract_id = env.register(AssetUpContract, ()); + let client = AssetUpContractClient::new(env, &contract_id); + + env.mock_all_auths(); + client.initialize(&admin); + env.set_auths(&[]); + + (client, admin) +} + +fn asset_id(env: &Env, seed: u8) -> BytesN<32> { + BytesN::from_array(env, &[seed; 32]) +} + +// --------------------------------------------------------------------------- +// initialize +// --------------------------------------------------------------------------- + +#[test] +fn initialize_requires_the_admins_authorization() { + let env = create_env(); + let admin = Address::generate(&env); + let contract_id = env.register(AssetUpContract, ()); + let client = AssetUpContractClient::new(&env, &contract_id); + + let res = client.try_initialize(&admin); + assert!( + res.is_err(), + "initialize must not succeed without the admin's authorization" + ); +} + +// --------------------------------------------------------------------------- +// register_asset +// --------------------------------------------------------------------------- + +#[test] +fn register_asset_rejects_an_unauthenticated_caller() { + let env = create_env(); + let (client, admin) = setup_unmocked(&env); + let owner = Address::generate(&env); + let asset = create_test_asset(&env, &owner, asset_id(&env, 1)); + + // `admin` is an authorized registrar, but this call carries no signature + // from them. + let res = client.try_register_asset(&asset, &admin); + assert!( + res.is_err(), + "naming an authorized registrar must not be enough to register an asset" + ); +} + +#[test] +fn register_asset_succeeds_once_the_caller_authenticates() { + let env = create_env(); + let (client, admin) = setup_unmocked(&env); + let owner = Address::generate(&env); + let id = asset_id(&env, 2); + let asset = create_test_asset(&env, &owner, id.clone()); + + env.mock_all_auths(); + client.register_asset(&asset, &admin); + + assert_eq!(client.get_asset(&id).owner, owner); +} + +// --------------------------------------------------------------------------- +// transfer_asset_ownership — the asset-theft path +// --------------------------------------------------------------------------- + +#[test] +fn transfer_asset_ownership_rejects_an_unauthenticated_caller() { + let env = create_env(); + let (client, admin) = setup_unmocked(&env); + let owner = Address::generate(&env); + let id = asset_id(&env, 3); + + env.mock_all_auths(); + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + + // An attacker names the real owner as `caller` and tries to move the asset + // to themselves. Without authentication this would succeed. + env.set_auths(&[]); + let attacker = Address::generate(&env); + let res = client.try_transfer_asset_ownership(&id, &attacker, &owner); + + assert!( + res.is_err(), + "naming the owner must not be enough to transfer their asset" + ); + assert_eq!( + client.get_asset(&id).owner, + owner, + "ownership must be unchanged after the rejected transfer" + ); +} + +#[test] +fn transfer_asset_ownership_succeeds_for_the_authenticated_owner() { + let env = create_env(); + let (client, admin) = setup_unmocked(&env); + let owner = Address::generate(&env); + let new_owner = Address::generate(&env); + let id = asset_id(&env, 4); + + env.mock_all_auths(); + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + client.transfer_asset_ownership(&id, &new_owner, &owner); + + assert_eq!(client.get_asset(&id).owner, new_owner); +} + +// --------------------------------------------------------------------------- +// update_asset_metadata +// --------------------------------------------------------------------------- + +#[test] +fn update_asset_metadata_rejects_an_unauthenticated_caller() { + let env = create_env(); + let (client, admin) = setup_unmocked(&env); + let owner = Address::generate(&env); + let id = asset_id(&env, 5); + + env.mock_all_auths(); + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + + env.set_auths(&[]); + let res = client.try_update_asset_metadata( + &id, + &Some(String::from_str(&env, "rewritten")), + &None, + &None, + &owner, + ); + + assert!( + res.is_err(), + "naming the owner must not be enough to rewrite their asset's metadata" + ); +} + +// --------------------------------------------------------------------------- +// retire_asset +// --------------------------------------------------------------------------- + +#[test] +fn retire_asset_rejects_an_unauthenticated_caller() { + let env = create_env(); + let (client, admin) = setup_unmocked(&env); + let owner = Address::generate(&env); + let id = asset_id(&env, 6); + + env.mock_all_auths(); + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + + env.set_auths(&[]); + let res = client.try_retire_asset(&id, &owner); + + assert!( + res.is_err(), + "naming the owner must not be enough to retire their asset" + ); + assert_eq!( + client.get_asset(&id).status, + crate::types::AssetStatus::Active, + "the asset must still be active after the rejected retire" + ); +} + +// --------------------------------------------------------------------------- +// Admin-gated entrypoints +// --------------------------------------------------------------------------- + +#[test] +fn update_admin_requires_the_current_admins_authorization() { + let env = create_env(); + let (client, _admin) = setup_unmocked(&env); + let usurper = Address::generate(&env); + + let res = client.try_update_admin(&usurper); + assert!( + res.is_err(), + "admin transfer must not succeed without the current admin's auth" + ); +} + +#[test] +fn add_authorized_registrar_requires_the_admins_authorization() { + let env = create_env(); + let (client, _admin) = setup_unmocked(&env); + let candidate = Address::generate(&env); + + let res = client.try_add_authorized_registrar(&candidate); + assert!( + res.is_err(), + "granting registrar rights must require the admin's auth" + ); + // And the grant must not have taken effect. + assert!(!client.is_authorized_registrar(&candidate)); +} + +#[test] +fn remove_authorized_registrar_requires_the_admins_authorization() { + let env = create_env(); + let (client, _admin) = setup_unmocked(&env); + let candidate = Address::generate(&env); + + env.mock_all_auths(); + client.add_authorized_registrar(&candidate); + assert!(client.is_authorized_registrar(&candidate)); + + env.set_auths(&[]); + let res = client.try_remove_authorized_registrar(&candidate); + assert!( + res.is_err(), + "revoking registrar rights must require the admin's auth" + ); + assert!(client.is_authorized_registrar(&candidate)); +} + +#[test] +fn pause_and_unpause_require_the_admins_authorization() { + let env = create_env(); + let (client, _admin) = setup_unmocked(&env); + + assert!( + client.try_pause_contract().is_err(), + "pause must require the admin's auth" + ); + assert!(!client.is_paused()); + + env.mock_all_auths(); + client.pause_contract(); + assert!(client.is_paused()); + + env.set_auths(&[]); + assert!( + client.try_unpause_contract().is_err(), + "unpause must require the admin's auth" + ); + assert!(client.is_paused(), "the contract must still be paused"); +} + +// --------------------------------------------------------------------------- +// Reads stay open +// --------------------------------------------------------------------------- + +#[test] +fn read_entrypoints_do_not_require_authorization() { + // The audit is only useful if it distinguishes "protected" from "locked + // down"; reads must stay callable by anyone. + let env = create_env(); + let (client, admin) = setup_unmocked(&env); + let owner = Address::generate(&env); + let id = asset_id(&env, 7); + + env.mock_all_auths(); + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + env.set_auths(&[]); + + assert_eq!(client.get_asset(&id).owner, owner); + assert!(client.check_asset_exists(&id)); + assert_eq!(client.get_total_asset_count(), 1); + assert_eq!(client.get_admin(), admin); + assert!(!client.is_paused()); + assert_eq!(client.get_assets_by_owner(&owner).len(), 1); + assert_eq!(client.get_asset_info(&id).id, id); + + let ids: Vec> = Vec::from_array(&env, [id.clone()]); + assert_eq!(client.batch_get_asset_info(&ids).len(), 1); +} + +// --------------------------------------------------------------------------- +// The auth is bound to the right principal, not just "some" auth +// --------------------------------------------------------------------------- + +#[test] +fn a_third_party_signature_does_not_authorize_a_transfer() { + // `mock_auths` grants exactly one address's authorization. An attacker who + // can sign for themselves must still not be able to move someone else's + // asset by naming the owner as `caller`. + let env = create_env(); + let (client, admin) = setup_unmocked(&env); + let owner = Address::generate(&env); + let attacker = Address::generate(&env); + let id = asset_id(&env, 8); + + env.mock_all_auths(); + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + + env.set_auths(&[soroban_sdk::testutils::MockAuth { + address: &attacker, + invoke: &soroban_sdk::testutils::MockAuthInvoke { + contract: &client.address, + fn_name: "transfer_asset_ownership", + args: (id.clone(), attacker.clone(), owner.clone()).into_val(&env), + sub_invokes: &[], + }, + } + .into()]); + + let res = client.try_transfer_asset_ownership(&id, &attacker, &owner); + assert!( + res.is_err(), + "the attacker's own signature must not authorize the owner's transfer" + ); + assert_eq!(client.get_asset(&id).owner, owner); +} diff --git a/contracts/assetsup/src/tests/mod.rs b/contracts/assetsup/src/tests/mod.rs index a44a81fdf..0f87bfca6 100644 --- a/contracts/assetsup/src/tests/mod.rs +++ b/contracts/assetsup/src/tests/mod.rs @@ -5,6 +5,7 @@ mod helpers; mod admin; mod asset; mod audit_trail; +mod auth; mod initialization; // Tokenization and ownership tests @@ -28,3 +29,6 @@ mod integration; mod tokenization_new; mod transfer_restrictions_new; mod voting_new; + +// Storage TTL policy tests +mod ttl; diff --git a/contracts/assetsup/src/tests/ttl.rs b/contracts/assetsup/src/tests/ttl.rs new file mode 100644 index 000000000..30ff2abed --- /dev/null +++ b/contracts/assetsup/src/tests/ttl.rs @@ -0,0 +1,116 @@ +//! Storage TTL tests ([SC-44]). +//! +//! These advance the ledger past the default entry lifetime and prove critical +//! registry entries are still readable. Without the `extend_ttl` calls in +//! `lib.rs` these fail: Soroban archives the entry and the asset simply +//! disappears from the contract's view. + +use soroban_sdk::testutils::{Address as _, Ledger as _, LedgerInfo}; +use soroban_sdk::{Address, BytesN, Env}; + +use super::helpers::{create_env, create_test_asset, initialize_contract}; +use crate::ttl::{LEDGERS_PER_DAY, PERSISTENT_EXTEND_TO}; +use crate::AssetUpContractClient; + +fn asset_id(env: &Env, seed: u8) -> BytesN<32> { + BytesN::from_array(env, &[seed; 32]) +} + +/// Sets a ledger with a short default entry lifetime so archival is reachable +/// within a test, then advances by `ledgers`. +fn advance_ledgers(env: &Env, ledgers: u32) { + env.ledger().with_mut(|li: &mut LedgerInfo| { + li.sequence_number += ledgers; + li.timestamp += (ledgers as u64) * 5; + }); +} + +fn setup(env: &Env) -> (AssetUpContractClient<'_>, Address) { + // A generous max entry TTL so the contract's own extension is what keeps + // entries alive, rather than the harness silently capping it. + env.ledger().with_mut(|li: &mut LedgerInfo| { + li.sequence_number = 1; + li.min_persistent_entry_ttl = 100; + li.min_temp_entry_ttl = 16; + li.max_entry_ttl = PERSISTENT_EXTEND_TO + LEDGERS_PER_DAY; + }); + + let admin = Address::generate(env); + let client = initialize_contract(env, &admin); + (client, admin) +} + +#[test] +fn a_registered_asset_survives_past_the_default_entry_lifetime() { + let env = create_env(); + let (client, admin) = setup(&env); + let owner = Address::generate(&env); + let id = asset_id(&env, 1); + + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + + // Well past the 100-ledger default set above, and past the point where an + // unextended entry would have been archived. + advance_ledgers(&env, 60 * LEDGERS_PER_DAY); + + let asset = client.get_asset(&id); + assert_eq!( + asset.owner, owner, + "the ownership record must still be readable" + ); +} + +#[test] +fn reading_an_asset_keeps_it_alive() { + // The property that makes read-side extension necessary: an asset that is + // queried but never modified must not be archived. + let env = create_env(); + let (client, admin) = setup(&env); + let owner = Address::generate(&env); + let id = asset_id(&env, 2); + + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + + // Three hops, each shorter than the extension window, reading each time. + // Cumulatively they exceed the window, so only read-side extension keeps + // the entry alive to the end. + for _ in 0..3 { + advance_ledgers(&env, 45 * LEDGERS_PER_DAY); + let asset = client.get_asset(&id); + assert_eq!(asset.owner, owner); + } + + assert!(client.check_asset_exists(&id)); +} + +#[test] +fn a_transferred_asset_survives_past_the_default_lifetime() { + let env = create_env(); + let (client, admin) = setup(&env); + let owner = Address::generate(&env); + let new_owner = Address::generate(&env); + let id = asset_id(&env, 3); + + client.register_asset(&create_test_asset(&env, &owner, id.clone()), &admin); + client.transfer_asset_ownership(&id, &new_owner, &owner); + + advance_ledgers(&env, 60 * LEDGERS_PER_DAY); + + assert_eq!( + client.get_asset(&id).owner, + new_owner, + "the post-transfer ownership record must survive" + ); +} + +#[test] +fn contract_configuration_survives_past_the_default_lifetime() { + let env = create_env(); + let (client, admin) = setup(&env); + + advance_ledgers(&env, 60 * LEDGERS_PER_DAY); + + assert_eq!(client.get_admin(), admin, "admin must still be readable"); + assert!(!client.is_paused()); + assert!(client.is_authorized_registrar(&admin)); +} diff --git a/contracts/assetsup/src/tokenization.rs b/contracts/assetsup/src/tokenization.rs index 7402eb8d0..beb71973d 100644 --- a/contracts/assetsup/src/tokenization.rs +++ b/contracts/assetsup/src/tokenization.rs @@ -1,5 +1,6 @@ use crate::audit; use crate::error::Error; +use crate::math; use crate::types::{OwnershipRecord, TokenDataKey, TokenMetadata, TokenizedAsset}; use soroban_sdk::{Address, BytesN, Env, String, Vec}; @@ -126,19 +127,21 @@ pub fn mint_tokens( } // Update total supply - tokenized_asset.total_supply += amount; - tokenized_asset.tokens_in_circulation += amount; + tokenized_asset.total_supply = math::add(tokenized_asset.total_supply, amount)?; + tokenized_asset.tokens_in_circulation = + math::add(tokenized_asset.tokens_in_circulation, amount)?; // Update tokenizer's ownership let holder_key = TokenDataKey::TokenHolder(asset_id, minter.clone()); let mut ownership: OwnershipRecord = store.get(&holder_key).ok_or(Error::HolderNotFound)?; - ownership.balance += amount; + ownership.balance = math::add(ownership.balance, amount)?; ownership.voting_power = ownership.balance; ownership.dividend_entitlement = ownership.balance; // Recalculate ownership percentage - ownership.ownership_percentage = (ownership.balance * 10000) / tokenized_asset.total_supply; + ownership.ownership_percentage = + math::mul_div(ownership.balance, 10000, tokenized_asset.total_supply)?; store.set(&holder_key, &ownership); store.set(&key, &tokenized_asset.clone()); @@ -194,15 +197,17 @@ pub fn burn_tokens( } // Update balances - ownership.balance -= amount; + ownership.balance = math::sub(ownership.balance, amount)?; ownership.voting_power = ownership.balance; ownership.dividend_entitlement = ownership.balance; // Recalculate ownership percentage - ownership.ownership_percentage = (ownership.balance * 10000) / tokenized_asset.total_supply; + ownership.ownership_percentage = + math::mul_div(ownership.balance, 10000, tokenized_asset.total_supply)?; - tokenized_asset.total_supply -= amount; - tokenized_asset.tokens_in_circulation -= amount; + tokenized_asset.total_supply = math::sub(tokenized_asset.total_supply, amount)?; + tokenized_asset.tokens_in_circulation = + math::sub(tokenized_asset.tokens_in_circulation, amount)?; store.set(&holder_key, &ownership); store.set(&key, &tokenized_asset.clone()); @@ -282,17 +287,17 @@ pub fn transfer_tokens( }; // Update balances - from_ownership.balance -= amount; + from_ownership.balance = math::sub(from_ownership.balance, amount)?; from_ownership.voting_power = from_ownership.balance; from_ownership.dividend_entitlement = from_ownership.balance; from_ownership.ownership_percentage = - (from_ownership.balance * 10000) / tokenized_asset.total_supply; + math::mul_div(from_ownership.balance, 10000, tokenized_asset.total_supply)?; - to_ownership.balance += amount; + to_ownership.balance = math::add(to_ownership.balance, amount)?; to_ownership.voting_power = to_ownership.balance; to_ownership.dividend_entitlement = to_ownership.balance; to_ownership.ownership_percentage = - (to_ownership.balance * 10000) / tokenized_asset.total_supply; + math::mul_div(to_ownership.balance, 10000, tokenized_asset.total_supply)?; store.set(&from_holder_key, &from_ownership); store.set(&to_holder_key, &to_ownership); @@ -433,7 +438,7 @@ pub fn calculate_ownership_percentage( return Ok(0); } - Ok((ownership.balance * 10000) / tokenized_asset.total_supply) + math::mul_div(ownership.balance, 10000, tokenized_asset.total_supply) } /// Get tokenized asset details diff --git a/contracts/assetsup/src/ttl.rs b/contracts/assetsup/src/ttl.rs new file mode 100644 index 000000000..a31222d31 --- /dev/null +++ b/contracts/assetsup/src/ttl.rs @@ -0,0 +1,84 @@ +//! Storage time-to-live policy ([SC-44]). +//! +//! Soroban charges rent and **archives entries whose TTL lapses**. An archived +//! asset registry entry is a correctness bug, not a performance concern: the +//! ownership record simply stops being readable until someone restores it. +//! +//! All TTL constants live here rather than being scattered as magic numbers at +//! call sites, so the policy can be reviewed in one place. +//! +//! ## Durability choices +//! +//! | Data | Durability | Why | +//! |---|---|---| +//! | Asset records, ownership, token balances, leases, policies | `persistent` | Must outlive any single session. Losing one loses the ownership record. | +//! | Admin, pause flag, registrar allowlist, counters, metadata | `persistent` | Contract-level configuration with the same lifetime as the contract. | +//! | — | `temporary` | Nothing in this contract is short-lived enough to justify it. Long-lived data in `temporary` is the classic archival bug. | +//! +//! ## Policy +//! +//! Every read and write of a persistent entry extends its TTL. Extending on +//! **read** as well as write is the important part: an asset that is only ever +//! queried, never modified, would otherwise expire despite being actively used. +//! +//! The threshold/extend pair follows the usual shape — if the entry has fewer +//! than `*_THRESHOLD` ledgers left, push it back out to `*_EXTEND_TO`. Setting +//! the threshold below the target means the extension is a no-op on most calls +//! and only costs rent when an entry is genuinely approaching expiry. + +use soroban_sdk::{Env, IntoVal, Val}; + +/// Ledgers per day, at the nominal 5 second close time. +pub const LEDGERS_PER_DAY: u32 = 17_280; + +/// Bump persistent entries when they have less than 30 days left... +pub const PERSISTENT_THRESHOLD: u32 = 30 * LEDGERS_PER_DAY; +/// ...back out to 90 days. +/// +/// Chosen so a contract that goes untouched for a full quarter still retains +/// its registry, and so that routine traffic keeps entries alive without +/// paying to extend on every call. +pub const PERSISTENT_EXTEND_TO: u32 = 90 * LEDGERS_PER_DAY; + +/// Instance storage (contract-level config) uses the same window. If the +/// instance is archived the contract is unusable regardless of what else +/// survives, so it should never be the first thing to lapse. +pub const INSTANCE_THRESHOLD: u32 = PERSISTENT_THRESHOLD; +pub const INSTANCE_EXTEND_TO: u32 = PERSISTENT_EXTEND_TO; + +/// Extends a persistent entry's TTL, if it exists. +/// +/// Safe to call on a key that has never been written; `extend_ttl` on a +/// missing entry would trap, so existence is checked first. +pub fn extend_persistent(env: &Env, key: &K) +where + K: IntoVal + Clone, +{ + let store = env.storage().persistent(); + if store.has(key) { + store.extend_ttl(key, PERSISTENT_THRESHOLD, PERSISTENT_EXTEND_TO); + } +} + +/// Bumps the contract instance TTL. +/// +/// Call from every entrypoint that touches instance storage, including reads. +pub fn extend_instance(env: &Env) { + env.storage() + .instance() + .extend_ttl(INSTANCE_THRESHOLD, INSTANCE_EXTEND_TO); +} + +/// The extension target must be further out than the trigger point, otherwise +/// every call would pay to extend while never actually pushing the entry out. +/// +/// Checked at compile time; the values are constants, so a runtime assertion +/// would be trivially true and clippy rightly rejects it. +const _: () = { + assert!(PERSISTENT_THRESHOLD < PERSISTENT_EXTEND_TO); + assert!(INSTANCE_THRESHOLD < INSTANCE_EXTEND_TO); + // A quarter of headroom, so a registry is not archived during a quiet + // period. + assert!(PERSISTENT_EXTEND_TO == 90 * LEDGERS_PER_DAY); + assert!(PERSISTENT_THRESHOLD >= 30 * LEDGERS_PER_DAY); +}; diff --git a/contracts/contrib/src/lib.rs b/contracts/contrib/src/lib.rs index 539797f3d..ba98f21d3 100644 --- a/contracts/contrib/src/lib.rs +++ b/contracts/contrib/src/lib.rs @@ -62,6 +62,10 @@ pub struct ContribContract; impl ContribContract { /// Initialize the contract with an admin. pub fn initialize(env: Env, admin: Address) { + // Without this, whoever calls initialize first becomes admin of a + // freshly deployed contract, regardless of who deployed it. + admin.require_auth(); + if env.storage().persistent().has(&DataKey::Admin) { panic!("Already initialized"); } diff --git a/contracts/contrib/src/tests/asset_registry_tests.rs b/contracts/contrib/src/tests/asset_registry_tests.rs index b00991fa2..7ac7bfee5 100644 --- a/contracts/contrib/src/tests/asset_registry_tests.rs +++ b/contracts/contrib/src/tests/asset_registry_tests.rs @@ -7,6 +7,8 @@ fn setup_test(env: &Env) -> (ContribContractClient<'_>, Address) { let admin = Address::generate(env); let contract_id = env.register(ContribContract, ()); let client = ContribContractClient::new(env, &contract_id); + // initialize now requires the admin's authorization. + env.mock_all_auths(); client.initialize(&admin); (client, admin) } diff --git a/contracts/multisig_transfer/src/auth_tests.rs b/contracts/multisig_transfer/src/auth_tests.rs new file mode 100644 index 000000000..e4ddb2b92 --- /dev/null +++ b/contracts/multisig_transfer/src/auth_tests.rs @@ -0,0 +1,162 @@ +//! Authorization tests ([SC-42]). +//! +//! Every entrypoint in this crate compared a caller-supplied `caller` argument +//! against the stored admin, the asset's owner, or the approver set — and none +//! authenticated it. Approvals could be forged by naming an authorized +//! approver; approval rules could be rewritten by naming the admin. +//! +//! These tests run **without** `mock_all_auths`, which is the only way to tell +//! an entrypoint that authenticates from one that merely looks like it does. +#![cfg(test)] + +use soroban_sdk::testutils::Address as _; +use soroban_sdk::{Address, BytesN, Env}; + +use crate::types::ApprovalRule; +use crate::{MultiSigTransferContract, MultiSigTransferContractClient}; + +fn setup(env: &Env) -> (MultiSigTransferContractClient<'_>, Address) { + let contract_id = env.register(MultiSigTransferContract, ()); + let client = MultiSigTransferContractClient::new(env, &contract_id); + let admin = Address::generate(env); + let registry = Address::generate(env); + + env.mock_all_auths(); + client.initialize(&admin, ®istry); + env.set_auths(&[]); + + (client, admin) +} + +fn category(env: &Env) -> BytesN<32> { + BytesN::from_array(env, &[9u8; 32]) +} + +fn rule(env: &Env, required: u32) -> ApprovalRule { + ApprovalRule { + category: category(env), + required_approvals: required, + approvers: soroban_sdk::Vec::from_array(env, [Address::generate(env)]), + approval_timeout_secs: 3600, + auto_approve: false, + priority: 1, + } +} + +#[test] +fn initialize_requires_the_admins_authorization() { + // Otherwise whoever calls initialize first owns a freshly deployed + // contract, regardless of who deployed it. + let env = Env::default(); + let contract_id = env.register(MultiSigTransferContract, ()); + let client = MultiSigTransferContractClient::new(&env, &contract_id); + + let res = client.try_initialize(&Address::generate(&env), &Address::generate(&env)); + assert!( + res.is_err(), + "initialize must require the incoming admin's authorization" + ); +} + +#[test] +fn configure_approval_rule_rejects_an_unauthenticated_caller() { + // Naming the admin must not be enough to rewrite the approval threshold — + // that would let anyone drop it to one and self-approve every transfer. + let env = Env::default(); + let (client, admin) = setup(&env); + + let res = client.try_configure_approval_rule(&admin, &rule(&env, 1)); + assert!( + res.is_err(), + "naming the admin must not authorize a rule change" + ); +} + +#[test] +fn configure_approval_rule_succeeds_once_the_admin_authenticates() { + let env = Env::default(); + let (client, admin) = setup(&env); + + env.mock_all_auths(); + client.configure_approval_rule(&admin, &rule(&env, 2)); + + // The rule is stored, observable through its approver list. + assert_eq!( + client + .get_required_approvers_category(&category(&env)) + .len(), + 1 + ); +} + +#[test] +fn a_non_admin_cannot_configure_an_approval_rule_even_when_authenticated() { + // Authentication is necessary but not sufficient: the admin check must + // still apply to a caller who can legitimately sign for themselves. + let env = Env::default(); + let (client, _admin) = setup(&env); + let stranger = Address::generate(&env); + + env.mock_all_auths(); + let res = client.try_configure_approval_rule(&stranger, &rule(&env, 1)); + assert!( + res.is_err(), + "a signed call from a non-admin must still be rejected" + ); +} + +#[test] +fn approve_transfer_request_rejects_an_unauthenticated_caller() { + let env = Env::default(); + let (client, _admin) = setup(&env); + let approver = Address::generate(&env); + + // The request does not exist, so this would fail either way — but it must + // fail on authorization first, before any state is read. + let res = client.try_approve_transfer_request(&approver, &1); + assert!( + res.is_err(), + "approvals must not be forgeable by naming an approver" + ); +} + +#[test] +fn cancel_transfer_request_rejects_an_unauthenticated_caller() { + let env = Env::default(); + let (client, admin) = setup(&env); + + let res = client.try_cancel_transfer_request(&admin, &1); + assert!( + res.is_err(), + "cancellation must not be forgeable by naming the admin" + ); +} + +#[test] +fn reject_transfer_request_rejects_an_unauthenticated_caller() { + let env = Env::default(); + let (client, _admin) = setup(&env); + let approver = Address::generate(&env); + + let res = + client.try_reject_transfer_request(&approver, &1, &BytesN::from_array(&env, &[0u8; 32])); + assert!( + res.is_err(), + "rejections must not be forgeable by naming an approver" + ); +} + +#[test] +fn read_entrypoints_do_not_require_authorization() { + let env = Env::default(); + let (client, _admin) = setup(&env); + + // No auths are mocked; these must still answer. + assert_eq!(client.get_asset_history(&category(&env)).len(), 0); + assert_eq!( + client + .get_pending_transfers_approver(&Address::generate(&env)) + .len(), + 0 + ); +} diff --git a/contracts/multisig_transfer/src/lib.rs b/contracts/multisig_transfer/src/lib.rs index 5cb3fc176..06e0dad28 100644 --- a/contracts/multisig_transfer/src/lib.rs +++ b/contracts/multisig_transfer/src/lib.rs @@ -3,6 +3,7 @@ use soroban_sdk::{contract, contractimpl, Address, BytesN, Env, Vec}; mod approvals; +mod auth_tests; mod errors; mod events; mod registry; @@ -24,6 +25,10 @@ impl MultiSigTransferContract { // Init // ---------------------------- pub fn initialize(e: Env, admin: Address, asset_registry: Address) { + // Without this, whoever calls initialize first becomes admin of a + // freshly deployed contract, regardless of who deployed it. + admin.require_auth(); + storage::set_admin(&e, &admin); storage::set_registry(&e, &asset_registry); e.storage() @@ -39,6 +44,10 @@ impl MultiSigTransferContract { caller: Address, rule: ApprovalRule, ) -> Result<(), MultiSigError> { + // require_admin only compares `caller` against the stored admin. + // Without authenticating `caller` first, anyone could pass the admin's + // address and pass that check. + caller.require_auth(); utils::require_admin(&e, &caller)?; let mut rules_map = storage::rules_map(&e); @@ -63,6 +72,9 @@ impl MultiSigTransferContract { expires_at: u64, execute_after: Option, ) -> Result { + // The ownership check below compares the registry's owner against a + // caller-supplied address; authenticate it first. + caller.require_auth(); let (_admin, registry_addr) = utils::require_init(&e)?; if caller == new_owner { @@ -155,6 +167,10 @@ impl MultiSigTransferContract { caller: Address, request_id: u64, ) -> Result<(), MultiSigError> { + // The approver-membership check below compares against a + // caller-supplied address; authenticate it first or approvals can be + // forged by naming an authorized approver. + caller.require_auth(); let (_admin, _registry) = utils::require_init(&e)?; let mut requests = storage::requests_map(&e); @@ -211,6 +227,7 @@ impl MultiSigTransferContract { request_id: u64, reason_hash: BytesN<32>, ) -> Result<(), MultiSigError> { + caller.require_auth(); let (_admin, _registry) = utils::require_init(&e)?; let mut requests = storage::requests_map(&e); @@ -245,7 +262,12 @@ impl MultiSigTransferContract { // ---------------------------- pub fn execute_transfer(e: Env, caller: Address, request_id: u64) -> Result<(), MultiSigError> { let (_admin, registry_addr) = utils::require_init(&e)?; - let _ = caller; // anyone can execute, kept for audit if desired + // Deliberately permissionless, like multisig-wallet's execute_*: the + // authorization decision was already made by the approvers, and + // requiring one of them to also submit the execution adds liveness + // risk without adding security. `caller` is retained for the audit + // trail only and is intentionally not authenticated. + let _ = caller; let mut requests = storage::requests_map(&e); let mut req = requests @@ -291,6 +313,9 @@ impl MultiSigTransferContract { caller: Address, request_id: u64, ) -> Result<(), MultiSigError> { + // Cancellation is restricted to the requester or the admin, both + // compared against a caller-supplied address. + caller.require_auth(); let (admin, _registry) = utils::require_init(&e)?; let mut requests = storage::requests_map(&e);