Skip to content

Commit

Permalink
Merge pull request #793 from CosmWasm/validate-allowance-expiration-2
Browse files Browse the repository at this point in the history 
Validate allowance expiration, closes #628
  • Loading branch information
@chipshort
chipshort committed Sep 5, 2022
2 parents 084e96d + 5d7e213 commit bc33936
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 23 deletions.
168 changes: 148 additions & 20 deletions contracts/cw20-base/src/allowances.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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)?;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {});
}
Expand Down Expand Up @@ -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 {});
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions contracts/cw20-base/src/enumerable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions contracts/cw20-base/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ pub enum ContractError {
#[error("Invalid png header")]
InvalidPngHeader {},

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

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

0 comments on commit bc33936

Please sign in to comment.