Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Network configuration for Ethereum Classic #3812

Merged

Conversation

splix
Copy link
Contributor

@splix splix commented Dec 11, 2016

EIP155, EIP160, Testnet

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling aed633d on ethereumproject:splix/classic-replay-protection into ** on ethcore:master**.

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Dec 11, 2016
"eip155Transition": "0x7fffffffffffffff",
"eip160Transition": "0x7fffffffffffffff",
"eip155Transition": 3000000,
"eip160Transition": 3000000,
Copy link
Contributor

@rphmeier rphmeier Dec 11, 2016

Choose a reason for hiding this comment

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

decimal instead of hex numbers here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, is that bad? because I see all new changes to configuration are using decimal for blocks

Copy link
Contributor

Choose a reason for hiding this comment

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

just odd that it's inconsistent with the other transitions definition. I'd suggest to pick one and go with it (with the exception of those set to "0x7ffff..." since that's easiest to read as hex)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was unrelated to this commits, so I didn't touch it. But I prefer decimal of course, so as I can change other values, I'm go to do it right now

@@ -25,7 +25,7 @@
"accountStartNonce": "0x00",
"maximumExtraDataSize": "0x20",
"minGasLimit": "0x1388",
"networkID" : "0x1",
"networkID" : "0x3d",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changing this will break connectivity with all peers on the old network. Just FYI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see. I misinterpreted this as a chainID. Thank you.
Where is CHAIN_ID option btw? I can't find it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently there's no separate setting. This value is used for both network protocol id and EIP-155 transaction signing id

@splix
Copy link
Contributor Author

splix commented Dec 11, 2016

@arkpar I think I can add this CHAIN_ID configuration option. Unless you're already working on this feature. Is that ok?

@arkpar
Copy link
Collaborator

arkpar commented Dec 11, 2016

@splix Fine with me as long as it defaults to network_id if not specified.

@gavofyork
Copy link
Contributor

we don't really want to keep the "original" morden. do you guys really want to have a "morden classic"? you could just take "morden.json" itself.

@splix
Copy link
Contributor Author

splix commented Dec 11, 2016

@gavofyork It's up to you actually. Yes, we can reuse existing morden, if no one uses it currently. I just see it uses ETH specific configuration for 1885k, so it's already incompatible. But there are only 1108 blocks after that fork, so probably nobody cares.

@splix
Copy link
Contributor Author

splix commented Dec 12, 2016

Submitted with Morden config fixed for ETC

@splix
Copy link
Contributor Author

splix commented Dec 12, 2016

@arkpar chain_id seems to be a different issue, I mean different PR

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e61d14d on ethereumproject:splix/classic-replay-protection into ** on ethcore:master**.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 13, 2016
@gavofyork gavofyork added the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Dec 13, 2016
@gavofyork gavofyork merged commit 80d6e49 into openethereum:master Dec 13, 2016
@splix splix mentioned this pull request Dec 21, 2016
This was referenced Jan 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants