core-geth icon indicating copy to clipboard operation
core-geth copied to clipboard

Add keccak mining

Open atoulme opened this issue 4 years ago • 29 comments
trafficstars

This PR adds keccak mining to Core-Geth.

Keccak mining is tested with test vectors.

atoulme avatar May 01 '21 05:05 atoulme

@atoulme Great to have you here, and thank you for this proposal.

Given the size and complexity, this will take a bit to review, but in general it looks like quite strong. We'll start taking a look at this this week, and will keep you in the loop throughout review. Please feel welcome to solicit feedback on anything you'd like specific attention on; we're here and happy to help.

meowsbits avatar May 03 '21 12:05 meowsbits

Thanks @meowsbits.

I was trying to run acceptance tests and had issues syncing with Astor with my modifications. The astor genesis file is here: https://github.com/hyperledger/besu/blob/master/config/src/main/resources/astor.json

Geth raises this error:

WARN [05-01|00:32:33.211] Invalid header encountered               number=1 hash="713364…241bb3" parent="f9c534…15f5bc" err="invalid difficulty: have 1046361407488, want 1098974756864"
DEBUG[05-01|00:32:33.211] Block body download terminated           err="syncing canceled (requested)"
DEBUG[05-01|00:32:33.211] Transaction receipt download terminated  err="syncing canceled (requested)"

I was wondering if the change in difficulty calculation might be tied to a hard fork schedule that's missing from the astor genesis file. Would you please take a quick look?

atoulme avatar May 08 '21 05:05 atoulme

@atoulme Apologies for being tardy in responding. Somehow I missed the notification entirely.

I'm starting to write a test for this -- here -- to investigate, but am coming up short on some stuff. I can't find any specification for the Astor testnet config (aside from the Besu implementation). Is there one?

To take a stab in the dark without any detailed information about Astor, I note that the chain configuration doesn't specify any features pertaining to difficulty. This means that core-geth will use the Frontier difficulty algorithm and bomb schedule.

Probably, this doesn't make a lot of sense for a new testnet, since it would likely want to use the latest difficulty algorithm upgrades from EIP2 and EIP100, and to disable the difficulty bomb (ECIP1041). Without knowing implementation defaults or configuration options for Besu, nor the configuration Astor specifies, I can't confirm if this is relevant or not.

A small side note is the genesis block uses a timestamp of 0x0, so the first block will have the least possible difficulty since it will have been apparently created some 50 years after genesis.

meowsbits avatar May 14 '21 11:05 meowsbits

I found some FixedDifficulty calculators at Besu, which appear contemporary with Keccak256 mining. Maybe this is related?

meowsbits avatar May 14 '21 11:05 meowsbits

OK, the astor testnet is probably best described under https://astor.host. Probably not to your satisfaction though.

The testnet is using the fork block ecip1049Block set to 0, as the latest hard fork. This fork block was added after the last hard fork for etc supported by besu, which would be thanos. The definition is here: https://github.com/hyperledger/besu/blob/e230a445bb457bbac27bf4c384da449ccbd3f043/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/ClassicProtocolSpecs.java#L265

Didn't think about the block timestamp. We did manage to produce blocks now, you can see our progress at astor.tmio.io.

atoulme avatar May 15 '21 08:05 atoulme

Alright, so chain configuration is definitely the root problem here then (or at least an important contributor).

AFAIU, Besu uses a "stacked subset features" paradigm, eg. a "Forks"-based configuration schema. This suggests to me that since Astor assumes ECIP1049 as an inheritor of the Thanos fork, that it will implicitly assume all of the preceding canonical ETC mainnet features to have been activated. CoreGeth uses a "Features, not Forks" configuration schema paradigm, so all desired features need to be explicitly articulated. You can reference the ETC config for CoreGeth here: https://github.com/etclabscore/core-geth/blob/master/params/config_classic.go#L30.

meowsbits avatar May 15 '21 10:05 meowsbits

This is great. So it looks like I could potentially use the same approach that was taken for Mordor, I'll look at the ins and outs of it.

atoulme avatar May 16 '21 01:05 atoulme

It's getting better. The difficulty calculations are still failing:

WARN [05-16|00:07:23.579] Synchronisation failed, dropping peer    peer=0cf4136259a482a1 err="retrieved hash chain is invalid: invalid difficulty: have 995780460544, want 997313216512"

You can now join Astor with a --astor command.

atoulme avatar May 16 '21 07:05 atoulme

OK, I fixed the difficulty algorithm issues. I managed to run geth and import blocks from astor 🎉

atoulme avatar May 16 '21 07:05 atoulme

Well done! I am also now the proud owner of 95 ATH (Astor-ETC ;) at 0x67ff22551a2e2daf142bb36e0409f4ce9cb38059; mining seems to be working OK.

meowsbits avatar May 16 '21 11:05 meowsbits

Congratulations, and welcome to the close circle of ATH 🐳

Confirmed mining with an external GPU miner works for me and for miners helping run the network.

atoulme avatar May 16 '21 15:05 atoulme

The build doesn't pass, but the failure seems unrelated to the work here. Please advise. AFAICT, everything is ok.

atoulme avatar May 18 '21 06:05 atoulme

update go.mod (missing deps [however they appear to already be in go.sum])

go get github.com/naoina/go-stringutil
go get github.com/steakknife/hamming

edit: this is likely a local issue on my end. but here for note taking purposes.

iquidus avatar May 18 '21 10:05 iquidus

Hey @meowsbits, @iquidus and @ziogaschr!

Is there anything specific which is still needed for this PR to be merged? Thanks!

@atoulme resolved the merge conflicts.

bobsummerwill avatar Jun 25 '21 21:06 bobsummerwill

Requesting another review (or two) given the scope and size of changes; otherwise LGTM.

There's a couple of nitpicks I noticed for small stuff like out-of-place comments, but I'm not going to bother messing with them at this point; we can do a cleanup review+PR as desired later.

The tests are rather sparse (eg. keccakHasher), but these are either common and only-duplicated/reused functions which already have other unit tests (and proven use), or I'm wishing for e2e tests that don't already exist and can't be expected in this changeset.

meowsbits avatar Jun 28 '21 14:06 meowsbits

I was able to sync and mine with Astor testnet 2-3 weeks ago. I have tried doing the same today and it is syncing, but on the last blocks it goes too slow and it loses the peers.

> eth.syncing
{
  currentBlock: 197922,
  highestBlock: 200033,
  knownStates: 28,
  pulledStates: 28,
  startingBlock: 197796
}

I will leave it on for sync to finish.

@atoulme any idea if Astor network is still available. As I can't finish syncing.

ziogaschr avatar Jun 29 '21 20:06 ziogaschr

The bootnode is still up and going: http://astor.tmio.io/

atoulme avatar Jun 29 '21 20:06 atoulme

I restarted the bootnode just in case it was stuck.

atoulme avatar Jun 29 '21 20:06 atoulme

I just tried updating my astor sync again, got an error about invalid difficulty at the top. Off by 1.

I build from source at https://github.com/etclabscore/core-geth/pull/369/commits/d8fa81e0e50d30d13ec1ed7b3011810ef52eb1aa

err="retrieved hash chain is invalid: invalid difficulty: have 13603621, want 13603622"
Jul 05 17:20:59 ubp52 sh[15634]: INFO [07-05|17:20:59.739] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=113.834ms   mgasps=0.000 number=196,898 hash=5f6ea4..b59398 age=3w2h59m   dirty=97.20KiB
Jul 05 17:21:00 ubp52 sh[15634]: INFO [07-05|17:21:00.004] Looking for peers                        peercount=1 tried=55  static=0
Jul 05 17:21:00 ubp52 sh[15634]: INFO [07-05|17:21:00.074] Storing bloom trie                       section=5 head=b733669aa07a3f3b3dd473fbd45dc32278d7fe8cf20ddb0fc6ad888701e20b8b root=56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421 compression=0.000
Jul 05 17:21:00 ubp52 sh[15634]: INFO [07-05|17:21:00.273] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=86.556ms    mgasps=0.000 number=197,090 hash=dd6b67..16e1e0 age=3w2h12m   dirty=97.20KiB
Jul 05 17:21:00 ubp52 sh[15634]: INFO [07-05|17:21:00.346] Storing CHT                              section=5 head=b733669aa07a3f3b3dd473fbd45dc32278d7fe8cf20ddb0fc6ad888701e20b8b root=cbb9d06dd19ed2636bc0897232eb7e301bfe816c24c8d4e28f54360b4c153807
Jul 05 17:21:00 ubp52 sh[15634]: INFO [07-05|17:21:00.865] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=105.213ms   mgasps=0.000 number=197,282 hash=947aca..5ca3d6 age=3w1h30m   dirty=97.20KiB
Jul 05 17:21:01 ubp52 sh[15634]: INFO [07-05|17:21:01.461] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=87.891ms    mgasps=0.000 number=197,474 hash=8bc66d..b3f442 age=3w53m13s  dirty=97.20KiB
Jul 05 17:21:02 ubp52 sh[15634]: INFO [07-05|17:21:02.087] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=147.333ms   mgasps=0.000 number=197,666 hash=2d82a8..ee101c age=3w12m45s  dirty=97.20KiB
Jul 05 17:21:02 ubp52 sh[15634]: INFO [07-05|17:21:02.630] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=120.589ms   mgasps=0.000 number=197,858 hash=98b6c3..b7d469 age=2w6d23h   dirty=97.20KiB
Jul 05 17:21:03 ubp52 sh[15634]: INFO [07-05|17:21:03.178] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=63.441ms    mgasps=0.000 number=198,050 hash=9347b1..4b62f9 age=2w6d23h   dirty=97.20KiB
Jul 05 17:21:03 ubp52 sh[15634]: INFO [07-05|17:21:03.762] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=75.534ms    mgasps=0.000 number=198,242 hash=5e390a..da78b4 age=2w6d22h   dirty=97.20KiB
Jul 05 17:21:04 ubp52 sh[15634]: INFO [07-05|17:21:04.349] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=98.210ms    mgasps=0.000 number=198,434 hash=426e71..0828c0 age=2w6d21h   dirty=97.20KiB
Jul 05 17:21:04 ubp52 sh[15634]: INFO [07-05|17:21:04.911] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=61.200ms    mgasps=0.000 number=198,626 hash=a5dbc7..2bd200 age=2w6d20h   dirty=97.20KiB
Jul 05 17:21:05 ubp52 sh[15634]: INFO [07-05|17:21:05.466] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=52.356ms    mgasps=0.000 number=198,818 hash=424c91..a488f4 age=2w6d20h   dirty=97.20KiB
Jul 05 17:21:06 ubp52 sh[15634]: INFO [07-05|17:21:06.050] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=62.510ms    mgasps=0.000 number=199,010 hash=e41778..87173c age=2w6d19h   dirty=97.20KiB
Jul 05 17:21:06 ubp52 sh[15634]: INFO [07-05|17:21:06.637] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=52.647ms    mgasps=0.000 number=199,202 hash=ee67ef..d6378e age=2w6d18h   dirty=97.20KiB
Jul 05 17:21:07 ubp52 sh[15634]: INFO [07-05|17:21:07.241] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=82.318ms    mgasps=0.000 number=199,394 hash=0cd8f2..5a82d3 age=2w6d18h   dirty=97.20KiB
Jul 05 17:21:07 ubp52 sh[15634]: INFO [07-05|17:21:07.872] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=104.620ms   mgasps=0.000 number=199,586 hash=b8ff2d..f048ae age=2w6d17h   dirty=97.20KiB
Jul 05 17:21:08 ubp52 sh[15634]: INFO [07-05|17:21:08.378] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=54.122ms    mgasps=0.000 number=199,778 hash=f8128b..7c108c age=2w6d16h   dirty=97.20KiB
Jul 05 17:21:09 ubp52 sh[15634]: INFO [07-05|17:21:09.002] Imported new chain segment               blocks=192   txs=0 mgas=0.000 elapsed=103.560ms   mgasps=0.000 number=199,970 hash=299324..71e773 age=2w6d16h   dirty=97.20KiB
Jul 05 17:21:12 ubp52 sh[15634]: WARN [07-05|17:21:12.766] Synchronisation failed, dropping peer    peer=0cf4136259a482a1f75318eee4d063889339529b1ba699895f90dd8ffd34eef8 err="retrieved hash chain is invalid: invalid difficulty: have 13603621, want 13603622"
Jul 05 17:21:12 ubp52 sh[15634]: ERROR[07-05|17:21:12.766] Ethereum peer removal failed             peer=0cf41362 err="peer not registered"
Jul 05 17:21:12 ubp52 sh[15634]: INFO [07-05|17:21:12.766] Commit new mining work                   number=200,000 sealhash=abe203..50bf0f uncles=0 txs=0 gas=0 fees=0 elapsed="145.839µs"
Jul 05 17:21:20 ubp52 sh[15634]: INFO [07-05|17:21:20.134] Looking for peers                        peercount=0 tried=52  static=0
Jul 05 17:21:30 ubp52 sh[15634]: INFO [07-05|17:21:30.301] Looking for peers                        peercount=0 tried=108 static=0
Jul 05 17:21:34 ubp52 sh[15634]: INFO [07-05|17:21:34.392] Deep froze chain segment                 blocks=12317 elapsed=1.613s      number=109,999 hash=7dd0e9..a1514d
Jul 05 17:21:40 ubp52 sh[15634]: INFO [07-05|17:21:40.764] Looking for peers                        peercount=0 tried=72  static=0
Jul 05 17:21:51 ubp52 sh[15634]: INFO [07-05|17:21:51.875] Looking for peers                        peercount=0 tried=23  static=0
Jul 05 17:22:01 ubp52 sh[15634]: INFO [07-05|17:22:01.996] Looking for peers                        peercount=2 tried=94  static=0
xJul 05 17:22:12 ubp52 sh[15634]: INFO [07-05|17:22:12.797] Looking for peers                        peercount=0 tried=26  static=0

meowsbits avatar Jul 05 '21 22:07 meowsbits

https://github.com/etclabscore/core-geth/pull/369#issuecomment-874352402

I get exactly the same as @meowsbits. @atoulme any clues on this one?

Here you can find the full log file

ziogaschr avatar Jul 07 '21 10:07 ziogaschr

Found the problem. Guard missing on the ECIP1041 Transition. With this added, node is now syncing with the boot node and mining. @atoulme

@ziogaschr @meowsbits hopefully you can confirm this works for you as well

http://astor.tmio.io/

image

jpcomps avatar Jul 14 '21 01:07 jpcomps

@jpcomps I can confirm that the fix works nice for both syncing and mining. Nice find man. Can you please commit the fix?

ziogaschr avatar Jul 14 '21 12:07 ziogaschr

OK, I removed difficulty bomb calculations for keccak since they should be separate ECIPs. I can sync locally to the head of the chain now.

atoulme avatar Jul 14 '21 13:07 atoulme

Just got around to re-syncing with the new changes. Looks good! Hopefully good to merge now. Change makes more sense than my previous "hack"

Thanks @atoulme

jpcomps avatar Jul 23 '21 19:07 jpcomps

Removing implementation by the Keccak engine for difficulty bomb seems a little troublesome to me in a couple ways.

  • The code design was (and is) almost entirely verbatim copy-paste from the Ethash engine, with the sole difference being the sealing and seal-validation. Carving out a chunk of the mirrored code creates an asymmetry which makes sense in the context of the Astor testnet, but isn't a necessarily general case.
  • If it is does make sense for Keccak to carve this chunk out (and indeed, the difficulty bomb is a very strange chunk and (hopefully) very particular to Ethash) -- then I think we just need to follow through with that assumption and add a few associated conditions in the Configurator, so both describe and enforce this logic. Comment above points to an example of a harmless but misleading use of the configuration vs. implementation (where the chain config describes parameters relating to difficulty, which will be silently ignored).

In general, I think the cited patch could probably be replaced with a small tweak to configuration only.

EDIT

On second thought, having doubts. It is called EthashECIP1041Transition after all.... will follow up.

meowsbits avatar Jul 28 '21 11:07 meowsbits

I'd look at it as this change matches exactly how Besu behaves and how the ECIP-1049 was drafted. We don't have a difficulty bomb designed for keccak mining.

atoulme avatar Jul 28 '21 18:07 atoulme

I have a branch going for development on this at https://github.com/etclabscore/core-geth/tree/keccak_mining-rebase-047f1129f3. PTAL before developing yourself, might save some time.

TLDR:

  • Rebase on master, which now includes a similar parallel idea of a Lyra2 engine.
  • Implement Keccak-specific configurator interfaces for EIP100b and ECIP1041 (re difficulty bomb)
  • Get rid of ethash.ModeFake etc. import that keccak package was re-using, and duplicate those types and logic in the keccak package itself (the idea is to make keccak actually it's own thing, and to trim out the hacky copy-pasta from ethash imports)
  • Still TODO: handle the ECIP1099 feature for keccak, which I gather the astor testnet presumes from block 0. I think this can just be removed since it should be totally irrelevant without a DAG.
  • TODO: Double check the ModeTest* logics, since I'm not sure off the top of my head what they want to do that might be Ethash-specific

In general these are sort of sticky configuration/implementation ideas, since it seems like Astor presumes a lot of stuff about network configuration as copy-paste from antecedent Ethash-based network configurations and upgrades. This is sort of ugly, since much of these specifications were/are explicitly for Ethash and the networks which used it.

meowsbits avatar Aug 04 '21 17:08 meowsbits

How can we progress this support, @meowsbits?

What can we do to help?

bobsummerwill avatar Oct 20 '21 18:10 bobsummerwill

second @bobsummerwill . let us know how we can help @meowsbits

jpcomps avatar Nov 08 '21 21:11 jpcomps