Repository: DA0-DA0/dao-contracts
Pull request #322 (2022-06-06 22:43:44)

Dedup duplicates
Patch issue 1 from our Oak Security audit.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 6b906cce..0a297e01 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -192,6 +192,7 @@ dependencies = [
  "cw-core-interface",
  "cw-core-macros",
  "cw-multi-test",
+ "cw-paginate",
  "cw-proposal-sudo",
  "cw-storage-plus",
  "cw-utils",
@@ -282,6 +283,17 @@ dependencies = [
  "thiserror",
 ]
 
+[[package]]
+name = "cw-paginate"
+version = "0.1.0"
+dependencies = [
+ "cosmwasm-std",
+ "cosmwasm-storage",
+ "cw-multi-test",
+ "cw-storage-plus",
+ "serde",
+]
+
 [[package]]
 name = "cw-proposal-single"
 version = "0.1.0"
diff --git a/contracts/cw-core/Cargo.toml b/contracts/cw-core/Cargo.toml
index 68068ad9..cf59b78b 100644
--- a/contracts/cw-core/Cargo.toml
+++ b/contracts/cw-core/Cargo.toml
@@ -41,6 +41,7 @@ serde = { version = "1.0", default-features = false, features = ["derive"] }
 thiserror = { version = "1.0" }
 cw-core-interface = { version = "0.1.0", path = "../../packages/cw-core-interface" }
 cw-core-macros = { version = "0.1.0", path = "../../packages/cw-core-macros" }
+cw-paginate = { version = "0.1.0", path = "../../packages/cw-paginate" }
 
 [dev-dependencies]
 cosmwasm-schema = { version = "1.0.0" }
diff --git a/contracts/cw-core/schema/get_item_response.json b/contracts/cw-core/schema/get_item_response.json
index 2e8bdf6f..11fd7904 100644
--- a/contracts/cw-core/schema/get_item_response.json
+++ b/contracts/cw-core/schema/get_item_response.json
@@ -6,20 +6,10 @@
   "properties": {
     "item": {
       "description": "`None` if no item with the provided key was found, `Some` otherwise.",
-      "anyOf": [
-        {
-          "$ref": "#/definitions/Addr"
-        },
-        {
-          "type": "null"
-        }
+      "type": [
+        "string",
+        "null"
       ]
     }
-  },
-  "definitions": {
-    "Addr": {
-      "description": "A human readable address.\n\nIn Cosmos, this is typically bech32 encoded. But for multi-chain smart contracts no assumptions should be made other than being UTF-8 encoded and of reasonable length.\n\nThis type represents a validated address. It can be created in the following ways 1. Use `Addr::unchecked(input)` 2. Use `let checked: Addr = deps.api.addr_validate(input)?` 3. Use `let checked: Addr = deps.api.addr_humanize(canonical_addr)?` 4. Deserialize from JSON. This must only be done from JSON that was validated before such as a contract's state. `Addr` must not be used in messages sent by the user because this would result in unvalidated instances.\n\nThis type is immutable. If you really need to mutate it (Really? Are you sure?), create a mutable copy using `let mut mutable = Addr::to_string()` and operate on that `String` instance.",
-      "type": "string"
-    }
   }
 }
diff --git a/contracts/cw-core/schema/instantiate_msg.json b/contracts/cw-core/schema/instantiate_msg.json
index 1df7c493..41f7da75 100644
--- a/contracts/cw-core/schema/instantiate_msg.json
+++ b/contracts/cw-core/schema/instantiate_msg.json
@@ -127,69 +127,19 @@
     "InitialItem": {
       "type": "object",
       "required": [
-        "info",
-        "name"
+        "key",
+        "value"
       ],
       "properties": {
-        "info": {
-          "description": "The info from which to derive the address.",
-          "allOf": [
-            {
-              "$ref": "#/definitions/InitialItemInfo"
-            }
-          ]
-        },
-        "name": {
+        "key": {
           "description": "The name of the item.",
           "type": "string"
-        }
-      }
-    },
-    "InitialItemInfo": {
-      "oneOf": [
-        {
-          "description": "An existing contract address.",
-          "type": "object",
-          "required": [
-            "Existing"
-          ],
-          "properties": {
-            "Existing": {
-              "type": "object",
-              "required": [
-                "address"
-              ],
-              "properties": {
-                "address": {
-                  "type": "string"
-                }
-              }
-            }
-          },
-          "additionalProperties": false
         },
-        {
-          "description": "Info for instantiating a new contract.",
-          "type": "object",
-          "required": [
-            "Instantiate"
-          ],
-          "properties": {
-            "Instantiate": {
-              "type": "object",
-              "required": [
-                "info"
-              ],
-              "properties": {
-                "info": {
-                  "$ref": "#/definitions/ModuleInstantiateInfo"
-                }
-              }
-            }
-          },
-          "additionalProperties": false
+        "value": {
+          "description": "The value the item will have at instantiation time.",
+          "type": "string"
         }
-      ]
+      }
     },
     "ModuleInstantiateInfo": {
       "description": "Information needed to instantiate a proposal or voting module.",
diff --git a/contracts/cw-core/schema/query_msg.json b/contracts/cw-core/schema/query_msg.json
index 83352ccb..9ce26da7 100644
--- a/contracts/cw-core/schema/query_msg.json
+++ b/contracts/cw-core/schema/query_msg.json
@@ -43,7 +43,7 @@
                 "integer",
                 "null"
               ],
-              "format": "uint64",
+              "format": "uint32",
               "minimum": 0.0
             },
             "start_at": {
@@ -72,7 +72,7 @@
                 "integer",
                 "null"
               ],
-              "format": "uint64",
+              "format": "uint32",
               "minimum": 0.0
             },
             "start_at": {
@@ -101,7 +101,7 @@
                 "integer",
                 "null"
               ],
