Add EIP-5163: Rich Site-Proposed Contract Metadata #5163
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s): (fail) eip-5163.md
|
MicahZoltu
reviewed
Jun 16, 2022
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
MicahZoltu
reviewed
Jun 22, 2022
MicahZoltu
reviewed
Jun 22, 2022
Pandapip1
suggested changes
Jul 15, 2022
github-actions
bot
added
c-new
Pandapip1
changed the title
|
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
xinbenlv
reviewed
Sep 24, 2022
| - Returns ABIs | ||
| - Has no parameter names, leaving most of the transaction illegible. | ||
| - Do not exist on all chains | ||
| - Trusting the contract for its own metadata (EIP-719, EIP-4430) |
There was a problem hiding this comment.
Suggested change
| - Trusting the contract for its own metadata (EIP-719, EIP-4430) | |
| - Trusting the contract for its own metadata ([EIP-719](./eip-719.md), [EIP-4430](./eip-4430.md)) |
xinbenlv
reviewed
Sep 24, 2022
| - Is largely centralized today | ||
| - Only works for verified contracts on etherscan (although designed for eventual Sourcify support). | ||
|
|
||
| Additionally, this method serves to help populate the wallet with trustworthy information about what it holds. It reduces the need to rely on any central services for indexing (enhancing performance and privacy), and eliminates dangers that come from index-based automatic asset detection, like receiving harrassment, or Airdrop scams. |
There was a problem hiding this comment.
Suggested change
| Additionally, this method serves to help populate the wallet with trustworthy information about what it holds. It reduces the need to rely on any central services for indexing (enhancing performance and privacy), and eliminates dangers that come from index-based automatic asset detection, like receiving harrassment, or Airdrop scams. | |
| Additionally, this method serves to help populate the wallet with trustworthy information about what it holds. It reduces the need to rely on any central services for indexing (enhancing performance and privacy), and eliminates dangers that come from index-based automatic asset detection, like receiving harassment, or Airdrop scams. |
xinbenlv
reviewed
Sep 24, 2022
|
|
||
| Achieving that initial safety would require a widespread push for adoption, but since it's just about adding a new optional parameter and it keeps a site's users more resistant to phishing, I think this effort could be effective. | ||
|
|
||
| Some users would not be protected by this: Users who acquired their assets previously, and do not have the relevant metadata for the assets they are holding. Those users would be at no greater risk than they are at today. One of these users being phished to a malicious site could be either presented with a malicious transaction or malicious metadata, but for a scammer the current behavior (transfer the asset) is simply fewer steps to success, so it seems unlikely this would be a desireable attack for phishers. |
There was a problem hiding this comment.
Suggested change
| Some users would not be protected by this: Users who acquired their assets previously, and do not have the relevant metadata for the assets they are holding. Those users would be at no greater risk than they are at today. One of these users being phished to a malicious site could be either presented with a malicious transaction or malicious metadata, but for a scammer the current behavior (transfer the asset) is simply fewer steps to success, so it seems unlikely this would be a desireable attack for phishers. | |
| Some users would not be protected by this: Users who acquired their assets previously, and do not have the relevant metadata for the assets they are holding. Those users would be at no greater risk than they are at today. One of these users being phished to a malicious site could be either presented with a malicious transaction or malicious metadata, but for a scammer the current behavior (transfer the asset) is simply fewer steps to success, so it seems unlikely this would be a desirable attack for phishers. |
xinbenlv
reviewed
Sep 24, 2022
| proposedContracts: { | ||
| [contractId: CrossChainAddress]: { | ||
| proposedName: string, | ||
| abi?: Abi, |
There was a problem hiding this comment.
is abi and contractData going to be optional?
|
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
|
This pull request was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Discussions on ethereum-magicians