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.

Sign up for GitHub

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

Jump to bottom

EIP-712 for java should work well without domain's "chainId" to support 0xProtocol #1063

Closed
avaness opened this issue Oct 3, 2019 · 2 comments
Closed

EIP-712 for java should work well without domain's "chainId" to support 0xProtocol #1063

avaness opened this issue Oct 3, 2019 · 2 comments
Labels
enhancement a feature request

Comments

@avaness
Copy link
Contributor

avaness commented Oct 3, 2019

Feature description_

0xProtocol domain schema doesn't contain chainId at all for now. 0xProject/0x-protocol-specification#11

Current web3j implementation throws exception on missed chainId.

here's 0xProtocol control sample

{
  "types": {
    "EIP712Domain": [
      {"name": "name", "type": "string"},
      {"name": "version", "type": "string"},
      {"name": "verifyingContract", "type": "address"}
    ],
    "Order": [
      { "name": "makerAddress", "type": "address" },
      { "name": "takerAddress", "type": "address" },
      { "name": "feeRecipientAddress", "type": "address" },
      { "name": "senderAddress", "type": "address" },
      { "name": "makerAssetAmount", "type": "uint256" },
      { "name": "takerAssetAmount", "type": "uint256" },
      { "name": "makerFee", "type": "uint256" },
      { "name": "takerFee", "type": "uint256" },
      { "name": "expirationTimeSeconds", "type": "uint256" },
      { "name": "salt", "type": "uint256" },
      { "name": "makerAssetData", "type": "bytes" },
      { "name": "makerAssetData", "type": "bytes" }
  	]
  },
  "primaryType": "Order",
  "domain": {
    "name": "0x Protocol",
    "version": "2",
    "verifyingContract": "0x12459c951127e0c374ff9105dda097662a027093"
  },
  "message": {
    "makerAddress": "0x9e56625509c2f60af937f23b7b532600390e8c8b",
    "takerAddress": "0xa2b31dacf30a9c50ca473337c01d8a201ae33e32",
    "feeRecipientAddress": "0xb046140686d052fff581f63f8136cce132e857da",
    "senderAddress": "0xa2b31dacf30a9c50ca473337c01d8a201ae33e32",
    "makerAssetAmount": 10000000000000000,
    "takerAssetAmount": 20000000000000000,
    "makerFee": 100000000000000,
    "takerFee": 200000000000000,
    "expirationTimeSeconds": 1532560590,
    "salt": 1532559225,
    "makerAssetData": "0xf47261b0000000000000000000000000e41d2489571d322189246dafa5ebde1f4699f498",
    "takerAssetData": "0x02571792000000000000000000000000371b13d97f4bf77d724e78c16b7dc74099f40e840000000000000000000000000000000000000000000000000000000000000063"
   }
}

with following expected control hash

0xccb29124860915763e8cd9257da1260abc7df668fde282272587d84b594f37f6

We can add support of missed chainId to EIP712 implementation
@avaness avaness added the enhancement a feature request label Oct 3, 2019
@avaness avaness changed the title EIP-712 for java should work well without domain's "chainId", for 0xProtocol for example EIP-712 for java should work well without domain's "chainId" for 0xProtocol Oct 3, 2019
@avaness avaness mentioned this issue Oct 3, 2019
Merged
avaness added a commit to avaness/web3j that referenced this issue Oct 3, 2019
@avaness
fixing hyperledger-web3j#1063 - no more explicit needs of chainId
5bfe6af
@avaness avaness changed the title EIP-712 for java should work well without domain's "chainId" for 0xProtocol EIP-712 for java should work well without domain's "chainId" for 0xProtocol support Oct 3, 2019
@avaness avaness changed the title EIP-712 for java should work well without domain's "chainId" for 0xProtocol support EIP-712 for java should work well without domain's "chainId" to support 0xProtocol Oct 3, 2019
antonydenyer pushed a commit to avaness/web3j that referenced this issue Oct 14, 2019
@avaness @antonydenyer
fixing hyperledger-web3j#1063 - no more explicit needs of chainId
136e6f6
iikirilov pushed a commit that referenced this issue Oct 28, 2019
@avaness @iikirilov
bytes, bytes1..32 fix; chainId is not restrictly required anymore (is…
73ab9ab 
…sues #1054, #1063) (#1055)

* fix (#1054)

* formatting fix

* bugfix for "bytes" type, value should be sha3(Numeric.hexStringToByteArray((String)value)) instead of Numeric.hexStringToByteArray((String) value)

* formatting fix

* fixing #1063 - no more explicit needs of chainId

* restarting travis ci

* restarting travis ci

* restarting travis ci

* formatting fix
@avaness
Copy link
Contributor Author

avaness commented Oct 28, 2019

@iikirilov do we close this issue according to #1055? it's also covered by the pull request

@iikirilov
Copy link
Contributor

@avaness yes - thank you for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement a feature request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@avaness @iikirilov