dex: Close open orders accounts (#112)

* dex: Close open orders accounts

* Zero out open orders account data on close

* Fix length assertion

* Remove token account parsing

* Remove rent from close ix

* Add close open orders to whole shebang

* Add close open orders to fuzz test

* Fix fuzz

* Update u128

* Cancel orders in whole shebang

* Update docker iamge

* travis: Update solana cli

* json compact output

* Add error logging and move free slot bits check

* Add Closed AccountFlag

* Replace .bits() with as u64 cast

* Skip errors instead of removing owner from set

* Fix derive ordering

* Skip WrongOrdersAccount error if account is closed

Co-authored-by: Sebastian Conybeare <sebastian@alameda-research.com>

.travis.yml

@@ -19,7 +19,7 @@ _localnet: &localnet
     - 14
   before_script:
     - sudo apt-get install -y pkg-config build-essential libudev-dev
-    - sh -c "$(curl -sSfL https://release.solana.com/v1.5.5/install)"
+    - sh -c "$(curl -sSfL https://release.solana.com/v1.6.9/install)"
     - export PATH="/home/travis/.local/share/solana/install/active_release/bin:$PATH"
 
 jobs:

dex/crank/src/lib.rs

@@ -40,7 +40,11 @@ use serum_common::client::rpc::{
 };
 use serum_common::client::Cluster;
 use serum_context::Context;
-use serum_dex::instruction::{MarketInstruction, NewOrderInstructionV3, SelfTradeBehavior};
+use serum_dex::instruction::{
+    cancel_order_by_client_order_id as cancel_order_by_client_order_id_ix,
+    close_open_orders as close_open_orders_ix, MarketInstruction, NewOrderInstructionV3,
+    SelfTradeBehavior,
+};
 use serum_dex::matching::{OrderType, Side};
 use serum_dex::state::gen_vault_signer_key;
 use serum_dex::state::Event;
@@ -890,6 +894,16 @@ fn whole_shebang(client: &RpcClient, program_id: &Pubkey, payer: &Keypair) -> Re
         },
     )?;
 
+    // Cancel the open order so that we can close it later.
+    cancel_order_by_client_order_id(
+        client,
+        program_id,
+        payer,
+        &market_keys,
+        &orders.unwrap(),
+        985982,
+    )?;
+
     debug_println!("Ask account: {}", orders.unwrap());
 
     debug_println!("Consuming events in 15s ...");
@@ -912,6 +926,74 @@ fn whole_shebang(client: &RpcClient, program_id: &Pubkey, payer: &Keypair) -> Re
         &coin_wallet.pubkey(),
         &pc_wallet.pubkey(),
     )?;
+    close_open_orders(
+        client,
+        program_id,
+        payer,
+        &market_keys,
+        orders.as_ref().unwrap(),
+    )?;
+    Ok(())
+}
+
+pub fn cancel_order_by_client_order_id(
+    client: &RpcClient,
+    program_id: &Pubkey,
+    owner: &Keypair,
+    state: &MarketPubkeys,
+    orders: &Pubkey,
+    client_order_id: u64,
+) -> Result<()> {
+    let ixs = &[cancel_order_by_client_order_id_ix(
+        program_id,
+        &state.market,
+        &state.bids,
+        &state.asks,
+        orders,
+        &owner.pubkey(),
+        &state.event_q,
+        client_order_id,
+    )?];
+    let (recent_hash, _fee_calc) = client.get_recent_blockhash()?;
+    let txn = Transaction::new_signed_with_payer(ixs, Some(&owner.pubkey()), &[owner], recent_hash);
+
+    debug_println!("Canceling order by client order id instruction ...");
+    let result = simulate_transaction(client, &txn, true, CommitmentConfig::confirmed())?;
+    if let Some(e) = result.value.err {
+        debug_println!("{:#?}", result.value.logs);
+        return Err(format_err!("simulate_transaction error: {:?}", e));
+    }
+
+    send_txn(client, &txn, false)?;
+    Ok(())
+}
+
+pub fn close_open_orders(
+    client: &RpcClient,
+    program_id: &Pubkey,
+    owner: &Keypair,
+    state: &MarketPubkeys,
+    orders: &Pubkey,
+) -> Result<()> {
+    debug_println!("Closing open orders...");
+    let ixs = &[close_open_orders_ix(
+        program_id,
+        orders,
+        &owner.pubkey(),
+        &owner.pubkey(),
+        &state.market,
+    )?];
+    let (recent_hash, _fee_calc) = client.get_recent_blockhash()?;
+    let txn = Transaction::new_signed_with_payer(ixs, Some(&owner.pubkey()), &[owner], recent_hash);
+
+    debug_println!("Simulating close open orders instruction ...");
+    let result = simulate_transaction(client, &txn, true, CommitmentConfig::confirmed())?;
+    if let Some(e) = result.value.err {
+        debug_println!("{:#?}", result.value.logs);
+        return Err(format_err!("simulate_transaction error: {:?}", e));
+    }
+
+    send_txn(client, &txn, false)?;
     Ok(())
 }
 

