AVM: Allow access to boxes for apps made in the same group without explicit boxrefs

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
 

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

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 {

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,18 +557,18 @@ 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{
 		Txns:                     txnResults,
 		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),

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 {

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()

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
 	}