AVM: Rework around opcode fields for more flexible costs (#3832)

Author: jannotti

cmd/opdoc/opdoc.go

@@ -77,11 +77,11 @@ func integerConstantsTableMarkdown(out io.Writer) {
 	out.Write([]byte("\n"))
 }
 
-type speccer interface {
-	SpecByName(name string) logic.FieldSpec
+func fieldGroupMarkdown(out io.Writer, group logic.FieldGroup) {
+	fieldSpecsMarkdown(out, group.Names, group.Specs)
 }
 
-func fieldSpecsMarkdown(out io.Writer, names []string, specs speccer) {
+func fieldSpecsMarkdown(out io.Writer, names []string, specs logic.FieldSpecMap) {
 	showTypes := false
 	showVers := false
 	spec0 := specs.SpecByName(names[0])
@@ -127,37 +127,37 @@ func fieldSpecsMarkdown(out io.Writer, names []string, specs speccer) {
 
 func transactionFieldsMarkdown(out io.Writer) {
 	fmt.Fprintf(out, "\n`txn` Fields (see [transaction reference](https://developer.algorand.org/docs/reference/transactions/)):\n\n")
-	fieldSpecsMarkdown(out, logic.TxnFieldNames, logic.TxnFieldSpecByName)
+	fieldGroupMarkdown(out, logic.TxnFields)
 }
 
 func globalFieldsMarkdown(out io.Writer) {
 	fmt.Fprintf(out, "\n`global` Fields:\n\n")
-	fieldSpecsMarkdown(out, logic.GlobalFieldNames, logic.GlobalFieldSpecByName)
+	fieldGroupMarkdown(out, logic.GlobalFields)
 }
 
 func assetHoldingFieldsMarkdown(out io.Writer) {
 	fmt.Fprintf(out, "\n`asset_holding_get` Fields:\n\n")
-	fieldSpecsMarkdown(out, logic.AssetHoldingFieldNames, logic.AssetHoldingFieldSpecByName)
+	fieldGroupMarkdown(out, logic.AssetHoldingFields)
 }
 
 func assetParamsFieldsMarkdown(out io.Writer) {
 	fmt.Fprintf(out, "\n`asset_params_get` Fields:\n\n")
-	fieldSpecsMarkdown(out, logic.AssetParamsFieldNames, logic.AssetParamsFieldSpecByName)
+	fieldGroupMarkdown(out, logic.AssetParamsFields)
 }
 
 func appParamsFieldsMarkdown(out io.Writer) {
 	fmt.Fprintf(out, "\n`app_params_get` Fields:\n\n")
-	fieldSpecsMarkdown(out, logic.AppParamsFieldNames, logic.AppParamsFieldSpecByName)
+	fieldGroupMarkdown(out, logic.AppParamsFields)
 }
 
 func acctParamsFieldsMarkdown(out io.Writer) {
 	fmt.Fprintf(out, "\n`acct_params_get` Fields:\n\n")
-	fieldSpecsMarkdown(out, logic.AcctParamsFieldNames, logic.AcctParamsFieldSpecByName)
+	fieldGroupMarkdown(out, logic.AcctParamsFields)
 }
 
 func ecDsaCurvesMarkdown(out io.Writer) {
 	fmt.Fprintf(out, "\n`ECDSA` Curves:\n\n")
-	fieldSpecsMarkdown(out, logic.EcdsaCurveNames, logic.EcdsaCurveSpecByName)
+	fieldGroupMarkdown(out, logic.EcdsaCurves)
 }
 
 func immediateMarkdown(op *logic.OpSpec) string {
@@ -214,19 +214,19 @@ func opToMarkdown(out io.Writer, op *logic.OpSpec) (err error) {
 		fmt.Fprintf(out, "- **Cost**:\n")
 		for _, cost := range costs {
 			if cost.From == cost.To {
-				fmt.Fprintf(out, "    - %d (v%d)\n", cost.Cost, cost.To)
+				fmt.Fprintf(out, "    - %s (v%d)\n", cost.Cost, cost.To)
 			} else {
 				if cost.To < logic.LogicVersion {
-					fmt.Fprintf(out, "    - %d (v%d - v%d)\n", cost.Cost, cost.From, cost.To)
+					fmt.Fprintf(out, "    - %s (v%d - v%d)\n", cost.Cost, cost.From, cost.To)
 				} else {
-					fmt.Fprintf(out, "    - %d (since v%d)\n", cost.Cost, cost.From)
+					fmt.Fprintf(out, "    - %s (since v%d)\n", cost.Cost, cost.From)
 				}
 			}
 		}
 	} else {
 		cost := costs[0].Cost
-		if cost != 1 {
-			fmt.Fprintf(out, "- **Cost**: %d\n", cost)
+		if cost != "1" {
+			fmt.Fprintf(out, "- **Cost**: %s\n", cost)
 		}
 	}
 	if op.Version > 1 {
@@ -250,10 +250,8 @@ func opToMarkdown(out io.Writer, op *logic.OpSpec) (err error) {
 		appParamsFieldsMarkdown(out)
 	case "acct_params_get":
 		acctParamsFieldsMarkdown(out)
-	default:
-		if strings.HasPrefix(op.Name, "ecdsa") {
-			ecDsaCurvesMarkdown(out)
-		}
+	case "ecdsa_verify":
+		ecDsaCurvesMarkdown(out)
 	}
 	ode := logic.OpDocExtra(op.Name)
 	if ode != "" {
@@ -280,7 +278,6 @@ type OpRecord struct {
 	Name    string
 	Args    string `json:",omitempty"`
 	Returns string `json:",omitempty"`
-	Cost    int
 	Size    int
 
 	ArgEnum      []string `json:",omitempty"`
@@ -321,29 +318,33 @@ func typeString(types []logic.StackType) string {
 	return string(out)
 }
 
-func fieldsAndTypes(names []string, specs speccer) ([]string, string) {
-	types := make([]logic.StackType, len(names))
-	for i, name := range names {
-		types[i] = specs.SpecByName(name).Type()
+func fieldsAndTypes(group logic.FieldGroup) ([]string, string) {
+	// reminder: group.Names can be "sparse" See: logic.TxnaFields
+	fields := make([]string, 0, len(group.Names))
+	types := make([]logic.StackType, 0, len(group.Names))
+	for _, name := range group.Names {
+		if name != "" {
+			fields = append(fields, name)
+			types = append(types, group.Specs.SpecByName(name).Type())
+		}
 	}
-	return names, typeString(types)
+	return fields, typeString(types)
 }
 
 func argEnums(name string) (names []string, types string) {
 	switch name {
 	case "txn", "gtxn", "gtxns", "itxn", "gitxn", "itxn_field":
-		return fieldsAndTypes(logic.TxnFieldNames, logic.TxnFieldSpecByName)
+		return fieldsAndTypes(logic.TxnFields)
 	case "global":
 		return
 	case "txna", "gtxna", "gtxnsa", "txnas", "gtxnas", "gtxnsas", "itxna", "gitxna":
-		// Map is the whole txn field spec map.  That's fine, we only lookup the given names.
-		return fieldsAndTypes(logic.TxnaFieldNames(), logic.TxnFieldSpecByName)
+		return fieldsAndTypes(logic.TxnaFields)
 	case "asset_holding_get":
-		return fieldsAndTypes(logic.AssetHoldingFieldNames, logic.AssetHoldingFieldSpecByName)
+		return fieldsAndTypes(logic.AssetHoldingFields)
 	case "asset_params_get":
-		return fieldsAndTypes(logic.AssetParamsFieldNames, logic.AssetParamsFieldSpecByName)
+		return fieldsAndTypes(logic.AssetParamsFields)
 	case "app_params_get":
-		return fieldsAndTypes(logic.AppParamsFieldNames, logic.AppParamsFieldSpecByName)
+		return fieldsAndTypes(logic.AppParamsFields)
 	default:
 		return nil, ""
 	}
@@ -357,7 +358,6 @@ func buildLanguageSpec(opGroups map[string][]string) *LanguageSpec {
 		records[i].Name = spec.Name
 		records[i].Args = typeString(spec.Args)
 		records[i].Returns = typeString(spec.Returns)
-		records[i].Cost = spec.Details.Cost
 		records[i].Size = spec.Details.Size
 		records[i].ArgEnum, records[i].ArgEnumTypes = argEnums(spec.Name)
 		records[i].Doc = logic.OpDoc(spec.Name)
@@ -400,29 +400,18 @@ func main() {
 	integerConstantsTableMarkdown(constants)
 	constants.Close()
 
-	txnfields := create("txn_fields.md")
-	fieldSpecsMarkdown(txnfields, logic.TxnFieldNames, logic.TxnFieldSpecByName)
-	txnfields.Close()
-
-	globalfields := create("global_fields.md")
-	fieldSpecsMarkdown(globalfields, logic.GlobalFieldNames, logic.GlobalFieldSpecByName)
-	globalfields.Close()
-
-	assetholding := create("asset_holding_fields.md")
-	fieldSpecsMarkdown(assetholding, logic.AssetHoldingFieldNames, logic.AssetHoldingFieldSpecByName)
-	assetholding.Close()
-
-	assetparams := create("asset_params_fields.md")
-	fieldSpecsMarkdown(assetparams, logic.AssetParamsFieldNames, logic.AssetParamsFieldSpecByName)
-	assetparams.Close()
-
-	appparams := create("app_params_fields.md")
-	fieldSpecsMarkdown(appparams, logic.AppParamsFieldNames, logic.AppParamsFieldSpecByName)
-	appparams.Close()
-
-	acctparams, _ := os.Create("acct_params_fields.md")
-	fieldSpecsMarkdown(acctparams, logic.AcctParamsFieldNames, logic.AcctParamsFieldSpecByName)
-	acctparams.Close()
+	written := make(map[string]bool)
+	opSpecs := logic.OpcodesByVersion(logic.LogicVersion)
+	for _, spec := range opSpecs {
+		for _, imm := range spec.Details.Immediates {
+			if imm.Group != nil && !written[imm.Group.Name] {
+				out := create(imm.Group.Name + "_fields.md")
+				fieldSpecsMarkdown(out, imm.Group.Names, imm.Group.Specs)
+				out.Close()
+				written[imm.Group.Name] = true
+			}
+		}
+	}
 
 	langspecjs := create("langspec.json")
 	enc := json.NewEncoder(langspecjs)

cmd/opdoc/tmLanguage.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/algorand/go-algorand/data/transactions/logic"
@@ -122,11 +123,18 @@ func buildSyntaxHighlight() *tmLanguage {
 		},
 	}
 	var allNamedFields []string
-	allNamedFields = append(allNamedFields, logic.TxnFieldNames...)
-	allNamedFields = append(allNamedFields, logic.GlobalFieldNames...)
-	allNamedFields = append(allNamedFields, logic.AssetHoldingFieldNames...)
-	allNamedFields = append(allNamedFields, logic.AssetParamsFieldNames...)
-	allNamedFields = append(allNamedFields, logic.OnCompletionNames...)
+	allNamedFields = append(allNamedFields, logic.TxnTypeNames[:]...)
+	allNamedFields = append(allNamedFields, logic.OnCompletionNames[:]...)
+	accumulated := make(map[string]bool)
+	opSpecs := logic.OpcodesByVersion(logic.LogicVersion)
+	for _, spec := range opSpecs {
+		for _, imm := range spec.Details.Immediates {
+			if imm.Group != nil && !accumulated[imm.Group.Name] {
+				allNamedFields = append(allNamedFields, imm.Group.Names[:]...)
+				accumulated[imm.Group.Name] = true
+			}
+		}
+	}
 
 	literals.Patterns = append(literals.Patterns, pattern{
 		Name:  "variable.parameter.teal",
@@ -153,7 +161,15 @@ func buildSyntaxHighlight() *tmLanguage {
 		},
 	}
 	var allArithmetics []string
-	for grp, names := range logic.OpGroups {
+
+	var keys []string
+	for key := range logic.OpGroups {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	for _, grp := range keys {
+		names := logic.OpGroups[grp]
+		sort.Strings(names)
 		switch grp {
 		case "Flow Control":
 			keywords.Patterns = append(keywords.Patterns, pattern{

data/transactions/logic/.gitignore

@@ -436,7 +436,7 @@ Some of these have immediate data in the byte or bytes after the opcode.
 | 16 | TypeEnum | uint64 |      | See table below |
 | 17 | XferAsset | uint64 |      | Asset ID |
 | 18 | AssetAmount | uint64 |      | value in Asset's units |
-| 19 | AssetSender | []byte |      | 32 byte address. Causes clawback of all value of asset from AssetSender if Sender is the Clawback address of the asset. |
+| 19 | AssetSender | []byte |      | 32 byte address. Moves asset from AssetSender if Sender is the Clawback address of the asset. |
 | 20 | AssetReceiver | []byte |      | 32 byte address |
 | 21 | AssetCloseTo | []byte |      | 32 byte address |
 | 22 | GroupIndex | uint64 |      | Position of this transaction within an atomic transaction group. A stand-alone transaction is implicitly element 0 in a group of 1 |
@@ -457,7 +457,7 @@ Some of these have immediate data in the byte or bytes after the opcode.
 | 37 | ConfigAssetUnitName | []byte | v2  | Unit name of the asset |
 | 38 | ConfigAssetName | []byte | v2  | The asset name |
 | 39 | ConfigAssetURL | []byte | v2  | URL |
-| 40 | ConfigAssetMetadataHash | []byte | v2  | 32 byte commitment to some unspecified asset metadata |
+| 40 | ConfigAssetMetadataHash | []byte | v2  | 32 byte commitment to unspecified asset metadata |
 | 41 | ConfigAssetManager | []byte | v2  | 32 byte address |
 | 42 | ConfigAssetReserve | []byte | v2  | 32 byte address |
 | 43 | ConfigAssetFreeze | []byte | v2  | 32 byte address |
@@ -527,7 +527,7 @@ Asset fields include `AssetHolding` and `AssetParam` fields that are used in the
 | 4 | AssetName | []byte |      | Asset name |
 | 5 | AssetURL | []byte |      | URL with additional info about the asset |
 | 6 | AssetMetadataHash | []byte |      | Arbitrary commitment |
-| 7 | AssetManager | []byte |      | Manager commitment |
+| 7 | AssetManager | []byte |      | Manager address |
 | 8 | AssetReserve | []byte |      | Reserve address |
 | 9 | AssetFreeze | []byte |      | Freeze address |
 | 10 | AssetClawback | []byte |      | Clawback address |

data/transactions/logic/README.md

@@ -50,15 +50,15 @@ The 32 byte public key is the last element on the stack, preceded by the 64 byte
 - Opcode: 0x05 {uint8 curve index}
 - Stack: ..., A: []byte, B: []byte, C: []byte, D: []byte, E: []byte → ..., uint64
 - for (data A, signature B, C and pubkey D, E) verify the signature of the data against the pubkey => {0 or 1}
-- **Cost**: 1700
+- **Cost**:  Secp256k1=1700 Secp256r1=2500
 - Availability: v5
 
 `ECDSA` Curves:
 
 | Index | Name | In | Notes |
 | - | ------ | - | --------- |
-| 0 | Secp256k1 |      | secp256k1 curve |
-| 1 | Secp256r1 | v7  | secp256r1 curve |
+| 0 | Secp256k1 |      | secp256k1 curve, used in Bitcoin |
+| 1 | Secp256r1 | v7  | secp256r1 curve, NIST standard |
 
 
 The 32 byte Y-component of a public key is the last element on the stack, preceded by X-component of a pubkey, preceded by S and R components of a signature, preceded by the data that is fifth element on the stack. All values are big-endian encoded. The signed data must be 32 bytes long, and signatures in lower-S form are only accepted.
@@ -68,17 +68,9 @@ The 32 byte Y-component of a public key is the last element on the stack, preced
 - Opcode: 0x06 {uint8 curve index}
 - Stack: ..., A: []byte → ..., X: []byte, Y: []byte
 - decompress pubkey A into components X, Y
-- **Cost**: 650
+- **Cost**:  Secp256k1=650 Secp256r1=2400
 - Availability: v5
 
-`ECDSA` Curves:
-
-| Index | Name | In | Notes |
-| - | ------ | - | --------- |
-| 0 | Secp256k1 |      | secp256k1 curve |
-| 1 | Secp256r1 | v7  | secp256r1 curve |
-
-
 The 33 byte public key in a compressed form to be decompressed into X and Y (top) components. All values are big-endian encoded.
 
 ## ecdsa_pk_recover v
@@ -89,14 +81,6 @@ The 33 byte public key in a compressed form to be decompressed into X and Y (top
 - **Cost**: 2000
 - Availability: v5
 
-`ECDSA` Curves:
-
-| Index | Name | In | Notes |
-| - | ------ | - | --------- |
-| 0 | Secp256k1 |      | secp256k1 curve |
-| 1 | Secp256r1 | v7  | secp256r1 curve |
-
-
 S (top) and R elements of a signature, recovery id and data (bottom) are expected on the stack and used to deriver a public key. All values are big-endian encoded. The signed data must be 32 bytes long.
 
 ## +
@@ -396,7 +380,7 @@ The notation J,K indicates that two uint64 values J and K are interpreted as a u
 | 16 | TypeEnum | uint64 |      | See table below |
 | 17 | XferAsset | uint64 |      | Asset ID |
 | 18 | AssetAmount | uint64 |      | value in Asset's units |
-| 19 | AssetSender | []byte |      | 32 byte address. Causes clawback of all value of asset from AssetSender if Sender is the Clawback address of the asset. |
+| 19 | AssetSender | []byte |      | 32 byte address. Moves asset from AssetSender if Sender is the Clawback address of the asset. |
 | 20 | AssetReceiver | []byte |      | 32 byte address |
 | 21 | AssetCloseTo | []byte |      | 32 byte address |
 | 22 | GroupIndex | uint64 |      | Position of this transaction within an atomic transaction group. A stand-alone transaction is implicitly element 0 in a group of 1 |
@@ -417,7 +401,7 @@ The notation J,K indicates that two uint64 values J and K are interpreted as a u
 | 37 | ConfigAssetUnitName | []byte | v2  | Unit name of the asset |
 | 38 | ConfigAssetName | []byte | v2  | The asset name |
 | 39 | ConfigAssetURL | []byte | v2  | URL |
-| 40 | ConfigAssetMetadataHash | []byte | v2  | 32 byte commitment to some unspecified asset metadata |
+| 40 | ConfigAssetMetadataHash | []byte | v2  | 32 byte commitment to unspecified asset metadata |
 | 41 | ConfigAssetManager | []byte | v2  | 32 byte address |
 | 42 | ConfigAssetReserve | []byte | v2  | 32 byte address |
 | 43 | ConfigAssetFreeze | []byte | v2  | 32 byte address |
@@ -933,7 +917,7 @@ params: Txn.Accounts offset (or, since v4, an _available_ address), asset id (or
 | 4 | AssetName | []byte |      | Asset name |
 | 5 | AssetURL | []byte |      | URL with additional info about the asset |
 | 6 | AssetMetadataHash | []byte |      | Arbitrary commitment |
-| 7 | AssetManager | []byte |      | Manager commitment |
+| 7 | AssetManager | []byte |      | Manager address |
 | 8 | AssetReserve | []byte |      | Reserve address |
 | 9 | AssetFreeze | []byte |      | Freeze address |
 | 10 | AssetClawback | []byte |      | Clawback address |