Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 97 additions & 1 deletion cmd/goal/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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)
}
}
Comment on lines +1098 to +1114
Copy link
Contributor Author

@jasonpaulos jasonpaulos Dec 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately there's a bit of code duplication between these 3 branches. Without generics I can't think of a good way to compress this

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",
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions data/abi/abi_encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == ')' {
Comment on lines +537 to +539
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm perfectly happy with this change, but FYI, if you really like switch version, you can use labels to break out of the outer for loop.
https://www.ardanlabs.com/blog/2013/11/label-breaks-in-go.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's good to know, but I'll stick with the if statement for now

if depth == 0 {
err = fmt.Errorf("Unpaired parenthesis in method signature: %s", methodSig)
return
Expand Down
52 changes: 52 additions & 0 deletions data/abi/abi_encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
25 changes: 24 additions & 1 deletion data/abi/abi_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/scripts/e2e_subs/e2e-app-abi-method.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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\""
Expand Down
Loading