From db8e5f191c96ab3cbe552bc8fb5c91dcfdd9f5a4 Mon Sep 17 00:00:00 2001 From: Connor Barr Date: Sun, 18 Feb 2024 15:13:41 +0000 Subject: [PATCH] Added cancel limit --- contracts/orderbook/src/contract.rs | 15 +- contracts/orderbook/src/error.rs | 15 +- contracts/orderbook/src/msg.rs | 6 +- contracts/orderbook/src/order.rs | 75 ++++++- contracts/orderbook/src/tests/test_order.rs | 237 +++++++++++++++++++- contracts/orderbook/src/types/mod.rs | 2 + contracts/orderbook/src/types/order.rs | 1 - contracts/orderbook/src/types/orderbook.rs | 33 ++- contracts/orderbook/src/types/reply_id.rs | 1 + 9 files changed, 361 insertions(+), 24 deletions(-) create mode 100644 contracts/orderbook/src/types/reply_id.rs diff --git a/contracts/orderbook/src/contract.rs b/contracts/orderbook/src/contract.rs index 6452678..6959c42 100644 --- a/contracts/orderbook/src/contract.rs +++ b/contracts/orderbook/src/contract.rs @@ -71,7 +71,11 @@ pub fn execute( } => order::place_limit(deps, env, info, book_id, tick_id, order_direction, quantity), // Cancels limit order with given ID - ExecuteMsg::CancelLimit => order::cancel_limit(deps, env, info), + ExecuteMsg::CancelLimit { + book_id, + tick_id, + order_id, + } => order::cancel_limit(deps, env, info, book_id, tick_id, order_id), // Places a market order on the passed in market ExecuteMsg::PlaceMarket => order::place_market(deps, env, info), @@ -93,9 +97,14 @@ pub fn query(_deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { /// Handling submessage reply. /// For more info on submessage and reply, see https://github.com/CosmWasm/cosmwasm/blob/main/SEMANTICS.md#submessages #[cfg_attr(not(feature = "library"), entry_point)] -pub fn reply(_deps: DepsMut, _env: Env, _msg: Reply) -> Result { +pub fn reply(_deps: DepsMut, _env: Env, msg: Reply) -> Result { // With `Response` type, it is still possible to dispatch message to invoke external logic. // See: https://github.com/CosmWasm/cosmwasm/blob/main/SEMANTICS.md#dispatching-messages - + if msg.result.is_err() { + return Err(ContractError::ReplyError { + id: msg.id, + error: msg.result.unwrap_err(), + }); + } todo!() } diff --git a/contracts/orderbook/src/error.rs b/contracts/orderbook/src/error.rs index 66c66be..87c16e9 100644 --- a/contracts/orderbook/src/error.rs +++ b/contracts/orderbook/src/error.rs @@ -1,4 +1,4 @@ -use cosmwasm_std::{CoinsError, StdError, Uint128}; +use cosmwasm_std::{CoinsError, OverflowError, StdError, Uint128}; use cw_utils::PaymentError; use thiserror::Error; @@ -7,6 +7,9 @@ pub enum ContractError { #[error("{0}")] Std(#[from] StdError), + #[error("{0}")] + Overflow(#[from] OverflowError), + #[error("Unauthorized")] Unauthorized {}, @@ -27,4 +30,14 @@ pub enum ContractError { #[error(transparent)] PaymentError(#[from] PaymentError), + + #[error("Order not found: {book_id:?}, {tick_id:?}, {order_id:?}")] + OrderNotFound { + book_id: u64, + tick_id: i64, + order_id: u64, + }, + + #[error("Reply error: {id:?}, {error:?}")] + ReplyError { id: u64, error: String }, } diff --git a/contracts/orderbook/src/msg.rs b/contracts/orderbook/src/msg.rs index 2033316..20ca004 100644 --- a/contracts/orderbook/src/msg.rs +++ b/contracts/orderbook/src/msg.rs @@ -20,7 +20,11 @@ pub enum ExecuteMsg { order_direction: OrderDirection, quantity: Uint128, }, - CancelLimit, + CancelLimit { + book_id: u64, + tick_id: i64, + order_id: u64, + }, PlaceMarket, } diff --git a/contracts/orderbook/src/order.rs b/contracts/orderbook/src/order.rs index 7f8b287..1e10216 100644 --- a/contracts/orderbook/src/order.rs +++ b/contracts/orderbook/src/order.rs @@ -1,9 +1,11 @@ use crate::error::ContractError; use crate::state::*; use crate::state::{MAX_TICK, MIN_TICK, ORDERBOOKS}; -use crate::types::{LimitOrder, OrderDirection}; -use cosmwasm_std::{coin, ensure, BankMsg, DepsMut, Env, MessageInfo, Response, Uint128}; -use cw_utils::must_pay; +use crate::types::{LimitOrder, OrderDirection, REPLY_ID_REFUND}; +use cosmwasm_std::{ + coin, ensure, ensure_eq, BankMsg, DepsMut, Env, MessageInfo, Response, SubMsg, Uint128, +}; +use cw_utils::{must_pay, nonpayable}; #[allow(clippy::manual_range_contains)] pub fn place_limit( @@ -33,16 +35,14 @@ pub fn place_limit( ); // Determine the correct denom based on order direction - let expected_denom = match order_direction { - OrderDirection::Bid => orderbook.quote_denom, - OrderDirection::Ask => orderbook.base_denom, - }; + let expected_denom = orderbook.get_expected_denom_for_direction(&order_direction); // Verify the funds sent with the message match the `quantity` for the correct denom // We reject any quantity that is not exactly equal to the amount in the limit order being placed let received = must_pay(&info, &expected_denom)?; - ensure!( - received == quantity, + ensure_eq!( + received, + quantity, ContractError::InsufficientFunds { sent: received, required: quantity, @@ -65,6 +65,11 @@ pub fn place_limit( // Save the order to the orderbook orders().save(deps.storage, &(book_id, tick_id, order_id), &limit_order)?; + // Update tick liquidity + TICK_LIQUIDITY.update(deps.storage, &(book_id, tick_id), |liquidity| { + Ok::(liquidity.unwrap_or_default().checked_add(quantity)?) + })?; + Ok(Response::new() .add_message(BankMsg::Send { to_address: env.contract.address.to_string(), @@ -80,15 +85,61 @@ pub fn place_limit( } pub fn cancel_limit( - _deps: DepsMut, + deps: DepsMut, _env: Env, info: MessageInfo, + book_id: u64, + tick_id: i64, + order_id: u64, ) -> Result { - // TODO: Implement cancel_limit + nonpayable(&info)?; + let key = (book_id, tick_id, order_id); + // Check for the order, error if not found + let order = orders() + .may_load(deps.storage, &key)? + .ok_or(ContractError::OrderNotFound { + book_id, + tick_id, + order_id, + })?; + + // Ensure the sender is the order owner + ensure_eq!(info.sender, order.owner, ContractError::Unauthorized {}); + + // Remove order + orders().remove(deps.storage, &key)?; + + // Update tick liquidity + TICK_LIQUIDITY.update(deps.storage, &(book_id, tick_id), |liquidity| { + Ok::(liquidity.unwrap_or_default().checked_sub(order.quantity)?) + })?; + + // Get orderbook info for correct denomination + let orderbook = + ORDERBOOKS + .may_load(deps.storage, &order.book_id)? + .ok_or(ContractError::InvalidBookId { + book_id: order.book_id, + })?; + + // Generate refund + let expected_denom = orderbook.get_expected_denom_for_direction(&order.order_direction); + let coin_to_send = coin(order.quantity.u128(), expected_denom); + let refund_msg = SubMsg::reply_on_error( + BankMsg::Send { + to_address: order.owner.to_string(), + amount: vec![coin_to_send], + }, + REPLY_ID_REFUND, + ); Ok(Response::new() .add_attribute("method", "cancelLimit") - .add_attribute("owner", info.sender)) + .add_attribute("owner", info.sender) + .add_attribute("book_id", book_id.to_string()) + .add_attribute("tick_id", tick_id.to_string()) + .add_attribute("order_id", order_id.to_string()) + .add_submessage(refund_msg)) } pub fn place_market( diff --git a/contracts/orderbook/src/tests/test_order.rs b/contracts/orderbook/src/tests/test_order.rs index 465d573..c0da68b 100644 --- a/contracts/orderbook/src/tests/test_order.rs +++ b/contracts/orderbook/src/tests/test_order.rs @@ -2,9 +2,21 @@ use crate::error::ContractError; use crate::order::*; use crate::orderbook::*; use crate::state::*; +use crate::types::LimitOrder; use crate::types::OrderDirection; +use crate::types::REPLY_ID_REFUND; use cosmwasm_std::testing::{mock_dependencies_with_balances, mock_env, mock_info}; +use cosmwasm_std::BankMsg; +use cosmwasm_std::Coin; +use cosmwasm_std::Empty; +use cosmwasm_std::SubMsg; use cosmwasm_std::{coin, Addr, Uint128}; +use cw_utils::PaymentError; + +#[allow(clippy::uninlined_format_args)] +fn format_test_name(name: &str) -> String { + format!("\n\nTest case failed: {}\n", name) +} struct PlaceLimitTestCase { name: &'static str, @@ -16,11 +28,6 @@ struct PlaceLimitTestCase { expected_error: Option, } -#[allow(clippy::uninlined_format_args)] -fn format_test_name(name: &str) -> String { - format!("\n\nTest case failed: {}\n", name) -} - #[test] fn test_place_limit() { let valid_book_id = 0; @@ -275,3 +282,223 @@ fn test_place_limit() { ); } } + +struct CancelLimitTestCase { + name: &'static str, + book_id: u64, + tick_id: i64, + order_id: u64, + order_direction: OrderDirection, + quantity: Uint128, + place_order: bool, + expected_error: Option, + owner: &'static str, + sender: Option<&'static str>, + sent: Vec, +} + +#[test] +fn test_cancel_limit() { + let valid_book_id = 0; + let test_cases = vec![ + CancelLimitTestCase { + name: "valid order cancel", + book_id: valid_book_id, + tick_id: 1, + order_id: 0, + order_direction: OrderDirection::Ask, + quantity: Uint128::from(100u128), + place_order: true, + expected_error: None, + owner: "creator", + sender: None, + sent: vec![], + }, + CancelLimitTestCase { + name: "sent funds accidentally", + book_id: valid_book_id, + tick_id: 1, + order_id: 0, + order_direction: OrderDirection::Ask, + quantity: Uint128::from(100u128), + place_order: true, + expected_error: Some(ContractError::PaymentError(PaymentError::NonPayable {})), + owner: "creator", + sender: None, + sent: vec![coin(100, "quote")], + }, + CancelLimitTestCase { + name: "unauthorized cancel (not owner)", + book_id: valid_book_id, + tick_id: 1, + order_id: 0, + order_direction: OrderDirection::Ask, + quantity: Uint128::from(100u128), + place_order: true, + expected_error: Some(ContractError::Unauthorized {}), + owner: "creator", + sender: Some("malicious_user"), + sent: vec![], + }, + CancelLimitTestCase { + name: "order not found", + book_id: valid_book_id, + tick_id: 1, + order_id: 0, + order_direction: OrderDirection::Ask, + quantity: Uint128::from(100u128), + place_order: false, + expected_error: Some(ContractError::OrderNotFound { + book_id: valid_book_id, + tick_id: 1, + order_id: 0, + }), + owner: "creator", + sender: None, + sent: vec![], + }, + ]; + + for test in test_cases { + // --- Setup --- + + // Create a mock environment and info + let balances = [(test.owner, test.sent.as_slice())]; + let mut deps = mock_dependencies_with_balances(&balances); + let env = mock_env(); + let info = mock_info(test.sender.unwrap_or(test.owner), test.sent.as_slice()); + + // Create an orderbook to operate on + let quote_denom = "quote".to_string(); + let base_denom = "base".to_string(); + create_orderbook( + deps.as_mut(), + env.clone(), + info.clone(), + quote_denom.clone(), + base_denom.clone(), + ) + .unwrap(); + + if test.place_order { + orders() + .save( + deps.as_mut().storage, + &(test.book_id, test.tick_id, test.order_id), + &LimitOrder::new( + test.book_id, + test.tick_id, + test.order_id, + test.order_direction, + Addr::unchecked(test.owner), + test.quantity, + ), + ) + .unwrap(); + } + + // --- System under test --- + + let response = cancel_limit( + deps.as_mut(), + env.clone(), + info.clone(), + test.book_id, + test.tick_id, + test.order_id, + ); + + // --- Assertions --- + + // Error case assertions if applicable + if let Some(expected_error) = &test.expected_error { + assert_eq!( + response.unwrap_err(), + *expected_error, + "{}", + format_test_name(test.name) + ); + + // Verify that the order was not put in state + let order_result = orders() + .may_load(&deps.storage, &(test.book_id, test.tick_id, test.order_id)) + .unwrap(); + assert!( + order_result.is_some() == test.place_order, + "{}", + format_test_name(test.name) + ); + continue; + } + + // Assert no error and retrieve response contents + let response = response.unwrap(); + let refund_denom = match test.order_direction { + OrderDirection::Bid => quote_denom.clone(), + OrderDirection::Ask => base_denom.clone(), + }; + let expected_refund_msg: SubMsg = SubMsg::reply_on_error( + BankMsg::Send { + to_address: test.owner.to_string(), + amount: vec![coin(test.quantity.u128(), refund_denom)], + }, + REPLY_ID_REFUND, + ); + + // Assertions on the response for a valid order + assert_eq!( + response.attributes[0], + ("method", "cancelLimit"), + "{}", + format_test_name(test.name) + ); + assert_eq!( + response.attributes[1], + ("owner", test.owner), + "{}", + format_test_name(test.name) + ); + assert_eq!( + response.attributes[2], + ("book_id", test.book_id.to_string()), + "{}", + format_test_name(test.name) + ); + assert_eq!( + response.attributes[3], + ("tick_id", test.tick_id.to_string()), + "{}", + format_test_name(test.name) + ); + assert_eq!( + response.attributes[4], + ("order_id", test.order_id.to_string()), + "{}", + format_test_name(test.name) + ); + assert_eq!( + response.messages.len(), + 1, + "{}", + format_test_name(test.name) + ); + assert_eq!( + response.messages[0], + expected_refund_msg, + "{}", + format_test_name(test.name) + ); + + // Retrieve the order from storage to verify it was saved correctly + let expected_order_id = 0; + let order = orders() + .may_load( + &deps.storage, + &(test.book_id, test.tick_id, expected_order_id), + ) + .unwrap(); + + // Verify the order's fields + assert!(order.is_none(), "{}", format_test_name(test.name)); + } +} diff --git a/contracts/orderbook/src/types/mod.rs b/contracts/orderbook/src/types/mod.rs index fe05fae..30cb5df 100644 --- a/contracts/orderbook/src/types/mod.rs +++ b/contracts/orderbook/src/types/mod.rs @@ -1,5 +1,7 @@ mod order; mod orderbook; +mod reply_id; pub use self::order::*; pub use self::orderbook::*; +pub use self::reply_id::*; diff --git a/contracts/orderbook/src/types/order.rs b/contracts/orderbook/src/types/order.rs index dea4f07..64c6ae1 100644 --- a/contracts/orderbook/src/types/order.rs +++ b/contracts/orderbook/src/types/order.rs @@ -38,7 +38,6 @@ impl LimitOrder { } } -// TODO: Unnecessary if finite queries not required /// Defines the different way an owners orders can be filtered, all enums filter by owner with each getting more finite #[derive(Clone)] pub enum FilterOwnerOrders { diff --git a/contracts/orderbook/src/types/orderbook.rs b/contracts/orderbook/src/types/orderbook.rs index 86d53ea..c9482d9 100644 --- a/contracts/orderbook/src/types/orderbook.rs +++ b/contracts/orderbook/src/types/orderbook.rs @@ -1,5 +1,7 @@ use cosmwasm_schema::cw_serde; +use super::OrderDirection; + #[cw_serde] pub struct Orderbook { pub book_id: u64, @@ -11,4 +13,33 @@ pub struct Orderbook { pub current_tick: i64, pub next_bid_tick: i64, pub next_ask_tick: i64, -} \ No newline at end of file +} + +impl Orderbook { + pub fn new( + book_id: u64, + quote_denom: String, + base_denom: String, + current_tick: i64, + next_bid_tick: i64, + next_ask_tick: i64, + ) -> Self { + Orderbook { + book_id, + quote_denom, + base_denom, + current_tick, + next_bid_tick, + next_ask_tick, + } + } + + /// Get the expected denomination for a given order direction. + #[inline] + pub fn get_expected_denom_for_direction(&self, order_direction: &OrderDirection) -> String { + match order_direction { + OrderDirection::Bid => self.quote_denom.clone(), + OrderDirection::Ask => self.base_denom.clone(), + } + } +} diff --git a/contracts/orderbook/src/types/reply_id.rs b/contracts/orderbook/src/types/reply_id.rs new file mode 100644 index 0000000..9168d2c --- /dev/null +++ b/contracts/orderbook/src/types/reply_id.rs @@ -0,0 +1 @@ +pub const REPLY_ID_REFUND: u64 = 1;