solana icon indicating copy to clipboard operation
solana copied to clipboard

zeroize: Apply `zeroize` patch to all repository crates.

Open ilya-bobyr opened this issue 1 year ago • 8 comments

Similar to the changes in the main workspace Cargo.toml, we want to remove constraints for zeroize in the programs/sbf and other workspace crates.

See

commit a099c7a Author: Illia Bobyr [email protected] Date: Mon Oct 23 12:19:59 2023 -0700

zeroize: Allow versions newer than 1.3 for curve25519-dalek (#33516)

and

commit 01f1bf2 Author: Illia Bobyr [email protected] Date: Fri Oct 20 18:20:51 2023 -0700

zeroize: Allow versions newer than 1.3 for aes-gcm-siv (#33618)

ilya-bobyr avatar Oct 25 '23 00:10 ilya-bobyr

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.9%. Comparing base (9ffbe2a) to head (50c886a). Report is 855 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #33853   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         809      809           
  Lines      217717   217717           
=======================================
+ Hits       178345   178379   +34     
+ Misses      39372    39338   -34     

codecov[bot] avatar Oct 25 '23 02:10 codecov[bot]

Applying this PR makes the CI green for the whole repo, not just the main workspace: https://buildkite.com/solana-labs/solana/builds/103656. So, hopefully, it will make it possible to have green CI when we include a newer version of zeroize in the k8s client repo.

ilya-bobyr avatar Oct 25 '23 04:10 ilya-bobyr

Hello @ilya-bobyr ,

Currently, I'm working with a crate that demands curve25519-dalek version greater than 4, and due to the existing dependency issue, it's impossible to use this crate alongside Solana crates. This related issue (see #26688) shows that others have faced the same problem. The changes proposed in this PR are a much-needed solution to enable the use of both crates simultaneously.

Can you please share if there are plans to merge it in the near future?

Thank you!

tareknaser avatar Oct 26 '23 21:10 tareknaser

Hello @ilya-bobyr ,

Currently, I'm working with a crate that demands curve25519-dalek version greater than 4, and due to the existing dependency issue, it's impossible to use this crate alongside Solana crates. This related issue (see #26688) shows that others have faced the same problem. The changes proposed in this PR are a much-needed solution to enable the use of both crates simultaneously.

Can you please share if there are plans to merge it in the near future?

Thank you!

I think I will merge this PR in the next couple of days. Does it allow you to build your crate, one of the solana crates and curve25519-dalek version 4 all together?

Long term, it solana should certainly upgrade to a newer version of both curve25519-dalek and aes-gcm-siv. And remove these patches.

ilya-bobyr avatar Oct 28 '23 01:10 ilya-bobyr

This PR indeed resolves the zeroize pinning issue. However, it's worth mentioning that the patch for aes-gcm-siv still introduces a constraint on the subtle crate, limiting it to (>=2, <2.5). In my situation, this constraint still presents some difficulties. I'd suggest this patch instead:

[patch.crates-io.aes-gcm-siv]
git = "https://github.com/RustCrypto/AEADs"
rev = "e1e35e0c4f4943da0a99ceb8477c421dcfae2c33"

tareknaser avatar Oct 28 '23 11:10 tareknaser

I've tried this patch and solved my zeroize conflicts. Thank you for the solution.

palinko91 avatar Nov 16 '23 11:11 palinko91

@gregcusack @joncinque Should this be re-opened? These dependencies have been causing problems in Rust projects for a long time now. #26688

ronanyeah avatar Dec 08 '23 20:12 ronanyeah

Sorry, I dropped the ball on this. Got distracted with other work. Need to look into the proposed newer versions, to see if it is safe to use them.

ilya-bobyr avatar Dec 08 '23 20:12 ilya-bobyr

This repository is no longer in use. Please re-open this pull request in the agave repo: https://github.com/anza-xyz/agave

willhickey avatar Mar 03 '24 04:03 willhickey