dex/fuzz/fuzz_targets/multiple_orders.rs

@@ -22,8 +22,8 @@ use serum_dex::matching::Side;
 use serum_dex::state::{strip_header, MarketState, OpenOrders, ToAlignedBytes};
 use serum_dex_fuzz::{
     get_token_account_balance, new_dex_owned_account_with_lamports, new_sol_account,
-    new_token_account, process_instruction, setup_market, MarketAccounts, COIN_LOT_SIZE,
-    PC_LOT_SIZE, NoSolLoggingStubs,
+    new_token_account, process_instruction, setup_market, MarketAccounts, NoSolLoggingStubs,
+    COIN_LOT_SIZE, PC_LOT_SIZE,
 };
 
 #[derive(Debug, Arbitrary, Clone)]
@@ -45,6 +45,9 @@ enum Action {
     ConsumeEvents(u16),
     SettleFunds(OwnerId, Option<ReferrerId>),
     SweepFees,
+    CloseOpenOrders {
+        owner_id: OwnerId,
+    },
 }
 
 #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, PartialOrd, Ord)]
@@ -84,6 +87,7 @@ struct Owner<'bump> {
     orders_account: AccountInfo<'bump>,
     coin_account: AccountInfo<'bump>,
     pc_account: AccountInfo<'bump>,
+    closed_open_orders: bool,
 }
 
 const INITIAL_COIN_BALANCE: u64 = 1_000_000_000;
@@ -117,6 +121,7 @@ impl<'bump> Owner<'bump> {
             orders_account,
             coin_account,
             pc_account,
+            closed_open_orders: false,
         }
     }
 
@@ -129,8 +134,13 @@ impl<'bump> Owner<'bump> {
 impl<'bump> Referrer<'bump> {
     fn new(market_accounts: &MarketAccounts<'bump>, bump: &'bump Bump) -> Self {
         let signer_account = new_sol_account(10, &bump);
-        let pc_account =
-            new_token_account(market_accounts.pc_mint.key, signer_account.key, 0, &bump, market_accounts.rent());
+        let pc_account = new_token_account(
+            market_accounts.pc_mint.key,
+            signer_account.key,
+            0,
+            &bump,
+            market_accounts.rent(),
+        );
         Self { pc_account }
     }
 }
@@ -220,6 +230,9 @@ fn run_actions(actions: Vec<Action>) {
             Err(e) if e == DexErrorCode::RentNotProvided.into() => {
                 continue;
             }
+            Err(e) if e == DexErrorCode::WrongOrdersAccount.into() && owner.closed_open_orders => {
+                continue
+            }
             _ => load_orders_result.unwrap(),
         };
         assert_eq!(identity(open_orders.free_slot_bits), !0);
@@ -371,8 +384,11 @@ fn run_action<'bump>(
             .map_err(|e| match e {
                 DexError::ErrorCode(DexErrorCode::InsufficientFunds) => {}
                 DexError::ErrorCode(DexErrorCode::RequestQueueFull) => {}
+                DexError::ErrorCode(DexErrorCode::OrdersNotRentExempt) => {}
                 DexError::ErrorCode(DexErrorCode::WouldSelfTrade)
                     if instruction.self_trade_behavior == SelfTradeBehavior::AbortTransaction => {}
+                DexError::ErrorCode(DexErrorCode::WrongOrdersAccount)
+                    if owner.closed_open_orders => {}
                 e => Err(e).unwrap(),
             })
             .ok();
@@ -433,7 +449,10 @@ fn run_action<'bump>(
             .map_err(|e| match e {
                 DexError::ErrorCode(DexErrorCode::OrderNotFound) => {}
                 DexError::ErrorCode(DexErrorCode::RequestQueueFull) => {}
+                DexError::ErrorCode(DexErrorCode::RentNotProvided) => {}
                 DexError::ErrorCode(DexErrorCode::ClientOrderIdIsZero) if expects_zero_id => {}
+                DexError::ErrorCode(DexErrorCode::WrongOrdersAccount)
+                    if owner.closed_open_orders => {}
                 e => Err(e).unwrap(),
             })
             .map(|_| {
@@ -476,6 +495,8 @@ fn run_action<'bump>(
                 DexError::ErrorCode(DexErrorCode::OrderNotFound) => {}
                 DexError::ErrorCode(DexErrorCode::OrderNotYours) => {}
                 DexError::ErrorCode(DexErrorCode::RentNotProvided) => {}
+                DexError::ErrorCode(DexErrorCode::WrongOrdersAccount)
+                    if owner.closed_open_orders => {}
                 e => Err(e).unwrap(),
             })
             .ok();
@@ -540,7 +561,13 @@ fn run_action<'bump>(
                 &accounts,
                 &MarketInstruction::SettleFunds.pack(),
             )
-            .unwrap();
+            .map_err(|e| match e {
+                DexError::ErrorCode(DexErrorCode::RentNotProvided) => {}
+                DexError::ErrorCode(DexErrorCode::WrongOrdersAccount)
+                    if owner.closed_open_orders => {}
+                e => Err(e).unwrap(),
+            })
+            .ok();
         }
 
         Action::SweepFees => {
@@ -558,6 +585,33 @@ fn run_action<'bump>(
             )
             .unwrap();
         }
+        Action::CloseOpenOrders { owner_id } => {
+            let owner = owners
+                .entry(owner_id)
+                .or_insert_with(|| Owner::new(&market_accounts, &bump));
+            process_instruction(
+                market_accounts.market.owner,
+                &[
+                    owner.orders_account.clone(),
+                    owner.signer_account.clone(),
+                    owner.signer_account.clone(), // SOL destination.
+                    market_accounts.market.clone(),
+                ],
+                &MarketInstruction::CloseOpenOrders.pack(),
+            )
+            .map_err(|e| match e {
+                DexError::ErrorCode(DexErrorCode::TooManyOpenOrders) => {}
+                DexError::ErrorCode(DexErrorCode::RentNotProvided) => {}
+                DexError::ErrorCode(DexErrorCode::WrongOrdersAccount)
+                    if owner.closed_open_orders => {}
+                e => Err(e).unwrap(),
+            })
+            .map(|r| {
+                owner.closed_open_orders = true;
+                r
+            })
+            .ok();
+        }
     };
 
     if *VERBOSE >= 2 {

dex/src/instruction.rs

@@ -6,6 +6,7 @@ use serde::{Deserialize, Serialize};
 use solana_program::{
     instruction::{AccountMeta, Instruction},
     pubkey::Pubkey,
+    sysvar::rent,
 };
 use std::{cmp::max, convert::TryInto};
 
@@ -432,6 +433,11 @@ pub enum MarketInstruction {
     /// 3. `[writable]` OpenOrders
     /// 4. `[]`
     SendTake(SendTakeInstruction),
+    /// 0. `[writable]` OpenOrders
+    /// 1. `[signer]` the OpenOrders owner
+    /// 2. `[writable]` the destination account to send rent exemption SOL to
+    /// 3. `[]` market
+    CloseOpenOrders,
 }
 
 impl MarketInstruction {
@@ -523,6 +529,7 @@ impl MarketInstruction {
                 let data_arr = array_ref![data, 0, 46];
                 SendTakeInstruction::unpack(data_arr)?
             }),
+            (14, 0) => MarketInstruction::CloseOpenOrders,
             _ => return None,
         })
     }
@@ -623,7 +630,7 @@ pub fn new_order(
     client_order_id: u64,
     self_trade_behavior: SelfTradeBehavior,
     limit: u16,
-    max_native_pc_qty_including_fees: NonZeroU64
+    max_native_pc_qty_including_fees: NonZeroU64,
 ) -> Result<Instruction, DexError> {
     let data = MarketInstruction::NewOrderV3(NewOrderInstructionV3 {
         side,
@@ -633,7 +640,7 @@ pub fn new_order(
         client_order_id,
         self_trade_behavior,
         limit,
-        max_native_pc_qty_including_fees
+        max_native_pc_qty_including_fees,
     })
     .pack();
     let mut accounts = vec![
@@ -726,11 +733,7 @@ pub fn cancel_order(
     side: Side,
     order_id: u128,
 ) -> Result<Instruction, DexError> {
-    let data = MarketInstruction::CancelOrderV2(CancelOrderInstructionV2 {
-        side,
-        order_id,
-    })
-    .pack();
+    let data = MarketInstruction::CancelOrderV2(CancelOrderInstructionV2 { side, order_id }).pack();
     let accounts: Vec<AccountMeta> = vec![
         AccountMeta::new_readonly(*market, false),
         AccountMeta::new_readonly(*market_bids, false),
@@ -849,6 +852,27 @@ pub fn sweep_fees(
     })
 }
 
+pub fn close_open_orders(
+    program_id: &Pubkey,
+    open_orders: &Pubkey,
+    owner: &Pubkey,
+    destination: &Pubkey,
+    market: &Pubkey,
+) -> Result<Instruction, DexError> {
+    let data = MarketInstruction::CloseOpenOrders.pack();
+    let accounts: Vec<AccountMeta> = vec![
+        AccountMeta::new(*open_orders, false),
+        AccountMeta::new_readonly(*owner, true),
+        AccountMeta::new(*destination, false),
+        AccountMeta::new_readonly(*market, false),
+    ];
+    Ok(Instruction {
+        program_id: *program_id,
+        data,
+        accounts,
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

dex/src/state.rs

@@ -29,9 +29,9 @@ use crate::{
     error::{DexErrorCode, DexResult, SourceFileId},
     fees::{self, FeeTier},
     instruction::{
-        disable_authority, fee_sweeper, msrm_token, srm_token,
-        CancelOrderInstructionV2, InitializeMarketInstruction, MarketInstruction,
-        NewOrderInstructionV3, SelfTradeBehavior, SendTakeInstruction,
+        disable_authority, fee_sweeper, msrm_token, srm_token, CancelOrderInstructionV2,
+        InitializeMarketInstruction, MarketInstruction, NewOrderInstructionV3, SelfTradeBehavior,
+        SendTakeInstruction,
     },
     matching::{OrderBookState, OrderType, RequestProceeds, Side},
 };
@@ -60,6 +60,7 @@ pub enum AccountFlag {
     Bids = 1u64 << 5,
     Asks = 1u64 << 6,
     Disabled = 1u64 << 7,
+    Closed = 1u64 << 8,
 }
 
 #[derive(Copy, Clone)]
@@ -376,9 +377,9 @@ impl MarketState {
     }
 }
 
-#[cfg_attr(feature = "fuzz", derive(Debug))]
 #[repr(packed)]
 #[derive(Copy, Clone)]
+#[cfg_attr(feature = "fuzz", derive(Debug))]
 pub struct OpenOrders {
     pub account_flags: u64, // Initialized, OpenOrders
     pub market: [u64; 4],
@@ -1795,7 +1796,6 @@ pub(crate) mod account_parser {
         }
     }
 
-
     pub struct CancelOrderByClientIdV2Args<'a, 'b: 'a> {
         pub client_order_id: NonZeroU64,
         pub open_orders_address: [u64; 4],
@@ -1994,6 +1994,60 @@ pub(crate) mod account_parser {
             f(args)
         }
     }
+
+    pub struct CloseOpenOrdersArgs<'a, 'b: 'a> {
+        pub open_orders: &'a mut OpenOrders,
+        pub open_orders_acc: &'a AccountInfo<'b>,
+        pub dest_acc: &'a AccountInfo<'b>,
+    }
+
+    impl<'a, 'b: 'a> CloseOpenOrdersArgs<'a, 'b> {
+        pub fn with_parsed_args<T>(
+            program_id: &'a Pubkey,
+            accounts: &'a [AccountInfo<'b>],
+            f: impl FnOnce(CloseOpenOrdersArgs) -> DexResult<T>,
+        ) -> DexResult<T> {
+            // Parse accounts.
+            check_assert_eq!(accounts.len(), 4)?;
+            #[rustfmt::skip]
+            let &[
+                ref open_orders_acc,
+                ref owner_acc,
+                ref dest_acc,
+                ref market_acc,
+            ] = array_ref![accounts, 0, 4];
+
+            // Validate the accounts given are valid.
+            let owner = SignerAccount::new(owner_acc)?;
+            let market: RefMut<'a, MarketState> = MarketState::load(market_acc, program_id)?;
+            let mut open_orders =
+                market.load_orders_mut(open_orders_acc, Some(owner.inner()), program_id, None)?;
+
+            // Only accounts with no funds associated with them can be closed.
+            if open_orders.free_slot_bits != std::u128::MAX {
+                return Err(DexErrorCode::TooManyOpenOrders.into());
+            }
+            if open_orders.native_coin_total != 0 {
+                solana_program::msg!(
+                    "Base currency total must be zero to close the open orders account"
+                );
+                return Err(DexErrorCode::TooManyOpenOrders.into());
+            }
+            if open_orders.native_pc_total != 0 {
+                solana_program::msg!(
+                    "Quote currency total must be zero to close the open orders account"
+                );
+                return Err(DexErrorCode::TooManyOpenOrders.into());
+            }
+
+            // Invoke processor.
+            f(CloseOpenOrdersArgs {
+                open_orders: open_orders.deref_mut(),
+                open_orders_acc,
+                dest_acc,
+            })
+        }
+    }
 }
 
 #[inline]
