Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ABI support to goal #3088

Merged
merged 106 commits into from
Nov 9, 2021
Merged

Add ABI support to goal #3088

merged 106 commits into from
Nov 9, 2021

Conversation

ahangsu
Copy link
Contributor

@ahangsu ahangsu commented Oct 18, 2021

Summary

  • Implement JSON unmarshal to golang values on ABI encoding
  • Implement a method sub-command to accept method signature and application args
  • Add a way to view the return value of an app call.

Addressing #2803

Test Plan

  • Test unmarshal JSON values from command line to ABI encoding

cmd/goal/application.go Outdated Show resolved Hide resolved
cmd/goal/application.go Outdated Show resolved Hide resolved
cmd/goal/application.go Outdated Show resolved Hide resolved
cmd/goal/application.go Outdated Show resolved Hide resolved
cmd/goal/application.go Outdated Show resolved Hide resolved
jasonpaulos
jasonpaulos previously approved these changes Nov 9, 2021
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Looking good, just a nit

test/scripts/e2e_subs/e2e-app-abi-arg.sh Outdated Show resolved Hide resolved
test/scripts/e2e_subs/e2e-app-abi-add.sh Outdated Show resolved Hide resolved
jasonpaulos
jasonpaulos previously approved these changes Nov 9, 2021
cmd/goal/application.go Outdated Show resolved Hide resolved
data/abi/abi_json.go Outdated Show resolved Hide resolved
data/abi/abi_json_test.go Show resolved Hide resolved
cmd/goal/application.go Outdated Show resolved Hide resolved
var methodAppCmd = &cobra.Command{
Use: "method",
Short: "Invoke a method",
Long: `Invoke a method in an App (stateful contract) with an application call transaction`,
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 not proposing any changes here, but view this as a learning opportunity for me about Algorand's conventions. Cleary the convention in the file is for this field to use raw string literals. I guess that makes sense, because in theory the Long description could span several lines. However, none of the descriptions in this file seem to utilize any of the string literal's special features. Any thoughts? You can also totally ignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have specific thoughts or idea about it, just following the previous conventions. I just noticed the nearest editing for adding method/updating description is 16 months ago... and I suppose they might want to do something with string literal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use ABI encoding support in goal
7 participants