Skip to content

Commit

Permalink
penumbra: update ecosystem tendermint/ibc crates (#4980)
Browse files Browse the repository at this point in the history
## Describe your changes
This PR re-implements the changes from #4973, which were merged into the
`release/v0.82.x` branch, but never landed on main. I'm resubmitting
them so that we can address the HTTPS breakage, documented below, prior
to tackling the rest of the changes required for getting the workspace
crates published (#4978).

Continuation of #4963, into a release branch `v0.82.x` before tagging a
release candidate at that version and publishing the workspace using an
`alpha` version.

This handles the domain type change for upgradeable channels
(penumbra-zone/ibc-types#84) smoothly. It makes
sure to write default values to the new fields, which avoids wire
protocol changes, and makes this PR non consensus/state breaking.

Includes substantial version changes to:

* tendermint-rs
* tonic #4400
* ibc-types #4682
* cnidarium #4956

## Issue ticket number and link

This PR resubmits the changes in #4973, in an attempt to isolate
problematic behavior.

## Testing and review

The primary motivation for this changeset was to address the following
error, which occurred when I tried to sync a wallet against testnet
(using an HTTPS connection):
```
2025-01-10T22:41:38.135876Z DEBUG load_or_initialize{path=Some("/tmp/nix-shell.5VH2AV/tmp.4Ay22mKmyY/pcli-view.sqlite") url=https://testnet.plinfra.net/}: penumbra_view::storage: database does not exist path="/tmp/nix-shell.5VH2AV/tmp.4Ay22mKmyY/pcli-view.sqlite"
thread 'main' panicked at /home/conor/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.23.20/src/crypto/mod.rs:249:14:
no process-level CryptoProvider available -- call CryptoProvider::install_default() before this point
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

The most recent commit on this branch addresses that problem, by

1. explicitly configuring a default crypto provider via
[rustls](https://docs.rs/rustls)
2. reusing a new `ViewServer::get_pd_channel` method throughout the
codebase to handle conditional TLS config

Feedback welcome on whether the new logic is clearly documented and
stored in the right place.

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

> There are significant version bumps in this patch set, but previous
discussion concluded that the changes are not consensus-breaking; see
#4682 (comment)

---------

Co-authored-by: Erwan Or <[email protected]>
Co-authored-by: Richard Janis Goldschmidt <[email protected]>
  • Loading branch information
3 people authored Jan 14, 2025
1 parent 94d6812 commit 64c32ef
Show file tree
Hide file tree
Showing 173 changed files with 6,441 additions and 19,949 deletions.
1 change: 0 additions & 1 deletion .github/workflows/buf-pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ jobs:
./deployments/scripts/protobuf-codegen
# https://github.com/penumbra-zone/penumbra/issues/3038#issuecomment-1722534133
git checkout crates/proto/src/gen/proto_descriptor.bin.no_lfs
git checkout crates/cnidarium/src/gen/proto_descriptor.bin.no_lfs
s="$(git status --porcelain)"
if [[ -n "$s" ]]; then
echo "ERROR: protobuf files must be regenerated and committed."
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ jobs:
fi
- name: Run tests with nextest
run: cargo nextest run --release --features migration
run: cargo nextest run --release
env:
CARGO_TERM_COLOR: always

Expand Down
Loading

0 comments on commit 64c32ef

Please sign in to comment.