Support reference types in goal app method (#3275)

* Fix method signature parse bug

* Support reference types

* Review dog fixes

* Fix comments

Author: jasonpaulos

cmd/goal/application.go

@@ -1074,7 +1074,7 @@ func populateMethodCallTxnArgs(types []string, values []string) ([]transactions.
 		}
 
 		expectedType := types[i]
-		if expectedType != "txn" && txn.Txn.Type != protocol.TxType(expectedType) {
+		if expectedType != abi.AnyTransactionType && txn.Txn.Type != protocol.TxType(expectedType) {
 			return nil, fmt.Errorf("Transaction from %s does not match method argument type. Expected %s, got %s", txFilename, expectedType, txn.Txn.Type)
 		}
 
@@ -1084,6 +1084,82 @@ func populateMethodCallTxnArgs(types []string, values []string) ([]transactions.
 	return loadedTxns, nil
 }
 
+// populateMethodCallReferenceArgs parses reference argument types and resolves them to an index
+// into the appropriate foreign array. Their placement will be as compact as possible, which means
+// values will be deduplicated and any value that is the sender or the current app will not be added
+// to the foreign array.
+func populateMethodCallReferenceArgs(sender string, currentApp uint64, types []string, values []string, accounts *[]string, apps *[]uint64, assets *[]uint64) ([]int, error) {
+	resolvedIndexes := make([]int, len(types))
+
+	for i, value := range values {
+		var resolved int
+
+		switch types[i] {
+		case abi.AccountReferenceType:
+			if value == sender {
+				resolved = 0
+			} else {
+				duplicate := false
+				for j, account := range *accounts {
+					if value == account {
+						resolved = j + 1 // + 1 because 0 is the sender
+						duplicate = true
+						break
+					}
+				}
+				if !duplicate {
+					resolved = len(*accounts) + 1
+					*accounts = append(*accounts, value)
+				}
+			}
+		case abi.ApplicationReferenceType:
+			appID, err := strconv.ParseUint(value, 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Unable to parse application ID '%s': %s", value, err)
+			}
+			if appID == currentApp {
+				resolved = 0
+			} else {
+				duplicate := false
+				for j, app := range *apps {
+					if appID == app {
+						resolved = j + 1 // + 1 because 0 is the current app
+						duplicate = true
+						break
+					}
+				}
+				if !duplicate {
+					resolved = len(*apps) + 1
+					*apps = append(*apps, appID)
+				}
+			}
+		case abi.AssetReferenceType:
+			assetID, err := strconv.ParseUint(value, 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("Unable to parse asset ID '%s': %s", value, err)
+			}
+			duplicate := false
+			for j, asset := range *assets {
+				if assetID == asset {
+					resolved = j
+					duplicate = true
+					break
+				}
+			}
+			if !duplicate {
+				resolved = len(*assets)
+				*assets = append(*assets, assetID)
+			}
+		default:
+			return nil, fmt.Errorf("Unknown reference type: %s", types[i])
+		}
+
+		resolvedIndexes[i] = resolved
+	}
+
+	return resolvedIndexes, nil
+}
+
 var methodAppCmd = &cobra.Command{
 	Use:   "method",
 	Short: "Invoke a method",
@@ -1138,17 +1214,37 @@ var methodAppCmd = &cobra.Command{
 		var txnArgValues []string
 		var basicArgTypes []string
 		var basicArgValues []string
+		var refArgTypes []string
+		var refArgValues []string
+		refArgIndexToBasicArgIndex := make(map[int]int)
 		for i, argType := range argTypes {
 			argValue := methodArgs[i]
 			if abi.IsTransactionType(argType) {
 				txnArgTypes = append(txnArgTypes, argType)
 				txnArgValues = append(txnArgValues, argValue)
 			} else {
+				if abi.IsReferenceType(argType) {
+					refArgIndexToBasicArgIndex[len(refArgTypes)] = len(basicArgTypes)
+					refArgTypes = append(refArgTypes, argType)
+					refArgValues = append(refArgValues, argValue)
+					// treat the reference as a uint8 for encoding purposes
+					argType = "uint8"
+				}
 				basicArgTypes = append(basicArgTypes, argType)
 				basicArgValues = append(basicArgValues, argValue)
 			}
 		}
 
+		refArgsResolved, err := populateMethodCallReferenceArgs(account, appIdx, refArgTypes, refArgValues, &appAccounts, &foreignApps, &foreignAssets)
+		if err != nil {
+			reportErrorf("error populating reference arguments: %v", err)
+		}
+		for i, resolved := range refArgsResolved {
+			basicArgIndex := refArgIndexToBasicArgIndex[i]
+			// use the foreign array index as the encoded argument value
+			basicArgValues[basicArgIndex] = strconv.Itoa(resolved)
+		}
+
 		err = abi.ParseArgJSONtoByteSlice(basicArgTypes, basicArgValues, &applicationArgs)
 		if err != nil {
 			reportErrorf("cannot parse arguments to ABI encoding: %v", err)

data/abi/abi_encode.go

@@ -534,10 +534,9 @@ func ParseMethodSignature(methodSig string) (name string, argTypes []string, ret
 	argsEnd := -1
 	depth := 0
 	for index, char := range methodSig {
-		switch char {
-		case '(':
+		if char == '(' {
 			depth++
-		case ')':
+		} else if char == ')' {
 			if depth == 0 {
 				err = fmt.Errorf("Unpaired parenthesis in method signature: %s", methodSig)
 				return

data/abi/abi_encode_test.go

@@ -1001,3 +1001,55 @@ func TestRandomABIEncodeDecodeRoundTrip(t *testing.T) {
 	addTupleRandomValues(t, Tuple, &testValuePool)
 	categorySelfRoundTripTest(t, testValuePool[Tuple])
 }
+
+func TestParseMethodSignature(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	tests := []struct {
+		signature  string
+		name       string
+		argTypes   []string
+		returnType string
+	}{
+		{
+			signature:  "add(uint8,uint16,pay,account,txn)uint32",
+			name:       "add",
+			argTypes:   []string{"uint8", "uint16", "pay", "account", "txn"},
+			returnType: "uint32",
+		},
+		{
+			signature:  "nothing()void",
+			name:       "nothing",
+			argTypes:   []string{},
+			returnType: "void",
+		},
+		{
+			signature:  "tupleArgs((uint8,uint128),account,(string,(bool,bool)))bool",
+			name:       "tupleArgs",
+			argTypes:   []string{"(uint8,uint128)", "account", "(string,(bool,bool))"},
+			returnType: "bool",
+		},
+		{
+			signature:  "tupleReturn(uint64)(bool,bool,bool)",
+			name:       "tupleReturn",
+			argTypes:   []string{"uint64"},
+			returnType: "(bool,bool,bool)",
+		},
+		{
+			signature:  "tupleArgsAndReturn((uint8,uint128),account,(string,(bool,bool)))(bool,bool,bool)",
+			name:       "tupleArgsAndReturn",
+			argTypes:   []string{"(uint8,uint128)", "account", "(string,(bool,bool))"},
+			returnType: "(bool,bool,bool)",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.signature, func(t *testing.T) {
+			name, argTypes, returnType, err := ParseMethodSignature(test.signature)
+			require.NoError(t, err)
+			require.Equal(t, test.name, name)
+			require.Equal(t, test.argTypes, argTypes)
+			require.Equal(t, test.returnType, returnType)
+		})
+	}
+}

data/abi/abi_type.go

@@ -458,11 +458,34 @@ func (t Type) ByteLen() (int, error) {
 	}
 }
 
+// AnyTransactionType is the ABI argument type string for a nonspecific transaction argument
+const AnyTransactionType = "txn"
+
 // IsTransactionType checks if a type string represents a transaction type
 // argument, such as "txn", "pay", "keyreg", etc.
 func IsTransactionType(s string) bool {
 	switch s {
-	case "txn", "pay", "keyreg", "acfg", "axfer", "afrz", "appl":
+	case AnyTransactionType, "pay", "keyreg", "acfg", "axfer", "afrz", "appl":
+		return true
+	default:
+		return false
+	}
+}
+
+// AccountReferenceType is the ABI argument type string for account references
+const AccountReferenceType = "account"
+
+// AssetReferenceType is the ABI argument type string for asset references
+const AssetReferenceType = "asset"
+
+// ApplicationReferenceType is the ABI argument type string for application references
+const ApplicationReferenceType = "application"
+
+// IsReferenceType checks if a type string represents a reference type argument,
+// such as "account", "asset", or "application".
+func IsReferenceType(s string) bool {
+	switch s {
+	case AccountReferenceType, AssetReferenceType, ApplicationReferenceType:
 		return true
 	default:
 		return false

test/scripts/e2e_subs/e2e-app-abi-method.sh

@@ -62,6 +62,14 @@ if [[ $RES != *"${EXPECTED}"* ]]; then
     false
 fi
 
+# Foreign reference test
+RES=$(${gcmd} app method --method "referenceTest(account,application,account,asset,account,asset,asset,application,application)uint8[9]" --arg KGTOR3F3Q74JP4LB5M3SOCSJ4BOPOKZ2GPSLMLLGCWYWRXZJNN4LYQJXXU --arg $APPID --arg $ACCOUNT --arg 10 --arg KGTOR3F3Q74JP4LB5M3SOCSJ4BOPOKZ2GPSLMLLGCWYWRXZJNN4LYQJXXU --arg 11 --arg 10 --arg 20 --arg 21 --app-account 2R5LMPTYLVMWYEG4RPI26PJAM7ARTGUB7LZSONQPGLUWTPOP6LQCJTQZVE --foreign-app 21 --foreign-asset 10 --app-id $APPID --from $ACCOUNT 2>&1 || true)
+EXPECTED="method referenceTest(account,application,account,asset,account,asset,asset,application,application)uint8[9] succeeded with output: [2,0,2,0,2,1,0,1,0]"
+if [[ $RES != *"${EXPECTED}"* ]]; then
+    date '+app-abi-method-test FAIL the method call to referenceTest(account,application,account,asset,account,asset,asset,application,application)uint8[9] should not fail %Y%m%d_%H%M%S'
+    false
+fi
+
 # Close out
 RES=$(${gcmd} app method --method "closeOut()string" --on-completion closeout --app-id $APPID --from $ACCOUNT 2>&1 || true)
 EXPECTED="method closeOut()string succeeded with output: \"goodbye Algorand Fan\""