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

Rollback to Tendermint 0.34 #1271

Closed
5 tasks
redshiftzero opened this issue Aug 12, 2022 · 5 comments
Closed
5 tasks

Rollback to Tendermint 0.34 #1271

redshiftzero opened this issue Aug 12, 2022 · 5 comments
Assignees
Milestone

Comments

@redshiftzero
Copy link
Member

redshiftzero commented Aug 12, 2022

We would like to roll back to Tendermint 0.34 due to instability of the p2p layer, which caused Tendermint 0.35 (and 0.36) to be abandoned.

However, this is fairly difficult for us, because we depend on tendermint-rs to speak ABCI to Tendermint. The ABCI implementation was written a year and a half ago, targeting 0.35 (which was about to be released at the time). However, because of issues with the tendermint-rs release process (which will hopefully be resolved by informalsystems/tendermint-rs#1128), this code never made it into a published release, only prereleases, and can't be used drop-in with Tendermint 0.34, which has a different format of the length delimiter for ABCI messages.

Moreover, because we use tendermint-rs and ibc-rs (and tower-abci), which are interlinked, it's complicated to change one of them without changing both, especially because we cannot use semver to unify versions in git dependencies. Our options are:

  1. Wait for a 1.0 release of tendermint-rs and ibc-rs that has the ABCI domain types we wrote;
  2. Move completely to our own forks of tendermint-rs and ibc-rs, and also fork any other crates in our dep tree that use either to use our versions.

It's unfortunate that the plan for Tendermint is to wait for new features before doing an 0.37 release, because that pushes out the timeline for (1) and a return to a "normal" release process for tendermint-rs where we could use published versions with semver resolution and actually get PRs included in them. However, that decision was made already, so it seems like the next best option for us to get off of 0.34 would be to deeply fork the crates.

Beyond that decision, the steps are:

  • Backporting ABCI domain types changes
  • Edit all transitive dependents of tendermint-rs to use a version with those changes
  • Propagate those changes through tower-abci
  • Edit docker compose files to use the 0.34 version
  • Check to see if there's other API changes e.g. in RPC
@thanethomson
Copy link

If someone could assist with porting informalsystems/tendermint-rs#1022 to the tendermint-rs main branch (and ideally address informalsystems/tendermint-rs#1090, but not a blocker) we could easily cut a tendermint-rs v0.25 release that provides the ABCI domain types with Tendermint Core v0.34 support. I just haven't had the bandwidth to address this myself recently.

The 1.0 release goal involves supporting multiple versions of Tendermint Core from a single version of tendermint-rs - this doesn't seem like something you need in the short-term? In any case, there's a pretty small surface area diff between Tendermint Core v0.34 and v0.37, compared to the diff between Tendermint Core v0.34 and v0.35/v0.36.

So once the ABCI domain types work's done, doing the work necessary to cut a tendermint-rs 1.0 release that provides support for both Tendermint Core v0.34 and v0.37 should be a pretty small lift.

@hdevalence
Copy link
Member

hdevalence commented Sep 21, 2022

The bulk of the backporting work is now done here: informalsystems/tendermint-rs#1185

The ABCI domain types were mixed together with RPC changes to support 0.35, so backporting required disentangling the RPC changes from the domain types. While I was able to untangle those changes, the resulting RPC code is broken, and the tests don't pass. Those tests were unrelated to the ABCI domain types I wrote, so I don't know how to fix them. However, we don't use that part of the RPC code, so we can use that branch to escape Tendermint 0.35.

We've already had to fork the ibc-rs crates in #731 because of breakage resulting from the lack of semver-compatible releases of tendermint-rs.

Ideally, we'd be able to consume semver releases from upstream, but that won't be possible until the release process is fixed, and that's not scheduled to happen until some time in the future, after upstream tendermint-rs gets multi-version support.

Since we can't rely on releases from upstream, but we need to stop using Tendermint 0.35, we should move from our current fork to one that is 0.34 compatible. To do this, we should:

  • create a penumbra-034 branch on our fork of tendermint-rs, with the current contents of Rebase ABCI Domain Types onto v0.34.20 branch. informalsystems/tendermint-rs#1185
  • create a penumbra-034 branch on our fork of ibc-rs, using the penumbra-034 branch of tendermint-rs
  • create a penumbra-034 branch on our tower-abci repo, using the penumbra-034 branch of tendermint-rs
  • migrate all uses of the tendermint, ibc, and tower-abci crates in this repo to use those forks.

@thanethomson
Copy link

Ideally, we'd be able to consume semver releases from upstream, but that won't be possible until the release process is fixed, and that's not scheduled to happen until some time in the future, after upstream tendermint-rs gets multi-version support.

I've already offered to cut additional tendermint-rs "alias" releases off of the v0.23.x series to address the breaking changes in patch versions (an unfortunate historical consequence of only supporting a single version of Tendermint Core in tendermint-rs). For example, we could release v0.25.0 as an alias for v0.23.8, and v0.26.0 as an alias for v0.23.9. But @hdevalence said this wouldn't help?

@thanethomson
Copy link

Also, I have explicitly offered several times to cut a semver-compatible release prior to us implementing multi-version support, but the pre-requisite there is that the ABCI domain types work needs to be integrated first in a working way (something that'd be necessary to do for your fork anyways?).

@redshiftzero redshiftzero assigned avahowell and unassigned hdevalence Sep 22, 2022
@hdevalence
Copy link
Member

I've already offered to cut additional tendermint-rs "alias" releases off of the v0.23.x series to address the breaking changes in patch versions (an unfortunate historical consequence of only supporting a single version of Tendermint Core in tendermint-rs). For example, we could release v0.25.0 as an alias for v0.23.8, and v0.26.0 as an alias for v0.23.9. But @hdevalence said this wouldn't help?

This doesn't help us with the concrete problem of getting off of Tendermint 0.35, because it's not just tendermint-rs that we need to pin, but all of the sequence of crates that depend on it, and dealing with all of the entangled changes that we weren't able to pick up, as an unfortunate historical consequence of the tendermint-rs and ibc-rs processes.

We've been able to do that using the forking strategy described above, and we're now running on Tendermint 0.34 (with our own collection of forked crates). Those forks aren't something we want to maintain long-term; they're just a way to escape the 0.35 problems.

Having tendermint-rs and ibc-rs on a semver release process would definitely help avoid the same problem (having to pin, and then pinning causing accumulated changes, and then having to make painful upgrades all at once) from recurring in the future, though, and we'd like to work with you on getting there, now that we're able to run on 0.34.

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

No branches or pull requests

5 participants