[Orderbook] Cleanup client order id mapping when taking ready time based order (#18417)

Author: sitalkedia

aptos-move/framework/aptos-experimental/doc/single_order_book.md

@@ -1255,12 +1255,21 @@ Removes and returns the orders that are ready to be executed based on the time c
     self: &<b>mut</b> <a href="single_order_book.md#0x7_single_order_book_SingleOrderBook">SingleOrderBook</a>&lt;M&gt;, order_limit: u64
 ): <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;SingleOrder&lt;M&gt;&gt; {
     <b>let</b> self_orders = &<b>mut</b> self.orders;
+    <b>let</b> self_client_order_ids = &<b>mut</b> self.client_order_ids;
     <b>let</b> order_ids = self.pending_orders.<a href="single_order_book.md#0x7_single_order_book_take_ready_time_based_orders">take_ready_time_based_orders</a>(order_limit);
     <b>let</b> orders = <a href="../../aptos-framework/../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_empty">vector::empty</a>();
 
     order_ids.for_each(|order_id| {
         <b>let</b> order_with_state = self_orders.remove(&order_id);
         <b>let</b> (order, _) = order_with_state.destroy_order_from_state();
+        <b>let</b> client_order_id = order.get_client_order_id();
+        <b>if</b> (client_order_id.is_some()) {
+            self_client_order_ids.remove(
+                &new_account_client_order_id(
+                    order.get_account(), client_order_id.destroy_some()
+                )
+            );
+        };
         orders.push_back(order);
     });
     orders

aptos-move/framework/aptos-experimental/sources/trading/order_book/single_order_book.move

@@ -537,12 +537,21 @@ module aptos_experimental::single_order_book {
         self: &mut SingleOrderBook<M>, order_limit: u64
     ): vector<SingleOrder<M>> {
         let self_orders = &mut self.orders;
+        let self_client_order_ids = &mut self.client_order_ids;
         let order_ids = self.pending_orders.take_ready_time_based_orders(order_limit);
         let orders = vector::empty();
 
         order_ids.for_each(|order_id| {
             let order_with_state = self_orders.remove(&order_id);
             let (order, _) = order_with_state.destroy_order_from_state();
+            let client_order_id = order.get_client_order_id();
+            if (client_order_id.is_some()) {
+                self_client_order_ids.remove(
+                    &new_account_client_order_id(
+                        order.get_account(), client_order_id.destroy_some()
+                    )
+                );
+            };
             orders.push_back(order);
         });
         orders

aptos-move/framework/aptos-experimental/sources/trading/tests/order_book/order_book_client_order_id.move

@@ -2,7 +2,8 @@
 module aptos_experimental::order_book_client_order_id {
     use std::option;
     use std::signer;
-    use aptos_experimental::order_book_types::{new_order_id_type, price_move_up_condition};
+    use aptos_framework::timestamp;
+    use aptos_experimental::order_book_types::{new_order_id_type, price_move_up_condition, new_time_based_trigger_condition};
     use aptos_experimental::order_book_types::good_till_cancelled;
     use aptos_experimental::order_book::{new_single_order_request, destroy_order_book, set_up_test_with_id};
 
@@ -535,4 +536,51 @@ module aptos_experimental::order_book_client_order_id {
         order_book.destroy_order_book();
     }
 
+    #[test(user1 = @0x456, aptos = @0x1)]
+    public fun test_client_order_id_cleanup_for_time_based_pending_order(
+        user1: &signer,
+        aptos: &signer
+    ) {
+        // Setup timestamp for time-based triggers
+        timestamp::set_time_has_started_for_testing(aptos);
+        timestamp::update_global_time_for_test_secs(1000);
+
+        // Setup a basic order book
+        let order_book = set_up_test_with_id();
+        let user1_addr = signer::address_of(user1);
+        let client_order_id = std::string::utf8(b"time_order_1");
+        let order_id = new_order_id_type(1);
+
+        // Create an order request with client order ID and time-based trigger
+        let order_req =
+            new_single_order_request(
+                user1_addr,
+                order_id,
+                option::some(client_order_id),
+                1000, // price
+                100, // orig_size
+                100, // remaining_size
+                true, // is_bid
+                option::some(new_time_based_trigger_condition(1500)), // trigger_condition - time-based
+                good_till_cancelled(),
+                42 // metadata
+            );
+
+        // Place the maker order
+        order_book.place_maker_order(order_req);
+
+        // Advance time to trigger the condition
+        timestamp::update_global_time_for_test_secs(1600);
+
+        // This should remove the pending order from the pending orders map
+        order_book.take_ready_time_based_orders(1);
+
+        // Try cancelling the order - should return none since it was already removed from pending orders
+        let cancel_result =
+            order_book.try_cancel_single_order_with_client_order_id(user1_addr, client_order_id);
+        assert!(cancel_result.is_none());
+
+        order_book.destroy_order_book();
+    }
+
 }