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

Implementers' Guide: Chain Selection #3262

Merged
8 commits merged into from
Jun 17, 2021
Merged

Implementers' Guide: Chain Selection #3262

8 commits merged into from
Jun 17, 2021

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Jun 15, 2021

  1. high-level discussion of fork-choice and chain selection
  2. Chain selection subsystem description (high-level)
  3. Update of the grandpa-voting-rule document

Based on discussions and experience of the implementers' guide in the past, I avoided specifying the subsystem in too much low-level detail. Although those types of descriptions are helpful for handing off code to be implemented, they lead to often-tedious and overlooked guide updates when making minor edits to the code after the initial implementation is done. The higher-level description taken here should allow for a lot of flexibility and further changes of implementation without those types of edits being made.

@rphmeier rphmeier added A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Jun 15, 2021
@rphmeier rphmeier marked this pull request as ready for review June 16, 2021 17:03
@rphmeier rphmeier added the A0-please_review Pull request needs code review. label Jun 16, 2021
@github-actions github-actions bot removed the A3-in_progress Pull request is in progress. No review needed at this stage. label Jun 16, 2021
@rphmeier rphmeier changed the title Fork Choice changes for the guide Implementers' Guide: Chain Selection Jun 16, 2021
@rphmeier rphmeier mentioned this pull request Jun 17, 2021
6 tasks
Copy link
Contributor

@Lldenaurois Lldenaurois left a comment

Choose a reason for hiding this comment

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

First pass

roadmap/implementers-guide/src/protocol-chain-selection.md Outdated Show resolved Hide resolved
roadmap/implementers-guide/src/node/grandpa-voting-rule.md Outdated Show resolved Hide resolved
roadmap/implementers-guide/src/protocol-chain-selection.md Outdated Show resolved Hide resolved
@@ -287,21 +287,6 @@ enum CandidateBackingMessage {
}
```

## Candidate Selection Message
Copy link
Contributor

Choose a reason for hiding this comment

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

I should have caught this. Making a note that when modifying subsystems, it's important to grep for all mentions of that subsystem in the implementer's guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed it when running mdbook serve locally; there were outdated references that printed warnings

rphmeier and others added 3 commits June 17, 2021 15:47
Co-authored-by: Lldenaurois <ljdenaurois@gmail.com>
Co-authored-by: Lldenaurois <ljdenaurois@gmail.com>
Co-authored-by: Lldenaurois <ljdenaurois@gmail.com>
@rphmeier
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Jun 17, 2021

Waiting for commit status.

@ghost ghost merged commit 13ee3e2 into master Jun 17, 2021
@ghost ghost deleted the rh-forkchoice-guide branch June 17, 2021 15:10
ordian added a commit that referenced this pull request Jun 17, 2021
* master:
  Companion #9019 (max rpc payload override) (#3276)
  Implementers' Guide: Chain Selection (#3262)
  CLI: Add missing feature checking and check if someone passes a file (#3283)
  Export 'TakeRevenue' trait. (#3278)
  Add XCM Decode Limit (#3273)
  Allow Council to Use Scheduler (#3237)
  fix xcm pallet origin (#3272)
  extract determine_new_blocks into a separate utility (#3261)
  Approval checking unit tests (#3252)
  bridges: update finality-grandpa to 0.14.1 (#3266)
  malus - mockable overseer mvp (#3224)
  use safe math (#3249)
  Companion for #8920 (Control Staking) (#3260)
  Companion for #8949 (#3216)
This pull request was closed.
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. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants