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

chore: switch out espresso fork of blst #481

Merged
merged 2 commits into from
Feb 7, 2024
Merged

chore: switch out espresso fork of blst #481

merged 2 commits into from
Feb 7, 2024

Conversation

nomaxg
Copy link
Contributor

@nomaxg nomaxg commented Feb 6, 2024

I am working on integrating namespace proofs into Arbitrum and the Arbitrum prover relies on this crate that relies on another version of the native blst crate, resulting in the following compiler error:

   Updating git repository `https://github.com/EspressoSystems/tagged-base64.git`
error: failed to select a version for `blst`.
    ... required by package `jf-primitives v0.4.0-pre.0 (https://github.com/EspressoSystems/jellyfish#d9c1b634)`
    ... which satisfies git dependency `jf-primitives` of package `vid v0.1.0 (/Users/ngolub/Documents/projects/nitro-espresso-integration/arbitrator/wasm-libraries/vid)`
    ... which satisfies path dependency `vid` of package `jit v0.1.0 (/Users/ngolub/Documents/projects/nitro-espresso-integration/arbitrator/jit)`
versions that meet the requirements `*` are: 0.3.11

the package `blst` links to the native library `blst`, but it conflicts with a previous package which links to `blst` as well:
package `blst v0.3.11`
    ... which satisfies dependency `blst = "^0.3.11"` of package `c-kzg v0.4.0`
    ... which satisfies dependency `c-kzg = "^0.4.0"` of package `prover v0.1.0 (/Users/ngolub/Documents/projects/nitro-espresso-integration/arbitrator/prover)`
Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary. Try to adjust you
r dependencies so that only one package uses the links ='blst' value. For more information, see https://doc.rust-lang.org/cargo/reference/resolver.html#links.

Switching out our fork for the crates.io hosted blst crate seems to solve this issue.

@nomaxg nomaxg requested a review from mrain February 6, 2024 01:56
@nomaxg nomaxg changed the title Switch out espresso fork of blst chore: switch out espresso fork of blst Feb 6, 2024
Copy link
Contributor

@mrain mrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Pending CI

Copy link
Contributor

@ggutoski ggutoski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, though I am curious why it was ever switched to our fork. See comment below.

@@ -25,7 +25,7 @@ ark-pallas = "0.4.0"
ark-poly = "0.4.0"
ark-serialize = "0.4.0"
ark-std = { version = "0.4.0", default-features = false }
blst = { git = "https://github.com/EspressoSystems/blst.git", branch = "no-std", default-features = false } # TODO: pin to a tag or commit
blst = { version = "0.3.11", default-features = false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was originally changed to our fork in #254 . Prior to that it was

blst = "0.3.10"

@alxiong any reason for this switch? Any reason not to revert it now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably for no_std. IIRC we have CI for it

@ggutoski
Copy link
Contributor

ggutoski commented Feb 6, 2024

CI fail for some reason. I successfully ran ./scripts/run_tests.sh on my local machine with no problem. 🤷

@mrain
Copy link
Contributor

mrain commented Feb 6, 2024

CI fail for some reason. I successfully ran ./scripts/run_tests.sh on my local machine with no problem. 🤷

This failure only happens with rust nightly build 2024-02-05, other's are experiencing it as well

we can force merge it if it's urgent

@nomaxg
Copy link
Contributor Author

nomaxg commented Feb 6, 2024

CI fail for some reason. I successfully ran ./scripts/run_tests.sh on my local machine with no problem. 🤷

This failure only happens with rust nightly build 2024-02-05, other's are experiencing it as well

we can force merge it if it's urgent

No rush, looks like there’s a pending fix this morning so I’ll rerun ci later.

@mrain mrain merged commit bc8894f into main Feb 7, 2024
5 checks passed
@mrain mrain deleted the update-blst branch February 7, 2024 15:10
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.

None yet

3 participants