Skip to content

feat(exchange): add typed order grouping - #195

Closed
drewstone wants to merge 5 commits into
hyperliquid-dex:masterfrom
tangle-network:feat/typed-order-grouping
Closed

drewstone wants to merge 5 commits into
hyperliquid-dex:masterfrom
tangle-network:feat/typed-order-grouping

Conversation

@drewstone

@drewstone drewstone commented Jul 28, 2026 •

Copy link
Copy Markdown

Summary

Add a typed OrderGrouping enum for the three Hyperliquid grouping values, and expose grouping on bulk orders.

  • OrderGrouping::{Na, NormalTpsl, PositionTpsl} with as_str() for the wire value
  • bulk_order_with_grouping and bulk_order_with_builder_and_grouping
  • bulk_order and bulk_order_with_builder keep their signatures and delegate with OrderGrouping::Na

This lets callers send atomic normalTpsl brackets without passing an arbitrary grouping string.

Compatibility

BulkOrder.grouping stays a String. The enum only chooses the value, so nothing on the wire changes. Two tests prove it:

  • test_typed_grouping_serializes_like_the_string_it_replaces builds the same bulk order twice, once from OrderGrouping and once from the grouping string used before, then asserts an equal action hash, an equal JSON body, and that the derived Serialize agrees with as_str().
  • test_default_grouping_keeps_the_known_signatures asserts that OrderGrouping::Na still produces the two signatures that test_limit_order_action_hashing already pinned.

Both tests fail if any wire value changes. I checked this: changing Na to "NA" makes both fail.

No public item is removed or renamed. No existing test changed.

Scope

This branch first also added raw asset-index order, cancel, and leverage methods, plus Serialize on the exchange responses. That is a separate feature, so I reverted it here to keep this pull request to one subject. I can open it as its own pull request if you want it.

Proof

Ran ./ci.sh on 08eea95, rebased on current master (aac7558):

  • cargo build: clean
  • cargo fmt -- --check: clean
  • cargo clippy -- -D warnings: clean
  • cargo clippy --all-targets -- -D warnings: clean
  • cargo test: 14 passed, 0 failed
  • CI checks passed successfully.

The pull request title promises typed order grouping only. The raw
asset-index order, cancel, and leverage methods and the response
Serialize derives are a separate feature. They move to their own pull
request. This keeps the change focused on one thing.

This reverts commit fea9dcc.
This reverts commit 9dc8b25.
Add two tests that answer the compatibility question directly.

The first builds one bulk order twice, once from OrderGrouping and once
from the grouping string that the SDK used before. It asserts an equal
action hash, an equal JSON body, and that the derived Serialize agrees
with as_str.

The second asserts that OrderGrouping::Na still produces the two
signatures that test_limit_order_action_hashing already pinned.

Both tests fail if any wire value changes.
@drewstone

Copy link
Copy Markdown
Author

Rebased on current master (aac7558) and green. Head is now 08eea95.

I also cut this pull request back to its title. It previously carried raw asset-index order, cancel, and leverage methods plus Serialize on the exchange responses. That is a separate feature, so I reverted it here. Non-test code is now 28 added lines and 4 removed lines across 2 files, down from 90 added and 25 removed across 4 files.

On backward compatibility: BulkOrder.grouping stays a String, so the enum only chooses the value and nothing on the wire changes. Two new tests prove it rather than assert it:

  • test_typed_grouping_serializes_like_the_string_it_replaces builds the same bulk order twice, once from OrderGrouping and once from the grouping string used before, then asserts an equal action hash, an equal JSON body, and that the derived Serialize agrees with as_str().
  • test_default_grouping_keeps_the_known_signatures asserts that OrderGrouping::Na still produces the two signatures that test_limit_order_action_hashing already pinned.

I checked that these tests can fail. Changing Na to "NA" makes both fail, the second with the wrong signature.

Commands and results on 08eea95:

$ ./ci.sh
   cargo build          clean
   cargo fmt -- --check clean
   cargo clippy -- -D warnings   clean
   cargo test           14 passed; 0 failed
CI checks passed successfully.

$ cargo clippy --all-targets -- -D warnings
   clean, no warnings

No public item is removed or renamed, and no existing test changed.

One note: the workflow run for this branch is at action_required, so it waits for a maintainer to approve it. The earlier red mark on this pull request was the same gate, not a test failure. Those runs executed 0 jobs. Please approve the run when you get a chance, and tell me if you want the raw asset-index methods as their own pull request.

@drewstone

Copy link
Copy Markdown
Author

Closing this July fork PR: upstream CI still needs maintainer approval to run, so this head has no green-check or merge path; the branch remains available for a fresh request.

@drewstone drewstone closed this Sep 24, 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.

1 participant