-
Notifications
You must be signed in to change notification settings - Fork 206
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
Consensus Trace Validation improvements #6183
Comments
+1 for the second proposal. However, there should be a comment for the benefit of beginners that explains why the actual requests do not matter. |
For the record, I disagree with the removal of the formulas that are commented with TODO that need investigation. These are exactly the formulas that have to be added, thus, more precise than any github issue can be. If there is a clear reason why an assertion does not hold, the formula should be replaced with a proper comment that explain why the assertions do not hold. |
@lemmy the project uses GitHub issues to track, discuss and prioritise work items. Please see the contributing guidelines for code formatting rules. If an issue is important to you, please feel free to open a PR with a corresponding code change. This issue can be used to claim items and avoid duplicate effort. |
+1 for option 2. The specific request does not matter for what we are modelling in |
Specs are not code.
…On May 17, 2024 1:55:11 AM PDT, Amaury Chamayou ***@***.***> wrote:
@lemmy the project uses issues to track, discuss and prioritise work items. Please see the [contributing guidelines](https://github.com/microsoft/CCF/blob/main/.github/CONTRIBUTING.md) for code formatting rules. are If an issue is important to you, please feel free to open a PR with a corresponding code change.
|
[request -> 42, contentType |-> TypeEntry]
. Two ways to go here:ClientRequest
from them.request
altogether.To be discussed:
IsSendAppendEntries
,sent_idx
does not matchnextIndex[i][j]
. Investigated, explained, and options proposed.Requires Investigation:
IsSendAppendEntries
Len(log'[logline.msg.state.node_id])
does not matchlogline.msg.state.last_idx
. Needs investigation.IsRcvAppendEntriesRequest
,leadershipState[logline.msg.state.node_id]
does not matchToLeadershipState[logline.msg.state.leadership_state]
. Needs investigation.IsRcvAppendEntriesRequest
Len(log'[logline.msg.state.node_id])
does not matchlogline.msg.state.last_idx
. Needs investigation.IsAddConfiguration
, committable indices, commit Index, membershipState and last_idx don't match. I looked at this, and in situations where we receive an AE range that contains a configuration at first followed by committable indices, recv_append_entries will update the committable indices in the spec, but not in the impl state, which then goes on to handle an add_configuration event on which state->committable_indices is empty. Needs investigation.IsAdvanceCommitIndex
, thecommit_idx
andlast_idx
don't match in theFollower
case. Needs investigation.IsRcvAppendEntriesResponse
, theleadershipState
does not match. Needs investigation.IsRcvRequestVoteRequest
, theleadershipState
andlast_idx
do not match. Needs investigation.IsExecuteAppendEntries
, thecommit_idx
andlast_idx
do not match. Needs investigation.IsRcvRequestVoteResponse
, theleadershipState
does not match. Needs investigation.The text was updated successfully, but these errors were encountered: