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

Fork Names Standard EIP #1848

Closed
wants to merge 5 commits into from
Closed

Fork Names Standard EIP #1848

wants to merge 5 commits into from

Conversation

winsvega
Copy link
Contributor

@winsvega winsvega commented Mar 18, 2019

Please review the proposal for how we deal with the fork names.
I think this is a step required before we come with a generic genesis file format.

EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
@winsvega
Copy link
Contributor Author

updated

@axic
Copy link
Member

axic commented Mar 19, 2019

Is clarifying these in https://eips.ethereum.org/EIPS/eip-233 an alternative to this EIP?

EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
@nicksavers
Copy link
Contributor

@winsvega It looks better now, but I would recommend going over the document again to remove any spelling and grammar mistakes and possibly further clarify some of the paragraphs. The general idea is clear, but it's still difficult to read at times. I also like the idea by @axic to merge this into the Draft for EIP #233.

@Arachnid
Copy link
Contributor

Arachnid commented Mar 19, 2019

Linear numbering doesn't really work, when each fork can potentially result in a new chain with its own forks.

What you can do instead is set the next largest unused bit whenever a fork is taken, and leave it clear when it isn't. So, with this (incomplete) list of forks:

  • Metropolis
  • DAO
  • Spurious Dragon
  • Byzantium
  • Constantinople

And with an initial number of 0, Metropolis is 1, DAO is 3, Spurious Dragon is 7, Byzantium 15, Constantinople is 31.

ETC, initially continuing the Metropolis chain, remained as 1 until they forked, at which point their fork ID was 5 (101b) - indicating they skipped the DAO fork.

@winsvega
Copy link
Contributor Author

winsvega commented Mar 21, 2019

@Arachnid
How will it work for this set of forks?

Frontier -> Homestead -> Dao -> Metropolis -> Const -> ConsFix
--------------------------------------------------- -> ConstUnFix
-----------------------> MetropolisC  -> ConstC -> ConsFixC
----------------------------------------------- -> ConstUnFixC

should we really try to describe all of the sidechain forks ?
But I agree that having a unique number assigned to any unique fork is way to go

@winsvega
Copy link
Contributor Author

This EIP could go with https://eips.ethereum.org/EIPS/eip-233 as a resolution discussion of its point

the desired codename of the hard fork,

with additional note: all clients must parse genesis with this fork codename described in particular json structure

@carver
Copy link
Contributor

carver commented Mar 24, 2019

How will it work for this set of forks?

Frontier -> Homestead -> Dao -> Metropolis -> Const -> ConsFix
--------------------------------------------------- -> ConstUnFix
-----------------------> MetropolisC  -> ConstC -> ConsFixC
----------------------------------------------- -> ConstUnFixC

So I think it would be:

  • Frontier: 0b1
  • Homestead: 0b11
  • DAO: 0b111
  • Metropolis: 0b1111
  • Const: 0b11111
  • ConstFix: 0b111111
  • ConstUnFix: 0b1011111
  • MetropolisC: 0b1011
  • ConstC: 0b11011
  • ConsFixC: 0b111011
  • ConstUnFixC: 0b1011011

(Note: these are not the actual production numbers, since the example skipped a few forks for clarity)

should we really try to describe all of the sidechain forks ?

I don't think we have a choice. We have to leave room in the naming for it.

But I agree that having a unique number assigned to any unique fork is way to go

Yeah, first impression: 👍

@winsvega
Copy link
Contributor Author

winsvega commented Mar 24, 2019 via email

@winsvega
Copy link
Contributor Author

winsvega commented Mar 25, 2019

Hmm. How about a description section for the forknames in a genesis file.
saying like.

forks : [ "Frontier", "Homestead", "ETH-0003"]  

then each fork is translated

