From 7b65e3f706b3650310a57f7d0c07c72ca3dd18c2 Mon Sep 17 00:00:00 2001 From: Geoffrey Ragot Date: Wed, 3 Jun 2026 14:04:45 +0200 Subject: [PATCH] test(asset_scaling): pin getAssets prefix-match bug (failing) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getAssets(balance, baseAsset)` uses `strings.HasPrefix(asset, baseAsset)`, which accidentally matches any asset whose key shares the base name as a string prefix: - `USDT` and `USDT/2` are matched as scaled variants of `USD` - `USD_RED` and `USD_RED/2` (suffix-encoded color variants emitted by the experimental asset-colors feature flag) are also matched Worse: because the result map is keyed by precision scale only, an entry like `USD_RED/2 = 500` overwrites the true `USD/2 = 2` value in the scaling pool. A user holding both `USD/2 = 2` and `USD_RED/2 = 500` on the same account will see scaling compute conversions against the wrong amount — silent corruption. This commit introduces a failing regression test `TestGetAssetsRejectsSpuriousPrefixMatches` that pins the bug. CI is intentionally red on this branch. The fix is in a stacked PR that targets this branch — merging it flips the test green. Flagged by CodeRabbit during review of #139. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/interpreter/asset_scaling_test.go | 46 ++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/internal/interpreter/asset_scaling_test.go b/internal/interpreter/asset_scaling_test.go index 989a9ade..a08aa206 100644 --- a/internal/interpreter/asset_scaling_test.go +++ b/internal/interpreter/asset_scaling_test.go @@ -272,3 +272,49 @@ func TestUnboundedScalinHigherAssetTrimRemainder(t *testing.T) { {3, big.NewInt(10)}, }, sol) } + +// getAssets must match a base asset exactly: assets that merely share the +// base name as a string prefix (e.g. "USDT" or "USD_RED") are NOT scaled +// forms of "USD" and must be ignored. Only the bare base and its +// "base/N" precision variants belong in the scaling pool. +// +// Sub-tests are deliberately split so each is deterministic regardless of +// Go's randomized map iteration order. A combined fixture is flaky: +// `getAssets` keys its result map by precision alone, so when both +// `USD/2 = 2` and `USD_RED/2 = 500` are present the buggy implementation +// writes both to `result[2]`, and the final value depends on iteration +// order — sometimes the correct one wins, sometimes the spurious one. +func TestGetAssetsRejectsSpuriousPrefixMatches(t *testing.T) { + t.Run("USDT does not pollute USD pool", func(t *testing.T) { + balance := AccountBalance{ + "USDT": big.NewInt(100), + "USDT/2": big.NewInt(200), + } + got := getAssets(balance, "USD") + require.Empty(t, got, "USDT-prefixed entries must not be folded into USD") + }) + + t.Run("color-suffixed variants do not pollute USD pool", func(t *testing.T) { + balance := AccountBalance{ + "USD_RED": big.NewInt(50), + "USD_RED/2": big.NewInt(500), + } + got := getAssets(balance, "USD") + require.Empty(t, got, "USD_RED-prefixed entries must not be folded into USD") + }) + + t.Run("USD precision variants are picked up", func(t *testing.T) { + balance := AccountBalance{ + "USD": big.NewInt(1), + "USD/2": big.NewInt(2), + "USD/3": big.NewInt(3), + } + got := getAssets(balance, "USD") + require.Equal(t, map[int64]*big.Int{ + 0: big.NewInt(1), + 2: big.NewInt(2), + 3: big.NewInt(3), + }, got) + }) + +}