From 22571f4d4cfbdf500620320db2cd89dcda9bb39e Mon Sep 17 00:00:00 2001 From: hawthorne-abendsen Date: Tue, 13 Feb 2024 16:22:19 +0200 Subject: [PATCH] admin arg removed from administration functions --- src/extensions/env_extensions.rs | 18 +-- src/lib.rs | 23 ++- src/test.rs | 233 +++++++++++++++---------------- 3 files changed, 128 insertions(+), 146 deletions(-) diff --git a/src/extensions/env_extensions.rs b/src/extensions/env_extensions.rs index ba69956..5fe1eb8 100644 --- a/src/extensions/env_extensions.rs +++ b/src/extensions/env_extensions.rs @@ -16,8 +16,6 @@ const DECIMALS: &str = "decimals"; const RESOLUTION: &str = "resolution"; pub trait EnvExtensions { - fn is_authorized(&self, invoker: &Address) -> bool; - fn get_admin(&self) -> Option
; fn set_admin(&self, admin: &Address); @@ -54,7 +52,7 @@ pub trait EnvExtensions { fn get_asset_index(&self, asset: &Asset) -> Option; - fn panic_if_not_admin(&self, invoker: &Address); + fn panic_if_not_admin(&self); fn is_initialized(&self) -> bool; @@ -62,14 +60,6 @@ pub trait EnvExtensions { } impl EnvExtensions for Env { - fn is_authorized(&self, invoker: &Address) -> bool { - invoker.require_auth(); - - //invoke get_admin to check if the admin is set - let admin = self.get_admin(); - !admin.is_none() && invoker == &admin.unwrap() - } - fn is_initialized(&self) -> bool { get_instance_storage(&self).has(&ADMIN_KEY) } @@ -181,10 +171,12 @@ impl EnvExtensions for Env { return Some(index.unwrap() as u8); } - fn panic_if_not_admin(&self, invoker: &Address) { - if !self.is_authorized(invoker) { + fn panic_if_not_admin(&self) { + let admin = self.get_admin(); + if admin.is_none() { panic_with_error!(self, Error::Unauthorized); } + admin.unwrap().require_auth() } fn bump(&self, ledgers_to_live: u32) { diff --git a/src/lib.rs b/src/lib.rs index bfd6aa3..ef4cb8a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -282,14 +282,11 @@ impl PriceOracleContract { // # Panics // // Panics if the contract is already initialized, or if the version is invalid - pub fn config(e: Env, admin: Address, config: ConfigData) { - admin.require_auth(); + pub fn config(e: Env, config: ConfigData) { + config.admin.require_auth(); if e.is_initialized() { e.panic_with_error(Error::AlreadyInitialized); } - if admin != config.admin { - e.panic_with_error(Error::Unauthorized); - } e.set_admin(&config.admin); e.set_base_asset(&config.base_asset); e.set_decimals(config.decimals); @@ -323,8 +320,8 @@ impl PriceOracleContract { // # Panics // // Panics if the caller doesn't match admin address, or if the assets are already added - pub fn add_assets(e: Env, admin: Address, assets: Vec) { - e.panic_if_not_admin(&admin); + pub fn add_assets(e: Env, assets: Vec) { + e.panic_if_not_admin(); Self::__add_assets(&e, assets); } @@ -339,8 +336,8 @@ impl PriceOracleContract { // # Panics // // Panics if the caller doesn't match admin address, or if the period/version is invalid - pub fn set_period(e: Env, admin: Address, period: u64) { - e.panic_if_not_admin(&admin); + pub fn set_period(e: Env, period: u64) { + e.panic_if_not_admin(); e.set_retention_period(period); } @@ -355,8 +352,8 @@ impl PriceOracleContract { // # Panics // // Panics if the caller doesn't match admin address, or if the price snapshot record is invalid - pub fn set_price(e: Env, admin: Address, updates: Vec, timestamp: u64) { - e.panic_if_not_admin(&admin); + pub fn set_price(e: Env, updates: Vec, timestamp: u64) { + e.panic_if_not_admin(); let updates_len = updates.len(); if updates_len == 0 || updates_len >= 256 { panic_with_error!(&e, Error::InvalidUpdateLength); @@ -399,8 +396,8 @@ impl PriceOracleContract { // # Panics // // Panics if the caller doesn't match admin address - pub fn update_contract(env: Env, admin: Address, wasm_hash: BytesN<32>) { - env.panic_if_not_admin(&admin); + pub fn update_contract(env: Env, wasm_hash: BytesN<32>) { + env.panic_if_not_admin(); env.deployer().update_current_contract_wasm(wasm_hash) } diff --git a/src/test.rs b/src/test.rs index 7ec9903..d1a9ae0 100644 --- a/src/test.rs +++ b/src/test.rs @@ -1,13 +1,18 @@ #![cfg(test)] -extern crate std; extern crate alloc; +extern crate std; use super::*; use alloc::string::ToString; -use soroban_sdk::{testutils::{Address as _, Ledger, LedgerInfo}, Address, Env, Symbol, String}; +use soroban_sdk::{ + testutils::{Address as _, Ledger, LedgerInfo, MockAuth, MockAuthInvoke}, Address, Env, String, Symbol, TryIntoVal +}; use std::panic::{self, AssertUnwindSafe}; -use {extensions::{u64_extensions::U64Extensions, i128_extensions::I128Extensions}, types::asset::Asset}; +use { + extensions::i128_extensions::I128Extensions, + types::asset::Asset, +}; const RESOLUTION: u32 = 300_000; const DECIMALS: u32 = 14; @@ -24,7 +29,10 @@ fn init_contract_with_admin<'a>() -> (Env, PriceOracleContractClient<'a>, Config let admin = Address::generate(&env); - let contract_id = &Address::from_string(&String::from_str(&env, "CDXHQTB7FGRMWTLJJLNI3XPKVC6SZDB5SFGZUYDPEGQQNC4G6CKE4QRC")); + let contract_id = &Address::from_string(&String::from_str( + &env, + "CDXHQTB7FGRMWTLJJLNI3XPKVC6SZDB5SFGZUYDPEGQQNC4G6CKE4QRC", + )); env.register_contract(contract_id, PriceOracleContract); let client: PriceOracleContractClient<'a> = PriceOracleContractClient::new(&env, contract_id); @@ -37,13 +45,13 @@ fn init_contract_with_admin<'a>() -> (Env, PriceOracleContractClient<'a>, Config assets: generate_assets(&env, 10, 0), base_asset: Asset::Stellar(Address::generate(&env)), decimals: 14, - resolution: RESOLUTION + resolution: RESOLUTION, }; env.mock_all_auths(); //set admin - client.config(&admin, &init_data); + client.config(&init_data); (env, client, init_data) } @@ -58,7 +66,10 @@ fn generate_assets(e: &Env, count: usize, start_index: u32) -> Vec { if i % 2 == 0 { assets.push_back(Asset::Stellar(Address::generate(&e))); } else { - assets.push_back(Asset::Other(Symbol::new(e, &("ASSET_".to_string() + &(start_index + i as u32).to_string())))); + assets.push_back(Asset::Other(Symbol::new( + e, + &("ASSET_".to_string() + &(start_index + i as u32).to_string()), + ))); } } assets @@ -114,7 +125,6 @@ fn bump_test() { fn set_price_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; let assets = init_data.assets; let timestamp = 600_000; @@ -123,7 +133,7 @@ fn set_price_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); } #[test] @@ -131,7 +141,6 @@ fn set_price_test() { fn set_price_zero_timestamp_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; let assets = init_data.assets; let timestamp = 0; @@ -140,7 +149,7 @@ fn set_price_zero_timestamp_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); } #[test] @@ -148,7 +157,6 @@ fn set_price_zero_timestamp_test() { fn set_price_invalid_timestamp_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; let assets = init_data.assets; let timestamp = 600_001; @@ -157,7 +165,7 @@ fn set_price_invalid_timestamp_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); } #[test] @@ -165,7 +173,6 @@ fn set_price_invalid_timestamp_test() { fn set_price_future_timestamp_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; let assets = init_data.assets; let timestamp = 1_200_000; @@ -174,14 +181,13 @@ fn set_price_future_timestamp_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); } #[test] fn last_price_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; let assets = init_data.assets; let timestamp = 600_000; @@ -190,13 +196,13 @@ fn last_price_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); let timestamp = 900_000; let updates = get_updates(&env, &&assets, normalize_price(200)); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); //check last prices let result = client.lastprice(&assets.get_unchecked(1)); @@ -214,7 +220,6 @@ fn last_price_test() { fn last_timestamp_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; let assets = init_data.assets; let mut result = client.last_timestamp(); @@ -227,8 +232,8 @@ fn last_timestamp_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); - + client.set_price(&updates, ×tamp); + result = client.last_timestamp(); assert_eq!(result, 600_000); @@ -238,13 +243,11 @@ fn last_timestamp_test() { fn add_assets_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; - let assets = generate_assets(&env, 10, init_data.assets.len() - 1); env.mock_all_auths(); - client.add_assets(&admin, &assets); + client.add_assets(&assets); let result = client.assets(); @@ -256,14 +259,10 @@ fn add_assets_test() { assert_eq!(result, expected_assets); } - - #[test] #[should_panic] fn add_assets_duplicate_test() { - let (env, client, init_data) = init_contract_with_admin(); - - let admin = &init_data.admin; + let (env, client, _) = init_contract_with_admin(); let mut assets = Vec::new(&env); let duplicate_asset = Asset::Other(Symbol::new(&env, &("ASSET_DUPLICATE"))); @@ -272,57 +271,54 @@ fn add_assets_duplicate_test() { env.mock_all_auths(); - client.add_assets(&admin, &assets); + client.add_assets(&assets); } #[test] #[should_panic] fn assets_update_overflow_test() { - let (env, client, init_data) = init_contract_with_admin(); - + let (env, client, _) = init_contract_with_admin(); + env.mock_all_auths(); env.budget().reset_unlimited(); - - let admin = &init_data.admin; let mut assets = Vec::new(&env); for i in 1..=256 { - assets.push_back(Asset::Other(Symbol::new(&env, &("Asset".to_string() + &i.to_string())))); + assets.push_back(Asset::Other(Symbol::new( + &env, + &("Asset".to_string() + &i.to_string()), + ))); } - client.add_assets(&admin, &assets); + client.add_assets(&assets); } #[test] #[should_panic] fn prices_update_overflow_test() { - let (env, client, init_data) = init_contract_with_admin(); - + let (env, client, _) = init_contract_with_admin(); + env.mock_all_auths(); env.budget().reset_unlimited(); - - let admin = &init_data.admin; let mut updates = Vec::new(&env); for i in 1..=256 { updates.push_back(normalize_price(i as i128 + 1)); } - client.set_price(&admin, &updates, &600_000); + client.set_price(&updates, &600_000); } #[test] fn set_period_test() { - let (env, client, init_data) = init_contract_with_admin(); - - let admin = &init_data.admin; + let (env, client, _) = init_contract_with_admin(); let period = 100_000; env.mock_all_auths(); - client.set_period(&admin, &period); + client.set_period(&period); let result = client.period().unwrap(); @@ -333,7 +329,6 @@ fn set_period_test() { fn get_price_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; let assets = init_data.assets; let timestamp = 600_000; @@ -341,12 +336,12 @@ fn get_price_test() { env.mock_all_auths(); - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); let timestamp = 900_000; let updates = get_updates(&env, &assets, normalize_price(200)); - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); //check last prices let mut result = client.lastprice(&assets.get_unchecked(1)); @@ -374,8 +369,7 @@ fn get_price_test() { #[test] fn get_lastprice_delayed_update_test() { let (env, client, init_data) = init_contract_with_admin(); - - let admin = &init_data.admin; + let assets = init_data.assets; let timestamp = 300_000; @@ -383,7 +377,7 @@ fn get_lastprice_delayed_update_test() { env.mock_all_auths(); - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); //check last prices let result = client.lastprice(&assets.get_unchecked(1)); @@ -393,8 +387,7 @@ fn get_lastprice_delayed_update_test() { #[test] fn get_x_last_price_test() { let (env, client, init_data) = init_contract_with_admin(); - - let admin = &init_data.admin; + let assets = init_data.assets; let timestamp = 600_000; @@ -402,13 +395,10 @@ fn get_x_last_price_test() { env.mock_all_auths(); - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); //check last x price - let result = client.x_last_price( - &assets.get_unchecked(1), - &assets.get_unchecked(2), - ); + let result = client.x_last_price(&assets.get_unchecked(1), &assets.get_unchecked(2)); assert_ne!(result, None); assert_eq!( result, @@ -419,16 +409,10 @@ fn get_x_last_price_test() { ); } - - - - - #[test] fn get_x_price_with_zero_test() { let (env, client, init_data) = init_contract_with_admin(); - - let admin = &init_data.admin; + let assets = init_data.assets; let timestamp = 600_000; @@ -438,7 +422,7 @@ fn get_x_price_with_zero_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); let result = client.x_price(&assets.get(0).unwrap(), &assets.get(1).unwrap(), ×tamp); @@ -448,8 +432,7 @@ fn get_x_price_with_zero_test() { #[test] fn get_x_price_test() { let (env, client, init_data) = init_contract_with_admin(); - - let admin = &init_data.admin; + let assets = init_data.assets; let timestamp = 600_000; @@ -458,19 +441,16 @@ fn get_x_price_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); let timestamp = 900_000; let updates = get_updates(&env, &assets, normalize_price(200)); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); //check last prices - let mut result = client.x_last_price( - &assets.get_unchecked(1), - &assets.get_unchecked(2), - ); + let mut result = client.x_last_price(&assets.get_unchecked(1), &assets.get_unchecked(2)); assert_ne!(result, None); assert_eq!( result, @@ -481,11 +461,7 @@ fn get_x_price_test() { ); //check price at 899_000 - result = client.x_price( - &assets.get_unchecked(1), - &assets.get_unchecked(2), - &899_000, - ); + result = client.x_price(&assets.get_unchecked(1), &assets.get_unchecked(2), &899_000); assert_ne!(result, None); assert_eq!( result, @@ -499,8 +475,7 @@ fn get_x_price_test() { #[test] fn twap_test() { let (env, client, init_data) = init_contract_with_admin(); - - let admin = &init_data.admin; + let assets = init_data.assets; let timestamp = 600_000; @@ -509,13 +484,13 @@ fn twap_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); let timestamp = 900_000; let updates = get_updates(&env, &assets, normalize_price(200)); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); let result = client.twap(&assets.get_unchecked(1), &2); @@ -526,8 +501,7 @@ fn twap_test() { #[test] fn x_twap_test() { let (env, client, init_data) = init_contract_with_admin(); - - let admin = &init_data.admin; + let assets = init_data.assets; //set prices for assets @@ -537,19 +511,15 @@ fn x_twap_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); let timestamp = 900_000; let updates = get_updates(&env, &assets, normalize_price(200)); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); - let result = client.x_twap( - &assets.get_unchecked(1), - &assets.get_unchecked(2), - &2, - ); + let result = client.x_twap(&assets.get_unchecked(1), &assets.get_unchecked(2), &2); assert_ne!(result, None); assert_eq!(result.unwrap(), normalize_price(1)); @@ -560,7 +530,6 @@ fn x_twap_test() { fn x_twap_with_gap_test() { let (env, client, init_data) = init_contract_with_admin(); - let admin = &init_data.admin; let assets = init_data.assets; //set prices for assets with gap @@ -570,19 +539,15 @@ fn x_twap_with_gap_test() { env.mock_all_auths(); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); let timestamp = 900_000; let updates = get_updates(&env, &assets, normalize_price(200)); //set prices for assets - client.set_price(&admin, &updates, ×tamp); + client.set_price(&updates, ×tamp); - let result = client.x_twap( - &assets.get_unchecked(1), - &assets.get_unchecked(2), - &3, - ); + let result = client.x_twap(&assets.get_unchecked(1), &assets.get_unchecked(2), &3); assert_ne!(result, None); assert_eq!(result.unwrap(), normalize_price(1)); @@ -590,7 +555,6 @@ fn x_twap_with_gap_test() { #[test] fn get_non_registered_asset_price_test() { - let (env, client, config_data) = init_contract_with_admin(); //try to get price for unknown Stellar asset @@ -602,15 +566,24 @@ fn get_non_registered_asset_price_test() { assert_eq!(result, None); //try to get price for unknown base asset - result = client.x_last_price(&Asset::Stellar(Address::generate(&env)), &config_data.assets.get_unchecked(1)); + result = client.x_last_price( + &Asset::Stellar(Address::generate(&env)), + &config_data.assets.get_unchecked(1), + ); assert_eq!(result, None); //try to get price for unknown quote asset - result = client.x_last_price(&config_data.assets.get_unchecked(1), &Asset::Stellar(Address::generate(&env))); + result = client.x_last_price( + &config_data.assets.get_unchecked(1), + &Asset::Stellar(Address::generate(&env)), + ); assert_eq!(result, None); //try to get price for both unknown assets - result = client.x_last_price(&Asset::Stellar(Address::generate(&env)), &Asset::Other(Symbol::new(&env, "NonRegisteredAsset"))); + result = client.x_last_price( + &Asset::Stellar(Address::generate(&env)), + &Asset::Other(Symbol::new(&env, "NonRegisteredAsset")), + ); assert_eq!(result, None); } @@ -618,7 +591,6 @@ fn get_non_registered_asset_price_test() { fn get_asset_price_for_invalid_timestamp_test() { let (env, client, config_data) = init_contract_with_admin(); - let mut result = client.price(&config_data.assets.get_unchecked(1), &u64::MAX); assert_eq!(result, None); @@ -628,31 +600,52 @@ fn get_asset_price_for_invalid_timestamp_test() { } #[test] -#[should_panic] -fn unauthorized_test() { - let (env, client, init_data) = init_contract_with_admin(); +fn authorized_test() { + let (env, client, config_data) = init_contract_with_admin(); - let assets = init_data.assets; + let period: u64 = 100; + //set prices for assets + client.mock_auths(&[MockAuth { + address: &config_data.admin, + invoke: &MockAuthInvoke { + contract: &client.address, + fn_name: "set_period", + args: Vec::from_array(&env, [period.clone().try_into_val(&env).unwrap()]), + sub_invokes: &[], + }, + }]).set_period(&period); +} - let updates = get_updates(&env, &assets, 100); +#[test] +#[should_panic] +fn unauthorized_test() { + let (env, client, _) = init_contract_with_admin(); let account = Address::generate(&env); - let timestamp = (112331 as u64).get_normalized_timestamp(RESOLUTION as u64); - - //mock auth to check only contract's admin validation - env.mock_all_auths(); + let period: u64 = 100; //set prices for assets - client.set_price(&account, &updates, ×tamp); + client.mock_auths(&[MockAuth { + address: &account, + invoke: &MockAuthInvoke { + contract: &client.address, + fn_name: "set_period", + args: Vec::from_array(&env, [period.clone().try_into_val(&env).unwrap()]), + sub_invokes: &[], + }, + }]).set_period(&period); } - #[test] fn div_tests() { let test_cases = [ (154467226919499, 133928752749774, 115335373284703), - (i128::MAX/100, 231731687303715884105728, 734216306110962248249052545), - (231731687303715884105728, i128::MAX/100, 13), + ( + i128::MAX / 100, + 231731687303715884105728, + 734216306110962248249052545, + ), + (231731687303715884105728, i128::MAX / 100, 13), // -1 expected result for errors (1, 0, -1), (0, 1, -1), @@ -663,11 +656,11 @@ fn div_tests() { ]; for (a, b, expected) in test_cases.iter() { - let result = panic::catch_unwind(AssertUnwindSafe(|| {a.fixed_div_floor(*b, 14)})); + let result = panic::catch_unwind(AssertUnwindSafe(|| a.fixed_div_floor(*b, 14))); if expected == &-1 { assert!(result.is_err()); } else { assert_eq!(result.unwrap(), *expected); } } -} \ No newline at end of file +}