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.

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

Validate allowance expiration #793

Merged
merged 7 commits into from
Sep 5, 2022
Merged
Changes from all commits
Commits
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
168 changes: 148 additions & 20 deletions contracts/cw20-base/src/allowances.rs
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ use crate::state::{ALLOWANCES, ALLOWANCES_SPENDER, BALANCES, TOKEN_INFO};

pub fn execute_increase_allowance(
deps: DepsMut,
_env: Env,
env: Env,
info: MessageInfo,
spender: String,
amount: Uint128,
@@ -20,9 +20,12 @@ pub fn execute_increase_allowance(
return Err(ContractError::CannotSetOwnAccount {});
}

let update_fn = |allow: Option<AllowanceResponse>| -> StdResult<_> {
let update_fn = |allow: Option<AllowanceResponse>| -> Result<_, _> {
let mut val = allow.unwrap_or_default();
if let Some(exp) = expires {
if exp.is_expired(&env.block) {
return Err(ContractError::InvalidExpiration {});
}
val.expires = exp;
}
val.allowance += amount;
@@ -42,7 +45,7 @@ pub fn execute_increase_allowance(

pub fn execute_decrease_allowance(
deps: DepsMut,
_env: Env,
env: Env,
info: MessageInfo,
spender: String,
amount: Uint128,
@@ -68,6 +71,9 @@ pub fn execute_decrease_allowance(
.checked_sub(amount)
.map_err(StdError::overflow)?;
if let Some(exp) = expires {
if exp.is_expired(&env.block) {
return Err(ContractError::InvalidExpiration {});
}
allowance.expires = exp;
}
ALLOWANCES.save(deps.storage, key, &allowance)?;
@@ -300,7 +306,7 @@ mod tests {

// set allowance with height expiration
let allow1 = Uint128::new(7777);
let expires = Expiration::AtHeight(5432);
let expires = Expiration::AtHeight(123_456);
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: allow1,
@@ -393,7 +399,7 @@ mod tests {

// set allowance with height expiration
let allow1 = Uint128::new(7777);
let expires = Expiration::AtHeight(5432);
let expires = Expiration::AtHeight(123_456);
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: allow1,
@@ -548,15 +554,17 @@ mod tests {
let err = execute(deps.as_mut(), env, info, msg).unwrap_err();
assert!(matches!(err, ContractError::Std(StdError::Overflow { .. })));

// let us increase limit, but set the expiration (default env height is 12_345)
// let us increase limit, but set the expiration to expire in the next block
let info = mock_info(owner.as_ref(), &[]);
let env = mock_env();
let mut env = mock_env();
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: Uint128::new(1000),
expires: Some(Expiration::AtHeight(env.block.height)),
expires: Some(Expiration::AtHeight(env.block.height + 1)),
};
execute(deps.as_mut(), env, info, msg).unwrap();
execute(deps.as_mut(), env.clone(), info, msg).unwrap();

env.block.height += 1;

// we should now get the expiration error
let msg = ExecuteMsg::TransferFrom {
@@ -565,7 +573,6 @@ mod tests {
amount: Uint128::new(33443),
};
let info = mock_info(spender.as_ref(), &[]);
let env = mock_env();
let err = execute(deps.as_mut(), env, info, msg).unwrap_err();
assert_eq!(err, ContractError::Expired {});
}
@@ -625,23 +632,25 @@ mod tests {
let err = execute(deps.as_mut(), env, info, msg).unwrap_err();
assert!(matches!(err, ContractError::Std(StdError::Overflow { .. })));

// let us increase limit, but set the expiration (default env height is 12_345)
// let us increase limit, but set the expiration to expire in the next block
let info = mock_info(owner.as_ref(), &[]);
let env = mock_env();
let mut env = mock_env();
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: Uint128::new(1000),
expires: Some(Expiration::AtHeight(env.block.height)),
expires: Some(Expiration::AtHeight(env.block.height + 1)),
};
execute(deps.as_mut(), env, info, msg).unwrap();
execute(deps.as_mut(), env.clone(), info, msg).unwrap();

// increase block height, so the limit is expired now
env.block.height += 1;

// we should now get the expiration error
let msg = ExecuteMsg::BurnFrom {
owner,
amount: Uint128::new(33443),
};
let info = mock_info(spender.as_ref(), &[]);
let env = mock_env();
let err = execute(deps.as_mut(), env, info, msg).unwrap_err();
assert_eq!(err, ContractError::Expired {});
}
@@ -726,15 +735,18 @@ mod tests {
let err = execute(deps.as_mut(), env, info, msg).unwrap_err();
assert!(matches!(err, ContractError::Std(StdError::Overflow { .. })));

// let us increase limit, but set the expiration to current block (expired)
// let us increase limit, but set the expiration to the next block
let info = mock_info(owner.as_ref(), &[]);
let env = mock_env();
let mut env = mock_env();
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: Uint128::new(1000),
expires: Some(Expiration::AtHeight(env.block.height)),
expires: Some(Expiration::AtHeight(env.block.height + 1)),
};
execute(deps.as_mut(), env, info, msg).unwrap();
execute(deps.as_mut(), env.clone(), info, msg).unwrap();

// increase block height, so the limit is expired now
env.block.height += 1;

// we should now get the expiration error
let msg = ExecuteMsg::SendFrom {
@@ -744,8 +756,124 @@ mod tests {
msg: send_msg,
};
let info = mock_info(spender.as_ref(), &[]);
let env = mock_env();
let err = execute(deps.as_mut(), env, info, msg).unwrap_err();
assert_eq!(err, ContractError::Expired {});
}

#[test]
fn no_past_expiration() {
let mut deps = mock_dependencies_with_balance(&coins(2, "token"));

let owner = String::from("addr0001");
let spender = String::from("addr0002");
let info = mock_info(owner.as_ref(), &[]);
let env = mock_env();
do_instantiate(deps.as_mut(), owner.clone(), Uint128::new(12340000));

// set allowance with height expiration at current block height
let expires = Expiration::AtHeight(env.block.height);
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: Uint128::new(7777),
expires: Some(expires),
};

// ensure it is rejected
assert_eq!(
Err(ContractError::InvalidExpiration {}),
execute(deps.as_mut(), env.clone(), info.clone(), msg)
);

// set allowance with time expiration in the past
let expires = Expiration::AtTime(env.block.time.minus_seconds(1));
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: Uint128::new(7777),
expires: Some(expires),
};

// ensure it is rejected
assert_eq!(
Err(ContractError::InvalidExpiration {}),
execute(deps.as_mut(), env.clone(), info.clone(), msg)
);

// set allowance with height expiration at next block height
let expires = Expiration::AtHeight(env.block.height + 1);
let allow = Uint128::new(7777);
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: allow,
expires: Some(expires),
};

execute(deps.as_mut(), env.clone(), info.clone(), msg).unwrap();

// ensure it looks good
let allowance = query_allowance(deps.as_ref(), owner.clone(), spender.clone()).unwrap();
assert_eq!(
allowance,
AllowanceResponse {
allowance: allow,
expires
}
);

// set allowance with time expiration in the future
let expires = Expiration::AtTime(env.block.time.plus_seconds(10));
let allow = Uint128::new(7777);
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: allow,
expires: Some(expires),
};

execute(deps.as_mut(), env.clone(), info.clone(), msg).unwrap();

// ensure it looks good
let allowance = query_allowance(deps.as_ref(), owner.clone(), spender.clone()).unwrap();
assert_eq!(
allowance,
AllowanceResponse {
allowance: allow + allow, // we increased twice
expires
}
);

// decrease with height expiration at current block height
let expires = Expiration::AtHeight(env.block.height);
let allow = Uint128::new(7777);
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: allow,
expires: Some(expires),
};

// ensure it is rejected
assert_eq!(
Err(ContractError::InvalidExpiration {}),
execute(deps.as_mut(), env.clone(), info.clone(), msg)
);

// decrease with height expiration at next block height
let expires = Expiration::AtHeight(env.block.height + 1);
let allow = Uint128::new(7777);
let msg = ExecuteMsg::DecreaseAllowance {
spender: spender.clone(),
amount: allow,
expires: Some(expires),
};

execute(deps.as_mut(), env, info, msg).unwrap();

// ensure it looks good
let allowance = query_allowance(deps.as_ref(), owner, spender).unwrap();
assert_eq!(
allowance,
AllowanceResponse {
allowance: allow,
expires
}
);
}
}
2 changes: 1 addition & 1 deletion contracts/cw20-base/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1403,7 +1403,7 @@ mod tests {

// Set allowance
let allow1 = Uint128::new(7777);
let expires = Expiration::AtHeight(5432);
let expires = Expiration::AtHeight(123_456);
let msg = CosmosMsg::Wasm(WasmMsg::Execute {
contract_addr: cw20_addr.to_string(),
msg: to_binary(&ExecuteMsg::IncreaseAllowance {
4 changes: 2 additions & 2 deletions contracts/cw20-base/src/enumerable.rs
Original file line number Diff line number Diff line change
@@ -127,7 +127,7 @@ mod tests {

// set allowance with height expiration
let allow1 = Uint128::new(7777);
let expires = Expiration::AtHeight(5432);
let expires = Expiration::AtHeight(123_456);
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender1.clone(),
amount: allow1,
@@ -192,7 +192,7 @@ mod tests {

// set allowance with height expiration
let allow1 = Uint128::new(7777);
let expires = Expiration::AtHeight(5432);
let expires = Expiration::AtHeight(123_456);
let msg = ExecuteMsg::IncreaseAllowance {
spender: spender.clone(),
amount: allow1,
3 changes: 3 additions & 0 deletions contracts/cw20-base/src/error.rs
Original file line number Diff line number Diff line change
@@ -33,6 +33,9 @@ pub enum ContractError {
#[error("Invalid png header")]
InvalidPngHeader {},

#[error("Invalid expiration value")]
InvalidExpiration {},

#[error("Duplicate initial balance addresses")]
DuplicateInitialBalanceAddresses {},
}