docs: update crypto feature Godoc and ADR #1371
Merged
Conversation
This file contains 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
MSalopek
approved these changes
Oct 20, 2023
bermuell
reviewed
Nov 2, 2023
|
|
||
| Once a double signing evidence is committed to a block, the consensus layer will report the equivocation to the evidence module of the Cosmos SDK application layer. | ||
| The application will, in turn, punish the malicious validator through jailing, tombstoning and slashing | ||
| (see [handleEquivocationEvidence](https://github.com/cosmos/cosmos-sdk/blob/v0.45.16-ics-lsm/x/evidence/keeper/infraction.go#L263)). |
There was a problem hiding this comment.
L263 doesn't exist. should it be L26 instead of L263?
|
|
||
| ### Double Signing Attack | ||
|
|
||
| In the second part of the feature, we introduce a new endpoint `HandleConsumerDoubleVoting( |
There was a problem hiding this comment.
wondering if we really should use the function signiture and not just the function name as parameters might change?
same comment to L86
| In the first iteration of the feature, we will introduce a new endpoint: `HandleConsumerMisbehaviour(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour)`. | ||
| ### Light Client Attack | ||
|
|
||
| In the first part of the feature, we introduce a new endpoint: `HandleConsumerMisbehaviour(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour)`. |
There was a problem hiding this comment.
Suggested change
| In the first part of the feature, we introduce a new endpoint: `HandleConsumerMisbehaviour(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour)`. | |
| In the first part of the feature, we introduce a new functionality: `HandleConsumerMisbehaviour(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour)`. |
The term 'endpoint' I linked more with a node than a function. Same in line 101.
Comment on lines
+133
to
+134
| - For the same reasons explained above, the age of a consumer double signing evidence can't be verified, | ||
| either using its infraction height or its unsigned timestamp. Note that changes the jailing behaviour, potentially leading to a validator's jailing based on some "old" evidence from a consumer, which wouldn't occur if the consumer were a standalone chain. |
There was a problem hiding this comment.
"Note that changes ..." this is not clear to me? can you pls rephrase this?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
docs:prefix in the PR titleReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
docs:prefix in the PR titlemake build-docs)