From f80f1dfb0c56837026629588b9a0b9a8e2c6e335 Mon Sep 17 00:00:00 2001 From: aftermath2 Date: Sun, 16 Jun 2024 10:53:24 -0300 Subject: [PATCH 1/4] Add cancel_status to POST /api/order --- api/logics.py | 12 ++++- api/oas_schemas.py | 4 ++ api/serializers.py | 7 +++ api/views.py | 3 +- docs/assets/schemas/api-latest.yaml | 3 ++ frontend/src/components/TradeBox/index.tsx | 11 ++++- robosats/settings.py | 3 ++ tests/test_trade_pipeline.py | 51 ++++++++++++++++++++-- tests/utils/trade.py | 4 +- 9 files changed, 89 insertions(+), 9 deletions(-) diff --git a/api/logics.py b/api/logics.py index d710fe86..5b68242c 100644 --- a/api/logics.py +++ b/api/logics.py @@ -1024,7 +1024,17 @@ class Logics: return False, None @classmethod - def cancel_order(cls, order, user, state=None): + def cancel_order(cls, order, user, cancel_status=None): + # If cancel status is specified, do no cancel the order + # if it is not the correct one. + # This prevents the client from cancelling an order that + # recently changed status. + if cancel_status is not None: + if order.status != cancel_status: + return False, { + "bad_request": f"Current order status is {order.status}, not {cancel_status}." + } + # Do not change order status if an is in order # any of these status do_not_cancel = [ diff --git a/api/oas_schemas.py b/api/oas_schemas.py index 8d214f32..f2a7de0c 100644 --- a/api/oas_schemas.py +++ b/api/oas_schemas.py @@ -245,6 +245,10 @@ class OrderViewSchema: - `17` - Maker lost dispute - `18` - Taker lost dispute + The client can use `cancel_status` to cancel the order only + if it is in the specified status. The server will + return an error without cancelling the trade otherwise. + Note that there are penalties involved for cancelling a order mid-trade so use this action carefully: diff --git a/api/serializers.py b/api/serializers.py index aa583e02..7f88e7e6 100644 --- a/api/serializers.py +++ b/api/serializers.py @@ -642,6 +642,13 @@ class UpdateOrderSerializer(serializers.Serializer): mining_fee_rate = serializers.DecimalField( max_digits=6, decimal_places=3, allow_null=True, required=False, default=None ) + cancel_status = serializers.ChoiceField( + choices=Order.Status.choices, + allow_null=True, + allow_blank=True, + default=None, + help_text="Status the order should have for it to be cancelled.", + ) class ClaimRewardSerializer(serializers.Serializer): diff --git a/api/views.py b/api/views.py index d38385d0..d0dd53d9 100644 --- a/api/views.py +++ b/api/views.py @@ -540,6 +540,7 @@ class OrderView(viewsets.ViewSet): mining_fee_rate = serializer.data.get("mining_fee_rate") statement = serializer.data.get("statement") rating = serializer.data.get("rating") + cancel_status = serializer.data.get("cancel_status") # 1) If action is take, it is a taker request! if action == "take": @@ -571,7 +572,7 @@ class OrderView(viewsets.ViewSet): # 2) If action is cancel elif action == "cancel": - valid, context = Logics.cancel_order(order, request.user) + valid, context = Logics.cancel_order(order, request.user, cancel_status) if not valid: return Response(context, status.HTTP_400_BAD_REQUEST) diff --git a/docs/assets/schemas/api-latest.yaml b/docs/assets/schemas/api-latest.yaml index 1dd6d431..08575d94 100644 --- a/docs/assets/schemas/api-latest.yaml +++ b/docs/assets/schemas/api-latest.yaml @@ -1974,6 +1974,9 @@ components: format: decimal pattern: ^-?\d{0,3}(?:\.\d{0,3})?$ nullable: true + cancel_status: + allOf: + - $ref: '#/components/schemas/StatusEnum' required: - action Version: diff --git a/frontend/src/components/TradeBox/index.tsx b/frontend/src/components/TradeBox/index.tsx index 223e8db3..07003020 100644 --- a/frontend/src/components/TradeBox/index.tsx +++ b/frontend/src/components/TradeBox/index.tsx @@ -150,6 +150,7 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element => mining_fee_rate?: number; statement?: string; rating?: number; + cancel_status?: number; } const renewOrder = function (): void { @@ -188,6 +189,7 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element => mining_fee_rate, statement, rating, + cancel_status }: SubmitActionProps): void { const slot = garage.getSlot(); @@ -201,6 +203,7 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element => mining_fee_rate, statement, rating, + cancel_status }) .then((data: Order) => { setOpen(closeAll); @@ -222,8 +225,14 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element => }; const cancel = function (): void { + const order = garage.getSlot()?.activeOrder; + const noConfirmation = Boolean(order?.is_maker && [0, 1, 2].includes(order?.status)); + setLoadingButtons({ ...noLoadingButtons, cancel: true }); - submitAction({ action: 'cancel' }); + submitAction({ + action: 'cancel', + cancel_status: noConfirmation ? order?.status : undefined + }); }; const openDispute = function (): void { diff --git a/robosats/settings.py b/robosats/settings.py index 3104be8b..71c3c285 100644 --- a/robosats/settings.py +++ b/robosats/settings.py @@ -143,6 +143,9 @@ SPECTACULAR_SETTINGS = { } }, "REDOC_DIST": "SIDECAR", + "ENUM_NAME_OVERRIDES": { + "StatusEnum": "api.models.order.Order.Status", + } } diff --git a/tests/test_trade_pipeline.py b/tests/test_trade_pipeline.py index ea58ed45..82046363 100644 --- a/tests/test_trade_pipeline.py +++ b/tests/test_trade_pipeline.py @@ -1085,9 +1085,6 @@ class TradeTest(BaseAPITestCase): trade.cancel_order(trade.maker_index) data = trade.response.json() - self.assertEqual( - data["bad_request"], "This order has been cancelled by the maker" - ) trade.get_order(trade.taker_index) data = trade.response.json() @@ -1095,12 +1092,58 @@ class TradeTest(BaseAPITestCase): data["bad_request"], "This order has been cancelled by the maker" ) - trade.get_order(trade.third_index) + def test_cancel_order_cancel_status(self): + """ + Tests the cancellation of a public order using cancel_status. + """ + trade = Trade(self.client) + trade.publish_order() data = trade.response.json() + + self.assertEqual(trade.response.status_code, 200) + self.assertResponse(trade.response) + + self.assertEqual(data["status_message"], Order.Status(Order.Status.PUB).label) + + # Cancel order if the order status is public + trade.cancel_order(cancel_status=Order.Status.PUB) + + self.assertEqual(trade.response.status_code, 400) + self.assertResponse(trade.response) + self.assertEqual( data["bad_request"], "This order has been cancelled by the maker" ) + def test_cancel_order_different_cancel_status(self): + """ + Tests the cancellation of a paused order with a different cancel_status. + """ + trade = Trade(self.client) + trade.publish_order() + trade.pause_order() + data = trade.response.json() + + self.assertEqual(trade.response.status_code, 200) + self.assertResponse(trade.response) + + self.assertEqual(data["status_message"], Order.Status(Order.Status.PAU).label) + + # Try to cancel order if it is public + trade.cancel_order(cancel_status=Order.Status.PUB) + data = trade.response.json() + + self.assertEqual(trade.response.status_code, 400) + self.assertResponse(trade.response) + + self.assertEqual( + data["bad_request"], + f"Current order status is {Order.Status.PAU}, not {Order.Status.PUB}." + ) + + # Cancel order to avoid leaving pending HTLCs after a successful test + trade.cancel_order() + def test_collaborative_cancel_order_in_chat(self): """ Tests the collaborative cancellation of an order in the chat state diff --git a/tests/utils/trade.py b/tests/utils/trade.py index 1753495a..85b948eb 100644 --- a/tests/utils/trade.py +++ b/tests/utils/trade.py @@ -114,11 +114,11 @@ class Trade: self.response = self.client.get(path + params, **headers) @patch("api.tasks.send_notification.delay", send_notification) - def cancel_order(self, robot_index=1): + def cancel_order(self, robot_index=1, cancel_status=None): path = reverse("order") params = f"?order_id={self.order_id}" headers = self.get_robot_auth(robot_index) - body = {"action": "cancel"} + body = {"action": "cancel", "cancel_status": cancel_status} self.response = self.client.post(path + params, body, **headers) @patch("api.tasks.send_notification.delay", send_notification) From 0d932feedb442905c42eda56b75354570cf2b6f0 Mon Sep 17 00:00:00 2001 From: aftermath2 Date: Sun, 7 Jul 2024 10:40:57 -0300 Subject: [PATCH 2/4] Set cancel_status if it's not None --- frontend/src/components/TradeBox/index.tsx | 2 +- frontend/src/models/Order.model.ts | 1 + tests/test_trade_pipeline.py | 9 +++++++++ tests/utils/trade.py | 4 +++- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/TradeBox/index.tsx b/frontend/src/components/TradeBox/index.tsx index 07003020..b14dcea1 100644 --- a/frontend/src/components/TradeBox/index.tsx +++ b/frontend/src/components/TradeBox/index.tsx @@ -203,7 +203,7 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element => mining_fee_rate, statement, rating, - cancel_status + cancel_status }) .then((data: Order) => { setOpen(closeAll); diff --git a/frontend/src/models/Order.model.ts b/frontend/src/models/Order.model.ts index ede9e4bd..acae1da3 100644 --- a/frontend/src/models/Order.model.ts +++ b/frontend/src/models/Order.model.ts @@ -21,6 +21,7 @@ export interface SubmitActionProps { statement?: string; rating?: number; amount?: number; + cancel_status?: number; } export interface TradeRobotSummary { diff --git a/tests/test_trade_pipeline.py b/tests/test_trade_pipeline.py index 82046363..f7252bfe 100644 --- a/tests/test_trade_pipeline.py +++ b/tests/test_trade_pipeline.py @@ -1085,6 +1085,9 @@ class TradeTest(BaseAPITestCase): trade.cancel_order(trade.maker_index) data = trade.response.json() + self.assertEqual( + data["bad_request"], "This order has been cancelled by the maker" + ) trade.get_order(trade.taker_index) data = trade.response.json() @@ -1092,6 +1095,12 @@ class TradeTest(BaseAPITestCase): data["bad_request"], "This order has been cancelled by the maker" ) + trade.get_order(trade.third_index) + data = trade.response.json() + self.assertEqual( + data["bad_request"], "This order has been cancelled by the maker" + ) + def test_cancel_order_cancel_status(self): """ Tests the cancellation of a public order using cancel_status. diff --git a/tests/utils/trade.py b/tests/utils/trade.py index 85b948eb..3c74f6d9 100644 --- a/tests/utils/trade.py +++ b/tests/utils/trade.py @@ -118,7 +118,9 @@ class Trade: path = reverse("order") params = f"?order_id={self.order_id}" headers = self.get_robot_auth(robot_index) - body = {"action": "cancel", "cancel_status": cancel_status} + body = {"action": "cancel"} + if cancel_status is not None: + body.update({"cancel_status": cancel_status}) self.response = self.client.post(path + params, body, **headers) @patch("api.tasks.send_notification.delay", send_notification) From 2b63c0fbfc8cd96a1775cecd8e0eacf0aa0f7830 Mon Sep 17 00:00:00 2001 From: aftermath2 Date: Mon, 24 Mar 2025 10:42:05 -0300 Subject: [PATCH 3/4] Update order status list --- frontend/src/components/TradeBox/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/TradeBox/index.tsx b/frontend/src/components/TradeBox/index.tsx index b14dcea1..2a1c0cbf 100644 --- a/frontend/src/components/TradeBox/index.tsx +++ b/frontend/src/components/TradeBox/index.tsx @@ -226,7 +226,7 @@ const TradeBox = ({ currentOrder, onStartAgain }: TradeBoxProps): JSX.Element => const cancel = function (): void { const order = garage.getSlot()?.activeOrder; - const noConfirmation = Boolean(order?.is_maker && [0, 1, 2].includes(order?.status)); + const noConfirmation = Boolean(order?.is_maker && [0, 1, 2, 3].includes(order?.status)); setLoadingButtons({ ...noLoadingButtons, cancel: true }); submitAction({ From efae3c4e0ad338906672ada5ff5b774b40e88092 Mon Sep 17 00:00:00 2001 From: aftermath2 Date: Sat, 29 Mar 2025 21:20:21 -0300 Subject: [PATCH 4/4] Update unit tests --- tests/test_trade_pipeline.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_trade_pipeline.py b/tests/test_trade_pipeline.py index f7252bfe..54100dbf 100644 --- a/tests/test_trade_pipeline.py +++ b/tests/test_trade_pipeline.py @@ -1121,7 +1121,7 @@ class TradeTest(BaseAPITestCase): self.assertResponse(trade.response) self.assertEqual( - data["bad_request"], "This order has been cancelled by the maker" + trade.response.json()["bad_request"], "This order has been cancelled by the maker" ) def test_cancel_order_different_cancel_status(self): @@ -1146,7 +1146,7 @@ class TradeTest(BaseAPITestCase): self.assertResponse(trade.response) self.assertEqual( - data["bad_request"], + trade.response.json()["bad_request"], f"Current order status is {Order.Status.PAU}, not {Order.Status.PUB}." )