From 815717d8d2dc7e6873b3509750b09be39b82b20c Mon Sep 17 00:00:00 2001 From: Noah Saso Date: Wed, 30 Oct 2024 21:56:06 -0400 Subject: [PATCH] make cw721 claims paginatable with backwards compatibility --- Cargo.lock | 20 +- Cargo.toml | 3 + .../src/tests/dao_voting_cw721_staked_test.rs | 82 +---- .../voting/dao-voting-cw721-roles/Cargo.toml | 1 - .../voting/dao-voting-cw721-staked/Cargo.toml | 1 + .../schema/dao-voting-cw721-staked.json | 27 +- .../dao-voting-cw721-staked/src/contract.rs | 94 ++++-- .../dao-voting-cw721-staked/src/error.rs | 6 +- .../voting/dao-voting-cw721-staked/src/msg.rs | 13 +- .../dao-voting-cw721-staked/src/state.rs | 11 +- .../src/testing/adversarial.rs | 42 +-- .../src/testing/execute.rs | 4 +- .../src/testing/queries.rs | 2 + .../src/testing/tests.rs | 23 -- .../voting/dao-voting-onft-staked/Cargo.toml | 1 + .../schema/dao-voting-onft-staked.json | 27 +- .../dao-voting-onft-staked/src/contract.rs | 94 ++++-- .../dao-voting-onft-staked/src/error.rs | 6 +- .../voting/dao-voting-onft-staked/src/msg.rs | 13 +- .../dao-voting-onft-staked/src/state.rs | 11 +- .../src/testing/execute.rs | 4 +- .../src/testing/queries.rs | 2 + .../src/testing/tests.rs | 53 --- packages/cw721-controllers/src/lib.rs | 2 +- packages/cw721-controllers/src/nft_claim.rs | 312 +++++++++++++----- 25 files changed, 512 insertions(+), 342 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 600502734..b7842abd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1621,6 +1621,19 @@ dependencies = [ "thiserror", ] +[[package]] +name = "cw721-controllers" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8e48a8280ab1863169ce8caa395c72e7cbb0a4638323ddafbd3777da002fa89" +dependencies = [ + "cosmwasm-schema", + "cosmwasm-std", + "cw-storage-plus 1.2.0", + "cw-utils 1.0.3", + "thiserror", +] + [[package]] name = "cw721-controllers" version = "2.5.1" @@ -2472,7 +2485,6 @@ dependencies = [ "cw4 1.1.2", "cw721 0.18.0", "cw721-base 0.18.0", - "cw721-controllers", "cw721-roles", "dao-cw721-extensions", "dao-dao-macros 2.5.1", @@ -2496,7 +2508,8 @@ dependencies = [ "cw2 1.1.2", "cw721 0.18.0", "cw721-base 0.18.0", - "cw721-controllers", + "cw721-controllers 2.5.0", + "cw721-controllers 2.5.1", "dao-dao-macros 2.5.1", "dao-hooks 2.5.1", "dao-interface 2.5.1", @@ -2524,7 +2537,8 @@ dependencies = [ "cw-storage-plus 1.2.0", "cw-utils 1.0.3", "cw2 1.1.2", - "cw721-controllers", + "cw721-controllers 2.5.0", + "cw721-controllers 2.5.1", "dao-dao-macros 2.5.1", "dao-hooks 2.5.1", "dao-interface 2.5.1", diff --git a/Cargo.toml b/Cargo.toml index cc005ca1e..78910ae61 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -158,3 +158,6 @@ dao-proposal-multiple-v241 = { package = "dao-proposal-multiple", version = "=2. dao-proposal-single-v241 = { package = "dao-proposal-single", version = "=2.4.1" } dao-voting-cw4-v241 = { package = "dao-voting-cw4", version = "=2.4.1" } dao-voting-v241 = { package = "dao-voting", version = "=2.4.1" } + +# v2.5.0 dependencies. +cw721-controllers-v250 = { package = "cw721-controllers", version = "=2.5.0" } diff --git a/ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs b/ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs index dd13c47fb..c49cca134 100644 --- a/ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs +++ b/ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs @@ -1,4 +1,4 @@ -use cosm_orc::orchestrator::{ExecReq, SigningKey}; +use cosm_orc::orchestrator::SigningKey; use cosmwasm_std::{Binary, Empty, Uint128}; use cw_utils::Duration; use test_context::test_context; @@ -126,7 +126,9 @@ pub fn claim_nfts(chain: &mut Chain, sender: &SigningKey) { .execute( CONTRACT_NAME, "claim_nfts", - &module::msg::ExecuteMsg::ClaimNfts {}, + &module::msg::ExecuteMsg::ClaimNfts { + token_ids: Some(vec![]), + }, sender, vec![], ) @@ -189,79 +191,3 @@ fn cw721_stake_tokens(chain: &mut Chain) { let voting_power = query_voting_power(chain, &user_addr, None); assert_eq!(voting_power, Uint128::zero()); } - -#[test_context(Chain)] -#[test] -#[ignore] -fn cw721_stake_max_claims_works(chain: &mut Chain) { - use module::state::MAX_CLAIMS; - - let user_addr = chain.users["user1"].account.address.clone(); - let user_key = chain.users["user1"].key.clone(); - - let CommonTest { module, .. } = - setup_test(chain, Some(Duration::Height(1)), &user_key, &user_addr); - - // Create `MAX_CLAIMS` claims. - - // batch_size * 3 = the number of msgs to be batched per tx. - // We cant batch all of the msgs under a single tx because we hit MAX_BLOCK_GAS limits. - let batch_size = 10; - let mut total_msgs = 0; - - let mut reqs = vec![]; - for i in 0..MAX_CLAIMS { - let token_id = i.to_string(); - - reqs.push(ExecReq { - contract_name: CW721_NAME.to_string(), - msg: Box::new(cw721_base::ExecuteMsg::Mint:: { - token_id: token_id.clone(), - owner: user_addr.to_string(), - token_uri: None, - extension: Empty::default(), - }), - funds: vec![], - }); - - reqs.push(ExecReq { - contract_name: CW721_NAME.to_string(), - msg: Box::new(cw721::Cw721ExecuteMsg::SendNft { - contract: module.to_string(), - token_id: token_id.clone(), - msg: Binary::default(), - }), - funds: vec![], - }); - - reqs.push(ExecReq { - contract_name: CONTRACT_NAME.to_string(), - msg: Box::new(module::msg::ExecuteMsg::Unstake { - token_ids: vec![token_id], - }), - funds: vec![], - }); - - if (i != 0 && i % batch_size == 0) || i == MAX_CLAIMS - 1 { - total_msgs += reqs.len(); - - chain - .orc - .execute_batch("batch_cw721_stake_max_claims", reqs, &user_key) - .unwrap(); - - reqs = vec![]; - } - } - - assert_eq!(total_msgs as u64, MAX_CLAIMS * 3); - - chain - .orc - .poll_for_n_blocks(1, core::time::Duration::from_millis(20_000), false) - .unwrap(); - - // If this works, we're golden. Other tests make sure that the - // NFTs get returned as a result of this. - claim_nfts(chain, &user_key); -} diff --git a/contracts/voting/dao-voting-cw721-roles/Cargo.toml b/contracts/voting/dao-voting-cw721-roles/Cargo.toml index e816d763c..7fbf4360a 100644 --- a/contracts/voting/dao-voting-cw721-roles/Cargo.toml +++ b/contracts/voting/dao-voting-cw721-roles/Cargo.toml @@ -22,7 +22,6 @@ dao-cw721-extensions = { workspace = true } dao-dao-macros = { workspace = true } dao-interface = { workspace = true } cw721-base = { workspace = true, features = ["library"] } -cw721-controllers = { workspace = true } cw-ownable = { workspace = true } cw721 = { workspace = true } cw-utils = { workspace = true } diff --git a/contracts/voting/dao-voting-cw721-staked/Cargo.toml b/contracts/voting/dao-voting-cw721-staked/Cargo.toml index af7d5532e..f63e4dc12 100644 --- a/contracts/voting/dao-voting-cw721-staked/Cargo.toml +++ b/contracts/voting/dao-voting-cw721-staked/Cargo.toml @@ -31,6 +31,7 @@ cw-hooks = { workspace = true } cw721 = { workspace = true } cw721-base = { workspace = true, features = ["library"] } cw721-controllers = { workspace = true } +cw721-controllers-v250 = { workspace = true } cw-utils = { workspace = true } cw2 = { workspace = true } dao-dao-macros = { workspace = true } diff --git a/contracts/voting/dao-voting-cw721-staked/schema/dao-voting-cw721-staked.json b/contracts/voting/dao-voting-cw721-staked/schema/dao-voting-cw721-staked.json index 346476689..a6f5473c8 100644 --- a/contracts/voting/dao-voting-cw721-staked/schema/dao-voting-cw721-staked.json +++ b/contracts/voting/dao-voting-cw721-staked/schema/dao-voting-cw721-staked.json @@ -265,7 +265,7 @@ "additionalProperties": false }, { - "description": "Claim NFTs that have been unstaked for the specified duration.", + "description": "Claim NFTs that have been unstaked for the specified duration. If none are provided, it attempts to claim all legacy claims. If token IDs are provided, only those are claimed. If an empty vector is provided, it attempts to claim all non-legacy claims.", "type": "object", "required": [ "claim_nfts" @@ -273,6 +273,17 @@ "properties": { "claim_nfts": { "type": "object", + "properties": { + "token_ids": { + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + } + }, "additionalProperties": false } }, @@ -525,6 +536,20 @@ "properties": { "address": { "type": "string" + }, + "limit": { + "type": [ + "integer", + "null" + ], + "format": "uint32", + "minimum": 0.0 + }, + "start_after": { + "type": [ + "string", + "null" + ] } }, "additionalProperties": false diff --git a/contracts/voting/dao-voting-cw721-staked/src/contract.rs b/contracts/voting/dao-voting-cw721-staked/src/contract.rs index fc83cce13..ff8444cbe 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/contract.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/contract.rs @@ -20,7 +20,8 @@ use dao_voting::threshold::{ use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, NftContract, QueryMsg}; use crate::state::{ register_staked_nft, register_unstaked_nfts, Config, ACTIVE_THRESHOLD, CONFIG, DAO, HOOKS, - INITIAL_NFTS, MAX_CLAIMS, NFT_BALANCES, NFT_CLAIMS, STAKED_NFTS_PER_OWNER, TOTAL_STAKED_NFTS, + INITIAL_NFTS, LEGACY_NFT_CLAIMS, NFT_BALANCES, NFT_CLAIMS, STAKED_NFTS_PER_OWNER, + TOTAL_STAKED_NFTS, }; use crate::ContractError; @@ -204,7 +205,7 @@ pub fn execute( match msg { ExecuteMsg::ReceiveNft(msg) => execute_stake(deps, env, info, msg), ExecuteMsg::Unstake { token_ids } => execute_unstake(deps, env, info, token_ids), - ExecuteMsg::ClaimNfts {} => execute_claim_nfts(deps, env, info), + ExecuteMsg::ClaimNfts { token_ids } => execute_claim_nfts(deps, env, info, token_ids), ExecuteMsg::UpdateConfig { duration } => execute_update_config(info, deps, duration), ExecuteMsg::AddHook { addr } => execute_add_hook(deps, info, addr), ExecuteMsg::RemoveHook { addr } => execute_remove_hook(deps, info, addr), @@ -314,13 +315,6 @@ pub fn execute_unstake( } Some(duration) => { - let outstanding_claims = NFT_CLAIMS - .query_claims(deps.as_ref(), &info.sender)? - .nft_claims; - if outstanding_claims.len() + token_ids.len() > MAX_CLAIMS as usize { - return Err(ContractError::TooManyClaims {}); - } - // Out of gas here is fine - just try again with fewer // tokens. NFT_CLAIMS.create_nft_claims( @@ -343,22 +337,57 @@ pub fn execute_claim_nfts( deps: DepsMut, env: Env, info: MessageInfo, + token_ids: Option>, ) -> Result { - let nfts = NFT_CLAIMS.claim_nfts(deps.storage, &info.sender, &env.block)?; - if nfts.is_empty() { - return Err(ContractError::NothingToClaim {}); - } + let token_ids = match token_ids { + // attempt to claim all legacy NFTs + None => { + // claim all legacy NFTs + let legacy_nfts = + LEGACY_NFT_CLAIMS.claim_nfts(deps.storage, &info.sender, &env.block)?; + + if legacy_nfts.is_empty() { + return Err(ContractError::NothingToClaim {}); + } + + legacy_nfts + } + // attempt to claim non-legacy NFTs + Some(token_ids) => { + let token_ids = if token_ids.is_empty() { + // query all NFT claims if none are provided + NFT_CLAIMS + .query_claims(deps.as_ref(), &info.sender, None, None)? + .nft_claims + .into_iter() + .filter(|nft| nft.release_at.is_expired(&env.block)) + .map(|nft| nft.token_id) + .collect::>() + } else { + // use provided NFTs if any + token_ids + }; + + if token_ids.is_empty() { + return Err(ContractError::NothingToClaim {}); + } + + NFT_CLAIMS.claim_nfts(deps.storage, &info.sender, &token_ids, &env.block)?; + + token_ids + } + }; let config = CONFIG.load(deps.storage)?; - let msgs = nfts + let msgs = token_ids .into_iter() - .map(|nft| -> StdResult { + .map(|token_id| -> StdResult { Ok(WasmMsg::Execute { contract_addr: config.nft_address.to_string(), msg: to_json_binary(&cw721::Cw721ExecuteMsg::TransferNft { recipient: info.sender.to_string(), - token_id: nft, + token_id, })?, funds: vec![], } @@ -485,7 +514,11 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult { QueryMsg::Dao {} => query_dao(deps), QueryMsg::Info {} => query_info(deps), QueryMsg::IsActive {} => query_is_active(deps, env), - QueryMsg::NftClaims { address } => query_nft_claims(deps, address), + QueryMsg::NftClaims { + address, + start_after, + limit, + } => query_nft_claims(deps, address, start_after, limit), QueryMsg::Hooks {} => query_hooks(deps), QueryMsg::StakedNfts { address, @@ -606,8 +639,31 @@ pub fn query_dao(deps: Deps) -> StdResult { to_json_binary(&dao) } -pub fn query_nft_claims(deps: Deps, address: String) -> StdResult { - to_json_binary(&NFT_CLAIMS.query_claims(deps, &deps.api.addr_validate(&address)?)?) +pub fn query_nft_claims( + deps: Deps, + address: String, + start_after: Option, + limit: Option, +) -> StdResult { + let addr = deps.api.addr_validate(&address)?; + + // load all legacy claims since it does not support pagination + let legacy_claims = LEGACY_NFT_CLAIMS + .query_claims(deps, &addr)? + .nft_claims + .into_iter() + .map(|c| cw721_controllers::NftClaim::new(c.token_id, c.release_at)) + .collect::>(); + + // paginate all new claims + let claims = NFT_CLAIMS + .query_claims(deps, &addr, start_after.as_ref(), limit)? + .nft_claims; + + // combine legacy and new claims + let nft_claims = legacy_claims.into_iter().chain(claims).collect(); + + to_json_binary(&cw721_controllers::NftClaimsResponse { nft_claims }) } pub fn query_hooks(deps: Deps) -> StdResult { diff --git a/contracts/voting/dao-voting-cw721-staked/src/error.rs b/contracts/voting/dao-voting-cw721-staked/src/error.rs index df9526bd2..97f41436b 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/error.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/error.rs @@ -20,6 +20,9 @@ pub enum ContractError { #[error(transparent)] UnstakingDurationError(#[from] dao_voting::duration::UnstakingDurationError), + #[error(transparent)] + NftClaimError(#[from] cw721_controllers::NftClaimError), + #[error("Can not stake that which has already been staked")] AlreadyStaked {}, @@ -44,9 +47,6 @@ pub enum ContractError { #[error("Can not unstake that which you have not staked (unstaking {token_id})")] NotStaked { token_id: String }, - #[error("Too many outstanding claims. Claim some tokens before unstaking more.")] - TooManyClaims {}, - #[error("Unauthorized")] Unauthorized {}, diff --git a/contracts/voting/dao-voting-cw721-staked/src/msg.rs b/contracts/voting/dao-voting-cw721-staked/src/msg.rs index 837851ed3..8643d6ddc 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/msg.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/msg.rs @@ -54,8 +54,11 @@ pub enum ExecuteMsg { /// sender. token_ids must have unique values and have non-zero /// length. Unstake { token_ids: Vec }, - /// Claim NFTs that have been unstaked for the specified duration. - ClaimNfts {}, + /// Claim NFTs that have been unstaked for the specified duration. If none + /// are provided, it attempts to claim all legacy claims. If token IDs are + /// provided, only those are claimed. If an empty vector is provided, it + /// attempts to claim all non-legacy claims. + ClaimNfts { token_ids: Option> }, /// Updates the contract configuration, namely unstaking duration. /// Only callable by the DAO that initialized this voting contract. UpdateConfig { duration: Option }, @@ -80,7 +83,11 @@ pub enum QueryMsg { #[returns(crate::state::Config)] Config {}, #[returns(::cw721_controllers::NftClaimsResponse)] - NftClaims { address: String }, + NftClaims { + address: String, + start_after: Option, + limit: Option, + }, #[returns(::cw_controllers::HooksResponse)] Hooks {}, // List the staked NFTs for a given address. diff --git a/contracts/voting/dao-voting-cw721-staked/src/state.rs b/contracts/voting/dao-voting-cw721-staked/src/state.rs index 0d8e8b62f..99341499a 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/state.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/state.rs @@ -42,9 +42,14 @@ pub const TOTAL_STAKED_NFTS: SnapshotItem = SnapshotItem::new( Strategy::EveryBlock, ); -/// The maximum number of claims that may be outstanding. -pub const MAX_CLAIMS: u64 = 70; -pub const NFT_CLAIMS: NftClaims = NftClaims::new("nft_claims"); +/// The legacy NFT claims storage uses a non-paginatable vector, which limits +/// the number of claims that may be outstanding. This is horrible UX, +/// especially for large NFT collections. To allow DAOs to upgrade, we must keep +/// the legacy NFT claims storage, but we can paginate the new storage. +pub const LEGACY_NFT_CLAIMS: cw721_controllers_v250::NftClaims = + cw721_controllers_v250::NftClaims::new("nft_claims"); + +pub const NFT_CLAIMS: NftClaims = NftClaims::new("nc"); // Hooks to contracts that will receive staking and unstaking // messages. diff --git a/contracts/voting/dao-voting-cw721-staked/src/testing/adversarial.rs b/contracts/voting/dao-voting-cw721-staked/src/testing/adversarial.rs index e5c587564..bc81a2119 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/testing/adversarial.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/testing/adversarial.rs @@ -1,14 +1,10 @@ use cosmwasm_std::Uint128; use cw_multi_test::next_block; -use cw_utils::Duration; - -use crate::{ - state::MAX_CLAIMS, - testing::{ - execute::{stake_nft, unstake_nfts}, - instantiate::instantiate_cw721_base, - queries::query_voting_power, - }, + +use crate::testing::{ + execute::{stake_nft, unstake_nfts}, + instantiate::instantiate_cw721_base, + queries::query_voting_power, }; use super::{ @@ -145,31 +141,3 @@ fn test_query_the_future() -> anyhow::Result<()> { Ok(()) } - -/// I can not unstake more than one NFT in a TX in order to bypass the -/// MAX_CLAIMS limit. -#[test] -fn test_bypass_max_claims() -> anyhow::Result<()> { - let CommonTest { - mut app, - module, - nft, - } = setup_test(Some(Duration::Height(1))); - let mut to_stake = vec![]; - for i in 1..(MAX_CLAIMS + 10) { - let i_str = &i.to_string(); - mint_and_stake_nft(&mut app, &nft, &module, CREATOR_ADDR, i_str)?; - if i < MAX_CLAIMS { - // unstake MAX_CLAMS - 1 NFTs - unstake_nfts(&mut app, &module, CREATOR_ADDR, &[i_str])?; - } else { - // push rest of NFT ids to vec - to_stake.push(i_str.clone()); - } - } - let binding = to_stake.iter().map(|s| s.as_str()).collect::>(); - let to_stake_slice: &[&str] = binding.as_slice(); - let res = unstake_nfts(&mut app, &module, CREATOR_ADDR, to_stake_slice); - is_error!(res => "Too many outstanding claims. Claim some tokens before unstaking more."); - Ok(()) -} diff --git a/contracts/voting/dao-voting-cw721-staked/src/testing/execute.rs b/contracts/voting/dao-voting-cw721-staked/src/testing/execute.rs index dd5b60877..08a1b397a 100644 --- a/contracts/voting/dao-voting-cw721-staked/src/testing/execute.rs +++ b/contracts/voting/dao-voting-cw721-staked/src/testing/execute.rs @@ -110,7 +110,9 @@ pub fn claim_nfts(app: &mut App, module: &Addr, sender: &str) -> AnyResult StdResult anyhow::Result<()> { Ok(()) } -// I can not have more than MAX_CLAIMS claims pending. -#[test] -fn test_max_claims() -> anyhow::Result<()> { - let CommonTest { - mut app, - module, - nft, - } = setup_test(Some(Duration::Height(1))); - - for i in 0..MAX_CLAIMS { - let i_str = &i.to_string(); - mint_and_stake_nft(&mut app, &nft, &module, CREATOR_ADDR, i_str)?; - unstake_nfts(&mut app, &module, CREATOR_ADDR, &[i_str])?; - } - - mint_and_stake_nft(&mut app, &nft, &module, CREATOR_ADDR, "a")?; - let res = unstake_nfts(&mut app, &module, CREATOR_ADDR, &["a"]); - is_error!(res => "Too many outstanding claims. Claim some tokens before unstaking more."); - - Ok(()) -} - // I can list all of the currently staked NFTs for an address. #[test] fn test_list_staked_nfts() -> anyhow::Result<()> { diff --git a/contracts/voting/dao-voting-onft-staked/Cargo.toml b/contracts/voting/dao-voting-onft-staked/Cargo.toml index 9c96ec2d7..eaf1f53fa 100644 --- a/contracts/voting/dao-voting-onft-staked/Cargo.toml +++ b/contracts/voting/dao-voting-onft-staked/Cargo.toml @@ -33,6 +33,7 @@ cw-storage-plus = { workspace = true } cw-controllers = { workspace = true } cw-hooks = { workspace = true } cw721-controllers = { workspace = true } +cw721-controllers-v250 = { workspace = true } cw-utils = { workspace = true } cw2 = { workspace = true } dao-dao-macros = { workspace = true } diff --git a/contracts/voting/dao-voting-onft-staked/schema/dao-voting-onft-staked.json b/contracts/voting/dao-voting-onft-staked/schema/dao-voting-onft-staked.json index ede91d3be..76958af84 100644 --- a/contracts/voting/dao-voting-onft-staked/schema/dao-voting-onft-staked.json +++ b/contracts/voting/dao-voting-onft-staked/schema/dao-voting-onft-staked.json @@ -274,7 +274,7 @@ "additionalProperties": false }, { - "description": "Claim NFTs that have been unstaked for the specified duration.", + "description": "Claim NFTs that have been unstaked for the specified duration. If none are provided, it attempts to claim all legacy claims. If token IDs are provided, only those are claimed. If an empty vector is provided, it attempts to claim all non-legacy claims.", "type": "object", "required": [ "claim_nfts" @@ -282,6 +282,17 @@ "properties": { "claim_nfts": { "type": "object", + "properties": { + "token_ids": { + "type": [ + "array", + "null" + ], + "items": { + "type": "string" + } + } + }, "additionalProperties": false } }, @@ -509,6 +520,20 @@ "properties": { "address": { "type": "string" + }, + "limit": { + "type": [ + "integer", + "null" + ], + "format": "uint32", + "minimum": 0.0 + }, + "start_after": { + "type": [ + "string", + "null" + ] } }, "additionalProperties": false diff --git a/contracts/voting/dao-voting-onft-staked/src/contract.rs b/contracts/voting/dao-voting-onft-staked/src/contract.rs index 2ab1ebd73..01efa76dd 100644 --- a/contracts/voting/dao-voting-onft-staked/src/contract.rs +++ b/contracts/voting/dao-voting-onft-staked/src/contract.rs @@ -19,7 +19,8 @@ use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, OnftCollection, QueryMs use crate::omniflix::{get_onft_transfer_msg, query_onft_owner, query_onft_supply}; use crate::state::{ register_staked_nfts, register_unstaked_nfts, Config, ACTIVE_THRESHOLD, CONFIG, DAO, HOOKS, - MAX_CLAIMS, NFT_BALANCES, NFT_CLAIMS, PREPARED_ONFTS, STAKED_NFTS_PER_OWNER, TOTAL_STAKED_NFTS, + LEGACY_NFT_CLAIMS, NFT_BALANCES, NFT_CLAIMS, PREPARED_ONFTS, STAKED_NFTS_PER_OWNER, + TOTAL_STAKED_NFTS, }; use crate::ContractError; @@ -97,7 +98,7 @@ pub fn execute( recipient, } => execute_cancel_stake(deps, env, info, token_ids, recipient), ExecuteMsg::Unstake { token_ids } => execute_unstake(deps, env, info, token_ids), - ExecuteMsg::ClaimNfts {} => execute_claim_nfts(deps, env, info), + ExecuteMsg::ClaimNfts { token_ids } => execute_claim_nfts(deps, env, info, token_ids), ExecuteMsg::UpdateConfig { duration } => execute_update_config(info, deps, duration), ExecuteMsg::AddHook { addr } => execute_add_hook(deps, info, addr), ExecuteMsg::RemoveHook { addr } => execute_remove_hook(deps, info, addr), @@ -371,13 +372,6 @@ pub fn execute_unstake( } Some(duration) => { - let outstanding_claims = NFT_CLAIMS - .query_claims(deps.as_ref(), &info.sender)? - .nft_claims; - if outstanding_claims.len() + token_ids.len() > MAX_CLAIMS as usize { - return Err(ContractError::TooManyClaims {}); - } - // Out of gas here is fine - just try again with fewer // tokens. NFT_CLAIMS.create_nft_claims( @@ -400,20 +394,55 @@ pub fn execute_claim_nfts( deps: DepsMut, env: Env, info: MessageInfo, + token_ids: Option>, ) -> Result { - let nfts = NFT_CLAIMS.claim_nfts(deps.storage, &info.sender, &env.block)?; - if nfts.is_empty() { - return Err(ContractError::NothingToClaim {}); - } + let token_ids = match token_ids { + // attempt to claim all legacy NFTs + None => { + // claim all legacy NFTs + let legacy_nfts = + LEGACY_NFT_CLAIMS.claim_nfts(deps.storage, &info.sender, &env.block)?; + + if legacy_nfts.is_empty() { + return Err(ContractError::NothingToClaim {}); + } + + legacy_nfts + } + // attempt to claim non-legacy NFTs + Some(token_ids) => { + let token_ids = if token_ids.is_empty() { + // query all NFT claims if none are provided + NFT_CLAIMS + .query_claims(deps.as_ref(), &info.sender, None, None)? + .nft_claims + .into_iter() + .filter(|nft| nft.release_at.is_expired(&env.block)) + .map(|nft| nft.token_id) + .collect::>() + } else { + // use provided NFTs if any + token_ids + }; + + if token_ids.is_empty() { + return Err(ContractError::NothingToClaim {}); + } + + NFT_CLAIMS.claim_nfts(deps.storage, &info.sender, &token_ids, &env.block)?; + + token_ids + } + }; let config = CONFIG.load(deps.storage)?; - let msgs = nfts + let msgs = token_ids .into_iter() - .map(|nft| -> CosmosMsg { + .map(|token_id| -> CosmosMsg { get_onft_transfer_msg( &config.onft_collection_id, - &nft, + &token_id, env.contract.address.as_str(), info.sender.as_str(), ) @@ -534,7 +563,11 @@ pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult { QueryMsg::Dao {} => query_dao(deps), QueryMsg::Info {} => query_info(deps), QueryMsg::IsActive {} => query_is_active(deps, env), - QueryMsg::NftClaims { address } => query_nft_claims(deps, address), + QueryMsg::NftClaims { + address, + start_after, + limit, + } => query_nft_claims(deps, address, start_after, limit), QueryMsg::Hooks {} => query_hooks(deps), QueryMsg::StakedNfts { address, @@ -652,8 +685,31 @@ pub fn query_dao(deps: Deps) -> StdResult { to_json_binary(&dao) } -pub fn query_nft_claims(deps: Deps, address: String) -> StdResult { - to_json_binary(&NFT_CLAIMS.query_claims(deps, &deps.api.addr_validate(&address)?)?) +pub fn query_nft_claims( + deps: Deps, + address: String, + start_after: Option, + limit: Option, +) -> StdResult { + let addr = deps.api.addr_validate(&address)?; + + // load all legacy claims since it does not support pagination + let legacy_claims = LEGACY_NFT_CLAIMS + .query_claims(deps, &addr)? + .nft_claims + .into_iter() + .map(|c| cw721_controllers::NftClaim::new(c.token_id, c.release_at)) + .collect::>(); + + // paginate all new claims + let claims = NFT_CLAIMS + .query_claims(deps, &addr, start_after.as_ref(), limit)? + .nft_claims; + + // combine legacy and new claims + let nft_claims = legacy_claims.into_iter().chain(claims).collect(); + + to_json_binary(&cw721_controllers::NftClaimsResponse { nft_claims }) } pub fn query_hooks(deps: Deps) -> StdResult { diff --git a/contracts/voting/dao-voting-onft-staked/src/error.rs b/contracts/voting/dao-voting-onft-staked/src/error.rs index 252e8caf4..a57d785b5 100644 --- a/contracts/voting/dao-voting-onft-staked/src/error.rs +++ b/contracts/voting/dao-voting-onft-staked/src/error.rs @@ -16,6 +16,9 @@ pub enum ContractError { #[error(transparent)] UnstakingDurationError(#[from] dao_voting::duration::UnstakingDurationError), + #[error(transparent)] + NftClaimError(#[from] cw721_controllers::NftClaimError), + #[error("Nothing to claim")] NothingToClaim {}, @@ -34,9 +37,6 @@ pub enum ContractError { #[error("Can not unstake that which you have not staked (unstaking {token_id})")] NotStaked { token_id: String }, - #[error("Too many outstanding claims. Claim some tokens before unstaking more.")] - TooManyClaims {}, - #[error("Unauthorized")] Unauthorized {}, diff --git a/contracts/voting/dao-voting-onft-staked/src/msg.rs b/contracts/voting/dao-voting-onft-staked/src/msg.rs index 08e589bc5..0567aa93d 100644 --- a/contracts/voting/dao-voting-onft-staked/src/msg.rs +++ b/contracts/voting/dao-voting-onft-staked/src/msg.rs @@ -75,8 +75,11 @@ pub enum ExecuteMsg { /// Unstakes the specified token_ids on behalf of the sender. token_ids must /// have unique values and have non-zero length. Unstake { token_ids: Vec }, - /// Claim NFTs that have been unstaked for the specified duration. - ClaimNfts {}, + /// Claim NFTs that have been unstaked for the specified duration. If none + /// are provided, it attempts to claim all legacy claims. If token IDs are + /// provided, only those are claimed. If an empty vector is provided, it + /// attempts to claim all non-legacy claims. + ClaimNfts { token_ids: Option> }, /// Updates the contract configuration, namely unstaking duration. Only /// callable by the DAO that initialized this voting contract. UpdateConfig { duration: Option }, @@ -101,7 +104,11 @@ pub enum QueryMsg { #[returns(crate::state::Config)] Config {}, #[returns(::cw721_controllers::NftClaimsResponse)] - NftClaims { address: String }, + NftClaims { + address: String, + start_after: Option, + limit: Option, + }, #[returns(::cw_controllers::HooksResponse)] Hooks {}, // List the staked NFTs for a given address. diff --git a/contracts/voting/dao-voting-onft-staked/src/state.rs b/contracts/voting/dao-voting-onft-staked/src/state.rs index 0e85822ab..0fab86f74 100644 --- a/contracts/voting/dao-voting-onft-staked/src/state.rs +++ b/contracts/voting/dao-voting-onft-staked/src/state.rs @@ -48,9 +48,14 @@ pub const TOTAL_STAKED_NFTS: SnapshotItem = SnapshotItem::new( Strategy::EveryBlock, ); -/// The maximum number of claims that may be outstanding. -pub const MAX_CLAIMS: u64 = 70; -pub const NFT_CLAIMS: NftClaims = NftClaims::new("nft_claims"); +/// The legacy NFT claims storage uses a non-paginatable vector, which limits +/// the number of claims that may be outstanding. This is horrible UX, +/// especially for large NFT collections. To allow DAOs to upgrade, we must keep +/// the legacy NFT claims storage, but we can paginate the new storage. +pub const LEGACY_NFT_CLAIMS: cw721_controllers_v250::NftClaims = + cw721_controllers_v250::NftClaims::new("nft_claims"); + +pub const NFT_CLAIMS: NftClaims = NftClaims::new("nc"); // Hooks to contracts that will receive staking and unstaking // messages. diff --git a/contracts/voting/dao-voting-onft-staked/src/testing/execute.rs b/contracts/voting/dao-voting-onft-staked/src/testing/execute.rs index fd79fd25e..bef18ff42 100644 --- a/contracts/voting/dao-voting-onft-staked/src/testing/execute.rs +++ b/contracts/voting/dao-voting-onft-staked/src/testing/execute.rs @@ -198,7 +198,9 @@ pub fn claim_nfts(app: &mut OmniflixApp, module: &Addr, sender: &str) -> AnyResu app.execute_contract( addr!(sender), module.clone(), - &ExecuteMsg::ClaimNfts {}, + &ExecuteMsg::ClaimNfts { + token_ids: Some(vec![]), + }, &[], ) } diff --git a/contracts/voting/dao-voting-onft-staked/src/testing/queries.rs b/contracts/voting/dao-voting-onft-staked/src/testing/queries.rs index 3ff1facde..5cd6cb45d 100644 --- a/contracts/voting/dao-voting-onft-staked/src/testing/queries.rs +++ b/contracts/voting/dao-voting-onft-staked/src/testing/queries.rs @@ -20,6 +20,8 @@ pub fn query_claims(app: &OmniflixApp, module: &Addr, addr: &str) -> StdResult anyhow::Result<()> { Ok(()) } -// I can not have more than MAX_CLAIMS claims pending. -#[test] -fn test_max_claims() -> anyhow::Result<()> { - let CommonTest { - mut app, - module, - nft, - .. - } = setup_test(Some(Duration::Height(1)), None); - - for i in 0..MAX_CLAIMS { - let i_str = &i.to_string(); - mint_and_stake_nft(&mut app, &nft, &module, STAKER, i_str)?; - unstake_nfts(&mut app, &module, STAKER, &[i_str])?; - } - - mint_and_stake_nft(&mut app, &nft, &module, STAKER, "a")?; - let res = unstake_nfts(&mut app, &module, STAKER, &["a"]); - is_error!(res => "Too many outstanding claims. Claim some tokens before unstaking more."); - - Ok(()) -} - // I can list all of the currently staked NFTs for an address. #[test] fn test_list_staked_nfts() -> anyhow::Result<()> { @@ -987,35 +963,6 @@ fn test_query_the_future() -> anyhow::Result<()> { Ok(()) } -/// I can not unstake more than one NFT in a TX in order to bypass the -/// MAX_CLAIMS limit. -#[test] -fn test_bypass_max_claims() -> anyhow::Result<()> { - let CommonTest { - mut app, - module, - nft, - .. - } = setup_test(Some(Duration::Height(1)), None); - let mut to_stake = vec![]; - for i in 1..(MAX_CLAIMS + 10) { - let i_str = &i.to_string(); - mint_and_stake_nft(&mut app, &nft, &module, STAKER, i_str)?; - if i < MAX_CLAIMS { - // unstake MAX_CLAMS - 1 NFTs - unstake_nfts(&mut app, &module, STAKER, &[i_str])?; - } else { - // push rest of NFT ids to vec - to_stake.push(i_str.clone()); - } - } - let binding = to_stake.iter().map(|s| s.as_str()).collect::>(); - let to_stake_slice: &[&str] = binding.as_slice(); - let res = unstake_nfts(&mut app, &module, STAKER, to_stake_slice); - is_error!(res => "Too many outstanding claims. Claim some tokens before unstaking more."); - Ok(()) -} - /// I can cancel my own prepared stake. #[test] fn test_preparer_cancel_prepared_stake() -> anyhow::Result<()> { diff --git a/packages/cw721-controllers/src/lib.rs b/packages/cw721-controllers/src/lib.rs index d4f58091a..66823d06a 100644 --- a/packages/cw721-controllers/src/lib.rs +++ b/packages/cw721-controllers/src/lib.rs @@ -2,4 +2,4 @@ mod nft_claim; -pub use nft_claim::{NftClaim, NftClaims, NftClaimsResponse}; +pub use nft_claim::{NftClaim, NftClaimError, NftClaims, NftClaimsResponse}; diff --git a/packages/cw721-controllers/src/nft_claim.rs b/packages/cw721-controllers/src/nft_claim.rs index 4b8702f9a..f2fe60b9e 100644 --- a/packages/cw721-controllers/src/nft_claim.rs +++ b/packages/cw721-controllers/src/nft_claim.rs @@ -1,7 +1,20 @@ use cosmwasm_schema::cw_serde; -use cosmwasm_std::{Addr, BlockInfo, CustomQuery, Deps, StdResult, Storage}; -use cw_storage_plus::Map; +use cosmwasm_std::{Addr, BlockInfo, CustomQuery, Deps, Order, StdError, StdResult, Storage}; +use cw_storage_plus::{Bound, Map}; use cw_utils::Expiration; +use thiserror::Error; + +#[derive(Error, Debug, PartialEq)] +pub enum NftClaimError { + #[error(transparent)] + Std(#[from] StdError), + + #[error("NFT claim not found for {token_id}")] + NotFound { token_id: String }, + + #[error("One or more NFTs is not ready to be claimed")] + NotReady {}, +} #[cw_serde] pub struct NftClaimsResponse { @@ -15,22 +28,22 @@ pub struct NftClaim { } impl NftClaim { - pub fn new(token_id: String, released: Expiration) -> Self { + pub fn new(token_id: String, release_at: Expiration) -> Self { NftClaim { token_id, - release_at: released, + release_at, } } } -pub struct NftClaims<'a>(Map<'a, &'a Addr, Vec>); +pub struct NftClaims<'a>(Map<'a, (&'a Addr, &'a String), Expiration>); impl<'a> NftClaims<'a> { pub const fn new(storage_key: &'a str) -> Self { NftClaims(Map::new(storage_key)) } - /// Creates a number of NFT claims simeltaniously for a given + /// Creates a number of NFT claims simultaneously for a given /// address. /// /// # Invariants @@ -46,51 +59,70 @@ impl<'a> NftClaims<'a> { token_ids: Vec, release_at: Expiration, ) -> StdResult<()> { - self.0.update(storage, addr, |old| -> StdResult<_> { - Ok(old - .unwrap_or_default() - .into_iter() - .chain(token_ids.into_iter().map(|token_id| NftClaim { - token_id, - release_at, - })) - .collect::>()) - })?; + token_ids + .into_iter() + .map(|token_id| self.0.save(storage, (addr, &token_id), &release_at)) + .collect::>>()?; Ok(()) } - /// This iterates over all mature claims for the address, and removes them, up to an optional cap. - /// it removes the finished claims and returns the total amount of tokens to be released. + /// This iterates over all claims for the given IDs, removing them if they + /// are all mature and erroring if any is not. pub fn claim_nfts( &self, storage: &mut dyn Storage, addr: &Addr, + token_ids: &[String], block: &BlockInfo, - ) -> StdResult> { - let mut to_send = vec![]; - self.0.update(storage, addr, |nft_claims| -> StdResult<_> { - let (_send, waiting): (Vec<_>, _) = - nft_claims.unwrap_or_default().into_iter().partition(|c| { - // if mature and we can pay fully, then include in _send - if c.release_at.is_expired(block) { - to_send.push(c.token_id.clone()); - true - } else { - // not to send, leave in waiting and save again - false + ) -> Result<(), NftClaimError> { + token_ids + .iter() + .map(|token_id| -> Result<(), NftClaimError> { + match self.0.may_load(storage, (addr, token_id)) { + Ok(Some(expiration)) => { + // if claim is expired, continue + if expiration.is_expired(block) { + Ok(()) + } else { + // if claim is not expired, error + Err(NftClaimError::NotReady {}) + } } - }); - Ok(waiting) - })?; - Ok(to_send) + // if claim is not found, error + Ok(None) => Err(NftClaimError::NotFound { + token_id: token_id.clone(), + }), + Err(e) => Err(e.into()), + } + }) + .collect::, NftClaimError>>()?; + + // remove all now that we've confirmed they're mature + token_ids + .iter() + .for_each(|token_id| self.0.remove(storage, (addr, token_id))); + + Ok(()) } pub fn query_claims( &self, deps: Deps, address: &Addr, + start_after: Option<&String>, + limit: Option, ) -> StdResult { - let nft_claims = self.0.may_load(deps.storage, address)?.unwrap_or_default(); + let limit = limit.map(|l| l as usize).unwrap_or(usize::MAX); + let start = start_after.map(Bound::<&String>::exclusive); + + let nft_claims = self + .0 + .prefix(address) + .range(deps.storage, start, None, Order::Ascending) + .take(limit) + .map(|item| item.map(|(token_id, v)| NftClaim::new(token_id, v))) + .collect::>>()?; + Ok(NftClaimsResponse { nft_claims }) } } @@ -147,11 +179,13 @@ mod test { // Assert that claims creates a map and there is one claim for the address. let saved_claims = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr")) + .prefix(&Addr::unchecked("addr")) + .range(deps.as_mut().storage, None, None, Order::Ascending) + .collect::>>() .unwrap(); assert_eq!(saved_claims.len(), 1); - assert_eq!(saved_claims[0].token_id, TEST_BAYC_TOKEN_ID.to_string()); - assert_eq!(saved_claims[0].release_at, TEST_EXPIRATION); + assert_eq!(saved_claims[0].0, TEST_BAYC_TOKEN_ID.to_string()); + assert_eq!(saved_claims[0].1, TEST_EXPIRATION); // Adding another claim to same address, make sure that both claims are saved. claims @@ -166,16 +200,15 @@ mod test { // Assert that both claims exist for the address. let saved_claims = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr")) + .prefix(&Addr::unchecked("addr")) + .range(deps.as_mut().storage, None, None, Order::Ascending) + .collect::>>() .unwrap(); assert_eq!(saved_claims.len(), 2); - assert_eq!(saved_claims[0].token_id, TEST_BAYC_TOKEN_ID.to_string()); - assert_eq!(saved_claims[0].release_at, TEST_EXPIRATION); - assert_eq!( - saved_claims[1].token_id, - TEST_CRYPTO_PUNKS_TOKEN_ID.to_string() - ); - assert_eq!(saved_claims[1].release_at, TEST_EXPIRATION); + assert_eq!(saved_claims[0].0, TEST_BAYC_TOKEN_ID.to_string()); + assert_eq!(saved_claims[0].1, TEST_EXPIRATION); + assert_eq!(saved_claims[1].0, TEST_CRYPTO_PUNKS_TOKEN_ID.to_string()); + assert_eq!(saved_claims[1].1, TEST_EXPIRATION); // Adding another claim to different address, make sure that other address only has one claim. claims @@ -190,12 +223,16 @@ mod test { // Assert that both claims exist for the address. let saved_claims = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr")) + .prefix(&Addr::unchecked("addr")) + .range(deps.as_mut().storage, None, None, Order::Ascending) + .collect::>>() .unwrap(); let saved_claims_addr2 = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr2")) + .prefix(&Addr::unchecked("addr2")) + .range(deps.as_mut().storage, None, None, Order::Ascending) + .collect::>>() .unwrap(); assert_eq!(saved_claims.len(), 2); assert_eq!(saved_claims_addr2.len(), 1); @@ -206,19 +243,37 @@ mod test { let mut deps = mock_dependencies(); let claims = NftClaims::new("claims"); - let nfts = claims + let env = mock_env(); + let error = claims .claim_nfts( deps.as_mut().storage, &Addr::unchecked("addr"), + &["404".to_string()], + &env.block, + ) + .unwrap_err(); + assert_eq!( + error, + NftClaimError::NotFound { + token_id: "404".to_string() + } + ); + + claims + .claim_nfts( + deps.as_mut().storage, + &Addr::unchecked("addr"), + &[], &mock_env().block, ) .unwrap(); let saved_claims = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr")) + .prefix(&Addr::unchecked("addr")) + .range_raw(deps.as_mut().storage, None, None, Order::Ascending) + .collect::>>() .unwrap(); - assert_eq!(nfts.len(), 0); assert_eq!(saved_claims.len(), 0); } @@ -248,24 +303,31 @@ mod test { let mut env = mock_env(); env.block.height = 0; // the address has two claims however they are both not expired - let nfts = claims - .claim_nfts(deps.as_mut().storage, &Addr::unchecked("addr"), &env.block) - .unwrap(); + let error = claims + .claim_nfts( + deps.as_mut().storage, + &Addr::unchecked("addr"), + &[ + TEST_CRYPTO_PUNKS_TOKEN_ID.to_string(), + TEST_BAYC_TOKEN_ID.to_string(), + ], + &env.block, + ) + .unwrap_err(); + assert_eq!(error, NftClaimError::NotReady {}); let saved_claims = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr")) + .prefix(&Addr::unchecked("addr")) + .range(deps.as_mut().storage, None, None, Order::Ascending) + .collect::>>() .unwrap(); - assert_eq!(nfts.len(), 0); assert_eq!(saved_claims.len(), 2); - assert_eq!( - saved_claims[0].token_id, - TEST_CRYPTO_PUNKS_TOKEN_ID.to_string() - ); - assert_eq!(saved_claims[0].release_at, Expiration::AtHeight(10)); - assert_eq!(saved_claims[1].token_id, TEST_BAYC_TOKEN_ID.to_string()); - assert_eq!(saved_claims[1].release_at, Expiration::AtHeight(100)); + assert_eq!(saved_claims[0].0, TEST_BAYC_TOKEN_ID.to_string()); + assert_eq!(saved_claims[0].1, Expiration::AtHeight(100)); + assert_eq!(saved_claims[1].0, TEST_CRYPTO_PUNKS_TOKEN_ID.to_string()); + assert_eq!(saved_claims[1].1, Expiration::AtHeight(10)); } #[test] @@ -294,23 +356,25 @@ mod test { let mut env = mock_env(); env.block.height = 20; // the address has two claims and the first one can be released - let nfts = claims - .claim_nfts(deps.as_mut().storage, &Addr::unchecked("addr"), &env.block) + claims + .claim_nfts( + deps.as_mut().storage, + &Addr::unchecked("addr"), + &[TEST_BAYC_TOKEN_ID.to_string()], + &env.block, + ) .unwrap(); let saved_claims = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr")) + .prefix(&Addr::unchecked("addr")) + .range(deps.as_mut().storage, None, None, Order::Ascending) + .collect::>>() .unwrap(); - assert_eq!(nfts.len(), 1); - assert_eq!(nfts[0], TEST_BAYC_TOKEN_ID.to_string()); assert_eq!(saved_claims.len(), 1); - assert_eq!( - saved_claims[0].token_id, - TEST_CRYPTO_PUNKS_TOKEN_ID.to_string() - ); - assert_eq!(saved_claims[0].release_at, Expiration::AtHeight(100)); + assert_eq!(saved_claims[0].0, TEST_CRYPTO_PUNKS_TOKEN_ID.to_string()); + assert_eq!(saved_claims[0].1, Expiration::AtHeight(100)); } #[test] @@ -339,22 +403,25 @@ mod test { let mut env = mock_env(); env.block.height = 1000; // the address has two claims and both can be released - let nfts = claims - .claim_nfts(deps.as_mut().storage, &Addr::unchecked("addr"), &env.block) + claims + .claim_nfts( + deps.as_mut().storage, + &Addr::unchecked("addr"), + &[ + TEST_BAYC_TOKEN_ID.to_string(), + TEST_CRYPTO_PUNKS_TOKEN_ID.to_string(), + ], + &env.block, + ) .unwrap(); let saved_claims = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr")) + .prefix(&Addr::unchecked("addr")) + .range(deps.as_mut().storage, None, None, Order::Ascending) + .collect::>>() .unwrap(); - assert_eq!( - nfts, - vec![ - TEST_BAYC_TOKEN_ID.to_string(), - TEST_CRYPTO_PUNKS_TOKEN_ID.to_string() - ] - ); assert_eq!(saved_claims.len(), 0); } @@ -373,15 +440,88 @@ mod test { .unwrap(); let queried_claims = claims - .query_claims(deps.as_ref(), &Addr::unchecked("addr")) + .query_claims(deps.as_ref(), &Addr::unchecked("addr"), None, None) .unwrap(); let saved_claims = claims .0 - .load(deps.as_mut().storage, &Addr::unchecked("addr")) + .prefix(&Addr::unchecked("addr")) + .range(deps.as_mut().storage, None, None, Order::Ascending) + .map(|item| item.map(|(token_id, v)| NftClaim::new(token_id, v))) + .collect::>>() .unwrap(); + assert_eq!(queried_claims.nft_claims, saved_claims); } + #[test] + fn test_query_claims_returns_correct_claims_paginated() { + let mut deps = mock_dependencies(); + let claims = NftClaims::new("claims"); + + claims + .create_nft_claims( + deps.as_mut().storage, + &Addr::unchecked("addr"), + vec![ + TEST_BAYC_TOKEN_ID.to_string(), + TEST_CRYPTO_PUNKS_TOKEN_ID.to_string(), + ], + Expiration::AtHeight(10), + ) + .unwrap(); + + let queried_claims = claims + .query_claims(deps.as_ref(), &Addr::unchecked("addr"), None, None) + .unwrap(); + assert_eq!( + queried_claims.nft_claims, + vec![ + NftClaim::new(TEST_BAYC_TOKEN_ID.to_string(), Expiration::AtHeight(10)), + NftClaim::new( + TEST_CRYPTO_PUNKS_TOKEN_ID.to_string(), + Expiration::AtHeight(10) + ), + ] + ); + + let queried_claims = claims + .query_claims(deps.as_ref(), &Addr::unchecked("addr"), None, Some(1)) + .unwrap(); + assert_eq!( + queried_claims.nft_claims, + vec![NftClaim::new( + TEST_BAYC_TOKEN_ID.to_string(), + Expiration::AtHeight(10) + ),] + ); + + let queried_claims = claims + .query_claims( + deps.as_ref(), + &Addr::unchecked("addr"), + Some(&TEST_BAYC_TOKEN_ID.to_string()), + None, + ) + .unwrap(); + assert_eq!( + queried_claims.nft_claims, + vec![NftClaim::new( + TEST_CRYPTO_PUNKS_TOKEN_ID.to_string(), + Expiration::AtHeight(10) + )] + ); + + let queried_claims = claims + .query_claims( + deps.as_ref(), + &Addr::unchecked("addr"), + Some(&TEST_CRYPTO_PUNKS_TOKEN_ID.to_string()), + None, + ) + .unwrap(); + assert_eq!(queried_claims.nft_claims.len(), 0); + } + #[test] fn test_query_claims_returns_empty_for_non_existent_user() { let mut deps = mock_dependencies(); @@ -397,7 +537,7 @@ mod test { .unwrap(); let queried_claims = claims - .query_claims(deps.as_ref(), &Addr::unchecked("addr2")) + .query_claims(deps.as_ref(), &Addr::unchecked("addr2"), None, None) .unwrap(); assert_eq!(queried_claims.nft_claims.len(), 0);