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

Enforce Pigeon minimum version in Paloma #919

Merged

Conversation

MechanicalTyler
Copy link
Contributor

@MechanicalTyler MechanicalTyler commented Jul 14, 2023

Related Github tickets

Background

This has been a long time coming. We often release code that is incompatible with previous versions of Pigeon. Even when done in a backwards-compatible way, backwards compatibility isn't meant to extend backwards forever. We need a way of specifying the minimum version of Pigeon that the current version of Paloma works with. It's currently expected that any newer versions of Pigeon will also work with the current version of Paloma

The minimum required version of Pigeon is specified in the app.go file and should be mentioned in any proposed paloma update that goes through governance. Users can upgrade to the latest version of Pigeon ahead of the Paloma upgrade.

Part of this PR includes cleaning up extra places where we were calling the keepalive functionality. Keepalive should only happen in its own workflow, not as a tag-along to other workflows.

I also removed the ability to send a keepalive message with the CLI. I can come up with no reason why we would want to have this ability.

More info can be seen in the Epic: https://www.notion.so/volumefi/Epic-07-14-23-Governance-for-pigeon-version-check-8f8fd1dbd4d24a40a3007294f172c738

Testing completed

  • test coverage exists or has been added/updated
  • tested in a private testnet

Breaking changes

  • I have checked my code for breaking changes
  • If there are breaking changes, there is a supporting migration.

Tyler Ruppert added 6 commits July 14, 2023 09:09
There's no reason I can think of why this command should exist.  It
allows short circuiting pigeon and keeping a validator alive when they
should not be.
These don't belong here.  Pigeons should be sending their keepalives
separately from these other processes
app/app.go Show resolved Hide resolved
@MechanicalTyler
Copy link
Contributor Author

I'm marking this as ready for review. After discussion in Discord, it sounds like we're aligned and agree on this strategy

@MechanicalTyler MechanicalTyler marked this pull request as ready for review July 14, 2023 20:29
Copy link
Contributor

@byte-bandit byte-bandit left a comment

Choose a reason for hiding this comment

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

LGMT 👍

@MechanicalTyler MechanicalTyler merged commit 1c1b288 into palomachain:master Jul 17, 2023
@MechanicalTyler MechanicalTyler deleted the toaster/jail-old-pigeon-versions branch July 17, 2023 13:05
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.

3 participants