-              "format": "uint64",
+              "format": "uint32",
               "minimum": 0.0
             },
             "start_at": {
@@ -150,7 +150,7 @@
       "additionalProperties": false
     },
     {
-      "description": "Lists all of the item keys associted with the contract. For example, given the items `{ \"group\": \"...\", \"subdao\": \"...\"}` this query would return `[\"group\", \"subdao\"]`.",
+      "description": "Lists all of the items associted with the contract. For example, given the items `{ \"group\": \"foo\", \"subdao\": \"bar\"}` this query would return `[(\"group\", \"foo\"), (\"subdao\", \"bar\")]`.",
       "type": "object",
       "required": [
         "list_items"
@@ -164,7 +164,7 @@
                 "integer",
                 "null"
               ],
-              "format": "uint64",
+              "format": "uint32",
               "minimum": 0.0
             },
             "start_at": {
@@ -193,7 +193,7 @@
                 "integer",
                 "null"
               ],
-              "format": "uint64",
+              "format": "uint32",
               "minimum": 0.0
             },
             "start_at": {
diff --git a/contracts/cw-core/src/contract.rs b/contracts/cw-core/src/contract.rs
index 24c01576..686dfbcf 100644
--- a/contracts/cw-core/src/contract.rs
+++ b/contracts/cw-core/src/contract.rs
@@ -1,5 +1,3 @@
-use std::convert::TryInto;
-
 #[cfg(not(feature = "library"))]
 use cosmwasm_std::entry_point;
 use cosmwasm_std::{
@@ -7,19 +5,19 @@ use cosmwasm_std::{
     StdResult, SubMsg,
 };
 use cw2::{get_contract_version, set_contract_version};
-use cw_storage_plus::{Bound, Map};
+use cw_storage_plus::Map;
 use cw_utils::{parse_reply_instantiate_data, Duration};
 
 use cw_core_interface::voting;
+use cw_paginate::{paginate_map, paginate_map_keys};
 
 use crate::error::ContractError;
 use crate::msg::{
-    ExecuteMsg, InitialItemInfo, InstantiateMsg, MigrateMsg, ModuleInstantiateInfo, QueryMsg,
+    ExecuteMsg, InitialItem, InstantiateMsg, MigrateMsg, ModuleInstantiateInfo, QueryMsg,
 };
 use crate::query::{Cw20BalanceResponse, DumpStateResponse, GetItemResponse, PauseInfoResponse};
 use crate::state::{
-    Config, ADMIN, CONFIG, CW20_LIST, CW721_LIST, ITEMS, PAUSED, PENDING_ITEM_INSTANTIATION_NAMES,
-    PROPOSAL_MODULES, VOTING_MODULE,
+    Config, ADMIN, CONFIG, CW20_LIST, CW721_LIST, ITEMS, PAUSED, PROPOSAL_MODULES, VOTING_MODULE,
 };
 
 // version info for migration info
@@ -30,15 +28,6 @@ const PROPOSAL_MODULE_REPLY_ID: u64 = 0;
 const VOTE_MODULE_INSTANTIATE_REPLY_ID: u64 = 1;
 const VOTE_MODULE_UPDATE_REPLY_ID: u64 = 2;
 
-// Start at this ID since the items to instantiate on the instantiation
-// of this contract can be arbitrarily long. Everything with a reply ID
-// greater than or equal to this value will be considered an instantiated
-// item to store in the items map.
-const PENDING_ITEM_REPLY_ID_START: u64 = 100;
-// The maximum number of items that can be instantiated when this
-// contract is instantiated.
-const MAX_ITEM_INSTANTIATIONS_ON_INSTANTIATE: u64 = u64::MAX - PENDING_ITEM_REPLY_ID_START;
-
 #[cfg_attr(not(feature = "library"), entry_point)]
 pub fn instantiate(
     deps: DepsMut,
@@ -79,52 +68,15 @@ pub fn instantiate(
         return Err(ContractError::NoProposalModule {});
     }
 
-    // Add or instantiate items if any are present.
-    let mut instantiate_item_msgs: Vec<SubMsg<Empty>> = vec![];
-    if let Some(items) = msg.initial_items {
-        if !items.is_empty() {
-            if items.len() > MAX_ITEM_INSTANTIATIONS_ON_INSTANTIATE.try_into().unwrap() {
-                return Err(ContractError::TooManyItems(
-                    MAX_ITEM_INSTANTIATIONS_ON_INSTANTIATE,
-                    items.len(),
-                ));
-            }
-
-            for (idx, item) in items.into_iter().enumerate() {
-                match item.info {
-                    // Use existing address.
-                    InitialItemInfo::Existing { address } => {
-                        let addr = deps.api.addr_validate(&address)?;
-                        ITEMS.save(deps.storage, item.name, &addr)?;
-                    }
-                    // Instantiate new contract and capture address on successful reply.
-                    InitialItemInfo::Instantiate { info } => {
-                        // Offset reply ID with index.
-                        let reply_id = PENDING_ITEM_REPLY_ID_START + idx as u64;
-
-                        // Create and add submessage.
-                        let item_msg = info.into_wasm_msg(env.contract.address.clone());
-                        let item_msg: SubMsg<Empty> = SubMsg::reply_on_success(item_msg, reply_id);
-                        instantiate_item_msgs.push(item_msg);
-
-                        // Store name in map for later retrieval if the contract instantiation succeeds.
-                        PENDING_ITEM_INSTANTIATION_NAMES.save(
-                            deps.storage,
-                            reply_id,
-                            &item.name,
-                        )?;
-                    }
-                }
-            }
-        }
+    for InitialItem { key, value } in msg.initial_items.unwrap_or_default() {
+        ITEMS.save(deps.storage, key, &value)?;
     }
 
     Ok(Response::new()
         .add_attribute("action", "instantiate")
         .add_attribute("sender", info.sender)
         .add_submessage(vote_module_msg)
-        .add_submessages(proposal_module_msgs)
-        .add_submessages(instantiate_item_msgs))
+        .add_submessages(proposal_module_msgs))
 }
 
 #[cfg_attr(not(feature = "library"), entry_point)]
@@ -404,18 +356,17 @@ pub fn execute_set_item(
     env: Env,
     sender: Addr,
     key: String,
-    addr: String,
+    value: String,
 ) -> Result<Response, ContractError> {
     if env.contract.address != sender {
         return Err(ContractError::Unauthorized {});
     }
 
-    let addr = deps.api.addr_validate(&addr)?;
-    ITEMS.save(deps.storage, key.clone(), &addr)?;
+    ITEMS.save(deps.storage, key.clone(), &value)?;
     Ok(Response::default()
         .add_attribute("action", "execute_set_item")
         .add_attribute("key", key)
-        .add_attribute("addr", addr))
+        .add_attribute("addr", value))
 }
 
 pub fn execute_remove_item(
@@ -502,9 +453,8 @@ pub fn query_voting_module(deps: Deps) -> StdResult<Binary> {
 pub fn query_proposal_modules(
     deps: Deps,
     start_at: Option<String>,
-    limit: Option<u64>,
+    limit: Option<u32>,
 ) -> StdResult<Binary> {
-    let start_at = start_at.map(|a| deps.api.addr_validate(&a)).transpose()?;
     // This query is will run out of gas due to the size of the
     // returned message before it runs out of compute so taking a
     // limit here is still nice. As removes happen in constant time
@@ -518,19 +468,13 @@ pub fn query_proposal_modules(
     //
     // Even if this does lock up one can determine the existing
     // proposal modules by looking at past transactions on chain.
-    let modules = PROPOSAL_MODULES.keys(
-        deps.storage,
-        start_at.map(Bound::inclusive),
-        None,
+    to_binary(&paginate_map_keys(
+        deps,
+        &PROPOSAL_MODULES,
+        start_at.map(|s| deps.api.addr_validate(&s)).transpose()?,
+        limit,
         cosmwasm_std::Order::Descending,
-    );
-    let modules: Vec<Addr> = match limit {
-        Some(limit) => modules
-            .take(limit as usize)
-            .collect::<Result<Vec<Addr>, _>>()?,
-        None => modules.collect::<Result<Vec<Addr>, _>>()?,
-    };
-    to_binary(&modules)
+    )?)
 }
 
 fn get_pause_info(deps: Deps, env: Env) -> StdResult<PauseInfoResponse> {
@@ -603,73 +547,42 @@ pub fn query_info(deps: Deps) -> StdResult<Binary> {
 pub fn query_list_items(
     deps: Deps,
     start_at: Option<String>,
-    limit: Option<u64>,
+    limit: Option<u32>,
 ) -> StdResult<Binary> {
-    let items = ITEMS.keys(
-        deps.storage,
-        start_at.map(Bound::inclusive),
-        None,
-        cosmwasm_std::Order::Descending,
-    );
-    let items = match limit {
-        Some(limit) => items
-            .take(limit as usize)
-            .collect::<Result<Vec<String>, _>>()?,
-        None => items.collect::<Result<Vec<String>, _>>()?,
-    };
-
-    to_binary(&items)
-}
-
-// Can't be generic over key type. Otherwise, we could use this in
-// `query_list_items` as well.
-// <https://github.com/CosmWasm/cw-plus/issues/691>
-fn list_addr_keys<V>(
-    deps: Deps,
-    map: Map<Addr, V>,
-    start_at: Option<Addr>,
-    limit: Option<u64>,
-) -> StdResult<Vec<Addr>>
-where
-    V: serde::de::DeserializeOwned + serde::Serialize,
-{
-    let items = map.keys(
-        deps.storage,
-        start_at.map(Bound::inclusive),
-        None,
+    to_binary(&paginate_map(
+        deps,
+        &ITEMS,
+        start_at,
+        limit,
         cosmwasm_std::Order::Descending,
-    );
-    match limit {
-        Some(limit) => Ok(items
-            .take(limit as usize)
-            .collect::<Result<Vec<Addr>, _>>()?),
-        None => Ok(items.collect::<Result<Vec<Addr>, _>>()?),
-    }
+    )?)
 }
 
 pub fn query_cw20_list(
     deps: Deps,
     start_at: Option<String>,
-    limit: Option<u64>,
+    limit: Option<u32>,
 ) -> StdResult<Binary> {
-    to_binary(&list_addr_keys(
+    to_binary(&paginate_map_keys(
         deps,
-        CW20_LIST,
+        &CW20_LIST,
         start_at.map(|s| deps.api.addr_validate(&s)).transpose()?,
         limit,
+        cosmwasm_std::Order::Descending,
     )?)
 }
 
 pub fn query_cw721_list(
     deps: Deps,
     start_at: Option<String>,
-    limit: Option<u64>,
+    limit: Option<u32>,
 ) -> StdResult<Binary> {
-    to_binary(&list_addr_keys(
+    to_binary(&paginate_map_keys(
         deps,
-        CW721_LIST,
+        &CW721_LIST,
         start_at.map(|s| deps.api.addr_validate(&s)).transpose()?,
         limit,
+        cosmwasm_std::Order::Descending,
     )?)
 }
 
@@ -677,13 +590,14 @@ pub fn query_cw20_balances(
     deps: Deps,
     env: Env,
     start_at: Option<String>,
-    limit: Option<u64>,
+    limit: Option<u32>,
 ) -> StdResult<Binary> {
-    let addrs = list_addr_keys(
+    let addrs = paginate_map_keys(
         deps,
-        CW20_LIST,
+        &CW20_LIST,
         start_at.map(|a| deps.api.addr_validate(&a)).transpose()?,
         limit,
+        cosmwasm_std::Order::Descending,
     )?;
     let balances = addrs
         .into_iter()
@@ -742,20 +656,6 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result<Response, ContractE
 
             Ok(Response::default().add_attribute("voting_module", vote_module_addr))
         }
-        reply_id if reply_id >= PENDING_ITEM_REPLY_ID_START => {
-            // Retrieve the name using the ID. If it doesn't exist,
-            // we didn't expect this reply or it was a redundant execution.
-            let item_name = PENDING_ITEM_INSTANTIATION_NAMES.load(deps.storage, reply_id)?;
-
-            let res = parse_reply_instantiate_data(msg)?;
-            let item_addr = deps.api.addr_validate(&res.contract_address)?;
-
-            ITEMS.save(deps.storage, item_name, &item_addr)?;
-            // Remove from pending map since we now have the contract address.
-            PENDING_ITEM_INSTANTIATION_NAMES.remove(deps.storage, reply_id);
-
-            Ok(Response::default().add_attribute("item", item_addr))
-        }
         _ => Err(ContractError::UnknownReplyID {}),
     }
 }
diff --git a/contracts/cw-core/src/msg.rs b/contracts/cw-core/src/msg.rs
index 089bff4f..22e61114 100644
--- a/contracts/cw-core/src/msg.rs
+++ b/contracts/cw-core/src/msg.rs
@@ -33,20 +33,12 @@ pub struct ModuleInstantiateInfo {
     pub label: String,
 }
 
-#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)]
-pub enum InitialItemInfo {
-    /// An existing contract address.
-    Existing { address: String },
-    /// Info for instantiating a new contract.
-    Instantiate { info: ModuleInstantiateInfo },
-}
-
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)]
 pub struct InitialItem {
     /// The name of the item.
-    pub name: String,
-    /// The info from which to derive the address.
-    pub info: InitialItemInfo,
+    pub key: String,
+    /// The value the item will have at instantiation time.
+    pub value: String,
 }
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema)]
@@ -149,19 +141,19 @@ pub enum QueryMsg {
     /// contract.
     Cw20Balances {
         start_at: Option<String>,
-        limit: Option<u64>,
+        limit: Option<u32>,
     },
     /// Lists the addresses of the cw20 tokens in this contract's
     /// treasury.
     Cw20TokenList {
         start_at: Option<String>,
-        limit: Option<u64>,
+        limit: Option<u32>,
     },
     /// Lists the addresses of the cw721 tokens in this contract's
     /// treasury.
     Cw721TokenList {
         start_at: Option<String>,
-        limit: Option<u64>,
+        limit: Option<u32>,
     },
     /// Dumps all of the core contract's state in a single
     /// query. Useful for frontends as performance for queries is more
@@ -170,18 +162,19 @@ pub enum QueryMsg {
     DumpState {},
     /// Gets the address associated with an item key.
     GetItem { key: String },
-    /// Lists all of the item keys associted with the contract. For
-    /// example, given the items `{ "group": "...", "subdao": "..."}`
-    /// this query would return `["group", "subdao"]`.
+    /// Lists all of the items associted with the contract. For
+    /// example, given the items `{ "group": "foo", "subdao": "bar"}`
+    /// this query would return `[("group", "foo"), ("subdao",
+    /// "bar")]`.
     ListItems {
         start_at: Option<String>,
-        limit: Option<u64>,
+        limit: Option<u32>,
     },
     /// Gets the proposal modules assocaited with the
     /// contract. Returns Vec<Addr>.
     ProposalModules {
         start_at: Option<String>,
-        limit: Option<u64>,
+        limit: Option<u32>,
     },
     /// Returns information about if the contract is currently paused.
     PauseInfo {},
diff --git a/contracts/cw-core/src/query.rs b/contracts/cw-core/src/query.rs
index e6fd0434..ee10975d 100644
--- a/contracts/cw-core/src/query.rs
+++ b/contracts/cw-core/src/query.rs
@@ -37,7 +37,7 @@ pub enum PauseInfoResponse {
 pub struct GetItemResponse {
     /// `None` if no item with the provided key was found, `Some`
     /// otherwise.
-    pub item: Option<Addr>,
+    pub item: Option<String>,
 }
 
 /// Returned by Cw20Balances query.
diff --git a/contracts/cw-core/src/state.rs b/contracts/cw-core/src/state.rs
index ee44b95f..705ebb0b 100644
--- a/contracts/cw-core/src/state.rs
+++ b/contracts/cw-core/src/state.rs
@@ -31,9 +31,8 @@ pub const PAUSED: Item<Expiration> = Item::new("paused");
 pub const VOTING_MODULE: Item<Addr> = Item::new("voting_module");
 pub const PROPOSAL_MODULES: Map<Addr, Empty> = Map::new("proposal_modules");
 
-pub const ITEMS: Map<String, Addr> = Map::new("items");
-pub const PENDING_ITEM_INSTANTIATION_NAMES: Map<u64, String> =
-    Map::new("pending_item_instantiations");
+// General purpose KV store for DAO associated state.
+pub const ITEMS: Map<String, String> = Map::new("items");
 
 /// Set of cw20 tokens that have been registered with this contract's
 /// treasury.
diff --git a/contracts/cw4-voting/src/contract.rs b/contracts/cw4-voting/src/contract.rs
index c28a4e39..57c1a97f 100644
--- a/contracts/cw4-voting/src/contract.rs
+++ b/contracts/cw4-voting/src/contract.rs
@@ -27,9 +27,18 @@ pub fn instantiate(
     if msg.initial_members.is_empty() {
         return Err(ContractError::NoMembers {});
     }
+    let original_len = msg.initial_members.len();
+    let mut initial_members = msg.initial_members;
+    initial_members.sort_by(|a, b| a.addr.cmp(&b.addr));
+    initial_members.dedup();
+    let new_len = initial_members.len();
+
+    if original_len != new_len {
+        return Err(ContractError::DuplicateMembers {});
+    }
 
     let mut total_weight = Uint128::zero();
-    for member in msg.initial_members.iter() {
+    for member in initial_members.iter() {
         let member_addr = deps.api.addr_validate(&member.addr)?;
         let weight = Uint128::from(member.weight);
         USER_WEIGHTS.save(deps.storage, &member_addr, &weight, env.block.height)?;
@@ -47,7 +56,7 @@ pub fn instantiate(
         code_id: msg.cw4_group_code_id,
         msg: to_binary(&cw4_group::msg::InstantiateMsg {
             admin: Some(env.contract.address.to_string()),
-            members: msg.initial_members,
+            members: initial_members,
         })?,
         funds: vec![],
         label: env.contract.address.to_string(),
diff --git a/contracts/cw4-voting/src/error.rs b/contracts/cw4-voting/src/error.rs
index d5bb7be3..a8bd1a90 100644
--- a/contracts/cw4-voting/src/error.rs
+++ b/contracts/cw4-voting/src/error.rs
@@ -18,6 +18,9 @@ pub enum ContractError {
     #[error("Cannot instantiate a group contract with no initial members")]
     NoMembers {},
 
+    #[error("Cannot instantiate a group contract with duplicate initial members")]
+    DuplicateMembers {},
+
     #[error("Total weight of the CW4 contract cannot be zero")]
     ZeroTotalWeight {},
 
diff --git a/packages/cw-paginate/Cargo.toml b/packages/cw-paginate/Cargo.toml
new file mode 100644
index 00000000..5257b616
--- /dev/null
+++ b/packages/cw-paginate/Cargo.toml
@@ -0,0 +1,15 @@
+[package]
+name = "cw-paginate"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+cosmwasm-std = { version = "1.0.0" }
+cosmwasm-storage = { version = "1.0.0" }
+cw-storage-plus = "0.13"
+serde = { version = "1.0", default-features = false }
+
+[dev-dependencies]
+cw-multi-test = "0.13"
diff --git a/packages/cw-paginate/src/lib.rs b/packages/cw-paginate/src/lib.rs
new file mode 100644
index 00000000..4f1526b8
--- /dev/null
+++ b/packages/cw-paginate/src/lib.rs
@@ -0,0 +1,124 @@
+use cosmwasm_std::{Deps, Order, StdResult};
+use cw_storage_plus::{Bound, Bounder, KeyDeserialize, Map};
+
+/// Generic function for paginating a list of (K, V) pairs in a
+/// CosmWasm Map.
+pub fn paginate_map<'a, K, V>(
+    deps: Deps,
+    map: &Map<'a, K, V>,
+    start_at: Option<K>,
+    limit: Option<u32>,
+    order: Order,
+) -> StdResult<Vec<(K, V)>>
+where
+    K: Bounder<'a> + KeyDeserialize<Output = K> + 'static,
+    V: serde::de::DeserializeOwned + serde::Serialize,
+{
+    let items = map.range(deps.storage, start_at.map(Bound::inclusive), None, order);
+    match limit {
+        Some(limit) => Ok(items
+            .take(limit.try_into().unwrap())
+            .collect::<StdResult<_>>()?),
+        None => Ok(items.collect::<StdResult<_>>()?),
+    }
+}
+
+/// Same as `paginate_map` but only returns the keys.
+pub fn paginate_map_keys<'a, K, V>(
+    deps: Deps,
+    map: &Map<'a, K, V>,
+    start_at: Option<K>,
+    limit: Option<u32>,
+    order: Order,
+) -> StdResult<Vec<K>>
+where
+    K: Bounder<'a> + KeyDeserialize<Output = K> + 'static,
+    V: serde::de::DeserializeOwned + serde::Serialize,
+{
+    let items = map.keys(deps.storage, start_at.map(Bound::inclusive), None, order);
+    match limit {
+        Some(limit) => Ok(items
+            .take(limit.try_into().unwrap())
+            .collect::<StdResult<_>>()?),
+        None => Ok(items.collect::<StdResult<_>>()?),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use cosmwasm_std::testing::mock_dependencies;
+
+    #[test]
+    fn pagination() {
+        let mut deps = mock_dependencies();
+        let map: Map<String, String> = Map::new("items");
+
+        for num in 1..3 {
+            map.save(&mut deps.storage, num.to_string(), &(num * 2).to_string())
+                .unwrap();
+        }
+
+        let items = paginate_map(deps.as_ref(), &map, None, None, Order::Descending).unwrap();
+        assert_eq!(
+            items,
+            vec![
+                ("2".to_string(), "4".to_string()),
+                ("1".to_string(), "2".to_string())
+            ]
+        );
+
+        let items = paginate_map(deps.as_ref(), &map, None, None, Order::Ascending).unwrap();
+        assert_eq!(
+            items,
+            vec![
+                ("1".to_string(), "2".to_string()),
+                ("2".to_string(), "4".to_string())
+            ]
+        );
+
+        let items = paginate_map(
+            deps.as_ref(),
+            &map,
+            Some("2".to_string()),
+            None,
+            Order::Ascending,
+        )
+        .unwrap();
+        assert_eq!(items, vec![("2".to_string(), "4".to_string())]);
+
+        let items = paginate_map(deps.as_ref(), &map, None, Some(1), Order::Ascending).unwrap();
+        assert_eq!(items, vec![("1".to_string(), "2".to_string())]);
+    }
+
+    #[test]
+    fn key_pagination() {
+        let mut deps = mock_dependencies();
+        let map: Map<String, String> = Map::new("items");
+
+        for num in 1..3 {
+            map.save(&mut deps.storage, num.to_string(), &(num * 2).to_string())
+                .unwrap();
+        }
+
+        let items = paginate_map_keys(deps.as_ref(), &map, None, None, Order::Descending).unwrap();
+        assert_eq!(items, vec!["2".to_string(), "1".to_string()]);
+
+        let items = paginate_map_keys(deps.as_ref(), &map, None, None, Order::Ascending).unwrap();
+        assert_eq!(items, vec!["1".to_string(), "2".to_string()]);
+
+        let items = paginate_map_keys(
+            deps.as_ref(),
+            &map,
+            Some("2".to_string()),
+            None,
+            Order::Ascending,
+        )
+        .unwrap();
+        assert_eq!(items, vec!["2"]);
+
+        let items =
+            paginate_map_keys(deps.as_ref(), &map, None, Some(1), Order::Ascending).unwrap();
+        assert_eq!(items, vec!["1".to_string()]);
+    }
+}


Test patch:
diff --git a/contracts/cw-core/src/tests.rs b/contracts/cw-core/src/tests.rs
index 4da879a0..063cc7be 100644
--- a/contracts/cw-core/src/tests.rs
+++ b/contracts/cw-core/src/tests.rs
@@ -6,8 +6,7 @@ use cw_utils::{Duration, Expiration};
 
 use crate::{
     msg::{
-        Admin, ExecuteMsg, InitialItem, InitialItemInfo, InstantiateMsg, MigrateMsg,
-        ModuleInstantiateInfo, QueryMsg,
+        Admin, ExecuteMsg, InitialItem, InstantiateMsg, MigrateMsg, ModuleInstantiateInfo, QueryMsg,
     },
     query::{Cw20BalanceResponse, DumpStateResponse, GetItemResponse, PauseInfoResponse},
     state::Config,
@@ -1046,8 +1045,8 @@ fn list_items(
     app: &mut App,
     gov_addr: Addr,
     start_at: Option<String>,
-    limit: Option<u64>,
-) -> Vec<String> {
+    limit: Option<u32>,
+) -> Vec<(String, String)> {
     app.wrap()
         .query_wasm_smart(gov_addr, &QueryMsg::ListItems { start_at, limit })
         .unwrap()
@@ -1070,7 +1069,7 @@ fn test_add_remove_get() {
     assert_eq!(
         a,
         GetItemResponse {
-            item: Some(Addr::unchecked("aaaaaaddr"))
+            item: Some("aaaaaaddr".to_string())
         }
     );
 
@@ -1153,9 +1152,11 @@ fn test_list_items() {
         "baraddr".to_string(),
     );
 
+    // Foo returned as we are only getting one item and items are in
+    // decending order.
     let first_item = list_items(&mut app, gov_addr.clone(), None, Some(1));
     assert_eq!(first_item.len(), 1);
-    assert_eq!(first_item[0], "fookey".to_string());
+    assert_eq!(first_item[0], ("fookey".to_string(), "fooaddr".to_string()));
 
     let no_items = list_items(&mut app, gov_addr.clone(), None, Some(0));
     assert_eq!(no_items.len(), 0);
@@ -1164,7 +1165,10 @@ fn test_list_items() {
     // no limit ought to give us a single item.
     let second_item = list_items(&mut app, gov_addr, Some("foo".to_string()), None);
     assert_eq!(second_item.len(), 1);
-    assert_eq!(second_item[0], "fookey".to_string());
+    assert_eq!(
+        second_item[0],
+        ("fookey".to_string(), "fooaddr".to_string())
+    );
 }
 
 #[test]
@@ -1214,32 +1218,16 @@ fn test_instantiate_with_items() {
         }],
         initial_items: Some(vec![
             InitialItem {
-                name: "item0".to_string(),
-                info: InitialItemInfo::Instantiate {
-                    info: ModuleInstantiateInfo {
-                        code_id: voting_id,
-                        msg: to_binary(&voting_instantiate).unwrap(),
-                        admin: Admin::CoreContract {},
-                        label: "item0: a voting module".to_string(),
-                    },
-                },
+                key: "item0".to_string(),
+                value: "item0_value".to_string(),
             },
             InitialItem {
-                name: "item1".to_string(),
-                info: InitialItemInfo::Instantiate {
-                    info: ModuleInstantiateInfo {
-                        code_id: voting_id,
-                        msg: to_binary(&voting_instantiate).unwrap(),
-                        admin: Admin::CoreContract {},
-                        label: "item1: another voting module".to_string(),
-                    },
-                },
+                key: "item1".to_string(),
+                value: "item1_value".to_string(),
             },
             InitialItem {
-                name: "item2".to_string(),
-                info: InitialItemInfo::Existing {
-                    address: "item2_addr".to_string(),
-                },
+                key: "item0".to_string(),
+                value: "item0_value_override".to_string(),
             },
         ]),
     };
@@ -1255,24 +1243,23 @@ fn test_instantiate_with_items() {
         )
         .unwrap();
 
-    // Ensure initial items were added.
+    // Ensure initial items were added. One was overriden.
     let items = list_items(&mut app, gov_addr.clone(), None, None);
-    assert_eq!(items.len(), 3);
+    assert_eq!(items.len(), 2);
 
-    // Descending order, so item2 is first.
-    assert_eq!(items[0], "item2".to_string());
-    let get_item2 = get_item(&mut app, gov_addr.clone(), "item2".to_string());
+    // Descending order, so item1 is first.
+    assert_eq!(items[1].0, "item0".to_string());
+    let get_item0 = get_item(&mut app, gov_addr.clone(), "item0".to_string());
     assert_eq!(
-        get_item2,
+        get_item0,
         GetItemResponse {
-            item: Some(Addr::unchecked("item2_addr")),
+            item: Some("item0_value_override".to_string()),
         }
     );
 
-    assert_eq!(items[1], "item1".to_string());
-    get_item(&mut app, gov_addr, "item1".to_string())
-        .item
-        .unwrap();
+    assert_eq!(items[0].0, "item1".to_string());
+    let item1_value = get_item(&mut app, gov_addr, "item1".to_string()).item;
+    assert_eq!(item1_value, Some("item1_value".to_string()))
 }
 
 #[test]
diff --git a/contracts/cw4-voting/src/tests.rs b/contracts/cw4-voting/src/tests.rs
index 4a3d6541..6b07dfb0 100644
--- a/contracts/cw4-voting/src/tests.rs
+++ b/contracts/cw4-voting/src/tests.rs
@@ -541,3 +541,47 @@ fn test_migrate() {
 
     assert_eq!(new_power, power)
 }
+
+#[test]
+fn test_duplicate_member() {
+    let mut app = App::default();
+    let _voting_addr = setup_test_case(&mut app);
+    let voting_id = app.store_code(voting_contract());
+    let cw4_id = app.store_code(cw4_contract());
+    // Instantiate with members but have a duplicate
+    // Total weight is actually 69 but ADDR3 appears twice.
+    let msg = InstantiateMsg {
+        cw4_group_code_id: cw4_id,
+        initial_members: vec![
+            cw4::Member {
+                addr: ADDR3.to_string(), // same address above
+                weight: 19,
+            },
+            cw4::Member {
+                addr: ADDR1.to_string(),
+                weight: 25,
+            },
+            cw4::Member {
+                addr: ADDR2.to_string(),
+                weight: 25,
+            },
+            cw4::Member {
+                addr: ADDR3.to_string(),
+                weight: 19,
+            },
+        ],
+    };
+    // Previous versions voting power was 100, due to no dedup.
+    // Now we error
+    // Bug busted : )
+    let _voting_addr = app
+        .instantiate_contract(
+            voting_id,
+            Addr::unchecked(DAO_ADDR),
+            &msg,
+            &[],
+            "voting module",
+            None,
+        )
+        .unwrap_err();
+}