diff --git a/bytecode-verifier.md b/bytecode-verifier.md index 8bfbdd2f..0244d2a4 100644 --- a/bytecode-verifier.md +++ b/bytecode-verifier.md @@ -83,6 +83,18 @@ verified, so `ErrNoOpenMark` and the INVARIANT documented on `MarkEnd` stay. it is a legitimate user-facing error (`InvalidUncappedSource`, "unbounded source is not allowed here"), not a malformed program. Deliberately left to run time. +### Funds left in the queue + +A pull debits its source at once; the posting only exists once a send drains the +queue. Funds still queued when `Op_SetCurrentAsset` runs would be posted under +the new asset, and funds still queued when the run ends are a debit that never +became a posting. Whether a capped send empties the queue depends on the +balances, so neither is decidable from the instruction stream: `Exec` checks +both at run time (`errSetAssetWithQueuedFunds`, `errEndWithQueuedFunds`, via +`runstate.HasQueuedFunds()`). Compiled numscript never trips them: every send +statement either pulls exactly what it sends (`Op_CheckEnoughFunds`) or sends +everything it pulled. + ### Anything semantic A verified program can still produce nonsense: allotment portions that don't sum diff --git a/instruction-encoding.md b/instruction-encoding.md index bd026985..31f5b545 100644 --- a/instruction-encoding.md +++ b/instruction-encoding.md @@ -27,7 +27,7 @@ Instructions are **4 bytes** wide: `[Opcode: 8] [A: 8] [B: 8] [C: 8]`. 00x00SET_CURRENT_ASSET asset-- - Sets the current asset (used by PULL_ACCOUNT / SEND_TO_ACCOUNT) from str_regs[A] + Sets the current asset (used by PULL_ACCOUNT / SEND_TO_ACCOUNT) from str_regs[A]. Traps while a mark is open, or while the source queue still holds funds pulled under the previous asset 10x01ASSERT_SAME_ASSET diff --git a/internal/funds/funds.go b/internal/funds/funds.go index ab26f93c..ec043698 100644 --- a/internal/funds/funds.go +++ b/internal/funds/funds.go @@ -244,9 +244,15 @@ func (s *RunState) GetAccountBalance(account, scope, asset, color string) (*big. return new(big.Int).Set(bal), nil } +// HasQueuedFunds reports whether any source is still queued. Every queued +// source holds a positive amount: Pull and PullUncapped never queue a zero. +func (s *RunState) HasQueuedFunds() bool { + return len(s.sources) > 0 +} + // Pull debits up to cap from src's (currentAsset, color) balance, queues the -// pulled amount as a funding source tagged with color, and writes the amount made -// available into out: +// pulled amount as a funding source tagged with color (only when positive), and +// writes the amount made available into out: // // overdraft == nil -> unbounded: available = max(0, cap) // overdraft == b -> available = min(max(0, balance + max(0,b)), max(0, cap)) @@ -263,8 +269,7 @@ func (s *RunState) Pull(out *big.Int, src string, scope string, cap *big.Int, ov if out.Sign() < 0 { out.SetInt64(0) } - amt := new(big.Int).Set(out) - s.sources = append(s.sources, source{src, scope, amt, color}) + s.enqueue(src, scope, out, color) e := s.entryFor(PairKey{src, scope, s.currentAsset, color}) e.amount.Sub(&e.amount, out) return nil @@ -292,11 +297,7 @@ func (s *RunState) Pull(out *big.Int, src string, scope string, cap *big.Int, ov out.SetInt64(0) } - // an independent copy: out stays the caller's, while the queued amount is - // mutated in place by compactAt/Send - amt := new(big.Int).Set(out) - s.sources = append(s.sources, source{src, scope, amt, color}) - + s.enqueue(src, scope, out, color) currentBal.Sub(currentBal, out) return nil } @@ -321,14 +322,20 @@ func (s *RunState) PullUncapped(out *big.Int, src string, scope string, overdraf out.SetInt64(0) } - if out.Sign() > 0 { - amt := new(big.Int).Set(out) - s.sources = append(s.sources, source{src, scope, amt, color}) - currentBal.Sub(currentBal, out) // debit in place; cache keeps the pointer - } + s.enqueue(src, scope, out, color) + currentBal.Sub(currentBal, out) // debit in place; cache keeps the pointer return nil } +// enqueue queues a copy of amount, so the caller keeps amount while the queued +// one is mutated in place by compactAt/Send. A zero amount is not queued. +func (s *RunState) enqueue(src, scope string, amount *big.Int, color string) { + if amount.Sign() > 0 { + out := new(big.Int).Set(amount) + s.sources = append(s.sources, source{src, scope, out, color}) + } +} + // Send drains queued funding sources in FIFO order until cap is satisfied or // eligible sources run out. Each emitted posting carries the *consumed source's* // own color. diff --git a/internal/vm/export_test.go b/internal/vm/export_test.go index 77388479..c7a3ffed 100644 --- a/internal/vm/export_test.go +++ b/internal/vm/export_test.go @@ -1,3 +1,7 @@ package vm -var ErrSendWhileMarkOpen = errSendWhileMarkOpen +var ( + ErrSendWhileMarkOpen = errSendWhileMarkOpen + ErrSetAssetWithQueuedFunds = errSetAssetWithQueuedFunds + ErrEndWithQueuedFunds = errEndWithQueuedFunds +) diff --git a/internal/vm/instruction.go b/internal/vm/instruction.go index 5c4c0a52..a6e4fe7d 100644 --- a/internal/vm/instruction.go +++ b/internal/vm/instruction.go @@ -36,6 +36,8 @@ type Opcode byte // a category without renumbering. See instruction-encoding.md. const ( // --- state & assertions (0x00) --- + // InternalError while a mark is open or while the source queue still holds + // funds pulled under the previous asset Op_SetCurrentAsset Opcode = 0x00 Op_AssertSameAsset Opcode = 0x01 diff --git a/internal/vm/ir_test.go b/internal/vm/ir_test.go index 49214979..839abfa7 100644 --- a/internal/vm/ir_test.go +++ b/internal/vm/ir_test.go @@ -1401,3 +1401,34 @@ func TestIRPullDestAliasingItsBoundIsRejected(t *testing.T) { require.ErrorContains(t, vm.Verify(program), "aliases its cap or overdraft") }) } + +// Funds pulled under one asset and still queued when the asset changes would be +// posted under the new asset. Not statically decidable (a capped send drains the +// queue or not depending on the balances), so this is a run-time check only. +func TestIRSetCurrentAssetWithQueuedFundsIsAnInternalError(t *testing.T) { + execErr := runIRExpectingError(t, ` + $usd = "USD/2" + set_current_asset($usd) + $src = "src" + $dest = "dest" + $overdraft = 0 + $cap = 100 + $pulled = pull_account(account: $src, cap: $cap, overdraft: $overdraft) + $eur = "EUR/2" + set_current_asset($eur) + send_to_account(account: $dest) +`, balances(map[string]int64{"src": 100}), nil) + require.Equal(t, vm.InternalError{Err: vm.ErrSetAssetWithQueuedFunds}, execErr) +} + +func TestIREndWithQueuedFundsIsAnInternalError(t *testing.T) { + execErr := runIRExpectingError(t, ` + $asset = "USD/2" + set_current_asset($asset) + $src = "src" + $overdraft = 0 + $cap = 100 + $pulled = pull_account(account: $src, cap: $cap, overdraft: $overdraft) +`, balances(map[string]int64{"src": 100}), nil) + require.Equal(t, vm.InternalError{Err: vm.ErrEndWithQueuedFunds}, execErr) +} diff --git a/internal/vm/vm.go b/internal/vm/vm.go index 2001ffa8..c04317ad 100644 --- a/internal/vm/vm.go +++ b/internal/vm/vm.go @@ -30,6 +30,13 @@ var ( errSaveWhileMarkOpen = errors.New("save while a mark is open") ) +// Not statically decidable: whether a send drains the queue depends on the +// balances. +var ( + errSetAssetWithQueuedFunds = errors.New("set_current_asset with funds still queued") + errEndWithQueuedFunds = errors.New("run ended with funds still queued") +) + type Vm struct { Program Program runstate *funds.RunState @@ -156,9 +163,6 @@ func Exec[S Store]( switch Opcode(instr.Opcode) { // --- Domain-specific ops case Op_PullAccount: - // TODO crashes if this is the last instruction (the ext word is - // missing): instrs[pc] reads past the end. e.g. a program ending in a - // lone Op_PullAccount word. instrExt := instrs[pc] pc++ @@ -292,6 +296,10 @@ func Exec[S Store]( if runstate.HasOpenMark() { return funds.ExecutionResult{}, InternalError{Err: errSetAssetWhileMarkOpen} } + // funds queued under the previous asset would be posted under the new one + if runstate.HasQueuedFunds() { + return funds.ExecutionResult{}, InternalError{Err: errSetAssetWithQueuedFunds} + } currentAsset = stringsRegs[instr.A] runstate.SetCurrentAsset(currentAsset) @@ -426,8 +434,6 @@ func Exec[S Store]( portionsRegs[instr.A].Set(r) case Op_MetaMonetary: - // TODO crashes if this is the last instruction (the ext word carrying the - // amount destination is missing), same as Op_PullAccount. instrExt := instrs[pc] pc++ @@ -635,6 +641,11 @@ func Exec[S Store]( }) } + // a debit that never became a posting + if runstate.HasQueuedFunds() { + return funds.ExecutionResult{}, InternalError{Err: errEndWithQueuedFunds} + } + postings := runstate.GetPostings() for _, p := range postings { if !funds.ValidatePosting(p) { diff --git a/internal/vm/vm_test.go b/internal/vm/vm_test.go index 6d8922e7..ac377b59 100644 --- a/internal/vm/vm_test.go +++ b/internal/vm/vm_test.go @@ -537,3 +537,81 @@ func TestExecutionErrorsUnwrap(t *testing.T) { require.ErrorIs(t, InternalError{Err: cause}, cause) require.ErrorIs(t, StoreError{Wrapped: cause}, cause) } + +// --- malformed pull/send sequences ----------------------------------------- + +func TestSetCurrentAssetWithQueuedFundsIsAnInternalError(t *testing.T) { + // pull 100 USD/2 from @src, switch to EUR/2, send: the posting would be in + // EUR/2 while the debit was in USD/2 + prog := Program{ + Instructions: []Instruction{ + bc(Op_LoadStr, 0, 0), + bc(Op_LoadStr, 1, 1), + bc(Op_LoadStr, 2, 2), + bc(Op_LoadStr, 3, 3), + bc(Op_LoadInt, 1, 0), + bc(Op_LoadInt, 2, 1), + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, 0, 1, 1), + abc(0, 2, nilReg, nilReg), // ext + abc(Op_SetCurrentAsset, 3, nilReg, nilReg), + abc(Op_SendToAccount, 2, nilReg, nilReg), + }, + StringsPool: []string{"USD/2", "src", "dest", "EUR/2"}, + IntsPool: []big.Int{*big.NewInt(100), *big.NewInt(0)}, + } + store := mockStore{bal: map[funds.PairKey]int64{{Account: "src", Asset: "USD/2"}: 100}} + + require.NoError(t, Verify(fullBanks(prog))) + _, err := Exec(context.Background(), newTestVm(prog), nil, store) + require.Equal(t, InternalError{Err: errSetAssetWithQueuedFunds}, err) +} + +func TestEndWithQueuedFundsIsAnInternalError(t *testing.T) { + prog := Program{ + Instructions: []Instruction{ + bc(Op_LoadStr, 0, 0), + bc(Op_LoadStr, 1, 1), + bc(Op_LoadInt, 1, 0), + bc(Op_LoadInt, 2, 1), + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, 0, 1, 1), + abc(0, 2, nilReg, nilReg), // ext + }, + StringsPool: []string{"USD/2", "src"}, + IntsPool: []big.Int{*big.NewInt(100), *big.NewInt(0)}, + } + store := mockStore{bal: map[funds.PairKey]int64{{Account: "src", Asset: "USD/2"}: 100}} + + require.NoError(t, Verify(fullBanks(prog))) + _, err := Exec(context.Background(), newTestVm(prog), nil, store) + require.Equal(t, InternalError{Err: errEndWithQueuedFunds}, err) +} + +// `send [USD/2 0]` lowers to a zero-cap pull followed by a zero-cap send, which +// never drains the queue: the zero pull must not count as queued funds, at the +// next asset switch or at the end of the run. +func TestQueuedZeroAmountIsNotQueuedFunds(t *testing.T) { + prog := Program{ + Instructions: []Instruction{ + bc(Op_LoadStr, 0, 0), + bc(Op_LoadStr, 1, 1), + bc(Op_LoadStr, 2, 2), + bc(Op_LoadInt, 0, 0), // 0 + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, 1, 1, 0), + abc(0, nilReg, nilReg, nilReg), // ext: unbounded overdraft + abc(Op_SendToAccount, 2, 0, nilReg), + abc(Op_SetCurrentAsset, 0, nilReg, nilReg), + abc(Op_PullAccount, 1, 1, 0), + abc(0, nilReg, nilReg, nilReg), // ext: unbounded overdraft + }, + StringsPool: []string{"USD/2", "world", "dest"}, + IntsPool: []big.Int{*big.NewInt(0)}, + } + + require.NoError(t, Verify(fullBanks(prog))) + res, err := Exec(context.Background(), newTestVm(prog), nil, mockStore{}) + require.Nil(t, err) + require.Empty(t, res.Postings) +}