-
Notifications
You must be signed in to change notification settings - Fork 30
feat(docs): ADR 003 - Propagation of Proposed Values #884
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
99a715b
docs: added file to ADR 003 on values propopagation
cason c462f6f
docs: ADR 003 general Context
cason 0900b60
docs: ADR 003, Tendermint context vs general one
cason 1f564d7
docs: ADR 003, Value Propagation context
cason ead61e6
docs: ADR 103 Alternatives overview
cason f0ee5ed
docs: ADR 003, Context simplified and rewritten
cason 99a288c
docs: ADR 003, sketched Alternatives sections
cason 749a3b1
Merge branch 'main' into cason/365-adr
cason 443f5cc
Merge branch 'main' into cason/365-adr
cason b23e839
Merge branch 'main' into cason/365-adr
cason 0a52f8a
docs: ADR 003, overivew of dissemination approaches
cason 49345cb
Merge branch 'cason/365-adr' of github.com:informalsystems/malachite …
ebd5da7
first take on describing current designs
82a0d08
small improvments on the text
651dfd9
Merge branch 'main' into cason/365-adr
romac f5d2768
feat: mermaid diagrams of value propagation modes
bastienfaivre 879dc27
Minor changes
nenadmilosevic95 f830402
Small text changes
nenadmilosevic95 f245045
fix: diagrams
bastienfaivre 001a1be
Update docs/architecture/adr-003-values-propagation.md
nenadmilosevic95 9c98fea
Use V and v, fix diagrams, add some app requirements
ancazamfir ba2f599
Fix typos
ancazamfir 6d92a9a
Separate consensus core and engine, remove network
ancazamfir 489dd73
Correct diagrams
ancazamfir fd01fec
Address review comments
ancazamfir 6656c7b
Add details for the different diagram blocks
ancazamfir 7426672
Add warnings for ProposalOnly
ancazamfir 864e8d6
Apply suggestions from code review
ancazamfir b930576
Small changes
romac c2ae8a5
Remvove TODO
ancazamfir 799f4c5
Show streaming before LocallyProposedValue, add own Proposal msg
ancazamfir 0e63684
Rearange receiver boxes
ancazamfir d85bb5d
Apply suggestions from code review
ancazamfir 0e8ba53
initial pass, ~50% done
adizere d7a0d06
pass on value propagation considerations section
02cddd9
Merge branch 'cason/365-adr' of github.com:informalsystems/malachite …
bc6b588
Generate proposal for PartsOnly in the engine
ancazamfir af34a62
Explicit requirements for PartsOnly
ancazamfir 23093ce
Add reference to tendermint consensus algo changes
ancazamfir aa57421
Spelling
ancazamfir ce82a67
Small fixes
ancazamfir 369f61a
Clarify ProposalOnly for large values
ancazamfir 90a25d6
Fix Engine description
ancazamfir 47953d7
Apply suggestions from code review
ancazamfir 67ef528
Ref fixes
ancazamfir 5db0a31
Update docs/architecture/adr-003-values-propagation.md
ancazamfir 77d21f2
Update docs/architecture/adr-003-values-propagation.md
ancazamfir a044bdb
Apply suggestions from code review
ancazamfir 0370ded
valid_round mandatory for PartsOnly
ancazamfir fa36fbf
second pass
adizere 2fdd936
minor rephrasing
adizere 7af4487
Merge branch 'main' into cason/365-adr
adizere 0d33f11
Cleanup
ancazamfir 753d9d6
Review comment
ancazamfir 05c95dc
Add restreaming section
ancazamfir c33540f
Review comments - correct restreaming behavior at non-proposer
ancazamfir 7be2b78
Review comments
ancazamfir e9aff4c
small updates
nenadmilosevic95 77cc03a
add `V` in consensus by value section
nenadmilosevic95 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -324,6 +324,25 @@ the `Publish` effect is not emitted and a `Proposal` message is not sent through | |||||||||
|
||||||||||
At the receiving side, consensus core waits to receive `ProposedValue(ProposedValue(v, validity))` input and when this happens it considers the proposal as complete and proceeds. The application generates this input upon receiving the full value `V` from the network. As a result, in this case value propagation is totally delegated to the application. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the core that has the "complete proposal" logic. |
||||||||||
|
||||||||||
### Value Restreaming | ||||||||||
|
||||||||||
When operating in either `ProposalAndParts` or `PartsOnly` modes, there are cases where a proposer needs to re-propose a value that was previously seen in an earlier round. This occurs when implementing L16 of the Tendermint algorithm, where a proposer enters a new round with a valid value from a previous round. | ||||||||||
|
||||||||||
The restreaming flow follows a similar pattern to the initial proposal flow, but with these key differences: | ||||||||||
|
||||||||||
1. **Effect Generation**: | ||||||||||
- Instead of `GetValue`, the consensus core generates a `RestreamProposal` effect | ||||||||||
ancazamfir marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
- This signals to the application that it should re-disseminate an existing value rather than generate a new one | ||||||||||
|
||||||||||
2. **Proposer Behavior**: | ||||||||||
- The consensus core already has the valid value from the previous round | ||||||||||
- Unlike the initial proposal flow, it does not need to wait for a `LocallyProposedValue` input | ||||||||||
- The application only needs to handle re-dissemination of the value parts through the network | ||||||||||
ancazamfir marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
|
||||||||||
3. **Non-Proposer Behavior**: | ||||||||||
- If a non-proposer node has already validated this value in a previous round, it can skip waiting for the `ProposedValue` input | ||||||||||
ancazamfir marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
- This optimization helps reduce unnecessary validation work and speeds up consensus | ||||||||||
|
||||||||||
### Summary | ||||||||||
|
||||||||||
To sum up, different modes rely on different inputs to achieve the same effect as | ||||||||||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.