Process finality proofs in solidity PoA -> Substrate contract #69

svyatonik · 2020-04-20T15:44:00Z

Based on top of #65. Right now this PR has the single commit.

There's a lot of things to do - I'm opening it early to discuss things that must be discussed.

A brief overview of TODOs:

Discuss: how should we verify grandpa justifications - write custom code in builtin? Extract GrandpaJustification::decode_and_verify() in separate crate? Make Substrate RPC for that?
Discuss: do we want to support other kinds of finality proofs? Like Commit? Do we need custom RPCs for that? Create issue, if required.
Extract runtime changes to separate PR;
Support forced validators set changes (or at least create issue for that - logged Support forced GRANDPA validators change #76 )
Provide tests, where possible

HCastano · 2020-05-08T02:43:20Z

I've gone through this again, and I'm still struggling to fully understand the changes in headers.rs. I realize that the root of my problem is that I don't understand what it means for a header to be complete or incomplete. Can you clarify this for me Slava?

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

svyatonik · 2020-05-08T08:16:39Z

I've gone through this again, and I'm still struggling to fully understand the changes in headers.rs. I realize that the root of my problem is that I don't understand what it means for a header to be complete or incomplete. Can you clarify this for me Slava?

Most of Substrate headers are "complete". The only case when header is "incomplete", is when we it requires justification (i.e. it is the header that enacts new GRANDPA authorities set). This requirement is tracked by the contract. So when contract accepts "incomplete" header, it stops accepting its descendants, until justification is submitted.

tomusdrw

lgtm! @HCastano any outstanding issues you'd like to see addressed?

modules/ethereum-contract/builtin/src/lib.rs

relays/ethereum/src/headers.rs

HCastano · 2020-05-10T19:33:32Z

relays/ethereum/src/headers.rs

@@ -411,6 +587,14 @@ fn remove_header<P: HeadersSyncPipeline>(
 	header
 }

+/// Get header from the qeueue.
+fn header<'a, P: HeadersSyncPipeline>(


Suggested change

fn header<'a, P: HeadersSyncPipeline>(

fn get_header_from_queue<'a, P: HeadersSyncPipeline>(

I'm suggesting this since when I was reading some of the code that was like HeaderStatus::MaybeOrphan => header(...) it wasn't clear at first glance that header() was fetching stuff from the queues in the same way that seeing queue.get(id) would be.

relays/ethereum/src/headers.rs

HCastano · 2020-05-10T19:57:16Z

I've gone through this again, and I'm still struggling to fully understand the changes in headers.rs. I realize that the root of my problem is that I don't understand what it means for a header to be complete or incomplete. Can you clarify this for me Slava?

Most of Substrate headers are "complete". The only case when header is "incomplete", is when we it requires justification (i.e. it is the header that enacts new GRANDPA authorities set). This requirement is tracked by the contract. So when contract accepts "incomplete" header, it stops accepting its descendants, until justification is submitted.

Thanks for the clarification. I think you should add this (or some version of it) as a comment to the HeaderCompletion struct. I also think that the associated Completion types could have a short explanation (e.g the Ethereum Completion type is () because it doesn't not require any sort of Justification).

HCastano · 2020-05-10T19:57:54Z

I've left a few comments, mainly just typo fixes and re-naming suggestions, nothing major :)

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

@s3krit

This action will publish all pre-released, edited and published releases to a dedicated release notes Matrix room using https://github.com/marketplace/actions/matrix-message @s3krit already added the necessary secrets to the repository settings.

…tech#69) * solidity contract * continue * upd * cargo update * fixes * ehtereum_headers -> headers * extracted some common stuff * ethereum_sync.rs -> sync.rs * make sync generic * continue extracting * continue * add eth-contract argument * continue * some fixes * contract v2 * continue * more fixes * more fixes * deal with duplicated params * removed multiple call_rpc variants * bail_on_error!() * fn submit_ethereum_transaction * more fixes * cargo fmt --all * fix * bail_on_arg_error!() * fix * fix * remove async_extra stuff * start work on finality builtin remove async_extra stuff continue continue local testnet (Alice + Bob) for node * added TODO * substrate-bridge.json -> substrate-bridge-abi.json * get rid of substrate transactions hashes * get rid of ethereum transactions hashes * extracted contract bytecode to separate file * cargo fmt --all * avoid duplicate import in contracts * removed Default::default() * swapped configurations for sub2eth && eth2sub * fix compilation * do not double gas limit when submitting Substrate headers * fix finality storage * at least 1 validator required * shift_session_manager_works * cargo fmt --all * solidity contract removed * consts * extracted solc compilation details to separate file * removed (obsolete in future Vec<u8> justification) * fixed cli option description * fix typos * fix grumble * extracted constants * log decoded header * new substrate version + actually verify justification * intermediate cargo fmt --all * comments * disable completion data resubmission * increased timeouts + _MS -> Duration * forget completion data after submission * builtin tests * headers tests * cargo fmt --all * update contract * Update relays/ethereum/src/ethereum_sync_loop.rs Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * Update relays/ethereum/src/ethereum_sync_loop.rs Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * added docs * OwnedFutureOutput * more docs fixes * cargo fmt --all * encode headers * consts + docs * aliases again * cargo fmt --all * Update relays/ethereum/src/ethereum_sync_loop.rs Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * Update relays/ethereum/src/ethereum_sync_loop.rs Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * Use Duration::from_secs() instead of from_millis() * grumbles * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * incomplete_headers_are_still_incomplete_after_advance * add hex-encoded headers to substrate_header_without_signal_parsed * cargo fmt --all * Update relays/ethereum/src/sync_loop.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * Update relays/ethereum/src/headers.rs Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com> * added comments on Extra and Completion Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

svyatonik added 30 commits April 3, 2020 16:59

solidity contract

dbd7f0e

continue

a9d58a0

upd

6995aac

cargo update

7a55f48

fixes

f7e9768

ehtereum_headers -> headers

3a13d1f

extracted some common stuff

39555d0

ethereum_sync.rs -> sync.rs

5831990

make sync generic

4dfffd8

continue extracting

33e9e31

Merge branch 'master' into solidity-contract

e132276

continue

4ec3f6f

add eth-contract argument

bf77451

continue

1312d6e

some fixes

cfc9235

contract v2

b73aa54

continue

f29b2e5

more fixes

d90dff3

Merge branch 'master' into solidity-contract

eb07871

more fixes

ac393da

deal with duplicated params

abebf1a

removed multiple call_rpc variants

8168278

bail_on_error!()

fd02e2f

fn submit_ethereum_transaction

e15bd1f

more fixes

4fd2cff

cargo fmt --all

ea6cc8c

fix

dbda454

bail_on_arg_error!()

74cd275

fix

84443a1

fix

8118db6

Update relays/ethereum/src/sync_loop.rs

0798757

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

tomusdrw approved these changes May 8, 2020

View reviewed changes

modules/ethereum-contract/builtin/src/lib.rs Show resolved Hide resolved