combine app state key/value size limits

config/consensus.go

@@ -257,6 +257,9 @@ type ConsensusParams struct {
 	// local key/value store
 	MaxAppBytesValueLen int
 
+	// maximum sum of the lengths of the key and value of one app state entry
+	MaxAppSumKeyValueLens int
+
 	// maximum number of applications a single account can create and store
 	// AppParams for at once
 	MaxAppsCreated int
@@ -815,6 +818,7 @@ func initConsensusProtocols() {
 	v24.MaxAppProgramLen = 1024
 	v24.MaxAppKeyLen = 64
 	v24.MaxAppBytesValueLen = 64
+	v24.MaxAppSumKeyValueLens = 128 // Set here to have no effect until MaxAppBytesValueLen increases
 
 	// 0.1 Algos (Same min balance cost as an Asset)
 	v24.AppFlatParamsMinBalance = 100000
@@ -904,6 +908,9 @@ func initConsensusProtocols() {
 	vFuture := v27
 	vFuture.ApprovedUpgrades = map[protocol.ConsensusVersion]uint64{}
 
+	// Let the bytes value take more space. Key+Value is still limited to 128
+	vFuture.MaxAppBytesValueLen = 128
+
 	// FilterTimeout for period 0 should take a new optimized, configured value, need to revisit this later
 	vFuture.AgreementFilterTimeoutPeriod0 = 4 * time.Second
 

daemon/algod/api/server/v2/dryrun_test.go

@@ -351,6 +351,7 @@ func init() {
 	proto.MaxAppProgramCost = 700
 	proto.MaxAppKeyLen = 64
 	proto.MaxAppBytesValueLen = 64
+	proto.MaxAppSumKeyValueLens = 128
 
 	config.Consensus[dryrunProtoVersion] = proto
 }

data/basics/teal.go

@@ -109,7 +109,10 @@ func (sd StateDelta) Valid(proto *config.ConsensusParams) error {
 		switch delta.Action {
 		case SetBytesAction:
 			if len(delta.Bytes) > proto.MaxAppBytesValueLen {
-				return fmt.Errorf("cannot set value for key 0x%x, too long: length was %d, maximum is %d", key, len(delta.Bytes), proto.MaxAppBytesValueLen)
+				return fmt.Errorf("value too long for key 0x%x: length was %d", key, len(delta.Bytes))
+			}
+			if sum := len(key) + len(delta.Bytes); sum > proto.MaxAppSumKeyValueLens {
+				return fmt.Errorf("key/value total too long for key 0x%x: sum was %d", key, sum)
 			}
 		case SetUintAction:
 		case DeleteAction:

data/basics/teal_test.go

@@ -47,25 +47,27 @@ func TestStateDeltaValid(t *testing.T) {
 	a.Error(err)
 	a.Contains(err.Error(), "proto.MaxAppKeyLen is 0")
 
-	// test proto with applications
+	// test vFuture proto with applications
 	sd = StateDelta{"key": ValueDelta{Action: SetBytesAction, Bytes: "val"}}
 	protoF := config.Consensus[protocol.ConsensusFuture]
 	err = sd.Valid(&protoF)
 	a.NoError(err)
 
+	// vFuture: key too long, short value
 	tooLongKey := strings.Repeat("a", protoF.MaxAppKeyLen+1)
-	sd[tooLongKey] = ValueDelta{Action: SetBytesAction, Bytes: "val"}
+	sd = StateDelta{tooLongKey: ValueDelta{Action: SetBytesAction, Bytes: "val"}}
 	err = sd.Valid(&protoF)
 	a.Error(err)
 	a.Contains(err.Error(), "key too long")
 	delete(sd, tooLongKey)
 
+	// vFuture: max size key, value too long: total size bigger than MaxAppSumKeyValueLens
 	longKey := tooLongKey[1:]
-	tooLongValue := strings.Repeat("b", protoF.MaxAppBytesValueLen+1)
-	sd[longKey] = ValueDelta{Action: SetBytesAction, Bytes: tooLongValue}
+	tooLongValue := strings.Repeat("b", protoF.MaxAppSumKeyValueLens-len(longKey)+1)
+	sd = StateDelta{longKey: ValueDelta{Action: SetBytesAction, Bytes: tooLongValue}}
 	err = sd.Valid(&protoF)
 	a.Error(err)
-	a.Contains(err.Error(), "cannot set value for key")
+	a.Contains(err.Error(), "key/value total too long for key")
 
 	sd[longKey] = ValueDelta{Action: SetBytesAction, Bytes: tooLongValue[1:]}
 	sd["intval"] = ValueDelta{Action: DeltaAction(10), Uint: 0}
@@ -79,6 +81,27 @@ func TestStateDeltaValid(t *testing.T) {
 	a.NoError(err)
 }
 
+func TestStateDeltaValidV24(t *testing.T) {
+	a := require.New(t)
+
+	// v24: short key, value too long: hits MaxAppBytesValueLen
+	protoV24 := config.Consensus[protocol.ConsensusV24]
+	shortKey := "k"
+	reallyLongValue := strings.Repeat("b", protoV24.MaxAppBytesValueLen+1)
+	sd := StateDelta{shortKey: ValueDelta{Action: SetBytesAction, Bytes: reallyLongValue}}
+	err := sd.Valid(&protoV24)
+	a.Error(err)
+	a.Contains(err.Error(), "value too long for key")
+
+	// v24: key too long, short value
+	tooLongKey := strings.Repeat("a", protoV24.MaxAppKeyLen+1)
+	sd = StateDelta{tooLongKey: ValueDelta{Action: SetBytesAction, Bytes: "val"}}
+	err = sd.Valid(&protoV24)
+	a.Error(err)
+	a.Contains(err.Error(), "key too long")
+	delete(sd, tooLongKey)
+}
+
 func TestStateDeltaEqual(t *testing.T) {
 	a := require.New(t)
 

data/basics/userBalance_test.go

@@ -154,22 +154,26 @@ func TestEncodedAccountDataSize(t *testing.T) {
 	maxProg := []byte(makeString(currentConsensusParams.MaxAppProgramLen))
 	maxGlobalState := make(TealKeyValue, currentConsensusParams.MaxGlobalSchemaEntries)
 	maxLocalState := make(TealKeyValue, currentConsensusParams.MaxLocalSchemaEntries)
-	maxValue := TealValue{
-		Type:  TealBytesType,
-		Bytes: makeString(currentConsensusParams.MaxAppBytesValueLen),
-	}
 
 	for globalKey := uint64(0); globalKey < currentConsensusParams.MaxGlobalSchemaEntries; globalKey++ {
 		prefix := fmt.Sprintf("%d|", globalKey)
 		padding := makeString(currentConsensusParams.MaxAppKeyLen - len(prefix))
 		maxKey := prefix + padding
+		maxValue := TealValue{
+			Type:  TealBytesType,
+			Bytes: makeString(currentConsensusParams.MaxAppSumKeyValueLens - len(maxKey)),
+		}
 		maxGlobalState[maxKey] = maxValue
 	}
 
 	for localKey := uint64(0); localKey < currentConsensusParams.MaxLocalSchemaEntries; localKey++ {
 		prefix := fmt.Sprintf("%d|", localKey)
 		padding := makeString(currentConsensusParams.MaxAppKeyLen - len(prefix))
 		maxKey := prefix + padding
+		maxValue := TealValue{
+			Type:  TealBytesType,
+			Bytes: makeString(currentConsensusParams.MaxAppSumKeyValueLens - len(maxKey)),
+		}
 		maxLocalState[maxKey] = maxValue
 	}
 

data/transactions/logic/evalStateful_test.go

@@ -2145,6 +2145,40 @@ byte "myval"
 	require.Empty(t, delta.LocalDeltas)
 }
 
+func TestBlankKey(t *testing.T) {
+	t.Parallel()
+	source := `
+byte ""
+app_global_get
+int 0
+==
+assert
+
+byte ""
+int 7
+app_global_put
+
+byte ""
+app_global_get
+int 7
+==
+`
+	ep := defaultEvalParams(nil, nil)
+	txn := makeSampleTxn()
+	txn.Txn.ApplicationID = 100
+	ep.Txn = &txn
+	ledger := makeTestLedger(
+		map[basics.Address]uint64{
+			txn.Txn.Sender: 1,
+		},
+	)
+	ep.Ledger = ledger
+	ledger.newApp(txn.Txn.Sender, 100, makeSchemas(0, 0, 0, 0))
+
+	delta := testApp(t, source, ep)
+	require.Empty(t, delta.LocalDeltas)
+}
+
 func TestAppGlobalDelete(t *testing.T) {
 	t.Parallel()
 

ledger/accountdb_test.go

@@ -137,7 +137,7 @@ func randomFullAccountData(rewardsLevel, lastCreatableID uint64) (basics.Account
 				tv := basics.TealValue{
 					Type: basics.TealBytesType,
 				}
-				bytes := make([]byte, crypto.RandUint64()%uint64(config.MaxBytesKeyValueLen))
+				bytes := make([]byte, crypto.RandUint64()%uint64(config.MaxBytesKeyValueLen-len(appName)))
 				crypto.RandBytes(bytes[:])
 				tv.Bytes = string(bytes)
 				ap.KeyValue[appName] = tv

ledger/appcow.go

@@ -331,8 +331,13 @@ func (cb *roundCowState) SetKey(addr basics.Address, aidx basics.AppIndex, globa
 	}
 
 	// Enforce maximum value length
-	if value.Type == basics.TealBytesType && len(value.Bytes) > cb.proto.MaxAppBytesValueLen {
-		return fmt.Errorf("value too long for key 0x%x: length was %d, maximum is %d", key, len(value.Bytes), cb.proto.MaxAppBytesValueLen)
+	if value.Type == basics.TealBytesType {
+		if len(value.Bytes) > cb.proto.MaxAppBytesValueLen {
+			return fmt.Errorf("value too long for key 0x%x: length was %d", key, len(value.Bytes))
+		}
+		if sum := len(key) + len(value.Bytes); sum > cb.proto.MaxAppSumKeyValueLens {
+			return fmt.Errorf("key/value total too long for key 0x%x: sum was %d", key, sum)
+		}
 	}
 
 	// Check that account has allocated storage

ledger/appcow_test.go

@@ -1171,6 +1171,39 @@ func TestCowSetKey(t *testing.T) {
 	a.Panics(func() { c.SetKey(addr, aidx+1, false, key, tv, 0) })
 }
 
+func TestCowSetKeyVFuture(t *testing.T) {
+	a := require.New(t)
+
+	addr := getRandomAddress(a)
+	aidx := basics.AppIndex(1)
+	c := getCow([]modsData{
+		{addr, basics.CreatableIndex(aidx), basics.AppCreatable},
+	})
+	protoF := config.Consensus[protocol.ConsensusFuture]
+	c.proto = protoF
+
+	key := strings.Repeat("key", 100)
+	val := "val"
+	tv := basics.TealValue{Type: basics.TealBytesType, Bytes: val}
+	err := c.SetKey(addr, aidx, true, key, tv, 0)
+	a.Error(err)
+	a.Contains(err.Error(), "key too long")
+
+	key = "key"
+	val = strings.Repeat("val", 100)
+	tv = basics.TealValue{Type: basics.TealBytesType, Bytes: val}
+	err = c.SetKey(addr, aidx, true, key, tv, 0)
+	a.Error(err)
+	a.Contains(err.Error(), "value too long")
+
+	key = strings.Repeat("k", protoF.MaxAppKeyLen)
+	val = strings.Repeat("v", protoF.MaxAppSumKeyValueLens-len(key)+1)
+	tv = basics.TealValue{Type: basics.TealBytesType, Bytes: val}
+	err = c.SetKey(addr, aidx, true, key, tv, 0)
+	a.Error(err)
+	a.Contains(err.Error(), "key/value total too long")
+}
+
 func TestCowAccountIdx(t *testing.T) {
 	a := require.New(t)
 

ledger/catchpointwriter_test.go

@@ -94,16 +94,15 @@ func makeTestEncodedBalanceRecord(t *testing.T) encodedBalanceRecord {
 
 	maxApps := currentConsensusParams.MaxAppsCreated
 	maxOptIns := currentConsensusParams.MaxAppsOptedIn
-	maxBytesLen := currentConsensusParams.MaxAppKeyLen
-	if maxBytesLen > currentConsensusParams.MaxAppBytesValueLen {
-		maxBytesLen = currentConsensusParams.MaxAppBytesValueLen
-	}
+	maxKeyBytesLen := currentConsensusParams.MaxAppKeyLen
+	maxSumBytesLen := currentConsensusParams.MaxAppSumKeyValueLens
+
 	genKey := func() (string, basics.TealValue) {
-		len := int(crypto.RandUint64() % uint64(maxBytesLen))
+		len := int(crypto.RandUint64() % uint64(maxKeyBytesLen))
 		if len == 0 {
 			return "k", basics.TealValue{Type: basics.TealUintType, Uint: 0}
 		}
-		key := make([]byte, len)
+		key := make([]byte, maxSumBytesLen-len)
 		crypto.RandBytes(key)
 		return string(key), basics.TealValue{Type: basics.TealUintType, Bytes: string(key)}
 	}

test/scripts/e2e_subs/app-key-value-128.sh

@@ -0,0 +1,72 @@
+#!/bin/bash
+
+filename=$(basename "$0")
+scriptname="${filename%.*}"
+date "+${scriptname} start %Y%m%d_%H%M%S"
+
+set -e
+set -x
+set -o pipefail
+export SHELLOPTS
+
+WALLET=$1
+
+TEAL=test/scripts/e2e_subs/tealprogs
+
+gcmd="goal -w ${WALLET}"
+
+ACCOUNT=$(${gcmd} account list|awk '{ print $3 }')
+
+APPID=$(${gcmd} app create --creator "${ACCOUNT}" --approval-prog=${TEAL}/state-rw.teal --global-byteslices 2 --global-ints 0 --local-byteslices 2 --local-ints 0  --clear-prog=${TEAL}/approve-all.teal | grep Created | awk '{ print $6 }')
+
+function call {
+    ${gcmd} app call --app-id=$APPID --from=$ACCOUNT --app-arg=str:$1  --app-arg=str:$2  --app-arg=str:$3  --app-arg=str:$4
+}
+
+BIG64="1234567890123456789012345678901234567890123456789012345678901234"
+
+# This should work because the value is longer than 64, but the sum is still under 128
+call write global hello ${BIG64}EVENLONGEREVENLONGEREVENLONGEREVENLONGER
+call check global hello ${BIG64}EVENLONGEREVENLONGEREVENLONGEREVENLONGER
+
+# And this is on the edge of ok - both are 64
+call write global $BIG64 $BIG64
+call check global $BIG64 $BIG64
+
+# This causes problems because the sum is too big
+call write global $BIG64 ${BIG64}X && exit 1
+
+# These test some details of the checking, using the error message to
+# confirm which code path is being tested.  Details of strings are irrelevant
+set +o pipefail
+call write global $BIG64 ${BIG64}X 2>&1 | grep "key/value total too long"
+# This value so big that it fails before the sum is considered
+call write global $BIG64 ${BIG64}${BIG64}X 2>&1 | grep "value too long" | grep length
+set -o pipefail
+
+
+# Same tests below, but on LOCAL state (so first have to deal with opt-in)
+
+set +o pipefail
+call check local hello xyz && exit 1
+call check local hello xyz 2>&1 | grep "has not opted in"
+set -o pipefail
+
+${gcmd} app optin --app-id "$APPID" --from "${ACCOUNT}"
+
+# This should work because the value is longer than 64, but the sum is still under 128
+call write local hello ${BIG64}EVENLONGEREVENLONGEREVENLONGEREVENLONGER
+call check local hello ${BIG64}EVENLONGEREVENLONGEREVENLONGEREVENLONGER
+
+# And this is on the edge of ok - both are 64
+call write local $BIG64 $BIG64
+call check local $BIG64 $BIG64
+
+# This should not work because the key 64 and the value is 65
+set +o pipefail
+call write local $BIG64 ${BIG64}X 2>&1 && exit 1
+call write local $BIG64 ${BIG64}X 2>&1 | grep "key/value total too long"
+set -o pipefail
+
+
+date "+${scriptname} OK %Y%m%d_%H%M%S"

test/scripts/e2e_subs/tealprogs/approve-all.teal

@@ -0,0 +1,2 @@
+#pragma version 2
+        int 1