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

refactor: move remaining code in penumbra-crypto to penumbra-chain #2798

Merged
merged 4 commits into from
Jul 10, 2023

Conversation

redshiftzero
Copy link
Member

Closes #2765

@redshiftzero redshiftzero temporarily deployed to smoke-test July 7, 2023 02:08 — with GitHub Actions Inactive
@redshiftzero redshiftzero temporarily deployed to smoke-test July 7, 2023 14:01 — with GitHub Actions Inactive
@redshiftzero
Copy link
Member Author

I split out #2800 into another PR since this PR is failing the wasm build and I need to debug further to understand why.

@redshiftzero redshiftzero changed the title refactor: move remaining code in penumbra-crypto to penumbra-component wip - refactor: move remaining code in penumbra-crypto to penumbra-component Jul 7, 2023
@redshiftzero redshiftzero temporarily deployed to smoke-test July 10, 2023 17:17 — with GitHub Actions Inactive
@redshiftzero redshiftzero temporarily deployed to smoke-test July 10, 2023 19:00 — with GitHub Actions Inactive
@redshiftzero redshiftzero changed the title wip - refactor: move remaining code in penumbra-crypto to penumbra-component wip - refactor: move remaining code in penumbra-crypto to penumbra-chain Jul 10, 2023
@conorsch
Copy link
Contributor

We aim to land this PR prior to shipping Testnet 56 (#2761).

@redshiftzero
Copy link
Member Author

redshiftzero commented Jul 10, 2023

For posterity, the issue with the wasm build was:

On main,

  • penumbra-component depends on penumbra-storage
  • penumbra-storage depends on tokio

In this PR previously:

  • penumbra-component is added to the dependencies for penumbra-transaction, among other crates that need to be wasm-friendly -> the problem
  • so the approach of moving effect_hash.rs to penumbra-component just doesn't work

Two ways to address this would be to:

  1. either move effect_hash.rs to penumbra-chain (already a dependency of penumbra-wasm ) OR
  2. make a minimal crate e.g. penumbra-effect-hash that can be used by penumbra-transaction and the various penumbra component crates

I've done approach 1 here to resolve since the amount of code in question is fairly small.

@redshiftzero redshiftzero changed the title wip - refactor: move remaining code in penumbra-crypto to penumbra-chain refactor: move remaining code in penumbra-crypto to penumbra-chain Jul 10, 2023
@redshiftzero redshiftzero merged commit 465f037 into main Jul 10, 2023
@redshiftzero redshiftzero deleted the crypto-del branch July 10, 2023 19:33
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.

Proposed penumbra-crypto code reorganization
2 participants