Skip to content

fix: asset manager storage migration #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Itshyphen wants to merge 2 commits into
base: fix/bnusd-storage-migration
Choose a base branch
from
Open

fix: asset manager storage migration #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
726c8cc
fix: asset manager storage migration
Itshyphen Nov 13, 2024
cb532a1
fix: return type defined
Itshyphen Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 60 additions & 75 deletions contracts/asset_manager/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ mod xcall {
soroban_sdk::contractimport!(file = "../../wasm/xcall.wasm");
}
use crate::errors::ContractError;
use crate::storage_types::TokenData;
use crate::states::{self};
use crate::storage_types::{ConfigData, RateLimit};
use crate::{
config::{self, get_config, set_config, ConfigData},
states::{
extent_ttl, has_registry, read_administrator, read_token_data, read_tokens,
write_administrator, write_registry, write_token_data, write_tokens,
extent_ttl, has_upgrade_authority, read_administrator,write_administrator,
},
storage_types::POINTS,
xcall_manager_interface::XcallManagerClient,
Expand All @@ -31,18 +30,27 @@ pub struct AssetManager;

#[contractimpl]
impl AssetManager {
pub fn initialize(env: Env, registry: Address, admin: Address, config: ConfigData) -> Result<(), ContractError> {
if has_registry(&env.clone()) {
pub fn initialize(env: Env, admin: Address, xcall: Address, xcall_manager: Address, native_address: Address, icon_asset_manager: String, upgrade_authority: Address) -> Result<(), ContractError> {
if has_upgrade_authority(&env.clone()) {
return Err(ContractError::ContractAlreadyInitialized)
}
write_registry(&env, &registry);
write_administrator(&env, &admin);
Self::configure(env, config);
states::write_xcall(&env, xcall);
states::write_xcall_manager(&env, xcall_manager);
states::write_icon_asset_manager(&env, icon_asset_manager);
states::write_upgrade_authority(&env, upgrade_authority);
states::write_native_address(&env, native_address);
Ok(())
}

pub fn get_config(env: Env) -> ConfigData {
get_config(&env)
ConfigData {
xcall: states::read_xcall(&env),
xcall_manager: states::read_xcall_manager(&env),
native_address: states::read_native_address(&env),
icon_asset_manager: states::read_icon_asset_manager(&env),
upgrade_authority: states::read_upgrade_authority(&env),
}
}
Copy link
Collaborator

@ibrizsabin ibrizsabin Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we are splitting storage items , its better to have separate getter methods as well. reading all storage item just to get one is unnecessary . separate getters would be more readable as well. If we may need entire config object , return type still can be 'config' struct. makes it more readable and type safe.

Copy link
Author

@Itshyphen Itshyphen Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah returned using struct


pub fn set_admin(env: Env, new_admin: Address) {
Expand All @@ -56,13 +64,6 @@ impl AssetManager {
read_administrator(&env)
}

pub fn configure(env: Env, config: ConfigData) {
let admin = read_administrator(&env);
admin.require_auth();

set_config(&env, config);
}

pub fn configure_rate_limit(
env: Env,
token_address: Address,
Expand All @@ -71,48 +72,32 @@ impl AssetManager {
) -> Result<(), ContractError> {
let admin = read_administrator(&env);
admin.require_auth();
let tokens = read_tokens(&env);
if tokens.contains(&token_address) {
return Err(ContractError::TokenExists);
} else {
write_tokens(&env, token_address.clone());
}

if percentage > POINTS as u32 {
return Err(ContractError::PercentageShouldBeLessThanOrEqualToPOINTS);
}

write_token_data(
&env,
token_address,
TokenData {
period,
percentage,
last_update: env.ledger().timestamp(),
current_limit: 0,
},
);
states::write_period(&env, token_address.clone(), period);
states::write_percentage(&env, token_address.clone(), percentage);
states::write_last_update(&env, token_address.clone(), env.ledger().timestamp());
states::write_current_limit(&env, token_address, 0);
Ok(())
}

pub fn get_rate_limit(env: Env, token_address: Address) -> Result<(u64, u32, u64, u64), ContractError> {
let data: TokenData = read_token_data(&env, token_address)?;

Ok((
data.period,
data.percentage,
data.last_update,
data.current_limit,
))
pub fn get_rate_limit(env: Env, token_address: Address) -> RateLimit {
RateLimit {
period: states::read_period(&env, token_address.clone()),
percentage: states::read_percentage(&env, token_address.clone()),
last_update: states::read_last_update(&env, token_address.clone()),
current_limit: states::read_current_limit(&env, token_address),
}
}

Copy link
Collaborator

@ibrizsabin ibrizsabin Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here . its so unintuitive to return tuple of primitive types. Lets wrap the values in struct before returning so that caller knows which value is what. or it should only return current_limit.

Copy link
Author

@Itshyphen Itshyphen Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made change!

pub fn reset_limit(env: Env, token: Address) -> Result<bool, ContractError> {
let admin = read_administrator(&env);
admin.require_auth();
let balance = Self::get_token_balance(&env, token.clone());
let mut data: TokenData = read_token_data(&env, token.clone())?;
data.current_limit = (balance * data.percentage as u128 / POINTS) as u64;
write_token_data(&env, token, data);
let percentage = states::read_percentage(&env, token.clone());
let current_limit = (balance * percentage as u128 / POINTS) as u64;
states::write_current_limit(&env, token, current_limit);
Ok(true)
}

Expand All @@ -132,10 +117,9 @@ impl AssetManager {
if balance - amount < limit {
return Err(ContractError::ExceedsWithdrawLimit);
};
let mut data: TokenData = read_token_data(&env, token.clone())?;
data.current_limit = limit as u64;
data.last_update = env.ledger().timestamp();
write_token_data(&env, token, data);

states::write_current_limit(&env, token.clone(), limit as u64);
states::write_last_update(&env, token.clone(), env.ledger().timestamp());
Ok(true)
}

Expand All @@ -144,21 +128,22 @@ impl AssetManager {
balance: u128,
token: Address,
) -> Result<u128, ContractError> {
let data: TokenData = read_token_data(&env, token)?;
let period: u128 = data.period as u128;
let percentage: u128 = data.percentage as u128;
let period: u128 = states::read_period(&env, token.clone()) as u128;
let percentage: u128 = states::read_percentage(&env, token.clone()) as u128;
let last_update: u64 = states::read_last_update(&env, token.clone());
let current_limit: u64 = states::read_current_limit(&env, token.clone());

if period == 0 {
return Ok(0);
}

let min_reserve = (balance * percentage) / POINTS;

let max_withdraw = balance - min_reserve;
let last_update: u64 = data.last_update;
let time_diff = &env.ledger().timestamp() - last_update;

let allowed_withdrawal = (max_withdraw * time_diff as u128) / period;
let mut reserve: u128 = data.current_limit as u128;
let mut reserve: u128 = current_limit as u128;

if reserve > allowed_withdrawal {
reserve = reserve - allowed_withdrawal;
Expand Down Expand Up @@ -223,12 +208,15 @@ impl AssetManager {
data,
);

let xcall = states::read_xcall(&e);
let xcall_manager = states::read_xcall_manager(&e);
let icon_asset_manager = states::read_icon_asset_manager(&e);

let rollback: DepositRevert = DepositRevert::new(token, from.clone(), amount);
let config = get_config(&e);
let rollback_bytes = rollback.encode(&e, String::from_str(&e, DEPOSIT_REVERT_NAME));
let message_bytes = xcall_message.encode(&e, String::from_str(&e, DEPOSIT_NAME));
let (sources, destinations) =
Self::xcall_manager(&e, &config.xcall_manager).get_protocols();
Self::xcall_manager(&e, &xcall_manager).get_protocols();
let message = AnyMessage::CallMessageWithRollback(CallMessageWithRollback {
data: message_bytes,
rollback: rollback_bytes,
Expand All @@ -239,11 +227,11 @@ impl AssetManager {
sources,
};

Self::xcall_client(&e, &config.xcall).send_call(
Self::xcall_client(&e, &xcall).send_call(
&from,
&current_address,
envelope,
&config.icon_asset_manager,
&icon_asset_manager,
);
Ok(())
}
Expand All @@ -263,13 +251,14 @@ impl AssetManager {
data: Bytes,
protocols: Vec<String>,
) -> Result<(), ContractError> {
let config = get_config(&e);
let xcall = config.xcall;
let xcall = states::read_xcall(&e);
let xcall_manager = states::read_xcall_manager(&e);
let icon_asset_manager = states::read_icon_asset_manager(&e);

xcall.require_auth();

let method = Deposit::get_method(&e, data.clone());

let icon_asset_manager = config.icon_asset_manager;
let current_contract = e.current_contract_address();
if method == String::from_str(&e, &WITHDRAW_TO_NAME) {
if from != icon_asset_manager {
Expand Down Expand Up @@ -304,7 +293,7 @@ impl AssetManager {
} else {
return Err(ContractError::UnknownMessageType);
}
if !Self::xcall_manager(&e, &config.xcall_manager).verify_protocols(&protocols) {
if !Self::xcall_manager(&e, &xcall_manager).verify_protocols(&protocols) {
return Err(ContractError::ProtocolMismatch);
}
Ok(())
Expand Down Expand Up @@ -343,29 +332,25 @@ impl AssetManager {
return token_client.balance(&e.current_contract_address());
}

pub fn has_registry(e: Env) -> bool {
has_registry(&e)
pub fn is_initialized(e: Env) -> bool {
states::has_upgrade_authority(&e)
}

pub fn set_upgrade_authority(e: Env, upgrade_authority: Address) {
let mut config = config::get_config(&e);
pub fn set_upgrade_authority(e: Env, new_upgrade_authority: Address) {
let upgrade_authority = states::read_upgrade_authority(&e);
upgrade_authority.require_auth();

config.upgrade_authority.require_auth();

config.upgrade_authority = upgrade_authority;
config::set_config(&e, config);
states::write_upgrade_authority(&e, new_upgrade_authority);
}

pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) {
let config = get_config(&e);
config.upgrade_authority.require_auth();
let upgrade_authority = states::read_upgrade_authority(&e);
upgrade_authority.require_auth();

e.deployer().update_current_contract_wasm(new_wasm_hash);
}

pub fn extend_ttl(e: Env) {
extent_ttl(&e);
}


}
1 change: 0 additions & 1 deletion contracts/asset_manager/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,5 @@ pub mod contract;
pub mod storage_types;
pub mod tests;
pub mod states;
mod config;
mod errors;
mod xcall_manager_interface;
Loading
Loading