Publish workspace to crates.io #4978

conorsch · 2025-01-10T17:22:06Z

Overview

We want to publish the crates in the protocol repo workspace to https://crates.io, so that external developers can depend on Penumbra code when writing downstream tooling. To date, we've recommended that external developers use git dependendencies in Rust projects, e.g.

penumbra-transaction = { git = "https://github.com/penumbra-zone/penumbra", rev = "ac7abacc9bb09503d6fd6a396bc0b6850079084e", features = [
   download-proving-keys",
] }

but doing so is not possible for projects already publishing their crates to crates.io. Two notable projects we intend to support are Astria (#3125) and Hermes. We're already maintaining a fork of Hermes, and publishing the crates would unblock us from submitting the Penumbra integration for Hermes to upstream.

Work to date

There's currently some WIP around the workspace consolidation happening in the following branches:

That work should be appended to, and organized in a feature branch that can be PR'd into main. In the process, we'll likely attempt to resolve both #4400 & #4682.

The text was updated successfully, but these errors were encountered:

erwanor · 2025-01-10T18:44:04Z

Thanks for writing this up and tracking it, fwiw:

In the process, we'll likely attempt to resolve both #4400 & #4682.

I think this is already done in v0.82.x branch. v1 reflection was only released as part of [email protected] iirc.

The release branch should have all of the ecosystem bumps (tonic, ibc-types, tendermint, tower, etc.)

conorsch · 2025-01-10T20:44:20Z

The release branch should have all of the ecosystem bumps (tonic, ibc-types, tendermint, tower, etc.)

That's true, the release branch does contain a commit that bumps the deps (#4973), but unfortunately it contains regressions like breaking HTTPS support for tonic clients. Fixing that regression is a bit of a side quest from the described goal of publishing the workspace, but worthwhile to get things straightened out. I'll follow up with PRs into main that describe the problem in detail.

## Describe your changes This test is off-by-default, given that it talks to a remote endpoint, and is rather slow. It can be run directly via: cargo nextest run --release -p pcli --features integration-testnet sync_wallet_on_public_testnet As usual, make sure to include the `--release` flag, otherwise it'll be much slower. ## Issue ticket number and link Related to changes described in #4978. Specifically, there were changes made in #4973 that broke ClientTls configs for tonic connections, but our test suite didn't catch that. ## Testing and review 1. Check this branch out locally and run `cargo nextest run --release -p pcli --features integration-testnet sync_wallet_on_public_testnet` 2. Confirm that test passes! It's also worth considering clicking this on in CI so we get assurance; I left it off by default but plan to ask for it to be used in subsequent testing towards #4978. Maybe we should just stick it in CI, at least temporarily? ## 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: > Test-only, no changes to app code. Intended to reduce the odds of breaking changes being merged.

erwanor · 2025-01-12T22:55:15Z

Thanks for the write-up, my understanding is that this is caused by the rustls backend change to aws-lc-rs (and away from ring). I think this might potentially cause platform issues down the road, but we'll figure it out when we get there (and hopefully, the ecosystem will beat us to it).

cargo add aws-lc-rs
let _ = aws_lc_rs::default_provider().install_default(); per process (so, e.g, one in `main.rs in the pcli bin)
Tracking the tls_config calls to make sure that they configured with the bundled certificates: tls_config(ClientTlsConfig::new().with_webpki_roots())?

I am not entirely sure that this is sufficient to unblock, but that's what I got from hacking on this a bit.

## 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]>

## Describe your changes Once again, we update significant dependencies on the main branch. Significantly, we've removed all git-dependencies from the workspace, which required upgrading `tonic` to upstream, which cascaded into other dependencies updates. With this change, we make the workspace nearly ready for publication to crates.io (#4978): still to come is a superficial refactor to rename the crates to `penumbra-sdk-*`, which I'll submit in a follow-up PR, to aid in review. This reverts commit a454870. ## Issue ticket number and link We've added and then removed this changeset a few times over the past week: * #4973 * #4980 * #4993 Now that we've tested it extensively, it's time to merge it into main and keep moving forward. Relevant version bumps were tracked in the following issues: * tonic #4400 * ibc-types #4682 * cnidarium #4956 * tendermint-rs (no issue) ## Testing and review In order to be certain that the changes honor protocol compatibility, I made sure to test syncing a mainnet fullnode based on this changeset from height 2622918—which is shortly after the change upgrade to 0.81.0—to 3136597, which is current height at time of writing. This gives us great confidence that the hard work toward ensuring that changes like #4682 were achieved in a compatible way. ## 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: > see testing notes above: we're confident we've done the needful with this patch Co-authored-by: Erwan Or <[email protected]> Co-authored-by: Erwan Or <[email protected]> Co-authored-by: Richard Janis Goldschmidt <[email protected]>

## Describe your changes Workspace refactor, preparing to publish the Penumbra crates. In doing so, renames all crates to have a prefix `penumbra-sdk-`, which is unique and controlled by PL. There's also a custom script that handles the publication action, which is non-standard. The major obstacle is that the `penumbra-sdk-proof-params` crate contains binary keyfiles, which are managed in git-lfs. We cannot upload the raw keyfiles to crates.io, because that'd result in a 100MB crate. Instead, we use a custom script to revert the binary keyfiles to plaintext lfs pointers immediately prior to publishing to crates.io, which stays under the limit, and allows third-party tools to opt into downloading the key material via the `download-proving-keys` feature. Given that `docs.rs` doesn't permit network access during crate build, we default to _warning_ about the missing keys during build of proof-params, recommending enabling the option. We could go further add fail the build, conditionally warning only in the [docsrs build context](https://docs.rs/about/builds). I chose to minimize special-casing as much as possible. ## Issue ticket number and link Towards #4978. ## Testing and review 1. Browse the recently published "alpha" crates: https://crates.io/search?q=penumbra-sdk 2. Try updating a Rust project that depends on Penumbra, as the [hermes fork](https://github.com/penumbra-zone/hermes) or the [reindexer](https://github.com/penumbra-zone/reindexer), to depend on the published crates. 3. Confirm that docs are available: https://docs.rs/penumbra-sdk-app/latest/penumbra_sdk_app/ ## 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: > Careful attention was paid to make this a non-breaking change. All the crates have been renamed, but the code behaves the same. Testing should focus on making sure that key material for the proof params crate is fetched correctly, to avoid breakage. --------- Co-authored-by: Erwan Or <[email protected]>

conorsch · 2025-01-21T19:29:31Z

The workspace has been published to crates.io as v1, e.g. https://crates.io/crates/penumbra-sdk-app/1.0.0. There's still a bit of follow-up required to finalize the overall release, but that's tracked in #4991.

Last remaining steps to wrap the workspace publication:

edit crate permissions to include primary maintainers via github auth
add workflow to publish crates on tag, same as we do the binaries.

Then we can close out this issue as complete.

conorsch · 2025-01-24T20:47:45Z

This work is done.

Last remaining steps to wrap the workspace publication

now tracked in #5005

conorsch self-assigned this Jan 10, 2025

github-actions bot added the needs-refinement unclear, incomplete, or stub issue that needs work label Jan 10, 2025

conorsch removed the needs-refinement unclear, incomplete, or stub issue that needs work label Jan 10, 2025

This was referenced Jan 10, 2025

test(pcli): sync wallet against testnet #4979

Merged

penumbra: update ecosystem tendermint/ibc crates #4980

Merged

This was referenced Jan 15, 2025

feat: publish workspace crates #4986

Merged

penumbra: update ecosystem tendermint/ibc crates #4997

Merged

conorsch mentioned this issue Jan 24, 2025

ci: add workflow for crate publishing #5005

Open

3 tasks

conorsch closed this as completed Jan 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Publish workspace to crates.io #4978

Publish workspace to crates.io #4978

conorsch commented Jan 10, 2025

erwanor commented Jan 10, 2025

conorsch commented Jan 10, 2025 •

edited

Loading

erwanor commented Jan 12, 2025

conorsch commented Jan 21, 2025

conorsch commented Jan 24, 2025

Publish workspace to crates.io #4978

Publish workspace to crates.io #4978

Comments

conorsch commented Jan 10, 2025

Overview

Work to date

erwanor commented Jan 10, 2025

conorsch commented Jan 10, 2025 • edited Loading

erwanor commented Jan 12, 2025

conorsch commented Jan 21, 2025

conorsch commented Jan 24, 2025

conorsch commented Jan 10, 2025 •

edited

Loading