Skip to content
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

draft: fix: balance calculations and scout-audit warnings #79

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

draft: fix: balance calculations and scout-audit warnings #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4571afc
fix: add proper error handling to pool getters and setters
Teolhyn Dec 12, 2024
9318d2b
fix: add Result type and pool errors to interest calculations
Teolhyn Dec 12, 2024
57f0d85
fix: add Result types and error handling
Teolhyn Dec 12, 2024
d867a50
refactor: remove same postfixes from error enum variants
Teolhyn Dec 12, 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
49 changes: 30 additions & 19 deletions contracts/loan_manager/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const REFLECTOR_ADDRESS: &str = "CCYOZJCOPG34LLQQ7N24YXBM7LL62R7ONMZ3G6WZAAYPB5O
pub enum Error {
AlreadyInitialized = 1,
LoanAlreadyExists = 2,
AdminNotFound = 3,
}

#[contract]
Expand Down Expand Up @@ -57,8 +58,11 @@ impl LoanManager {
.with_current_contract(salt)
.deploy_v2(wasm_hash, ());

let admin: Address = e.storage().persistent().get(&LoansDataKey::Admin).unwrap();
admin.require_auth();

Comment on lines +61 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe that deploy pool test fails because the test doesn't init the manager like it should? This could probably be fixed by e.mock_all_auths() in that deploy_pool test.

// Add the new address to storage
let mut pool_addresses = e
let mut pool_addresses: Vec<Address> = e
.storage()
.persistent()
.get(&LoansDataKey::PoolAddresses)
Expand All @@ -85,24 +89,31 @@ impl LoanManager {
}

/// Upgrade deployed loan pools and the loan manager WASM.
pub fn upgrade(e: Env, new_manager_wasm_hash: BytesN<32>, new_pool_wasm_hash: BytesN<32>) {
let admin: Address = e.storage().persistent().get(&LoansDataKey::Admin).unwrap();
admin.require_auth();

// Upgrade the loan pools.
e.storage()
.persistent()
.get(&LoansDataKey::PoolAddresses)
.unwrap_or(vec![&e])
.iter()
.for_each(|pool| {
let pool_client = loan_pool::Client::new(&e, &pool);
pool_client.upgrade(&new_pool_wasm_hash);
});

// Upgrade the loan manager.
e.deployer()
.update_current_contract_wasm(new_manager_wasm_hash);
pub fn upgrade(
e: Env,
new_manager_wasm_hash: BytesN<32>,
new_pool_wasm_hash: BytesN<32>,
) -> Result<(), Error> {
let admin: Option<Address> = e.storage().persistent().get(&LoansDataKey::Admin);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have option values in variables like this, I think we should standardize our naming so the option-wrapper variable is named opt_admin and the unwrapper data is admin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As it's already returning Return type I'll just switch the if let Some to '?'

if let Some(admin) = admin {
admin.require_auth();
e.storage()
.persistent()
.get(&LoansDataKey::PoolAddresses)
.unwrap_or(vec![&e])
.iter()
.for_each(|pool| {
let pool_client = loan_pool::Client::new(&e, &pool);
pool_client.upgrade(&new_pool_wasm_hash);
});

e.deployer()
.update_current_contract_wasm(new_manager_wasm_hash);

Ok(())
} else {
Err(Error::AdminNotFound)
}
}

/// Initialize a new loan
Expand Down
127 changes: 68 additions & 59 deletions contracts/loan_pool/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::interest::{self, get_interest};
use crate::pool::Currency;
use crate::pool::{Currency, Error};
use crate::positions;
use crate::{pool, storage_types::Positions};

Expand Down Expand Up @@ -34,26 +34,29 @@ impl LoanPoolContract {
pool::write_accrual_last_updated(&e, e.ledger().timestamp());
}

pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) {
let loan_manager_addr = pool::read_loan_manager_addr(&e);
pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) -> Result<(), Error> {
let loan_manager_addr = pool::read_loan_manager_addr(&e)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Love it

loan_manager_addr.require_auth();

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

/// Deposits token. Also, mints pool shares for the "user" Identifier.
pub fn deposit(e: Env, user: Address, amount: i128) -> i128 {
pub fn deposit(e: Env, user: Address, amount: i128) -> Result<i128, Error> {
user.require_auth();
assert!(amount > 0, "Amount must be positive!");

Self::add_interest_to_accrual(e.clone());
Self::add_interest_to_accrual(e.clone())?;

let client = token::Client::new(&e, &pool::read_currency(&e).token_address);
let token_address = pool::read_currency(&e)?.token_address;

let client = token::Client::new(&e, &token_address);
client.transfer(&user, &e.current_contract_address(), &amount);

pool::change_available_balance(&e, amount);
pool::change_total_shares(&e, amount);
pool::change_total_balance(&e, amount);
pool::change_available_balance(&e, amount)?;
pool::change_total_shares(&e, amount)?;
pool::change_total_balance(&e, amount)?;

// Increase users position in pool as they deposit
// as this is deposit amount is added to receivables and
Expand All @@ -62,14 +65,14 @@ impl LoanPoolContract {
let collateral: i128 = 0; // temp test param
positions::increase_positions(&e, user.clone(), amount, liabilities, collateral);

amount
Ok(amount)
}

/// Transfers share tokens back, burns them and gives corresponding amount of tokens back to user. Returns amount of tokens withdrawn
pub fn withdraw(e: Env, user: Address, amount: i128) -> (i128, i128) {
pub fn withdraw(e: Env, user: Address, amount: i128) -> Result<(i128, i128), Error> {
user.require_auth();

Self::add_interest_to_accrual(e.clone());
Self::add_interest_to_accrual(e.clone())?;

// Get users receivables
let Positions { receivables, .. } = positions::read_positions(&e, &user);
Expand All @@ -79,47 +82,48 @@ impl LoanPoolContract {
amount <= receivables,
"Amount can not be greater than receivables!"
);
assert!(amount <= Self::get_available_balance(e.clone()));
let available_balance = Self::get_available_balance(e.clone())?;
assert!(amount <= available_balance);

// TODO: Decrease AvailableBalance
pool::change_available_balance(&e, -amount);
pool::change_available_balance(&e, -amount)?;
// TODO: Decrease TotalShares - Positions should have shares if we use them
// TODO: Decrease TotalBalance
pool::change_total_balance(&e, -amount);
pool::change_total_balance(&e, -amount)?;

// Decrease users position in pool as they withdraw
let liabilities: i128 = 0;
let collateral: i128 = 0;
positions::decrease_positions(&e, user.clone(), amount, liabilities, collateral);

// Transfer tokens from pool to user
let token_address = &pool::read_currency(&e).token_address;
let token_address = &pool::read_currency(&e)?.token_address;
let client = token::Client::new(&e, token_address);
client.transfer(&e.current_contract_address(), &user, &amount);

(amount, amount)
Ok((amount, amount))
}

/// Borrow tokens from the pool
pub fn borrow(e: Env, user: Address, amount: i128) -> i128 {
pub fn borrow(e: Env, user: Address, amount: i128) -> Result<i128, Error> {
/*
Borrow should only be callable from the loans contract. This is as the loans contract will
include the logic and checks that the borrowing can be actually done. Therefore we need to
include a check that the caller is the loans contract.
*/
let loan_manager_addr = pool::read_loan_manager_addr(&e);
let loan_manager_addr = pool::read_loan_manager_addr(&e)?;
loan_manager_addr.require_auth();
user.require_auth();

Self::add_interest_to_accrual(e.clone());
Self::add_interest_to_accrual(e.clone())?;

let balance = pool::read_available_balance(&e);
let balance = pool::read_available_balance(&e)?;
assert!(
amount < balance,
"Borrowed amount has to be less than available balance!"
); // Check that there is enough available balance

pool::change_available_balance(&e, -amount);
pool::change_available_balance(&e, -amount)?;

// Increase users position in pool as they deposit
// as this is debt amount is added to liabilities and
Expand All @@ -128,21 +132,21 @@ impl LoanPoolContract {
let receivables: i128 = 0; // temp test param
positions::increase_positions(&e, user.clone(), receivables, amount, collateral);

let token_address = &pool::read_currency(&e).token_address;
let token_address = &pool::read_currency(&e)?.token_address;
let client = token::Client::new(&e, token_address);
client.transfer(&e.current_contract_address(), &user, &amount);

amount
Ok(amount)
}

/// Deposit tokens to the pool to be used as collateral
pub fn deposit_collateral(e: Env, user: Address, amount: i128) -> i128 {
pub fn deposit_collateral(e: Env, user: Address, amount: i128) -> Result<i128, Error> {
user.require_auth();
assert!(amount > 0, "Amount must be positive!");

Self::add_interest_to_accrual(e.clone());
Self::add_interest_to_accrual(e.clone())?;

let token_address = &pool::read_currency(&e).token_address;
let token_address = &pool::read_currency(&e)?.token_address;
let client = token::Client::new(&e, token_address);
client.transfer(&user, &e.current_contract_address(), &amount);

Expand All @@ -153,18 +157,18 @@ impl LoanPoolContract {
let receivables: i128 = 0; // temp test param
positions::increase_positions(&e, user.clone(), receivables, liabilities, amount);

amount
Ok(amount)
}

pub fn withdraw_collateral(e: Env, user: Address, amount: i128) -> i128 {
pub fn withdraw_collateral(e: Env, user: Address, amount: i128) -> Result<i128, Error> {
user.require_auth();
Self::add_interest_to_accrual(e.clone());
Self::add_interest_to_accrual(e.clone())?;

let loan_manager_addr = pool::read_loan_manager_addr(&e);
let loan_manager_addr = pool::read_loan_manager_addr(&e)?;
loan_manager_addr.require_auth();
assert!(amount > 0, "Amount must be positive!");

let token_address = &pool::read_currency(&e).token_address;
let token_address = &pool::read_currency(&e)?.token_address;
let client = token::Client::new(&e, token_address);
client.transfer(&e.current_contract_address(), &user, &amount);

Expand All @@ -175,29 +179,30 @@ impl LoanPoolContract {
let receivables: i128 = 0; // temp test param
positions::decrease_positions(&e, user.clone(), receivables, liabilities, amount);

amount
Ok(amount)
}

pub fn add_interest_to_accrual(e: Env) {
pub fn add_interest_to_accrual(e: Env) -> Result<(), Error> {
const DECIMAL: i128 = 10000000;
const SECONDS_IN_YEAR: u64 = 31_556_926;

let current_timestamp = e.ledger().timestamp();
let accrual = pool::read_accrual(&e);
let accrual_last_update = pool::read_accrual_last_updated(&e);
let accrual = pool::read_accrual(&e)?;
let accrual_last_update = pool::read_accrual_last_updated(&e)?;
let ledgers_since_update = current_timestamp - accrual_last_update;
let ledger_ratio: i128 =
(i128::from(ledgers_since_update) * DECIMAL) / (i128::from(SECONDS_IN_YEAR));

let interest_rate: i128 = get_interest(e.clone());
let interest_rate: i128 = get_interest(e.clone())?;
let interest_amount_in_year: i128 = (accrual * interest_rate) / DECIMAL;
let interest_since_update: i128 = (interest_amount_in_year * ledger_ratio) / DECIMAL;
let new_accrual: i128 = accrual + interest_since_update;

pool::write_accrual(&e, new_accrual);
Ok(())
}

pub fn get_accrual(e: Env) -> i128 {
pub fn get_accrual(e: Env) -> Result<i128, Error> {
pool::read_accrual(&e)
}

Expand All @@ -207,34 +212,35 @@ impl LoanPoolContract {
}

/// Get contract data entries
pub fn get_contract_balance(e: Env) -> i128 {
pub fn get_contract_balance(e: Env) -> Result<i128, Error> {
pool::read_total_balance(&e)
}

pub fn get_available_balance(e: Env) -> i128 {
pub fn get_available_balance(e: Env) -> Result<i128, Error> {
pool::read_available_balance(&e)
}

pub fn get_currency(e: Env) -> Currency {
pub fn get_currency(e: Env) -> Result<Currency, Error> {
pool::read_currency(&e)
}

pub fn get_interest(e: Env) -> i128 {
pub fn get_interest(e: Env) -> Result<i128, Error> {
interest::get_interest(e)
}

pub fn increase_liabilities(e: Env, user: Address, amount: i128) {
let loan_manager_addr = pool::read_loan_manager_addr(&e);
pub fn increase_liabilities(e: Env, user: Address, amount: i128) -> Result<(), Error> {
let loan_manager_addr = pool::read_loan_manager_addr(&e)?;
loan_manager_addr.require_auth();

positions::increase_positions(&e, user, 0, amount, 0);
Ok(())
}

pub fn repay(e: Env, user: Address, amount: i128, unpaid_interest: i128) {
let loan_manager_addr = pool::read_loan_manager_addr(&e);
pub fn repay(e: Env, user: Address, amount: i128, unpaid_interest: i128) -> Result<(), Error> {
let loan_manager_addr = pool::read_loan_manager_addr(&e)?;
loan_manager_addr.require_auth();

Self::add_interest_to_accrual(e.clone());
Self::add_interest_to_accrual(e.clone())?;

let amount_to_admin = if amount < unpaid_interest {
amount / 10
Expand All @@ -244,13 +250,14 @@ impl LoanPoolContract {

let amount_to_pool = amount - amount_to_admin;

let client = token::Client::new(&e, &pool::read_currency(&e).token_address);
let client = token::Client::new(&e, &pool::read_currency(&e)?.token_address);
client.transfer(&user, &e.current_contract_address(), &amount_to_pool);
client.transfer(&user, &loan_manager_addr, &amount_to_admin);

positions::decrease_positions(&e, user, 0, amount, 0);
pool::change_available_balance(&e, amount);
pool::change_total_balance(&e, amount);
pool::change_available_balance(&e, amount)?;
pool::change_total_balance(&e, amount)?;
Ok(())
}

pub fn liquidate(
Expand All @@ -259,11 +266,11 @@ impl LoanPoolContract {
amount: i128,
unpaid_interest: i128,
loan_owner: Address,
) {
let loan_manager_addr = pool::read_loan_manager_addr(&e);
) -> Result<(), Error> {
let loan_manager_addr = pool::read_loan_manager_addr(&e)?;
loan_manager_addr.require_auth();

Self::add_interest_to_accrual(e.clone());
Self::add_interest_to_accrual(e.clone())?;

let amount_to_admin = if amount < unpaid_interest {
amount / 10
Expand All @@ -273,28 +280,30 @@ impl LoanPoolContract {

let amount_to_pool = amount - amount_to_admin;

let client = token::Client::new(&e, &pool::read_currency(&e).token_address);
let client = token::Client::new(&e, &pool::read_currency(&e)?.token_address);
client.transfer(&user, &e.current_contract_address(), &amount_to_pool);
client.transfer(&user, &loan_manager_addr, &amount_to_admin);

positions::decrease_positions(&e, loan_owner, 0, amount, 0);
pool::change_available_balance(&e, amount);
pool::change_total_balance(&e, amount);
pool::change_available_balance(&e, amount)?;
pool::change_total_balance(&e, amount)?;
Ok(())
}

pub fn liquidate_transfer_collateral(
e: Env,
user: Address,
amount_collateral: i128,
loan_owner: Address,
) {
let loan_manager_addr = pool::read_loan_manager_addr(&e);
) -> Result<(), Error> {
let loan_manager_addr = pool::read_loan_manager_addr(&e)?;
loan_manager_addr.require_auth();

let client = token::Client::new(&e, &pool::read_currency(&e).token_address);
let client = token::Client::new(&e, &pool::read_currency(&e)?.token_address);
client.transfer(&e.current_contract_address(), &user, &amount_collateral);

positions::decrease_positions(&e, loan_owner, 0, 0, amount_collateral);
Ok(())
}
}

Expand Down
Loading
Loading