From 495b5ae0ac57df89441b5d56f4caff45b8bd1890 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Tue, 29 Aug 2023 11:42:16 +0200 Subject: [PATCH 1/3] Add optional delete field to burner migrate msg --- contracts/burner/schema/burner.json | 7 +++ contracts/burner/schema/raw/migrate.json | 7 +++ contracts/burner/src/contract.rs | 57 +++++++++++++++++++----- contracts/burner/src/msg.rs | 5 +++ contracts/burner/tests/integration.rs | 3 +- 5 files changed, 68 insertions(+), 11 deletions(-) diff --git a/contracts/burner/schema/burner.json b/contracts/burner/schema/burner.json index 9aa679c82d..94c2124eb3 100644 --- a/contracts/burner/schema/burner.json +++ b/contracts/burner/schema/burner.json @@ -19,6 +19,13 @@ "payout" ], "properties": { + "delete": { + "description": "Optional amount of items to delete in this call. If it is not provided, nothing will be deleted. You can delete further items in a subsequent execute call.", + "default": 0, + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, "payout": { "description": "The address we send all remaining balance to", "type": "string" diff --git a/contracts/burner/schema/raw/migrate.json b/contracts/burner/schema/raw/migrate.json index 30b6f16527..752175c3ea 100644 --- a/contracts/burner/schema/raw/migrate.json +++ b/contracts/burner/schema/raw/migrate.json @@ -6,6 +6,13 @@ "payout" ], "properties": { + "delete": { + "description": "Optional amount of items to delete in this call. If it is not provided, nothing will be deleted. You can delete further items in a subsequent execute call.", + "default": 0, + "type": "integer", + "format": "uint32", + "minimum": 0.0 + }, "payout": { "description": "The address we send all remaining balance to", "type": "string" diff --git a/contracts/burner/src/contract.rs b/contracts/burner/src/contract.rs index d50ec14761..5921261da1 100644 --- a/contracts/burner/src/contract.rs +++ b/contracts/burner/src/contract.rs @@ -1,5 +1,5 @@ use cosmwasm_std::{ - entry_point, BankMsg, DepsMut, Env, MessageInfo, Order, Response, StdError, StdResult, + entry_point, BankMsg, DepsMut, Env, MessageInfo, Order, Response, StdError, StdResult, Storage, }; use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg}; @@ -24,10 +24,18 @@ pub fn migrate(deps: DepsMut, env: Env, msg: MigrateMsg) -> StdResult to_address: msg.payout.clone(), amount: balance, }; + + let deleted = if msg.delete != 0 { + cleanup(deps.storage, Some(msg.delete)) + } else { + 0 + }; + Ok(Response::new() .add_message(send) - .add_attribute("action", "burn") - .add_attribute("payout", msg.payout)) + .add_attribute("action", "migrate") + .add_attribute("payout", msg.payout) + .add_attribute("deleted_entries", deleted.to_string())) } #[entry_point] @@ -43,6 +51,14 @@ pub fn execute_cleanup( _info: MessageInfo, limit: Option, ) -> StdResult { + let deleted = cleanup(deps.storage, limit); + + Ok(Response::new() + .add_attribute("action", "burn") + .add_attribute("deleted_entries", deleted.to_string())) +} + +fn cleanup(storage: &mut dyn Storage, limit: Option) -> usize { // the number of elements we can still take (decreasing over time) let mut limit = limit.unwrap_or(u32::MAX) as usize; @@ -50,14 +66,13 @@ pub fn execute_cleanup( const PER_SCAN: usize = 20; loop { let take_this_scan = std::cmp::min(PER_SCAN, limit); - let keys: Vec<_> = deps - .storage + let keys: Vec<_> = storage .range_keys(None, None, Order::Ascending) .take(take_this_scan) .collect(); let deleted_this_scan = keys.len(); for k in keys { - deps.storage.remove(&k); + storage.remove(&k); } deleted += deleted_this_scan; limit -= deleted_this_scan; @@ -66,9 +81,7 @@ pub fn execute_cleanup( } } - Ok(Response::new() - .add_attribute("action", "burn") - .add_attribute("deleted_entries", deleted.to_string())) + deleted } #[cfg(test)] @@ -114,6 +127,7 @@ mod tests { let payout = String::from("someone else"); let msg = MigrateMsg { payout: payout.clone(), + delete: 0, }; let res = migrate(deps.as_mut(), mock_env(), msg).unwrap(); // check payout @@ -128,6 +142,29 @@ mod tests { ); } + #[test] + fn migrate_with_delete() { + let mut deps = mock_dependencies_with_balance(&coins(123456, "gold")); + + // store some sample data + deps.storage.set(b"foo", b"bar"); + deps.storage.set(b"key2", b"data2"); + deps.storage.set(b"key3", b"cool stuff"); + let cnt = deps.storage.range(None, None, Order::Ascending).count(); + assert_eq!(cnt, 3); + + // migrate all of the data in one go + let msg = MigrateMsg { + payout: "user".to_string(), + delete: 100, + }; + migrate(deps.as_mut(), mock_env(), msg).unwrap(); + + // no more data + let cnt = deps.storage.range(None, None, Order::Ascending).count(); + assert_eq!(cnt, 0); + } + #[test] fn execute_cleans_up_data() { let mut deps = mock_dependencies_with_balance(&coins(123456, "gold")); @@ -141,7 +178,7 @@ mod tests { // change the verifier via migrate let payout = String::from("someone else"); - let msg = MigrateMsg { payout }; + let msg = MigrateMsg { payout, delete: 0 }; let _res = migrate(deps.as_mut(), mock_env(), msg).unwrap(); let res = execute( diff --git a/contracts/burner/src/msg.rs b/contracts/burner/src/msg.rs index 602223eccb..f8d7f4558d 100644 --- a/contracts/burner/src/msg.rs +++ b/contracts/burner/src/msg.rs @@ -4,6 +4,11 @@ use cosmwasm_schema::cw_serde; pub struct MigrateMsg { /// The address we send all remaining balance to pub payout: String, + /// Optional amount of items to delete in this call. + /// If it is not provided, nothing will be deleted. + /// You can delete further items in a subsequent execute call. + #[serde(default)] + pub delete: u32, } /// A placeholder where we don't take any input diff --git a/contracts/burner/tests/integration.rs b/contracts/burner/tests/integration.rs index fe6069d4c9..c783a6fa3c 100644 --- a/contracts/burner/tests/integration.rs +++ b/contracts/burner/tests/integration.rs @@ -62,6 +62,7 @@ fn migrate_sends_funds() { let payout = String::from("someone else"); let msg = MigrateMsg { payout: payout.clone(), + delete: 0, }; let res: Response = migrate(&mut deps, mock_env(), msg).unwrap(); // check payout @@ -94,7 +95,7 @@ fn execute_cleans_up_data() { // change the verifier via migrate let payout = String::from("someone else"); - let msg = MigrateMsg { payout }; + let msg = MigrateMsg { payout, delete: 0 }; let _res: Response = migrate(&mut deps, mock_env(), msg).unwrap(); let res: Response = execute( From 569c366a3b7cdde9d3d84d4570e7648a28c044ec Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Tue, 29 Aug 2023 12:32:27 +0200 Subject: [PATCH 2/3] Apply review feedback --- contracts/burner/src/contract.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/contracts/burner/src/contract.rs b/contracts/burner/src/contract.rs index 5921261da1..1276720c6d 100644 --- a/contracts/burner/src/contract.rs +++ b/contracts/burner/src/contract.rs @@ -25,11 +25,7 @@ pub fn migrate(deps: DepsMut, env: Env, msg: MigrateMsg) -> StdResult amount: balance, }; - let deleted = if msg.delete != 0 { - cleanup(deps.storage, Some(msg.delete)) - } else { - 0 - }; + let deleted = cleanup(deps.storage, Some(msg.delete)); Ok(Response::new() .add_message(send) @@ -54,7 +50,7 @@ pub fn execute_cleanup( let deleted = cleanup(deps.storage, limit); Ok(Response::new() - .add_attribute("action", "burn") + .add_attribute("action", "cleanup") .add_attribute("deleted_entries", deleted.to_string())) } From 3a962cca657df8b1e1a23e0b1e50b55fb61e374f Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Tue, 29 Aug 2023 13:55:33 +0200 Subject: [PATCH 3/3] Refactor --- contracts/burner/src/contract.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/contracts/burner/src/contract.rs b/contracts/burner/src/contract.rs index 1276720c6d..d6a96a0e08 100644 --- a/contracts/burner/src/contract.rs +++ b/contracts/burner/src/contract.rs @@ -25,7 +25,7 @@ pub fn migrate(deps: DepsMut, env: Env, msg: MigrateMsg) -> StdResult amount: balance, }; - let deleted = cleanup(deps.storage, Some(msg.delete)); + let deleted = cleanup(deps.storage, msg.delete as usize); Ok(Response::new() .add_message(send) @@ -47,6 +47,7 @@ pub fn execute_cleanup( _info: MessageInfo, limit: Option, ) -> StdResult { + let limit = limit.unwrap_or(u32::MAX) as usize; let deleted = cleanup(deps.storage, limit); Ok(Response::new() @@ -54,10 +55,7 @@ pub fn execute_cleanup( .add_attribute("deleted_entries", deleted.to_string())) } -fn cleanup(storage: &mut dyn Storage, limit: Option) -> usize { - // the number of elements we can still take (decreasing over time) - let mut limit = limit.unwrap_or(u32::MAX) as usize; - +fn cleanup(storage: &mut dyn Storage, mut limit: usize) -> usize { let mut deleted = 0; const PER_SCAN: usize = 20; loop { @@ -71,6 +69,7 @@ fn cleanup(storage: &mut dyn Storage, limit: Option) -> usize { storage.remove(&k); } deleted += deleted_this_scan; + // decrease the number of elements we can still take limit -= deleted_this_scan; if limit == 0 || deleted_this_scan < take_this_scan { break;