From 09b7b31427eb9c484e070697c91dba89c1a22310 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Fri, 9 Oct 2020 22:39:01 +0200 Subject: [PATCH] Don't filter out expired allowances, but add metadata to response --- .../schema/all_nft_info_response.json | 5 ++ .../schema/approved_for_all_response.json | 5 ++ .../cw721-base/schema/owner_of_response.json | 5 ++ contracts/cw721-base/src/contract.rs | 55 +++++++++++++------ .../cw721/schema/all_nft_info_response.json | 5 ++ .../schema/approved_for_all_response.json | 5 ++ packages/cw721/schema/owner_of_response.json | 5 ++ packages/cw721/src/query.rs | 2 + 8 files changed, 69 insertions(+), 18 deletions(-) diff --git a/contracts/cw721-base/schema/all_nft_info_response.json b/contracts/cw721-base/schema/all_nft_info_response.json index 1a3214ed1..7d20d7b75 100644 --- a/contracts/cw721-base/schema/all_nft_info_response.json +++ b/contracts/cw721-base/schema/all_nft_info_response.json @@ -29,6 +29,7 @@ "type": "object", "required": [ "expires", + "is_expired", "spender" ], "properties": { @@ -40,6 +41,10 @@ } ] }, + "is_expired": { + "description": "true if already expired (for easy filtering in a client)", + "type": "boolean" + }, "spender": { "description": "Account that can transfer/send the token", "allOf": [ diff --git a/contracts/cw721-base/schema/approved_for_all_response.json b/contracts/cw721-base/schema/approved_for_all_response.json index 5b013ee49..fff6b37d4 100644 --- a/contracts/cw721-base/schema/approved_for_all_response.json +++ b/contracts/cw721-base/schema/approved_for_all_response.json @@ -18,6 +18,7 @@ "type": "object", "required": [ "expires", + "is_expired", "spender" ], "properties": { @@ -29,6 +30,10 @@ } ] }, + "is_expired": { + "description": "true if already expired (for easy filtering in a client)", + "type": "boolean" + }, "spender": { "description": "Account that can transfer/send the token", "allOf": [ diff --git a/contracts/cw721-base/schema/owner_of_response.json b/contracts/cw721-base/schema/owner_of_response.json index 8aa467831..6b6bad6e8 100644 --- a/contracts/cw721-base/schema/owner_of_response.json +++ b/contracts/cw721-base/schema/owner_of_response.json @@ -28,6 +28,7 @@ "type": "object", "required": [ "expires", + "is_expired", "spender" ], "properties": { @@ -39,6 +40,10 @@ } ] }, + "is_expired": { + "description": "true if already expired (for easy filtering in a client)", + "type": "boolean" + }, "spender": { "description": "Account that can transfer/send the token", "allOf": [ diff --git a/contracts/cw721-base/src/contract.rs b/contracts/cw721-base/src/contract.rs index 32ccff260..845e7345c 100644 --- a/contracts/cw721-base/src/contract.rs +++ b/contracts/cw721-base/src/contract.rs @@ -458,19 +458,15 @@ fn query_all_approvals( let res: StdResult> = operators_read(&deps.storage, &owner_raw) .range(start.as_deref(), None, Order::Ascending) - .filter_map(|item| match item { - Ok((k, expires)) => { - // we remove all expired operators from the result - if expires.is_expired(&env.block) { - None - } else { - match deps.api.human_address(&k.into()) { - Ok(spender) => Some(Ok(cw721::Approval { spender, expires })), - Err(e) => Some(Err(e)), - } - } - } - Err(e) => Some(Err(e)), + .map(|item| { + item.and_then(|(k, expires)| { + let spender = deps.api.human_address(&k.into())?; + Ok(cw721::Approval { + spender, + expires, + is_expired: expires.is_expired(&env.block), + }) + }) }) // FIXME: decide: take before (fix gas costs) or after (fix return value size)? .take(limit) @@ -520,15 +516,19 @@ fn humanize_approvals( ) -> StdResult> { info.approvals .iter() - .filter(|apr| !apr.expires.is_expired(block)) - .map(|apr| humanize_approval(api, apr)) + .map(|apr| humanize_approval(api, block, apr)) .collect() } -fn humanize_approval(api: A, approval: &Approval) -> StdResult { +fn humanize_approval( + api: A, + block: &BlockInfo, + approval: &Approval, +) -> StdResult { Ok(cw721::Approval { spender: api.human_address(&approval.spender)?, expires: approval.expires, + is_expired: approval.expires.is_expired(&block), }) } @@ -980,7 +980,8 @@ mod tests { ApprovedForAllResponse { operators: vec![cw721::Approval { spender: "operator".into(), - expires: Expiration::Never {} + expires: Expiration::Never {}, + is_expired: false, }] } ); @@ -1002,6 +1003,7 @@ mod tests { operators: vec![cw721::Approval { spender: "buddy".into(), expires: buddy_expires, + is_expired: false, }] } ); @@ -1018,7 +1020,8 @@ mod tests { ApprovedForAllResponse { operators: vec![cw721::Approval { spender: "operator".into(), - expires: Expiration::Never {} + expires: Expiration::Never {}, + is_expired: false, }] } ); @@ -1036,6 +1039,22 @@ mod tests { operators: vec![cw721::Approval { spender: "buddy".into(), expires: buddy_expires, + is_expired: false, + }] + } + ); + + // show the is_expired flag is set at the proper time + let mut late_env = mock_env(); + late_env.block.height = 1234569; // expired + let res = query_all_approvals(&deps, late_env, "person".into(), None, None).unwrap(); + assert_eq!( + res, + ApprovedForAllResponse { + operators: vec![cw721::Approval { + spender: "buddy".into(), + expires: buddy_expires, + is_expired: true, }] } ); diff --git a/packages/cw721/schema/all_nft_info_response.json b/packages/cw721/schema/all_nft_info_response.json index 1a3214ed1..7d20d7b75 100644 --- a/packages/cw721/schema/all_nft_info_response.json +++ b/packages/cw721/schema/all_nft_info_response.json @@ -29,6 +29,7 @@ "type": "object", "required": [ "expires", + "is_expired", "spender" ], "properties": { @@ -40,6 +41,10 @@ } ] }, + "is_expired": { + "description": "true if already expired (for easy filtering in a client)", + "type": "boolean" + }, "spender": { "description": "Account that can transfer/send the token", "allOf": [ diff --git a/packages/cw721/schema/approved_for_all_response.json b/packages/cw721/schema/approved_for_all_response.json index 5b013ee49..fff6b37d4 100644 --- a/packages/cw721/schema/approved_for_all_response.json +++ b/packages/cw721/schema/approved_for_all_response.json @@ -18,6 +18,7 @@ "type": "object", "required": [ "expires", + "is_expired", "spender" ], "properties": { @@ -29,6 +30,10 @@ } ] }, + "is_expired": { + "description": "true if already expired (for easy filtering in a client)", + "type": "boolean" + }, "spender": { "description": "Account that can transfer/send the token", "allOf": [ diff --git a/packages/cw721/schema/owner_of_response.json b/packages/cw721/schema/owner_of_response.json index 8aa467831..6b6bad6e8 100644 --- a/packages/cw721/schema/owner_of_response.json +++ b/packages/cw721/schema/owner_of_response.json @@ -28,6 +28,7 @@ "type": "object", "required": [ "expires", + "is_expired", "spender" ], "properties": { @@ -39,6 +40,10 @@ } ] }, + "is_expired": { + "description": "true if already expired (for easy filtering in a client)", + "type": "boolean" + }, "spender": { "description": "Account that can transfer/send the token", "allOf": [ diff --git a/packages/cw721/src/query.rs b/packages/cw721/src/query.rs index 3816e12f7..1bb46e12f 100644 --- a/packages/cw721/src/query.rs +++ b/packages/cw721/src/query.rs @@ -63,6 +63,8 @@ pub struct Approval { pub spender: HumanAddr, /// When the Approval expires (maybe Expiration::never) pub expires: Expiration, + /// true if already expired (for easy filtering in a client) + pub is_expired: bool, } #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]