penumbra icon indicating copy to clipboard operation
penumbra copied to clipboard

refactor(decaf377): bump dep

Open TalDerei opened this issue 1 year ago • 7 comments

Describe your changes

Updates the decaf377, decaf377-rdsa, and poseidon377 dep versions

Issue ticket number and link

References https://github.com/penumbra-zone/penumbra/issues/3676 and consumes changes in #3678. unblocked by https://github.com/penumbra-zone/decaf377/issues/101

Checklist before requesting a review

  • [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:

TalDerei avatar Feb 12 '24 23:02 TalDerei

While the circuits are passing in bin/pcli/tests/proof.rs we're still getting some failures in the penumbra-app package I'll spend today debugging.

TalDerei avatar Feb 15 '24 14:02 TalDerei

There's still smoke test failures that need to be resolved.

TalDerei avatar Mar 04 '24 17:03 TalDerei

currently soft blocked by https://github.com/penumbra-zone/decaf377/pull/91. Once that's merged, I can modify the decaf377, decaf377-rdsa, and poseidon377 deps in this branch to point to main.

TalDerei avatar Mar 04 '24 22:03 TalDerei

There's an arkworks workstream to support 32-bit limbs (64-bit multiplier):

  • https://github.com/arkworks-rs/algebra/pull/797
  • https://github.com/arkworks-rs/algebra/issues/769#issuecomment-1920661476

This PR will (maybe?) remain blocked until that work is merged. This is based on the observation that our fiat-crypto generated field impl (benchmarked in https://github.com/penumbra-zone/penumbra/pull/3976 and recorded in https://docs.google.com/spreadsheets/d/1DonNNCN6-Nm3SxSpOymkGYOQi2wAK0l3_CfuDPq4vQc/edit#gid=0) is currently slower than the arkworks backends

TalDerei avatar Mar 08 '24 06:03 TalDerei

@redshiftzero what's the current status of this?

TalDerei avatar May 04 '24 06:05 TalDerei

will be unblocked by work-stream push in https://github.com/penumbra-zone/decaf377/issues/101

TalDerei avatar May 16 '24 00:05 TalDerei

@redshiftzero @cronokirby rebased PR, and crate deps have been updated for decaf377 (0.5.0 to 0.10.0), decaf377-rdsa, and poseidon377. We have smoke test coverage, but still need to resolve the remaining CI failures. We should cut any necessary releases for decaf377-rdsa and poseidon377 now that their cargo files have been updated to use version 0.10.0 of decaf. I think we're close to finally closing the tracking issue.

TalDerei avatar Jul 01 '24 08:07 TalDerei

one last thing to figure out here actually is why we need the getrandom dependency added everywhere, since it's a dev dependency only of decaf377

redshiftzero avatar Jul 09 '24 17:07 redshiftzero

cargo tree --package penumbra-community-pool (the failing package here) on a5270922cafea8f89ed895e4376516bc90c82cb0 indicates that getrandom is being pulled in due to ark-ff:

penumbra-community-pool v0.79.0-alpha.1 
├── anyhow v1.0.86
├── ark-ff v0.4.2
│   ├── ark-ff-asm v0.4.2 (proc-macro)
│   │   ├── quote v1.0.36
│   │   │   └── proc-macro2 v1.0.86
│   │   │       └── unicode-ident v1.0.12
│   │   └── syn v1.0.109
│   │       ├── proc-macro2 v1.0.86 (*)
│   │       ├── quote v1.0.36 (*)
│   │       └── unicode-ident v1.0.12
│   ├── ark-ff-macros v0.4.2 (proc-macro)
│   │   ├── num-bigint v0.4.6
│   │   │   ├── num-integer v0.1.46
│   │   │   │   └── num-traits v0.2.19
│   │   │   │       [build-dependencies]
│   │   │   │       └── autocfg v1.3.0
│   │   │   └── num-traits v0.2.19 (*)
│   │   ├── num-traits v0.2.19 (*)
│   │   ├── proc-macro2 v1.0.86 (*)
│   │   ├── quote v1.0.36 (*)
│   │   └── syn v1.0.109 (*)
│   ├── ark-serialize v0.4.2
│   │   ├── ark-serialize-derive v0.4.2 (proc-macro)
│   │   │   ├── proc-macro2 v1.0.86 (*)
│   │   │   ├── quote v1.0.36 (*)
│   │   │   └── syn v1.0.109 (*)
│   │   ├── ark-std v0.4.0
│   │   │   ├── num-traits v0.2.19 (*)
│   │   │   ├── rand v0.8.5
│   │   │   │   ├── libc v0.2.155
│   │   │   │   ├── rand_chacha v0.3.1
│   │   │   │   │   ├── ppv-lite86 v0.2.17
│   │   │   │   │   └── rand_core v0.6.4
│   │   │   │   │       └── getrandom v0.2.15
│   │   │   │   │           ├── cfg-if v1.0.0
│   │   │   │   │           └── libc v0.2.155
│   │   │   │   └── rand_core v0.6.4 (*)
│   │   │   └── rayon v1.10.0
[snip]

i.e., ark-ff is pulling in ark-serialize which is pulling in ark-std which eventually pulls in rand_core and getrandom. rand_core pulling in getrandom here makes sense given feature unification across the workspace since rand_core elsewhere in the workspace uses the getrandom feature.

redshiftzero avatar Jul 09 '24 19:07 redshiftzero