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
35 changes: 28 additions & 7 deletions inputs.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
"definitions": {
"Balances": {
"type": "array",
"description": "List of account balances. The (account, asset, color) triple of each entry must be unique within the list.",
"description": "List of account balances. The (account, asset, color, scope) tuple of each entry must be unique within the list.",
"items": { "$ref": "#/definitions/BalanceRow" }
},

"BalanceRow": {
"type": "object",
"description": "The balance of a given (account, asset, color)",
"description": "The balance of a given (account, asset, color, scope)",
"additionalProperties": false,
"required": ["account", "asset", "amount"],
"properties": {
Expand All @@ -46,6 +46,10 @@
"color": {
"type": "string",
"pattern": "^[A-Z]*$"
},
"scope": {
"type": "string",
"pattern": "^[a-z0-9_]*$"
}
}
},
Expand All @@ -60,13 +64,30 @@
},

"AccountsMetadata": {
"type": "array",
"description": "List of account metadata entries. The (account, key, scope) tuple of each entry must be unique within the list.",
"items": { "$ref": "#/definitions/AccountMetadataRow" }
},

"AccountMetadataRow": {
"type": "object",
"description": "Map of an account metadata to the account's metadata",
"description": "A single metadata entry: the value of a given (account, key, scope)",
"additionalProperties": false,
"patternProperties": {
"^([a-zA-Z0-9_-]+(:[a-zA-Z0-9_-]+)*)$": {
"type": "object",
"additionalProperties": { "type": "string" }
"required": ["account", "key", "value"],
"properties": {
"account": {
"type": "string",
"pattern": "^([a-zA-Z0-9_-]+(:[a-zA-Z0-9_-]+)*)$"
},
"key": {
"type": "string"
},
"value": {
"type": "string"
},
"scope": {
"type": "string",
"pattern": "^[a-z0-9_]*$"
}
}
},
Expand Down
31 changes: 24 additions & 7 deletions internal/analysis/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,19 @@ func (StatementFnCallResolution) fnCallResolution() {}
func (r VarOriginFnCallResolution) GetParams() []string { return r.Params }
func (r StatementFnCallResolution) GetParams() []string { return r.Params }

const FnSetTxMeta = "set_tx_meta"
const FnSetAccountMeta = "set_account_meta"
const FnVarOriginMeta = "meta"
const FnVarOriginBalance = "balance"
const FnVarOriginOverdraft = "overdraft"
const FnVarOriginGetAsset = "get_asset"
const FnVarOriginGetAmount = "get_amount"
const (
// Statemetn fns
FnSetTxMeta = "set_tx_meta"
FnSetAccountMeta = "set_account_meta"

// Expr fns
FnVarOriginMeta = "meta"
FnVarOriginBalance = "balance"
FnVarOriginOverdraft = "overdraft"
FnVarOriginGetAsset = "get_asset"
FnVarOriginGetAmount = "get_amount"
FnVarOriginScoped = "scoped"
)

var Builtins = map[string]FnCallResolution{
FnSetTxMeta: StatementFnCallResolution{
Expand Down Expand Up @@ -114,6 +120,17 @@ var Builtins = map[string]FnCallResolution{
},
},
},
FnVarOriginScoped: VarOriginFnCallResolution{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [major] Teach involved-account analysis about scoped()

Registering scoped as an account-returning expression also needs support in GetInvolvedAccounts: scripts that use scoped(@src, "x") in a source, destination, balance, or metadata expression now reach the involved-account evaluator's function switch and fall through to UnboundFunctionErr. This breaks the public ParseResult.GetInvolvedAccounts API for the new feature.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [major] Handle scoped() in involved-account analysis

Adding scoped() as a supported account expression leaves GetInvolvedAccounts incomplete: its function-call switch still has no FnVarOriginScoped case, so any script using scoped(@acc, "x") in a source/destination is executable but the public involved-accounts API returns UnboundFunctionErr. Please add an involved-account expression for scoped accounts or fold it into the account expression there as well.

Params: []string{TypeAccount, TypeString},
Return: TypeAccount,
Docs: "returns the scoped version of that account. Empty string means no scope. Overwrites the previous scope",
VersionConstraints: []VersionClause{
{
Version: parser.NewVersionInterpreter(0, 0, 25),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 [blocker] Require the scoped feature flag for scoped()

When a script enables only experimental-scoped-function, analysis still reports the get-amount feature as missing because this builtin is wired to ExperimentalGetAmountFunctionFeatureFlag. The new scoped fixture declares the scoped flag but not get-amount, so it will be rejected before runtime; this should reference ExperimentalScopedFunction instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [major] Require the scoped feature flag for scoped()

When a script uses scoped() with experimental-scoped-function enabled, the analyzer still reports the missing experimental-get-amount-function flag because the builtin is registered with the get-amount flag here. This makes the documented/tested scoped flag insufficient during version-gated analysis.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [major] Use the scoped feature flag for scoped()

For versioned scripts that call scoped, the analyzer currently requires experimental-get-amount-function instead of experimental-scoped-function. A script declaring the scoped flag will still get an experimental-feature diagnostic, while declaring the get-amount flag lets analysis pass but runtime rejects the call unless the scoped flag is also enabled.

FeatureFlag: flags.ExperimentalScopedFunction,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 [major] Use the scoped feature flag for scoped()

For versioned programs using scoped(), the static checker currently requires experimental-get-amount-function instead of experimental-scoped-function. A script that enables the new scoped flag will still get an experimental-feature diagnostic for the wrong flag, while enabling the get-amount flag passes analysis but then fails at runtime because scoped() checks the scoped flag.

},
},
},
}

type Diagnostic struct {
Expand Down
14 changes: 13 additions & 1 deletion internal/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,27 @@ func run(scriptPath string, opts RunArgs) error {
}

// Reject a malformed inputs file before running anything: a balance list is a
// map keyed by (account, asset, color), so a repeated key is ambiguous.
// map keyed by (account, asset, color, scope), so a repeated key is ambiguous.
if dup, ok := inputs.Balances.FirstDuplicate(); ok {
key := fmt.Sprintf("account=%q asset=%q", dup.Account, dup.Asset)
if dup.Color != "" {
key += fmt.Sprintf(" color=%q", dup.Color)
}
if dup.Scope != "" {
key += fmt.Sprintf(" scope=%q", dup.Scope)
}
return fmt.Errorf("invalid inputs file '%s': balances must not contain duplicate entries: duplicate entry for %s", inputsPath, key)
}

// Likewise, a metadata list is keyed by (account, key, scope).
if dup, ok := inputs.Meta.FirstDuplicate(); ok {
key := fmt.Sprintf("account=%q key=%q", dup.Account, dup.Key)
if dup.Scope != "" {
key += fmt.Sprintf(" scope=%q", dup.Scope)
}
return fmt.Errorf("invalid inputs file '%s': metadata must not contain duplicate entries: duplicate entry for %s", inputsPath, key)
}

featureFlags := map[string]struct{}{}
for _, flag := range inputs.FeatureFlags {
featureFlags[flag] = struct{}{}
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/test_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func makeSpecsFile(
DefaultBalance: defaultBalance,
StaticStore: interpreter.StaticStore{
Balances: interpreter.Balances{},
Meta: make(interpreter.AccountsMetadata),
Meta: interpreter.AccountsMetadata{},
},
}

Expand Down
2 changes: 2 additions & 0 deletions internal/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const (
ExperimentalOverdraftFunctionFeatureFlag FeatureFlag = "experimental-overdraft-function"
ExperimentalGetAssetFunctionFeatureFlag FeatureFlag = "experimental-get-asset-function"
ExperimentalGetAmountFunctionFeatureFlag FeatureFlag = "experimental-get-amount-function"
ExperimentalScopedFunction FeatureFlag = "experimental-scoped-function"
ExperimentalOneofFeatureFlag FeatureFlag = "experimental-oneof"
ExperimentalAccountInterpolationFlag FeatureFlag = "experimental-account-interpolation"
ExperimentalMidScriptFunctionCall FeatureFlag = "experimental-mid-script-function-call"
Expand All @@ -17,6 +18,7 @@ var AllFlags []string = []string{
ExperimentalOverdraftFunctionFeatureFlag,
ExperimentalGetAssetFunctionFeatureFlag,
ExperimentalGetAmountFunctionFeatureFlag,
ExperimentalScopedFunction,
ExperimentalOneofFeatureFlag,
ExperimentalAccountInterpolationFlag,
ExperimentalMidScriptFunctionCall,
Expand Down
13 changes: 13 additions & 0 deletions internal/interpreter/__snapshots__/accounts_metadata_test.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

[TestPrettyPrintAccountsMetadata/without_scope_(no_Scope_column) - 1]
| Account | Name | Value  |
| alice | kyc | verified |
| bob | tier | gold |
---

[TestPrettyPrintAccountsMetadata/with_scope_(Scope_column_shown) - 1]
| Account | Scope | Name | Value  |
| alice | eu | kyc | pending |
| alice | | kyc | verified |
| bob | | tier | gold |
---
85 changes: 53 additions & 32 deletions internal/interpreter/accounts_metadata.go
Original file line number Diff line number Diff line change
@@ -1,53 +1,74 @@
package interpreter

import (
"slices"

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

type AccountMetadata = map[string]string
type AccountsMetadata map[string]AccountMetadata

func (m AccountsMetadata) fetchAccountMetadata(account string) AccountMetadata {
return utils.MapGetOrPutDefault(m, account, func() AccountMetadata {
return AccountMetadata{}
})
}

func (m AccountsMetadata) DeepClone() AccountsMetadata {
cloned := make(AccountsMetadata)
for account, accountBalances := range m {
for asset, metadataValue := range accountBalances {
clonedAccountBalances := cloned.fetchAccountMetadata(account)
utils.MapGetOrPutDefault(clonedAccountBalances, asset, func() string {
return metadataValue
})
}
}
return cloned
type AccountMetadataRow struct {
Account string `json:"account"`
Key string `json:"key"`
Value string `json:"value"`
Scope string `json:"scope,omitempty"`
}

func (m AccountsMetadata) Merge(update AccountsMetadata) {
for acc, accBalances := range update {
cachedAcc := utils.MapGetOrPutDefault(m, acc, func() AccountMetadata {
return AccountMetadata{}
})
// AccountsMetadata is the external, serialized representation of account
// metadata. The runtime works with the in-memory InternalAccountsMetadata and
// converts to this at the boundaries (store queries, execution result).
type AccountsMetadata []AccountMetadataRow

for curr, amt := range accBalances {
cachedAcc[curr] = amt
// FirstDuplicate returns the first row whose (account, key, scope) key already
// appeared earlier in the list, if any. That triple is the identity of a
// metadata entry and the value is its content, so a repeated key is an
// ambiguous, malformed input.
func (rows AccountsMetadata) FirstDuplicate() (AccountMetadataRow, bool) {
seen := make(map[[3]string]struct{}, len(rows))
for _, row := range rows {
key := [3]string{row.Account, row.Key, row.Scope}
if _, ok := seen[key]; ok {
return row, true
}
seen[key] = struct{}{}
}
return AccountMetadataRow{}, false
}

func (m AccountsMetadata) PrettyPrint() string {
header := []string{"Account", "Name", "Value"}
// the Scope column is shown only when at least one entry has a scope
hasScope := slices.ContainsFunc(m, func(row AccountMetadataRow) bool {
return row.Scope != ""
})

var header []string
if hasScope {
header = []string{"Account", "Scope", "Name", "Value"}
} else {
header = []string{"Account", "Name", "Value"}
}

var rows [][]string
for account, accMetadata := range m {
for name, value := range accMetadata {
row := []string{account, name, value}
rows = append(rows, row)
for _, row := range m {
if hasScope {
rows = append(rows, []string{row.Account, row.Scope, row.Key, row.Value})
} else {
rows = append(rows, []string{row.Account, row.Key, row.Value})
}
}

return utils.CsvPretty(header, rows, true)
}

// CompareAccountsMetadata reports whether two metadata lists hold the same set
// of rows, ignoring order.
func CompareAccountsMetadata(a AccountsMetadata, b AccountsMetadata) bool {
if len(a) != len(b) {
return false
}
for _, row := range a {
if !slices.Contains(b, row) {
return false
}
}
return true
Comment on lines +64 to +73

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix multiset comparison in CompareAccountsMetadata.

This implementation can return true for unequal datasets when duplicates exist (e.g., Line 39 loop + slices.Contains only checks presence, not count). That can mask assertion failures where this comparator is used by specs.

Suggested fix
-import (
-	"slices"
-
-	"github.com/formancehq/numscript/internal/utils"
-)
+import "github.com/formancehq/numscript/internal/utils"
@@
 func CompareAccountsMetadata(a AccountsMetadata, b AccountsMetadata) bool {
 	if len(a) != len(b) {
 		return false
 	}
-	for _, row := range a {
-		if !slices.Contains(b, row) {
+	counts := make(map[AccountMetadataRow]int, len(a))
+	for _, row := range a {
+		counts[row]++
+	}
+	for _, row := range b {
+		if counts[row] == 0 {
 			return false
 		}
+		counts[row]--
 	}
 	return true
 }
📝 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
func CompareAccountsMetadata(a AccountsMetadata, b AccountsMetadata) bool {
if len(a) != len(b) {
return false
}
for _, row := range a {
if !slices.Contains(b, row) {
return false
}
}
return true
func CompareAccountsMetadata(a AccountsMetadata, b AccountsMetadata) bool {
if len(a) != len(b) {
return false
}
counts := make(map[AccountMetadataRow]int, len(a))
for _, row := range a {
counts[row]++
}
for _, row := range b {
if counts[row] == 0 {
return false
}
counts[row]--
}
return true
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/interpreter/accounts_metadata.go` around lines 35 - 44, The
CompareAccountsMetadata function uses slices.Contains to check if elements from
slice a exist in slice b, but this approach only verifies presence and not
count, allowing unequal datasets with duplicate values to incorrectly return
true. Fix this by implementing proper multiset comparison that accounts for
element counts: either sort both AccountsMetadata slices and use slices.Equal
for direct comparison, or use a map-based approach to count occurrences of each
element in both slices and verify the counts match. Ensure the fix correctly
handles the case where slices have equal length but different element
frequencies.

}
28 changes: 28 additions & 0 deletions internal/interpreter/accounts_metadata_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package interpreter

import (
"testing"

"github.com/gkampitakis/go-snaps/snaps"
)

func TestPrettyPrintAccountsMetadata(t *testing.T) {
t.Run("without scope (no Scope column)", func(t *testing.T) {
meta := AccountsMetadata{
{Account: "alice", Key: "kyc", Value: "verified"},
{Account: "bob", Key: "tier", Value: "gold"},
}

snaps.MatchSnapshot(t, meta.PrettyPrint())
})

t.Run("with scope (Scope column shown)", func(t *testing.T) {
meta := AccountsMetadata{
{Account: "alice", Key: "kyc", Value: "verified"},
{Account: "alice", Scope: "eu", Key: "kyc", Value: "pending"},
{Account: "bob", Key: "tier", Value: "gold"},
}

snaps.MatchSnapshot(t, meta.PrettyPrint())
})
}
11 changes: 11 additions & 0 deletions internal/interpreter/append_scope.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package interpreter

import (
"regexp"
)

var scopeRegex = regexp.MustCompile(`^[a-z0-9_]*$`)

func validateScope(scope string) bool {
return scopeRegex.MatchString(scope)
}
36 changes: 36 additions & 0 deletions internal/interpreter/append_scope_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package interpreter

import (
"testing"

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

func TestAccountAddressString(t *testing.T) {
t.Run("no scope", func(t *testing.T) {
require.Equal(t, AccountAddress{Name: "acc"}.String(), "acc")
})

t.Run("with scope", func(t *testing.T) {
require.Equal(t, AccountAddress{Name: "acc", Scope: "xyz"}.String(), "acc/xyz")
})
}

func TestScopeValidation(t *testing.T) {
t.Run("valid scopes", func(t *testing.T) {
require.True(t, validateScope(""))
require.True(t, validateScope("myscope"))
require.True(t, validateScope("x"))
require.True(t, validateScope("x1"))
require.True(t, validateScope("my_scope_with_underscores"))
})

t.Run("invalid scopes", func(t *testing.T) {
require.False(t, validateScope("!"))
require.False(t, validateScope("$"))
require.False(t, validateScope("UPPERCASE"))
require.False(t, validateScope("dash-case"))
require.False(t, validateScope("colons:within"))
})

}
6 changes: 3 additions & 3 deletions internal/interpreter/args_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestParseValid(t *testing.T) {

p := NewArgsParser([]Value{
NewMonetaryInt(42),
AccountAddress("user:001"),
AccountAddress{Name: "user:001"},
})
a1 := parseArg(p, parser.Range{}, expectNumber)
a2 := parseArg(p, parser.Range{}, expectAccount)
Expand All @@ -48,15 +48,15 @@ func TestParseValid(t *testing.T) {
require.NotNil(t, a2, "a2 should not be nil")

require.Equal(t, a1, MonetaryInt(*big.NewInt(42)))
require.Equal(t, a2, AccountAddress("user:001"))
require.Equal(t, a2, AccountAddress{Name: "user:001"})
}

func TestParseBadType(t *testing.T) {
t.Parallel()

p := NewArgsParser([]Value{
NewMonetaryInt(42),
AccountAddress("user:001"),
AccountAddress{Name: "user:001"},
})
parseArg(p, parser.Range{}, expectMonetary)
parseArg(p, parser.Range{}, expectAccount)
Expand Down
Loading