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

Byzantium Update for Expanse #7146

Closed
wants to merge 2 commits into from

Conversation

chrisfranko
Copy link
Contributor

Here the changes go. Hope I didnt miss anything.

Here the changes go. Hope I didnt miss anything.
@parity-cla-bot
Copy link

It looks like @chrisfranko signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@debris
Copy link
Collaborator

debris commented Nov 27, 2017

I believe these are indeed all changes. @5chdn can you confirm?

also, @chrisfranko do you have any document specifying these changes (and block transition number)? It would be nice to keep a link to a doc like that in the pr.

@debris
Copy link
Collaborator

debris commented Nov 27, 2017

@closes #7061

@5chdn 5chdn added A0-pleasereview 🤓 Pull request needs code review. M2-config 📂 Chain specifications and node configurations. labels Nov 27, 2017
@5chdn 5chdn added this to the 1.9 milestone Nov 27, 2017
Copy link
Contributor

@5chdn 5chdn left a comment

Choose a reason for hiding this comment

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

Doing a full sync now to test. Did you test that configuration beyond block 800_000 @chrisfranko ?

Checked against your changelog, and the changes I see include:

  • Addition of ‘REVERT’ opcode, which permits error handling without consuming all gas (EIP 140)
  • Transaction receipts now include a status field to indicate success or failure EIP 658)
  • Elliptic curve addition and scalar multiplication on alt_bn128 (EIP 196) and pairing checks (EIP 197), permitting
  • ZK-Snarks and other cryptographic mathemagic™ ??
  • Support for big integer modular exponentiation (EIP 198), enabling RSA signature verification and other cryptographic applications
  • Support for variable length return values (EIP 211)
  • Addition of the ‘STATICCALL’ opcode, permitting non-state-changing calls to other contracts (EIP 214)
  • Changes to the difficulty adjustment formula to take uncles into account (EIP 100)
  • Fork Block Number: 800,000
  • New Block Target: 30 Seconds ??
  • New Block Reward: 4 EXP

Regarding zksnarks: I don't exactly understand how this was activated in Byzantium.

Did you manually retarget the block time or did you just delay the difficulty bomb via EIP-649?

@5chdn 5chdn added the B0-patch label Nov 27, 2017
@chrisfranko
Copy link
Contributor Author

we removed the difficulty bomb all together early on.

@chrisfranko
Copy link
Contributor Author

the zsnarks stuff that was in the gexp changelog so i just rolled with it

Copy link
Contributor

@5chdn 5chdn left a comment

Choose a reason for hiding this comment

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

Does not work.

2017-11-27 13:57:47  Syncing  #791501 62b0…cb2e    72 blk/s  495 tx/s  11 Mgas/s    964+   19 Qed   #792873    4/25 peers   293 MiB chain 98 MiB db 3 MiB queue 3 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:57:56  Syncing  #792508 8909…0320   114 blk/s  765 tx/s  16 Mgas/s   1306+    3 Qed   #793810    4/25 peers   293 MiB chain 98 MiB db 4 MiB queue 3 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:58:06  Syncing  #793514 7b9d…e9c6   104 blk/s  691 tx/s  14 Mgas/s   1288+   17 Qed   #794826    4/25 peers   293 MiB chain 98 MiB db 4 MiB queue 4 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:58:16  Syncing  #794405 1ce6…0061    90 blk/s  557 tx/s  12 Mgas/s    802+   10 Qed   #795207    4/25 peers   293 MiB chain 98 MiB db 3 MiB queue 4 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:58:26  Syncing  #795389 fe17…c305   102 blk/s  620 tx/s  13 Mgas/s   1087+    1 Qed   #796477    4/25 peers   293 MiB chain 98 MiB db 3 MiB queue 3 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:58:37  Syncing  #796263 4d05…6a4d    90 blk/s  626 tx/s  13 Mgas/s   1252+   12 Qed   #797537    4/25 peers   294 MiB chain 98 MiB db 4 MiB queue 3 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:58:46  Syncing  #797512 1047…5ed7   134 blk/s  893 tx/s  19 Mgas/s   1367+    5 Qed   #798890    4/25 peers   294 MiB chain 98 MiB db 4 MiB queue 3 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:58:56  Syncing  #799153 4b63…3142   168 blk/s  996 tx/s  23 Mgas/s   1155+    2 Qed   #800287    3/25 peers   295 MiB chain 98 MiB db 3 MiB queue 4 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:59:01  Stage 3 block verification failed for #800000 (56cb…06af)
Error: Block(InvalidDifficulty(Mismatch { expected: 23024933077909, found: 23069903650326 }))
2017-11-27 13:59:06  Syncing  #799999 6b8a…bcb2    89 blk/s  376 tx/s   9 Mgas/s    739+    0 Qed   #801811    3/25 peers   295 MiB chain 98 MiB db 2 MiB queue 3 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 13:59:41     2/25 peers   296 MiB chain 98 MiB db 0 bytes queue 15 KiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-11-27 14:00:16     2/25 peers   296 MiB chain 98 MiB db 0 bytes queue 15 KiB sync  RPC:  0 conn,  0 req/s,   0 µs

