solana: clean up close fast market order

Author: a5-pickle

solana/programs/matching-engine/src/fallback/processor/close_fast_market_order.rs

@@ -1,94 +1,81 @@
-use crate::error::MatchingEngineError;
-use crate::state::FastMarketOrder;
 use anchor_lang::prelude::*;
 use solana_program::instruction::Instruction;
-use solana_program::program_error::ProgramError;
 
-use super::helpers::require_min_account_infos_len;
-use super::FallbackMatchingEngineInstruction;
+use crate::{error::MatchingEngineError, state::FastMarketOrder};
 
 pub struct CloseFastMarketOrderAccounts<'ix> {
-    /// The fast market order account created from the initialize fast market order instruction
+    /// The fast market order account to be closed.
     pub fast_market_order: &'ix Pubkey,
-    /// The account that will receive the refund. CHECK: Must be a signer.
-    /// CHECK: Must match the close account refund recipient in the fast market order account
+    /// The account that will receive rent from the fast market order account.
+    /// This account is the only authority that can close the fast market order.
+    /// TODO: Rename to "refund_recipient".
     pub close_account_refund_recipient: &'ix Pubkey,
 }
 
-impl<'ix> CloseFastMarketOrderAccounts<'ix> {
-    pub fn to_account_metas(&self) -> Vec<AccountMeta> {
-        vec![
-            AccountMeta::new(*self.fast_market_order, false),
-            AccountMeta::new(*self.close_account_refund_recipient, true),
-        ]
-    }
-}
-
+/// Closes the fast market order and transfers the lamports from the fast market
+/// order to its refund recipient.
 pub struct CloseFastMarketOrder<'ix> {
     pub program_id: &'ix Pubkey,
     pub accounts: CloseFastMarketOrderAccounts<'ix>,
 }
 
 impl CloseFastMarketOrder<'_> {
     pub fn instruction(&self) -> Instruction {
+        let CloseFastMarketOrderAccounts {
+            fast_market_order,
+            close_account_refund_recipient: refund_recipient,
+        } = self.accounts;
+
         Instruction {
             program_id: *self.program_id,
-            accounts: self.accounts.to_account_metas(),
-            data: FallbackMatchingEngineInstruction::CloseFastMarketOrder.to_vec(),
+            accounts: vec![
+                AccountMeta::new(*fast_market_order, false),
+                AccountMeta::new(*refund_recipient, true),
+            ],
+            data: super::FallbackMatchingEngineInstruction::CloseFastMarketOrder.to_vec(),
         }
     }
 }
 
-/// Closes the fast market order and transfers the lamports from the fast market order to the close account refund recipient
-///
-/// # Arguments
-///
-/// * `accounts` - The accounts of the fast market order and the close account refund recipient
-///
-/// # Returns
-///
-/// Result<()>
 pub fn close_fast_market_order(accounts: &[AccountInfo]) -> Result<()> {
-    require_min_account_infos_len(accounts, 2)?;
+    super::helpers::require_min_account_infos_len(accounts, 2)?;
 
-    let fast_market_order = &accounts[0];
-    let close_account_refund_recipient = &accounts[1];
+    // We need to check the refund recipient account against what we know as the
+    // refund recipient encoded in the fast market order account.
+    let fast_market_order_info = &accounts[0];
+    let refund_recipient_info = &accounts[1];
 
-    // Check that the close_account_refund_recipient is a signer, otherwise someone might call this and steal the lamports
-    if !close_account_refund_recipient.is_signer {
-        msg!("Refund recipient (account #2) is not a signer");
-        return Err(ProgramError::InvalidAccountData.into());
-    }
-    let fast_market_order_data = &fast_market_order.data.borrow()[..];
-    let fast_market_order_deserialized = FastMarketOrder::try_read(fast_market_order_data)?;
-    // Check that the fast_market_order is owned by the close_account_refund_recipient
-    if fast_market_order_deserialized.close_account_refund_recipient
-        != close_account_refund_recipient.key()
-    {
+    let fast_market_order_data = &fast_market_order_info.data.borrow()[..];
+
+    // NOTE: We do not need to verify that the owner of this account is this
+    // program because the lamport transfer will fail otherwise.
+    let fast_market_order = FastMarketOrder::try_read(fast_market_order_data)?;
+
+    // Check that the refund recipient provided in this instruction is the one
+    // encoded in the fast market order account.
+    let expected_refund_recipient_key = fast_market_order.close_account_refund_recipient;
+    if refund_recipient_info.key != &expected_refund_recipient_key {
         return Err(MatchingEngineError::MismatchingCloseAccountRefundRecipient.into()).map_err(
-            |e: Error| {
-                e.with_pubkeys((
-                    fast_market_order_deserialized.close_account_refund_recipient,
-                    close_account_refund_recipient.key(),
-                ))
-            },
+            |e: Error| e.with_pubkeys((*refund_recipient_info.key, expected_refund_recipient_key)),
         );
     }
 
-    // First, get the current lamports value
-    let current_recipient_lamports = **close_account_refund_recipient.lamports.borrow();
-
-    // Then, get the fast market order lamports
-    let mut fast_market_order_lamports = fast_market_order.lamports.borrow_mut();
+    // This refund recipient must sign to invoke this instruction. He is the
+    // only authority allowed to perform this action.
+    if !refund_recipient_info.is_signer {
+        return Err(ErrorCode::AccountNotSigner.into())
+            .map_err(|e: Error| e.with_account_name("refund_recipient"));
+    }
 
-    // Calculate the new amount
-    let new_amount = current_recipient_lamports.saturating_add(**fast_market_order_lamports);
+    let mut fast_market_order_info_lamports = fast_market_order_info.lamports.borrow_mut();
 
-    // Now update the recipient's lamports
-    **close_account_refund_recipient.lamports.borrow_mut() = new_amount;
+    // Move lamports to the refund recipient.
+    let mut recipient_info_lamports = refund_recipient_info.lamports.borrow_mut();
+    **recipient_info_lamports =
+        recipient_info_lamports.saturating_add(**fast_market_order_info_lamports);
 
-    // Zero out the fast market order lamports
-    **fast_market_order_lamports = 0;
+    // Zero out the fast market order lamports.
+    **fast_market_order_info_lamports = 0;
 
     Ok(())
 }