AssetMantle / AssetMantle/modules
orders: exchange-rate encoding panics on rate>1 and collapses same-rate mirrors to one OrderID
- Dominant language
- Go
- Stars
- 21
- Forks
- 12
- PR merge metrics
- No merged PRs in 30d
Description
Two related defects in the order exchange-rate encoding, both PRE-EXISTING and surfaced by the immediate-matcher review (`rwa/phase3-identity-compliance`).
## A. Rate > 1 panics in OrderID derivation (user-reachable)
`make`/`put`/`immediate` store `exchangeRate = takerSplit / 1e-18 / makerSplit`. When `takerSplit > makerSplit` the value exceeds `LegacyMaxSortableDec (1e18)`, and `NewOrderID -> Immutables.GenerateHashID -> DecData.Bytes -> LegacySortableDecBytes` panics `"dec must be within bounds"`. `ValidateBasic` only checks positivity (`x/orders/transactions/make/message.go`, `immediate/message.go`), so a normal user message with a sell price above 1:1 reaches the panic. baseapp recovers it (failed tx, not a halt), but it silently forbids all rate>1 orders.
## B. Same-rate mirror shares an OrderID
`Immutables.GenerateHashID` (schema `qualified/base/immutables.go`) hashes each property's data ID and **sorts** them, ignoring the property key. Swapping MakerAssetID/TakerAssetID at the same rate/height/parties yields the **same** OrderID, so a legitimate counterparty at an identical rate is rejected as `EntityAlreadyExists`, and the immediate matcher must rely on rate differing to see a counterparty.
## Proposed direction (needs a design decision)
Either (a) change the rate encoding so the full economic range is representable and hashable, or (b) explicitly reject rate>1 in `ValidateBasic` with a clear error (a cap, not a panic) and document the constraint. (B) is a footgun-removal; (A) is the real fix and touches schema hashing (wide blast radius).
## Fix-priority scorecard
| SEC | UX | AUTO | YAGNI | DELTA | FIN |
|-----|----|------|-------|-------|-----|
| 40 | 55 | 35 | 45 | 40 | 43 |
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with x/orders/transactions/make/message.go and immediate/message.go, then trace NewOrderID through qualified/base/immutables.go and DecData.Bytes. Reproduce both rate>1 and same-rate mirror cases before choosing between a representable encoding and explicit validation. Done means the selected behavior is implemented, covered for both defects, and no longer panics or aliases legitimate orders.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100