How did you change the block time/difficulty calculation?

@5chdn 5chdn added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 27, 2017
@chrisfranko
Copy link
Contributor Author

@5chdn
Copy link
Contributor

5chdn commented Nov 27, 2017

It seems you don't implement EIP 649 at all. Diff to go-ethereum:

screenshot at 2017-11-27 14-51-17

@chrisfranko
Copy link
Contributor Author

Right because we completely removed the bomb before homestead. From my recollection parity had a bomb defuse param in the config file.

So if !bombDefusal do eip649 stuff, else dont.

@chrisfranko
Copy link
Contributor Author

Also what software is that, because that is super helpful.

@5chdn
Copy link
Contributor

5chdn commented Nov 27, 2017

I'm using http://meldmerge.org/ for quick diffs and merging.

So you are not using EIP-649, what else changes the difficulty in Byzantium?

@chrisfranko
Copy link
Contributor Author

The uncle calculations are pretty much the only thing extra in Byzantium. And the block reward being 4 instead of 8 and blocktime being 30 seconds instead of 60.

@chrisfranko
Copy link
Contributor Author

I think we found what I forgot to change. Before byzantium blocktime and after.

@5chdn
Copy link
Contributor

5chdn commented Nov 27, 2017

Requires some logic in the code. Does this fork have a name yet? Would certainly help.

Maybe looking into #6621 can help you to understand how we implement custom chain parameters (look for the mcip-3 fork). Or look at ecip-1010 #3179.

@chrisfranko
Copy link
Contributor Author

In GO
ETH - x.Div(x, big9)
EXP - x.Div(x, big30)

In Parity i'm assuming this is duration_limit

https://github.com/ethereumproject/parity/blob/fe84144f56d97a82f54619f8f96d3d83c50108ee/ethcore/src/ethereum/ethash.rs#L86

This changes in eth from 10, to 9, but i dont cant seem to find where. Im sorry if im over looking it, i'm a complete rust noob.

@5chdn
Copy link
Contributor

5chdn commented Dec 7, 2017

@chrisfranko do you have any kind of formalization what exactly changes at 800_000? I personally want to see Expanse support in Parity, but I have issues to follow what this hard-fork exactly does. Do you have any Expanse Improvement Proposals or any similar process? Anything that allows developers to understand which consensus rules were changed and how?

@chrisfranko
Copy link
Contributor Author

chrisfranko commented Dec 7, 2017 via email

@5chdn
Copy link
Contributor

5chdn commented Dec 8, 2017

But how did you reduce it to 30 seconds? Simply doing a Byzantium transition on EXP does not do the trick. Did you change any code, could you point me to a commit maybe?

@chrisfranko
Copy link
Contributor Author

@5chdn 5chdn added the A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. label Dec 30, 2017
@5chdn 5chdn modified the milestones: 1.9, 1.10 Dec 30, 2017
@5chdn 5chdn closed this Jan 2, 2018
This was referenced Jan 2, 2018
@chrisfranko
Copy link
Contributor Author

Blocked? And removed? I was on vacation. We did everything we were supposed to do to maintain this.

@5chdn
Copy link
Contributor

5chdn commented Jan 3, 2018

Not sure why this card glitched into the Documentation board. Removed it.

@chrisfranko
Copy link
Contributor Author

im so confused

@5chdn
Copy link
Contributor

5chdn commented Jan 3, 2018

Let me try to explain. Your chain spec is fine. But it's not enough, you have some custom changes in your difficulty calculation that are not in line with those we implemented for our Ethereum-based engines. That's totally fine but requires some logic on core code-level to describe the difficulty correctly in case of the Expanse fork.

First, we need a name, let's call it EXP-800. We then need to introduce some exp800transition in the chain spec and also in the Ethash module. Compare with other available transitions, e.g., MCIP-3, ECIP-1010 or EIP-649.

See https://github.com/paritytech/parity/blob/master/ethcore/src/ethereum/ethash.rs#L340-L411 for example. This is the parity implementation of calculate_difficulty we discussed earlier. This needs a special case for EXP-800. If you read through this function, you will see a lot of checks and special cases for other transitions, like EIP-100B, EIP-649, ECIP-1010.

Now, all we need is a working implementation of the difficulty for EXP-800. You could template it on top of EIP-649 and modify it to match your custom Go-implementation. I'm sorry if this was not clear in my earlier comments.

Just to highlight two things: (1) A chain spec is only able to modify the specification within the existing boundaries and EXP-800 is beyond that scope. (2) Parity does not run one chain but many different ones; this abstraction allows for so many opportunities configuring different blockchain environments, but once you want custom consensus rules, it must be carefully specified somewhere, not only for one chain but for all supported chains. :)

@chrisfranko
Copy link
Contributor Author

chrisfranko commented Jan 3, 2018 via email

@5chdn 5chdn reopened this Jan 3, 2018
@5chdn 5chdn added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Jan 3, 2018
@5chdn 5chdn modified the milestones: 1.10, 1.9 Jan 3, 2018
@5chdn 5chdn removed the A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. label Jan 3, 2018
@5chdn 5chdn added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Jan 4, 2018
@5chdn
Copy link
Contributor

5chdn commented Jan 4, 2018

I don't think this is enough consensus-wise. It does not compile.

I'll check if it syncs later tonight. WIP: https://github.com/paritytech/parity/commits/chrisfranko-patch-2

@5chdn 5chdn added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 4, 2018
self.ethash_params.expip2_duration_limit
}else{
self.ethash_params.duration_limit
}
Copy link
Contributor

Choose a reason for hiding this comment

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

};

@5chdn
Copy link
Contributor

5chdn commented Jan 4, 2018

2018-01-04 20:08:50  Syncing  #790509 01bc…da92   288 blk/s 2023 tx/s  44 Mgas/s   7721+  121 Qed   #798364    6/25 peers   7 MiB chain 98 MiB db 25 MiB queue 12 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2018-01-04 20:09:00  Syncing  #794463 451f…aa6b   401 blk/s 2613 tx/s  57 Mgas/s   7938+   50 Qed   #802474    3/25 peers   7 MiB chain 98 MiB db 23 MiB queue 8 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2018-01-04 20:09:10  Syncing  #798298 bc4c…466b   381 blk/s 2524 tx/s  55 Mgas/s   5116+   38 Qed   #803446    3/25 peers   7 MiB chain 98 MiB db 13 MiB queue 8 MiB sync  RPC:  0 conn,  0 req/s,   0 µs
2018-01-04 20:09:14  Stage 3 block verification failed for #800000 (68b8…a7ed)
Error: Block(InvalidDifficulty(Mismatch { expected: 23024933077909, found: 23069903650326 }))
2018-01-04 20:09:20  Syncing  #799999 6b8a…bcb2   174 blk/s  821 tx/s  20 Mgas/s    284+    0 Qed   #804594    5/25 peers   4 MiB chain 98 MiB db 672 KiB queue 8 MiB sync  RPC:  0 conn,  0 req/s,   0 µs

Hm ...

@5chdn
Copy link
Contributor

5chdn commented Jan 4, 2018

metropolisDifficultyIncrementDivisor was missing. #7463 should do the trick.

@5chdn 5chdn closed this Jan 4, 2018
@chrisfranko
Copy link
Contributor Author

chrisfranko commented Jan 5, 2018 via email

@5chdn
Copy link
Contributor

5chdn commented Jan 5, 2018

That was a duplicate entry.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M2-config 📂 Chain specifications and node configurations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants