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

EIP-1191: Extend EIP-55: Add chain id to mixed-case checksum address encoding #1121

Closed
Tracked by #5111
juli opened this issue May 29, 2018 · 69 comments
Closed
Tracked by #5111

Comments

@juli
Copy link
Contributor

juli commented May 29, 2018

eip: 1191 
title: Add chain id to mixed-case checksum address encoding 
author: Juliano Rizzo (@juli)
status: Last Call
review-period-end: 2019-11-18
type: Standards Track
category: ERC
created: 2018-03-18
requires: 55, 155
discussions-to: https://github.com/ethereum/EIPs/issues/1121

Simple Summary

This EIP extends EIP-55 by optionally adding a chain id defined by EIP-155 to the checksum calculation.

Abstract

The EIP-55 was created to prevent users from losing funds by sending them to invalid addresses. This EIP extends EIP-55 to protect users from losing funds by sending them to addresses that are valid but that where obtained from a client of another network.For example, if this EIP is implemented, a wallet can alert the user that is trying to send funds to an Ethereum Testnet address from an Ethereum Mainnet wallet.

Motivation

The motivation of this proposal is to provide a mechanism to allow software to distinguish addresses from different Ethereum based networks. This proposal is necessary because Ethereum addresses are hashes of public keys and do not include any metadata. By extending the EIP-55 checksum algorithm it is possible to achieve this objective.

Specification

Convert the address using the same algorithm defined by EIP-55 but if a registered chain id is provided, add it to the input of the hash function. If the chain id passed to the function belongs to a network that opted for using this checksum variant, prefix the address with the chain id and the 0x separator before calculating the hash. Then convert the address to hexadecimal, but if the ith digit is a letter (ie. it's one of abcdef) print it in uppercase if the 4*ith bit of the calculated hash is 1 otherwise print it in lowercase.

Rationale

Benefits:

  • By means of a minimal code change on existing libraries, users are protected from losing funds by mixing addresses of different Ethereum based networks.

Backwards Compatibility

This proposal is fully backward compatible. The checksum calculation is changed only for new networks that choose to adopt this EIP and add their chain numbers to the Adoption Table included in this document.

Implementation

#!/usr/bin/python3
from sha3 import keccak_256
import random
"""
   addr (str): Hexadecimal address, 40 characters long with 2 characters prefix
   chainid (int): chain id from EIP-155 """
def eth_checksum_encode(addr, chainid=1):
    adopted_eip1191 = [30, 31]
    hash_input = str(chainid) + addr.lower() if chainid in adopted_eip1191 else addr[2:].lower()
    hash_output = keccak_256(hash_input.encode('utf8')).hexdigest()
    aggregate = zip(addr[2:].lower(),hash_output)
    out = addr[:2] + ''.join([c.upper() if int(a,16) >= 8 else c for c,a in aggregate])
    return out

Test Cases

eth_mainnet = [
"0x27b1fdb04752bbc536007a920d24acb045561c26",
"0x3599689E6292b81B2d85451025146515070129Bb",
"0x42712D45473476b98452f434e72461577D686318",
"0x52908400098527886E0F7030069857D2E4169EE7",
"0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed",
"0x6549f4939460DE12611948b3f82b88C3C8975323",
"0x66f9664f97F2b50F62D13eA064982f936dE76657",
"0x8617E340B3D01FA5F11F306F4090FD50E238070D",
"0x88021160C5C792225E4E5452585947470010289D",
"0xD1220A0cf47c7B9Be7A2E6BA89F429762e7b9aDb",
"0xdbF03B407c01E7cD3CBea99509d93f8DDDC8C6FB",
"0xde709f2102306220921060314715629080e2fb77",
"0xfB6916095ca1df60bB79Ce92cE3Ea74c37c5d359",
]
rsk_mainnet = [
"0x27b1FdB04752BBc536007A920D24ACB045561c26",
"0x3599689E6292B81B2D85451025146515070129Bb",
"0x42712D45473476B98452f434E72461577d686318",
"0x52908400098527886E0F7030069857D2E4169ee7",
"0x5aaEB6053f3e94c9b9a09f33669435E7ef1bEAeD",
"0x6549F4939460DE12611948B3F82B88C3C8975323",
"0x66F9664f97f2B50F62d13EA064982F936de76657",
"0x8617E340b3D01Fa5f11f306f4090fd50E238070D",
"0x88021160c5C792225E4E5452585947470010289d",
"0xD1220A0Cf47c7B9BE7a2e6ba89F429762E7B9adB",
"0xDBF03B407c01E7CD3cBea99509D93F8Dddc8C6FB",
"0xDe709F2102306220921060314715629080e2FB77",
"0xFb6916095cA1Df60bb79ce92cE3EA74c37c5d359",
]
rsk_testnet = [
"0x27B1FdB04752BbC536007a920D24acB045561C26",
"0x3599689e6292b81b2D85451025146515070129Bb",
"0x42712D45473476B98452F434E72461577D686318",
"0x52908400098527886E0F7030069857D2e4169EE7",
"0x5aAeb6053F3e94c9b9A09F33669435E7EF1BEaEd",
"0x6549f4939460dE12611948b3f82b88C3c8975323",
"0x66f9664F97F2b50f62d13eA064982F936DE76657",
"0x8617e340b3D01fa5F11f306F4090Fd50e238070d",
"0x88021160c5C792225E4E5452585947470010289d",
"0xd1220a0CF47c7B9Be7A2E6Ba89f429762E7b9adB",
"0xdbF03B407C01E7cd3cbEa99509D93f8dDDc8C6fB",
"0xDE709F2102306220921060314715629080e2Fb77",
"0xFb6916095CA1dF60bb79CE92ce3Ea74C37c5D359",
]
test_cases = {30 : rsk_mainnet, 31 : rsk_testnet, 1 : eth_mainnet}

for chainid, cases in test_cases.items():
    for addr in cases:
        assert ( addr == eth_checksum_encode(addr,chainid) )

Adoption

Adoption Table

Network Chain id Supports this EIP
RSK Mainnet 30 Yes
RSK Testnet 31 Yes

Implementation Table

Project Adopted this EIP Implementation
MyCrypto Yes JavaScript
MyEtherWallet Yes JavaScript
Ledger Yes C
Trezor Yes Python and C
Web3.js Yes JavaScript
EthereumJS-util Yes JavaScript
ENS address-encoder Yes TypeScript

Copyright

Copyright and related rights waived via CC0.

@SergioDemianLerner
Copy link
Contributor

This is an excellent idea, because it prevents using addresses of Ethereum in Ethereum Classic or RSK by mistake and vice-versa.

@juli
Copy link
Contributor Author

juli commented Jun 12, 2019

EIP-1191 already adopted by Trezor, Ledger, MEW, MyCrypto and web3

@axic
Copy link
Member

axic commented Jun 12, 2019

@juli can you please check this comment: #1191 (comment) ?

@MicahZoltu
Copy link
Contributor

The rationale says:

By means of a minimal code change on existing libraries, users are protected from losing funds by mixing addresses of different Ethereum based networks.

Is the idea here that a user will receive a contract address from some source that uses this new checksum, and then when interacting with that contract address their tooling will tell them the address is invalid if they choose a different network during signing?

I'm assuming that the intention is that EOA addresses would continue to use the EIP-55 checksum, since EOA addresses do work across chains? Presumably, the same for contracts that deploy to a standardized address across chains (like EIP 1820)? If so, I think these things should be mentioned in the EIP to prevent implementers from going overboard and using this new checksum everywhere (it is not appropriate everywhere).


Why do the test case arrays have different addresses for each network? It feels like the same set of addresses should be encoded for each network in the tests.

@axic
Copy link
Member

axic commented Nov 3, 2019

Why do the test case arrays have different addresses for each network? It feels like the same set of addresses should be encoded for each network in the tests.

This is debated on #2143.

@MicahZoltu
Copy link
Contributor

@axic I don't see that particular topic debated over there. Am I missing something?

@MicahZoltu
Copy link
Contributor

I do agree with the arguments made in #2143 that "Ethereum" doesn't adopt this EIP, instead wallets adopt this EIP. A wallet checking a checksum can check both the EIP-55 checksum and the EIP-1191 checksum and if either match green light the transaction. When generating a checksum, it would be up to the wallet whether to generate an EIP-1191 checksum or an EIP-55 checksum for the user. Getting adoption on generation is a bit harder, since it means creating addresses that are incompatible with wallets that do checksum validation but don't support EIP-1191.

@juli
Copy link
Contributor Author

juli commented Nov 3, 2019

@MicahZoltu Thanks for your comments:

  1. It should be used for EAO addresses. For example, if you send ETC or RSK funds to your Ethereum address and you use a hardware wallet, the wallet can prevent you from signing transactions cross chain (derivation path checks) and you'll be forced to load the same seed in a software wallet to recover the funds. In some cases accessing the private key won't be possible for end users or will be too risky for organizations.

  2. Networks decide and specify what address format to use, not wallets alone. Ideally, Copy&paste of addresses should work across exchanges, explorers, and wallets without requiring users to do a conversion.
    In the case of EIP-155, wallets can decide to protect users or not by adding the chain id to the signature but networks decide to implement EIP-155 in nodes and whether nodes allow signatures without chain id or not.

  3. If a wallet knows about EIP-55 and EIP-1191 then it should at least warn the user when a EIP-1191 RSK address is being used in a ETH transaction instead of green lighting it.

  4. Yes, adoption on generation before other wallets and exchanges implement EIP-1191 could be a problem and force users to convert the addresses for a while but wallets can decide to protect their own users by generating EIP-1191 addresses for networks that adopted it, we can consider it to be "secure by default" logic.

  5. The first item of each test case list is an all-uppercase valid checksum addresses for each chain id and that is why it is different for each chain id. The order in each list is different due to the upper/lower changes. RSK testnet has an extra test case. I can make changes to the test cases if you think it is important.

@axic
Copy link
Member

axic commented Nov 3, 2019

The "network" does not care, i.e. the RPC interface ignores the checksum. And the RPC interface is the closest you can get to the network, anything below that just deals with bytes and not hex strings. Checksums are only enforced by wallets.

This was the case for years, correct me if I'm wrong and the JSON RPC in sendTransaction and others enforce checksum now.

@MicahZoltu
Copy link
Contributor

@axic I'm confident that checksums are not required. It is possible the client will error on invalid checksum in a JSON-RPC request (I have never tried).

@juli Some users intentionally use the same address across multiple chains as that allows them to only have to secure a single private key. It certainly is unfortunate that hardware wallets chain lock like you have described. The reason they do it is to protect users from a hardware wallet app attack, where some malicious app gets onto the hardware wallet and leaks a private key, it is restricted to what derivation paths it can leak keys for

Given that, I would suggest that EOA addresses be optionally checksumed in this way, to cater to both usage scenarios. It would be up to the wallet which solution was appropriate for a given address.

Nitpick: I would like to see the test cases sorted the same for each network and the same set of addresses included for each network. It may also be valuable to show the same set of addresses encoded with EIP-55 checksum only, mainly for illustration purposes.

@juli
Copy link
Contributor Author

juli commented Nov 3, 2019

@axic I know the checksum is for user facing applications, I know how Ethereum nodes work. But what I tried to say is that deciding address format is something that should be part of the standards of a cryptocurrency system and not a decision of individual wallets. In the same way a chain id and BIP44/SLIP-0044 ID is decided.

@MicahZoltu Those users are doing it wrong, reusing the same private key for transactions in different networks is risky. They can use the same seed without reusing keys, that is why HD wallets were invented. This EIP makes sense if it is used for EAO and contract addresses.

Nitpick: done #2345

The addresses encoded with EIP-55 only are in the eth_mainnet list, EIP-1191 is backward compatible.

@axic axic changed the title Extend EIP-55: Add chain id to mixed-case checksum address encoding EIP-1191: Extend EIP-55: Add chain id to mixed-case checksum address encoding Nov 4, 2019
@axic
Copy link
Member

axic commented Nov 4, 2019

The rendering for the "Adoption" table seems to be broken here: https://eips.ethereum.org/EIPS/eip-1191

@juli
Copy link
Contributor Author

juli commented Nov 4, 2019

@axic Fixed and a new implementation added.

@MicahZoltu
Copy link
Contributor

Rendering of "Backward Compatibility" is also broken at that link.

@MicahZoltu
Copy link
Contributor

FWIW, I don't agree with the EIP as currently worded for the reasons I mentioned above. I think that checksumming like this should NOT be applied to all addresses, but be opt-in per address (a decision a wallet can choose to make). I also agree with other commenters that the decision to support this is a wallet decision, not a chain decision, just like EIP-55. Because of this, I don't think that the Adoption Table should be included.

That being said, I see nothing technically wrong with this EIP, thus the above concerns are not a blocker for moving this EIP to final should the authors disagree with my assessment.

@juli
Copy link
Contributor Author

juli commented Nov 6, 2019

@MicahZoltu thanks for your comments. EIP-55 nor EIP-1191 can solve the initial mistake: using hexadecimal addresses. At least one very popular wallet fixed their EIP-55 implementation while adding EIP-1191. The EIP-55 bug of that wallet had not been reported before, that probably means most users don't pay attention to the upper/lower case differences. My main motivation to work on this EIP to make it final is that it has been already implemented by wallets and libraries and having a peer reviewed EIP can contribute to adoption by other wallets and exchanges and allow users to copy&paste without doing conversions while reducing the probability of mistakes. I don't think this EIP is enough to solve the problem with hexadecimal addresses but it is at least an attempt. Using a better address format is the real solution. If you think the current version is good enough I'll move to work on something more useful than this for Ethereum users. Thank you.

@fulldecent
Copy link
Contributor

The list

It is not clear to me how enumerating specific networks in the EIP is part of the specification.

If a network is NOT listed in the EIP then can they use the checksums?

Listing networks adds a cost to maintaining this document.

Recommendation: remove this part

Mutable

The specification is mutable. It requires implementations to constantly check for an updated version of the EIP to know if a specific checksumming is valid:

... If the chain id passed to the function belongs to a network that opted for using this checksum variant, prefix the address with the chain id and the 0x separator before calculating the hash

It is not specified how to know if "a network that opted for using this checksum variant". Presumably this is indicated by including the network on the list.

The reference implementation does not load https://eips.ethereum.org/EIPS/eip-1191 to check the list and so it does not properly implement the specification.

Spec undefined behavior

The spec states:

...prefix the address with the chain id and the 0x separator before calculating the hash

It is not clear if chain id shall be lowercase, uppercase, or as provided, if it is provided as a string.

Not backwards compatible

Given the above, I believe it is not correct to say this is backwards compatible. It is fine to be not backwards compatible, but it should be clearly said that the new EIP replaces the old EIP for certain cases. But EIP-1191 will not implement and extend EIP-155

Limited utility / Will's explanation of why everyone is using CHAIN ID wrong

This item is a comment. It is meant to open discussion. This criticism is not a technical reason to reject this EIP.

I believe this EIP has limited or no utility because chain IDs are being used wrong currently and they should be changing more frequently in the future. There is a full discussion on this here https://ethereum-magicians.org/t/eip-1344-add-chain-id-opcode/1131/13

Background: nowadays it is popular when you are installing software to just give your root password to the software and let it install itself, or if you are setting up a brokerage account you give the brokerage your bank account password to login as you and do whatever it wants with your account. Similarly, when you sign a transaction on Ethereum, you permit that transaction to execute today on the current version of Ethereum or any future version of the consensus client that might be created in the future.

Details: The attack vector is: [SIGN+SUBMIT TRANSACTION] -> [ALL CLIENTS UPGRADE] -> [TRANSACTION EXECUTES]. Here the transaction you signed is not the one that executes. I consider this unacceptable. This is a design weakness and the solution is that every change to the consensus client or state hardfork must result in the chain id changing.

This affects the current EIP because if chain ids are changing frequently then the address checksumming is changing. It that would be a bad customer experience.

@juli
Copy link
Contributor Author

juli commented Nov 6, 2019

Chain ids are integers so they can't be lowercase. Everything in Ethereum, including the EVM is vaguely defined I can't fix that. I agree identifying genesis + network upgrade/hard forks makes more sense than a fixed short id but the chain id was the only id available when I looked at this problem.

About mutability: keeping a JSON document for wallets sounds like a potential solution.

Wallets and libraries already have their own static list of networks with their BIP44 values. Example: MyCrypto Ledger

@SergioDemianLerner
Copy link
Contributor

SergioDemianLerner commented Nov 7, 2019

We should always put users first. This EIP improves the user experience and protects them from costly mistakes. I think is our duty to finalize this EIP, which has been de-facto standardized by wallets and is technically sound.
In following EIPs the community can create new address formats and protect from more exotic problems regarding network forks.

@fulldecent
Copy link
Contributor

@juli Currently in the specification the integer is lowercase.

"add it to the input of the hash function."

"Add" is underspecified.

If the specification is that a JSON document is required and there is a reference document published in the EIP then the reference implementation must include an HTTP client to retrieve this JSON file.

@fulldecent
Copy link
Contributor

fulldecent commented Nov 8, 2019

@SergioDemianLerner This EIP may be technically sound after it addresses the technical issues I have raised above.

To be clear, and as noted above, my complaint about chain ID in general is not a technical complaint against this EIP. But it is relevant for future readers and that's why I put it here.

@juli
Copy link
Contributor Author

juli commented Nov 8, 2019

@fulldecent

  • The summary says adding a chain id defined by EIP-155 then chain id format is defined by EIP-155. Please contribute with text to make it more clear if you think it is necessary to specify how to create the prefix ASCII string from the chain id decimal integer and how to make it lowercase.

  • Yes, add is not defined in that sentence but it is explained in the next one and code and test vectors are provided.

  • A JSON document is not required but I mentioned it as a way to enable automatic checks, conversions, and notifications.

  • One possibility is adding "eip1191" : true` to the ethereum-list JSON documents here

  • HTTP Client: Please mention this to the EIP-155 authors so we can use the same standard HTTP client for EIP sample code. EIP-155 also defines a list of chain ids and links to a repository of JSON documents.

@fulldecent
Copy link
Contributor

fulldecent commented Nov 9, 2019

Proposed new specification

An address (string of 40 hex digits) and an EIP-155 chain ID (string of digits with no loading zeros) are required to calculate an EIP-1191 checksummed address.

First calculate a reference string. This is the chain ID with the lowercased address concatenated after, the result sha3 hashed into hexadecimal format. The EIP-1191 checksummed address shall be equal to the original address with each hexdigit A through F at location i in the address shown in uppercase iff the hexdigit i in the reference string is greater than or equal to 8. Typically the checksummed address is shown with "0x" prefixed.

Proposed new implementation

I could not get the existing implementation to run (library missing). And then assertion failures.

Attached is an update including modern import, PEP8 styling, docstring documentation, and Python 3 static typing.

The output is wrong. Or the test case is wrong. Please help.

#!/usr/bin/python3
import hashlib

def eip1191_checksum(ethereum_address: str, eip155_chainid: str) -> str:
    """
    @param address: an Ethereum address, 40 hex digits
    @param eip_155_chainid: digits with no loading zeros
    """
    assert(len(ethereum_address) == 40)
    assert(int(ethereum_address, 16) > 0)
    assert(int(eip155_chainid, 10) > 0)
    assert(str(int(eip155_chainid, 10)) == eip155_chainid)
    hash_input = eip155_chainid + ethereum_address.lower()
    reference_string = hashlib.sha3_256(hash_input.encode('utf8')).hexdigest()
    aggregate = zip(ethereum_address.lower(), reference_string)
    return "0x" + "".join([c if a < "8" else c.upper() for c, a in aggregate])

eth_mainnet = [
    "0x27b1fdb04752bbc536007a920d24acb045561c26",
    "0x3599689E6292b81B2d85451025146515070129Bb",
    "0x42712D45473476b98452f434e72461577D686318",
    "0x52908400098527886E0F7030069857D2E4169EE7",
    "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed",
    "0x6549f4939460DE12611948b3f82b88C3C8975323",
    "0x66f9664f97F2b50F62D13eA064982f936dE76657",
    "0x8617E340B3D01FA5F11F306F4090FD50E238070D",
    "0x88021160C5C792225E4E5452585947470010289D",
    "0xD1220A0cf47c7B9Be7A2E6BA89F429762e7b9aDb",
    "0xdbF03B407c01E7cD3CBea99509d93f8DDDC8C6FB",
    "0xde709f2102306220921060314715629080e2fb77",
    "0xfB6916095ca1df60bB79Ce92cE3Ea74c37c5d359",
]
rsk_mainnet = [
    "0x27b1FdB04752BBc536007A920D24ACB045561c26",
    "0x3599689E6292B81B2D85451025146515070129Bb",
    "0x42712D45473476B98452f434E72461577d686318",
    "0x52908400098527886E0F7030069857D2E4169ee7",
    "0x5aaEB6053f3e94c9b9a09f33669435E7ef1bEAeD",
    "0x6549F4939460DE12611948B3F82B88C3C8975323",
    "0x66F9664f97f2B50F62d13EA064982F936de76657",
    "0x8617E340b3D01Fa5f11f306f4090fd50E238070D",
    "0x88021160c5C792225E4E5452585947470010289d",
    "0xD1220A0Cf47c7B9BE7a2e6ba89F429762E7B9adB",
    "0xDBF03B407c01E7CD3cBea99509D93F8Dddc8C6FB",
    "0xDe709F2102306220921060314715629080e2FB77",
    "0xFb6916095cA1Df60bb79ce92cE3EA74c37c5d359",
]
rsk_testnet = [
    "0x27B1FdB04752BbC536007a920D24acB045561C26",
    "0x3599689e6292b81b2D85451025146515070129Bb",
    "0x42712D45473476B98452F434E72461577D686318",
    "0x52908400098527886E0F7030069857D2e4169EE7",
    "0x5aAeb6053F3e94c9b9A09F33669435E7EF1BEaEd",
    "0x6549f4939460dE12611948b3f82b88C3c8975323",
    "0x66f9664F97F2b50f62d13eA064982F936DE76657",
    "0x8617e340b3D01fa5F11f306F4090Fd50e238070d",
    "0x88021160c5C792225E4E5452585947470010289d",
    "0xd1220a0CF47c7B9Be7A2E6Ba89f429762E7b9adB",
    "0xdbF03B407C01E7cd3cbEa99509D93f8dDDc8C6fB",
    "0xDE709F2102306220921060314715629080e2Fb77",
    "0xFb6916095CA1dF60bb79CE92ce3Ea74C37c5D359",
]
test_cases = {"30": rsk_mainnet, "31": rsk_testnet, "1": eth_mainnet}

for eip155_chainid, cases in test_cases.items():
    for address in cases:
        assert (address == eip1191_checksum(address[2:], eip155_chainid))

Discussion

The list in EIP-155 is provided for informational purposes only. There is no effect on the interpretation of EIP-155 based on the entries in the table list. That is distinguishable from EIP-1191 (DRAFT) which currently requires that output be different based on whether a chain ID "belongs to a network that opted...". Networks are not human and they do not opt. That specification is undefined behavior.

The solution is: add this paragraph to the specification:

Clients should use EIP-1191 addresses for networks which are known to support EIP-1191, see Known Supported Networks, below. For other networks, clients should continue to use EIP-155 addresses.

@juli
Copy link
Contributor Author

juli commented Nov 9, 2019

@fulldecent

  • Please read about the differences between SHA3 and Keccak 256 here.
  • Python 3 code from current version of EIP-1191 works for me.
  • Your code has the following problems:
    • it does not include '0x' between the chain id and the address
    • it uses EIP-1191 for the eth_mainnet test cases
  • Who opts to implement an EIP?
  • IMO the list of chain ids in the EIP-1191 implementation is more clear than adding the text you suggest.

@fulldecent
Copy link
Contributor

Please read about the differences between SHA3 and Keccak 256 here.

Please help to correct my implementation. I'm just here to point out problems.

Python 3 code from current version of EIP-1191 works for me.

Here is how I executed the current version of EIP-1191 (DRAFT). It does not compile.

python3 <<EOL
#!/usr/bin/python3
from sha3 import keccak_256
import random
"""
   addr (str): Hexadecimal address, 40 characters long with 2 characters prefix
   chainid (int): chain id from EIP-155 """
def eth_checksum_encode(addr, chainid=1):
    adopted_eip1191 = [30, 31]
    hash_input = str(chainid) + addr.lower() if chainid in adopted_eip1191 else addr[2:].lower()
    hash_output = keccak_256(hash_input.encode('utf8')).hexdigest()
    aggregate = zip(addr[2:].lower(),hash_output)
    out = addr[:2] + ''.join([c.upper() if int(a,16) >= 8 else c for c,a in aggregate])
    return out
EOL

Result

Traceback (most recent call last):
File "", line 2, in
ModuleNotFoundError: No module named 'sha3'

Your code has the following problems:

it does not include '0x' between the chain id and the address

Yes, this is intentional. EIP-55 does not use "0x" in the hash. Source https://eips.ethereum.org/EIPS/eip-55. Currently, EIP-1191 (DRAFT) also does not use "0x", source "if a registered chain id is provided, add it to the input of the [EIP-55] hash function." So my implementation does not use "0x".

it uses EIP-1191 for the eth_mainnet test cases

Perhaps the mainnet test cases should be removed.

Or, if a more complete test case is desired then we can add a function checksum_address() which decides whether to use EIP-1191 (DRAFT) or EIP-55.

Who opts to implement an EIP?

"Opt" is not used in my revised specification or my revised DRAFT implementation. I do not understand your question. But in general, anybody opts an EIP. This is specified at https://github.com/ethereum/EIPs

Ethereum Improvement Proposals (EIPs) describe standards for the Ethereum platform, including core protocol specifications, client APIs, and contract standards.

This must be read implicitly. Because this project scope statement says nothing of who opts to use EIPs, it is understood that anybody, or nobody can use an EIP.

IMO the list of chain ids in the EIP-1191 implementation is more clear than adding the text you suggest.

Agreed. Here is an update to be more clear:

An address (string of 40 hex digits) and an EIP-155 chain ID (string of digits with no loading zeros) are required to calculate an EIP-1191 checksummed address.

First calculate a reference string. This is the hexadecimal SHA-3 hash of the concatenation of the address and chain ID. The EIP-1191 checksummed address shall be equal to the original address with each hexdigit A through F at location i in the address shown in uppercase iff the hexdigit i in the reference string is greater than or equal to 8. Typically the checksummed address is shown prefixed with "0x".

Clients should use addresses in EIP-1191 format for networks listed in the Adoption Table below. For other networks, clients should use addresses in EIP-55 format.


Please note that another benefit of doing it this way ^^ is that it allows mainnet to upgrade to EIP-1191 (DRAFT) in the future. Clients will recognize EIP-1191 (DRAFT) addresses and EIP-55 addresses. There will probably even be a common emoji or color that clients show to differentiate chain-specific (1191) and chain-general (55) addresses. Color and emoji are out of the scope of this proposal. And supporting this use case is NOT a motivation for the above changes.

@juli
Copy link
Contributor Author

juli commented Nov 9, 2019

@fulldecent

  • When you write SHA3 you should write Keccak256, the output of both functions is different.
  • It will be more practical for everybody if you make a PR with your suggestions to the EIP instead of pasting them as Github comments.
  • You could also make a PR to EIP-55 because the way you write it is more clear than the original EIP-55 text.
  • Python3's sha3 module is necessary to make the code work.
  • EIP-55 for example requires the ethereum module in the same way EIP-1191 requires keccak_256 from ethereum import utils
  • Your implementation is wrong if it does not use 0x for EIP-1191.
  • The EIP references 8 different implementations of EIP-1191 including Trezor and Ledger wallets so if your alternative implementation is not compatible you should make a new EIP including the emoji ideas.

@juli
Copy link
Contributor Author

juli commented Nov 18, 2019

replying to @axic comment

It makes a lot more sense to specify how to calculate the checksum and leave the adoption outside
the scope of this EIP.

@alcuadrado @MicahZoltu

What do you think about:

  1. replacing the adoption table by a link to the ethereum-lists repository.
  2. Adding a JSON field such as eip55 or eip1191 or checksumType to the JSON documents where the chain id and name are specified?
  3. If you agree, what would be the preferred JSON variable name and where should it be specified?

Thanks

@fulldecent
Copy link
Contributor

I fully support this EIP for going to FINAL status as-is.

It is well-implemented, well-supported, well-specified.

If there are places that don't support this, I'm sure they will welcome a PR.

@marianasoff
Copy link
Contributor

My humble opinion is that sadly there are people in this community that do not want something things to improve or evolve due to personal and selfish reason. It is very easy to critisize instead of trying to help promote minor useful new changes. Participants such as @ricmoo can seriously harm and stall the evolution of this ecosistem.

@MicahZoltu
Copy link
Contributor

Please keep personal attacks and claims about motives out of the EIP process.

@ricmoo
Copy link
Contributor

ricmoo commented Sep 3, 2021

I fully support this EIP for going to FINAL status as-is.

This cannot be placed in Final without a backwards compatibility note. It (objectively) breaks backwards compatibility.

I believe that concern should qualify as an “unaddressed technical complaints”.

@tjayrush
Copy link

tjayrush commented Sep 3, 2021

@fulldecent Having now read the entire thread of this discussion, can you explain to me why you went from This should be demoted to Draft status on May 14th to I fully support this EIP going to FINAL status as is 34 minutes ago. I don't see any intervening discussion that may have changed your mind. May I ask why the change of sentiment (and if I may not ask, then please ignore...)?

@fulldecent
Copy link
Contributor

Good question, just change of sentiment. There is plenty of room to improve this specification. If they want a coauthor, I will help.

If you can be bothered to read the implementations (rather than the specification, which is just wrong) it is unambiguous. And also I think RSK network has the prerogative to set their preferred checksumming, which is mostly what this is about.

So what I'm trying to say is that the people who wanted to adopt this already have and if this EIP won't get any better then I would rather it live on as a FINAL than a DRAFT.

@gasolin
Copy link

gasolin commented Nov 14, 2021

I like the idea that helps different EVM compatible chain users to have a better awareness of what chain's address they are willing to send.

  • The EIP does not affect the current Ethereum users at all.
  • This EIP helps ethereum users to not mistakenly send tokens to invalid addresses (but valid in other EVM compatible chain)
  • Helps the token receivers (Exchange, Token sale etc) to provide tokens addresses that suit the right chain.

Will create some issues on the related tools and see if they are interesting in supporting this EIP

@ricmoo
Copy link
Contributor

ricmoo commented Nov 14, 2021

This does affect current Ethereum ecosystems.

For example, if you cut and paste your address from Etherscan, or if you click your account info in MetaMask (to copy the address to the clipboard) you won’t be able to paste it into the ropsten Etherscan site or paste it into MetaMask on a different network. You would need a tool to convert the address every time. It is very common in Ethereum to use the same address across multiple networks, and this would break much of the existing UX around this.

@gasolin
Copy link

gasolin commented Nov 14, 2021

@ricmoo thanks to point it out. I guess you mean the current checksum address is widely adapted, changing it will cause more issues than the benefits during the transition?

There are new competing attempts to solve this problem which do not break backwards compatibility, such as CAIP.

regarding the above comment, does it refers

https://github.com/ChainAgnostic/CAIPs/blob/master/CAIPs/caip-10.md#test-cases

?

@ricmoo
Copy link
Contributor

ricmoo commented Nov 14, 2021

@gasolin Exactly! :)

The CAIP system isn’t something I’ve followed closely, but it seems to be the front-runner for a backwards-compatible address format which is entangled with chain ID.

@MicahZoltu
Copy link
Contributor

Closing this for housekeeping reasons, but please feel free to continue using this issue as the discussions-to thread for EIP-1191

@xinbenlv
Copy link
Contributor

xinbenlv commented Nov 11, 2022

Hi from 2022, I like the idea of this EIP. One thing I love to bring up is that I happen to come across this particular line of code in geth which probably gets used widely. I wonder if we start adopting EIP-1191 would it break some software without a properly scheduled compatibility mitigation?

https://github.com/ethereum/go-ethereum/blob/a609e7b81fbf360745b06a35227da52cc50c2d1b/common/types.go#L221

@ricmoo
Copy link
Contributor

ricmoo commented Nov 11, 2022

@xinbenlv I’ve actually fought quite hard against that EIP. You should be able to see my comments on the EIP discussion. :)

Support for that EIP has been marked in the documentation of Web3.js and EthereumVM has been identified as unsafe and I believe they plan to (or may have already?) removed support for it, as it is dangerous.

This change is not backwards compatible with the Ethereum ecosystem, and would cause so much to break. For example, it means a checksum address copied from Etherscan for mainnet would fail when pasting it into MetaMask currently attached to Görli.

There are new address formats available (E.g. CAIP) that could have this change made without affecting the ecosystem in a way that would cause everything to break, but to modify a widespread standard in a way that changes the meaning and value of the checksum would collapse everything.

Another big problem with this idea in general. Is that many places when an address is needed, do not have access to the chain ID, so any checksum comparisons would be unable to be performed. For example, if a contract isn’t connected to a network, and you just wanted to use populateTransaction, the to address would not be able to be checksummed in the prepared transaction.

With ENS names, which I would prefer see get better cross-chain support (coming in v6) much of this is moot anyways, since we can just ask for that chain ID’s address, so it can be entirely different addresses, if need be. :)

@ricmoo
Copy link
Contributor

ricmoo commented Nov 11, 2022

(apologies, I get a lot of emails per day from ether’s repo and thought that this was an issue opened against ethers, I am just now realizing that it was posted on the discussions-to issue. Sorry for the noise.)

@xinbenlv
Copy link
Contributor

xinbenlv commented Nov 12, 2022

Hi @ricmoo , yes you are right. My comment was meant to bring up question / doubt about this EIP-1191, despite I can see merit about it.

I came across this EIP partly because (1) I am working on #2294 which needs some research on what could cause a problem if boundary of chainId changes. it somehow is stuck at Last Call Deadline; partly because (2) By researching geth I notice despite this is a ERC level of proposal, it could mean the users using geth could break if a string is sent without proper nomalization with under new EIP-1191. Thats why I raise the same concern about compatibility.

In summary, I am not advocating for it, I am just concerning if it's still a last call. You raise compatibility issue from ethers.js perspective, I raise compatibility issue from geth perspective.

It's a weird state where an EIP is being adopted by some clients, but rejected by some other clients. I wonder what state the EIP should be, Final? Withdrawn?

@Pandapip1
Copy link
Member

How about we use a different prefix for EIP-1191 addresses? Instead of 0x, how about 1x?

@fulldecent
Copy link
Contributor

This EIP will not break MetaMask and it will not stop you from copy pasting a Mainnet address to testnet.

MetaMask adopts things very slowly and this behavior is opt-in.

If a wallet, such as certain popular wallets, wanted to adopt EIP-1191 then if you tried to copy paste a Mainnet address it would probably give you a warning:

It looks like you copied a Mainnet address and you are using it on Goerli. Would you like to proceed? (We will update the formatting of the address for you.)

Even today, Etherscan still does not fully support EIP-55.


But either way if nobody is driving this then it doesn't matter because it wont go forward.

@Pandapip1
Copy link
Member

How about we use a different prefix for EIP-1191 addresses? Instead of 0x, how about 1x?

@fulldecent how does this look?

@MicahZoltu
Copy link
Contributor

0x means hexadecimal. Changing it to 1x would go against that standard: https://stackoverflow.com/a/4581351/879046

@xinbenlv
Copy link
Contributor

@Pandapip1 I can see merit of EIP-1191 to avoid error for executing on wrong chains, and I also worry about backward compatibility.

I wonder if a middle-ground we can leverage EIP-1191 but avoid backward incompatibility

Would it be acceptable if we only use 1191 in the context of CAIP-10 or other chain-aware format (e.g. https://eips.ethereum.org/EIPS/eip-3770)?

@ricmoo

@Pandapip1
Copy link
Member

Pandapip1 commented Nov 15, 2022

I'm well aware that 0x means hexadecimal. But in the case of Ethereum addresses, it also means EIP-55. Using a different prefix for Ethereum addresses can be useful for differentiation, even if it is non-standard for representing numbers.

@MicahZoltu
Copy link
Contributor

in the case of Ethereum addresses, it also means EIP-55

I'm not sure where you are getting this idea from, but it definitely isn't the case. Addresses have always been presented as 0x prefixed hexadecimal, even before EIP-55 and even when the address isn't checksummed.

@Pandapip1
Copy link
Member

Well... there was also the IBAN format.

I'm proposing that there be a new address format to intentionally break compatibility. Also, perhaps, instead of 1x, we could prefix it with the chain ID (so ethereum is 1x, optimism is 10x, polygon is 137x, etc).

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

No branches or pull requests