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

Jump to bottom

Validate allowance expiration #793

Merged
merged 7 commits into from
Sep 5, 2022
Merged

Validate allowance expiration #793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8de8ad1
Add tests for allowance expiration validation
chipshort Sep 5, 2022
4852776
Validate that expiration is not expired when set
chipshort Sep 5, 2022
acd61c3
Fix tests
chipshort Sep 5, 2022
299a47d
Validate allowance expiration when decreasing
chipshort Sep 5, 2022
ce57759
Merge branch 'main' into validate-allowance-expiration
chipshort Sep 5, 2022
7afa2c4
Add own error variant for expiration validation
chipshort Sep 5, 2022
5d7e213
Fix clippy warnings
chipshort Sep 5, 2022
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
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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 {},
}