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

Draft EIP for ENS support for contract ABIs #205

Merged
merged 6 commits into from Sep 13, 2018

Conversation

Projects
None yet
4 participants
@Arachnid
Collaborator

Arachnid commented Feb 6, 2017

No description provided.

| 2 | zlib-compressed JSON |
| 4 | CBOR |
| 8 | URI |

This comment has been minimized.

@axic

axic Feb 6, 2017

Member

I would add two new identifiers:

  • Swarm hash of the Solidity metadata hash
  • IPFS hash of the Solidity metadata hash

Or perhaps only URI of Solidity metadata hash, which can be either bzzr://.. or ipfs://.. at the moment.

Reason being that the metadata is (currently) a JSON object containing some details about the compilation process and the resulting ABI and Natspec objects.

For the metadata see http://solidity.readthedocs.io/en/develop/miscellaneous.html#contract-metadata.

This comment has been minimized.

@Arachnid

Arachnid Feb 6, 2017

Collaborator

Can't all of those just be URIs, though?

Edit: Which is to say, could we just add a 'metadata URI' option, assuming we don't ever want to store the actual metadata on chain?

This comment has been minimized.

@axic

axic Feb 6, 2017

Member

The URI in this spec points to an ABI object, while the metadata URI is not an ABI object, but contains the ABI too.

Potentially it could be stored if needed, but isn't Swarm a better way to store data? 😉

This comment has been minimized.

@Arachnid

Arachnid Feb 6, 2017

Collaborator

Right, so I'm suggesting adding one additional type, for a 'metadata URI', so callers can distinguish between getting back a URI to just the ABI, or to the whole metadata. There's no need to encode what kind of URI it is (swarm, ipfs, etc) in the resolver.

Alternately, we could define a completely separate 'metadata URI' resolver profile.

@Arachnid Arachnid force-pushed the Arachnid:ens-abi branch from 74e2c2e to b3e5913 Feb 7, 2017

@Arachnid Arachnid added the ERC label Apr 14, 2017

@Arachnid Arachnid referenced this pull request May 3, 2017

Closed

ERC: Ethereum Name Service #137

@frozeman

This comment has been minimized.

Member

frozeman commented Jul 12, 2017

It shouldn't be named ABI, its the JSON interface of the contract, which compiles the functions to its ABI.
i would call it contractData, so it can hold more values, like solc uploads

@MicahZoltu

This comment has been minimized.

Contributor

MicahZoltu commented Sep 12, 2017

Why is this being proposed as part of ENS? It seems more useful to have the ABI lookup occur on the contract address, and since ENS will resolve to a contract address it means we can easily go from current ENS to ABI lookup. In particular, if the ABI is tied to the ENS name then if the ENS name is changed to point at a different contract (e.g., as part of a contract upgrade) then the ABI potentially will no longer match.

Also, is there any way that the ABI can be verified? What prevents some random hacker from uploading an ABI for a particular contract address that is malicious (tricks users)?

@Arachnid

This comment has been minimized.

Collaborator

Arachnid commented Sep 13, 2017

Why is this being proposed as part of ENS? It seems more useful to have the ABI lookup occur on the contract address, and since ENS will resolve to a contract address it means we can easily go from current ENS to ABI lookup.

Doing so would require contracts to return their own ABIs. That has a couple of disadvantages over this scheme:

  • It makes deduplication of data impossible for contracts with identical ABIs
  • It requires that the contract add its ABI at deploy time

In particular, if the ABI is tied to the ENS name then if the ENS name is changed to point at a different contract (e.g., as part of a contract upgrade) then the ABI potentially will no longer match.

Like with all metadata, it's the name owner's responsibility to make sure it's up-to-date.

Also, is there any way that the ABI can be verified? What prevents some random hacker from uploading an ABI for a particular contract address that is malicious (tricks users)?

Only the owner of the reverse record can set ABI records for the contract address. Nothing prevents J Random Hacker setting up an ENS name with ABI record pointing at a contract, but by the time they've persuaded you to use their ENS name to interact with the contract without verifying anything, it's already too late to ameliorate.

@MicahZoltu

This comment has been minimized.

Contributor

MicahZoltu commented Sep 13, 2017

It makes deduplication of data impossible for contracts with identical ABIs

The contract could return a hash of the ABI rather than the full ABI, and the hash could then be used as a lookup key. Perhaps even better would be if the contract returned an array as a mechanism of asserting that it implements some interface (one of the hashes) and also some full ABI. However, your other point about deploy time is very valid.

Like with all metadata, it's the name owner's responsibility to make sure it's up-to-date.

I feel like the name owner should need to explicitly indicate which contract address an ABI is for. If the name redirects, the ABI should become invalid until such time as the name owner also redirects the ABI. From a management perspective, these two operations could be done at the same time, but I think defaulting to assume the ABI hasn't changed when the name redirects is introducing unnecessary risk.

@Arachnid

This comment has been minimized.

Collaborator

Arachnid commented Mar 27, 2018

This is a courtesy notice to let you know that the format for EIPs has been modified slightly. If you want your draft merged, you will need to make some small changes to how your EIP is formatted:

  • Frontmatter is now contained between lines with only a triple dash ('---')
  • Headers in the frontmatter are now lowercase.

If your PR is editing an existing EIP rather than creating a new one, this has already been done for you, and you need only rebase your PR.

In addition, a continuous build has been setup, which will check your PR against the rules for EIP formatting automatically once you update your PR. This build ensures all required headers are present, as well as performing a number of other checks.

Please rebase your PR against the latest master, and edit your PR to use the above format for frontmatter. For convenience, here's a sample header you can copy and adapt:

---
eip: <num>
title: <title>
author: <author>
type: [Standards Track|Informational|Meta]
category: [Core|Networking|Interface|ERC] (for type: Standards Track only)
status: Draft
created: <date>
---

Arachnid added some commits Sep 13, 2018

@Arachnid Arachnid merged commit 8ca9017 into ethereum:master Sep 13, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment