docs: spec for high throughput recovery #4285
Conversation
evan-forbes
added
the
WS: Big Blonks 🔭
evan-forbes
requested review from
cmwaters and
ninabarbakadze
and removed request for
a team
|
evan-forbes
changed the title
|
Warning Rate limit exceeded@evan-forbes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe update introduces a new high throughput recovery mechanism for the Celestia protocol called the Pull Based Broadcast Tree (PBBT). The document in Changes
Sequence Diagram(s)sequenceDiagram
participant Broadcaster
participant Overlay
participant Peer
Broadcaster->>Overlay: Send Commitment and 'Have' messages
Overlay->>Peer: Propagate 'Have' messages
Peer->>Overlay: Respond with 'Want' message
Overlay->>Broadcaster: Forward 'Want' request
Broadcaster->>Overlay: Send Data/Part messages
Overlay->>Peer: Deliver Data/Part messages
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (26)
specs/src/recovery.md (26)
1-10: Title and Summary Overview: Ensure Consistent Terminology and Spelling
The title and summary provide a clear overview of the new high throughput recovery mechanism. However, please ensure consistency in naming—for example, the title “Vacuum! Part I: High Throughput Recovery” (line 1) should match the image alt text in line 15 (which currently uses “vacumm! map”). Also, consider using hyphenated adjectives (e.g., “pull‐based” instead of “pull based”, “push‐based” instead of “push based”) throughout the document for improved clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ...propagation) could be achieved using a "Pull Based Broadcast Tree" (PBBT). PBBT works by e...(BASED_HYPHEN)
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ...dWantmessages in otherwise standard pull based gossip, and by distributing the burden ...(BASED_HYPHEN)
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ... across the broadcaster's peers. Unlike push based broadcast trees, the route to distribut...(BASED_HYPHEN)
[uncategorized] ~7-~7: This expression is usually spelled with a hyphen.
Context: ...n sybil attack both broadcast trees and pull based gossip in time restricted applications ...(BASED_HYPHEN)
11-18: Intro Section: Clarify Two-Phase Propagation
The introduction clearly distinguishes between the "Preparation" and "Recovery" phases. For additional clarity, consider using consistent quotation marks and terminology (for example, “pull‐based gossip” instead of “pull based gossip”) throughout this section.🧰 Tools
🪛 LanguageTool
[style] ~16-~16: Consider a shorter alternative to avoid wordiness.
Context: ...p](./figures/recovery/vacuum!_map.png) In order to use the data propagated during the prep...(IN_ORDER_TO_PREMIUM)
[uncategorized] ~17-~17: This expression is usually spelled with a hyphen.
Context: ... recovery phase has to use some form of pull based gossip. Recovery has the following con...(BASED_HYPHEN)
19-30: Recovery Constraints: Improve Readability
The listed constraints are comprehensive and well-organized. To enhance readability, consider rephrasing expressions such as “the overhead % of the block” to “the percentage overhead of the block” and ensure uniform styling of technical terms.🧰 Tools
🪛 LanguageTool
[uncategorized] ~30-~30: This expression is usually spelled with a hyphen.
Context: ...ock the proposer must upload Enter the Pull Based Broadcast Tree (PBBT). PBBT addresses:...(BASED_HYPHEN)
30-38: Introducing PBBT: Clarify and Standardize Terminology
The introductory lines for the Pull Based Broadcast Tree (PBBT) set the context nicely. It would be beneficial to use the hyphenated form (e.g., “pull‐based broadcast tree”) for consistency. Also, ensure that the transition into how PBBT addresses throughput bottlenecks is smooth.🧰 Tools
🪛 LanguageTool
[uncategorized] ~30-~30: This expression is usually spelled with a hyphen.
Context: ...ock the proposer must upload Enter the Pull Based Broadcast Tree (PBBT). PBBT addresses:...(BASED_HYPHEN)
[uncategorized] ~34-~34: This expression is usually spelled with a hyphen.
Context: ...resses: - efficiency: by utilizing pull based logic - speed: by pipeliningHave...(BASED_HYPHEN)
40-50: Pull Based Block Propagation: Consistency and Detail
This section describes the propagation mechanism effectively by listing the four message types. To improve clarity, consider revising the sentence on line 49 (e.g., remove extraneous commas and redundant phrasing) and standardize terms like “pull‐based” throughout.🧰 Tools
🪛 LanguageTool
[uncategorized] ~40-~40: This expression is usually spelled with a hyphen.
Context: ... components in an abstract fashion. ## Pull Based Block Propagation PBBT relies heavily ...(BASED_HYPHEN)
[uncategorized] ~42-~42: This expression is usually spelled with a hyphen.
Context: ...BBT relies heavily on slightly modified pull based gossiping mechanisms. There are four ge...(BASED_HYPHEN)
[uncategorized] ~49-~49: This expression is usually spelled with a hyphen.
Context: ...ta The only difference between typical pull based block propagation and what is used here...(BASED_HYPHEN)
64-79: Distribution Rules: Clarify Rules and Correct Minor Wording
The distribution rules are thorough. Consider rephrasing “The rules below goal is to facilitate…” (line 68) to “The rules below aim to facilitate…” and review the section for consistent use of capitalization and technical terminology.🧰 Tools
🪛 LanguageTool
[uncategorized] ~64-~64: This expression is usually spelled with a hyphen.
Context: ...->>NodeB: DATA ``` Figure 0: General pull based gossip steps. ### Distribution Rules...(BASED_HYPHEN)
[uncategorized] ~68-~68: This expression is usually spelled with a hyphen.
Context: ... rules below goal is to facilitate safe pull based propagation. - **Nodes MUST only propa...(BASED_HYPHEN)
[duplication] ~70-~70: Possible typo: you repeated a word.
Context: ...- Nodes MUST only propagate validated messages -Commitmentmessages MUST originate from the Proposer - `H...(ENGLISH_WORD_REPEAT_RULE)
[grammar] ~72-~72: You’ve repeated a verb. Did you mean to only write one of them?
Context: ... those committed to in theCommitmentmessage -Datamessages MUST match their correspondingHavea...(REPEATED_VERBS)
86-95: Pipelining ‘Have’ and ‘Want’ Messages: Enhance Clarity
This section effectively outlines the pipelining approach. A suggestion would be to tighten the sentence “Instead of waiting until data is downloaded to gossipHavemessages, in PBT, they are gossiped after verifying them” for conciseness, and to standardize terms such as “pull‐based” and “push‐based” for consistency.🧰 Tools
🪛 LanguageTool
[uncategorized] ~87-~87: This expression is usually spelled with a hyphen.
Context: ... PipeliningHaveandWantMessages Pull based mechanisms are very efficient, but sacr...(BASED_HYPHEN)
[uncategorized] ~88-~88: This expression is usually spelled with a hyphen.
Context: ... theHaveandWantmessages so that push based mechanisms can get close to the speed o...(BASED_HYPHEN)
[uncategorized] ~94-~94: This expression is usually spelled with a hyphen.
Context: ...ready been downloaded** Keep the other pull based rules in context as well, where nodes c...(BASED_HYPHEN)
116-118: Figure 1 Caption: Clarify Caption Consistency
The caption for the pipelining diagram is informative. Consider a slight rewording such as “Note the simultaneous transfer ofWantandHavemessages” to improve clarity.
119-130: Broadcast Tree Overview: Ensure Conceptual Clarity
The introduction to the broadcast tree concept is well done, and the definition of throughput parameters (T, B, n) is clear. It might help to introduce these variables with a brief contextual explanation before the formula appears.🧰 Tools
🪛 LanguageTool
[style] ~120-~120: Consider a shorter alternative to avoid wordiness.
Context: ...ally the same time. ## Broadcast Tree In order to increase the "recovery" throughput subs...(IN_ORDER_TO_PREMIUM)
135-138: Upload Bandwidth Explanation: Provide Additional Detail
The explanation on reducing the broadcaster's upload burden by distributing block data among peers is clear. A brief elaboration on how this distribution mechanism enhances overall network performance could further benefit the reader.
165-166: Figure 3 Caption: Minor Text Revision
The caption describing the broadcast tree diagram is succinct and informative. A minor revision such as “Note how portions of the block data are transferred between nodes” might further enhance clarity.
167-176: Optimal Routing Section: Clarify Language
This section explains the benefits of on-the-fly route generation well. Consider revising phrases like “on fly” to “on the fly” and “real time data” to “real-time data” to ensure both grammatical correctness and clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~169-~169: Possible missing article found.
Context: ...fits of generating the route of data on fly instead of pre-planning the route is th...(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~169-~169: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... route is that the plan can incorporate real time data. This inherently includes: - ne...(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.17.2)
169-169: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
177-182: FIFO Messaging and Congestion: Correct Terminology
The discussion on potential congestion whenHavemessages are enqueued in a FIFO manner is valuable. Please correct typographical errors such as changing “haves” (line 177) to “Havemessages” and “recieve” (line 177) to “receive.” A slight rephrasing for improved readability is recommended.🧰 Tools
🪛 LanguageTool
[grammar] ~177-~177: The verb form ‘take’ does not seem to be suitable in this context.
Context: ...on asData. This ensures that a haves take longer to be delivered to a congested c...(HAVE_VB)
[style] ~182-~182: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ... send them to many nodes in the network extremely quickly. Without additional rules, this would c...(EN_WEAK_ADJECTIVE)
[typographical] ~182-~182: The word “therefore” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...use that node to become a bottleneck in distribution, therefore we add a new rule to ensure this doesn'...(THUS_SENTENCE)
🪛 markdownlint-cli2 (0.17.2)
177-177: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
183-186: Concurrent Request Cap: Adequate Addition
The rule stating that nodes must have fewer thanPerPeerConcurrentRequestCapconcurrent requests per peer is a practical measure. Consider providing a brief rationale or an example to further illustrate its importance.
187-203: Security Section: Ensure Completeness and Clarity
The security section effectively highlights the need for a robust broadcasting mechanism that can withstand adversarial conditions. Minor adjustments such as rephrasing “held to same standard” to “held to the same standard” would improve readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~190-~190: Possible missing article found.
Context: ... broadcasting mechanism must be held to same standard. Meaning that if 2/3 of the vo...(AI_HYDRA_LEO_MISSING_THE)
[misspelling] ~197-~197: This word is normally spelled with a hyphen.
Context: ... every other honest node. The second is self explanatory. Recall the constraints for recovery: ...(EN_COMPOUNDS_SELF_EXPLANATORY)
211-222: Sybil Attacks (Pull Based Gossip): Minor Corrections
This section provides a good explanation of how sybil attacks can affect pull-based protocols. Please update “its easy” (line 220) to “it’s easy” and consider a slight rephrasing to further enhance clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~214-~214: This expression is usually spelled with a hyphen.
Context: ... you find one. ### Sybil Attacks #### Pull Based Gossip Upon requesting data in a stand...(BASED_HYPHEN)
[uncategorized] ~216-~216: This expression is usually spelled with a hyphen.
Context: ...sip Upon requesting data in a standard pull based protocol, faulty peers can simply not s...(BASED_HYPHEN)
[uncategorized] ~220-~220: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...icious peers are removed. > Side note: its easy to confuse a congested peer as a m...(AI_HYDRA_LEO_CPT_ITS_ITIS)
[uncategorized] ~220-~220: This expression is usually spelled with a hyphen.
Context: ...s peer scoring incredibly difficult for pull based systems. If timeouts for scoring or add...(BASED_HYPHEN)
🪛 markdownlint-cli2 (0.17.2)
220-220: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
227-234: Broadcast Trees Conclusions: Highlight Key Insights
The conclusions regarding broadcast trees are insightful. Ensure that the formatting and numbering remain consistent and consider emphasizing the importance of differentiating between validators and non-validators more explicitly.
235-238: Defending Against Attacks: Introduction Clarity
The introduction to the defensive measures is clear. It might be beneficial to further structure this section with subheadings or a numbered list to delineate each strategy more clearly.
239-253: Erasure Encoding Example: Code Clarity and Context
The Go code snippet is an effective illustration of erasure encoding. Although the use of ellipses (...) as placeholders is acceptable, adding brief comments to explain that these represent actual data would improve clarity. Also, ensure that the inline comment on line 251 is fully clear.
260-263: Erasure Encoding Conclusion: Emphasize Security Benefits
This section concisely conveys that adding parity data can mitigate attacks. Consider rephrasing “a powerful tool to get to our goal” to a more formal tone.🧰 Tools
🪛 LanguageTool
[uncategorized] ~263-~263: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...s a powerful tool to get to our goal: >Its important that new broadcasting mechani...(AI_HYDRA_LEO_CPT_ITS_ITIS)
264-273: Overlay Networks: Clarify Benefits and Implementation
The description of overlay networks highlights their role in mitigating sybil attacks effectively. A minor grammatical adjustment is needed—replace “can avoiding sybil nodes” (line 271) with “can avoid sybil nodes.”🧰 Tools
🪛 LanguageTool
[grammar] ~271-~271: Modal verbs like ‘can’ or ‘will’ require the following verb to be in its base form.
Context: ...y network is simply that validators can avoiding sybil nodes to the best of their knowle...(MD_BASEFORM)
274-276: Critical Note Reiteration: Confirm Emphasis
The reiterated note emphasizes that new broadcasting mechanisms must not lower the barrier to network halting. Please correct “Its important” (line 275) to “It’s important” for consistency and clarity.🧰 Tools
🪛 LanguageTool
[uncategorized] ~275-~275: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...y we have achieved our original goal. >Its important that new broadcasting mechani...(AI_HYDRA_LEO_CPT_ITS_ITIS)
277-283: Preparation Section: Reuse and Advantage
This section effectively explains how validators can leverage pre-distributed transactions to combat sybil attacks. A minor suggestion is to change “combatting” (line 279) to “combating” for correct spelling.🧰 Tools
🪛 LanguageTool
[style] ~279-~279: The phrase ‘have the ability to’ might be wordy. Consider using “can”.
Context: ... already does. #### Preparation If we have the ability to know what portion of the voting power a...(HAS_THE_ABILITY_TO)
[style] ~279-~279: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...as a given transaction, and the network is able to reuse the transactions they have alread...(BE_ABLE_TO)
🪛 markdownlint-cli2 (0.17.2)
279-279: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
284-293: Synthesis Section: Summarize Key Mechanisms
The synthesis succinctly combines the various mechanisms to meet the required security standards. Please correct recurring typographical errors (e.g., “Its important” should be “It’s important”) and consider streamlining the bullet points for enhanced clarity.🧰 Tools
🪛 LanguageTool
[typographical] ~285-~285: There might be a comma missing.
Context: ...sybil attacks. However, when we combine them we can meet our original goal: >Its im...(IF_PRP_PRP_COMMA)
[uncategorized] ~287-~287: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...e them we can meet our original goal: >Its important that new broadcasting mechani...(AI_HYDRA_LEO_CPT_ITS_ITIS)
[style] ~290-~290: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...erlay network is added, then validators are able to avoid sybil attacks from non-validators...(BE_ABLE_TO)
[style] ~291-~291: ‘a majority of’ might be wordy. Consider a shorter alternative.
Context: ...roposed block is already distributed to a majority of validators, then it has already been su...(EN_WORDINESS_PREMIUM_A_MAJORITY_OF)
294-317: Commitment and CompactBlock Definitions: Ensure Accuracy
The protobuf definitions forBlobMetaDataandCompactBlockare well presented. In the accompanying explanation (around line 313), please correct “siganture” to “signature” and ensure that the relationship betweenBlobMetaData,pbbt_root, and the other fields is described clearly.
349-358: Data Message Definition: Check for Completeness
ThePartmessage is straightforward and well-defined. Please confirm that the chosen field types, particularlyuint32for the index, are sufficient for the intended use case and that this structure matches downstream message processing.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
specs/src/figures/recovery/vacuum!_map.pngis excluded by!**/*.png
📒 Files selected for processing (1)
specs/src/recovery.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
specs/src/recovery.md
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ...propagation) could be achieved using a "Pull Based Broadcast Tree" (PBBT). PBBT works by e...
(BASED_HYPHEN)
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ...d Want messages in otherwise standard pull based gossip, and by distributing the burden ...
(BASED_HYPHEN)
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ... across the broadcaster's peers. Unlike push based broadcast trees, the route to distribut...
(BASED_HYPHEN)
[uncategorized] ~7-~7: This expression is usually spelled with a hyphen.
Context: ...n sybil attack both broadcast trees and pull based gossip in time restricted applications ...
(BASED_HYPHEN)
[style] ~16-~16: Consider a shorter alternative to avoid wordiness.
Context: ...p](./figures/recovery/vacuum!_map.png) In order to use the data propagated during the prep...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~17-~17: This expression is usually spelled with a hyphen.
Context: ... recovery phase has to use some form of pull based gossip. Recovery has the following con...
(BASED_HYPHEN)
[uncategorized] ~30-~30: This expression is usually spelled with a hyphen.
Context: ...ock the proposer must upload Enter the Pull Based Broadcast Tree (PBBT). PBBT addresses:...
(BASED_HYPHEN)
[uncategorized] ~34-~34: This expression is usually spelled with a hyphen.
Context: ...resses: - efficiency: by utilizing pull based logic - speed: by pipelining Have...
(BASED_HYPHEN)
[uncategorized] ~40-~40: This expression is usually spelled with a hyphen.
Context: ... components in an abstract fashion. ## Pull Based Block Propagation PBBT relies heavily ...
(BASED_HYPHEN)
[uncategorized] ~42-~42: This expression is usually spelled with a hyphen.
Context: ...BBT relies heavily on slightly modified pull based gossiping mechanisms. There are four ge...
(BASED_HYPHEN)
[uncategorized] ~49-~49: This expression is usually spelled with a hyphen.
Context: ...ta The only difference between typical pull based block propagation and what is used here...
(BASED_HYPHEN)
[uncategorized] ~64-~64: This expression is usually spelled with a hyphen.
Context: ...->>NodeB: DATA ``` Figure 0: General pull based gossip steps. ### Distribution Rules...
(BASED_HYPHEN)
[uncategorized] ~68-~68: This expression is usually spelled with a hyphen.
Context: ... rules below goal is to facilitate safe pull based propagation. - **Nodes MUST only propa...
(BASED_HYPHEN)
[duplication] ~70-~70: Possible typo: you repeated a word.
Context: ...- Nodes MUST only propagate validated messages - Commitment messages MUST originate from the Proposer - `H...
(ENGLISH_WORD_REPEAT_RULE)
[grammar] ~72-~72: You’ve repeated a verb. Did you mean to only write one of them?
Context: ... those committed to in the Commitment message - Data messages MUST match their corresponding Have a...
(REPEATED_VERBS)
[uncategorized] ~87-~87: This expression is usually spelled with a hyphen.
Context: ... Pipelining Have and Want Messages Pull based mechanisms are very efficient, but sacr...
(BASED_HYPHEN)
[uncategorized] ~88-~88: This expression is usually spelled with a hyphen.
Context: ... the Have and Want messages so that push based mechanisms can get close to the speed o...
(BASED_HYPHEN)
[uncategorized] ~94-~94: This expression is usually spelled with a hyphen.
Context: ...ready been downloaded** Keep the other pull based rules in context as well, where nodes c...
(BASED_HYPHEN)
[style] ~120-~120: Consider a shorter alternative to avoid wordiness.
Context: ...ally the same time. ## Broadcast Tree In order to increase the "recovery" throughput subs...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~169-~169: Possible missing article found.
Context: ...fits of generating the route of data on fly instead of pre-planning the route is th...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~169-~169: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... route is that the plan can incorporate real time data. This inherently includes: - ne...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~177-~177: The verb form ‘take’ does not seem to be suitable in this context.
Context: ...on as Data. This ensures that a haves take longer to be delivered to a congested c...
(HAVE_VB)
[style] ~182-~182: As an alternative to the over-used intensifier ‘extremely’, consider replacing this phrase.
Context: ... send them to many nodes in the network extremely quickly. Without additional rules, this would c...
(EN_WEAK_ADJECTIVE)
[typographical] ~182-~182: The word “therefore” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...use that node to become a bottleneck in distribution, therefore we add a new rule to ensure this doesn'...
(THUS_SENTENCE)
[uncategorized] ~190-~190: Possible missing article found.
Context: ... broadcasting mechanism must be held to same standard. Meaning that if 2/3 of the vo...
(AI_HYDRA_LEO_MISSING_THE)
[misspelling] ~197-~197: This word is normally spelled with a hyphen.
Context: ... every other honest node. The second is self explanatory. Recall the constraints for recovery: ...
(EN_COMPOUNDS_SELF_EXPLANATORY)
[uncategorized] ~204-~204: This expression is usually spelled with a hyphen.
Context: ...ST be <= the ProposalTimeout** While pull based gossip and broadcast trees are incredib...
(BASED_HYPHEN)
[uncategorized] ~214-~214: This expression is usually spelled with a hyphen.
Context: ... you find one. ### Sybil Attacks #### Pull Based Gossip Upon requesting data in a stand...
(BASED_HYPHEN)
[uncategorized] ~216-~216: This expression is usually spelled with a hyphen.
Context: ...sip Upon requesting data in a standard pull based protocol, faulty peers can simply not s...
(BASED_HYPHEN)
[uncategorized] ~220-~220: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...icious peers are removed. > Side note: its easy to confuse a congested peer as a m...
(AI_HYDRA_LEO_CPT_ITS_ITIS)
[uncategorized] ~220-~220: This expression is usually spelled with a hyphen.
Context: ...s peer scoring incredibly difficult for pull based systems. If timeouts for scoring or add...
(BASED_HYPHEN)
[style] ~224-~224: Consider a shorter alternative to avoid wordiness.
Context: ...lications require additional mechanisms in order to use pull based gossip. #### Broadcast ...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~224-~224: This expression is usually spelled with a hyphen.
Context: ...e additional mechanisms in order to use pull based gossip. #### Broadcast Trees Using th...
(BASED_HYPHEN)
[uncategorized] ~263-~263: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...s a powerful tool to get to our goal: >Its important that new broadcasting mechani...
(AI_HYDRA_LEO_CPT_ITS_ITIS)
[grammar] ~271-~271: Modal verbs like ‘can’ or ‘will’ require the following verb to be in its base form.
Context: ...y network is simply that validators can avoiding sybil nodes to the best of their knowle...
(MD_BASEFORM)
[uncategorized] ~275-~275: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...y we have achieved our original goal. >Its important that new broadcasting mechani...
(AI_HYDRA_LEO_CPT_ITS_ITIS)
[style] ~279-~279: The phrase ‘have the ability to’ might be wordy. Consider using “can”.
Context: ... already does. #### Preparation If we have the ability to know what portion of the voting power a...
(HAS_THE_ABILITY_TO)
[style] ~279-~279: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...as a given transaction, and the network is able to reuse the transactions they have alread...
(BE_ABLE_TO)
[typographical] ~285-~285: There might be a comma missing.
Context: ...sybil attacks. However, when we combine them we can meet our original goal: >Its im...
(IF_PRP_PRP_COMMA)
[uncategorized] ~287-~287: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...e them we can meet our original goal: >Its important that new broadcasting mechani...
(AI_HYDRA_LEO_CPT_ITS_ITIS)
[style] ~290-~290: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...erlay network is added, then validators are able to avoid sybil attacks from non-validators...
(BE_ABLE_TO)
[style] ~291-~291: ‘a majority of’ might be wordy. Consider a shorter alternative.
Context: ...roposed block is already distributed to a majority of validators, then it has already been su...
(EN_WORDINESS_PREMIUM_A_MAJORITY_OF)
🪛 markdownlint-cli2 (0.17.2)
specs/src/recovery.md
169-169: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
177-177: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
220-220: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
279-279: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (10)
specs/src/recovery.md (10)
51-63: Mermaid Diagram for Gossip Steps: Verify Rendering
The Mermaid diagram clearly illustrates the gossip steps. Please ensure that the diagram syntax is fully compatible with our documentation renderer and that the sequence of messages is correct.
80-85: Disconnection Rules: Clear and Comprehensive
The disconnection rules are well-defined and clearly describe when and why a node should disconnect from peers. No major issues are noted here.
96-115: Mermaid Diagram for Pipelining: Confirm Accuracy
The sequence diagram illustrating the pipelining process is very helpful. Please double-check that the timing indicators (e.g., “time 0”, “time 1”, etc.) accurately represent the intended flow of messages.
131-134: Throughput Calculation Snippet: Confirm Clarity
The Python snippet showingT = B / nis simple and clear. If intended for educational purposes, consider adding inline comments to explain what each variable represents.
139-164: Broadcast Tree Diagram: Check Diagram Accuracy
The Mermaid diagram for the broadcast tree effectively demonstrates how portions of the block data are transferred between nodes. Please verify that all message labels (such asHAVE_0,WANT_0, etc.) are consistent with the narrative description.
204-210: Denial of Service: Adequate Coverage
The discussion on how DoS attacks may manifest and the corresponding disconnection policies is concise and clear. No changes are needed here.🧰 Tools
🪛 LanguageTool
[uncategorized] ~204-~204: This expression is usually spelled with a hyphen.
Context: ...ST be <= theProposalTimeout** While pull based gossip and broadcast trees are incredib...(BASED_HYPHEN)
254-259: Erasure Encoding Throughput: Explain Trade-offs
The explanation that the maximum throughput is divided byeand the revised delivery constraints are clearly stated. This section is solid as is.
323-333: Have Message Definition: Clear and Concise
The protobuf definition forHavePartis clear and the verification note is helpful. No significant changes are needed here.
339-347: Want Message Definition: Verify Consistency
TheWantPartsmessage is defined clearly. Ensure that the use oftendermint.libs.bits.BitArrayaligns with the rest of the protocol specifications and documentation.
359-363: Data Verification: Ensure Correctness
The verification rule—that the hash of the bytes in the data field must match the correspondingHavemessage—is clearly stated and necessary. This section is well done with no further changes required.
Overview
spec for high throughput recovery. this can be considered part I of vacuum!.