From 8152bd28aa1ece3b94fa48de6a700d79dfc3d477 Mon Sep 17 00:00:00 2001 From: ascandone Date: Thu, 29 Aug 2024 11:11:56 +0200 Subject: [PATCH 01/10] feat: avoid tracking unbounded balances --- .../machine/script/compiler/compiler.go | 13 +++++++++++ .../machine/script/compiler/source.go | 23 +++++++++++++++++++ .../ledger/internal/machine/vm/machine.go | 2 +- .../internal/machine/vm/machine_test.go | 14 +++++++++-- .../internal/machine/vm/program/program.go | 1 + 5 files changed, 50 insertions(+), 3 deletions(-) diff --git a/components/ledger/internal/machine/script/compiler/compiler.go b/components/ledger/internal/machine/script/compiler/compiler.go index 207673918c..c3958ed417 100644 --- a/components/ledger/internal/machine/script/compiler/compiler.go +++ b/components/ledger/internal/machine/script/compiler/compiler.go @@ -26,6 +26,11 @@ type parseVisitor struct { varIdx map[string]machine.Address // needBalances store for each account, the set of assets needed neededBalances map[machine.Address]map[machine.Address]struct{} + + // like `sources`, but for accounts that aren't unbounded + // e.g. it doesn't include @world or accounts that appear within a + // `@acc allowing unbounded ovedraft` clause + boundedSources map[machine.Address]struct{} } // Allocates constants if it hasn't already been, @@ -680,6 +685,7 @@ func CompileFull(input string) CompileArtifacts { varIdx: make(map[string]machine.Address), neededBalances: make(map[machine.Address]map[machine.Address]struct{}), sources: map[machine.Address]struct{}{}, + boundedSources: map[machine.Address]struct{}{}, } err := visitor.VisitScript(tree) @@ -694,11 +700,18 @@ func CompileFull(input string) CompileArtifacts { } sort.Stable(sources) + boundedSources := make(machine.Addresses, 0) + for address := range visitor.boundedSources { + boundedSources = append(boundedSources, address) + } + sort.Stable(boundedSources) + artifacts.Program = &program.Program{ Instructions: visitor.instructions, Resources: visitor.resources, NeededBalances: visitor.neededBalances, Sources: sources, + BoundedSources: boundedSources, } return artifacts diff --git a/components/ledger/internal/machine/script/compiler/source.go b/components/ledger/internal/machine/script/compiler/source.go index 9db97a8d47..a4986f29cf 100644 --- a/components/ledger/internal/machine/script/compiler/source.go +++ b/components/ledger/internal/machine/script/compiler/source.go @@ -156,6 +156,29 @@ func (p *parseVisitor) VisitSource(c parser.ISourceContext, pushAsset func(), is fallback = &f } } + + isUnboundedOverdraft := func() bool { + if p.isWorld(*accAddr) { + return true + } + + if overdraft == nil { + return false + } + + switch overdraft.(type) { + case *parser.SrcAccountOverdraftUnboundedContext: + return true + case *parser.SrcAccountOverdraftSpecificContext: + return false + default: + panic("[unreachable]") + } + }() + if !isUnboundedOverdraft { + p.boundedSources[*accAddr] = struct{}{} + } + neededAccounts[*accAddr] = struct{}{} emptiedAccounts[*accAddr] = struct{}{} diff --git a/components/ledger/internal/machine/vm/machine.go b/components/ledger/internal/machine/vm/machine.go index 276fe94496..81f7fc7ebf 100644 --- a/components/ledger/internal/machine/vm/machine.go +++ b/components/ledger/internal/machine/vm/machine.go @@ -612,7 +612,7 @@ func (m *Machine) ResolveResources(ctx context.Context, store Store) ([]string, for _, accountAddress := range involvedAccountsMap { involvedAccounts = append(involvedAccounts, accountAddress) } - for _, machineAddress := range m.Program.Sources { + for _, machineAddress := range m.Program.BoundedSources { involvedSources = append(involvedSources, involvedAccountsMap[machineAddress]) } diff --git a/components/ledger/internal/machine/vm/machine_test.go b/components/ledger/internal/machine/vm/machine_test.go index c60b73d30f..cc6ad6eed3 100644 --- a/components/ledger/internal/machine/vm/machine_test.go +++ b/components/ledger/internal/machine/vm/machine_test.go @@ -924,9 +924,18 @@ func TestNeededBalances(t *testing.T) { } send [GEM 15] ( source = { + // normal accounts are tracked $a @b - @world + + // we don't want to track world, as it is an unbounded account + max [GEM 1] from @world + + // we want to lock bounded overdrafts account + @bounded allowing overdraft up to [GEM 1] + + // we don't want to lock unbounded overdrafts account + @unb allowing unbounded overdraft } destination = @c )`) @@ -943,8 +952,9 @@ func TestNeededBalances(t *testing.T) { if err != nil { t.Fatalf("did not expect error on SetVars, got: %v", err) } - _, _, err = m.ResolveResources(context.Background(), EmptyStore) + _, involvedSources, err := m.ResolveResources(context.Background(), EmptyStore) require.NoError(t, err) + require.Equal(t, []string{"a", "b", "bounded"}, involvedSources) err = m.ResolveBalances(context.Background(), EmptyStore) require.NoError(t, err) diff --git a/components/ledger/internal/machine/vm/program/program.go b/components/ledger/internal/machine/vm/program/program.go index 882b8202bb..7bc2051d03 100644 --- a/components/ledger/internal/machine/vm/program/program.go +++ b/components/ledger/internal/machine/vm/program/program.go @@ -13,6 +13,7 @@ type Program struct { Instructions []byte Resources []Resource Sources []machine.Address + BoundedSources []machine.Address NeededBalances map[machine.Address]map[machine.Address]struct{} } From 264318d9c67fecd2d6dd60636f449ccd85ae82fb Mon Sep 17 00:00:00 2001 From: ascandone Date: Thu, 29 Aug 2024 11:39:14 +0200 Subject: [PATCH 02/10] added test --- .../internal/machine/vm/machine_test.go | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/components/ledger/internal/machine/vm/machine_test.go b/components/ledger/internal/machine/vm/machine_test.go index cc6ad6eed3..5c721fe2f8 100644 --- a/components/ledger/internal/machine/vm/machine_test.go +++ b/components/ledger/internal/machine/vm/machine_test.go @@ -960,6 +960,32 @@ func TestNeededBalances(t *testing.T) { require.NoError(t, err) } +func TestNeededBalances2(t *testing.T) { + p, err := compiler.Compile(` + send [GEM 15] ( + source = { + // we want to track a balance even if it appears later on + // as an unbounded overdraft + max [GEM 1] from @a + @a allowing unbounded overdraft + } + destination = @c + )`) + + if err != nil { + t.Fatalf("did not expect error on Compile, got: %v", err) + } + + m := NewMachine(*p) + if err != nil { + t.Fatalf("did not expect error on SetVars, got: %v", err) + } + _, involvedSources, err := m.ResolveResources(context.Background(), EmptyStore) + require.NoError(t, err) + require.Equal(t, []string{"a"}, involvedSources) + +} + func TestSetTxMeta(t *testing.T) { p, err := compiler.Compile(` set_tx_meta("aaa", @platform) From 8f9d592ed4ed2344a12a0461666e6f501951f583 Mon Sep 17 00:00:00 2001 From: ascandone Date: Thu, 29 Aug 2024 14:49:45 +0200 Subject: [PATCH 03/10] chore: cleaned up code --- .../internal/engine/command/commander.go | 2 +- .../machine/script/compiler/source.go | 38 ++++++++++--------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/components/ledger/internal/engine/command/commander.go b/components/ledger/internal/engine/command/commander.go index 13fc302ac0..6ac007741c 100644 --- a/components/ledger/internal/engine/command/commander.go +++ b/components/ledger/internal/engine/command/commander.go @@ -141,7 +141,7 @@ func (commander *Commander) exec(ctx context.Context, parameters Parameters, scr worldFilter := collectionutils.FilterNot(collectionutils.FilterEq("world")) lockAccounts := Accounts{ Read: collectionutils.Filter(involvedAccounts, worldFilter), - Write: collectionutils.Filter(involvedSources, worldFilter), + Write: involvedSources, } unlock, err := func() (Unlock, error) { diff --git a/components/ledger/internal/machine/script/compiler/source.go b/components/ledger/internal/machine/script/compiler/source.go index a4986f29cf..ad3f201520 100644 --- a/components/ledger/internal/machine/script/compiler/source.go +++ b/components/ledger/internal/machine/script/compiler/source.go @@ -99,6 +99,25 @@ func (p *parseVisitor) TakeFromSource(fallback *FallbackAccount) error { return nil } +func (p parseVisitor) isOverdraftUnbounded(overdraftCtx parser.ISourceAccountOverdraftContext) bool { + if overdraftCtx == nil { + return false + } + + switch overdraftCtx.(type) { + case *parser.SrcAccountOverdraftUnboundedContext: + return true + case *parser.SrcAccountOverdraftSpecificContext: + return false + + default: + // even though this branch should be unreachable, + // we default to `false` instead of panicking + // in order to have a more conservative behaviour + return false + } +} + // VisitSource returns the resource addresses of all the accounts, // the addresses of accounts already emptied, // and possibly a fallback account if the source has an unbounded overdraft allowance or contains @world @@ -157,24 +176,7 @@ func (p *parseVisitor) VisitSource(c parser.ISourceContext, pushAsset func(), is } } - isUnboundedOverdraft := func() bool { - if p.isWorld(*accAddr) { - return true - } - - if overdraft == nil { - return false - } - - switch overdraft.(type) { - case *parser.SrcAccountOverdraftUnboundedContext: - return true - case *parser.SrcAccountOverdraftSpecificContext: - return false - default: - panic("[unreachable]") - } - }() + isUnboundedOverdraft := p.isWorld(*accAddr) || p.isOverdraftUnbounded(overdraft) if !isUnboundedOverdraft { p.boundedSources[*accAddr] = struct{}{} } From 98eb7287731f55eb7b8266013dff0a8728f61c68 Mon Sep 17 00:00:00 2001 From: ascandone Date: Thu, 29 Aug 2024 15:00:04 +0200 Subject: [PATCH 04/10] chore: simplified code --- .../ledger/internal/engine/command/commander.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/components/ledger/internal/engine/command/commander.go b/components/ledger/internal/engine/command/commander.go index 6ac007741c..c245e694ff 100644 --- a/components/ledger/internal/engine/command/commander.go +++ b/components/ledger/internal/engine/command/commander.go @@ -126,16 +126,9 @@ func (commander *Commander) exec(ctx context.Context, parameters Parameters, scr return nil, NewErrCompilationFailed(err) } - involvedAccounts, involvedSources, err := func() ([]string, []string, error) { - involvedAccounts, involvedSources, err := m.ResolveResources(ctx, commander.store) - if err != nil { - return nil, nil, NewErrCompilationFailed(err) - } - - return involvedAccounts, involvedSources, nil - }() + involvedAccounts, involvedSources, err := m.ResolveResources(ctx, commander.store) if err != nil { - return nil, err + return nil, NewErrCompilationFailed(err) } worldFilter := collectionutils.FilterNot(collectionutils.FilterEq("world")) From df6dc3fd47a41868225b06dd8c7a3e9f6f1c9c0f Mon Sep 17 00:00:00 2001 From: ascandone Date: Fri, 30 Aug 2024 14:50:53 +0200 Subject: [PATCH 05/10] feat: more precise realock/writelock accounts --- .../internal/engine/command/commander.go | 9 +- .../machine/script/compiler/compiler.go | 54 ++++++------ .../machine/script/compiler/compiler_test.go | 4 +- .../machine/script/compiler/destination.go | 3 +- .../machine/script/compiler/source.go | 12 ++- .../ledger/internal/machine/vm/machine.go | 31 +++---- .../internal/machine/vm/machine_test.go | 82 ++++++++++++++++++- .../internal/machine/vm/program/program.go | 5 +- 8 files changed, 144 insertions(+), 56 deletions(-) diff --git a/components/ledger/internal/engine/command/commander.go b/components/ledger/internal/engine/command/commander.go index c245e694ff..2f41c8c897 100644 --- a/components/ledger/internal/engine/command/commander.go +++ b/components/ledger/internal/engine/command/commander.go @@ -17,7 +17,6 @@ import ( "github.com/formancehq/ledger/internal/bus" "github.com/formancehq/ledger/internal/engine/utils/batching" "github.com/formancehq/ledger/internal/machine/vm" - "github.com/formancehq/stack/libs/go-libs/collectionutils" "github.com/formancehq/stack/libs/go-libs/metadata" "github.com/pkg/errors" ) @@ -126,15 +125,13 @@ func (commander *Commander) exec(ctx context.Context, parameters Parameters, scr return nil, NewErrCompilationFailed(err) } - involvedAccounts, involvedSources, err := m.ResolveResources(ctx, commander.store) + readLockAccounts, writeLockAccounts, err := m.ResolveResources(ctx, commander.store) if err != nil { return nil, NewErrCompilationFailed(err) } - - worldFilter := collectionutils.FilterNot(collectionutils.FilterEq("world")) lockAccounts := Accounts{ - Read: collectionutils.Filter(involvedAccounts, worldFilter), - Write: involvedSources, + Read: readLockAccounts, + Write: writeLockAccounts, } unlock, err := func() (Unlock, error) { diff --git a/components/ledger/internal/machine/script/compiler/compiler.go b/components/ledger/internal/machine/script/compiler/compiler.go index c3958ed417..7507963233 100644 --- a/components/ledger/internal/machine/script/compiler/compiler.go +++ b/components/ledger/internal/machine/script/compiler/compiler.go @@ -27,10 +27,14 @@ type parseVisitor struct { // needBalances store for each account, the set of assets needed neededBalances map[machine.Address]map[machine.Address]struct{} - // like `sources`, but for accounts that aren't unbounded - // e.g. it doesn't include @world or accounts that appear within a - // `@acc allowing unbounded ovedraft` clause - boundedSources map[machine.Address]struct{} + // The sources accounts that aren't unbounded + // that is, @world or sources that appear within a + // '.. allowing unboundeed overdraft' clause + writeLockAccounts map[machine.Address]struct{} + + // all the accounts that appear in either the destination + // or in the balance() function + readLockAccounts map[machine.Address]struct{} } // Allocates constants if it hasn't already been, @@ -585,6 +589,7 @@ func (p *parseVisitor) VisitVars(c *parser.VarListDeclContext) *CompileError { Account: *accAddr, Asset: *assAddr, }) + p.readLockAccounts[*accAddr] = struct{}{} if err != nil { return LogicError(c, err) } @@ -679,13 +684,14 @@ func CompileFull(input string) CompileArtifacts { } visitor := parseVisitor{ - errListener: errListener, - instructions: make([]byte, 0), - resources: make([]program.Resource, 0), - varIdx: make(map[string]machine.Address), - neededBalances: make(map[machine.Address]map[machine.Address]struct{}), - sources: map[machine.Address]struct{}{}, - boundedSources: map[machine.Address]struct{}{}, + errListener: errListener, + instructions: make([]byte, 0), + resources: make([]program.Resource, 0), + varIdx: make(map[string]machine.Address), + neededBalances: make(map[machine.Address]map[machine.Address]struct{}), + sources: map[machine.Address]struct{}{}, + writeLockAccounts: map[machine.Address]struct{}{}, + readLockAccounts: map[machine.Address]struct{}{}, } err := visitor.VisitScript(tree) @@ -694,24 +700,24 @@ func CompileFull(input string) CompileArtifacts { return artifacts } - sources := make(machine.Addresses, 0) - for address := range visitor.sources { - sources = append(sources, address) + readLockAccounts := make(machine.Addresses, 0) + for address := range visitor.readLockAccounts { + readLockAccounts = append(readLockAccounts, address) } - sort.Stable(sources) + sort.Stable(readLockAccounts) - boundedSources := make(machine.Addresses, 0) - for address := range visitor.boundedSources { - boundedSources = append(boundedSources, address) + writeLockAccounts := make(machine.Addresses, 0) + for address := range visitor.writeLockAccounts { + writeLockAccounts = append(writeLockAccounts, address) } - sort.Stable(boundedSources) + sort.Stable(writeLockAccounts) artifacts.Program = &program.Program{ - Instructions: visitor.instructions, - Resources: visitor.resources, - NeededBalances: visitor.neededBalances, - Sources: sources, - BoundedSources: boundedSources, + Instructions: visitor.instructions, + Resources: visitor.resources, + NeededBalances: visitor.neededBalances, + ReadLockAccounts: readLockAccounts, + WriteLockAccounts: writeLockAccounts, } return artifacts diff --git a/components/ledger/internal/machine/script/compiler/compiler_test.go b/components/ledger/internal/machine/script/compiler/compiler_test.go index b8b02870d8..a3b694022d 100644 --- a/components/ledger/internal/machine/script/compiler/compiler_test.go +++ b/components/ledger/internal/machine/script/compiler/compiler_test.go @@ -1045,7 +1045,7 @@ func TestSetAccountMeta(t *testing.T) { program2.OP_ASSET, program2.OP_APUSH, 04, 00, program2.OP_MONETARY_NEW, - program2.OP_TAKE_ALL, + program2.OP_TAKE_ALWAYS, program2.OP_APUSH, 02, 00, program2.OP_TAKE_MAX, program2.OP_APUSH, 05, 00, @@ -1177,7 +1177,7 @@ func TestVariableBalance(t *testing.T) { program2.OP_ASSET, program2.OP_APUSH, 04, 00, program2.OP_MONETARY_NEW, - program2.OP_TAKE_ALL, + program2.OP_TAKE_ALWAYS, program2.OP_APUSH, 02, 00, program2.OP_TAKE_MAX, program2.OP_APUSH, 05, 00, diff --git a/components/ledger/internal/machine/script/compiler/destination.go b/components/ledger/internal/machine/script/compiler/destination.go index 6b941143ee..52491be2ce 100644 --- a/components/ledger/internal/machine/script/compiler/destination.go +++ b/components/ledger/internal/machine/script/compiler/destination.go @@ -23,7 +23,7 @@ func (p *parseVisitor) VisitDestinationRecursive(c parser.IDestinationContext) * case *parser.DestAccountContext: p.AppendInstruction(program.OP_FUNDING_SUM) p.AppendInstruction(program.OP_TAKE) - ty, _, err := p.VisitExpr(c.Expression(), true) + ty, destAddr, err := p.VisitExpr(c.Expression(), true) if err != nil { return err } @@ -32,6 +32,7 @@ func (p *parseVisitor) VisitDestinationRecursive(c parser.IDestinationContext) * errors.New("wrong type: expected account as destination"), ) } + p.readLockAccounts[*destAddr] = struct{}{} p.AppendInstruction(program.OP_SEND) return nil case *parser.DestInOrderContext: diff --git a/components/ledger/internal/machine/script/compiler/source.go b/components/ledger/internal/machine/script/compiler/source.go index ad3f201520..a3d3dc0fb3 100644 --- a/components/ledger/internal/machine/script/compiler/source.go +++ b/components/ledger/internal/machine/script/compiler/source.go @@ -148,7 +148,11 @@ func (p *parseVisitor) VisitSource(c parser.ISourceContext, pushAsset func(), is return nil, nil, nil, LogicError(c, err) } p.AppendInstruction(program.OP_MONETARY_NEW) - p.AppendInstruction(program.OP_TAKE_ALL) + if p.isWorld(*accAddr) { + p.AppendInstruction(program.OP_TAKE_ALWAYS) + } else { + p.AppendInstruction(program.OP_TAKE_ALL) + } } else { if p.isWorld(*accAddr) { return nil, nil, nil, LogicError(c, errors.New("@world is already set to an unbounded overdraft")) @@ -170,7 +174,7 @@ func (p *parseVisitor) VisitSource(c parser.ISourceContext, pushAsset func(), is return nil, nil, nil, LogicError(c, err) } p.AppendInstruction(program.OP_MONETARY_NEW) - p.AppendInstruction(program.OP_TAKE_ALL) + p.AppendInstruction(program.OP_TAKE_ALWAYS) f := FallbackAccount(*accAddr) fallback = &f } @@ -178,10 +182,10 @@ func (p *parseVisitor) VisitSource(c parser.ISourceContext, pushAsset func(), is isUnboundedOverdraft := p.isWorld(*accAddr) || p.isOverdraftUnbounded(overdraft) if !isUnboundedOverdraft { - p.boundedSources[*accAddr] = struct{}{} + p.writeLockAccounts[*accAddr] = struct{}{} + neededAccounts[*accAddr] = struct{}{} } - neededAccounts[*accAddr] = struct{}{} emptiedAccounts[*accAddr] = struct{}{} if fallback != nil && isAll { diff --git a/components/ledger/internal/machine/vm/machine.go b/components/ledger/internal/machine/vm/machine.go index 81f7fc7ebf..0715b415f0 100644 --- a/components/ledger/internal/machine/vm/machine.go +++ b/components/ledger/internal/machine/vm/machine.go @@ -139,16 +139,16 @@ func (m *Machine) withdrawAlways(account machine.AccountAddress, mon machine.Mon if accBalance, ok := m.Balances[account]; ok { if balance, ok := accBalance[mon.Asset]; ok { accBalance[mon.Asset] = balance.Sub(mon.Amount) - return &machine.Funding{ - Asset: mon.Asset, - Parts: []machine.FundingPart{{ - Account: account, - Amount: mon.Amount, - }}, - }, nil } } - return nil, fmt.Errorf("missing %v balance from %v", mon.Asset, account) + + return &machine.Funding{ + Asset: mon.Asset, + Parts: []machine.FundingPart{{ + Account: account, + Amount: mon.Amount, + }}, + }, nil } func (m *Machine) credit(account machine.AccountAddress, funding machine.Funding) { @@ -607,16 +607,17 @@ func (m *Machine) ResolveResources(ctx context.Context, store Store) ([]string, m.Resources = append(m.Resources, val) } - involvedAccounts := make([]string, 0) - involvedSources := make([]string, 0) - for _, accountAddress := range involvedAccountsMap { - involvedAccounts = append(involvedAccounts, accountAddress) + readLockAccounts := make([]string, 0) + for _, accountAddress := range m.Program.ReadLockAccounts { + readLockAccounts = append(readLockAccounts, involvedAccountsMap[accountAddress]) } - for _, machineAddress := range m.Program.BoundedSources { - involvedSources = append(involvedSources, involvedAccountsMap[machineAddress]) + + writeLockAccounts := make([]string, 0) + for _, machineAddress := range m.Program.WriteLockAccounts { + writeLockAccounts = append(writeLockAccounts, involvedAccountsMap[machineAddress]) } - return involvedAccounts, involvedSources, nil + return readLockAccounts, writeLockAccounts, nil } func (m *Machine) SetVarsFromJSON(vars map[string]string) error { diff --git a/components/ledger/internal/machine/vm/machine_test.go b/components/ledger/internal/machine/vm/machine_test.go index 5c721fe2f8..8d31ba0494 100644 --- a/components/ledger/internal/machine/vm/machine_test.go +++ b/components/ledger/internal/machine/vm/machine_test.go @@ -528,6 +528,59 @@ func TestWorldSource(t *testing.T) { test(t, tc) } +func TestUnboundedSourceSimple(t *testing.T) { + tc := NewTestCase() + tc.compile(t, `send [GEM 15] ( + source = @unbounded allowing unbounded overdraft + destination = @b + )`) + tc.expected = CaseResult{ + Printed: []machine.Value{}, + Postings: []Posting{ + + { + Asset: "GEM", + Amount: machine.NewMonetaryInt(15), + Source: "unbounded", + Destination: "b", + }, + }, + Error: nil, + } + test(t, tc) +} + +func TestUnboundedSourceInorder(t *testing.T) { + tc := NewTestCase() + tc.compile(t, `send [GEM 15] ( + source = { + @a + @unbounded allowing unbounded overdraft + } + destination = @b + )`) + tc.setBalance("a", "GEM", 1) + tc.expected = CaseResult{ + Printed: []machine.Value{}, + Postings: []Posting{ + { + Asset: "GEM", + Amount: machine.NewMonetaryInt(1), + Source: "a", + Destination: "b", + }, + { + Asset: "GEM", + Amount: machine.NewMonetaryInt(14), + Source: "unbounded", + Destination: "b", + }, + }, + Error: nil, + } + test(t, tc) +} + func TestNoEmptyPostings(t *testing.T) { tc := NewTestCase() tc.compile(t, `send [GEM 2] ( @@ -952,9 +1005,10 @@ func TestNeededBalances(t *testing.T) { if err != nil { t.Fatalf("did not expect error on SetVars, got: %v", err) } - _, involvedSources, err := m.ResolveResources(context.Background(), EmptyStore) + readLockAccounts, writeLockAccounts, err := m.ResolveResources(context.Background(), EmptyStore) require.NoError(t, err) - require.Equal(t, []string{"a", "b", "bounded"}, involvedSources) + require.Equalf(t, []string{"c"}, readLockAccounts, "readlock") + require.Equalf(t, []string{"a", "b", "bounded"}, writeLockAccounts, "writelock") err = m.ResolveBalances(context.Background(), EmptyStore) require.NoError(t, err) @@ -986,6 +1040,30 @@ func TestNeededBalances2(t *testing.T) { } +func TestNeededBalancesBalanceFn(t *testing.T) { + p, err := compiler.Compile(`vars { + monetary $balance = balance(@acc, COIN) +} + +send $balance ( + source = @a + destination = @b +)`) + + if err != nil { + t.Fatalf("did not expect error on Compile, got: %v", err) + } + + m := NewMachine(*p) + if err != nil { + t.Fatalf("did not expect error on SetVars, got: %v", err) + } + rlAccounts, wlAccounts, err := m.ResolveResources(context.Background(), EmptyStore) + require.NoError(t, err) + require.Equal(t, []string{"a"}, wlAccounts) + require.Equal(t, []string{"acc", "b"}, rlAccounts) +} + func TestSetTxMeta(t *testing.T) { p, err := compiler.Compile(` set_tx_meta("aaa", @platform) diff --git a/components/ledger/internal/machine/vm/program/program.go b/components/ledger/internal/machine/vm/program/program.go index 7bc2051d03..fb93e8f9ae 100644 --- a/components/ledger/internal/machine/vm/program/program.go +++ b/components/ledger/internal/machine/vm/program/program.go @@ -12,9 +12,10 @@ import ( type Program struct { Instructions []byte Resources []Resource - Sources []machine.Address - BoundedSources []machine.Address NeededBalances map[machine.Address]map[machine.Address]struct{} + + ReadLockAccounts []machine.Address + WriteLockAccounts []machine.Address } func (p Program) String() string { From 432965098ff6dd16373cd6006752267ebee34a2a Mon Sep 17 00:00:00 2001 From: ascandone Date: Fri, 30 Aug 2024 15:05:42 +0200 Subject: [PATCH 06/10] test: asserting requested balances --- .../internal/machine/vm/machine_test.go | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/components/ledger/internal/machine/vm/machine_test.go b/components/ledger/internal/machine/vm/machine_test.go index 8d31ba0494..a78b93e2d8 100644 --- a/components/ledger/internal/machine/vm/machine_test.go +++ b/components/ledger/internal/machine/vm/machine_test.go @@ -1010,8 +1010,11 @@ func TestNeededBalances(t *testing.T) { require.Equalf(t, []string{"c"}, readLockAccounts, "readlock") require.Equalf(t, []string{"a", "b", "bounded"}, writeLockAccounts, "writelock") - err = m.ResolveBalances(context.Background(), EmptyStore) + store := mockStore{} + err = m.ResolveBalances(context.Background(), &store) require.NoError(t, err) + + require.Equal(t, []string{"a", "b", "bounded"}, store.RequestedAccounts) } func TestNeededBalances2(t *testing.T) { @@ -1062,6 +1065,11 @@ send $balance ( require.NoError(t, err) require.Equal(t, []string{"a"}, wlAccounts) require.Equal(t, []string{"acc", "b"}, rlAccounts) + + store := mockStore{} + err = m.ResolveBalances(context.Background(), &store) + require.NoError(t, err) + require.Equal(t, []string{"acc", "a"}, store.RequestedAccounts) } func TestSetTxMeta(t *testing.T) { @@ -2295,3 +2303,16 @@ send [COIN 100] ( } test(t, tc) } + +type mockStore struct { + RequestedAccounts []string +} + +func (s *mockStore) GetBalance(ctx context.Context, address, asset string) (*big.Int, error) { + s.RequestedAccounts = append(s.RequestedAccounts, address) + return big.NewInt(0), nil +} + +func (s *mockStore) GetAccount(ctx context.Context, address string) (*ledger.Account, error) { + panic("not implemented") +} From 1c3e608fba9a148b5f70ea2f77f0ba8acda0ee5c Mon Sep 17 00:00:00 2001 From: ascandone Date: Fri, 30 Aug 2024 16:27:50 +0200 Subject: [PATCH 07/10] fix: adding locks to accounts in metadata --- .../ledger/internal/machine/vm/machine.go | 6 +++ .../internal/machine/vm/machine_test.go | 43 ++++++++++++++++--- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/components/ledger/internal/machine/vm/machine.go b/components/ledger/internal/machine/vm/machine.go index 0715b415f0..7286a69dfc 100644 --- a/components/ledger/internal/machine/vm/machine.go +++ b/components/ledger/internal/machine/vm/machine.go @@ -13,6 +13,7 @@ import ( "encoding/binary" "fmt" "math/big" + "slices" "github.com/formancehq/ledger/internal/machine" @@ -574,6 +575,9 @@ func (m *Machine) ResolveResources(ctx context.Context, store Store) ([]string, if err != nil { return nil, nil, err } + if val.GetType() == machine.TypeAccount { + involvedAccountsMap[machine.Address(idx)] = string(val.(machine.AccountAddress)) + } case program.VariableAccountBalance: acc, _ := m.getResource(res.Account) address := string((*acc).(machine.AccountAddress)) @@ -617,6 +621,8 @@ func (m *Machine) ResolveResources(ctx context.Context, store Store) ([]string, writeLockAccounts = append(writeLockAccounts, involvedAccountsMap[machineAddress]) } + slices.Sort(readLockAccounts) + slices.Sort(writeLockAccounts) return readLockAccounts, writeLockAccounts, nil } diff --git a/components/ledger/internal/machine/vm/machine_test.go b/components/ledger/internal/machine/vm/machine_test.go index a78b93e2d8..68c5fc47e0 100644 --- a/components/ledger/internal/machine/vm/machine_test.go +++ b/components/ledger/internal/machine/vm/machine_test.go @@ -1034,9 +1034,6 @@ func TestNeededBalances2(t *testing.T) { } m := NewMachine(*p) - if err != nil { - t.Fatalf("did not expect error on SetVars, got: %v", err) - } _, involvedSources, err := m.ResolveResources(context.Background(), EmptyStore) require.NoError(t, err) require.Equal(t, []string{"a"}, involvedSources) @@ -1058,9 +1055,6 @@ send $balance ( } m := NewMachine(*p) - if err != nil { - t.Fatalf("did not expect error on SetVars, got: %v", err) - } rlAccounts, wlAccounts, err := m.ResolveResources(context.Background(), EmptyStore) require.NoError(t, err) require.Equal(t, []string{"a"}, wlAccounts) @@ -1072,6 +1066,43 @@ send $balance ( require.Equal(t, []string{"acc", "a"}, store.RequestedAccounts) } +func TestNeededBalancesBalanceOfMeta(t *testing.T) { + p, err := compiler.Compile(`vars { + account $src = meta(@x, "k") +} + +send [COIN 1] ( + source = $src + destination = @dest +)`) + + if err != nil { + t.Fatalf("did not expect error on Compile, got: %v", err) + } + m := NewMachine(*p) + + staticStore := StaticStore{ + "x": &AccountWithBalances{ + Account: ledger.Account{ + Address: "x", + Metadata: metadata.Metadata{ + "k": "src", + }, + }, + Balances: map[string]*big.Int{}, + }, + } + rlAccounts, wlAccounts, err := m.ResolveResources(context.Background(), staticStore) + require.NoError(t, err) + require.Equal(t, []string{"src"}, wlAccounts) + require.Equal(t, []string{"dest"}, rlAccounts) + + store := mockStore{} + err = m.ResolveBalances(context.Background(), &store) + require.NoError(t, err) + require.Equal(t, []string{"src"}, store.RequestedAccounts) +} + func TestSetTxMeta(t *testing.T) { p, err := compiler.Compile(` set_tx_meta("aaa", @platform) From 9e7d8b7cf9fe7d0a3e0b20bfae287668b128fcd1 Mon Sep 17 00:00:00 2001 From: ascandone Date: Fri, 30 Aug 2024 16:56:01 +0200 Subject: [PATCH 08/10] fix: fix flaky tests --- .../ledger/internal/machine/vm/machine_test.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/components/ledger/internal/machine/vm/machine_test.go b/components/ledger/internal/machine/vm/machine_test.go index 68c5fc47e0..c74e162897 100644 --- a/components/ledger/internal/machine/vm/machine_test.go +++ b/components/ledger/internal/machine/vm/machine_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "math/big" + "slices" "sync" "testing" @@ -1014,7 +1015,7 @@ func TestNeededBalances(t *testing.T) { err = m.ResolveBalances(context.Background(), &store) require.NoError(t, err) - require.Equal(t, []string{"a", "b", "bounded"}, store.RequestedAccounts) + require.Equal(t, []string{"a", "b", "bounded"}, store.GetRequestedAccounts()) } func TestNeededBalances2(t *testing.T) { @@ -1063,7 +1064,7 @@ send $balance ( store := mockStore{} err = m.ResolveBalances(context.Background(), &store) require.NoError(t, err) - require.Equal(t, []string{"acc", "a"}, store.RequestedAccounts) + require.Equal(t, []string{"a", "acc"}, store.GetRequestedAccounts()) } func TestNeededBalancesBalanceOfMeta(t *testing.T) { @@ -1100,7 +1101,7 @@ send [COIN 1] ( store := mockStore{} err = m.ResolveBalances(context.Background(), &store) require.NoError(t, err) - require.Equal(t, []string{"src"}, store.RequestedAccounts) + require.Equal(t, []string{"src"}, store.GetRequestedAccounts()) } func TestSetTxMeta(t *testing.T) { @@ -2336,11 +2337,16 @@ send [COIN 100] ( } type mockStore struct { - RequestedAccounts []string + requestedAccounts []string +} + +func (s *mockStore) GetRequestedAccounts() []string { + slices.Sort(s.requestedAccounts) + return s.requestedAccounts } func (s *mockStore) GetBalance(ctx context.Context, address, asset string) (*big.Int, error) { - s.RequestedAccounts = append(s.RequestedAccounts, address) + s.requestedAccounts = append(s.requestedAccounts, address) return big.NewInt(0), nil } From c4f59ef2aa820ff35f0cd37afb24d372fff5ea2f Mon Sep 17 00:00:00 2001 From: ascandone Date: Fri, 30 Aug 2024 17:23:49 +0200 Subject: [PATCH 09/10] feat: excluded @world from readLockAccounts --- .../ledger/internal/machine/script/compiler/destination.go | 4 +++- components/ledger/internal/machine/vm/machine_test.go | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/components/ledger/internal/machine/script/compiler/destination.go b/components/ledger/internal/machine/script/compiler/destination.go index 52491be2ce..1b454c13fa 100644 --- a/components/ledger/internal/machine/script/compiler/destination.go +++ b/components/ledger/internal/machine/script/compiler/destination.go @@ -32,7 +32,9 @@ func (p *parseVisitor) VisitDestinationRecursive(c parser.IDestinationContext) * errors.New("wrong type: expected account as destination"), ) } - p.readLockAccounts[*destAddr] = struct{}{} + if !p.isWorld(*destAddr) { + p.readLockAccounts[*destAddr] = struct{}{} + } p.AppendInstruction(program.OP_SEND) return nil case *parser.DestInOrderContext: diff --git a/components/ledger/internal/machine/vm/machine_test.go b/components/ledger/internal/machine/vm/machine_test.go index c74e162897..cc6d269c88 100644 --- a/components/ledger/internal/machine/vm/machine_test.go +++ b/components/ledger/internal/machine/vm/machine_test.go @@ -991,7 +991,10 @@ func TestNeededBalances(t *testing.T) { // we don't want to lock unbounded overdrafts account @unb allowing unbounded overdraft } - destination = @c + destination = { + max [GEM 1] to @c + remaining to @world + } )`) if err != nil { From 9e715712108868c699deb15876a46311f16eb925 Mon Sep 17 00:00:00 2001 From: ascandone Date: Mon, 2 Sep 2024 15:38:23 +0200 Subject: [PATCH 10/10] fix: fix user issue --- .../ledger/internal/machine/vm/machine.go | 12 +++- .../internal/machine/vm/machine_test.go | 59 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/components/ledger/internal/machine/vm/machine.go b/components/ledger/internal/machine/vm/machine.go index 7286a69dfc..c6b6123ec5 100644 --- a/components/ledger/internal/machine/vm/machine.go +++ b/components/ledger/internal/machine/vm/machine.go @@ -171,8 +171,16 @@ func (m *Machine) repay(funding machine.Funding) { if part.Account == "world" { continue } - balance := m.Balances[part.Account][funding.Asset] - m.Balances[part.Account][funding.Asset] = balance.Add(part.Amount) + accountBalance, ok := m.Balances[part.Account] + if !ok { + // no asset: the source has to be an unbounded source + // which NEVER appears as bounded + // this means we don't need to track it's balance + continue + } + + balance := accountBalance[funding.Asset] + accountBalance[funding.Asset] = balance.Add(part.Amount) } } diff --git a/components/ledger/internal/machine/vm/machine_test.go b/components/ledger/internal/machine/vm/machine_test.go index cc6d269c88..e2de91469c 100644 --- a/components/ledger/internal/machine/vm/machine_test.go +++ b/components/ledger/internal/machine/vm/machine_test.go @@ -2339,6 +2339,65 @@ send [COIN 100] ( test(t, tc) } +func TestRepayUnboundedMinimal(t *testing.T) { + tc := NewTestCase() + + tc.compile(t, ` +send [COIN 100]( + source = @src allowing unbounded overdraft + destination = { + max [COIN 1] to @d1 + remaining to @d2 + } + ) +`) + + tc.expected = CaseResult{ + Printed: []machine.Value{}, + Postings: []Posting{ + {"src", "d1", machine.NewMonetaryInt(1), "COIN"}, + {"src", "d2", machine.NewMonetaryInt(99), "COIN"}, + }, + } + test(t, tc) +} + +func TestRepayUnboundedComplex(t *testing.T) { + tc := NewTestCase() + + tc.compile(t, ` +send [EGP 86640]( + source = { + max [EGP 86640] from @asset:current_assets allowing unbounded overdraft + } + destination = { + max [EGP 86466] to @liability:client_balances + max [EGP 9] to @liability:current_liabilities:1 + max [EGP 9] to @liability:current_liabilities:2 + max [EGP 100] to @liability:current_liabilities:3 + max [EGP 4] to @liability:current_liabilities:4 + max [EGP 43] to @liability:current_liabilities:checks:5 + remaining to @liability:current_liabilities:6 + } + ) + +`) + + tc.expected = CaseResult{ + Printed: []machine.Value{}, + Postings: []Posting{ + {"asset:current_assets", "liability:client_balances", machine.NewMonetaryInt(86466), "EGP"}, + {"asset:current_assets", "liability:current_liabilities:1", machine.NewMonetaryInt(9), "EGP"}, + {"asset:current_assets", "liability:current_liabilities:2", machine.NewMonetaryInt(9), "EGP"}, + {"asset:current_assets", "liability:current_liabilities:3", machine.NewMonetaryInt(100), "EGP"}, + {"asset:current_assets", "liability:current_liabilities:4", machine.NewMonetaryInt(4), "EGP"}, + {"asset:current_assets", "liability:current_liabilities:checks:5", machine.NewMonetaryInt(43), "EGP"}, + {"asset:current_assets", "liability:current_liabilities:6", machine.NewMonetaryInt(9), "EGP"}, + }, + } + test(t, tc) +} + type mockStore struct { requestedAccounts []string }