Skip to content

ERC-820 Pseudo-introspection using a registry contract.#906

Merged
Arachnid merged 9 commits intoethereum:masterfrom
jbaylina:eip820
Apr 6, 2018
Merged

ERC-820 Pseudo-introspection using a registry contract.#906
Arachnid merged 9 commits intoethereum:masterfrom
jbaylina:eip820

Conversation

@jbaylina
Copy link
Contributor

@jbaylina jbaylina commented Feb 27, 2018

This is the PR discussed in #820
Currently, there is no open issues/conflicts in this standard proposal.

@fulldecent
Copy link
Contributor

fulldecent commented Feb 27, 2018

Notes:

I can contribute a PR if you consider these issues valid.

@jbaylina
Copy link
Contributor Author

jbaylina commented Feb 28, 2018

@fulldecent , just updated the EIP-820. With all the typos, links, and a better explanation of Nick's deployment method.

Regarding the other 2 issues, I have 2 questions:
1.- ERC820_ACCEPT_MAGIC you propose to use one that contains the word NFT (Non fungible token) for the calculation. I don't really see the point and the logic for what you are proposing. In any case, this is just a nonsense random number that warranties that a function is implemented specifically for that intention.
2.- 0xFFFFFFFF interface it will always return invalid, because it is checked here: https://github.com/jbaylina/eip820/blob/master/contracts/ERC820Registry.sol#L115 So I don't think it's any need to add any extra check.
The remaining comments are fixed!.

@fulldecent
Copy link
Contributor

fulldecent commented Feb 28, 2018

For magic, perhaps you can use:

bytes4(keccak256("canImplementInterfaceForAddress(address,bytes32)"))

It is an arbitrary choice. But someone reviewing our ERC-721 said that we should reuse ERC-165 style naming because we already have a way to turn a function into a bytes4.


Second item 2 Looks good to me!

@fulldecent
Copy link
Contributor

Last item: EIP-X format is to use email addresses in the byline at the top.

@jbaylina
Copy link
Contributor Author

jbaylina commented Mar 1, 2018

@fulldecent In the current implementation, the Magic number is a full word (bytes32) and not a bytes4. The porpoise of this magic number has nothing to do with EIP165. It makes no sense for me to follow that pattern. Making it so similar may bring to confusion.

@fulldecent
Copy link
Contributor

No worries. Thanks for the discussion.

EIPS/eip-820.md Outdated

### The smart contract

```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Solidity syntax highlighting here by using

```solidity

@Arachnid
Copy link
Contributor

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
Copy link
Contributor

Arachnid commented Apr 6, 2018

Please advise if you'd like this merged as a draft in its current state.

@Arachnid Arachnid merged commit ea5b7b3 into ethereum:master Apr 6, 2018
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request May 2, 2018
* ERC-820 Pseudo-introspection using a registry contract.

* Typos, links, and better explanation of Nicks deployment method

* Formating fix

* Email address

* solidity syntax highlight

* Adapt to new template for EIPs

* Discussions link added

* Fix link in discussion-to

* Type fixed
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

Successfully merging this pull request may close these issues.

4 participants