search

goerli / parity-goerli

The Clique engine for Parity Ethereum staging repository, will be merged upstream
GNU General Public License v3.0
22 stars 4 forks source link

Implement clique sealing logic #44

Closed 5chdn closed 5 years ago

5chdn commented 5 years ago

a parity node should be able to seal valid blocks in a clique network according to spec

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 2500.0 DAI (2500.0 USD @ $1.0/DAI) attached to it as part of the Görli Testnet Initiative fund.

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 2 weeks, 2 days ago. Please review their action plans below:

1) shamardy has applied to start work _(Funders only: approve worker | reject worker)_.

I would like to work on this issue. I have experience working with Rust and Parity. I am also familiar with Clique and Golang. I can have a PR ready for review in a week.

Learn more on the Gitcoin Issue Details page.

2) jwasinger has been approved to start work.

Sealing has been implemented for parity in https://github.com/goerli/parity-goerli/pull/29 and currently works: the client syncs to Goerli and Rinkeby, the client can function as a sealer (tested locally using Docker: https://github.com/paritytech/parity-deploy/pull/82 ). The implementation is awaiting review and further testing.

Learn more on the Gitcoin Issue Details page.

thefallentree commented 5 years ago

this part is mostly in the code now, and it also heavily involved with other clique code,. I don't think it should/is possible to have someone else independently working on this.

However, I really would like to see an refactoring of consensus module from other part of the parity codebase. the current consensus engine interface is very confusing, error prone and due to the crate layout, its compile time is ridiculous long and an change basically force everything else to recompile, would love to see someone understand and propose an new interface instead.

It would be wonderful to be able to write/compile/test consensus engine independently from whole client and better unit integration support.

5chdn commented 5 years ago

Yes, I give this @jwasinger who already started working on this.

gitcoinbot commented 5 years ago

@jwasinger Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

ghost commented 5 years ago

@thefallentree

However, I really would like to see an refactoring of consensus module from other part of the parity codebase. the current consensus engine interface is very confusing, error prone and due to the crate layout, its compile time is ridiculous long and an change basically force everything else to recompile, would love to see someone understand and propose an new interface instead.

It would be wonderful to be able to write/compile/test consensus engine independently from whole client and better unit integration support.

If I understood you correct, this should be the same that I mentioned as "Task B" :

https://github.com/goerli/testnet/issues/9#issuecomment-457967529

I've not yet looked at the code, but I expect that this would take weeks to be completed. The result would be of course that the effort for newly implemented engines would be reduced. Not to mention the benefit of easier maintenance.

thefallentree commented 5 years ago

It wasn't that bad. I think it's at most a weekend's work. I plan on doing that after I finish the code anyway.

gitcoinbot commented 5 years ago

@jwasinger Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

thefallentree commented 5 years ago

working sealing is implemented in 100b149d87fe225a7402205011a7ace8d665486c and the released in clique-v4

There are a few TODO here:

and I'm actually working on it

jwasinger commented 5 years ago

Sealing was working around https://github.com/goerli/parity-goerli/commit/abc625db5ecff5f5d17caed33098532afc21b9f9 before the refactor.

Implement wiggle delay for signing NOTURN blocks

This was already implemented pre-refactor inside generate_seal : https://github.com/goerli/parity-goerli/blob/abc625db5ecff5f5d17caed33098532afc21b9f9/ethcore/src/engines/clique/mod.rs#L359

I haven't really had time to look at the code after the refactor but I guess a lot of stuff needs to be re-implemented.

thefallentree commented 5 years ago

Sealing was working around abc625d before the refactor.

Implement wiggle delay for signing NOTURN blocks

This was already implemented pre-refactor inside generate_seal :

parity-goerli/ethcore/src/engines/clique/mod.rs

Line 359 in abc625d

