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

Support fixed arrays and tuples #35

Closed
wants to merge 3 commits into from
Closed

Support fixed arrays and tuples #35

wants to merge 3 commits into from

Conversation

PeterMPhillips
Copy link

Hi all. This PR adds fixes for #34 and weiroll/weiroll#56. It checks whether the array/tuple is fixed size or dynamic. If it's fixed, the parameter's value is broken up into 32 byte chunks and set as individual values in the state array. I've added some tests to confirm the functionality.

As far as I can tell this only works for literal values, if I attempt to pass a return value into an array or tuple I get this error: Error: invalid BigNumber value... since we're passing a ReturnValue object into the contract's function instead of a BigNumber. I suspect this requires further changes to the Contract type provided by planner.ts. Although I'm unsure what exactly is needed to fix this. Should I make this a separate issue or would you like it resolve in this PR?

@decanus decanus requested a review from Arachnid March 13, 2022 17:59
Copy link
Contributor

@Arachnid Arachnid 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 this! I would like to resolve the issue with only accepting literals in arrays and tuples in this PR if we can. I think the easiest way to do this would be to introduce ArrayValue and TupleValue types, and when encoding them, handle them by iterating over their elements and checking what sort of value they are.

@PeterMPhillips PeterMPhillips closed this by deleting the head repository Sep 17, 2022
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.

2 participants