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

aura: finalize blocks #9692

Merged
merged 8 commits into from
Oct 25, 2018
Merged

aura: finalize blocks #9692

merged 8 commits into from
Oct 25, 2018

Conversation

andresilva
Copy link
Contributor

Reopen stale #9113. I couldn't reopen #9113 since I rebased the branch and force-pushed.

  • The Aura engine was updated to emit ancestry actions to finalize blocks
  • The full client stores block finality in the database, the engine builds finality from an ancestry of ExtendedHeader
  • is_epoch_end was updated to take a vec of recently finalized headers
  • is_epoch_end_light was added which maintains the previous interface and is used by the light client since the client itself doesn't track finality

The full client now tracks finality by querying the engine on each block import,
and it also persists the finalization state to the DB. For the light client
current it doesn't persist finality information and only keeps track of finality
for epoch signals, by calling `is_epoch_end_light`. This method implements the
previously existing logic of building finality for all the blocks in the current
epoch and then checking the finalized blocks against the transition store.
- missing docs for is_epoch_end_light
- unused method unfinalized_hashes in RollingFinality
@andresilva andresilva added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Oct 3, 2018
@5chdn 5chdn added this to the 2.2 milestone Oct 3, 2018
Copy link
Contributor

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

In general LGTM. Is it possible to add a couple of tests here?

@ascjones ascjones added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 23, 2018
};

let ancestry_iter = ancestry.map(|header| {
let mut signers = vec![header.author().clone()];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let mut signers = vec![header.author().clone()];
let mut signers = vec![*header.author()];

}
}

let finalized = epoch_manager.finality_checker.push_hash(chain_head.hash(), vec![chain_head.author().clone()]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let finalized = epoch_manager.finality_checker.push_hash(chain_head.hash(), vec![chain_head.author().clone()]);
let finalized = epoch_manager.finality_checker.push_hash(chain_head.hash(), vec![*chain_head.author()]);

if !epoch_manager.zoom_to(&*client, &self.machine, &*self.validators, chain_head) {
return None;
}
let mut hash = chain_head.parent_hash().clone();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let mut hash = chain_head.parent_hash().clone();
let mut hash = *chain_head.parent_hash();

let mut ancestry = itertools::repeat_call(move || {
chain(hash).and_then(|header| {
if header.number() == 0 { return None }
hash = header.parent_hash().clone();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
hash = header.parent_hash().clone();
hash = *header.parent_hash();

Copy link
Collaborator

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

Looks good to me, nitpick clone on copy-type but I'm not that familiar with this part of codebase so probably good with another review!

@ascjones ascjones merged commit e7f1204 into master Oct 25, 2018
@ascjones ascjones deleted the andre/aura-finalize-blocks branch October 25, 2018 15:33
@5chdn 5chdn added the B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. label Oct 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants