Conversation
lidatong
force-pushed
the
chali/docs/notar-tower
branch
from
dc9c8cd to
cd4a841
Compare
There was a problem hiding this comment.
Pull request overview
Polishes and clarifies inline documentation for the TowerBFT (“tower”) and notarization (“notar”) components, aiming to better explain fork choice/voting mechanics and how fd_notar differs from fd_ghost/fd_tower.
Changes:
- Reformats and tightens the large TowerBFT preamble comment in
fd_tower.h(spacing/line breaks and a rewritten “how voting works” section). - Expands
fd_notar.hdocumentation describing conceptual differences betweenfd_ghost/fd_towerandfd_notar. - Minor prototype formatting / comment spacing cleanups.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/choreo/tower/fd_tower.h | Documentation reflow + updated explanation of voting/tower reconciliation; minor signature formatting. |
| src/choreo/notar/fd_notar.h | Expanded docs on how notar differs from ghost/tower (vote accounts vs vote transactions). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| replay votes as well, so it is guaranteed to be a superset of the | ||
| votes tracked by fd_ghost / fd_tower. |
There was a problem hiding this comment.
The comment says fd_notar is "guaranteed to be a superset" of votes tracked by fd_ghost/fd_tower. However, the current vote-txn ingestion path filters to TowerSync(Switch) only, so legacy/other vote instructions that still update the vote account could be reflected in fd_ghost/fd_tower but ignored by fd_notar. Consider softening this claim (e.g., "intended to be" / "for TowerSync votes"), or explicitly stating the assumption that only TowerSync(Switch) votes are supported/tracked here.
| replay votes as well, so it is guaranteed to be a superset of the | |
| votes tracked by fd_ghost / fd_tower. | |
| replay votes as well, so it is intended to be a superset of the | |
| TowerSync(Switch) votes tracked by fd_ghost / fd_tower, assuming only | |
| TowerSync(Switch) vote instructions are supported/tracked here. |
| On the flip side, validators also must stay in sync the other way from | ||
| cluster to local. If a validator has previously voted, then they have | ||
| an on-chain vote account containing the cluster's latest view of the | ||
| tower (as of a given a replay slot). If this on-chain tower is |
There was a problem hiding this comment.
Grammar: "as of a given a replay slot" reads like a duplicated article. Suggest rewording to "as of a given replay slot" (or similar) to avoid confusion in this explanation of the on-chain tower view.
| tower (as of a given a replay slot). If this on-chain tower is | |
| tower (as of a given replay slot). If this on-chain tower is |
Performance Measurements ⏳
|
No description provided.