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.
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
feat!: deprecate soft opt-out #1964
feat!: deprecate soft opt-out #1964
Changes from all commits
69fff54
fdcca88
621e112
3cb17be
dee262c
e22d3a3
9d2f0b6
e37808a
a423de0
1aed85f
a4ef241
59e46f9
82f5763
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ConsumerParams
is used to send data to the consumer chain "over the wire" - it is used in the consumer genesis.Setting it to deprecated allows the field to exist but it will always be empty, thus maintaining backward compatibility.
We will not need to write a special parser to go from v4.2.0
ConsumerParams
to some future versionConsumerParams
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. This is why we did not use
reserved
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added here and in
changeoverChain
because Ifaction.GenesisChanges
was the empty string,consumerGenesis
would end with|
and as a result thejq
transformation here would fail.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note for posterity: Increased this due to some flakiness in the test involving double signing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor the use of constants for repeated string literals.
The string
"auto"
is used multiple times across different functions. It is beneficial to define it as a constant at the beginning of the file to ensure consistency and ease of maintenance.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this function because it did not add much because we could use
stepsRedelegate
in its place.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has the ratio changed so much?
Are you trying to check that the soft opt-out no longer works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TL;DR because we removed opt-out related steps that were reducing the
ValPower
ofalice
.Specifically, we removed the
stepsRedelegateForOptOut
andstepsDowntimeWithOptOut
. ThestepsRedelegateForOptOut
was the one that reduced theValPower
ofalice
to60
so it could test the opt out functionality.Before, in the
happyPathSteps
we had:but we removed
stepsRedelegateForOptOut
andstepsDowntimeWithOptOut
and hence now whenstepsRedelegate
is called, theValPower
ofalice
is not60
anymore but510
. Redelegating400
toalice
makesalice
have aValPower
910
. This meant that allsteps*
functions called afterstepsRedelegate
had to be modified as well to contain the correctValPower
s.No.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for elaborating. It was not clear to me after going over it a couple times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test removes the threshold while the ones above have it stil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thresholds are removed from the individual
StarConsumerChainAction
andChangeOverChainAction
actions and are moved tostartConsumerChain
andchangeoverChain
so we do not have to set the threshold in a multitude of other places. This was just part of small refactoring.