initial commit for eip-6384 #6384

DeVaz1 · 2023-01-26T18:53:28Z

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md

We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:

The PR edits only existing draft PRs.
The build passes.
Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside .
If matching on email address, the email address is the one publicly listed on your GitHub profile.

eth-bot · 2023-01-26T18:55:22Z

All tests passed; auto-merging...

(pass) eip-6384.md

classification
`updateEIP`

passed!

(pass) assets/eip-6384/implementation/src/IEvalEIP712Buffer.sol

classification
`ambiguous`

file assets/eip-6384/implementation/src/IEvalEIP712Buffer.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/src/MyToken/MyToken.sol

classification
`ambiguous`

file assets/eip-6384/implementation/src/MyToken/MyToken.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/src/MyToken/MyToken712ParserHelper.sol

classification
`ambiguous`

file assets/eip-6384/implementation/src/MyToken/MyToken712ParserHelper.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/src/MyToken/MyTokenStructs.sol

classification
`ambiguous`

file assets/eip-6384/implementation/src/MyToken/MyTokenStructs.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/src/SeaPort/SeaPort712ParserHelper.sol

classification
`ambiguous`

file assets/eip-6384/implementation/src/SeaPort/SeaPort712ParserHelper.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/src/SeaPort/SeaPortMock.sol

classification
`ambiguous`

file assets/eip-6384/implementation/src/SeaPort/SeaPortMock.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/src/SeaPort/SeaPortStructs.sol

classification
`ambiguous`

file assets/eip-6384/implementation/src/SeaPort/SeaPortStructs.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/test/MyToken.t.sol

classification
`ambiguous`

file assets/eip-6384/implementation/test/MyToken.t.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/test/OrderGenerator.sol

classification
`ambiguous`

file assets/eip-6384/implementation/test/OrderGenerator.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/test/SeaPort712Parser.t.sol

classification
`ambiguous`

file assets/eip-6384/implementation/test/SeaPort712Parser.t.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/implementation/test/SigUtils.sol

classification
`ambiguous`

file assets/eip-6384/implementation/test/SigUtils.sol is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/media/MiceyMask-non-compliant.png

classification
`ambiguous`

file assets/eip-6384/media/MiceyMask-non-compliant.png is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

(pass) assets/eip-6384/media/ZenGo-EIP-compliant.png

classification
`ambiguous`

file assets/eip-6384/media/ZenGo-EIP-compliant.png is associated with EIP 6384; because there are also changes being made to EIPS/eip-6384.md all changes to corresponding assets are also allowed

SamWilsn

I haven't reviewed this at all, but I can tell from the number of changed files that something has been included here that shouldn't be.

Your reference implementation should contain only the minimum code required to implement your EIP, and shouldn't contain any build system files (eg. hardhat or foundry) or libraries.

EIPS/eip-readable-signatures.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

github-actions · 2023-01-30T15:35:57Z

The commit 80f5e6f (as a parent of 0c3bf82) contains errors.
Please inspect the Run Summary for details.

EIPS/eip-6384.md

SamWilsn · 2023-01-30T16:38:40Z

EIPS/eip-6384.md

+Please note that if the explanation was part of the message to sign it would have been under the control of the attacker and hence irrelevant for security purposes.
+
+Since the added functionality to the contract has the “view” modifier, it cannot change the on-chain state and harm the existing functionalities of the contract.
+


I think you should still mention the attack where an attacker creates both a fake website and a fake contract, and the fake contract gives a misleading description. If the real contract's domain separator doesn't contain verifyingContract than the fake contract can give whatever fraudulent description it wants.

Then you can say the attack is prevented by requiring that wallets only attempt to get descriptions when the domain separator contains a verifying contract.

if it's a fake contract, then it has no access to the users assets, therefore not a relevant attack scenario, hence not in scope

A signed message without verifyingContract in the domain separator would be valid for both the fake contract, and more importantly, for the real contract, right?

if there is no verifyingContract we already mentioned explicitly "In case the EIP-712 message is not bounded to a contract, the wallet cannot call this function as the contract address is unknown and SHOULD default to its current display."

Sorry the updated wording is "If verifyingContract is not included in the EIP-712 domain separator, wallets MUST NOT retrieve a human-readable description using this EIP. In this case, wallets SHOULD fallback to their original EIP-712 display."

Additionally, the signature of the rogue contract will not be valid for. the good contract, as the malicious contract buffer contains a verifyingContract in as the domain separator that would fail the signature validation for the good contract as it does not take this parameter into consideration when doing ecrecover.

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

SamWilsn

Looks good, few small things I missed previously.

Would you also mind removing the unused media files? The repository is big enough as it is 🤣

EIPS/eip-6384.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

EIPS/eip-6384.md

* initial commit for eip-readable-signatures * Update EIPS/eip-readable-signatures.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * removed build files * updated links * updated links * eip.md updated according to linter suggestions * updated EIP according to linter * updated the eip according to the linter * fixed linter errors at the md file * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * updated signature paramater name to typedDataBuffer * updated signature paramater name to typedDataBuffer * added explanation to the rationale section * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * Update EIPS/eip-6384.md Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com> * removed unnecessary media * removed unnecessary media * Update EIPS/eip-6384.md --------- Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

initial commit for eip-readable-signatures

5cc2de7

DeVaz1 requested a review from eth-bot as a code owner January 26, 2023 18:53

github-actions bot added c-new Creates a brand new proposal e-number Waiting on EIP Number assignment s-draft This EIP is a Draft t-interface labels Jan 26, 2023

github-actions bot added the w-ci Waiting on CI to pass label Jan 26, 2023

SamWilsn requested changes Jan 28, 2023

View reviewed changes

EIPS/eip-readable-signatures.md Outdated Show resolved Hide resolved

DeVaz1 and others added 4 commits January 29, 2023 11:11

Update EIPS/eip-readable-signatures.md

6e31edb

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

removed build files

463baa1

updated links

16b3619

updated links

1940bd6

SamWilsn closed this Jan 30, 2023

SamWilsn reopened this Jan 30, 2023

github-actions bot removed the e-number Waiting on EIP Number assignment label Jan 30, 2023

eip.md updated according to linter suggestions

80f5e6f

DeVaz1 added 3 commits January 30, 2023 17:38

updated EIP according to linter

364e8bf

updated the eip according to the linter

330496e

fixed linter errors at the md file

06107a5

github-actions bot removed the w-ci Waiting on CI to pass label Jan 30, 2023

SamWilsn reviewed Jan 30, 2023

View reviewed changes

DeVaz1 and others added 7 commits January 30, 2023 18:51

Update EIPS/eip-6384.md

65af800

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

f16add8

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

f6c4e4e

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

36bc310

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

c270b4d

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

5d0be3b

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

2505d1a

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

DeVaz1 and others added 9 commits January 30, 2023 19:06

Update EIPS/eip-6384.md

b5e5308

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

b743a00

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

01fb6d4

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

90323f0

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

61564b6

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

09adacc

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

updated signature paramater name to typedDataBuffer

d64285e

updated signature paramater name to typedDataBuffer

fd10314

added explanation to the rationale section

b1f711f

DeVaz1 changed the title ~~initial commit for eip-readable-signatures~~ initial commit for eip-6384 Jan 30, 2023

SamWilsn reviewed Jan 30, 2023

View reviewed changes

EIPS/eip-6384.md Outdated Show resolved Hide resolved

EIPS/eip-6384.md Outdated Show resolved Hide resolved

EIPS/eip-6384.md Outdated Show resolved Hide resolved

EIPS/eip-6384.md Outdated Show resolved Hide resolved

DeVaz1 and others added 4 commits January 31, 2023 00:08

Update EIPS/eip-6384.md

2d8483b

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

c244d65

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

564a561

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

Update EIPS/eip-6384.md

6af274c

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

DeVaz1 closed this Jan 30, 2023

DeVaz1 reopened this Jan 30, 2023

DeVaz1 added 2 commits January 31, 2023 01:14

removed unnecessary media

d160c99

removed unnecessary media

8b91a7c

SamWilsn reviewed Jan 31, 2023

View reviewed changes

EIPS/eip-6384.md Outdated Show resolved Hide resolved

Update EIPS/eip-6384.md

6aede6f

github-actions bot added t-erc and removed t-interface labels Jan 31, 2023

SamWilsn approved these changes Jan 31, 2023

View reviewed changes

eth-bot enabled auto-merge (squash) January 31, 2023 00:27

eth-bot approved these changes Jan 31, 2023

View reviewed changes

eth-bot merged commit ee5cf6d into ethereum:master Jan 31, 2023

sync-by-unito bot mentioned this pull request May 1, 2023

Create ERC for digital media indexing standard numbersprotocol/numbers-network#24

Closed

sync-by-unito bot mentioned this pull request May 21, 2023

Initiate discussion numbersprotocol/numbers-network#29

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial commit for eip-6384 #6384

initial commit for eip-6384 #6384

DeVaz1 commented Jan 26, 2023

eth-bot commented Jan 26, 2023 •

edited

Loading

SamWilsn left a comment

github-actions bot commented Jan 30, 2023

SamWilsn Jan 30, 2023

DeVaz1 Jan 30, 2023

SamWilsn Jan 30, 2023

talbeerysec Jan 30, 2023

talbeerysec Jan 30, 2023

talbeerysec Jan 30, 2023

SamWilsn left a comment

		Please note that if the explanation was part of the message to sign it would have been under the control of the attacker and hence irrelevant for security purposes.

		Since the added functionality to the contract has the “view” modifier, it cannot change the on-chain state and harm the existing functionalities of the contract.

initial commit for eip-6384 #6384

initial commit for eip-6384 #6384

Conversation

DeVaz1 commented Jan 26, 2023

eth-bot commented Jan 26, 2023 • edited Loading

All tests passed; auto-merging...

(pass) eip-6384.md

(pass) assets/eip-6384/implementation/src/IEvalEIP712Buffer.sol

(pass) assets/eip-6384/implementation/src/MyToken/MyToken.sol

(pass) assets/eip-6384/implementation/src/MyToken/MyToken712ParserHelper.sol

(pass) assets/eip-6384/implementation/src/MyToken/MyTokenStructs.sol

(pass) assets/eip-6384/implementation/src/SeaPort/SeaPort712ParserHelper.sol

(pass) assets/eip-6384/implementation/src/SeaPort/SeaPortMock.sol

(pass) assets/eip-6384/implementation/src/SeaPort/SeaPortStructs.sol

(pass) assets/eip-6384/implementation/test/MyToken.t.sol

(pass) assets/eip-6384/implementation/test/OrderGenerator.sol

(pass) assets/eip-6384/implementation/test/SeaPort712Parser.t.sol

(pass) assets/eip-6384/implementation/test/SigUtils.sol

(pass) assets/eip-6384/media/MiceyMask-non-compliant.png

(pass) assets/eip-6384/media/ZenGo-EIP-compliant.png

SamWilsn left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 30, 2023

SamWilsn Jan 30, 2023

Choose a reason for hiding this comment

DeVaz1 Jan 30, 2023

Choose a reason for hiding this comment

SamWilsn Jan 30, 2023

Choose a reason for hiding this comment

talbeerysec Jan 30, 2023

Choose a reason for hiding this comment

talbeerysec Jan 30, 2023

Choose a reason for hiding this comment

talbeerysec Jan 30, 2023

Choose a reason for hiding this comment

SamWilsn left a comment

Choose a reason for hiding this comment

eth-bot commented Jan 26, 2023 •

edited

Loading