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

Port CW3 to Secret #1

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

Port CW3 to Secret #1

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2f21b42
Port CW3 to Secret
apollo-bobby Sep 5, 2022
ce7cce5
Remove debug statements, working comments
apollo-bobby Sep 19, 2022
c01f7f1
More debug cleanup, unused code/import removal, change const -> static
apollo-bobby Sep 20, 2022
9a9ec23
Remove voter insertion in execute_propose
apollo-bobby Sep 20, 2022
267f277
Refactoring and bug fixes
apollo-bobby Sep 21, 2022
9934cdb
Update top-level Cargo.toml
apollo-bobby Sep 21, 2022
d25ae1d
Port cw3-flex-multisig
apollo-bobby Sep 21, 2022
70afacf
WIP
apollo-bobby Oct 3, 2022
9492551
Change serialization from Bincode2 to JSON
apollo-bobby Oct 7, 2022
5543fd4
Move BST to storage-plus
apollo-bobby Nov 1, 2022
48a5522
Fixed bug in list_votes() and list_voters()
apollo-bobby Nov 1, 2022
995c9bc
port snapshot stuff (not map)
apollo-bobby Nov 1, 2022
6d41153
Port CW4-Group to Secret Network
apollo-bobby Nov 2, 2022
778cbf3
Fix bug in list_votes() for cw3-flex
apollo-bobby Nov 2, 2022
d890f21
clean up cw3-flex
apollo-bobby Nov 2, 2022
56f2c1a
chore: update dependencies and address review comments
apollo-bobby Feb 13, 2023
c2c08ed
fix: Make BST iterate from next largest element on miss
apollo-bobby Feb 27, 2023
c7d0eae
test: Fix tests failing due to BST iter behavior
apollo-bobby Feb 27, 2023
1282f06
test: Remove comments and todo!() in cw4 test
apollo-bobby Feb 27, 2023
3077125
test: Remove duplicated BST tests
apollo-bobby Mar 1, 2023
9df8d89
fix: Correct BST iter_from() behavior
apollo-bobby Mar 1, 2023
23e6b88
fix: Properly implement iter_from() and update snapshot module to match
apollo-bobby Mar 2, 2023
42e926f
chore: Update iter_from() usage in contracts
apollo-bobby Mar 2, 2023
91c3224
feat: Add largest() and smallest() to BST
apollo-bobby Mar 3, 2023
065a81f
fix: Partial fix for remove_checkpoint and tests
apollo-bobby Mar 3, 2023
6c4e19d
chore: Remove debug printouts
apollo-bobby Mar 3, 2023
6fbc029
feat: Add permit check for querying
apollo-bobby Mar 7, 2023
11b28a2
fix: Add contract address as allowed token in permit query
apollo-bobby Mar 7, 2023
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
2 changes: 2 additions & 0 deletions contracts/cw3-fixed-multisig/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ cw3 = { path = "../../packages/cw3", version = "0.13.4" }
cw-storage-plus = { path = "../../packages/storage-plus", version = "0.13.4" }
cosmwasm-std = { git = "https://github.com/scrtlabs/cosmwasm", branch = "secret" }
schemars = "0.8.1"
#secret-toolkit = { version = "0.4", default-features = false, features = ["utils", "storage", "serialization"] }
apollo-bobby marked this conversation as resolved.
Show resolved Hide resolved
secret-toolkit = { git = "https://github.com/scrtlabs/secret-toolkit", branch = "cosmwasm-v1.0", default-features = false, features = ["utils", "storage", "serialization"]}
serde = { version = "1.0.103", default-features = false, features = ["derive"] }
thiserror = { version = "1.0.23" }

Expand Down
224 changes: 134 additions & 90 deletions contracts/cw3-fixed-multisig/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,23 @@ use std::cmp::Ordering;
#[cfg(not(feature = "library"))]
use cosmwasm_std::entry_point;
use cosmwasm_std::{
to_binary, Binary, BlockInfo, CosmosMsg, Deps, DepsMut, Empty, Env, MessageInfo, Order,
Response, StdResult,
to_binary, Addr, Binary, BlockInfo, CosmosMsg, Deps, DepsMut, Empty, Env, MessageInfo,
Response, StdError, StdResult, Storage,
};

use cw2::set_contract_version;
use cw3::{
ProposalListResponse, ProposalResponse, Status, Vote, VoteInfo, VoteListResponse, VoteResponse,
VoterDetail, VoterListResponse, VoterResponse,
};
use cw_storage_plus::Bound;
use cw_storage_plus::CwIntKey;
use cw_utils::{Expiration, ThresholdResponse};

use crate::error::ContractError;
use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg};
use crate::state::{next_id, Ballot, Config, Proposal, Votes, BALLOTS, CONFIG, PROPOSALS, VOTERS};
use crate::state::{
next_id, Ballot, Config, Proposal, Votes, BALLOTS, CONFIG, PROPOSALS, VOTERS, VOTER_ADDRESSES,
};

// version info for migration info
const CONTRACT_NAME: &str = "crates.io:cw3-fixed-multisig";
Expand Down Expand Up @@ -49,7 +51,8 @@ pub fn instantiate(
// add all voters
for voter in msg.voters.iter() {
let key = deps.api.addr_validate(&voter.addr)?;
VOTERS.save(deps.storage, &key, &voter.weight)?;
VOTERS.insert(deps.storage, &key, &voter.weight)?;
VOTER_ADDRESSES.insert(deps.storage, &key)?;
Comment on lines +50 to +51
Copy link

Choose a reason for hiding this comment

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

curious why we need a separate storage item for map keys instead of using Map::keys()?

Copy link
Author

Choose a reason for hiding this comment

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

The addresses are stored in a BST to have persistent ordering and to avoid re-sorting every time we list votes or voters.
Also please note that the map structure used is KeyMap from secret-toolkit as opposed to the regular Map from cw-storage-plus; KeyMap does not have the same interface.

Copy link

Choose a reason for hiding this comment

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

what about using .iter_keys()? https://github.com/scrtlabs/secret-toolkit/blob/master/packages/storage/src/keymap.rs#L560-L565

my concern is the keys from the VOTERS map going out of sync from the VOTER_ADDRESSES list, would be easier to manage one source of truth instead

Copy link
Author

Choose a reason for hiding this comment

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

That works but then we're back to performing potentially prohibitively expensive sorting operations as the number of voters increases. The idea behind the BST was to allow the CW3 to scale to support an arbitrary amount of voters without significant increases in computational cost. But if that's not an intended use case I guess it doesn't matter. In that case the BST is pretty useless too.

I don't see how they are going to get out of sync as they are always mutated together; the only scenario I see is if VOTERS is successfully mutated and VOTER_ADDRESSES fails with an error, but I now realize this was probably the motivation behind the other snippet you commented on:

for addr in addresses {
    let weight = match VOTERS.get(deps.storage, &addr) {
        Some(weight) => weight,
        None => continue,
    };
    ...

Although we probably need to swap the insertion order so that there are always at worst more addresses than voters:

VOTER_ADDRESSES.insert(deps.storage, &key)?;
VOTERS.insert(deps.storage, &key, &voter.weight)?;

}
Ok(Response::default())
}
Expand Down Expand Up @@ -86,7 +89,7 @@ pub fn execute_propose(
) -> Result<Response<Empty>, ContractError> {
// only members of the multisig can create a proposal
let vote_power = VOTERS
.may_load(deps.storage, &info.sender)?
.get(deps.storage, &info.sender)
.ok_or(ContractError::Unauthorized {})?;

let cfg = CONFIG.load(deps.storage)?;
Expand Down Expand Up @@ -115,14 +118,20 @@ pub fn execute_propose(
};
prop.update_status(&env.block);
let id = next_id(deps.storage)?;
PROPOSALS.save(deps.storage, id, &prop)?;
PROPOSALS.insert(deps.storage, &id, &prop)?;

// add the first yes vote from voter
let ballot = Ballot {
weight: vote_power,
vote: Vote::Yes,
};
BALLOTS.save(deps.storage, (id, &info.sender), &ballot)?;
let (_, exists) = VOTER_ADDRESSES.find(deps.storage, &info.sender)?;
if !exists {
VOTER_ADDRESSES.insert(deps.storage, &info.sender)?;
}
BALLOTS
.add_suffix(&id.to_ne_bytes())
.insert(deps.storage, &info.sender, &ballot)?;

Ok(Response::new()
.add_attribute("action", "propose")
Expand All @@ -139,14 +148,13 @@ pub fn execute_vote(
vote: Vote,
) -> Result<Response<Empty>, ContractError> {
// only members of the multisig with weight >= 1 can vote
let voter_power = VOTERS.may_load(deps.storage, &info.sender)?;
let vote_power = match voter_power {
let vote_power = match VOTERS.get(deps.storage, &info.sender) {
Some(power) if power >= 1 => power,
_ => return Err(ContractError::Unauthorized {}),
};
dirtyshab marked this conversation as resolved.
Show resolved Hide resolved

// ensure proposal exists and can be voted on
let mut prop = PROPOSALS.load(deps.storage, proposal_id)?;
let mut prop = get_proposal(deps.storage, &proposal_id)?;
if prop.status != Status::Open {
return Err(ContractError::NotOpen {});
}
Expand All @@ -155,18 +163,22 @@ pub fn execute_vote(
}

// cast vote if no vote previously cast
BALLOTS.update(deps.storage, (proposal_id, &info.sender), |bal| match bal {
let suffix = proposal_id.to_ne_bytes();
let ballot = match BALLOTS.add_suffix(&suffix).get(deps.storage, &info.sender) {
Some(_) => Err(ContractError::AlreadyVoted {}),
None => Ok(Ballot {
weight: vote_power,
vote,
}),
})?;
}?;
dirtyshab marked this conversation as resolved.
Show resolved Hide resolved
BALLOTS
.add_suffix(&suffix)
.insert(deps.storage, &info.sender, &ballot)?;

// update vote tally
prop.votes.add_vote(vote, vote_power);
prop.update_status(&env.block);
PROPOSALS.save(deps.storage, proposal_id, &prop)?;
PROPOSALS.insert(deps.storage, &proposal_id, &prop)?;

Ok(Response::new()
.add_attribute("action", "vote")
Expand All @@ -183,7 +195,7 @@ pub fn execute_execute(
) -> Result<Response, ContractError> {
// anyone can trigger this if the vote passed

let mut prop = PROPOSALS.load(deps.storage, proposal_id)?;
let mut prop = get_proposal(deps.storage, &proposal_id)?;
// we allow execution even after the proposal "expiration" as long as all vote come in before
// that point. If it was approved on time, it can be executed any time.
if prop.status != Status::Passed {
Expand All @@ -192,7 +204,7 @@ pub fn execute_execute(

// set it to executed
prop.status = Status::Executed;
PROPOSALS.save(deps.storage, proposal_id, &prop)?;
PROPOSALS.insert(deps.storage, &proposal_id, &prop)?;

// dispatch all proposed messages
Ok(Response::new()
Expand All @@ -210,7 +222,7 @@ pub fn execute_close(
) -> Result<Response<Empty>, ContractError> {
// anyone can trigger this if the vote passed

let mut prop = PROPOSALS.load(deps.storage, proposal_id)?;
let mut prop = get_proposal(deps.storage, &proposal_id)?;
if [Status::Executed, Status::Rejected, Status::Passed]
.iter()
.any(|x| *x == prop.status)
Expand All @@ -223,7 +235,7 @@ pub fn execute_close(

// set it to failed
prop.status = Status::Rejected;
PROPOSALS.save(deps.storage, proposal_id, &prop)?;
PROPOSALS.insert(deps.storage, &proposal_id, &prop)?;

Ok(Response::new()
.add_attribute("action", "close")
Expand All @@ -238,12 +250,16 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult<Binary> {
QueryMsg::Proposal { proposal_id } => to_binary(&query_proposal(deps, env, proposal_id)?),
QueryMsg::Vote { proposal_id, voter } => to_binary(&query_vote(deps, proposal_id, voter)?),
QueryMsg::ListProposals { start_after, limit } => {
to_binary(&list_proposals(deps, env, start_after, limit)?)
let reverse = false;
to_binary(&list_proposals(deps, env, reverse, start_after, limit)?)
}
QueryMsg::ReverseProposals {
start_before,
limit,
} => to_binary(&reverse_proposals(deps, env, start_before, limit)?),
} => {
let reverse = true;
to_binary(&list_proposals(deps, env, reverse, start_before, limit)?)
}
dirtyshab marked this conversation as resolved.
Show resolved Hide resolved
QueryMsg::ListVotes {
proposal_id,
start_after,
Expand All @@ -262,7 +278,7 @@ fn query_threshold(deps: Deps) -> StdResult<ThresholdResponse> {
}

fn query_proposal(deps: Deps, env: Env, id: u64) -> StdResult<ProposalResponse> {
let prop = PROPOSALS.load(deps.storage, id)?;
let prop = get_proposal_std(deps.storage, &id)?;
apollo-bobby marked this conversation as resolved.
Show resolved Hide resolved
let status = prop.current_status(&env.block);
let threshold = prop.threshold.to_response(prop.total_weight);
Ok(ProposalResponse {
Expand All @@ -283,59 +299,72 @@ const DEFAULT_LIMIT: u32 = 10;
fn list_proposals(
deps: Deps,
env: Env,
start_after: Option<u64>,
reverse: bool,
starting_point: Option<u64>,
limit: Option<u32>,
) -> StdResult<ProposalListResponse> {
let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize;
let start = start_after.map(Bound::exclusive);
let proposals = PROPOSALS
.range(deps.storage, start, None, Order::Ascending)
.take(limit)
.map(|p| map_proposal(&env.block, p))
.collect::<StdResult<_>>()?;

Ok(ProposalListResponse { proposals })
let start = starting_point.unwrap_or(0) as usize;

let page_keys: Vec<u64> = {
if reverse {
PROPOSALS
.iter_keys(deps.storage)?
.rev()
.skip(start)
.take(limit)
.collect::<StdResult<Vec<_>>>()
} else {
PROPOSALS
.iter_keys(deps.storage)?
.skip(start)
.take(limit)
.collect::<StdResult<Vec<_>>>()
}?
};
dirtyshab marked this conversation as resolved.
Show resolved Hide resolved

let mut page = vec![];
for id in page_keys {
page.push(map_proposal(
&env.block,
(id, get_proposal_std(deps.storage, &id)?),
));
}
apollo-bobby marked this conversation as resolved.
Show resolved Hide resolved

Ok(ProposalListResponse { proposals: page })
}

fn reverse_proposals(
deps: Deps,
env: Env,
start_before: Option<u64>,
limit: Option<u32>,
) -> StdResult<ProposalListResponse> {
let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize;
let end = start_before.map(Bound::exclusive);
let props: StdResult<Vec<_>> = PROPOSALS
.range(deps.storage, None, end, Order::Descending)
.take(limit)
.map(|p| map_proposal(&env.block, p))
.collect();

Ok(ProposalListResponse { proposals: props? })
fn map_proposal(block: &BlockInfo, item: (u64, Proposal)) -> ProposalResponse {
let (id, prop) = item;
let status = prop.current_status(block);
let threshold = prop.threshold.to_response(prop.total_weight);
ProposalResponse {
id,
title: prop.title,
description: prop.description,
msgs: prop.msgs,
status,
expires: prop.expires,
threshold,
}
}

fn map_proposal(
block: &BlockInfo,
item: StdResult<(u64, Proposal)>,
) -> StdResult<ProposalResponse> {
item.map(|(id, prop)| {
let status = prop.current_status(block);
let threshold = prop.threshold.to_response(prop.total_weight);
ProposalResponse {
id,
title: prop.title,
description: prop.description,
msgs: prop.msgs,
status,
expires: prop.expires,
threshold,
}
// @todo do this with macros
fn get_proposal(store: &dyn Storage, id: &u64) -> Result<Proposal, ContractError> {
PROPOSALS.get(store, id).ok_or(ContractError::NotFound {})
}

fn get_proposal_std(store: &dyn Storage, id: &u64) -> StdResult<Proposal> {
PROPOSALS.get(store, id).ok_or(StdError::NotFound {
kind: "Proposal".to_string(),
})
}

fn query_vote(deps: Deps, proposal_id: u64, voter: String) -> StdResult<VoteResponse> {
let voter = deps.api.addr_validate(&voter)?;
let ballot = BALLOTS.may_load(deps.storage, (proposal_id, &voter))?;
let ballot = BALLOTS
.add_suffix(&proposal_id.to_ne_bytes())
.get(deps.storage, &voter);
let vote = ballot.map(|b| VoteInfo {
proposal_id,
voter: voter.into(),
Expand All @@ -352,28 +381,36 @@ fn list_votes(
limit: Option<u32>,
) -> StdResult<VoteListResponse> {
let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize;
let start = start_after.map(|s| Bound::ExclusiveRaw(s.into()));

let votes = BALLOTS
.prefix(proposal_id)
.range(deps.storage, start, None, Order::Ascending)
.take(limit)
.map(|item| {
item.map(|(addr, ballot)| VoteInfo {
proposal_id,
voter: addr.into(),
vote: ballot.vote,
weight: ballot.weight,
})
})
.collect::<StdResult<_>>()?;
let start_address = start_after.map(|addr| Addr::unchecked(addr));

let suffix = proposal_id.to_ne_bytes();
let ballots = BALLOTS.add_suffix(&suffix);
let addresses = match start_address {
Some(addr) => VOTER_ADDRESSES.iter_from(deps.storage, &addr)?,
None => VOTER_ADDRESSES.iter(deps.storage),
}
.take(limit);
apollo-bobby marked this conversation as resolved.
Show resolved Hide resolved

let mut votes = vec![];
for addr in addresses {
let ballot = match ballots.get(deps.storage, &addr) {
Some(ballot) => ballot,
None => continue,
};
votes.push(VoteInfo {
proposal_id,
voter: addr.into(),
vote: ballot.vote,
weight: ballot.weight,
});
}

Ok(VoteListResponse { votes })
}

fn query_voter(deps: Deps, voter: String) -> StdResult<VoterResponse> {
let voter = deps.api.addr_validate(&voter)?;
let weight = VOTERS.may_load(deps.storage, &voter)?;
let weight = VOTERS.get(deps.storage, &voter);
dirtyshab marked this conversation as resolved.
Show resolved Hide resolved
Ok(VoterResponse { weight })
}

Expand All @@ -383,18 +420,25 @@ fn list_voters(
limit: Option<u32>,
) -> StdResult<VoterListResponse> {
let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize;
let start = start_after.map(|s| Bound::ExclusiveRaw(s.into()));

let voters = VOTERS
.range(deps.storage, start, None, Order::Ascending)
.take(limit)
.map(|item| {
item.map(|(addr, weight)| VoterDetail {
addr: addr.into(),
weight,
})
})
.collect::<StdResult<_>>()?;
let start_address = start_after.map(|addr| Addr::unchecked(addr));

let addresses = match start_address {
Some(addr) => VOTER_ADDRESSES.iter_from(deps.storage, &addr)?,
None => VOTER_ADDRESSES.iter(deps.storage),
}
.take(limit);

let mut voters = vec![];
for addr in addresses {
let weight = match VOTERS.get(deps.storage, &addr) {
Some(weight) => weight,
None => continue,
};
apollo-bobby marked this conversation as resolved.
Show resolved Hide resolved
voters.push(VoterDetail {
addr: addr.to_string(),
weight,
});
}

Ok(VoterListResponse { voters })
}
Expand Down
Loading