Skip to content

feat: Rework CancelReason - #29

Open
kalyan02 wants to merge 11 commits into
mainfrom
kn/twap-execution-failed
Open

feat: Rework CancelReason#29
kalyan02 wants to merge 11 commits into
mainfrom
kn/twap-execution-failed

Conversation

@kalyan02

@kalyan02 kalyan02 commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary


Note

Low Risk
Additive enum variants with docs only; low risk unless consumers use exhaustive matches or strict schema versioning without handling new snake_case values.

Overview
Extends the public CancelReason enum with five new matching-engine / lifecycle variants so CancelOrderV1, CancelTriggerOrderV1, and CancelTwapV1 can report more specific cancellation causes.

TwapExecutionFailed mirrors TriggerExecutionFailed: a TWAP slice fired but could not execute (runtime error, no liquidity, etc.), with detail expected on companion reject events. AutoDeleverage covers resting orders cleared when a position is force-closed via ADL. Expired is for makers removed during matching after their expiry time. TwapInsufficientMargin and MakerInsufficientMargin distinguish post-trade margin failures where the fill is rolled back and the TWAP or maker order is cancelled.

No other files change in this diff; wiring to emit these reasons would live elsewhere.

Reviewed by Cursor Bugbot for commit 873e157. Bugbot is set up for automated code reviews on this repo. Configure here.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Comment thread src/event.rs Outdated
@kalyan02
kalyan02 force-pushed the kn/twap-execution-failed branch from 0c49d84 to 44214e5 Compare May 11, 2026 15:59
@kalyan02
kalyan02 force-pushed the kn/twap-execution-failed branch from 0078c04 to 9b174c0 Compare May 21, 2026 11:11

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 342a3d9. Configure here.

Comment thread src/event.rs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/event.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/event.rs Outdated
kalyan and others added 7 commits July 2, 2026 12:04
Parallels TriggerExecutionFailed for the TWAP execution path. Emitted by
to_reject_and_cancel_event when a TWAP slice fires but cannot execute
(runtime error, no liquidity, etc). The accompanying RejectTwapOrder event
in the same tx carries the specific cause.
@vmmon
vmmon force-pushed the kn/twap-execution-failed branch from 873e157 to 03b74ed Compare July 2, 2026 10:09
@vmmon
vmmon force-pushed the kn/twap-execution-failed branch from 03b74ed to ccda2bd Compare July 2, 2026 13:00

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/event.rs">

<violation number="1" location="src/event.rs:727">
P1: `CancelOrderV1.order_id` was changed from the plain type `OrderId` to the enum `OrderReference`, which changes the serialized wire format for an existing public event variant. Because `Event` derives both `borsh` and `serde`, downstream consumers decoding `CancelOrderV1` will see a different binary layout (discriminant + payload) and a different JSON shape (tagged enum object instead of a scalar). If this variant is already emitted anywhere (unlike the removed `CancelTriggerOrderV1`/`CancelTwapV1`, which were reserved because they were never deployed), existing indexers, clients, and generated schemas can fail to deserialize or silently misinterpret the field. Consider either retaining backward compatibility—e.g., by keeping separate V1 variants for each order type—or ensuring all downstream consumers are version-locked and migrated together.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/event.rs
CancelOrderV1 {
user_address: Address,
order_id: OrderId,
order_id: OrderReference,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: CancelOrderV1.order_id was changed from the plain type OrderId to the enum OrderReference, which changes the serialized wire format for an existing public event variant. Because Event derives both borsh and serde, downstream consumers decoding CancelOrderV1 will see a different binary layout (discriminant + payload) and a different JSON shape (tagged enum object instead of a scalar). If this variant is already emitted anywhere (unlike the removed CancelTriggerOrderV1/CancelTwapV1, which were reserved because they were never deployed), existing indexers, clients, and generated schemas can fail to deserialize or silently misinterpret the field. Consider either retaining backward compatibility—e.g., by keeping separate V1 variants for each order type—or ensuring all downstream consumers are version-locked and migrated together.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/event.rs, line 727:

<comment>`CancelOrderV1.order_id` was changed from the plain type `OrderId` to the enum `OrderReference`, which changes the serialized wire format for an existing public event variant. Because `Event` derives both `borsh` and `serde`, downstream consumers decoding `CancelOrderV1` will see a different binary layout (discriminant + payload) and a different JSON shape (tagged enum object instead of a scalar). If this variant is already emitted anywhere (unlike the removed `CancelTriggerOrderV1`/`CancelTwapV1`, which were reserved because they were never deployed), existing indexers, clients, and generated schemas can fail to deserialize or silently misinterpret the field. Consider either retaining backward compatibility—e.g., by keeping separate V1 variants for each order type—or ensuring all downstream consumers are version-locked and migrated together.</comment>

<file context>
@@ -732,28 +724,16 @@ pub enum Event<Address> {
     CancelOrderV1 {
         user_address: Address,
-        order_id: OrderId,
+        order_id: OrderReference,
         market_id: MarketId,
         execution_timestamp: UnixTimestampMicros,
</file context>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CancelOrderV1 was never deployed either. So no breakage here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — if CancelOrderV1 was never deployed, then there’s no downstream breakage from changing its wire format here.

@vmmon
vmmon force-pushed the kn/twap-execution-failed branch 3 times, most recently from 0d219c2 to 407d4aa Compare July 2, 2026 15:35
@vmmon
vmmon force-pushed the kn/twap-execution-failed branch 2 times, most recently from d7a604b to dca0073 Compare July 2, 2026 18:44
@vmmon
vmmon force-pushed the kn/twap-execution-failed branch from dca0073 to 5a18eb3 Compare July 2, 2026 19:37
@vmmon vmmon changed the title feat: add TwapExecutionFailed CancelReason feat: Rework CancelReason Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants