Move cost checking into Check() for static cost checks.

This allows callers to do less work. The consensus protocol was already being supplied, so Check() has everything it needed to do the right thing. This also consolidates the code that will act differently for dynamic teal costs.
algorand · tsachiherman · May 14, 2021 · Mar 15, 2021 · Mar 19, 2021 · Apr 27, 2021
commit 60ce9eee7d79605d3cb60b493ca3a373e07fe688
diff --git a/cmd/goal/clerk.go b/cmd/goal/clerk.go
@@ -1080,13 +1080,10 @@ var dryrunCmd = &cobra.Command{
 				reportErrorf("program size too large: %d > %d", len(txn.Lsig.Logic), params.LogicSigMaxSize)
 			}
 			ep := logic.EvalParams{Txn: &txn, Proto: &params, GroupIndex: i, TxnGroup: txgroup}
-			cost, err := logic.Check(txn.Lsig.Logic, ep)
+			err := logic.Check(txn.Lsig.Logic, ep)
 			if err != nil {
 				reportErrorf("program failed Check: %s", err)
 			}
-			if uint64(cost) > params.LogicSigMaxCost {
-				reportErrorf("program cost too large: %d > %d", cost, params.LogicSigMaxCost)
-			}
 			sb := strings.Builder{}
 			ep = logic.EvalParams{
 				Txn:        &txn,
@@ -1097,7 +1094,7 @@ var dryrunCmd = &cobra.Command{
 			}
 			pass, err := logic.Eval(txn.Lsig.Logic, ep)
 			// TODO: optionally include `inspect` output here?
-			fmt.Fprintf(os.Stdout, "tx[%d] cost=%d trace:\n%s\n", i, cost, sb.String())
+			fmt.Fprintf(os.Stdout, "tx[%d] trace:\n%s\n", i, sb.String())
 			if pass {
 				fmt.Fprintf(os.Stdout, " - pass -\n")
 			} else {

diff --git a/config/consensus.go b/config/consensus.go
@@ -212,6 +212,9 @@ type ConsensusParams struct {
 	// sum of estimated op cost must be less than this
 	LogicSigMaxCost uint64
 
+	// calculate TEAL costs at runtime
+	DynamicTealCost bool
+
 	// max decimal precision for assets
 	MaxAssetDecimals uint32
 
@@ -910,6 +913,8 @@ func initConsensusProtocols() {
 	// Enable transaction Merkle tree.
 	vFuture.PaysetCommit = PaysetCommitMerkle
 
+	vFuture.DynamicTealCost = true
+
 	Consensus[protocol.ConsensusFuture] = vFuture
 }
 

diff --git a/data/transactions/logic/backwardCompat_test.go b/data/transactions/logic/backwardCompat_test.go
@@ -267,9 +267,9 @@ func TestBackwardCompatTEALv1(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, program, ops.Program)
 	// ensure the old program is the same as a new one except TEAL version byte
-	ops, err = AssembleStringWithVersion(sourceTEALv1, AssemblerMaxVersion)
+	opsV2, err := AssembleStringWithVersion(sourceTEALv1, 2)
 	require.NoError(t, err)
-	require.Equal(t, program[1:], ops.Program[1:])
+	require.Equal(t, program[1:], opsV2.Program[1:])
 
 	sig := c.Sign(Msg{
 		ProgramHash: crypto.HashObj(Program(program)),
@@ -285,14 +285,22 @@ func TestBackwardCompatTEALv1(t *testing.T) {
 	txn.Txn.RekeyTo = basics.Address{} // RekeyTo not allowed in TEAL v1
 
 	sb := strings.Builder{}
-	ep := defaultEvalParams(&sb, &txn)
+	ep := defaultEvalParamsWithVersion(&sb, &txn, 1)
 	ep.TxnGroup = txgroup
 
 	// ensure v1 program runs well on latest TEAL evaluator
 	require.Equal(t, uint8(1), program[0])
-	cost, err := Check(program, ep)
+
+	// Cost should stay exactly 2140
+	ep.Proto.LogicSigMaxCost = 2139
+	err = Check(program, ep)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "static cost")
+
+	ep.Proto.LogicSigMaxCost = 2140
+	err = Check(program, ep)
 	require.NoError(t, err)
-	require.Equal(t, 2140, cost)
+
 	pass, err := Eval(program, ep)
 	if err != nil || !pass {
 		t.Log(hex.EncodeToString(program))
@@ -301,12 +309,19 @@ func TestBackwardCompatTEALv1(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, pass)
 
-	cost2, err := Check(ops.Program, ep)
+	// Costs for v2 should be higher because of hash opcode cost changes
+	ep2 := defaultEvalParamsWithVersion(&sb, &txn, 2)
+	ep2.TxnGroup = txgroup
+	ep2.Proto.LogicSigMaxCost = 2307
+	err = Check(opsV2.Program, ep2)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "static cost")
+
+	ep2.Proto.LogicSigMaxCost = 2308
+	err = Check(opsV2.Program, ep2)
 	require.NoError(t, err)
 
-	// Costs for v2 should be higher because of hash opcode cost changes
-	require.Equal(t, 2308, cost2)
-	pass, err = Eval(ops.Program, ep)
+	pass, err = Eval(opsV2.Program, ep2)
 	if err != nil || !pass {
 		t.Log(hex.EncodeToString(ops.Program))
 		t.Log(sb.String())
@@ -315,19 +330,30 @@ func TestBackwardCompatTEALv1(t *testing.T) {
 	require.True(t, pass)
 
 	// ensure v0 program runs well on latest TEAL evaluator
+	ep = defaultEvalParams(&sb, &txn)
+	ep.TxnGroup = txgroup
 	program[0] = 0
 	sig = c.Sign(Msg{
 		ProgramHash: crypto.HashObj(Program(program)),
 		Data:        data[:],
 	})
 	txn.Lsig.Logic = program
 	txn.Lsig.Args = [][]byte{data[:], sig[:], pk[:], txn.Txn.Sender[:], txn.Txn.Note}
-	cost, err = Check(program, ep)
+
+	// Cost is now dynamic and exactly 1 less, because bnz skips "err". It's caught during Eval
+	ep.Proto.LogicSigMaxCost = 2138
+	err = Check(program, ep)
+	require.NoError(t, err)
+	_, err = Eval(program, ep)
+	require.Error(t, err)
+
+	ep.Proto.LogicSigMaxCost = 2139
+	err = Check(program, ep)
 	require.NoError(t, err)
-	require.Equal(t, 2140, cost)
 	pass, err = Eval(program, ep)
 	require.NoError(t, err)
 	require.True(t, pass)
+
 }
 
 // ensure v2 fields error on pre TEAL v2 logicsig version

diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go
@@ -431,26 +431,31 @@ func eval(program []byte, cx *evalContext) (pass bool, err error) {
 	return cx.stack[0].Uint != 0, nil
 }
 
-// CheckStateful should be faster than EvalStateful.
-// Returns 'cost' which is an estimate of relative execution time.
-func CheckStateful(program []byte, params EvalParams) (cost int, err error) {
+// CheckStateful should be faster than EvalStateful.  It can perform
+// static checks and reject programs that are invalid. Returns a cost
+// estimate of relative execution time. This cost is not relevent when
+// params.Proto.DynamicTealCost is true, and so 1 is returned so that
+// callers may continue to check for a high cost being invalid.
+func CheckStateful(program []byte, params EvalParams) error {
 	params.runModeFlags = runModeApplication
 	return check(program, params)
 }
 
-// Check should be faster than Eval.
-// Returns 'cost' which is an estimate of relative execution time.
-func Check(program []byte, params EvalParams) (cost int, err error) {
+// Check should be faster than Eval.  It can perform static checks and
+// reject programs that are invalid. Returns a cost estimate of
+// relative execution time. This cost is no relevent when
+// proto.DynamicTealCost is true, and so 1 is returned so that callers
+// may continue to check for a high cost being invalid.
+func Check(program []byte, params EvalParams) error {
 	params.runModeFlags = runModeSignature
 	return check(program, params)
 }
 
-func check(program []byte, params EvalParams) (cost int, err error) {
+func check(program []byte, params EvalParams) (err error) {
 	defer func() {
 		if x := recover(); x != nil {
 			buf := make([]byte, 16*1024)
 			stlen := runtime.Stack(buf, false)
-			cost = 0
 			errstr := string(buf[:stlen])
 			if params.Trace != nil {
 				if sb, ok := params.Trace.(*strings.Builder); ok {
@@ -462,18 +467,17 @@ func check(program []byte, params EvalParams) (cost int, err error) {
 		}
 	}()
 	if (params.Proto == nil) || (params.Proto.LogicSigVersion == 0) {
-		err = errLogicSigNotSupported
-		return
+		return errLogicSigNotSupported
 	}
 	version, vlen := binary.Uvarint(program)
 	if vlen <= 0 {
-		return 0, errors.New("invalid version")
+		return errors.New("invalid version")
 	}
 	if version > EvalMaxVersion {
-		return 0, fmt.Errorf("program version %d greater than max supported version %d", version, EvalMaxVersion)
+		return fmt.Errorf("program version %d greater than max supported version %d", version, EvalMaxVersion)
 	}
 	if version > params.Proto.LogicSigVersion {
-		return 0, fmt.Errorf("program version %d greater than protocol supported version %d", version, params.Proto.LogicSigVersion)
+		return fmt.Errorf("program version %d greater than protocol supported version %d", version, params.Proto.LogicSigVersion)
 	}
 
 	var minVersion uint64
@@ -483,7 +487,7 @@ func check(program []byte, params EvalParams) (cost int, err error) {
 		minVersion = *params.MinTealVersion
 	}
 	if version < minVersion {
-		return 0, fmt.Errorf("program version must be >= %d for this transaction group, but have version %d", minVersion, version)
+		return fmt.Errorf("program version must be >= %d for this transaction group, but have version %d", minVersion, version)
 	}
 
 	var cx evalContext
@@ -494,21 +498,30 @@ func check(program []byte, params EvalParams) (cost int, err error) {
 	cx.branchTargets = make(map[int]bool)
 	cx.instructionStarts = make(map[int]bool)
 
+	maxCost := params.budget()
+	if params.Proto.DynamicTealCost {
+		maxCost = math.MaxInt32
+	}
+	staticCost := 0
 	for cx.pc < len(cx.program) {
 		prevpc := cx.pc
-		err := cx.checkStep()
+		stepCost, err := cx.checkStep()
 		if err != nil {
-			return cx.cost, fmt.Errorf("%3d %w", cx.pc, err)
+			return fmt.Errorf("pc=%3d %w", cx.pc, err)
+		}
+		staticCost += stepCost
+		if staticCost > maxCost {
+			return fmt.Errorf("pc=%3d static cost budget %d exceeded", cx.pc, maxCost)
 		}
 		if cx.pc <= prevpc {
 			// Recall, this is advancing through opcodes
 			// without evaluation. It always goes forward,
 			// even if we're in v4 and the jump would go
 			// back.
-			return cx.cost, fmt.Errorf("pc did not advance, stuck at %d", cx.pc)
+			return fmt.Errorf("pc did not advance, stuck at %d", cx.pc)
 		}
 	}
-	return cx.cost, cx.err
+	return nil
 }
 
 func opCompat(expected, got StackType) bool {
@@ -636,26 +649,25 @@ func (cx *evalContext) step() {
 	}
 }
 
-func (cx *evalContext) checkStep() error {
+func (cx *evalContext) checkStep() (int, error) {
 	cx.instructionStarts[cx.pc] = true
 	opcode := cx.program[cx.pc]
 	spec := &opsByOpcode[cx.version][opcode]
 	if spec.op == nil {
-		return fmt.Errorf("%3d illegal opcode 0x%02x", cx.pc, opcode)
+		return 0, fmt.Errorf("%3d illegal opcode 0x%02x", cx.pc, opcode)
 	}
 	if (cx.runModeFlags & spec.Modes) == 0 {
-		return fmt.Errorf("%s not allowed in current mode", spec.Name)
+		return 0, fmt.Errorf("%s not allowed in current mode", spec.Name)
 	}
 	deets := spec.Details
 	if deets.Size != 0 && (cx.pc+deets.Size > len(cx.program)) {
-		return fmt.Errorf("%3d %s program ends short of immediate values", cx.pc, spec.Name)
+		return 0, fmt.Errorf("%3d %s program ends short of immediate values", cx.pc, spec.Name)
 	}
 	prevpc := cx.pc
-	cx.cost += deets.Cost
 	if deets.checkFunc != nil {
 		err := deets.checkFunc(cx)
 		if err != nil {
-			return err
+			return 0, err
 		}
 		if cx.nextpc != 0 {
 			cx.pc = cx.nextpc
@@ -672,11 +684,11 @@ func (cx *evalContext) checkStep() error {
 	if cx.err == nil {
 		for pc := prevpc + 1; pc < cx.pc; pc++ {
 			if _, ok := cx.branchTargets[pc]; ok {
-				return fmt.Errorf("branch target %d is not an aligned instruction", pc)
+				return 0, fmt.Errorf("branch target %d is not an aligned instruction", pc)
 			}
 		}
 	}
-	return nil
+	return deets.Cost, nil
 }
 
 func opErr(cx *evalContext) {