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

Enhancement: Add foreign array objects to ATC addMethodCall #725

Merged
merged 2 commits into from
Jan 10, 2023

Conversation

algochoi
Copy link
Contributor

@algochoi algochoi commented Dec 28, 2022

This PR adds foreign array objects as arguments in addMethodCall() for the atomic transaction composer.

Adds a test that mirrors the Go SDK: algorand/go-algorand-sdk#318. Currently written as a unit test, but could also benefit from a cucumber test since all the other SDKs implement this.

Addresses #653

src/composer.ts Outdated Show resolved Hide resolved
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.

Thanks for working on this, it's a much needed improvement. I found one small issue but this looks good to merge after that's addressed.

I agree that we need a new cucumber test to ensure all SDKs have appropriate coverage here, but I don't think that's a blocker for this PR.

src/composer.ts Outdated Show resolved Hide resolved
@algochoi algochoi marked this pull request as ready for review January 5, 2023 18:43
@algochoi algochoi merged commit e411146 into develop Jan 10, 2023
@algochoi algochoi deleted the foreign-refs-atc branch January 10, 2023 21:44
algochoi added a commit that referenced this pull request Jan 18, 2023
* ABI:  Refactor ABI encoding test to round-trip (#701)

* bump version and add to changelog

* update README.md for new version

* Packaging: Improve source map and browser usage for external bundlers (#707)

* bump version and add to changelog

* update README.md for new version

* v2: Make breaking changes from v1 to v2.0.0  (#717)

* bump version and add to changelog

* update README.md for new version

* remove enhancement section of recent changelog

* Enhancement: Add foreign array objects to ATC `addMethodCall` (#725)

* Add foreign array objects to ATC addmethodcall

* Copy array value so that inputs are not modified

Co-authored-by: Michael Diamant <[email protected]>
Co-authored-by: Lucky Baar <[email protected]>
Co-authored-by: Jason Paulos <[email protected]>
Co-authored-by: Jack Smith <[email protected]>
Co-authored-by: Barbara Poon <[email protected]>
algochoi added a commit that referenced this pull request Jan 18, 2023
* Add test for algod /v2/teal/disassemble

* Merge develop into PR (#736)

* ABI:  Refactor ABI encoding test to round-trip (#701)

* bump version and add to changelog

* update README.md for new version

* Packaging: Improve source map and browser usage for external bundlers (#707)

* bump version and add to changelog

* update README.md for new version

* v2: Make breaking changes from v1 to v2.0.0  (#717)

* bump version and add to changelog

* update README.md for new version

* remove enhancement section of recent changelog

* Enhancement: Add foreign array objects to ATC `addMethodCall` (#725)

* Add foreign array objects to ATC addmethodcall

* Copy array value so that inputs are not modified

Co-authored-by: Michael Diamant <[email protected]>
Co-authored-by: Lucky Baar <[email protected]>
Co-authored-by: Jason Paulos <[email protected]>
Co-authored-by: Jack Smith <[email protected]>
Co-authored-by: Barbara Poon <[email protected]>

* Explicitly import Buffer

* Revert test branch back to master

Co-authored-by: algochoi <[email protected]>
Co-authored-by: Lucky Baar <[email protected]>
Co-authored-by: Jason Paulos <[email protected]>
Co-authored-by: Jack Smith <[email protected]>
Co-authored-by: Barbara Poon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants