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

Light GRANDPA import handler #1669

Merged
merged 58 commits into from
May 13, 2019
Merged

Light GRANDPA import handler #1669

merged 58 commits into from
May 13, 2019

Conversation

svyatonik
Copy link
Contributor

closes #269

I'm opening this PR to share the ideas && to make sure I'm on the right track. So the general idea is:

  1. we do not pause syncing when see a block with consensus-related changes (which is just DigestItem::AuthoritiesChange now). This differs from original description of Light-client Authority Set Handoffs #269 and what has been discussed in GRANDPA finality proof draft #1268. But since full clients do not pause sync, I'm not sure - why light clients should (that's what bothering me). So light block importer only tries to get finalization of such blocks ASAP;
  2. getting finalization = checking justification. To check justification, we need to know GRANDPA authorities set. One possible option is to use existing finality proof every time, but I think I've found a better way:
    2.1) assuming that every change of this set if accompanied with justification;
    2.2) when seeing a justification we could assume that it is created with previous set;
    2.3) if check-with-previous-set fails, we're fetching GRANDPA authorities set from the remote full node;
    2.4) if recheck-with-new-set works, then we're finalizing the block + storing new set + new set_id (which is increased by one on every change) locally

This should work when justifications are already created (i.e. when we're syncing from the scratch). But there's a case when this strategy will fail:

  1. B1 which enacts new GRANDPA authorities set is imported on FULL client without justification (let's say it changes set_id from 10 to 11);
  2. FULL announces this block to LIGHT && light imports the block without finalization (set_id is still 10 on light);
  3. FULL creates justification J1 for B1, but it doesn't reach LIGHT yet;
  4. B2 which updates set_id from 11 to 12 is imported by FULL && justification J2 is created;
  5. light client tries to import B2+J2 && import fails, because it has missed 10 -> 11 change of set_id.

That's where I suppose finality proof could help (if changed a bit). So that new finality proof (which will work in both optimistic + pessimistic cases) will be:
REQUEST: prove finalization of BEST_BLOCK given that our best finalized is BEST_FINALIZED
RESPONSE:
    assume that we have best justification for BEST_BLOCK+2
    if GRANDPA set has NOT been changed in [BEST_FINALIZED; BEST_BLOCK+2]:
        we only need to respond with justification for BEST_BLOCK+2
    if GRANDPA set has been changed in that range:
        we need to respond with justifications for all intermediate changes
        + proof-of-execution for GRANDPA::authorities for all intermediate changes
        + justification for BEST_BLOCK+2

The goal of this PR is to have:

  1. light client that is able to import GRANDPA justifications;
  2. light client that is able to synchronize with minimal # of calls into runtime (i.e. only when proofs are required):
    2.1) I've reenabled authorities cache for that => when Aura verifier requests authorities, they're fetched from the cache;
    2.2) I've added extraction of new_authorities into Aura verifier => light client is notified when the set changes && the cache is updated accordingly.

cc @rphmeier

@svyatonik svyatonik added the A3-in_progress Pull request is in progress. No review needed at this stage. label Feb 4, 2019
@svyatonik
Copy link
Contributor Author

depends on #1603 - can't make tests with current test_client implementation

@rphmeier
Copy link
Contributor

rphmeier commented Feb 15, 2019

@svyatonik can we ice this until the offline-fallback is in? Mostly because I want to make sure that some things are handled correctly for light clients (that they are actually halting in case of a forced change.)

@svyatonik
Copy link
Contributor Author

@rphmeier Sure! Depends on #1619

@svyatonik svyatonik added the A0-please_review Pull request needs code review. label Apr 18, 2019
@pepyakin
Copy link
Contributor

@svyatonik this requires conflict fixes.

@rphmeier / @andresilva can you have another look at this?

@gavofyork
Copy link
Member

@svyatonik more conflicts to be resolved...

@svyatonik
Copy link
Contributor Author

testing blocked by #2499 + I have found some verification issue - the fix will be here

@svyatonik svyatonik added A4-gotissues and removed A0-please_review Pull request needs code review. labels May 7, 2019
@andresilva
Copy link
Contributor

Could you merge master to include #2499 and fix conflicts?

@svyatonik
Copy link
Contributor Author

@andresilva Will do, but there's (unfortunately) another major issue - I'm currently preparing separate PR for that && will leave this PR as gotissues until we will have that PR merged in.

@tomaka
Copy link
Contributor

tomaka commented May 9, 2019

This conflicts quite heavily with my WIP for a networking cleanup. I'm going to wait for this to be in to continue.

@svyatonik
Copy link
Contributor Author

@andresilva @tomaka Sorry for delay - I've merged with master (had the same "too many files open problem" issue - we need to figure out the reason). This is ready for review again, but if you want to do a real testing, you should also cherry-pick commits from #2512 (the Aura part was broken, the GRANDPA looks OK to me).

@svyatonik svyatonik added A0-please_review Pull request needs code review. and removed A4-gotissues labels May 10, 2019
@gavofyork
Copy link
Member

going to need resolving...

@gavofyork gavofyork merged commit 2b24101 into master May 13, 2019
@gavofyork gavofyork deleted the light_grandpa_importer2 branch May 13, 2019 09:36
MTDK1 pushed a commit to bdevux/substrate that referenced this pull request Jul 10, 2019
* GrandpaLightBlockImport

* extract authorities in AuraVerifier

* post-merge fix

* restore authorities cache

* license

* new finality proof draft

* generalized PendingJustifications

* finality proof messages

* fixed compilation

* pass verifier to import_finality_proof

* do not fetch remote proof from light import directly

* FinalityProofProvider

* fixed authorities cache test

* restored finality proof tests

* finality_proof docs

* use DB backend in test client

* justification_is_fetched_by_light_client_when_consensus_data_changes

* restore justification_is_fetched_by_light_client_when_consensus_data_changes

* some more tests

* added authorities-related TODO

* removed unneeded clear_finality_proof_requests field

* truncated some long lines

* more granular light import tests

* only provide finality proof if it is generated by the requested set

* post-merge fix

* finality_proof_is_none_if_first_justification_is_generated_by_unknown_set

* make light+grandpa test rely on finality proofs (instead of simple justifications)

* empty_finality_proof_is_returned_to_light_client_when_authority_set_is_different

* missing trait method impl

* fixed proof-of-finality docs

* one more doc fix

* fix docs

* initialize authorities cache (post-merge fix)

* fixed cache initialization (post-merge fix)

* post-fix merge: fix light + GRANDPA tests (bad way)

* proper fix of empty_finality_proof_is_returned_to_light_client_when_authority_set_is_different

* fixed easy grumbles

* import finality proofs in BlockImportWorker thread

* allow import of finality proofs for non-requested blocks

* limit number of fragments in finality proof

* GRANDPA post-merge fix

* BABE: pos-merge fix
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Light-client Authority Set Handoffs
6 participants