From a7f7135f1e5e91e7b5c01554909aa1f783c7602a Mon Sep 17 00:00:00 2001 From: Unique Divine Date: Wed, 25 Sep 2024 04:38:52 -0500 Subject: [PATCH] epic(lockup): docs, tests for all execute calls, and lock lifecycle clarity --- Cargo.lock | 3 +- contracts/lockup/Cargo.toml | 1 + contracts/lockup/src/contract.rs | 69 ++++++---- contracts/lockup/src/contract_test.rs | 183 ++++++++++++++++++++++++++ contracts/lockup/src/events.rs | 10 +- contracts/lockup/src/lib.rs | 3 + contracts/lockup/src/msgs.rs | 9 +- contracts/lockup/src/state.rs | 52 +++++++- 8 files changed, 287 insertions(+), 43 deletions(-) create mode 100644 contracts/lockup/src/contract_test.rs diff --git a/Cargo.lock b/Cargo.lock index fe79fcb..bc85616 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1739,6 +1739,7 @@ dependencies = [ name = "lockup" version = "0.2.0" dependencies = [ + "anyhow", "cosmwasm-schema 2.0.2", "cosmwasm-std 2.0.2", "cw-storage-plus 2.0.0", @@ -1935,7 +1936,7 @@ dependencies = [ [[package]] name = "nibiru-std" -version = "0.1.0" +version = "0.2.0" dependencies = [ "anyhow", "cosmwasm-schema 2.0.2", diff --git a/contracts/lockup/Cargo.toml b/contracts/lockup/Cargo.toml index 1060dd4..32c7ea6 100644 --- a/contracts/lockup/Cargo.toml +++ b/contracts/lockup/Cargo.toml @@ -21,5 +21,6 @@ cw20 = { workspace = true } schemars = { workspace = true } thiserror = { workspace = true } serde = { version = "1.0.188", default-features = false, features = ["derive"] } +anyhow = { workspace = true } [dev-dependencies] \ No newline at end of file diff --git a/contracts/lockup/src/contract.rs b/contracts/lockup/src/contract.rs index 83c04d8..a7c3d0d 100644 --- a/contracts/lockup/src/contract.rs +++ b/contracts/lockup/src/contract.rs @@ -1,13 +1,14 @@ use crate::error::ContractError; use crate::events::{ - new_coins_locked_event, new_funds_withdrawn_event, - new_unlock_initiation_event, + event_coins_locked, event_funds_withdrawn, event_unlock_initiated, }; use crate::msgs::{ExecuteMsg, InstantiateMsg, QueryMsg}; -use crate::state::{locks, Lock, LOCKS_ID, NOT_UNLOCKING_BLOCK_IDENTIFIER}; +use crate::state::{ + locks, Lock, LockState, LOCKS_ID, NOT_UNLOCKING_BLOCK_IDENTIFIER, +}; use cosmwasm_std::{ - to_json_binary, BankMsg, Binary, Deps, DepsMut, Env, Event, MessageInfo, - Order, Response, StdResult, + to_json_binary, BankMsg, Binary, CosmosMsg, Deps, DepsMut, Env, Event, + MessageInfo, Order, Response, StdResult, }; use cw_storage_plus::Bound; @@ -51,30 +52,36 @@ pub(crate) fn execute_withdraw_funds( ) -> Result { let locks = locks(); + let mut tx_msgs: Vec = Vec::new(); + // we update the lock to mark funds have been withdrawn let lock = locks.update(deps.storage, id, |lock| -> Result<_, ContractError> { let mut lock = lock.ok_or(ContractError::NotFound(id))?; - - if lock.funds_withdrawn { - return Err(ContractError::FundsAlreadyWithdrawn(id)); - } - - if lock.end_block < env.block.height { - return Err(ContractError::NotMatured(id)); + match lock.state(env.block.height) { + LockState::Matured => { + tx_msgs.push( + BankMsg::Send { + to_address: lock.owner.to_string(), + amount: vec![lock.coin.clone()], + } + .into(), + ); + lock.funds_withdrawn = true; + Ok(lock) + } + LockState::FundedPreUnlock | LockState::Unlocking => { + Err(ContractError::NotMatured(id)) + } + LockState::Withdrawn => { + Err(ContractError::FundsAlreadyWithdrawn(id)) + } } - - lock.funds_withdrawn = true; - - Ok(lock) })?; Ok(Response::new() - .add_event(new_funds_withdrawn_event(id, &lock.coin)) - .add_message(BankMsg::Send { - to_address: lock.owner.to_string(), - amount: vec![lock.coin], - })) + .add_event(event_funds_withdrawn(id, &lock.coin)) + .add_messages(tx_msgs)) } pub(crate) fn execute_initiate_unlock( @@ -89,15 +96,23 @@ pub(crate) fn execute_initiate_unlock( let lock = locks.update(deps.storage, id, |lock| -> Result<_, ContractError> { let mut lock = lock.ok_or(ContractError::NotFound(id))?; - if lock.end_block != NOT_UNLOCKING_BLOCK_IDENTIFIER { - return Err(ContractError::AlreadyUnlocking(id)); + + match lock.state(env.block.height) { + LockState::FundedPreUnlock => { + lock.end_block = env.block.height + lock.duration_blocks; + Ok(lock) + } + LockState::Unlocking | LockState::Matured => { + Err(ContractError::AlreadyUnlocking(id)) + } + LockState::Withdrawn => { + Err(ContractError::FundsAlreadyWithdrawn(id)) + } } - lock.end_block = env.block.height + lock.duration_blocks; - Ok(lock) })?; // emit unlock initiation event - Ok(Response::new().add_event(new_unlock_initiation_event( + Ok(Response::new().add_event(event_unlock_initiated( id, &lock.coin, lock.end_block, @@ -149,7 +164,7 @@ pub(crate) fn execute_lock( ) .expect("must never fail"); - events.push(new_coins_locked_event(id, &coin)) + events.push(event_coins_locked(id, &coin)) } Ok(Response::new().add_events(events)) diff --git a/contracts/lockup/src/contract_test.rs b/contracts/lockup/src/contract_test.rs new file mode 100644 index 0000000..1b4392e --- /dev/null +++ b/contracts/lockup/src/contract_test.rs @@ -0,0 +1,183 @@ +//! Tests for the execute calls of contract.rs + +use crate::contract::{execute, instantiate}; +use crate::error::ContractError; +use crate::msgs::{ExecuteMsg, InstantiateMsg}; +use crate::state::{locks, Lock, LockState, NOT_UNLOCKING_BLOCK_IDENTIFIER}; + +use cosmwasm_std::testing::{ + mock_dependencies, mock_env, mock_info, MockApi, MockQuerier, MockStorage, +}; +use cosmwasm_std::{coins, BankMsg, Coin, Env, MessageInfo, OwnedDeps, SubMsg}; + +const OWNER: &str = "owner"; +const USER: &str = "user"; +const DENOM: &str = "unibi"; + +fn setup_contract() -> anyhow::Result<( + OwnedDeps, + Env, + MessageInfo, +)> { + let mut deps = mock_dependencies(); + let env = mock_env(); + let info = mock_info(OWNER, &[]); + let msg = InstantiateMsg {}; + let res = instantiate(deps.as_mut(), env.clone(), info.clone(), msg)?; + assert_eq!(0, res.messages.len()); + Ok((deps, env, info)) +} + +pub type TestResult = anyhow::Result<()>; + +#[test] +fn test_execute_lock() -> TestResult { + let (mut deps, env, _info) = setup_contract()?; + + // Successful lock + let info = mock_info(USER, &coins(100, DENOM)); + let msg = ExecuteMsg::Lock { blocks: 100 }; + let res = execute(deps.as_mut(), env.clone(), info, msg)?; + assert_eq!(1, res.events.len()); + + // Query the lock + let locks = locks(); + let lock = locks.load(&deps.storage, 1)?; + assert_eq!(lock.owner, USER); + assert_eq!(lock.coin, Coin::new(100u128, DENOM)); + assert_eq!(lock.duration_blocks, 100); + assert_eq!(lock.start_block, env.block.height); + assert_eq!(lock.end_block, NOT_UNLOCKING_BLOCK_IDENTIFIER); + assert!(!lock.funds_withdrawn); + + // Attempt to lock with no funds + let info = mock_info(USER, &[]); + let msg = ExecuteMsg::Lock { blocks: 100 }; + let err = execute(deps.as_mut(), env.clone(), info, msg).unwrap_err(); + assert!(matches!(err, ContractError::InvalidCoins(_))); + + // Attempt to lock with zero duration + let info = mock_info(USER, &coins(100, DENOM)); + let msg = ExecuteMsg::Lock { blocks: 0 }; + let err = execute(deps.as_mut(), env, info, msg).unwrap_err(); + assert!(matches!(err, ContractError::InvalidLockDuration)); + Ok(()) +} + +#[test] +fn test_execute_initiate_unlock() -> TestResult { + let (mut deps, env, _info) = setup_contract()?; + + // Create a lock first + let info = mock_info(USER, &coins(100, DENOM)); + let msg = ExecuteMsg::Lock { blocks: 100 }; + let _ = execute(deps.as_mut(), env.clone(), info, msg)?; + + // Successful initiate unlock + let msg = ExecuteMsg::InitiateUnlock { id: 1 }; + let info = mock_info(USER, &[]); + let res = execute(deps.as_mut(), env.clone(), info, msg)?; + assert_eq!(1, res.events.len()); + + // Query the lock + let locks = locks(); + let lock = locks.load(&deps.storage, 1)?; + assert_eq!(lock.end_block, env.block.height + 100); + + // Attempt to initiate unlock again + let msg = ExecuteMsg::InitiateUnlock { id: 1 }; + let info = mock_info(USER, &[]); + let err = execute(deps.as_mut(), env.clone(), info, msg).unwrap_err(); + assert!(matches!(err, ContractError::AlreadyUnlocking(_))); + + // Attempt to initiate unlock for non-existent lock + let msg = ExecuteMsg::InitiateUnlock { id: 99 }; + let info = mock_info(USER, &[]); + let err = execute(deps.as_mut(), env, info, msg).unwrap_err(); + assert!(matches!(err, ContractError::NotFound(_))); + Ok(()) +} + +#[test] +fn test_execute_withdraw_funds() -> TestResult { + let (mut deps, mut env, _info) = setup_contract()?; + + // Create and initiate unlock for a lock + let info = mock_info(USER, &coins(100, DENOM)); + let msg = ExecuteMsg::Lock { blocks: 100 }; + let _ = execute(deps.as_mut(), env.clone(), info, msg)?; + + let msg = ExecuteMsg::InitiateUnlock { id: 1 }; + let info = mock_info(USER, &[]); + let _ = execute(deps.as_mut(), env.clone(), info, msg)?; + + // Attempt to withdraw before maturity + let msg = ExecuteMsg::WithdrawFunds { id: 1 }; + let info = mock_info(USER, &[]); + let err = execute(deps.as_mut(), env.clone(), info, msg).unwrap_err(); + assert!(matches!(err, ContractError::NotMatured(_))); + + // Fast forward to maturity + env.block.height += 101; + + // Successful withdraw + let msg = ExecuteMsg::WithdrawFunds { id: 1 }; + let info = mock_info(USER, &[]); + let res = execute(deps.as_mut(), env.clone(), info, msg)?; + assert_eq!(1, res.messages.len()); + assert_eq!( + res.messages[0], + SubMsg::new(BankMsg::Send { + to_address: USER.to_string(), + amount: vec![Coin::new(100u128, DENOM)] + }) + ); + + // Query the lock + let locks = locks(); + let lock = locks.load(&deps.storage, 1)?; + assert!(lock.funds_withdrawn); + + // Attempt to withdraw again + let msg = ExecuteMsg::WithdrawFunds { id: 1 }; + let info = mock_info(USER, &[]); + let err = execute(deps.as_mut(), env, info, msg).unwrap_err(); + assert!(matches!(err, ContractError::FundsAlreadyWithdrawn(_))); + + Ok(()) +} + +#[test] +fn test_lock_state() -> TestResult { + let (mut _deps, env, _info) = setup_contract()?; + + let lock = Lock { + id: 1, + coin: Coin::new(100u128, DENOM), + owner: USER.to_string(), + duration_blocks: 100, + start_block: env.block.height, + end_block: NOT_UNLOCKING_BLOCK_IDENTIFIER, + funds_withdrawn: false, + }; + + // Test FundedPreUnlock state + assert_eq!(lock.state(env.block.height), LockState::FundedPreUnlock); + + // Test Unlocking state + let mut unlocking_lock = lock.clone(); + unlocking_lock.end_block = env.block.height + 50; + assert_eq!(unlocking_lock.state(env.block.height), LockState::Unlocking); + + // Test Matured state + let mut matured_lock = lock.clone(); + matured_lock.end_block = env.block.height - 1; + assert_eq!(matured_lock.state(env.block.height), LockState::Matured); + + // Test Withdrawn state + let mut withdrawn_lock = lock; + withdrawn_lock.funds_withdrawn = true; + assert_eq!(withdrawn_lock.state(env.block.height), LockState::Withdrawn); + + Ok(()) +} diff --git a/contracts/lockup/src/events.rs b/contracts/lockup/src/events.rs index e55c76b..89b3339 100644 --- a/contracts/lockup/src/events.rs +++ b/contracts/lockup/src/events.rs @@ -4,24 +4,20 @@ pub const COINS_LOCKED_EVENT_NAME: &str = "coins_locked"; pub const UNLOCK_INITIATION_EVENT_NAME: &str = "unlock_initiated"; pub const LOCK_FUNDS_WITHDRAWN: &str = "funds_withdrawn"; -pub fn new_coins_locked_event(id: u64, coin: &Coin) -> Event { +pub fn event_coins_locked(id: u64, coin: &Coin) -> Event { Event::new(COINS_LOCKED_EVENT_NAME) .add_attribute("id", id.to_string()) .add_attribute("coins", coin.to_string()) } -pub fn new_unlock_initiation_event( - id: u64, - coin: &Coin, - unlock_block: u64, -) -> Event { +pub fn event_unlock_initiated(id: u64, coin: &Coin, unlock_block: u64) -> Event { Event::new(UNLOCK_INITIATION_EVENT_NAME) .add_attribute("id", id.to_string()) .add_attribute("coins", coin.to_string()) .add_attribute("unlock_block", unlock_block.to_string()) } -pub fn new_funds_withdrawn_event(id: u64, coin: &Coin) -> Event { +pub fn event_funds_withdrawn(id: u64, coin: &Coin) -> Event { Event::new(LOCK_FUNDS_WITHDRAWN) .add_attribute("id", id.to_string()) .add_attribute("coins", coin.to_string()) diff --git a/contracts/lockup/src/lib.rs b/contracts/lockup/src/lib.rs index c9cd8a3..387b58d 100644 --- a/contracts/lockup/src/lib.rs +++ b/contracts/lockup/src/lib.rs @@ -3,3 +3,6 @@ pub mod error; pub mod events; pub mod msgs; pub mod state; + +#[cfg(test)] +pub mod contract_test; diff --git a/contracts/lockup/src/msgs.rs b/contracts/lockup/src/msgs.rs index 150c114..732b954 100644 --- a/contracts/lockup/src/msgs.rs +++ b/contracts/lockup/src/msgs.rs @@ -1,10 +1,9 @@ -use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; +use cosmwasm_schema::cw_serde; -#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +#[cw_serde] pub struct InstantiateMsg {} -#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +#[cw_serde] pub enum ExecuteMsg { Lock { blocks: u64 }, @@ -13,7 +12,7 @@ pub enum ExecuteMsg { WithdrawFunds { id: u64 }, } -#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +#[cw_serde] pub enum QueryMsg { LocksByDenomUnlockingAfter { denom: String, diff --git a/contracts/lockup/src/state.rs b/contracts/lockup/src/state.rs index bd822c5..c2fc945 100644 --- a/contracts/lockup/src/state.rs +++ b/contracts/lockup/src/state.rs @@ -1,22 +1,68 @@ +use cosmwasm_schema::cw_serde; use cosmwasm_std::Coin; use cw_storage_plus::{Index, IndexList, IndexedMap, Item, MultiIndex}; -use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; +/// A sentinel value to represent that a Lock is not currently unlocking. It +/// ensures that the "not unlocking" state is distinct from any valid block +/// height and allows the contract to check ifi a Lock started unlocking or not. pub const NOT_UNLOCKING_BLOCK_IDENTIFIER: u64 = u64::MAX; + pub const LOCKS_ID: Item = Item::new("locks_id"); -#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema)] +/// Represents a lock on funds in the contract. +/// +/// A `Lock` is created when a user locks up their funds for a specified duration. +/// It keeps track of the locked funds, owner, duration, and various block heights +/// related to the lock's lifecycle. +#[cw_serde] pub struct Lock { + /// Unique identifier for the lock. pub id: u64, + /// The amount and denomination of the locked funds. pub coin: Coin, + /// The address of the lock owner. pub owner: String, + /// The duration of the lock in number of blocks. pub duration_blocks: u64, + /// The block height at which the lock was created. pub start_block: u64, + /// The block height at which the lock ends. + /// + /// This is set to `NOT_UNLOCKING_BLOCK_IDENTIFIER` when the lock is created, + /// and updated to an actual block height when unlocking is initiated. pub end_block: u64, + /// Indicates whether the funds have been withdrawn after the lock period. pub funds_withdrawn: bool, } +/// Lock Lifecycle States +#[derive(Debug, PartialEq)] +pub enum LockState { + /// The lock has been created and funds are locked + FundedPreUnlock, + /// Unlocking has been initiated, but the lock period hasn't expired + Unlocking, + /// The lock period has expired, funds are ready for withdrawal + Matured, + /// Funds have been withdrawn, the lock is now inactive + Withdrawn, +} + +impl Lock { + /// Computes the lifecycle state of the Lock + pub fn state(&self, current_block: u64) -> LockState { + if self.funds_withdrawn { + return LockState::Withdrawn; + } + + match self.end_block { + NOT_UNLOCKING_BLOCK_IDENTIFIER => LockState::FundedPreUnlock, + end_block if current_block >= end_block => LockState::Matured, + _ => LockState::Unlocking, + } + } +} + pub struct LockIndexes<'a> { pub denom_end: MultiIndex<'a, (String, u64), Lock, u64>, pub addr_denom_end: MultiIndex<'a, (String, String, u64), Lock, u64>,