Skip to content

Conversation

@dknopik
Copy link
Member

@dknopik dknopik commented Dec 5, 2024

This is the first part of my effort to make the eth2 crate more lightweight.

Proposed Changes

Remove ZeroizeString from account_utils, which uses the Zeroize derive macro from the zeroize crate, and use the Zeroizing<T> newtype provided by zeroize instead.

Our newtype did not add any value, except providing without_newlines, which replaced a one-liner and was used in only one location (outside of tests).

Additional Info

eth2 was depending on account_utils exclusively for ZeroizeString, which was the inspiration for this PR. Removing the account_utils dependency immediately made the crate lighter: cargo tree -p eth2 | wc -l went from 1995 down to 1865.

There are more newtypes scattered in the codebase that could be removed like this, but some arguably improve code readability by providing context specific names, such as DerivedKey and LamportSecretKey, so I left them alone for now.

@michaelsproul michaelsproul added ready-for-review The code is ready for review code-quality v7.0.0-beta.0 New release c. Q1 2025 labels Dec 9, 2024
Copy link
Member

@michaelsproul michaelsproul 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, thanks!

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Dec 9, 2024
dknopik and others added 2 commits December 10, 2024 08:36
thanks michael!

Co-authored-by: Michael Sproul <micsproul@gmail.com>
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

This is great! Let's lighten them dependencies

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Dec 11, 2024
@michaelsproul
Copy link
Member

@mergify queue

@mergify
Copy link

mergify bot commented Dec 11, 2024

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at a2b0009

@mergify mergify bot merged commit a2b0009 into sigp:unstable Dec 11, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-quality ready-for-merge This PR is ready to merge. v7.0.0-beta.0 New release c. Q1 2025

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants