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.

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

Update EIP-223: Remove standard function #7856

Closed
wants to merge 1 commit into from
Closed

Conversation

Pandapip1
Copy link
Member

@Pandapip1 Pandapip1 commented Oct 16, 2023

ERC-223 needs an errata:

  1. In Add EIP-223: Token standard with event handling implementation #6485 (comment), I noted that the standard function was out-of-scope and insisted that it be removed
  2. In Add EIP-223: Token standard with event handling implementation #6485 (comment), @Dexaran acknowledged that it was indeed out-of-scope and removed it
  3. In Update EIP-223: Update eip-223.md #7466 / ff3ec32, the function was mistakenly re-introduced
  4. The optional function remains out-of-scope and inadequately specified.

Removing this function is a backward compatible change, as applications cannot expect it to be present.

@Pandapip1 Pandapip1 requested a review from eth-bot as a code owner October 16, 2023 17:12
@github-actions github-actions bot added c-update Modifies an existing proposal s-final This EIP is Final t-erc labels Oct 16, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Oct 16, 2023

File EIPS/eip-223.md

Requires 1 more reviewers from @Dexaran
Requires 2 more reviewers from @axic, @gcolvin, @lightclient, @SamWilsn

@eth-bot eth-bot changed the title Update ERC-223: Remove standard function Update EIP-223: Remove standard function Oct 16, 2023
@eth-bot eth-bot added the e-consensus Waiting on editor consensus label Oct 16, 2023
@Dexaran
Copy link
Contributor

Dexaran commented Oct 16, 2023

I would support this changes but the removal of this function now would require the update of the reference implementation code as well since the function is also presented there.

I'm also working on a Token Standard Converter as according to EIP-7417 (btw that would help if you could merge the EIP-7417 PR, I'm waiting for it for 3 weeks without any response)

The standard() function is also part of the token converter (https://dexaran.github.io/token-converter/) now https://github.com/Dexaran/TokenStandardConverter/blob/main/TokenConverter.sol#L135 as it's the best way to identify if a token is supposed to work as a ERC-20 token or as a ERC-223 token. EIP-165 does not provide a deterministic answer since ERC-20 and ERC-223 token interfaces can match even though the logic of the transferring is different.

Even though the "standard recognition" can be reinstated as a separate informational EIP here is the question - is it worth the effort to remove it from a final EIP and reference code just to make it a separate EIP?

@Pandapip1
Copy link
Member Author

I would support this changes but the removal of this function now would require the update of the reference implementation code as well since the function is also presented there.

Actually, it's perfectly fine for you to keep the standard function there! If you'd like it removed, however, I'd be happy to comply.

I'm also working on a Token Standard Converter as according to #7418 (btw that would help if you could merge the EIP-7417 PR, I'm waiting for it for 3 weeks without any response)

Sorry about the wait. I'll look into it!

The standard() function is also part of the token converter (https://dexaran.github.io/token-converter/) now https://github.com/Dexaran/TokenStandardConverter/blob/main/TokenConverter.sol#L135 as it's the best way to identify if a token is supposed to work as a ERC-20 token or as a ERC-223 token. ERC-165 does not provide a deterministic answer since ERC-20 and ERC-223 token interfaces can match even though the logic of the transferring is different.

The interface ID is the XOR of all the function selectors, so they should be different. https://eips.ethereum.org/EIPS/eip-5269 and https://eips.ethereum.org/EIPS/eip-1046 might also interest you, although the latter applies less directly to your token converter EIP.

Even though the "standard recognition" can be reinstated as a separate informational EIP here is the question - is it worth the effort to remove it from a final EIP and reference code just to make it a separate EIP?

Considering most if not all of the work would be done by editors, I'd say yes, this is worth it. Take a look at ERC-165 and/or ERC-5269 and let me know what you think! ERC-5269 is still in Review, so if you have any strong feelings, @xinbenlv will probably be able to collaborate with you to refine them!

@SamWilsn
Copy link
Contributor

I am closing this pull request because we are in the process of separating EIPs and ERCs into distinct repositories. Unfortunately, as far as we are aware, GitHub does not provide any tools to ease this migration, so every pull request will need to be re-opened manually.

As this is a PR to create / modify an ERC, I will kindly ask you to redirect this to the new repository at ethereum/ERCs. We have prepared a guide to help with the process.

If there is relevant history here, please link to this PR from the new pull request.

On behalf of the EIP Editors, I apologize for this inconvenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal e-consensus Waiting on editor consensus s-final This EIP is Final t-erc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants