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

Candidate agreement "glue" code #34

Merged
merged 56 commits into from
Jan 23, 2018
Merged

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Jan 10, 2018

Closes #28

The previous PR (#29) implemented the core primitives for achieving consensus. This one wraps them one step further by

  • determining when to handle incoming messages
  • crafting availability and validity votes for incoming candidates asynchronously
  • creating new proposals out of viable candidate-sets periodically
  • creating batches of statements to send to other authorities (currently done naively, this is an easy low-hanging fruit for optimization: For Polkadot: Optimize candidate statement routing #36)

The next consensus PR will combine the networking code and the runtime from #32 with the code in the candidate-agreement module.

@rphmeier rphmeier added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jan 10, 2018
@rphmeier rphmeier added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jan 16, 2018
local_candidate
.into_future()
.map(table::Statement::Candidate)
.map(Some)
Copy link
Member

Choose a reason for hiding this comment

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

Are these two lines moot or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's necessary for the or_else in the next line which is currently used to swallow errors in candidate production.

let table = params.table;
params.timer.interval(params.form_proposal_interval)
.map_err(|_| Error::FaultyTimer)
.for_each(move |_| { table.update_proposal(); Ok(()) })
Copy link
Member

Choose a reason for hiding this comment

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

could be written .for_each(move |_| Ok(table.update_proposal())) which is a bit more concise.

Copy link
Member

@gavofyork gavofyork left a comment

Choose a reason for hiding this comment

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

quality looks good but haven't done deep review of logic.

@gavofyork gavofyork merged commit d477819 into master Jan 23, 2018
@gavofyork gavofyork deleted the rh-candidate-agreement-glue branch January 23, 2018 13:42
JoshOrndorff referenced this pull request in moonbeam-foundation/substrate Apr 21, 2021
Fix serialize-javascript vulnerability alert
imstar15 pushed a commit to imstar15/substrate that referenced this pull request Jan 3, 2023
…xecution-benchmarks

introduce compile time switch for omitting extrinsics execution
helin6 pushed a commit to boolnetwork/substrate that referenced this pull request Jul 25, 2023
…ech#34)

* Update default runtime types to match latest substrate

* Format code
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.

2 participants