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

Cannot encode bytes32 in structured data #90

Closed
jstoxrocky opened this issue Feb 5, 2020 · 12 comments
Closed

Cannot encode bytes32 in structured data #90

jstoxrocky opened this issue Feb 5, 2020 · 12 comments

Comments

@jstoxrocky
Copy link
Contributor

jstoxrocky commented Feb 5, 2020

What was wrong?

I cannot encode bytes32 in structured data. I am attempting to pass a struct containing a variable I want encoded as bytes32 to eth_account.messages.encode_structured_data. I am passing this struct to encode_structured_data as a Python dictionary. Including my bytes32 variable as a bytestring causes a JSON serialization error (eth_account/messages.py:143). Passing this variable as a hex string throws a TypeError (eth_account/_utils/structured_data/hashing.py:250).

Code that produced the error

from eth_account.messages import (
    encode_structured_data,
)
from eth_utils import (
    keccak,
)

message = {
	"types": {
		"EIP712Domain": [
			{"name": "name", "type": "string"},
			{"name": "version", "type": "string"},
			{"name": "chainId", "type": "uint256"},
			{"name": "verifyingContract", "type": "address"}
		],
		"coolStruct": [
			{"name": "coolBytes", "type": "bytes32"}
		]
	},
	"primaryType": "coolStruct",
	"domain": {
		"name": "coolContract",
		"version": "1",
		"chainId": 1,
		"verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC"
	},
	"message": {
		"coolBytes": keccak(b''), # Alternativley keccak(b'').hex()
	}
}

def test_hashed_structured_data():
    structured_msg = encode_structured_data(message)

Full error output

eth_account/messages.py:143: in encode_structured_data
    message_string = json.dumps(primitive)
TypeError: Object of type bytes is not JSON serializable
TypeError: Value of `coolBytes` (c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470) in the struct `coolStruct` is of the type `<class 'str'>`, but expected bytes32 value

Expected Result

It can definitely be done and should equal something like this:

    domain_type_hash = keccak(text='EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)')
    name = message['domain']['name']
    version = message['domain']['version']
    chain_id = message['domain']['chainId']
    verifying_contract = message['domain']['verifyingContract']
    domain_instance = encode_abi(
      ['bytes32', 'bytes32', 'bytes32', 'uint256', 'address'],
      [domain_type_hash, keccak(text=name), keccak(text=version), chain_id, verifying_contract],
    )
    domain_hash = keccak(domain_instance)

    cool_struct_type_hash = keccak(text='coolStruct(bytes32 coolBytes)')
    cool_bytes = message['message']['coolBytes']
    cool_struct_instance = encode_abi(
      ['bytes32', 'bytes32'],
      [cool_struct_type_hash, cool_bytes]
    )
    cool_struct_hash = keccak(cool_struct_instance)

    preamble = b'\x19'
    version = b'\x01'

    message_hash = keccak(preamble + version + domain_hash + cool_struct_hash)
    print(message_hash.hex()) # 6cb68ad2b02b4a7f086020d9c23dc1d9f3a56d60e9441b26b6fdd90e2518277a

How can it be fixed?

I'm posting an issue first before attempting to fix to ask yall how best you think this can be solved? Why do we need to json.dumps? Seems like thats a blocker since bytes can't be easily serialized into JSON.

PR #91

@bingen
Copy link

bingen commented Apr 20, 2021

Same happened to me with bytes (instead of bytes32)

@carlos-nyaga
Copy link

carlos-nyaga commented Oct 8, 2021

I got the same error too when using bytes

Error

eth_account/messages.py", line 146, in encode_structured_data
    message_string = json.dumps(primitive)
TypeError: Object of type bytes is not JSON serializable

Personally I believe:

The current implementation to json.dumps is not needed, reason being:

  • BY DEFAULT, json.dumps DOES NOT SUPPORT DIRECT ENCODING OF type bytes, unless one implements a custom Encoder for json.dumps. Kindly find the default supported objects and types here => https://docs.python.org/3/library/json.html#py-to-json-table
  • Implementing a custom Encoder for json.dumps, will also require one to implement a custom Decoder for json.loads

Thus I second @jstoxrocky 's solution at #91

@pipermerriam
Copy link
Member

Try adding a 0x prefix to your hex representation.

@RomanHiden
Copy link

RomanHiden commented Oct 20, 2021

also interested in the solution

@Polsaker
Copy link

Try adding a 0x prefix to your hex representation.

That won't work either:

TypeError: Value of `nonce` (0x4d3e68deb189cfe2df55e9a798fb31a46558914bb81fcabf362ec523802f804b) in the struct `TransferWithAuthorization` is of the type `<class 'str'>`, but expected bytes32 value

@fselmo
Copy link
Contributor

fselmo commented Jan 21, 2022

I think #91 is a good change, thanks @jstoxrocky! Sorry it has taken a minute to get to this.

I think something that may be missing (from a quick peek) is that we would need validation so that a bytes32 value doesn't exceed size 32 for example. But I also don't think we are validating any type (e.g. uint8, uint256, etc...) so this is probably out of scope here.

I think I'm OK with merging #91 in as a bug fix but we should definitely add a note there that types are not yet validated. I will reply in the PR as well and I can create an issue to add the type validation so we can track it. Unfortunately I may not get to it for a bit but I appreciate the recent attention to this older issue. Seems like encoded data needs some love if anyone is looking for some contributions.

@fselmo
Copy link
Contributor

fselmo commented Jan 21, 2022

closed by #91

@fselmo fselmo closed this as completed Jan 21, 2022
@joaquim-verges
Copy link

any chance to get a new release with this fix?

@kclowes
Copy link
Collaborator

kclowes commented Feb 18, 2022

Thanks for the ping! Yep, look for one next week!

@fselmo
Copy link
Contributor

fselmo commented Feb 24, 2022

@joaquim-verges 0.6.1 is out with these changes in there 🙂

@joaquim-verges
Copy link

@fselmo @kclowes thank you! now just waiting for the corresponding web3.py update 😅

web3 5.28.0 depends on eth-account<0.6.0 and >=0.5.7

@CombatCode
Copy link

Solution for eth_account < 0.6.0:
Instead of:

        message = encode_structured_data(primitive=msg)

use

        singnable_message = SignableMessage(
            HexBytes(b'\x01'),
            hash_domain(msg),
            hash_eip712_message(msg),
        )

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

No branches or pull requests

10 participants