-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Address Collision of Contract Address Causes Exceptional Halt #689
Conversation
See also #684 |
#684 works for me. |
Would be easier to merge this one, can you undelete the branch? |
Undeleted. |
@@ -0,0 +1,51 @@ | |||
## Preamble | |||
|
|||
EIP: <to be assigned> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use 689, and move this file to EIPS/eip-689.md
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:
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:
|
e681004
to
3903d66
Compare
according to ethereum#689 (comment)
EIPS/eip-689.md
Outdated
## Preamble | ||
|
||
--- | ||
eIP: 689 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be all lowercase.
EIPS/eip-689.md
Outdated
@@ -0,0 +1,50 @@ | |||
## Preamble |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These first 3 lines need deleting.
EIPS/eip-689.md
Outdated
|
||
This EIP has no effects after Constantinople fork because [EIP-86](https://github.com/ethereum/EIPs/pull/208) contains the changes proposed in this EIP. Even before the Constantinople fork, this EIP has no practical relevance because the change is visible only in case of a hash collision of keccak256. | ||
|
||
Regarding testing, this EIP releaves clients from supporting reversion of code overwriting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/releaves/relieves/
EIPS/eip-689.md
Outdated
|
||
## Rationale | ||
|
||
It seems unpractical to implement never-used features just for passing tests. Client implementations will be simpler with this EIP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/unpractical/impractical/
08b34cd
to
7e0dedc
Compare
@Arachnid I applied your comments. |
…um#689) * Add an EIP about making contract creation fail on address collision * Use 689 ethereum#689 (comment) * Adjust formats according to ethereum#689 (comment) * Cleanups by @Arachnid
* Add an EIP about making contract creation fail on address collision * Use 689 ethereum/EIPs#689 (comment) * Adjust formats according to ethereum/EIPs#689 (comment) * Cleanups by @Arachnid
This follows the discussion in coredev meeting 22, where no objections were made to the proposal to make contract creation fail in the case of address collision.