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

fix: support for List[EIP712Type] #56

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

flipdazed
Copy link

What I did

This fix allows the eip712 library to handle lists of EIP712Type instances correctly. Specifically, it supports nested structures where fields can be lists of custom types, enabling the generation of signable messages and hashes for more complex data structures.

How I did it

  1. Modified the _types_ Property:

    • Updated the _types_ property in the EIP712Type class to recursively build type definitions for fields that are lists of custom types.
    • Handled the case where list elements are instances of EIP712Type, ensuring their types are correctly added to the type definitions.
  2. Updated the _body_ Property:

    • Modified the _body_ property in the EIP712Message class to correctly serialize lists of EIP712Type instances.
    • Ensured that nested lists are correctly included in the message serialization.
  3. Enhanced Data Preparation for Hashing:

    • Improved the _prepare_data_for_hashing function to handle lists of dictionaries, ensuring nested structures are correctly flattened for hashing.

How to verify it

  1. Define Custom Types:

    • Create custom EIP712Message types, including a nested structure where one field is a list of another custom type.
    from eip712.messages import EIP712Message, EIP712Type
    from typing import List
    
    class NestedType(EIP712Message):
        field1: "string"  # type: ignore
        field2: "uint256"  # type: ignore
    
        def __post_init__(self):
            self._name_ = "NestedType"
            self._version_ = "1"
    
    class MainType(EIP712Message):
        name: "string"  # type: ignore
        age: "uint256"  # type: ignore
        nested: List[NestedType]
    
        def __post_init__(self):
            self._name_ = "MainType"
            self._version_ = "1"
    
    # Create instances of the nested type
    nested1 = NestedType(field1="nested1", field2=100)
    nested2 = NestedType(field1="nested2", field2=200)
    
    # Create an instance of the main type with a list of nested types
    main_instance = MainType(name="Alice", age=30, nested=[nested1, nested2])
    
    # Generate the signable message
    signable_message = main_instance.signable_message
    
    # Calculate the hash of the signable message
    message_hash = calculate_hash(signable_message)
    
    print("Signable Message:", signable_message)
    print("Message Hash:", message_hash.hex())
  2. Run the Example:

    • Run the provided example code.
    • Verify that the signable message is generated correctly, and the message hash is computed without errors.
  3. Check Outputs:

    • Ensure the printed signable message includes the nested types.
    • Confirm the message hash matches the expected format.

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

@flipdazed flipdazed changed the title Support for Nested Lists in EIP712Type Handling Fix: Support for Nested Lists in EIP712Type Handling Jul 13, 2024
@flipdazed flipdazed changed the title Fix: Support for Nested Lists in EIP712Type Handling fix: support for List[EIP712Type] Jul 13, 2024
Copy link
Member

@antazoey antazoey left a comment

Choose a reason for hiding this comment

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

Looks good! I can approve after List -> list

@@ -2,7 +2,7 @@
Message classes for typed structured data hashing and signing in Ethereum.
"""

from typing import Any, Optional
from typing import Any, Optional, List, get_origin, get_args
Copy link
Member

Choose a reason for hiding this comment

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

We can use lower-case list now (no import needed).

field_type = self.__annotations__[field]

if get_origin(field_type) is list:
elem_type = get_args(field_type)[0]
Copy link
Member

Choose a reason for hiding this comment

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

could this also be a list, like list[list[EIP712Type]]?

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