Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Lexer.g4
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ PLUS: '+';
MINUS: '-';
DIV: '/';
RESTRICT: '\\';
WITH: 'with';
SCALING: 'scaling';
THROUGH: 'through';

PERCENTAGE_PORTION_LITERAL: [0-9]+ ('.' [0-9]+)? '%';

Expand Down
8 changes: 4 additions & 4 deletions Numscript.g4
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ allotment:

colorConstraint: RESTRICT valueExpr;

source:
address = valueExpr colorConstraint? ALLOWING UNBOUNDED OVERDRAFT # srcAccountUnboundedOverdraft
| address = valueExpr colorConstraint? ALLOWING OVERDRAFT UP TO maxOvedraft = valueExpr #
srcAccountBoundedOverdraft
source
: address = valueExpr colorConstraint? ALLOWING UNBOUNDED OVERDRAFT # srcAccountUnboundedOverdraft
| address = valueExpr colorConstraint? ALLOWING OVERDRAFT UP TO maxOvedraft = valueExpr #srcAccountBoundedOverdraft
| address = valueExpr WITH SCALING THROUGH swap=valueExpr # srcAccountWithScaling
| valueExpr colorConstraint? # srcAccount
| LBRACE allotmentClauseSrc+ RBRACE # srcAllotment
| LBRACE source* RBRACE # srcInorder
Expand Down
5 changes: 5 additions & 0 deletions internal/analysis/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,11 @@ func (res *CheckResult) checkSource(source parser.Source) {
res.unifyNodeWith(*source.Bounded, res.stmtType)
}

case *parser.SourceWithScaling:
res.checkExpression(source.Address, TypeAccount)
res.checkExpression(source.Through, TypeAccount)
res.checkExpression(source.Color, TypeString)

case *parser.SourceInorder:
for _, source := range source.Sources {
res.checkSource(source)
Expand Down
12 changes: 12 additions & 0 deletions internal/analysis/hover.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,18 @@ func hoverOnSource(source parser.Source, position parser.Position) Hover {
return hover
}
}
case *parser.SourceWithScaling:
hover := hoverOnExpression(source.Address, position)
if hover != nil {
return hover
}

hover = hoverOnExpression(source.Through, position)
if hover != nil {
return hover
}

return nil

case *parser.SourceAllotment:
for _, item := range source.Items {
Expand Down
29 changes: 29 additions & 0 deletions internal/analysis/hover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,3 +523,32 @@ send [ASSET *] (
resolved := checkResult.ResolveVar(variableHover.Node)
require.NotNil(t, resolved)
}

func TestHover(t *testing.T) {

input := `vars {
account $x
}

send [EUR/2 *] (
source = {
@acc1
@acc2 with scaling through $x
}
destination = @dest
)

`

rng := parser.RangeOfIndexed(input, "$x", 1)
program := parser.Parse(input).Value
hover := analysis.HoverOn(program, rng.Start)

varHover, ok := hover.(*analysis.VariableHover)
if !ok {
t.Fatalf("Expected a VariableHover")
}

require.Equal(t, varHover.Node.Name, "x")
Comment on lines +545 to +552
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing nil check and incorrect require.Equal argument order.

  1. A nil check on hover before the type assertion would prevent a potential panic and provide a clearer failure message, consistent with all other tests in this file.
  2. The require.Equal arguments are reversed—testify convention is (t, expected, actual).
Proposed fix
 	rng := parser.RangeOfIndexed(input, "$x", 1)
 	program := parser.Parse(input).Value
 	hover := analysis.HoverOn(program, rng.Start)
+	require.NotNil(t, hover)
 
 	varHover, ok := hover.(*analysis.VariableHover)
 	if !ok {
 		t.Fatalf("Expected a VariableHover")
 	}
 
-	require.Equal(t, varHover.Node.Name, "x")
+	require.Equal(t, "x", varHover.Node.Name)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
hover := analysis.HoverOn(program, rng.Start)
varHover, ok := hover.(*analysis.VariableHover)
if !ok {
t.Fatalf("Expected a VariableHover")
}
require.Equal(t, varHover.Node.Name, "x")
hover := analysis.HoverOn(program, rng.Start)
require.NotNil(t, hover)
varHover, ok := hover.(*analysis.VariableHover)
if !ok {
t.Fatalf("Expected a VariableHover")
}
require.Equal(t, "x", varHover.Node.Name)
🤖 Prompt for AI Agents
In @internal/analysis/hover_test.go around lines 545 - 552, The test is missing
a nil check on hover before the type assertion and has reversed require.Equal
arguments; first assert hover != nil (e.g., fail with a clear message consistent
with other tests) before doing the type assertion to avoid a panic, then perform
the type assertion to *analysis.VariableHover and use require.Equal(t, "x",
varHover.Node.Name) so the expected and actual are in the correct order;
reference the hover variable, the VariableHover type, varHover.Node.Name, and
the require.Equal call when making the changes.


}
2 changes: 2 additions & 0 deletions internal/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const (
ExperimentalAccountInterpolationFlag FeatureFlag = "experimental-account-interpolation"
ExperimentalMidScriptFunctionCall FeatureFlag = "experimental-mid-script-function-call"
ExperimentalAssetColors FeatureFlag = "experimental-asset-colors"
AssetScaling FeatureFlag = "experimental-asset-scaling"
)

var AllFlags []string = []string{
Expand All @@ -20,4 +21,5 @@ var AllFlags []string = []string{
ExperimentalAccountInterpolationFlag,
ExperimentalMidScriptFunctionCall,
ExperimentalAssetColors,
AssetScaling,
}
135 changes: 135 additions & 0 deletions internal/interpreter/asset_scaling.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package interpreter

import (
"fmt"
"math/big"
"slices"
"strconv"
"strings"

"github.com/formancehq/numscript/internal/utils"
)

func assetToScaledAsset(asset string) string {
parts := strings.Split(asset, "/")
if len(parts) == 1 {
return asset + "/*"
}
return parts[0] + "/*"
}

func buildScaledAsset(baseAsset string, scale int64) string {
if scale == 0 {
return baseAsset
}
return fmt.Sprintf("%s/%d", baseAsset, scale)
}

func getAssetScale(asset string) (string, int64) {
parts := strings.Split(asset, "/")
if len(parts) == 2 {
scale, err := strconv.ParseInt(parts[1], 10, 64)
if err == nil {
return parts[0], scale
}
// fallback if parsing fails
return parts[0], 0
}
return asset, 0
}

func getAssets(balance AccountBalance, baseAsset string) map[int64]*big.Int {
result := make(map[int64]*big.Int)
for asset, amount := range balance {
if strings.HasPrefix(asset, baseAsset) {
_, scale := getAssetScale(asset)
result[scale] = amount
}
}
return result
}
Comment on lines +41 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Prefix matching may cause false positives.

strings.HasPrefix(asset, baseAsset) will incorrectly match assets like "USDC" or "USDC/2" when searching for "USD". Consider checking for an exact match or ensuring the prefix is followed by / or end-of-string.

🔎 Proposed fix
 func getAssets(balance AccountBalance, baseAsset string) map[int64]*big.Int {
 	result := make(map[int64]*big.Int)
 	for asset, amount := range balance {
-		if strings.HasPrefix(asset, baseAsset) {
+		base, _ := getAssetScale(asset)
+		if base == baseAsset {
 			_, scale := getAssetScale(asset)
 			result[scale] = amount
 		}
 	}
 	return result
 }
🤖 Prompt for AI Agents
internal/interpreter/asset_scaling.go around lines 41 to 50: the current
strings.HasPrefix(asset, baseAsset) check can produce false positives (e.g.
"USDC" matching "USD"); change the condition to accept either an exact match
(asset == baseAsset) or a prefix followed by a separator (e.g.
strings.HasPrefix(asset, baseAsset+"/")) so only the base asset or variants like
"USD/..." match; update the if to use this combined check and keep the rest of
the function unchanged.


type scalePair struct {
scale int64
amount *big.Int
}

// e.g.
//
// need=[EUR/2 100], got={EUR/2: 100, EUR: 1}
// => {EUR/2: 100, EUR: 1}
//
// need=[EUR 1], got={EUR/2: 100, EUR: 0}
// => {EUR/2: 100, EUR: 0}
//
// need=[EUR/2 199], got={EUR/2: 100, EUR: 2}
// => {EUR/2: 100, EUR: 1}
func findSolution(
neededAmt *big.Int, // <- can be nil
neededAmtScale int64,
scales map[int64]*big.Int,
) ([]scalePair, *big.Int) {
var assets []scalePair
for k, v := range scales {
assets = append(assets, scalePair{
scale: k,
amount: v,
})
}

// Sort in ASC order (e.g. EUR, EUR/2, ..)
slices.SortFunc(assets, func(p scalePair, other scalePair) int {
return int(p.scale - other.scale)
})

var out []scalePair

totalSent := big.NewInt(0)

for _, p := range assets {
scaleDiff := neededAmtScale - p.scale

exp := big.NewInt(scaleDiff)
exp.Abs(exp)
exp.Exp(big.NewInt(10), exp, nil)

// scalingFactor := 10 ^ (neededAmtScale - p.scale)
// note that 10^0 == 1 and 10^(-n) == 1/(10^n)
scalingFactor := new(big.Rat).SetInt(exp)
if scaleDiff < 0 {
scalingFactor.Inv(scalingFactor)
}

allowed := new(big.Int).Mul(p.amount, scalingFactor.Num())
allowed.Div(allowed, scalingFactor.Denom())

var leftAmt *big.Int
var taken *big.Int
if neededAmt == nil {
taken = new(big.Int).Set(allowed)
} else {
leftAmt = new(big.Int).Sub(neededAmt, totalSent)
taken = utils.MinBigInt(allowed, leftAmt)
}

totalSent.Add(totalSent, taken)

intPart := new(big.Int).Mul(taken, scalingFactor.Denom())
intPart.Div(intPart, scalingFactor.Num())

if intPart.Cmp(big.NewInt(0)) == 0 {
continue
}

out = append(out, scalePair{
scale: p.scale,
amount: intPart,
})

if leftAmt != nil && leftAmt.Cmp(big.NewInt(0)) != 1 {
break
}
Comment on lines +129 to +131
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Break condition may not trigger correctly.

leftAmt is computed before totalSent is updated, so after the update, leftAmt still holds the old value. The check leftAmt.Cmp(big.NewInt(0)) != 1 (i.e., leftAmt <= 0) should use the updated remaining amount. Currently, this may cause an extra iteration or incorrect early termination.

🔎 Proposed fix

Recompute the remaining amount after updating totalSent:

 		totalSent.Add(totalSent, taken)

 		intPart := new(big.Int).Mul(taken, scalingFactor.Denom())
 		intPart.Div(intPart, scalingFactor.Num())

 		if intPart.Cmp(big.NewInt(0)) == 0 {
 			continue
 		}

 		out = append(out, scalePair{
 			scale:  p.scale,
 			amount: intPart,
 		})

-		if leftAmt != nil && leftAmt.Cmp(big.NewInt(0)) != 1 {
+		if neededAmt != nil {
+			remaining := new(big.Int).Sub(neededAmt, totalSent)
+			if remaining.Cmp(big.NewInt(0)) <= 0 {
+				break
+			}
+		}
-			break
-		}
 	}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In internal/interpreter/asset_scaling.go around lines 129 to 131, the break
condition uses leftAmt computed before totalSent is updated, so it may be stale;
after you update totalSent recompute the remaining amount (e.g., recalc leftAmt
= totalNeeded.Sub(totalNeeded, totalSent) or recompute from original values) and
then check leftAmt.Cmp(big.NewInt(0)) != 1 to decide breaking; replace the
existing break check with one that uses this recomputed remaining amount so the
loop terminates correctly.

Copy link
Contributor Author

@ascandone ascandone Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai give me tests that can reproduce the error. Or describe them if you can't reproduce the numscript exactly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ascandone, here's a test case that demonstrates the issue:

func TestScalingBreakConditionStale(t *testing.T) {
	// Setup: Need exactly 100 at scale 0
	// Available: scale 0 has 100, scale 1 has 1000
	// Expected: Should only use scale 0 (since it satisfies the need exactly)
	// Bug: The function continues to the next iteration unnecessarily

	neededAmt := big.NewInt(100)
	neededScale := int64(0)
	scales := map[int64]*big.Int{
		0: big.NewInt(100), // Exactly what we need
		1: big.NewInt(1000), // Extra available at higher scale
	}

	result, totalSent := findSolution(neededAmt, neededScale, scales)

	// After processing scale 0:
	// - leftAmt = 100 - 0 = 100 (before update)
	// - taken = 100
	// - totalSent = 100
	// - Check: leftAmt (100) <= 0? False → continues to next iteration
	// 
	// Next iteration (scale 1):
	// - leftAmt = 100 - 100 = 0
	// - taken = 0
	// - Gets skipped due to intPart == 0 check
	//
	// While functionally correct, the break condition should have
	// terminated after the first iteration since remaining = 0

	assert.Equal(t, big.NewInt(100), totalSent)
	assert.Equal(t, 1, len(result)) // Should only have scale 0
	assert.Equal(t, int64(0), result[0].scale)
	assert.Equal(t, big.NewInt(100), result[0].amount)
}

Description of the issue:

The problem occurs when an asset scale exactly satisfies the remaining need. In the example above:

  1. First iteration: leftAmt = 100, we take all 100, update totalSent = 100
  2. The break check uses the old leftAmt (100), not the remaining after the update (0)
  3. Since 100 > 0, we don't break
  4. Second iteration computes leftAmt = 0, which correctly results in taken = 0 and gets skipped

While this doesn't produce incorrect results (due to the intPart == 0 check), it causes an unnecessary iteration. The fix would ensure we break immediately when the requirement is fully satisfied, making the logic clearer and slightly more efficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai I ran this test as-is and it's passing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ascandone, you're absolutely right — the test passes because the code is functionally correct. I need to clarify my original assessment:

What actually happens:

  1. When an asset scale exactly satisfies the remaining need:

    • leftAmt is computed as neededAmt - totalSent (old value)
    • We take the needed amount and update totalSent
    • The break condition checks the stale leftAmt (which is still > 0)
    • Loop continues to next iteration
  2. Next iteration:

    • Computes new leftAmt = neededAmt - totalSent (now 0)
    • taken = min(allowed, 0) = 0
    • intPart becomes 0
    • The check on line 119 (if intPart.Cmp(big.NewInt(0)) == 0) causes a continue, skipping that entry

The verdict:

This is a minor inefficiency (one unnecessary loop iteration), not a correctness bug. The intPart == 0 check prevents any incorrect results. My original severity rating of "Major" was too high.

The suggested fix would make the code slightly cleaner and more efficient by breaking immediately when the need is satisfied, but it's not critical. If you prefer to keep the code as-is for simplicity, that's reasonable.

Thank you for verifying — this helped clarify the actual impact! 🙏

}

return out, totalSent
}
153 changes: 153 additions & 0 deletions internal/interpreter/asset_scaling_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
package interpreter

import (
"math/big"
"testing"

"github.com/stretchr/testify/require"
)

func TestScalingZeroNeeded(t *testing.T) {
t.Skip()

// need [EUR/2 ]
sol, _ := findSolution(
big.NewInt(0),
42,
map[int64]*big.Int{
2: big.NewInt(100),
1: big.NewInt(1),
})

require.Equal(t, []scalePair{
{42, big.NewInt(0)},
}, sol)
}

func TestScalingSameAsset(t *testing.T) {
sol, _ := findSolution(
// Need [EUR/2 200]
big.NewInt(200),
2,

// Have: {EUR/2: 201}
map[int64]*big.Int{
2: big.NewInt(201),
})

require.Equal(t, []scalePair{
{2, big.NewInt(200)},
}, sol)
}

func TestScalingSolutionLowerScale(t *testing.T) {
sol, _ := findSolution(
big.NewInt(1),
0,
map[int64]*big.Int{
2: big.NewInt(900),
})

require.Equal(t, []scalePair{
{2, big.NewInt(100)},
}, sol)
}

func TestScalingSolutionHigherScale(t *testing.T) {
sol, _ := findSolution(
// Need [EUR/2 200]
big.NewInt(200),
2,

// Have: {EUR: 4} (eq to EUR/2 400)
map[int64]*big.Int{
0: big.NewInt(4),
})

require.Equal(t, []scalePair{
{0, big.NewInt(2)},
}, sol)
}

func TestScalingSolutionHigherScaleNoSolution(t *testing.T) {
sol, _ := findSolution(
big.NewInt(1),
2,
map[int64]*big.Int{
0: big.NewInt(100),
1: big.NewInt(100),
})

require.Nil(t, sol)
}

func TestMixedFail(t *testing.T) {
t.Skip()

sol, _ := findSolution(
big.NewInt(400),
2,
map[int64]*big.Int{
0: big.NewInt(1),
2: big.NewInt(200),
3: big.NewInt(10),
})

require.Nil(t, sol)
}

func TestUnboundedScalingSameAsset(t *testing.T) {
sol, _ := findSolution(
nil,
// Need USD/2
2,
// Have: {EUR/2: 201}
map[int64]*big.Int{
2: big.NewInt(123),
})

require.Equal(t, []scalePair{
{2, big.NewInt(123)},
}, sol)
}

func TestUnboundedScalingLowerAsset(t *testing.T) {
sol, _ := findSolution(
nil,
2,
map[int64]*big.Int{
0: big.NewInt(1),
})

require.Equal(t, []scalePair{
{0, big.NewInt(1)},
}, sol)
}

func TestUnboundedScalinHigherAsset(t *testing.T) {
sol, _ := findSolution(
nil,
2,
map[int64]*big.Int{
3: big.NewInt(10),
})

require.Equal(t,
[]scalePair{
{3, big.NewInt(10)},
},
sol)
}

func TestUnboundedScalinHigherAssetTrimRemainder(t *testing.T) {
sol, _ := findSolution(
nil,
2,
map[int64]*big.Int{
3: big.NewInt(15),
})

require.Equal(t, []scalePair{
{3, big.NewInt(10)},
}, sol)
}
Loading
Loading