@@ -2033,8 +2087,7 @@ impl State {
                     Self::process_new_order_v3,
                 )?
             }
-            MarketInstruction::MatchOrders(_limit) => {
-            }
+            MarketInstruction::MatchOrders(_limit) => {}
             MarketInstruction::ConsumeEvents(limit) => {
                 account_parser::ConsumeEventsArgs::with_parsed_args(
                     program_id,
@@ -2090,6 +2143,13 @@ impl State {
                     Self::process_send_take,
                 )?
             }
+            MarketInstruction::CloseOpenOrders => {
+                account_parser::CloseOpenOrdersArgs::with_parsed_args(
+                    program_id,
+                    accounts,
+                    Self::process_close_open_orders,
+                )?
+            }
         };
         Ok(())
     }
@@ -2099,6 +2159,27 @@ impl State {
         unimplemented!()
     }
 
+    fn process_close_open_orders(args: account_parser::CloseOpenOrdersArgs) -> DexResult {
+        let account_parser::CloseOpenOrdersArgs {
+            open_orders,
+            open_orders_acc,
+            dest_acc,
+        } = args;
+
+        // Transfer all lamports to the destination.
+        let dest_starting_lamports = dest_acc.lamports();
+        **dest_acc.lamports.borrow_mut() = dest_starting_lamports
+            .checked_add(open_orders_acc.lamports())
+            .unwrap();
+        **open_orders_acc.lamports.borrow_mut() = 0;
+
+        // Mark the account as closed to prevent it from being used before
+        // garbage collection.
+        open_orders.account_flags = AccountFlag::Closed as u64;
+
+        Ok(())
+    }
+
     #[cfg(feature = "program")]
     fn process_settle_funds(args: account_parser::SettleFundsArgs) -> DexResult {
         let account_parser::SettleFundsArgs {

docker/development/Dockerfile

@@ -3,7 +3,7 @@ FROM ubuntu:18.04
 ARG DEBIAN_FRONTEND=noninteractive
 
 ARG SOLANA_CHANNEL=v1.2.17
-ARG SOLANA_CLI=v1.5.5
+ARG SOLANA_CLI=v1.6.9
 
 ENV HOME="/root"
 ENV PATH="${HOME}/.cargo/bin:${PATH}"

scripts/travis/dex-tests.sh

@@ -42,7 +42,7 @@ dex_whole_shebang() {
     #
     # Deploy the program.
     #
-    local dex_program_id="$(solana deploy --url ${CLUSTER_URL} dex/target/bpfel-unknown-unknown/release/serum_dex.so | jq .ProgramId -r)"
+    local dex_program_id="$(solana deploy --output json-compact --url ${CLUSTER_URL} dex/target/bpfel-unknown-unknown/release/serum_dex.so | jq .programId -r)"
     #
     # Run the whole-shebang.
     #