Skip to content

Commit

Permalink
Stop treating storage errors as missing entries
Browse files Browse the repository at this point in the history
  • Loading branch information
@sisuresh
sisuresh committed Jul 6, 2023
1 parent 6845f9c commit 66a81b1
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 109 deletions.
1 change: 1 addition & 0 deletions soroban-env-host/src/native_contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ pub(crate) mod base_types;
pub(crate) mod common_types;
pub(crate) mod contract_error;
pub(crate) mod invoker_contract_auth;
pub(crate) mod storage_utils;
pub(crate) mod token;

use crate::host::{Host, HostError};
Expand Down
15 changes: 15 additions & 0 deletions soroban-env-host/src/native_contract/storage_utils.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
use soroban_env_common::{Env, StorageType, Val};

use crate::{Host, HostError};

pub(crate) struct StorageUtils {}

impl StorageUtils {
pub(crate) fn try_get(e: &Host, k: Val, t: StorageType) -> Result<Option<Val>, HostError> {
if e.has_contract_data(k, t.clone())?.into() {
Ok(Some(e.get_contract_data(k, t)?))
} else {
Ok(None)
}
}
}
56 changes: 32 additions & 24 deletions soroban-env-host/src/native_contract/token/allowance.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::host::Host;
use crate::native_contract::base_types::Address;
use crate::native_contract::contract_error::ContractError;
use crate::native_contract::storage_utils::StorageUtils;
use crate::native_contract::token::storage_types::{AllowanceDataKey, DataKey};
use crate::{err, HostError};
use soroban_env_common::{Env, StorageType, TryIntoVal};
Expand All @@ -10,15 +11,17 @@ use super::storage_types::AllowanceValue;
// Metering: covered by components
pub fn read_allowance(e: &Host, from: Address, spender: Address) -> Result<i128, HostError> {
let key = DataKey::Allowance(AllowanceDataKey { from, spender });
if let Ok(allowance) = e.get_contract_data(key.try_into_val(e)?, StorageType::Temporary) {
let val: AllowanceValue = allowance.try_into_val(e)?;
if val.expiration_ledger < e.get_ledger_sequence()?.into() {
Ok(0)
} else {
Ok(val.amount)
let res = StorageUtils::try_get(e, key.try_into_val(e)?, StorageType::Temporary)?;
match res {
Some(allowance) => {
let val: AllowanceValue = allowance.try_into_val(e)?;
if val.expiration_ledger < e.get_ledger_sequence()?.into() {
Ok(0)
} else {
Ok(val.amount)
}
}
} else {
Ok(0)
None => Ok(0),
}
}

Expand Down Expand Up @@ -63,23 +66,28 @@ pub fn write_allowance(
// Returns the allowance to write and the previous expiration of the existing allowance.
// If an allowance didn't exist, then the previous expiration will be None.
let allowance_with_old_expiration_option: Option<(AllowanceValue, Option<u32>)> =
if let Ok(allowance) = e.get_contract_data(key.try_into_val(e)?, StorageType::Temporary) {
let mut updated_allowance: AllowanceValue = allowance.try_into_val(e)?;
updated_allowance.amount = amount;
match StorageUtils::try_get(e, key.try_into_val(e)?, StorageType::Temporary)? {
Some(allowance) => {
let mut updated_allowance: AllowanceValue = allowance.try_into_val(e)?;
updated_allowance.amount = amount;

let old_expiration = updated_allowance.expiration_ledger;
updated_allowance.expiration_ledger = expiration;
Some((updated_allowance, Some(old_expiration)))
} else if amount > 0 {
Some((
AllowanceValue {
amount,
expiration_ledger: expiration,
},
None,
))
} else {
None
let old_expiration = updated_allowance.expiration_ledger;
updated_allowance.expiration_ledger = expiration;
Some((updated_allowance, Some(old_expiration)))
}
None => {
if amount > 0 {
Some((
AllowanceValue {
amount,
expiration_ledger: expiration,
},
None,
))
} else {
None
}
}
};

match allowance_with_old_expiration_option {
Expand Down
180 changes: 95 additions & 85 deletions soroban-env-host/src/native_contract/token/balance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::budget::AsBudget;
use crate::host::Host;
use crate::native_contract::base_types::{Address, BytesN};
use crate::native_contract::contract_error::ContractError;
use crate::native_contract::storage_utils::StorageUtils;
use crate::native_contract::token::asset_info::read_asset_info;
use crate::native_contract::token::public_types::AssetInfo;
use crate::native_contract::token::storage_types::DataKey;
Expand Down Expand Up @@ -30,13 +31,13 @@ pub fn read_balance(e: &Host, addr: Address) -> Result<i128, HostError> {
ScAddress::Account(acc_id) => Ok(get_classic_balance(e, acc_id)?.0.into()),
ScAddress::Contract(_) => {
let key = DataKey::Balance(addr);
if let Ok(raw_balance) =
e.get_contract_data(key.try_into_val(e)?, StorageType::Persistent)
{
let balance: BalanceValue = raw_balance.try_into_val(e)?;
Ok(balance.amount)
} else {
Ok(0)
let res = StorageUtils::try_get(e, key.try_into_val(e)?, StorageType::Persistent)?;
match res {
Some(raw_balance) => {
let balance: BalanceValue = raw_balance.try_into_val(e)?;
Ok(balance.amount)
}
None => Ok(0),
}
}
}
Expand Down Expand Up @@ -84,16 +85,16 @@ pub fn receive_balance(e: &Host, addr: Address, amount: i128) -> Result<(), Host
}
ScAddress::Contract(_) => {
let key = DataKey::Balance(addr.clone());
let mut balance = if let Ok(raw_balance) =
e.get_contract_data(key.try_into_val(e)?, StorageType::Persistent)
{
raw_balance.try_into_val(e)?
} else {
// balance passed the authorization check at the top of this function, so write true.
BalanceValue {
amount: 0,
authorized: true,
clawback: is_asset_clawback_enabled(e)?,
let res = StorageUtils::try_get(e, key.try_into_val(e)?, StorageType::Persistent)?;
let mut balance = match res {
Some(raw_balance) => raw_balance.try_into_val(e)?,
None => {
// balance passed the authorization check at the top of this function, so write true.
BalanceValue {
amount: 0,
authorized: true,
clawback: is_asset_clawback_enabled(e)?,
}
}
};

Expand Down Expand Up @@ -132,37 +133,41 @@ pub fn spend_balance_no_authorization_check(
// If a balance exists, calculate new amount and write the existing authorized state as is because
// this can be used to clawback when deauthorized.
let key = DataKey::Balance(addr.clone());
if let Ok(raw_balance) =
e.get_contract_data(key.try_into_val(e)?, StorageType::Persistent)
{
let mut balance: BalanceValue = raw_balance.try_into_val(e)?;
if balance.amount < amount {
return Err(err!(
e,
ContractError::BalanceError,
"balance is not sufficient to spend",
balance,
amount
));
} else {
let new_balance = balance.amount.checked_sub(amount).ok_or_else(|| {
e.error(
ContractError::OverflowError.into(),
"balance overflow in spend_balance_no_authorization_check",
&[],
)
})?;
balance.amount = new_balance;

write_balance(e, addr, balance)?
let res = StorageUtils::try_get(e, key.try_into_val(e)?, StorageType::Persistent)?;
match res {
Some(raw_balance) => {
let mut balance: BalanceValue = raw_balance.try_into_val(e)?;
if balance.amount < amount {
return Err(err!(
e,
ContractError::BalanceError,
"balance is not sufficient to spend",
balance,
amount
));
} else {
let new_balance = balance.amount.checked_sub(amount).ok_or_else(|| {
e.error(
ContractError::OverflowError.into(),
"balance overflow in spend_balance_no_authorization_check",
&[],
)
})?;
balance.amount = new_balance;

write_balance(e, addr, balance)?
}
}
None => {
if amount > 0 {
return Err(err!(
e,
ContractError::BalanceError,
"zero balance is not sufficient to spend",
amount
));
}
}
} else if amount > 0 {
return Err(err!(
e,
ContractError::BalanceError,
"zero balance is not sufficient to spend",
amount
));
}
Ok(())
}
Expand All @@ -188,13 +193,13 @@ pub fn is_authorized(e: &Host, addr: Address) -> Result<bool, HostError> {
ScAddress::Account(acc_id) => is_account_authorized(e, acc_id),
ScAddress::Contract(_) => {
let key = DataKey::Balance(addr);
if let Ok(raw_balance) =
e.get_contract_data(key.try_into_val(e)?, StorageType::Persistent)
{
let balance: BalanceValue = raw_balance.try_into_val(e)?;
Ok(balance.authorized)
} else {
Ok(!is_asset_auth_required(e)?)
let res = StorageUtils::try_get(e, key.try_into_val(e)?, StorageType::Persistent)?;
match res {
Some(raw_balance) => {
let balance: BalanceValue = raw_balance.try_into_val(e)?;
Ok(balance.authorized)
}
None => Ok(!is_asset_auth_required(e)?),
}
}
}
Expand All @@ -214,21 +219,24 @@ pub fn write_authorization(e: &Host, addr: Address, authorize: bool) -> Result<(
ScAddress::Account(acc_id) => set_authorization(e, acc_id, authorize),
ScAddress::Contract(_) => {
let key = DataKey::Balance(addr.clone());
if let Ok(raw_balance) =
e.get_contract_data(key.try_into_val(e)?, StorageType::Persistent)
{
let mut balance: BalanceValue = raw_balance.try_into_val(e)?;
balance.authorized = authorize;
write_balance(e, addr, balance)
} else {
// Balance does not exist, so write a 0 amount along with the authorization flag.
// No need to check auth_required because this function can only be called by the admin.
let balance = BalanceValue {
amount: 0,
authorized: authorize,
clawback: is_asset_clawback_enabled(e)?,
};
write_balance(e, addr, balance)

let res = StorageUtils::try_get(e, key.try_into_val(e)?, StorageType::Persistent)?;
match res {
Some(raw_balance) => {
let mut balance: BalanceValue = raw_balance.try_into_val(e)?;
balance.authorized = authorize;
write_balance(e, addr, balance)
}
None => {
// Balance does not exist, so write a 0 amount along with the authorization flag.
// No need to check auth_required because this function can only be called by the admin.
let balance = BalanceValue {
amount: 0,
authorized: authorize,
clawback: is_asset_clawback_enabled(e)?,
};
write_balance(e, addr, balance)
}
}
}
}
Expand Down Expand Up @@ -282,28 +290,30 @@ pub fn check_clawbackable(e: &Host, addr: Address) -> Result<(), HostError> {
},
ScAddress::Contract(_) => {
let key = DataKey::Balance(addr);
if let Ok(raw_balance) =
e.get_contract_data(key.try_into_val(e)?, StorageType::Persistent)
{
let balance: BalanceValue = raw_balance.try_into_val(e)?;
if !balance.clawback {

let res = StorageUtils::try_get(e, key.try_into_val(e)?, StorageType::Persistent)?;
match res {
Some(raw_balance) => {
let balance: BalanceValue = raw_balance.try_into_val(e)?;
if !balance.clawback {
return Err(e.error(
ContractError::BalanceError.into(),
"balance isn't clawbackable",
&[],
));
}
}
None => {
// We fail even if the clawback amount is 0. This is a better alternative than
// checking the issuer to make sure clawback is enabled and then succeeding
// in the 0 clawback case.
return Err(e.error(
ContractError::BalanceError.into(),
"balance isn't clawbackable",
"no balance to clawback",
&[],
));
}
} else {
// We fail even if the clawback amount is 0. This is a better alternative than
// checking the issuer to make sure clawback is enabled and then succeeding
// in the 0 clawback case.
return Err(e.error(
ContractError::BalanceError.into(),
"no balance to clawback",
&[],
));
}

Ok(())
}
}
Expand Down

0 comments on commit 66a81b1

Please sign in to comment.