Skip to content
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

fix!: persist new vals to the base comet state #160

Merged
merged 6 commits into from
Apr 15, 2024
Merged

Conversation

Reecepbcups
Copy link
Member

@Reecepbcups Reecepbcups commented Apr 14, 2024

Summary

Found by ethos in testing. Original #149

Despite PoA depending on x/staking for it's EndBlock actions, it was not persisting when changing Power amounts for new validators -> CometBFT consensus.

This PR has an issue where jailed validators attempt to jail per block. But it's large enough to merge to main

The "new" test are just copy paste from the previous monolithic test (TestPOA). Just added wait for blocks at the end of some to ensure ABCI val update events do not get stuck

Final

  • Ensure Jail works manually w/ quick slash & does not panic (has automated check now too)
  • Restart jail'ed validator (no easy way to test this atm)

@Reecepbcups Reecepbcups changed the title chore(test): spread out chore(test): spread out over multiple e2e files Apr 14, 2024
@Reecepbcups Reecepbcups changed the title chore(test): spread out over multiple e2e files 2: chore(test): spread out over multiple e2e files Apr 14, 2024
@Reecepbcups Reecepbcups changed the title 2: chore(test): spread out over multiple e2e files chore(test): spread out over multiple e2e files Apr 14, 2024
* `UpdatedValidatorsCacheKey`

* ABCI: `DeleteValidatorByPowerIndex` logic

* validate ABCI events are not stuck

* wait for blocks jail test

* BeforeJailedValidatorsKey

* lint + fix remove val test (check bonded)

* lint

* logs

* touchups
@Reecepbcups Reecepbcups changed the title chore(test): spread out over multiple e2e files fix!(abci v2): persist new vals to the base comet state Apr 14, 2024
@Reecepbcups Reecepbcups changed the title fix!(abci v2): persist new vals to the base comet state fix!(abci_v2): persist new vals to the base comet state Apr 14, 2024
@Reecepbcups Reecepbcups changed the title fix!(abci_v2): persist new vals to the base comet state fix!: persist new vals to the base comet state Apr 14, 2024
@Reecepbcups Reecepbcups marked this pull request as ready for review April 14, 2024 23:56
@Reecepbcups Reecepbcups linked an issue Apr 15, 2024 that may be closed by this pull request
@Reecepbcups Reecepbcups merged commit f9dc521 into main Apr 15, 2024
16 checks passed
Comment on lines +124 to +135
// TODO: If this is used here, it persist ABCI Updates. When removes, it looks like the validator gets slashed every block in x/staking? (when we do the hack and force set jailed = false)
// if err := sk.DeleteLastValidatorPower(ctx, valAddr); err != nil {
// return err
// }

// !IMPORTANT HACK: Set validator from jailed to not jailed to see what happens
// Okay so this like kind of worked for a split second
// Issue: the validator keeps trying to be converted to a jailed validator every single block when x/staking is calling it
val.Jailed = false
if err := sk.SetValidator(ctx, val); err != nil {
return err
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref: #163

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

new validators not being added post genesis
1 participant