if state.turn_delay(&block.header) { I haven't really had time to look at the code after the refactor but I guess a lot of stuff needs to be re-implemented.

don't worry, I have reimplemneted it in latest change.

thefallentree commented 5 years ago

Wiggle and Vote casting are implemented in dbf54d71f3006cb4bfb729e69a2dd1ac834a2feb and df1a38b3957e3b57cbce8f491a5949a2a507741e

5chdn commented 5 years ago

Thanks. I will try to setup a network and test sealing these days

5chdn commented 5 years ago

screenshot at 2019-02-07 14-06-10

5chdn commented 5 years ago

This works for me. Two Parity Ethereum sealers and one Geth syncing the chain.

There is one hiccup though, Geth seems to reject the first block with:

WARN [02-07|14:04:42.856] Discarded bad propagated block           number=1 hash=b8cd8a…19d6c7
WARN [02-07|14:04:55.469] Discarded bad propagated block           number=1 hash=b8cd8a…19d6c7
5chdn commented 5 years ago

But afterwards this accepts the chain (after block #2)

~/.opt/parity-goerli master*
❯ geth --datadir /tmp/g0 --networkid 106                
INFO [02-07|13:53:46.050] Maximum peer count                       ETH=25 LES=0 total=25
INFO [02-07|13:53:46.053] Starting peer-to-peer node               instance=Geth/v1.8.21-stable-9dc5d1a9/linux-amd64/go1.11.4
INFO [02-07|13:53:46.054] Allocated cache and file handles         database=/tmp/g0/geth/chaindata cache=512 handles=262144
INFO [02-07|13:53:46.074] Initialised chain configuration          config="{ChainID: 106 Homestead: 0 DAO: <nil> DAOSupport: false EIP150: 0 EIP155: 0 EIP158: 0 Byzantium: 0 Constantinople: 0 Engine: clique}"
INFO [02-07|13:53:46.074] Initialising Ethereum protocol           versions="[63 62]" network=106
INFO [02-07|13:53:46.146] Loaded most recent local header          number=0 hash=7589a4…14386f td=1 age=49y9mo3w
INFO [02-07|13:53:46.146] Loaded most recent local full block      number=0 hash=7589a4…14386f td=1 age=49y9mo3w
INFO [02-07|13:53:46.146] Loaded most recent local fast block      number=0 hash=7589a4…14386f td=1 age=49y9mo3w
INFO [02-07|13:53:46.147] Regenerated local transaction journal    transactions=0 accounts=0
INFO [02-07|13:53:46.148] Stored checkpoint snapshot to disk       number=0 hash=7589a4…14386f
INFO [02-07|13:53:46.158] New local node record                    seq=1 id=992583e250f129eb ip=127.0.0.1 udp=30303 tcp=30303
INFO [02-07|13:53:46.161] Started P2P networking                   self=enode://1a84804231dbb8e6bc8e6feb6356381938bd81f6184da80ac460bfec454558235f8fd9183d135539467197deafad2d8b972d50e3897636a83438479357902e93@127.0.0.1:30303
INFO [02-07|13:53:46.165] IPC endpoint opened                      url=/tmp/g0/geth.ipc
INFO [02-07|13:53:47.362] New local node record                    seq=2 id=992583e250f129eb ip=89.245.190.29 udp=30303 tcp=30303
2019/02/07 13:53:48 ssdp: got unexpected search target result "upnp:rootdevice"
2019/02/07 13:53:48 ssdp: got unexpected search target result "upnp:rootdevice"
2019/02/07 13:53:48 ssdp: got unexpected search target result "upnp:rootdevice"
2019/02/07 13:53:48 ssdp: got unexpected search target result "upnp:rootdevice"
2019/02/07 13:53:48 ssdp: got unexpected search target result "upnp:rootdevice"
2019/02/07 13:53:48 ssdp: got unexpected search target result "upnp:rootdevice"
WARN [02-07|14:04:42.856] Discarded bad propagated block           number=1 hash=b8cd8a…19d6c7
WARN [02-07|14:04:55.469] Discarded bad propagated block           number=1 hash=b8cd8a…19d6c7
INFO [02-07|14:05:01.574] Block synchronisation started 
INFO [02-07|14:05:01.577] Imported new block headers               count=2 elapsed=795.635µs number=2 hash=8de614…29fb8f
INFO [02-07|14:05:01.578] Imported new chain segment               blocks=2 txs=0 mgas=0.000 elapsed=499.541µs mgasps=0.000 number=2 hash=8de614…29fb8f cache=0.00B
INFO [02-07|14:05:01.579] Fast sync complete, auto disabling 
INFO [02-07|14:05:20.368] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=395.145µs mgasps=0.000 number=3 hash=c64786…906ce7 cache=0.00B
INFO [02-07|14:05:39.086] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=362.303µs mgasps=0.000 number=4 hash=e0cefe…8347c6 cache=0.00B
INFO [02-07|14:05:57.878] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=390.749µs mgasps=0.000 number=5 hash=9ccf2b…61dadf cache=0.00B
INFO [02-07|14:06:16.595] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=335.092µs mgasps=0.000 number=6 hash=53e2cd…91e8d2 cache=0.00B
INFO [02-07|14:06:35.386] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=314.669µs mgasps=0.000 number=7 hash=907336…78d99e cache=0.00B
INFO [02-07|14:06:54.111] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=365.305µs mgasps=0.000 number=8 hash=283a62…224b73 cache=0.00B
INFO [02-07|14:07:12.895] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=1.146ms   mgasps=0.000 number=9 hash=a34c4e…fb92ad cache=0.00B
INFO [02-07|14:07:32.121] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=336.254µs mgasps=0.000 number=10 hash=364cd2…2b7f79 cache=0.00B
INFO [02-07|14:07:50.409] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=283.138µs mgasps=0.000 number=11 hash=df8540…c5ad5e cache=0.00B
INFO [02-07|14:08:09.633] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=302.236µs mgasps=0.000 number=12 hash=107de8…f21144 cache=0.00B
5chdn commented 5 years ago

Parity Sealer:

~/.opt/parity-goerli master* 2m 38s
❯ target/release/parity --base-path /tmp/p2 --chain ethcore/res/clique.json --ports-shift 200 --force-sealing --engine-signer 0x13fe6c62876ceab7c1bd48453ebdae52827f1911 --password /tmp/pass --bootnodes enode://1a84804231dbb8e6bc8e6feb6356381938bd81f6184da80ac460bfec454558235f8fd9183d135539467197deafad2d8b972d50e3897636a83438479357902e93@127.0.0.1:30303,enode://469b91843186ec08fad1f8140eb7297f654a70c8c3f7b8358014b45bc359366516c4ce1617659ba251365cb85d2253c399e25c9579671084f8645434487ede5a@192.168.1.135:30403,enode://434835bf9a8bab1a61f8c91d0ac6e70abeb49f2d68f9abed1ad4bb1b397af8f6591625914a6d2e8a15a7c095d22fc5d401aef34921bd84abd330a9b3524506e4@192.168.1.135:30503
2019-02-07 14:04:54  Starting Parity-Ethereum/v2.4.0-nightly-97303717d-20190129/x86_64-linux-gnu/rustc1.32.0
2019-02-07 14:04:54  Keys path /tmp/p2/keys/clique
2019-02-07 14:04:54  DB path /tmp/p2/chains/clique/db/599db7c0ea3b4096
2019-02-07 14:04:54  State DB configuration: fast
2019-02-07 14:04:54  Operating mode: active
2019-02-07 14:04:54  Configured for TestClique using Clique engine
2019-02-07 14:04:54  Public node URL: enode://434835bf9a8bab1a61f8c91d0ac6e70abeb49f2d68f9abed1ad4bb1b397af8f6591625914a6d2e8a15a7c095d22fc5d401aef34921bd84abd330a9b3524506e4@192.168.1.135:30503
2019-02-07 14:04:54  Listening for new connections on 127.0.0.1:8746.
2019-02-07 14:05:01  Imported #2 0x8de6…fb8f (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB) + another 1 block(s) containing 0 tx(s)
2019-02-07 14:05:20  Imported #3 0xc647…6ce7 (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:05:24     2/25 peers   9 KiB chain 8 KiB db 0 bytes queue 37 KiB sync  RPC:  0 conn,    0 req/s,    0 µs
2019-02-07 14:05:39  Imported #4 0xe0ce…47c6 (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:05:54     2/25 peers   9 KiB chain 8 KiB db 0 bytes queue 37 KiB sync  RPC:  0 conn,    0 req/s,    0 µs
2019-02-07 14:05:57  Imported #5 0x9ccf…dadf (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:06:16  Imported #6 0x53e2…e8d2 (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:06:24     2/25 peers   9 KiB chain 8 KiB db 0 bytes queue 37 KiB sync  RPC:  0 conn,    0 req/s,    0 µs
2019-02-07 14:06:35  Imported #7 0x9073…d99e (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:06:54  Imported #8 0x283a…4b73 (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:06:54     2/25 peers   9 KiB chain 8 KiB db 0 bytes queue 37 KiB sync  RPC:  0 conn,    0 req/s,    0 µs
2019-02-07 14:07:12  Imported #9 0xa34c…92ad (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:07:24     2/25 peers   11 KiB chain 9 KiB db 0 bytes queue 37 KiB sync  RPC:  0 conn,    0 req/s,    0 µs
2019-02-07 14:07:32  Imported #10 0x364c…7f79 (0 txs, 0.00 Mgas, 1 ms, 0.59 KiB)
2019-02-07 14:07:50  Imported #11 0xdf85…ad5e (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:07:54     2/25 peers   11 KiB chain 9 KiB db 0 bytes queue 37 KiB sync  RPC:  0 conn,    0 req/s,    0 µs
2019-02-07 14:08:09  Imported #12 0x107d…1144 (0 txs, 0.00 Mgas, 1 ms, 0.59 KiB)
2019-02-07 14:08:24     2/25 peers   11 KiB chain 9 KiB db 0 bytes queue 37 KiB sync  RPC:  0 conn,    0 req/s,    0 µs
2019-02-07 14:08:27  Imported #13 0xa14b…a9da (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:08:47  Imported #14 0x12f6…b35e (0 txs, 0.00 Mgas, 0 ms, 0.59 KiB)
2019-02-07 14:08:54     2/25 peers   11 KiB chain 9 KiB db 0 bytes queue 37 KiB sync  RPC:  0 conn,    0 req/s,    0 µs
5chdn commented 5 years ago

Good work. Next step would be sealing on Görli and see how this works out.

This is my test genesis and chainspec: 9dbe5b34593d1b0de3698d57f794b499452f42e2

thefallentree commented 5 years ago

@5chdn is this same issue with https://github.com/ethereum/go-ethereum/issues/14945 ?

5chdn commented 5 years ago

Yes, it is! Otherwise it looks good! Was running for 24h without any issues.

gitcoinbot commented 5 years ago

@jwasinger Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

@jwasinger due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

jwasinger commented 5 years ago

Hi, I'm not sure how to proceed. Significant work towards sealing has been contributed by @thefallentree and myself. As such, it doesn't really make sense to have this whole bounty assigned to one participant.

thefallentree commented 5 years ago

I'm addressing the final PR comments, so this will be done within next week

5chdn commented 5 years ago

@thefallentree @jwasinger I will pay you both a significant part of the parity clique bounty before I hand over this project to MP/ChainSafe. I'm sorry for the lack of communication recently but it was a rough ride.

Please confirm that you are consent with paying out the bounty via Gitcoin? This is easiest for me, so all I need is your Github account to send it.

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Cancelled

The funding of 2500.0 DAI (2500.0 USD @ $1.0/DAI) attached to this issue has been cancelled by the bounty submitter

thefallentree commented 5 years ago

@5chdn yeah that works for me.

jwasinger commented 5 years ago

@5chdn Likewise, that works for me. You should have received my new address. Let me know if you didn't.

gitcoinbot commented 5 years ago

⚡️ A tip worth 7500.00000 DAI (7500.0 USD @ $1.0/DAI) has been granted to @thefallentree for this issue from @5chdn. ⚡️

Nice work @thefallentree! Your tip has automatically been deposited in the ETH address we have on file.

gitcoinbot commented 5 years ago

⚡️ A tip worth 7500.00000 DAI (7500.0 USD @ $1.0/DAI) has been granted to @jwasinger for this issue from @5chdn. ⚡️

Nice work @jwasinger! To redeem your tip, login to Gitcoin at https://gitcoin.co/explorer and select 'Claim Tip' from dropdown menu in the top right, or check your email for a link to the tip redemption page.

5chdn commented 5 years ago

:+1:

jwasinger commented 5 years ago

:+1:

thefallentree commented 5 years ago

I have reservations about the split here. @5chdn can we chat?

mariapaulafn commented 5 years ago

Hello there @thefallentree, I'm MP, one of the three custodians. I am trying to help all parts be aligned. you can talk to me. What's the best way to reach you? Also, would you please confirm you have been compensated accordingly for your work in the last few days?

jwasinger commented 5 years ago

@thefallentree If you are implying that I deserve less money for the work I have done, then I respectfully disagree.

During this whole process, you have not been forthcoming in communication around code design and I feel like some of the refactoring was "forced" without actually having a discussion about it. When a refactor requires re-implementation of features that were previously working and introduces new issues, that creates additional work (see my DM regarding the consensus issue). Your refactor around sealing created issues and I disagreed with the design.

I wanted to collaborate but you have made that excessively difficult. I cannot continue to hit a moving target if you dictate the direction without communication. I am happy with the amount I have received. The hours I have invested into this project are fairly reflected in that number.

Jared

mariapaulafn commented 5 years ago

Hey @jwasinger I dont think @thefallentree has said anything about you, but I'm committed to dig deeper. You have been compensated accordingly, and you deserve every DAI. We consider both of you valuable collaborators, and this is why we have been paying special attention to this issue.

Of course we cannot weigh into your internal issues, and understand your PoV, Jared, and if its lack of communication what's preventing you from collaborating, we'll find you a new opportunity, and talk with Yucong separately to hear his concerns.

Nevertheless, both parties are accordingly compensated, and Görli will not change anybody's compensation based on statements of others.

jwasinger commented 5 years ago

Yeah, I have my issues with the communication around design. That being said, @thefallentree has been pushing very hard on this project and is a very important asset to Goerli.

ghost commented 5 years ago

When a refactor requires re-implementation

then it's not a refactoring. Refactoring is an Art, which can usually be performed only by "naturals". All others mess it up, and start reDESING / reIMPLEMENTATION.

See, when I personally perform "The Art of Refactoring" (tiny steps, on code which is in production), then it is mostly sub-consciously driven: I cannot explain steps, cannot discuss, am kind of a semi-autistic lab-rat.

having said that, I have reservations about the split, too:

I should get the 15K for https://github.com/goerli/parity-goerli/issues/50, the 2 devs here should split my 1K !!!

$^%&#%^^@(&^(&

.

roninkaizen commented 5 years ago

Hello @all

to state honestly, i wrote with yucong before understanding some but not all, knowing both devs from reading, knowing about "money-questions" about this, i personally would suggest a "participating-parties-vote"- in the future (and only if needed) background: i use the software (clique.v5), as the only visible person running parity on one of the stats- asking kind "is my voice also worth to be heard within this discussion?" both commited work to the code, how much worthful each "participation" of both of them was, in its results it lead to a valid parity-codebase, which allowed it for me to participate within goerli- i wanted to state thankfulness to both of you, and insist in finding a better future solution for such questions.

ghost commented 5 years ago

...kaizen, you runied my closing-joke!!!

As for voting: this is for puppies.

A fight to the death, winner takes it all!

thefallentree commented 5 years ago

@lazaridiscom @jwasinger I choose to not debat and furtuer your obvious trolling. However I will still talk with @mariapaulafn and be sure my case is preseneted clearly. I will reconsider if in the future I still want to invovle in goerli project after that.

And congrats you two just made to my new blocked user list. Cheers.

ghost commented 5 years ago

a) Try "Thank you for the payment" as an intro, next time. b) If I wanted to troll, I would "attack" your photo (but me civilized, not doing such things...) c) The fact that you committed more code does not mean that you worked more that your co-worker. An analytical mind usually collapses by "moving targets" - that's my experience. So, its easy to "knock out" a co-dev (intentional or not), simply by checkin in changes fast. d) why would you even mention that you blocked folks? Get serious, please!

Next time just negotiated clearly before(!) you start work, thus you won't need to have "reservations" afterwards.

And may I remind you that this is a public project, with a public grant and public processing:

I protest that this is discussed in private.

I'll die from curiosity!!!

.

jwasinger commented 5 years ago

It's a shame that my attempt to engage in honest conversation was instantly shut down. But I guess I can't expect any less given our past history of "collaboration".

ghost commented 5 years ago

No worries. At the end, even lava cools down.

mariapaulafn commented 5 years ago

@lazaridiscom please drop this chat. you have no say on this conversation and are distracting the real parties that need to express their opinion, this is a warning.

mariapaulafn commented 5 years ago

I think both @jwasinger and @thefallentree have expressed fair concerns, and also jared's been open to communication. Thank you both, and we will make sure to hear all voices. That said, please avoid tagging each other as trolling. None of you are trolls, but contributors, and we expect decorum and to avoid name calling here. That's final. Appreciate @roninkaizen's perspective as well.

The decision to fund both contributors alike stands, and will not change. I ask you to stop focusing on each other's compensation and contributions, and focus on your own. We assessed this fairly and YES, by accepting your compensation it means you accept everything else. If you are not happy with something, then choose to reject all, and we'll listen.

ghost commented 5 years ago

Naturally, I have a say here. The fact that you fail to see it does not change that.

Funny thing, what a little bit authority and power can do to people.

Warn as much as you want, couldn't care less!

.

mariapaulafn commented 5 years ago

@lazaridiscom there are two people here with issues, and i am trying to mediate. you are trying to sensationalize and enjoy the show. How does this give you a say in a 3 people conversation, I don't understand. Don't stir the fire and respect the people that are trying to resolve their conflicts. You are only adding noise and stress here.

ghost commented 5 years ago

I don't understand

Obviously.

jwasinger commented 5 years ago

@mariapaulafn you are wasting your time. Trolls gonna troll.