diff --git a/Makefile b/Makefile index 76f4e27ce4..759e13d5ca 100644 --- a/Makefile +++ b/Makefile @@ -78,7 +78,6 @@ export SHORT_PART_PERIOD_FLAG := -s endif GOTAGS := --tags "$(GOTAGSLIST)" -GOTRIMPATH := $(shell GOPATH=$(GOPATH) && go help build | grep -q .-trimpath && echo -trimpath) GOLDFLAGS_BASE := -X github.com/algorand/go-algorand/config.BuildNumber=$(BUILDNUMBER) \ -X github.com/algorand/go-algorand/config.CommitHash=$(COMMITHASH) \ @@ -277,11 +276,11 @@ build: buildsrc buildsrc-special buildsrc: check-go-version crypto/libs/$(OS_TYPE)/$(ARCH)/lib/libsodium.a node_exporter NONGO_BIN - $(GO_INSTALL) $(GOTRIMPATH) $(GOTAGS) $(GOBUILDMODE) -ldflags="$(GOLDFLAGS)" ./... + $(GO_INSTALL) -trimpath $(GOTAGS) $(GOBUILDMODE) -ldflags="$(GOLDFLAGS)" ./... buildsrc-special: cd tools/block-generator && \ - $(GO_INSTALL) $(GOTRIMPATH) $(GOTAGS) $(GOBUILDMODE) -ldflags="$(GOLDFLAGS)" ./... + $(GO_INSTALL) -trimpath $(GOTAGS) $(GOBUILDMODE) -ldflags="$(GOLDFLAGS)" ./... check-go-version: ./scripts/check_golang_version.sh build @@ -292,15 +291,15 @@ check-go-version: ## the incredible performance impact of -race on Scrypt. build-race: build @mkdir -p $(GOBIN)-race - GOBIN=$(GOBIN)-race go install $(GOTRIMPATH) $(GOTAGS) -race -ldflags="$(GOLDFLAGS)" ./... + GOBIN=$(GOBIN)-race go install -trimpath $(GOTAGS) -race -ldflags="$(GOLDFLAGS)" ./... cp $(GOBIN)/kmd $(GOBIN)-race # Build binaries needed for e2e/integration tests build-e2e: check-go-version crypto/libs/$(OS_TYPE)/$(ARCH)/lib/libsodium.a @mkdir -p $(GOBIN)-race # Build regular binaries (kmd, algod, goal) and race binaries in parallel - $(GO_INSTALL) $(GOTRIMPATH) $(GOTAGS) $(GOBUILDMODE) -ldflags="$(GOLDFLAGS)" ./cmd/kmd ./cmd/algod ./cmd/goal & \ - GOBIN=$(GOBIN)-race go install $(GOTRIMPATH) $(GOTAGS) -race -ldflags="$(GOLDFLAGS)" ./cmd/goal ./cmd/algod ./cmd/algoh ./cmd/tealdbg ./cmd/msgpacktool ./cmd/algokey ./tools/teal/algotmpl ./test/e2e-go/cli/tealdbg/cdtmock & \ + $(GO_INSTALL) -trimpath $(GOTAGS) $(GOBUILDMODE) -ldflags="$(GOLDFLAGS)" ./cmd/kmd ./cmd/algod ./cmd/goal & \ + GOBIN=$(GOBIN)-race go install -trimpath $(GOTAGS) -race -ldflags="$(GOLDFLAGS)" ./cmd/goal ./cmd/algod ./cmd/algoh ./cmd/tealdbg ./cmd/msgpacktool ./cmd/algokey ./tools/teal/algotmpl ./test/e2e-go/cli/tealdbg/cdtmock & \ wait cp $(GOBIN)/kmd $(GOBIN)-race diff --git a/config/consensus.go b/config/consensus.go index 74bceadcfe..08fb39df6f 100644 --- a/config/consensus.go +++ b/config/consensus.go @@ -535,6 +535,11 @@ type ConsensusParams struct { // EnableBoxRefNameError specifies that box ref names should be validated early EnableBoxRefNameError bool + // EnableUnnamedBoxAccessInNewApps allows newly created (in this group) apps to + // create boxes that were not named in a box ref. Each empty box ref in the + // group allows one such creation. + EnableUnnamedBoxAccessInNewApps bool + // ExcludeExpiredCirculation excludes expired stake from the total online stake // used by agreement for Circulation, and updates the calculation of StateProofOnlineTotalWeight used // by state proofs to use the same method (rather than excluding stake from the top N stakeholders as before). @@ -1435,6 +1440,8 @@ func initConsensusProtocols() { vFuture.EnableAppVersioning = true // if not promoted when v12 goes into effect, update logic/field.go vFuture.EnableSha512BlockHash = true + vFuture.EnableUnnamedBoxAccessInNewApps = true + // txn.Access work vFuture.MaxAppTxnAccounts = 8 // Accounts are no worse than others, they should be the same vFuture.MaxAppAccess = 16 // Twice as many, though cross products are explicit diff --git a/daemon/algod/api/server/v2/handlers.go b/daemon/algod/api/server/v2/handlers.go index 78449fa7c2..b0f910f29a 100644 --- a/daemon/algod/api/server/v2/handlers.go +++ b/daemon/algod/api/server/v2/handlers.go @@ -1318,7 +1318,7 @@ func (v2 *Handlers) SimulateTransaction(ctx echo.Context, params model.SimulateT } } - response := convertSimulationResult(simulationResult) + response := convertSimulationResult(simulationResult, proto.EnableUnnamedBoxAccessInNewApps) handle, contentType, err := getCodecHandle((*string)(params.Format)) if err != nil { diff --git a/daemon/algod/api/server/v2/utils.go b/daemon/algod/api/server/v2/utils.go index 606a88bd73..36d72144c0 100644 --- a/daemon/algod/api/server/v2/utils.go +++ b/daemon/algod/api/server/v2/utils.go @@ -462,13 +462,13 @@ func convertTxnTrace(txnTrace *simulation.TransactionTrace) *model.SimulationTra } } -func convertTxnResult(txnResult simulation.TxnResult) PreEncodedSimulateTxnResult { +func convertTxnResult(txnResult simulation.TxnResult, simplify bool) PreEncodedSimulateTxnResult { result := PreEncodedSimulateTxnResult{ Txn: ConvertInnerTxn(&txnResult.Txn), AppBudgetConsumed: omitEmpty(txnResult.AppBudgetConsumed), LogicSigBudgetConsumed: omitEmpty(txnResult.LogicSigBudgetConsumed), TransactionTrace: convertTxnTrace(txnResult.Trace), - UnnamedResourcesAccessed: convertUnnamedResourcesAccessed(txnResult.UnnamedResourcesAccessed), + UnnamedResourcesAccessed: convertUnnamedResourcesAccessed(txnResult.UnnamedResourcesAccessed, simplify), } if !txnResult.FixedSigner.IsZero() { @@ -479,10 +479,13 @@ func convertTxnResult(txnResult simulation.TxnResult) PreEncodedSimulateTxnResul return result } -func convertUnnamedResourcesAccessed(resources *simulation.ResourceTracker) *model.SimulateUnnamedResourcesAccessed { +func convertUnnamedResourcesAccessed(resources *simulation.ResourceTracker, simplify bool) *model.SimulateUnnamedResourcesAccessed { if resources == nil { return nil } + if simplify { + resources.Simplify() + } return &model.SimulateUnnamedResourcesAccessed{ Accounts: sliceOrNil(stringSlice(slices.Collect(maps.Keys(resources.Accounts)))), Assets: sliceOrNil(slices.Collect(maps.Keys(resources.Assets))), @@ -554,10 +557,10 @@ func convertSimulateInitialStates(initialStates *simulation.ResourcesInitialStat } } -func convertTxnGroupResult(txnGroupResult simulation.TxnGroupResult) PreEncodedSimulateTxnGroupResult { +func convertTxnGroupResult(txnGroupResult simulation.TxnGroupResult, simplify bool) PreEncodedSimulateTxnGroupResult { txnResults := make([]PreEncodedSimulateTxnResult, len(txnGroupResult.Txns)) for i, txnResult := range txnGroupResult.Txns { - txnResults[i] = convertTxnResult(txnResult) + txnResults[i] = convertTxnResult(txnResult, simplify) } encoded := PreEncodedSimulateTxnGroupResult{ @@ -565,7 +568,7 @@ func convertTxnGroupResult(txnGroupResult simulation.TxnGroupResult) PreEncodedS FailureMessage: omitEmpty(txnGroupResult.FailureMessage), AppBudgetAdded: omitEmpty(txnGroupResult.AppBudgetAdded), AppBudgetConsumed: omitEmpty(txnGroupResult.AppBudgetConsumed), - UnnamedResourcesAccessed: convertUnnamedResourcesAccessed(txnGroupResult.UnnamedResourcesAccessed), + UnnamedResourcesAccessed: convertUnnamedResourcesAccessed(txnGroupResult.UnnamedResourcesAccessed, simplify), } if len(txnGroupResult.FailedAt) > 0 { @@ -576,7 +579,7 @@ func convertTxnGroupResult(txnGroupResult simulation.TxnGroupResult) PreEncodedS return encoded } -func convertSimulationResult(result simulation.Result) PreEncodedSimulateResponse { +func convertSimulationResult(result simulation.Result, simplify bool) PreEncodedSimulateResponse { var evalOverrides *model.SimulationEvalOverrides if result.EvalOverrides != (simulation.ResultEvalOverrides{}) { evalOverrides = &model.SimulationEvalOverrides{ @@ -590,9 +593,11 @@ func convertSimulationResult(result simulation.Result) PreEncodedSimulateRespons } return PreEncodedSimulateResponse{ - Version: result.Version, - LastRound: result.LastRound, - TxnGroups: util.Map(result.TxnGroups, convertTxnGroupResult), + Version: result.Version, + LastRound: result.LastRound, + TxnGroups: util.Map(result.TxnGroups, func(tg simulation.TxnGroupResult) PreEncodedSimulateTxnGroupResult { + return convertTxnGroupResult(tg, simplify) + }), EvalOverrides: evalOverrides, ExecTraceConfig: result.TraceConfig, InitialStates: convertSimulateInitialStates(result.InitialStates), diff --git a/data/basics/overflow.go b/data/basics/overflow.go index a277c276f3..17ae0b6769 100644 --- a/data/basics/overflow.go +++ b/data/basics/overflow.go @@ -17,6 +17,7 @@ package basics import ( + "math" "math/bits" "golang.org/x/exp/constraints" @@ -41,6 +42,22 @@ func OSub[T constraints.Unsigned](a, b T) (res T, overflowed bool) { return } +// ODiff should be used when you really do want the signed difference between +// uint64s, but still care about detecting overflow. I don't _think_ it can be +// generic to different bit widths. +func ODiff(a, b uint64) (res int64, overflowed bool) { + if a >= b { + if a-b > math.MaxInt64 { + return 0, true + } + return int64(a - b), false + } + if b-a > uint64(math.MaxInt64)+1 { + return 0, true + } + return -int64(b - a), false +} + // OMul multiplies 2 values with overflow detection func OMul[T constraints.Unsigned](a, b T) (res T, overflowed bool) { if b == 0 { diff --git a/data/basics/units_test.go b/data/basics/units_test.go index 93afd8ecb2..6ea36f2b05 100644 --- a/data/basics/units_test.go +++ b/data/basics/units_test.go @@ -22,9 +22,45 @@ import ( "testing" "github.com/algorand/go-algorand/test/partitiontest" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestODiff(t *testing.T) { + partitiontest.PartitionTest(t) + t.Parallel() + + cases := []struct { + a, b uint64 + diff int64 + o bool + }{ + {10, 8, 2, false}, + {10, 0, 10, false}, + {10, 80, -70, false}, + {0, 20, -20, false}, + + {math.MaxInt64 + 1, 0, 0, true}, + {math.MaxInt64, 0, math.MaxInt64, false}, + + {uint64(math.MaxInt64) + 2, 1, 0, true}, + {uint64(math.MaxInt64) + 2, 2, math.MaxInt64, false}, + + // Since minint has higher absolute value than maxint, no overflow here + {1, uint64(math.MaxInt64) + 2, math.MinInt64, false}, + {2, uint64(math.MaxInt64) + 2, math.MinInt64 + 1, false}, + + {math.MaxInt64 + 200, math.MaxInt64, 200, false}, + } + + for i, c := range cases { + diff, o := ODiff(c.a, c.b) + assert.Equal(t, c.diff, diff, + "#%d) %v - %v was %v, not %v", i, c.a, c.b, diff, c.diff) + assert.Equal(t, c.o, o, i) + } +} + func TestSubSaturate(t *testing.T) { partitiontest.PartitionTest(t) t.Parallel() diff --git a/data/transactions/logic/box.go b/data/transactions/logic/box.go index 3a85951292..a4e7845ff0 100644 --- a/data/transactions/logic/box.go +++ b/data/transactions/logic/box.go @@ -45,20 +45,45 @@ func (cx *EvalContext) availableBox(name string, operation BoxOperation, createS } dirty, ok := cx.available.boxes[basics.BoxRef{App: cx.appID, Name: name}] + + newAppAccess := false + // maybe allow it (and account for it) if a newly created app is accessing a + // box. we allow this because we know the box is empty upon first touch, so + // we don't have to go to the disk. but we only allow one such access for + // each spare (empty) box ref. that way, we can't end up needing to write + // many separate newly created boxes. + if !ok && cx.Proto.EnableUnnamedBoxAccessInNewApps { + if _, newAppAccess = cx.available.createdApps[cx.appID]; newAppAccess { + if cx.available.unnamedAccess > 0 { + ok = true // allow it + cx.available.unnamedAccess-- // account for it + dirty = false // no-op, but for clarity + + // it will be marked dirty and dirtyBytes will be incremented + // below, like any create. as a (good) side-effect it will go + // into `cx.available` so that later uses will see it in + // available.boxes, skipping this section + } + } + } + if !ok && cx.UnnamedResources != nil { - ok = cx.UnnamedResources.AvailableBox(cx.appID, name, operation, createSize) + ok = cx.UnnamedResources.AvailableBox(cx.appID, name, newAppAccess, createSize) } if !ok { return nil, false, fmt.Errorf("invalid Box reference %#x", name) } - // Since the box is in cx.available, we know this GetBox call is cheap. It - // will go (at most) to the cowRoundBase. Knowledge about existence - // simplifies write budget tracking, then we return the info to avoid yet - // another call to GetBox which most ops need anyway. - content, exists, err := cx.Ledger.GetBox(cx.appID, name) - if err != nil { - return nil, false, err + // If the box is in cx.available, GetBox() is cheap. It will go (at most) to + // the cowRoundBase. But if we did a "newAppAccess", GetBox would go to disk + // just to find the box is not there. So we skip it. + content, exists := []byte(nil), false + if !newAppAccess { + var getErr error + content, exists, getErr = cx.Ledger.GetBox(cx.appID, name) + if getErr != nil { + return nil, false, getErr + } } switch operation { @@ -69,8 +94,9 @@ func (cx *EvalContext) availableBox(name string, operation BoxOperation, createS } // Since it exists, we have no dirty work to do. The weird case of // box_put, which seems like a combination of create and write, is - // properly handled because already used boxWrite to declare the - // intent to write (and track dirtiness). + // properly handled because opBoxPut uses BoxWriteOperation to + // declare the intent to write (and track dirtiness). opBoxPut + // performs the length match check itself. return content, exists, nil } fallthrough // If it doesn't exist, a create is like write @@ -106,7 +132,7 @@ func (cx *EvalContext) availableBox(name string, operation BoxOperation, createS return content, exists, nil } -func argCheck(cx *EvalContext, name string, size uint64) error { +func lengthChecks(cx *EvalContext, name string, size uint64) error { // Enforce length rules. Currently these are the same as enforced by // ledger. If these were ever to change in proto, we would need to isolate // changes to different program versions. (so a v7 app could not see a @@ -130,7 +156,7 @@ func opBoxCreate(cx *EvalContext) error { name := string(cx.Stack[prev].Bytes) size := cx.Stack[last].Uint - err := argCheck(cx, name, size) + err := lengthChecks(cx, name, size) if err != nil { return err } @@ -160,7 +186,7 @@ func opBoxExtract(cx *EvalContext) error { start := cx.Stack[prev].Uint length := cx.Stack[last].Uint - err := argCheck(cx, name, basics.AddSaturate(start, length)) + err := lengthChecks(cx, name, basics.AddSaturate(start, length)) if err != nil { return err } @@ -187,7 +213,7 @@ func opBoxReplace(cx *EvalContext) error { start := cx.Stack[prev].Uint name := string(cx.Stack[pprev].Bytes) - err := argCheck(cx, name, basics.AddSaturate(start, uint64(len(replacement)))) + err := lengthChecks(cx, name, basics.AddSaturate(start, uint64(len(replacement)))) if err != nil { return err } @@ -215,7 +241,7 @@ func opBoxSplice(cx *EvalContext) error { start := cx.Stack[last-2].Uint name := string(cx.Stack[last-3].Bytes) - err := argCheck(cx, name, 0) + err := lengthChecks(cx, name, 0) if err != nil { return err } @@ -240,7 +266,7 @@ func opBoxDel(cx *EvalContext) error { last := len(cx.Stack) - 1 // name name := string(cx.Stack[last].Bytes) - err := argCheck(cx, name, 0) + err := lengthChecks(cx, name, 0) if err != nil { return err } @@ -266,7 +292,7 @@ func opBoxResize(cx *EvalContext) error { name := string(cx.Stack[prev].Bytes) size := cx.Stack[last].Uint - err := argCheck(cx, name, size) + err := lengthChecks(cx, name, size) if err != nil { return err } @@ -305,7 +331,7 @@ func opBoxLen(cx *EvalContext) error { last := len(cx.Stack) - 1 // name name := string(cx.Stack[last].Bytes) - err := argCheck(cx, name, 0) + err := lengthChecks(cx, name, 0) if err != nil { return err } @@ -323,7 +349,7 @@ func opBoxGet(cx *EvalContext) error { last := len(cx.Stack) - 1 // name name := string(cx.Stack[last].Bytes) - err := argCheck(cx, name, 0) + err := lengthChecks(cx, name, 0) if err != nil { return err } @@ -346,7 +372,7 @@ func opBoxPut(cx *EvalContext) error { value := cx.Stack[last].Bytes name := string(cx.Stack[prev].Bytes) - err := argCheck(cx, name, uint64(len(value))) + err := lengthChecks(cx, name, uint64(len(value))) if err != nil { return err } diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index f29b2b1a03..d8b30228c7 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -284,7 +284,8 @@ type UnnamedResourcePolicy interface { AvailableApp(app basics.AppIndex) bool AllowsHolding(addr basics.Address, asset basics.AssetIndex) bool AllowsLocal(addr basics.Address, app basics.AppIndex) bool - AvailableBox(app basics.AppIndex, name string, operation BoxOperation, createSize uint64) bool + AvailableBox(app basics.AppIndex, name string, newAppAccess bool, createSize uint64) bool + IOSurplus(surplus int64) bool } // EvalConstants contains constant parameters that are used by opcodes during evaluation (including both real-execution and simulation). @@ -363,10 +364,13 @@ type EvalParams struct { // readBudgetChecked allows us to only check the read budget once readBudgetChecked bool - // SurplusReadBudget is the number of bytes from the IO budget that were not used for reading - // in boxes before evaluation began. In other words, the txn group could have read in - // SurplusReadBudget more box bytes, but did not. - SurplusReadBudget uint64 + // SurplusReadBudget is the number of bytes from the IO budget that were not + // used for reading in boxes before evaluation began. In other words, the + // txn group could have read in SurplusReadBudget more box bytes, but did + // not. It is signed because `simulate` evaluates groups even if they come + // in with insufficient io budget, and reports the need, when invoked with + // AllowUnnamedResources. + SurplusReadBudget int64 EvalConstants @@ -1122,11 +1126,17 @@ func EvalContract(program []byte, gi int, aid basics.AppIndex, params *EvalParam // If this is a creation... if cx.txn.Txn.ApplicationID == 0 { // make any "0 index" box refs available now that we have an appID. + // This allows case 2b in TestNewAppBoxCreate of boxtxn_test.go for _, br := range cx.txn.Txn.Boxes { if br.Index == 0 { cx.EvalParams.available.boxes[basics.BoxRef{App: cx.appID, Name: string(br.Name)}] = false } } + for _, rr := range cx.txn.Txn.Access { + if len(rr.Box.Name) > 0 && rr.Box.Index == 0 { // len check ensures we have a box ref + cx.EvalParams.available.boxes[basics.BoxRef{App: cx.appID, Name: string(rr.Box.Name)}] = false + } + } // and add the appID to `createdApps` if cx.EvalParams.available.createdApps == nil { cx.EvalParams.available.createdApps = make(map[basics.AppIndex]struct{}) @@ -1146,9 +1156,11 @@ func EvalContract(program []byte, gi int, aid basics.AppIndex, params *EvalParam } } } - cx.ioBudget = bumps * cx.Proto.BytesPerBoxReference + cx.ioBudget = basics.MulSaturate(bumps, cx.Proto.BytesPerBoxReference) used := uint64(0) + var surplus int64 + var overflow bool for br := range cx.available.boxes { if len(br.Name) == 0 { // 0 length names are not allowed for actual created boxes, but @@ -1166,7 +1178,9 @@ func EvalContract(program []byte, gi int, aid basics.AppIndex, params *EvalParam cx.available.boxes[br] = false used = basics.AddSaturate(used, size) - if used > cx.ioBudget { + surplus, overflow = basics.ODiff(cx.ioBudget, used) + // we defer the check if we have cx.UnnamedResources, so we can ask for the entire surplus at the end. + if overflow || (surplus < 0 && cx.UnnamedResources == nil) { err = fmt.Errorf("box read budget (%d) exceeded", cx.ioBudget) if !cx.Proto.EnableBareBudgetError { // We return an EvalError here because we used to do @@ -1180,8 +1194,14 @@ func EvalContract(program []byte, gi int, aid basics.AppIndex, params *EvalParam return false, nil, err } } + + // Report the surplus/deficit to the policy, and find out if we should continue + if cx.UnnamedResources != nil && !cx.UnnamedResources.IOSurplus(surplus) { + return false, nil, fmt.Errorf("box read budget (%d) exceeded despite policy", cx.ioBudget) + } + cx.readBudgetChecked = true - cx.SurplusReadBudget = cx.ioBudget - used + cx.SurplusReadBudget = surplus // Can be negative, but only in `simulate` } if cx.Trace != nil && cx.caller != nil { diff --git a/data/transactions/logic/evalStateful_test.go b/data/transactions/logic/evalStateful_test.go index 40d380c36c..40251e7551 100644 --- a/data/transactions/logic/evalStateful_test.go +++ b/data/transactions/logic/evalStateful_test.go @@ -2875,10 +2875,10 @@ func allowsLocalEvent(addr basics.Address, aid basics.AppIndex) unnamedResourceP } } -func availableBoxEvent(app basics.AppIndex, name string, operation BoxOperation, createSize uint64) unnamedResourcePolicyEvent { +func availableBoxEvent(app basics.AppIndex, name string, newApp bool, createSize uint64) unnamedResourcePolicyEvent { return unnamedResourcePolicyEvent{ eventType: "AvailableBox", - args: []interface{}{app, name, operation, createSize}, + args: []interface{}{app, name, newApp, createSize}, } } @@ -2919,11 +2919,17 @@ func (p *mockUnnamedResourcePolicy) AllowsLocal(addr basics.Address, aid basics. return p.allowEverything } -func (p *mockUnnamedResourcePolicy) AvailableBox(app basics.AppIndex, name string, operation BoxOperation, createSize uint64) bool { - p.events = append(p.events, availableBoxEvent(app, name, operation, createSize)) +func (p *mockUnnamedResourcePolicy) AvailableBox(app basics.AppIndex, name string, newApp bool, createSize uint64) bool { + p.events = append(p.events, availableBoxEvent(app, name, newApp, createSize)) return p.allowEverything } +// If IOSurplus fails, then everything would fail before the "real" issue being +// tested. So we just pass this in the mock. +func (p *mockUnnamedResourcePolicy) IOSurplus(size int64) bool { + return true +} + func TestUnnamedResourceAccess(t *testing.T) { partitiontest.PartitionTest(t) t.Parallel() @@ -3125,7 +3131,7 @@ func TestUnnamedResourceAccess(t *testing.T) { if tc.allowsUnnamedResources { testApp(t, source, ep) if tc.policy != nil { - expectedEvents := []unnamedResourcePolicyEvent{availableBoxEvent(tx.ApplicationID, "box key", BoxReadOperation, 0)} + expectedEvents := []unnamedResourcePolicyEvent{availableBoxEvent(tx.ApplicationID, "box key", false, 0)} assert.Equal(t, expectedEvents, tc.policy.events) tc.policy.events = nil } @@ -3136,7 +3142,7 @@ func TestUnnamedResourceAccess(t *testing.T) { if tc.allowsUnnamedResources { testApp(t, source, ep) if tc.policy != nil { - expectedEvents := []unnamedResourcePolicyEvent{availableBoxEvent(tx.ApplicationID, "new box", BoxCreateOperation, 1)} + expectedEvents := []unnamedResourcePolicyEvent{availableBoxEvent(tx.ApplicationID, "new box", false, 1)} assert.Equal(t, expectedEvents, tc.policy.events) tc.policy.events = nil } diff --git a/data/transactions/logic/resources.go b/data/transactions/logic/resources.go index 7eb3435746..39204daaf3 100644 --- a/data/transactions/logic/resources.go +++ b/data/transactions/logic/resources.go @@ -55,6 +55,10 @@ type resources struct { // operation. boxes map[basics.BoxRef]bool + // unnamedAccess is the number of times that a newly created app may access + // a box that was not named. It is decremented for each box accessed this way. + unnamedAccess int + // dirtyBytes maintains a running count of the number of dirty bytes in `boxes` dirtyBytes uint64 } @@ -325,6 +329,11 @@ func (r *resources) fillApplicationCallAccess(ep *EvalParams, hdr *transactions. // ApplicationCallTxnFields.wellFormed ensures no error here. app, name, _ := rr.Box.Resolve(tx.Access) r.shareBox(basics.BoxRef{App: app, Name: name}, tx.ApplicationID) + default: + // all empty equals an "empty boxref" which allows one unnamed access + if ep.Proto.EnableUnnamedBoxAccessInNewApps { + r.unnamedAccess++ + } } } } @@ -365,9 +374,12 @@ func (r *resources) fillApplicationCallForeign(ep *EvalParams, hdr *transactions } for _, br := range tx.Boxes { - app := basics.AppIndex(0) // 0 can be handled by shareBox as current - if br.Index != 0 { - // Upper bounds check will already have been done by + if ep.Proto.EnableUnnamedBoxAccessInNewApps && br.Empty() { + r.unnamedAccess++ + } + var app basics.AppIndex + if br.Index > 0 { + // Bounds check will already have been done by // WellFormed. For testing purposes, it's better to panic // now than after returning a nil. app = tx.ForeignApps[br.Index-1] // shift for the 0=current convention diff --git a/ledger/boxtxn_test.go b/ledger/boxtxn_test.go index 93f228d596..1d52e3c888 100644 --- a/ledger/boxtxn_test.go +++ b/ledger/boxtxn_test.go @@ -27,6 +27,7 @@ import ( "github.com/algorand/go-algorand/config" "github.com/algorand/go-algorand/data/basics" "github.com/algorand/go-algorand/data/transactions" + "github.com/algorand/go-algorand/data/transactions/logic" "github.com/algorand/go-algorand/data/txntest" ledgertesting "github.com/algorand/go-algorand/ledger/testing" "github.com/algorand/go-algorand/logging" @@ -127,8 +128,12 @@ var passThruSource = main(` itxn_submit `) -const boxVersion = 36 -const boxQuotaBumpVersion = 41 +const ( + boxVersion = 36 + accessVersion = 38 + boxQuotaBumpVersion = 41 + newAppCreateVersion = 41 +) func boxFee(p config.ConsensusParams, nameAndValueSize uint64) uint64 { return p.BoxFlatMinBalance + p.BoxByteMinBalance*(nameAndValueSize) @@ -697,3 +702,169 @@ func TestBoxInners(t *testing.T) { dl.txn(call.Args("check", "x", "mark d")) }) } + +// Create the app with bytecode in txn.ApplicationArgs[0], pass my arg[1] as created arg[0] +var passThruCreator = main(` + itxn_begin + txn TypeEnum; itxn_field TypeEnum + txn ApplicationArgs 0; itxn_field ApprovalProgram + txn ApplicationArgs 0; itxn_field ClearStateProgram // need something, won't be used + txn ApplicationArgs 1; itxn_field ApplicationArgs + itxn_submit +`) + +// TestNewAppBoxCreate exercised proto.EnableUnnamedBoxCreate +func TestNewAppBoxCreate(t *testing.T) { + partitiontest.PartitionTest(t) + t.Parallel() + + genBalances, addrs, _ := ledgertesting.NewTestGenesis() + ledgertesting.TestConsensusRange(t, boxVersion, 0, func(t *testing.T, ver int, cv protocol.ConsensusVersion, cfg config.Local) { + dl := NewDoubleLedger(t, genBalances, cv, cfg) + defer dl.Close() + + // We're going to create an app that will, during its own creation, + // create a box. That requires two tricks. + + // 1) Figure out the appID it will have and prefund it. This _could_ be + // done within the group itself - an early transaction deduces the + // transaction counter, so it can know what the later create will be, + // and compute it's app address. + + // 2) a) Use the the predicted appID to name the box ref. + // or b) Use 0 as the app in the box ref, meaning "this app" + // or c) EnableUnnamedBoxCreate will allow such a creation if there are empty box refs. + + // 2a is pretty much impossible in practice, we can only do it here + // because our blockchain is "quiet" we know the upcoming appID. + + // 2b won't work for inner app creates, since the 0 appID gets replaced + // with the _top-level_ app's ID. + + // 2c would allow newly created apps, even if inners, to create boxes. + // It also has the nice property of not needing to know the box's + // name. So it can be computed in app. We don't need the name because + // we can short-circuit the lookup - the box _must_ be empty. + + // boxCreate is an app that tries to make a box from its first argument, even during its own creation. + boxCreate := "txn ApplicationArgs 0; int 24; box_create;" // Succeeds if the box is created and did not already exist + + // boxPut is an app that tries to make a box from its first argument, even during its own creation (but using box_put) + boxPut := "txn ApplicationArgs 0; int 24; bzero; box_put; int 1;" + + for _, createSrc := range []string{boxCreate, boxPut} { + // doubleSrc tries to create TWO boxes. The second is always named by ApplicationArgs 1 + doubleSrc := createSrc + `txn ApplicationArgs 1; int 24; box_create; pop;` // return result of FIRST box_create + // need to call one inner txn, and have have mbr for itself and inner created app + passID := dl.fundedApp(addrs[0], 201_000, passThruCreator) // Will be used to show inners have same power + + // Since we used fundedApp, the next app created would be passID+2. + // We'll prefund a whole bunch of the next apps that we can then create + // at will below. + + var testTxns = basics.AppIndex(20) + for i := range testTxns { + dl.txn(&txntest.Txn{Type: "pay", Sender: addrs[0], Receiver: (passID + 2 + testTxns + i).Address(), Amount: 500_000}) + } + + // Try to create it. It will fail because there's no box ref. (does not increment txncounter) + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: createSrc, ApplicationArgs: [][]byte{{0x01}}}, + "invalid Box reference 0x01") + + // 2a. Create it with a box ref of the predicted appID + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: createSrc, ApplicationArgs: [][]byte{{0x01}}, + ForeignApps: []basics.AppIndex{passID + testTxns + 2}, + Boxes: []transactions.BoxRef{{Index: 1, Name: []byte{0x01}}}}) + + // 2a. Create it with a box ref of the predicted appID (Access list) + if ver >= accessVersion { + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: createSrc, ApplicationArgs: [][]byte{{0x01}}, + Access: []transactions.ResourceRef{ + {App: passID + testTxns + 3}, + {Box: transactions.BoxRef{Index: 1, Name: []byte{0x01}}}}}) + } + + // 2b. Create it with a box ref of 0, which means "this app" + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: createSrc, ApplicationArgs: [][]byte{{0x01}}, + Boxes: []transactions.BoxRef{{Index: 0, Name: []byte{0x01}}}}) + + // 2b. Create it with a box ref of 0, which means "this app" (Access List) + if ver >= accessVersion { + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: createSrc, ApplicationArgs: [][]byte{{0x01}}, + Access: []transactions.ResourceRef{ + {Box: transactions.BoxRef{Index: 0, Name: []byte{0x01}}}}}) + } + + // you can manipulate it twice if you want (this tries to create it twice) + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: doubleSrc, ApplicationArgs: [][]byte{{0x01}, {0x01}}, + Boxes: []transactions.BoxRef{{Index: 0, Name: []byte{0x01}}}}) + + // but you still can't make a second box + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: doubleSrc, ApplicationArgs: [][]byte{{0x01}, {0x02}}, + Boxes: []transactions.BoxRef{{Index: 0, Name: []byte{0x01}}}}, + "invalid Box reference 0x02") + + // until you list it as well + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: doubleSrc, ApplicationArgs: [][]byte{{0x01}, {0x02}}, + Boxes: []transactions.BoxRef{ + {Index: 0, Name: []byte{0x01}}, + {Index: 0, Name: []byte{0x02}}, + }}) + + if ver >= newAppCreateVersion { + // 2c. Create it with an empty box ref + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: createSrc, ApplicationArgs: [][]byte{{0x01}}, + Boxes: []transactions.BoxRef{{}}}) + + // 2c. Create it with an empty box ref + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: createSrc, ApplicationArgs: [][]byte{{0x01}}, + Access: []transactions.ResourceRef{{Box: transactions.BoxRef{}}}}) + + // but you can't do a second create + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: doubleSrc, ApplicationArgs: [][]byte{{0x01}, {0x02}}, + Boxes: []transactions.BoxRef{{}}}, + "invalid Box reference 0x02") + + // until you add a second box ref + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: doubleSrc, ApplicationArgs: [][]byte{{0x01}, {0x02}}, + Boxes: []transactions.BoxRef{{}, {}}}) + + // Now confirm that 2c also works for an inner created app + ops, err := logic.AssembleString("#pragma version 12\n" + createSrc) + require.NoError(t, err, ops.Errors) + createSrcByteCode := ops.Program + // create app as an inner, fails w/o empty box ref + dl.txn(&txntest.Txn{Sender: addrs[0], + Type: "appl", + ApplicationID: passID, + ApplicationArgs: [][]byte{createSrcByteCode, {0x01}}, + }, "invalid Box reference 0x01") + // create app as an inner, succeeds w/ empty box ref + dl.txn(&txntest.Txn{Sender: addrs[0], + Type: "appl", + ApplicationID: passID, + ApplicationArgs: [][]byte{createSrcByteCode, {0x01}}, + Boxes: []transactions.BoxRef{{}}, + }) + } else { + // 2c. Doesn't work yet until `newAppCreateVersion` + dl.txn(&txntest.Txn{Type: "appl", Sender: addrs[0], + ApprovalProgram: createSrc, ApplicationArgs: [][]byte{{0x01}}, + Boxes: []transactions.BoxRef{{}}}, + "invalid Box reference 0x01") + } + } + }) +} diff --git a/ledger/simulation/resources.go b/ledger/simulation/resources.go index ae255c5e8d..9394bd8fbd 100644 --- a/ledger/simulation/resources.go +++ b/ledger/simulation/resources.go @@ -31,22 +31,47 @@ import ( // ResourceTracker calculates the additional resources that a transaction or group could use, and // it tracks any referenced unnamed resources that fit within those limits. type ResourceTracker struct { - Accounts map[basics.Address]struct{} + Accounts map[basics.Address]struct{} + // MaxAccounts is the largest number of new accounts that could possibly be + // referenced by the thing being tracked (txn or group) beyond what is + // already listed. MaxAccounts int - Assets map[basics.AssetIndex]struct{} + Assets map[basics.AssetIndex]struct{} + // MaxAssets is the largest number of new assets that could possibly be + // referenced by the thing being tracked (txn or group) beyond what is + // already listed. MaxAssets int - Apps map[basics.AppIndex]struct{} + Apps map[basics.AppIndex]struct{} + // MaxApps is the largest number of new apps that could possibly be + // referenced by the thing being tracked (txn or group) beyond what is + // already listed. MaxApps int // The map value is the size of the box loaded from the ledger prior to any writes. This is used // to track the box read budget. - Boxes map[basics.BoxRef]uint64 - MaxBoxes int + Boxes map[basics.BoxRef]BoxStat + MaxBoxes int + + // NumEmptyBoxRefs tracks the number of additional BoxRefs that will be + // required to handle the boxes this tracker has observed (for i/o budget, + // or to allow access to boxes in new apps). NumEmptyBoxRefs int - maxWriteBudget uint64 + // maxWriteBudget is the "high-water mark" for dirty box writes. A group + // must have enough write budget during its entire execution to accommodate + // writing its dirty boxes. This is maintained after each opcode. + maxWriteBudget uint64 + + // initialReadSurplus is the I/O surplus available based on the submitted + // group, so it takes into account the number of box refs supplied, and the + // size of the boxes named. It can be negative! + initialReadSurplus int64 + + // MaxTotalRefs is the largest number of new refs of any kind that could + // possibly be added tp the thing being tracked (txn or group) beyond what + // it began with. MaxTotalRefs int AssetHoldings map[ledgercore.AccountAsset]struct{} @@ -54,6 +79,12 @@ type ResourceTracker struct { MaxCrossProductReferences int } +// BoxStat is what needs to be tracked in order to figure out need resources (especially empty refs) +type BoxStat struct { + ReadSize uint64 // how much read quota did this access consume? + NewApp bool // was the box accessed by an app created in this group? +} + func makeTxnResourceTracker(txn *transactions.Transaction, proto *config.ConsensusParams) ResourceTracker { if txn.Type != protocol.ApplicationCallTx { return ResourceTracker{} @@ -70,6 +101,9 @@ func makeTxnResourceTracker(txn *transactions.Transaction, proto *config.Consens } } +// makeGlobalResourceTracker populates a tracker so that it knows what are the +// bounds on how many new references of various types that could potentially be +// allowed for the group. func makeGlobalResourceTracker(perTxnResources []ResourceTracker, nonAppCalls int, proto *config.ConsensusParams) ResourceTracker { // Calculate the maximum number of cross-product resources that can be accessed by one app call // under normal circumstances. This is calculated using the case of an app call with a full set @@ -107,17 +141,27 @@ func makeGlobalResourceTracker(perTxnResources []ResourceTracker, nonAppCalls in func (a *ResourceTracker) removePrivateFields() { a.maxWriteBudget = 0 + a.initialReadSurplus = 0 } // HasResources returns true if the tracker has any resources. -func (a *ResourceTracker) HasResources() bool { - return len(a.Accounts) != 0 || len(a.Assets) != 0 || len(a.Apps) != 0 || len(a.Boxes) != 0 || len(a.AssetHoldings) != 0 || len(a.AppLocals) != 0 +func (a ResourceTracker) HasResources() bool { + return len(a.Accounts) != 0 || len(a.Assets) != 0 || len(a.Apps) != 0 || len(a.Boxes) != 0 || len(a.AssetHoldings) != 0 || len(a.AppLocals) != 0 || a.NumEmptyBoxRefs != 0 +} + +// refCount returns the number of references this tracker has. +func (a ResourceTracker) refCount() int { + return len(a.Accounts) + len(a.Assets) + len(a.Apps) + a.boxCount() +} + +// boxCount returns the number of box references this tracker has. +func (a ResourceTracker) boxCount() int { + return len(a.Boxes) + a.NumEmptyBoxRefs } func (a *ResourceTracker) hasAccount(addr basics.Address, ep *logic.EvalParams, programVersion uint64) bool { // nil map lookup is ok - _, ok := a.Accounts[addr] - if ok { + if _, ok := a.Accounts[addr]; ok { return true } if programVersion >= 7 { // appAddressAvailableVersion @@ -131,7 +175,7 @@ func (a *ResourceTracker) hasAccount(addr basics.Address, ep *logic.EvalParams, } func (a *ResourceTracker) addAccount(addr basics.Address) bool { - if len(a.Accounts) >= a.MaxAccounts || len(a.Accounts)+len(a.Assets)+len(a.Apps)+len(a.Boxes)+a.NumEmptyBoxRefs >= a.MaxTotalRefs { + if len(a.Accounts) >= a.MaxAccounts || a.refCount() >= a.MaxTotalRefs { return false } if a.Accounts == nil { @@ -141,8 +185,11 @@ func (a *ResourceTracker) addAccount(addr basics.Address) bool { return true } +// removeAccountSlot notes that there is one less account slot available for +// referencing new assets. It's used to tell the global tracker that a local +// slot is used. func (a *ResourceTracker) removeAccountSlot() bool { - if len(a.Accounts) >= a.MaxAccounts || len(a.Accounts)+len(a.Assets)+len(a.Apps)+len(a.Boxes)+a.NumEmptyBoxRefs >= a.MaxTotalRefs { + if len(a.Accounts) >= a.MaxAccounts || a.refCount() >= a.MaxTotalRefs { return false } a.MaxAccounts-- @@ -150,14 +197,16 @@ func (a *ResourceTracker) removeAccountSlot() bool { return true } -func (a *ResourceTracker) hasAsset(aid basics.AssetIndex) bool { +func (a ResourceTracker) hasAsset(aid basics.AssetIndex) bool { // nil map lookup is ok _, ok := a.Assets[aid] return ok } +// addAsset records that an asset reference must be added. It return false if +// that is known to be impossible. func (a *ResourceTracker) addAsset(aid basics.AssetIndex) bool { - if len(a.Assets) >= a.MaxAssets || len(a.Accounts)+len(a.Assets)+len(a.Apps)+len(a.Boxes)+a.NumEmptyBoxRefs >= a.MaxTotalRefs { + if len(a.Assets) >= a.MaxAssets || a.refCount() >= a.MaxTotalRefs { return false } if a.Assets == nil { @@ -167,8 +216,11 @@ func (a *ResourceTracker) addAsset(aid basics.AssetIndex) bool { return true } +// removeAssetSlot notes that there is one less asset slot available for +// referencing new assets. It's used to tell the global tracker that a local +// slot is used. func (a *ResourceTracker) removeAssetSlot() bool { - if len(a.Assets) >= a.MaxAssets || len(a.Accounts)+len(a.Assets)+len(a.Apps)+len(a.Boxes)+a.NumEmptyBoxRefs >= a.MaxTotalRefs { + if len(a.Assets) >= a.MaxAssets || a.refCount() >= a.MaxTotalRefs { return false } a.MaxAssets-- @@ -176,7 +228,7 @@ func (a *ResourceTracker) removeAssetSlot() bool { return true } -func (a *ResourceTracker) hasApp(aid basics.AppIndex) bool { +func (a ResourceTracker) hasApp(aid basics.AppIndex) bool { // nil map lookup is ok _, ok := a.Apps[aid] return ok @@ -200,7 +252,7 @@ func (a *ResourceTracker) addApp(aid basics.AppIndex, ep *logic.EvalParams, prog } } - if len(a.Accounts)+len(a.Assets)+len(a.Apps)+len(a.Boxes)+a.NumEmptyBoxRefs >= a.MaxTotalRefs { + if a.refCount() >= a.MaxTotalRefs { return false } if a.Apps == nil { @@ -211,7 +263,7 @@ func (a *ResourceTracker) addApp(aid basics.AppIndex, ep *logic.EvalParams, prog } func (a *ResourceTracker) removeAppSlot() bool { - if len(a.Apps) >= a.MaxApps || len(a.Accounts)+len(a.Assets)+len(a.Apps)+len(a.Boxes)+a.NumEmptyBoxRefs >= a.MaxTotalRefs { + if len(a.Apps) >= a.MaxApps || a.refCount() >= a.MaxTotalRefs { return false } a.MaxApps-- @@ -222,21 +274,26 @@ func (a *ResourceTracker) removeAppSlot() bool { return true } -func (a *ResourceTracker) hasBox(app basics.AppIndex, name string) bool { +func (a ResourceTracker) hasBox(app basics.AppIndex, name string) bool { // nil map lookup is ok _, ok := a.Boxes[basics.BoxRef{App: app, Name: name}] return ok } -func (a *ResourceTracker) addBox(app basics.AppIndex, name string, readSize, additionalReadBudget, bytesPerBoxRef uint64) bool { +func (a *ResourceTracker) addBox(app basics.AppIndex, name string, newApp bool, readSize uint64, bytesPerBoxRef uint64) bool { usedReadBudget := basics.AddSaturate(a.usedBoxReadBudget(), readSize) // Adding bytesPerBoxRef to account for the new IO budget from adding an additional box ref - readBudget := additionalReadBudget + a.boxIOBudget(bytesPerBoxRef) + bytesPerBoxRef + ioBudget := a.boxIOBudget(bytesPerBoxRef) + bytesPerBoxRef + if a.initialReadSurplus > 0 { + ioBudget += uint64(a.initialReadSurplus) + } else { + ioBudget -= uint64(-a.initialReadSurplus) + } var emptyRefs int - if usedReadBudget > readBudget { + if usedReadBudget > ioBudget { // We need to allocate more empty box refs to increase the read budget - neededBudget := usedReadBudget - readBudget + neededBudget := usedReadBudget - ioBudget emptyRefsU64 := basics.DivCeil(neededBudget, bytesPerBoxRef) if emptyRefsU64 > math.MaxInt { // This should never happen, but if we overflow an int with the number of extra pages @@ -244,22 +301,45 @@ func (a *ResourceTracker) addBox(app basics.AppIndex, name string, readSize, add return false } emptyRefs = int(emptyRefsU64) - } else if a.NumEmptyBoxRefs != 0 { - surplusBudget := readBudget - usedReadBudget - if surplusBudget >= bytesPerBoxRef && readBudget-bytesPerBoxRef >= a.maxWriteBudget { - // If we already have enough read budget, remove one empty ref to be replaced by the new - // named box ref. + } else if a.NumEmptyBoxRefs > 0 { // If there are empties added for quota, we may not need as many. + surplusReadBudget := basics.SubSaturate(ioBudget, usedReadBudget) + surplusWriteBudget := basics.SubSaturate(ioBudget, a.maxWriteBudget) + if surplusReadBudget >= bytesPerBoxRef && surplusWriteBudget >= bytesPerBoxRef { + // By adding this box, we may no longer need an empty ref we previously added for quota. emptyRefs = -1 } } - if emptyRefs >= a.MaxBoxes-len(a.Boxes)-a.NumEmptyBoxRefs || emptyRefs >= a.MaxTotalRefs-len(a.Accounts)-len(a.Assets)-len(a.Apps)-len(a.Boxes)-a.NumEmptyBoxRefs { + if emptyRefs >= a.MaxBoxes-a.boxCount() || emptyRefs >= a.MaxTotalRefs-a.refCount() { return false } if a.Boxes == nil { - a.Boxes = make(map[basics.BoxRef]uint64) + a.Boxes = make(map[basics.BoxRef]BoxStat) + } + a.Boxes[basics.BoxRef{App: app, Name: name}] = BoxStat{readSize, newApp} + a.NumEmptyBoxRefs += emptyRefs + return true +} + +// ioSurplus notes whether extra bytes can be read because of the initially +// supplied box refs. If not, it ensures that empty box refs can be added to +// account for the deficit. +func (a *ResourceTracker) ioSurplus(amount int64, bytesPerBoxRef uint64) bool { + a.initialReadSurplus = amount + if amount > 0 { + return true + } + neededBudget := uint64(-amount) + emptyRefsU64 := basics.DivCeil(neededBudget, bytesPerBoxRef) + if emptyRefsU64 > math.MaxInt { + // This should never happen, but if we overflow an int with the number of extra pages + // needed, we can't support this request. + return false + } + emptyRefs := int(emptyRefsU64) + if emptyRefs >= a.MaxBoxes-a.boxCount() || emptyRefs >= a.MaxTotalRefs-a.refCount() { + return false } - a.Boxes[basics.BoxRef{App: app, Name: name}] = readSize a.NumEmptyBoxRefs += emptyRefs return true } @@ -276,7 +356,7 @@ func (a *ResourceTracker) addEmptyBoxRefsForWriteBudget(usedWriteBudget, additio return false } extraRefs := int(extraRefsU64) - if extraRefs > a.MaxBoxes-len(a.Boxes)-a.NumEmptyBoxRefs || extraRefs > a.MaxTotalRefs-len(a.Accounts)-len(a.Assets)-len(a.Apps)-len(a.Boxes)-a.NumEmptyBoxRefs { + if extraRefs > a.MaxBoxes-a.boxCount() || extraRefs > a.MaxTotalRefs-a.refCount() { return false } a.NumEmptyBoxRefs += extraRefs @@ -287,24 +367,20 @@ func (a *ResourceTracker) addEmptyBoxRefsForWriteBudget(usedWriteBudget, additio return true } -func (a *ResourceTracker) boxIOBudget(bytesPerBoxRef uint64) uint64 { - return uint64(len(a.Boxes)+a.NumEmptyBoxRefs) * bytesPerBoxRef +func (a ResourceTracker) boxIOBudget(bytesPerBoxRef uint64) uint64 { + return uint64(a.boxCount()) * bytesPerBoxRef } func (a *ResourceTracker) usedBoxReadBudget() uint64 { var budget uint64 - for _, readSize := range a.Boxes { - budget += readSize + for _, bs := range a.Boxes { + budget += bs.ReadSize } return budget } func (a *ResourceTracker) maxPossibleUnnamedBoxes() int { - numBoxes := a.MaxTotalRefs - len(a.Accounts) - len(a.Assets) - len(a.Apps) - if a.MaxBoxes < numBoxes { - numBoxes = a.MaxBoxes - } - return numBoxes + return min(a.MaxBoxes, a.MaxTotalRefs-len(a.Accounts)-len(a.Assets)-len(a.Apps)) } func (a *ResourceTracker) hasHolding(addr basics.Address, aid basics.AssetIndex) bool { @@ -345,6 +421,22 @@ func (a *ResourceTracker) addLocal(addr basics.Address, aid basics.AppIndex) boo return true } +// Simplify makes the ResourceTracker easier to consume for callers. It avoids +// returning boxes "named" by a newly created app. Those app IDs would not be +// usable in a later submission. An empty ref can stand in. Once Simplified, the +// tracker should not be used to for further tracking. +func (a *ResourceTracker) Simplify() { + for name, stat := range a.Boxes { + if stat.NewApp { + a.NumEmptyBoxRefs++ + delete(a.Boxes, name) + } + } + if len(a.Boxes) == 0 { + a.Boxes = nil + } +} + // groupResourceTracker calculates the additional resources that a transaction group could use, // and it tracks any referenced unnamed resources that fit within those limits. type groupResourceTracker struct { @@ -356,6 +448,7 @@ type groupResourceTracker struct { // sharing was added). localTxnResources []ResourceTracker + // startingBoxes is the total number of box references in the group, as submitted startingBoxes int } @@ -490,9 +583,14 @@ func (a *groupResourceTracker) hasBox(app basics.AppIndex, name string) bool { return a.globalResources.hasBox(app, name) } -func (a *groupResourceTracker) addBox(app basics.AppIndex, name string, readSize, additionalReadBudget, bytesPerBoxRef uint64) bool { +func (a *groupResourceTracker) addBox(app basics.AppIndex, name string, newApp bool, readSize uint64, bytesPerBoxRef uint64) bool { + // All boxes are global, never consult localTxnResources + return a.globalResources.addBox(app, name, newApp, readSize, bytesPerBoxRef) +} + +func (a *groupResourceTracker) ioSurplus(readSize int64, bytesPerBoxRef uint64) bool { // All boxes are global, never consult localTxnResources - return a.globalResources.addBox(app, name, readSize, additionalReadBudget, bytesPerBoxRef) + return a.globalResources.ioSurplus(readSize, bytesPerBoxRef) } func (a *groupResourceTracker) reconcileBoxWriteBudget(used uint64, bytesPerBoxRef uint64) error { @@ -532,7 +630,7 @@ func (a *groupResourceTracker) addLocal(addr basics.Address, aid basics.AppIndex type resourcePolicy struct { tracker groupResourceTracker ep *logic.EvalParams - initialBoxSurplusReadBudget *uint64 + initialBoxSurplusReadBudget *int64 txnRootIndex int programVersion uint64 @@ -588,13 +686,14 @@ func (p *resourcePolicy) AllowsLocal(addr basics.Address, aid basics.AppIndex) b return p.tracker.addLocal(addr, aid) } -func (p *resourcePolicy) AvailableBox(app basics.AppIndex, name string, operation logic.BoxOperation, createSize uint64) bool { +func (p *resourcePolicy) AvailableBox(app basics.AppIndex, name string, newApp bool, createSize uint64) bool { if p.tracker.hasBox(app, name) { - // We actually never expect this to happen, since the EvalContext remembers each box in - // order to track their dirty bytes, and it won't invoke this method if it's already seen - // the box. + // We never expect this to happen. The EvalContext remembers each box in + // order to track their dirty bytes, and it won't invoke this method if + // it's already seen the box. return true } + box, ok, err := p.ep.Ledger.GetBox(app, name) if err != nil { panic(err.Error()) @@ -603,5 +702,10 @@ func (p *resourcePolicy) AvailableBox(app basics.AppIndex, name string, operatio if ok { readSize = uint64(len(box)) } - return p.tracker.addBox(app, name, readSize, *p.initialBoxSurplusReadBudget, p.ep.Proto.BytesPerBoxReference) + + return p.tracker.addBox(app, name, newApp, readSize, p.ep.Proto.BytesPerBoxReference) +} + +func (p *resourcePolicy) IOSurplus(size int64) bool { + return p.tracker.ioSurplus(size, p.ep.Proto.BytesPerBoxReference) } diff --git a/ledger/simulation/simulation_eval_test.go b/ledger/simulation/simulation_eval_test.go index 936a3047c4..bfdc4e7df2 100644 --- a/ledger/simulation/simulation_eval_test.go +++ b/ledger/simulation/simulation_eval_test.go @@ -149,9 +149,17 @@ func simulationTest(t *testing.T, f func(env simulationtesting.Environment) simu } func runSimulationTestCase(t *testing.T, env simulationtesting.Environment, testcase simulationTestCase) { + t.Helper() + actual, err := simulation.MakeSimulator(env.Ledger, testcase.developerAPI).Simulate(testcase.input) require.NoError(t, err) + for i := range actual.TxnGroups { + if actual.TxnGroups[i].UnnamedResourcesAccessed != nil { + actual.TxnGroups[i].UnnamedResourcesAccessed.Simplify() + } + } + validateSimulationResult(t, actual) require.Len(t, testcase.expected.TxnGroups, len(testcase.input.TxnGroups), "Test case must expect the same number of transaction groups as its input") @@ -7067,9 +7075,9 @@ func TestUnnamedResources(t *testing.T) { if v >= 8 { // boxes introduced program += `byte "A"; int 64; box_create; assert;` program += `byte "B"; box_len; !; assert; !; assert;` - expectedUnnamedResourceGroupAssignment.Boxes = map[basics.BoxRef]uint64{ - {App: 0, Name: "A"}: 0, - {App: 0, Name: "B"}: 0, + expectedUnnamedResourceGroupAssignment.Boxes = map[basics.BoxRef]simulation.BoxStat{ + {App: 0, Name: "A"}: {}, + {App: 0, Name: "B"}: {}, } } @@ -7517,10 +7525,14 @@ int 1 } } -const boxTestProgram = `#pragma version %d +const verPragma = "#pragma version %d\n" + +const bailOnCreate = ` txn ApplicationID -bz end // Do nothing during create +bz end +` +const mainBoxTestProgram = ` byte "create" byte "delete" byte "read" @@ -7560,12 +7572,22 @@ end: int 1 ` +// boxTestProgram executes the operations defined by boxOperation +const boxTestProgram = verPragma + bailOnCreate + mainBoxTestProgram + +// boxDuringCreateProgram will even try to operate during the app creation. +const boxDuringCreateProgram = verPragma + mainBoxTestProgram + +// boxOperation is used to describe something we want done to a box. A +// transaction doing it will be created and run in a test. type boxOperation struct { op logic.BoxOperation name string createSize uint64 contents []byte otherRefCount int + withBoxRefs int // Add this many box refs to the generated transaction + duringCreate bool // If true, instantiate `boxDuringCreateProgram` to execute the op } func (o boxOperation) appArgs() [][]byte { @@ -7602,13 +7624,16 @@ func (o boxOperation) boxRefs() []transactions.BoxRef { } type boxTestResult struct { - Boxes map[basics.BoxRef]uint64 + Boxes map[basics.BoxRef]uint64 // maps observed boxes to their size when read NumEmptyBoxRefs int FailureMessage string FailingIndex int } +// testUnnamedBoxOperations creates a group with one transaction per boxOp, +// calling `app` with arguments meant to effect the boxOps. The results must +// match `expected`. func testUnnamedBoxOperations(t *testing.T, env simulationtesting.Environment, app basics.AppIndex, boxOps []boxOperation, expected boxTestResult) { t.Helper() @@ -7616,6 +7641,7 @@ func testUnnamedBoxOperations(t *testing.T, env simulationtesting.Environment, a require.LessOrEqual(t, len(boxOps), maxGroupSize) otherAssets := 0 + boxRefs := 0 txns := make([]*txntest.Txn, maxGroupSize) for i, op := range boxOps { txn := env.TxnInfo.NewTxn(txntest.Txn{ @@ -7624,10 +7650,18 @@ func testUnnamedBoxOperations(t *testing.T, env simulationtesting.Environment, a ApplicationID: app, ApplicationArgs: op.appArgs(), ForeignAssets: make([]basics.AssetIndex, op.otherRefCount), + Boxes: slices.Repeat(op.boxRefs(), op.withBoxRefs), Note: []byte{byte(i)}, // Make each txn unique }) + if op.duringCreate { + txn.ApplicationID = 0 + v := env.TxnInfo.CurrentProtocolParams().LogicSigVersion + txn.ApprovalProgram = fmt.Sprintf(boxDuringCreateProgram, v) + txn.ClearStateProgram = fmt.Sprintf("#pragma version %d\n int 1", v) + } txns[i] = &txn otherAssets += op.otherRefCount + boxRefs += op.withBoxRefs } for i := len(boxOps); i < maxGroupSize; i++ { // Fill out the rest of the group with non-app transactions. This reduces the amount of @@ -7649,6 +7683,11 @@ func testUnnamedBoxOperations(t *testing.T, env simulationtesting.Environment, a expectedTxnResults := make([]simulation.TxnResult, len(stxns)) for i := range expectedTxnResults { expectedTxnResults[i].AppBudgetConsumed = ignoreAppBudgetConsumed + if i < len(boxOps) && boxOps[i].duringCreate { + // 1007 here is because of the number of transactions we used to + // setup the env. See explanation in: TestUnnamedResourcesBoxIOBudget + expectedTxnResults[i].Txn.ApplyData.ApplicationID = 1007 + basics.AppIndex(i) + } } var failedAt simulation.TxnPath @@ -7661,14 +7700,19 @@ func testUnnamedBoxOperations(t *testing.T, env simulationtesting.Environment, a MaxAccounts: len(boxOps) * (proto.MaxAppTxnAccounts + proto.MaxAppTxnForeignApps), MaxAssets: len(boxOps)*proto.MaxAppTxnForeignAssets - otherAssets, MaxApps: len(boxOps) * proto.MaxAppTxnForeignApps, - MaxBoxes: len(boxOps) * proto.MaxAppBoxReferences, - MaxTotalRefs: len(boxOps)*proto.MaxAppTotalTxnReferences - otherAssets, + MaxBoxes: len(boxOps)*proto.MaxAppBoxReferences - boxRefs, + MaxTotalRefs: len(boxOps)*proto.MaxAppTotalTxnReferences - otherAssets - boxRefs, - Boxes: expected.Boxes, NumEmptyBoxRefs: expected.NumEmptyBoxRefs, MaxCrossProductReferences: len(boxOps) * proto.MaxAppTxnForeignApps * (proto.MaxAppTxnForeignApps + 2), } + if expected.Boxes != nil { + expectedUnnamedResources.Boxes = make(map[basics.BoxRef]simulation.BoxStat, len(expected.Boxes)) + for key, size := range expected.Boxes { + expectedUnnamedResources.Boxes[key] = simulation.BoxStat{ReadSize: size} + } + } if !expectedUnnamedResources.HasResources() { expectedUnnamedResources = nil @@ -7726,8 +7770,12 @@ func TestUnnamedResourcesBoxIOBudget(t *testing.T) { }) // MBR is needed for boxes. - transferable := env.Accounts[1].AcctData.MicroAlgos.Raw - proto.MinBalance - proto.MinTxnFee - env.TransferAlgos(env.Accounts[1].Addr, appID.Address(), transferable) + transferable := env.Accounts[1].AcctData.MicroAlgos.Raw - proto.MinBalance - 2*proto.MinTxnFee + env.TransferAlgos(env.Accounts[1].Addr, appID.Address(), transferable/2) + // we're also going to make new boxes in a new app, which will be + // the sixth txns after the appID creation (because of two + // TrsnaferAlgos and 3 env.Txn, below) + env.TransferAlgos(env.Accounts[1].Addr, (appID + 6).Address(), transferable/2) // Set up boxes A, B, C for testing. // A is a box with a size of exactly BytesPerBoxReference @@ -7776,7 +7824,7 @@ func TestUnnamedResourcesBoxIOBudget(t *testing.T) { // in separate simulations, so we can reuse the same environment and not have to worry // about the effects of one test interfering with another. - // Reading exisitng boxes + // Reading existing boxes testBoxOps([]boxOperation{ {op: logic.BoxReadOperation, name: "A"}, }, boxTestResult{ @@ -7800,6 +7848,12 @@ func TestUnnamedResourcesBoxIOBudget(t *testing.T) { // We need an additional empty box ref because the size of C exceeds BytesPerBoxReference NumEmptyBoxRefs: 1, }) + testBoxOps([]boxOperation{ + {op: logic.BoxReadOperation, name: "C", withBoxRefs: 1}, + }, boxTestResult{ + // We need an additional empty box ref because the size of C exceeds BytesPerBoxReference + NumEmptyBoxRefs: 1, + }) testBoxOps([]boxOperation{ {op: logic.BoxReadOperation, name: "A"}, {op: logic.BoxReadOperation, name: "B"}, @@ -7831,6 +7885,8 @@ func TestUnnamedResourcesBoxIOBudget(t *testing.T) { }, // No empty box refs needed because we have perfectly reached 3 * BytesPerBoxReference }) + + // non-existent box testBoxOps([]boxOperation{ {op: logic.BoxReadOperation, name: "Q"}, }, boxTestResult{ @@ -7873,6 +7929,30 @@ func TestUnnamedResourcesBoxIOBudget(t *testing.T) { }, }) + // Try to read during a new app create. These boxes _can't_ exist, so no need for extra read quota + testBoxOps([]boxOperation{ + {op: logic.BoxReadOperation, name: "X", duringCreate: true}, + }, boxTestResult{ + NumEmptyBoxRefs: 1, + }) + testBoxOps([]boxOperation{ + {op: logic.BoxReadOperation, name: "X", duringCreate: true}, + {op: logic.BoxReadOperation, name: "Y", duringCreate: true}, + }, boxTestResult{ + NumEmptyBoxRefs: 2, + }) + // now try to create, which can cause enough dirty bytes to require empty refs + testBoxOps([]boxOperation{ + {op: logic.BoxCreateOperation, name: "small", createSize: proto.BytesPerBoxReference, duringCreate: true}, + }, boxTestResult{ + NumEmptyBoxRefs: 1, + }) + testBoxOps([]boxOperation{ + {op: logic.BoxCreateOperation, name: "big", createSize: proto.BytesPerBoxReference + 1, duringCreate: true}, + }, boxTestResult{ + NumEmptyBoxRefs: 2, + }) + // Creating new boxes and reading existing ones testBoxOps([]boxOperation{ {op: logic.BoxCreateOperation, name: "D", createSize: proto.BytesPerBoxReference + 2}, @@ -8506,7 +8586,7 @@ func testUnnamedResourceLimits(t *testing.T, env simulationtesting.Environment, MaxBoxes: proto.MaxAppBoxReferences, MaxTotalRefs: proto.MaxAppTotalTxnReferences, - Boxes: mapWithKeys(boxNamesToRefs(app, resources.boxes()), uint64(0)), + Boxes: mapWithKeys(boxNamesToRefs(app, resources.boxes()), simulation.BoxStat{}), MaxCrossProductReferences: proto.MaxAppTxnForeignApps * (proto.MaxAppTxnForeignApps + 2), } diff --git a/ledger/simulation/testing/utils.go b/ledger/simulation/testing/utils.go index 36cd0a1d34..27f45a4e13 100644 --- a/ledger/simulation/testing/utils.go +++ b/ledger/simulation/testing/utils.go @@ -239,7 +239,7 @@ func (env *Environment) Rekey(account, rekeyTo basics.Address) { // PrepareSimulatorTest creates an environment to test transaction simulations. The caller is // responsible for calling Close() on the returned Environment. func PrepareSimulatorTest(t *testing.T) Environment { - genesisInitState, keys := ledgertesting.GenerateInitState(t, protocol.ConsensusFuture, 100) + genesisInitState, keys := ledgertesting.GenerateInitState(t, protocol.ConsensusFuture, 200) // Prepare ledger const inMem = true