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

Migrate to v0.11 #104

Merged
merged 20 commits into from
Oct 5, 2020
Merged

Migrate to v0.11 #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
87aaddc
Migrate contracts to >0.10.1 (0.11)
maurolacy Sep 20, 2020
bfbd3fa
Migrate to 0.11
maurolacy Sep 20, 2020
f11ff50
Update dependencies to 0.11.0-alpha2
maurolacy Sep 23, 2020
bd8c8b2
Change to new bucket parameters order
maurolacy Sep 23, 2020
0551997
(Manually) fix rebase from master
maurolacy Sep 24, 2020
b6c116e
Update deps to v0.11.0-alpha3
maurolacy Sep 30, 2020
a1e8c26
Migrate cw1-whitelist to ContractError
maurolacy Sep 30, 2020
c3bbfc2
Migrate cw1-subkeys to thiserror / custom ContractError
maurolacy Oct 2, 2020
4b47e56
Update Cargo.lock
maurolacy Oct 2, 2020
ccf4f9a
Migrate cw20-atomic-swap to thiserror / ContractError
maurolacy Oct 3, 2020
9bd5bda
Fix Underflow error match
maurolacy Oct 3, 2020
1c8bbb3
Fix Cargo.lock
maurolacy Oct 3, 2020
2b0f72d
Better / consolidated error types
maurolacy Oct 4, 2020
a89ffee
Reverting NotFound to NoAllowance
maurolacy Oct 4, 2020
b31a5f8
Custom errors specialization
maurolacy Oct 4, 2020
0ffdab5
Migrate cw20-base to thiserror / ContractError
maurolacy Oct 4, 2020
9685727
Migrate cw20-escrow to thiserror / ContractError
maurolacy Oct 4, 2020
678dba9
Migrate cw20-staking to thiserror / ContractError
maurolacy Oct 4, 2020
c4389eb
Upstream versions of cw* pkgs for cw3-fixed-multisig (not yet migrate…
maurolacy Oct 4, 2020
8e48aaa
Upstream versions of cw* pkgs for cw721-nft (not yet migrated to 0.11)
maurolacy Oct 4, 2020
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
279 changes: 180 additions & 99 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions contracts/cw1-subkeys/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ backtraces = ["cosmwasm-std/backtraces"]
library = []

[dependencies]
cosmwasm-std = { version = "0.10.1", features = ["iterator", "staking"] }
cosmwasm-storage = { version = "0.10.1", features = ["iterator"] }
cw0 = { path = "../../packages/cw0", version = "0.2.2" }
cw1 = { path = "../../packages/cw1", version = "0.2.2" }
cw2 = { path = "../../packages/cw2", version = "0.2.2" }
cw1-whitelist = { path = "../cw1-whitelist", version = "0.2.2", features = ["library"] }
cosmwasm-std = { version = "0.11.0-alpha3", features = ["iterator", "staking"] }
cosmwasm-storage = { version = "0.11.0-alpha3", features = ["iterator"] }
schemars = "0.7"
serde = { version = "1.0.103", default-features = false, features = ["derive"] }
snafu = { version = "0.6.3" }
thiserror = { version = "1.0.20" }

[dev-dependencies]
cosmwasm-schema = { version = "0.10.1" }
cosmwasm-schema = { version = "0.11.0-alpha3" }
146 changes: 75 additions & 71 deletions contracts/cw1-subkeys/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use schemars::JsonSchema;
use std::fmt;
use std::ops::{AddAssign, Sub};

use cosmwasm_std::{
log, to_binary, Api, BankMsg, Binary, CanonicalAddr, Coin, CosmosMsg, Empty, Env, Extern,
attr, to_binary, Api, BankMsg, Binary, CanonicalAddr, Coin, CosmosMsg, Empty, Env, Extern,
HandleResponse, HumanAddr, InitResponse, Order, Querier, StakingMsg, StdError, StdResult,
Storage,
};
Expand All @@ -15,15 +16,14 @@ use cw1_whitelist::{
};
use cw2::set_contract_version;

use crate::error::ContractError;
use crate::msg::{
AllAllowancesResponse, AllPermissionsResponse, AllowanceInfo, HandleMsg, PermissionsInfo,
QueryMsg,
};
use crate::state::{
allowances, allowances_read, permissions, permissions_read, Allowance, PermissionErr,
Permissions,
allowances, allowances_read, permissions, permissions_read, Allowance, Permissions,
};
use std::ops::{AddAssign, Sub};

// version info for migration info
const CONTRACT_NAME: &str = "crates.io:cw1-subkeys";
Expand All @@ -45,11 +45,11 @@ pub fn handle<S: Storage, A: Api, Q: Querier>(
// Note: implement this function with different type to add support for custom messages
// and then import the rest of this contract code.
msg: HandleMsg<Empty>,
) -> StdResult<HandleResponse<Empty>> {
) -> Result<HandleResponse<Empty>, ContractError> {
match msg {
HandleMsg::Execute { msgs } => handle_execute(deps, env, msgs),
HandleMsg::Freeze {} => handle_freeze(deps, env),
HandleMsg::UpdateAdmins { admins } => handle_update_admins(deps, env, admins),
HandleMsg::Freeze {} => Ok(handle_freeze(deps, env)?),
HandleMsg::UpdateAdmins { admins } => Ok(handle_update_admins(deps, env, admins)?),
HandleMsg::IncreaseAllowance {
spender,
amount,
Expand All @@ -71,7 +71,7 @@ pub fn handle_execute<S: Storage, A: Api, Q: Querier, T>(
deps: &mut Extern<S, A, Q>,
env: Env,
msgs: Vec<CosmosMsg<T>>,
) -> StdResult<HandleResponse<T>>
) -> Result<HandleResponse<T>, ContractError>
where
T: Clone + fmt::Debug + PartialEq + JsonSchema,
{
Expand All @@ -81,16 +81,15 @@ where
if cfg.is_admin(owner_raw) {
let mut res = HandleResponse::default();
res.messages = msgs;
res.log = vec![log("action", "execute"), log("owner", env.message.sender)];
res.attributes = vec![attr("action", "execute"), attr("owner", env.message.sender)];
Ok(res)
} else {
for msg in &msgs {
match msg {
CosmosMsg::Staking(staking_msg) => {
let permissions = permissions(&mut deps.storage);
let perm = permissions.may_load(owner_raw.as_slice())?;
let perm =
perm.ok_or_else(|| StdError::not_found("No permissions for this account"))?;
let perm = perm.ok_or_else(|| ContractError::NotAllowed {})?;

check_staking_permissions(staking_msg, perm)?;
}
Expand All @@ -101,21 +100,20 @@ where
}) => {
let mut allowances = allowances(&mut deps.storage);
let allow = allowances.may_load(owner_raw.as_slice())?;
let mut allowance = allow
.ok_or_else(|| StdError::not_found("No allowance for this account"))?;
let mut allowance = allow.ok_or_else(|| ContractError::NoAllowance {})?;
// Decrease allowance
allowance.balance = allowance.balance.sub(amount.clone())?;
allowances.save(owner_raw.as_slice(), &allowance)?;
}
_ => {
return Err(StdError::generic_err("Message type rejected"));
return Err(ContractError::MessageTypeRejected {});
}
}
}
// Relay messages
let res = HandleResponse {
messages: msgs,
log: vec![log("action", "execute"), log("owner", env.message.sender)],
attributes: vec![attr("action", "execute"), attr("owner", env.message.sender)],
data: None,
};
Ok(res)
Expand All @@ -125,26 +123,26 @@ where
pub fn check_staking_permissions(
staking_msg: &StakingMsg,
permissions: Permissions,
) -> Result<bool, PermissionErr> {
) -> Result<bool, ContractError> {
match staking_msg {
StakingMsg::Delegate { .. } => {
if !permissions.delegate {
return Err(PermissionErr::Delegate {});
return Err(ContractError::DelegatePerm {});
}
}
StakingMsg::Undelegate { .. } => {
if !permissions.undelegate {
return Err(PermissionErr::Undelegate {});
return Err(ContractError::UnDelegatePerm {});
}
}
StakingMsg::Redelegate { .. } => {
if !permissions.redelegate {
return Err(PermissionErr::Redelegate {});
return Err(ContractError::ReDelegatePerm {});
}
}
StakingMsg::Withdraw { .. } => {
if !permissions.withdraw {
return Err(PermissionErr::Withdraw {});
return Err(ContractError::WithdrawPerm {});
}
}
}
Expand All @@ -157,7 +155,7 @@ pub fn handle_increase_allowance<S: Storage, A: Api, Q: Querier, T>(
spender: HumanAddr,
amount: Coin,
expires: Option<Expiration>,
) -> StdResult<HandleResponse<T>>
) -> Result<HandleResponse<T>, ContractError>
where
T: Clone + fmt::Debug + PartialEq + JsonSchema,
{
Expand All @@ -166,13 +164,13 @@ where
let owner_raw = &deps.api.canonical_address(&env.message.sender)?;

if !cfg.is_admin(&owner_raw) {
return Err(StdError::unauthorized());
return Err(ContractError::Unauthorized {});
}
if spender_raw == owner_raw {
return Err(StdError::generic_err("Cannot set allowance to own account"));
return Err(ContractError::CannotSetOwnAccount {});
}

allowances(&mut deps.storage).update(spender_raw.as_slice(), |allow| {
allowances(&mut deps.storage).update::<_, StdError>(spender_raw.as_slice(), |allow| {
let mut allowance = allow.unwrap_or_default();
if let Some(exp) = expires {
allowance.expires = exp;
Expand All @@ -183,12 +181,12 @@ where

let res = HandleResponse {
messages: vec![],
log: vec![
log("action", "increase_allowance"),
log("owner", env.message.sender),
log("spender", spender),
log("denomination", amount.denom),
log("amount", amount.amount),
attributes: vec![
attr("action", "increase_allowance"),
attr("owner", env.message.sender),
attr("spender", spender),
attr("denomination", amount.denom),
attr("amount", amount.amount),
],
data: None,
};
Expand All @@ -201,7 +199,7 @@ pub fn handle_decrease_allowance<S: Storage, A: Api, Q: Querier, T>(
spender: HumanAddr,
amount: Coin,
expires: Option<Expiration>,
) -> StdResult<HandleResponse<T>>
) -> Result<HandleResponse<T>, ContractError>
where
T: Clone + fmt::Debug + PartialEq + JsonSchema,
{
Expand All @@ -210,34 +208,36 @@ where
let owner_raw = &deps.api.canonical_address(&env.message.sender)?;

if !cfg.is_admin(&owner_raw) {
return Err(StdError::unauthorized());
return Err(ContractError::Unauthorized {});
}
if spender_raw == owner_raw {
return Err(StdError::generic_err("Cannot set allowance to own account"));
return Err(ContractError::CannotSetOwnAccount {});
}

let allowance = allowances(&mut deps.storage).update(spender_raw.as_slice(), |allow| {
// Fail fast
let mut allowance =
allow.ok_or_else(|| StdError::not_found("No allowance for this account"))?;
if let Some(exp) = expires {
allowance.expires = exp;
}
allowance.balance = allowance.balance.sub_saturating(amount.clone())?; // Tolerates underflows (amount bigger than balance), but fails if there are no tokens at all for the denom (report potential errors)
Ok(allowance)
})?;
let allowance = allowances(&mut deps.storage).update::<_, ContractError>(
spender_raw.as_slice(),
|allow| {
// Fail fast
let mut allowance = allow.ok_or_else(|| ContractError::NoAllowance {})?;
if let Some(exp) = expires {
allowance.expires = exp;
}
allowance.balance = allowance.balance.sub_saturating(amount.clone())?; // Tolerates underflows (amount bigger than balance), but fails if there are no tokens at all for the denom (report potential errors)
Ok(allowance)
},
)?;
if allowance.balance.is_empty() {
allowances(&mut deps.storage).remove(spender_raw.as_slice());
}

let res = HandleResponse {
messages: vec![],
log: vec![
log("action", "decrease_allowance"),
log("owner", deps.api.human_address(owner_raw)?),
log("spender", spender),
log("denomination", amount.denom),
log("amount", amount.amount),
attributes: vec![
attr("action", "decrease_allowance"),
attr("owner", deps.api.human_address(owner_raw)?),
attr("spender", spender),
attr("denomination", amount.denom),
attr("amount", amount.amount),
],
data: None,
};
Expand All @@ -249,7 +249,7 @@ pub fn handle_set_permissions<S: Storage, A: Api, Q: Querier, T>(
env: Env,
spender: HumanAddr,
perm: Permissions,
) -> StdResult<HandleResponse<T>>
) -> Result<HandleResponse<T>, ContractError>
where
T: Clone + fmt::Debug + PartialEq + JsonSchema,
{
Expand All @@ -258,22 +258,20 @@ where
let owner_raw = &deps.api.canonical_address(&env.message.sender)?;

if !cfg.is_admin(&owner_raw) {
return Err(StdError::unauthorized());
return Err(ContractError::Unauthorized {});
}
if spender_raw == owner_raw {
return Err(StdError::generic_err(
"Cannot set permission to own account",
));
return Err(ContractError::CannotSetOwnAccount {});
}
permissions(&mut deps.storage).save(spender_raw.as_slice(), &perm)?;

let res = HandleResponse {
messages: vec![],
log: vec![
log("action", "set_permissions"),
log("owner", deps.api.human_address(owner_raw)?),
log("spender", spender),
log("permissions", perm),
attributes: vec![
attr("action", "set_permissions"),
attr("owner", deps.api.human_address(owner_raw)?),
attr("spender", spender),
attr("permissions", perm),
],
data: None,
};
Expand Down Expand Up @@ -424,14 +422,17 @@ pub fn query_all_permissions<S: Storage, A: Api, Q: Querier>(

#[cfg(test)]
mod tests {
use super::*;
use crate::state::Permissions;
use cosmwasm_std::testing::{mock_dependencies, mock_env, MOCK_CONTRACT_ADDR};
use cosmwasm_std::{coin, coins, StakingMsg};

use cw0::NativeBalance;
use cw1_whitelist::msg::AdminListResponse;
use cw2::{get_contract_version, ContractVersion};

use crate::state::Permissions;

use super::*;

// this will set up the init for other tests
fn setup_test_case<S: Storage, A: Api, Q: Querier>(
mut deps: &mut Extern<S, A, Q>,
Expand Down Expand Up @@ -676,7 +677,7 @@ mod tests {
delegate: false,
redelegate: false,
undelegate: false,
withdraw: false
withdraw: false,
},
);

Expand All @@ -688,7 +689,7 @@ mod tests {
delegate: false,
redelegate: false,
undelegate: false,
withdraw: false
withdraw: false,
},
);

Expand Down Expand Up @@ -1166,7 +1167,7 @@ mod tests {
let env = mock_env(&spender2, &[]);
let res = handle(&mut deps, env, handle_msg.clone());
match res.unwrap_err() {
StdError::NotFound { .. } => {}
ContractError::NoAllowance { .. } => {}
e => panic!("unexpected error: {}", e),
}

Expand All @@ -1175,14 +1176,14 @@ mod tests {
let res = handle(&mut deps, env.clone(), handle_msg.clone()).unwrap();
assert_eq!(res.messages, msgs);
assert_eq!(
res.log,
vec![log("action", "execute"), log("owner", spender1.clone())]
res.attributes,
vec![attr("action", "execute"), attr("owner", spender1.clone())]
);

// And then cannot (not enough funds anymore)
let res = handle(&mut deps, env, handle_msg.clone());
match res.unwrap_err() {
StdError::Underflow { .. } => {}
ContractError::Std(StdError::Underflow { .. }) => {}
Copy link
Member

Choose a reason for hiding this comment

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

Good example to show how this is compatible with StdErrors

Copy link
Member

Choose a reason for hiding this comment

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

But since we are are creating custom errors everywhere, maybe we want to map this.
Either where it is generated like .map_err(|e| ...). Or do something like:

impl From<cw1_whitelist::ContractError> for ContractError
    fn from(err: cw1_whitelist::ContractError) -> Self {
        match err {
            cw1_whitelist::ContractError::Std(error) => ContractError::Std(error),
            cw1_whitelist::ContractError::Unauthorized {} => ContractError::Unauthorized {},
        }
    }
}

But mapping StdError::Underflow{} => ContractError::InsufficientAllowance{}?

Copy link
Member

Choose a reason for hiding this comment

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

Although the underflow message does show you the amounts, which is nice and shouldn't be lost.

This is an idea, take it or leave it.

Copy link
Contributor Author

@maurolacy maurolacy Oct 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I would leave it as it is. Besides the extra boilerplate, it's pretty clear what's being done.

e => panic!("unexpected error: {}", e),
}

Expand All @@ -1191,8 +1192,8 @@ mod tests {
let res = handle(&mut deps, env.clone(), handle_msg.clone()).unwrap();
assert_eq!(res.messages, msgs);
assert_eq!(
res.log,
vec![log("action", "execute"), log("owner", owner.clone())]
res.attributes,
vec![attr("action", "execute"), attr("owner", owner.clone())]
);

// For admins, even other message types are allowed
Expand All @@ -1204,13 +1205,16 @@ mod tests {
let env = mock_env(&owner, &[]);
let res = handle(&mut deps, env, handle_msg.clone()).unwrap();
assert_eq!(res.messages, other_msgs);
assert_eq!(res.log, vec![log("action", "execute"), log("owner", owner)]);
assert_eq!(
res.attributes,
vec![attr("action", "execute"), attr("owner", owner)]
);

// But not for mere mortals
let env = mock_env(&spender1, &[]);
let res = handle(&mut deps, env, handle_msg.clone());
match res.unwrap_err() {
StdError::GenericErr { msg, .. } => assert_eq!(&msg, "Message type rejected"),
ContractError::MessageTypeRejected { .. } => {}
e => panic!("unexpected error: {}", e),
}
}
Expand Down
Loading