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

EIP-712: Update assets to show array encoding #1242

Closed
wants to merge 2 commits into from

Conversation

bitpshr
Copy link
Contributor

@bitpshr bitpshr commented Jul 21, 2018

This pull request updates the example assets for EIP-712 to include support for array encoding per the specification:

The array values are encoded as the keccak256 hash of the concatenated encodeData of their contents (i.e. the encoding of SomeType[5] is identical to that of a struct containing five members of type SomeType).

cc @recmo

@bitpshr bitpshr changed the title EIP-712: Update example assets to show Array type support EIP-712: Update assets to show Array type support Jul 21, 2018
@bitpshr bitpshr changed the title EIP-712: Update assets to show Array type support EIP-712: Update assets to show array encoding Jul 21, 2018
@bitpshr
Copy link
Contributor Author

bitpshr commented Jul 31, 2018

Hi @recmo. Did you have a chance to check if this array encoding implementation matches your intention with the proposal? Thanks in advance.

@mckingho
Copy link

mckingho commented Aug 15, 2018

Good to see an implemention on encoding array data. Thank @bitpshr.
I think this implementaion does not support array of primitive type. I am trying to add a new field in Person object. The field is an array of primitive data type, uint256[]. But it encounters error - TypeError: Cannot read property 'map' of undefined.
The function encodeData is an implementation of encoding 'struct' and the script fails when a primitive type is passed as primaryType in function encodeType.

@bitpshr bitpshr closed this Nov 5, 2018
@bitpshr bitpshr deleted the eip-712-arrays branch November 5, 2018 16:38
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.

3 participants