AssetMantle / AssetMantle/modules

orders: exchange-rate encoding panics on rate>1 and collapses same-rate mirrors to one OrderID

Open
#464 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.