fix: validate order identifiers and test public position flows - #110
Merged
Merged
Conversation
jwei-pm
approved these changes
Sep 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reject ambiguous or malformed order identifiers at runtime before metadata requests or signing. Inputs such as
{ tokenID: "123", positionID: null }, both IDs, neither ID, and null/blank/non-string IDs now fail with a clear error. An omitted orundefinedunused identifier remains valid.Add public limit and market order tests with real signing and mocked HTTP. They cover automatic V3 signing, V1/V2 overrides still routing positions to V3, position-based tick/fee/book lookups, balance adjustment, and the complete posting payload with the position in
tokenId. Token order version selection remains unchanged.Validation: 355 tests pass; Biome, ESM/CJS build with declarations, TypeScript
--noEmit, and diff checks pass. The new validation regressions failed before the fix. HTTP is mocked; no live orders were submitted.Note
Medium Risk
Stricter runtime validation can break callers that previously passed ambiguous or malformed IDs; order creation paths are affected but behavior for valid inputs is preserved.
Overview
Order identifier validation is tightened in
resolveOrderAssetID: callers must supply exactly one oftokenIDorpositionIDas a non-empty string. Ambiguous or bad inputs (both set, neither set,null, blank/whitespace-only, or non-string values) now throw a clear error before metadata fetches, signing, or HTTP posts. An omitted/undefinedunused field still works as before.Tests add unit coverage for the new validation on both
resolveOrderAssetIDandresolveOrderRouting, plus integration-style client tests for public limit/market position orders (real signing, mocked HTTP): Exchange V3 signing regardless of version override, position-based tick/fee/book lookups, balance-adjusted amounts, full/orderpayload with the position intokenId, early rejection without side effects, and unchanged token-order version routing.Reviewed by Cursor Bugbot for commit a57923e. Bugbot is set up for automated code reviews on this repo. Configure here.