"Frontier" : {
     "description" : "The initial ethereum protocol version",
     "activationBlock" : "1",
     "eipsIncluded" :  {
     }
},
"ETH-0003" : {
     "description" : "Update from 2018. description here: {URL} ",
     "activationBlock" : "10000000",
     "eipsIncluded" :  {
        "EIP-350"  : {
             "description" : "Sstore gas changes. description here: {URL}",
             "active" : "1"
        },
        "EIP-325" : {
             "description" : "description here: {URL}",
             "active" : "1"
        }
     }

this way any client could define any fork name they like, given that EIP numbers are fixed and sealed down in YP.
The client will just need to check that a given EIP number / name exist and return an error if such EIP does not exist. fork names are irrelevant. such configuration I could send via RPC to instantiate a version of a client with given set of activated EIPs.

@Arachnid
Copy link
Contributor

@winsvega What happens if there are two hardforks?
(3 new chains)

You mean, a three-way fork caused by a single change? Is that possible?

@winsvega
Copy link
Contributor Author

Is it true, that EIP names are fixed and immutable?
if so we could use it as anchor for the fork names or fork structure in genesis.json

#1848 (comment)

@carver
Copy link
Contributor

carver commented Mar 27, 2019

@winsvega What happens if there are two hardforks?
(3 new chains)

You mean, a three-way fork caused by a single change? Is that possible?

I suppose you could have two different hard fork changes that target the same block, like two different proposed solutions to the same problem. Presumably, this would only happen in a highly contentious situation.

I would just say we have to treat it as two consecutive forks in the numbering system.

Is it true, that EIP names are fixed and immutable?
if so we could use it as anchor for the fork names or fork structure in genesis.json

#1848 (comment)

I suppose you mean the EIP numbers? Yeah, my understanding is that once a number is assigned (even as a draft), that the number would not change for the same proposal, nor would the number ever be reassigned to a different proposal.

So maybe the best names for Constantinople and Petersburg are eip-1013 and eip-1716? (in the context of the genesis file). We just have to decide how important a goal it is for the genesis file to be human readable. If that's not an important goal, then referring to the EIP number seems like a strong option.

The client will just need to check that a given EIP number / name exist and return an error if such EIP does not exist. fork names are irrelevant. such configuration I could send via RPC to instantiate a version of a client with given set of activated EIPs.

I think this is a pretty cool idea as an extension, but it's asking more of the clients to implement. Every client may not have ways to easily flip each individual EIP on and off. So I think it could be a nice optional section of the specification, but we should only require the basic option of using a meta EIP number as the canonical hard fork name. (eg~ EIP-1013)

@bmann
Copy link
Contributor

bmann commented Apr 17, 2019

+1 on integrating with EIP-233 and a meta hard fork number as the consistent identifier.

Anything we need to do in the EIP233 updates to make this clearer?

@nicksavers nicksavers changed the title Fork Names Standart EIP Fork Names Standard EIP Apr 17, 2019
@axic
Copy link
Member

axic commented May 1, 2019

As an example, EIP-1716 for Petersburg has the following:

Codename: Petersburg
Aliases: St. Petersfork, Peter’s Fork, Constantinople Fix

I think we could clarify EIP-233 to require that clients and testing infrastructure ensures that the codename always works, but they are free to optionally support the aliases, if they chose.

@winsvega would that solve the problem?

@winsvega
Copy link
Contributor Author

winsvega commented May 1, 2019

Changing the code name from ConstantinopleFix will require to replace it all of the tests and aleth code. The codename must be sealed before the tests are created.

@axic axic added the meta label May 20, 2019
@axic
Copy link
Member

axic commented May 20, 2019

Changing the code name from ConstantinopleFix will require to replace it all of the tests and aleth code. The codename must be sealed before the tests are created.

I'd say for most of the time, the hardfork name was quite clear, with the exception of Constantinople. I hope that with Istanbul it will remain stable and the new process explored with EIP-233 tries to codify the codename very early on.

The suggestion by @carver to use the hardfork meta EIP number sounds like a great idea too. That means the next fork would be eip1679 in the test suite.

Also like @Arachnid's idea to if having a unique identifier for an entire chain based on the forks it took.

Is there something we need to do here? Does EIP-233 needs to be updated?

@winsvega I think for this PR you need to decide which of the discussed proposal you want to follow and perhaps focus this PR on testing implications (and specs) for hard forks.

@axic axic added the istanbul label May 20, 2019
EIPS/eip-1845.md Outdated
**Aleth:**
Aleth replace hardcoded variable names with an array that resolve
```
chainParams().forkblocks["ETH-0002-FB"]
Copy link
Member

Choose a reason for hiding this comment

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

Link to that implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its just a proposal. no implementation so far

@winsvega
Copy link
Contributor Author

The suggestion by @carver to use the hardfork meta EIP number sounds like a great idea too. That means the next fork would be eip1679 in the test suite.

That sounds nice. the actual changes are not clear from just the fork name such as Constantinple. if we refere to a specific eipNumber with the description it would be better. such approach will require to rename fork names to according eips in all of the tests.

also genesis standart should be in match with that. so genesis configuration shoud also support new fork names (such as would be eipXXXX, eipXXXX, eipXXXX)

@holiman would like to discuss this on AllCoreDev. the genesis standard of the fork names.
replace fork names with eipXXXX references.

@@ -0,0 +1,65 @@
---
eip: <to be assigned>
Copy link
Member

Choose a reason for hiding this comment

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

Please use 1848.

Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

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

Please rebase to master and fix the CI issues.

@shemnon
Copy link
Contributor

shemnon commented May 31, 2019

Can we get a table added, under backward compatibility, listing what the previous hard fork names were and what their corresponding number would be?

@shemnon
Copy link
Contributor

shemnon commented May 31, 2019

Also, how would you handle planned hard forks (Frontier, Homestead, Byzamtium, Istanbul) when an emergency fork (dao, tangerine whistle, spurious monkey) is needed but the next hard fork number has been designated to the next planned fork? Would the next planned fork have to switch numbers? Could a multi-step number be used like ETH-006-00 for Constantinople, ETH-006-01 for ConstantinopleFix, and ETH-007 for Istanbul to accommodate for this demonstrated need?

@winsvega
Copy link
Contributor Author

winsvega commented Jun 1, 2019

ETH-006-02 sounds like a nice versioning. Just need to create a page with description of exact EIPs included in each version

Copy link
Contributor

@shemnon shemnon left a comment

Choose a reason for hiding this comment

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

Here are a set of suggestions to use the ETH-xxx-yy format, along with a table of previous forks and mapping to the xxx-yy format.

EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
EIPS/eip-1845.md Outdated Show resolved Hide resolved
@shemnon
Copy link
Contributor

shemnon commented Jul 2, 2019

Hmm.... my backwards compatibility table got left out of the prior review. Here it is

Historical mappings

EIP-1848 number Reference Test Name Other Names
ETH-001-00 Frontier
ETH-002-00 Homestead
ETH-002-01 DAOFork
ETH-002-02 EIP150 Spurious Monkey
ETH-002-03 EIP158 Tengerine Whistle
ETH-003-00 Byzantium
ETH-004-00 Constantinople
ETH-004-01 ConstantinopleFix Petersburg
ETH-005-00 ETH-005-00 Istanbul
ETH-006-00 ETH-006-00 Asiago / Berlin

winsvega and others added 3 commits July 3, 2019 23:56
Co-Authored-By: Danno Ferrin <danno.ferrin@shemnon.com>
Co-Authored-By: Danno Ferrin <danno.ferrin@shemnon.com>
Co-Authored-By: Danno Ferrin <danno.ferrin@shemnon.com>
@chfast
Copy link
Member

chfast commented Jul 4, 2019

Personally, I think codenames are fine and this change is not needed. If you want to solve the issue of the changing name after some implementations are done I have a simple solution: refuse to do any implementation until the codename is decided.


## Motivation
**Clients:**
Different clients use different fork names even for the main network on their genesis config. If we want a generic genesis format, we have to use a standard for the fork names.
Copy link
Member

Choose a reason for hiding this comment

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

That's correctly identifies an issue, but do not explain why you need to create new standard instead of using the existing "standard" of "Hardfork Meta" EIPs: https://eips.ethereum.org/meta.

Copy link
Contributor

Choose a reason for hiding this comment

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

Name your pets, number your herd. This article is about servers but the intent is the same: https://www.engineyard.com/blog/pets-vs-cattle If we are going to go to 2-4 upgrades a year we don't need to spend time bikeshedding fork block names and waiting on someone to make it "official."

"ETH-xxx-yy-FB": "25000"
```

Where `ETH-001-00-FB` stands for `Frontier` fork block number.
Copy link
Member

Choose a reason for hiding this comment

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

What the ETH- stands for? Why is it needed?

"ETH-xxx-yy-FB": "25000"
```

Where `ETH-001-00-FB` stands for `Frontier` fork block number.
Copy link
Member

Choose a reason for hiding this comment

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

Explain what the -FB stands for.


Where `ETH-001-00-FB` stands for `Frontier` fork block number.

The counting pattern will follow an `ETH-xxx-yy` 'planned'/'unplanned' pattern, where the first `xxx` number increments for each planned fork, and each unplanned fork after the planned fork increments the `yy` number. Unplanned forks may be for emergency forks, such as Spurious Dragon, or for late revisions to fork definitions that was deployed onto long lived testnets, such as ConstantinopleFix/Petersburg.
Copy link
Member

Choose a reason for hiding this comment

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

Why is x.y not enough and we are inventing yet another versioning schema with some weird leading zeros and artificially limited number of digits?

**Aleth:**
Aleth replace hardcoded variable names with an array that resolve
```
chainParams().forkblocks["ETH-002-00-FB"]
Copy link
Member

Choose a reason for hiding this comment

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

This looks like quite inefficient where a struct field read is replaced with a map lookup by a string key.
I'd like to see full implementation. I believe these checks are done in almost every contract execution because EXP was used as a way to implement shifts and EXP cost depended on the fork being in.

@github-actions
Copy link

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Sep 15, 2020
@winsvega
Copy link
Contributor Author

I've implemented a map, so now its possible to translate test forknames into any type of client configuration file. its flexible with devs fork naming.

@winsvega winsvega closed this Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants