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

STR-728: Key Derivation Crate #520

Merged
merged 62 commits into from
Dec 13, 2024
Merged

STR-728: Key Derivation Crate #520

merged 62 commits into from
Dec 13, 2024

Conversation

storopoli
Copy link
Member

@storopoli storopoli commented Dec 4, 2024

Description

Adds the Key Derivation Library Crate.

TODO

  • Tests signing a transaction using a Bridge Operator's xpriv
  • Zeroize/ZeroizeOnDrop
    • Operator keys
    • Sequencer key
  • Sequencer keys
  • Refactor bridge-client CLI to use the key derivation crate.
  • Refactor sequencer binaries/libs to use the key derivation crate.
  • Refactor datatool to use key-derivation crate.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

The key derivation crate moves the logic of key derivation away from the bridge and sequencer binaries into a separate standalone crate.
This will be equipped with proper tests and error types.

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

STR-728

.github/CODEOWNERS Show resolved Hide resolved
crates/key-derivation/src/error.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 56.28672% with 372 lines in your changes missing coverage. Please review.

Project coverage is 56.68%. Comparing base (8c6ed31) to head (900251c).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
bin/datatool/src/util.rs 0.00% 270 Missing ⚠️
bin/bridge-client/src/xpriv.rs 0.00% 35 Missing ⚠️
crates/key-derivation/src/operator.rs 89.23% 31 Missing ⚠️
crates/key-derivation/src/sequencer.rs 91.30% 10 Missing ⚠️
bin/strata-client/src/helpers.rs 0.00% 8 Missing ⚠️
bin/datatool/src/args.rs 0.00% 7 Missing ⚠️
bin/bridge-client/src/modes/operator/bootstrap.rs 0.00% 3 Missing ⚠️
crates/key-derivation/src/error.rs 0.00% 3 Missing ⚠️
crates/primitives/src/keys.rs 94.64% 3 Missing ⚠️
bin/datatool/src/main.rs 0.00% 2 Missing ⚠️
@@            Coverage Diff             @@
##             main     #520      +/-   ##
==========================================
+ Coverage   56.12%   56.68%   +0.55%     
==========================================
  Files         298      303       +5     
  Lines       30528    30999     +471     
==========================================
+ Hits        17135    17571     +436     
- Misses      13393    13428      +35     
Files with missing lines Coverage Δ
bin/bridge-client/src/args.rs 0.00% <ø> (ø)
bin/bridge-client/src/main.rs 0.00% <ø> (ø)
bin/strata-client/src/main.rs 0.00% <ø> (ø)
crates/primitives/src/buf.rs 86.95% <100.00%> (+3.62%) ⬆️
crates/primitives/src/constants.rs 100.00% <100.00%> (ø)
bin/datatool/src/main.rs 0.00% <0.00%> (ø)
bin/bridge-client/src/modes/operator/bootstrap.rs 0.00% <0.00%> (ø)
crates/key-derivation/src/error.rs 0.00% <0.00%> (ø)
crates/primitives/src/keys.rs 94.64% <94.64%> (ø)
bin/datatool/src/args.rs 0.00% <0.00%> (ø)
... and 5 more

... and 4 files with indirect coverage changes

crates/key-derivation/Cargo.toml Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Show resolved Hide resolved
@storopoli storopoli marked this pull request as ready for review December 9, 2024 20:14
@storopoli storopoli requested review from a team as code owners December 9, 2024 20:14
.github/CODEOWNERS Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
bin/bridge-client/src/xpriv.rs Outdated Show resolved Hide resolved
bin/bridge-client/src/xpriv.rs Show resolved Hide resolved
bin/bridge-client/src/xpriv.rs Show resolved Hide resolved
bin/datatool/src/main.rs Outdated Show resolved Hide resolved
bin/strata-client/src/helpers.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/sequencer.rs Outdated Show resolved Hide resolved
bin/datatool/src/args.rs Outdated Show resolved Hide resolved
bin/datatool/src/args.rs Outdated Show resolved Hide resolved
@storopoli storopoli requested a review from a team as a code owner December 11, 2024 12:54
@john-light john-light changed the title STR-470: Key Derivation Crate STR-728: Key Derivation Crate Dec 11, 2024
Cargo.toml Show resolved Hide resolved
bin/bridge-client/src/xpriv.rs Outdated Show resolved Hide resolved
bin/bridge-client/src/xpriv.rs Outdated Show resolved Hide resolved
bin/datatool/README.md Show resolved Hide resolved
bin/datatool/src/util.rs Outdated Show resolved Hide resolved
bin/datatool/src/util.rs Outdated Show resolved Hide resolved
crates/key-derivation/src/operator.rs Show resolved Hide resolved
crates/key-derivation/src/operator.rs Show resolved Hide resolved
crates/key-derivation/src/operator.rs Show resolved Hide resolved
crates/primitives/src/buf.rs Show resolved Hide resolved
Signed-off-by: Jose Storopoli <[email protected]>
Signed-off-by: Jose Storopoli <[email protected]>
Signed-off-by: Jose Storopoli <[email protected]>
Signed-off-by: Jose Storopoli <[email protected]>
@delbonis
Copy link
Contributor

good comments!

bin/bridge-client/src/xpriv.rs Show resolved Hide resolved
bin/datatool/src/util.rs Show resolved Hide resolved
bin/datatool/src/util.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@AaronFeickert AaronFeickert left a comment

Choose a reason for hiding this comment

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

LGTM

bin/datatool/src/util.rs Show resolved Hide resolved
@storopoli storopoli dismissed delbonis’s stale review December 12, 2024 21:36

new review needed

@storopoli storopoli merged commit 0011f19 into main Dec 13, 2024
18 checks passed
@storopoli storopoli deleted the STR-470 branch December 13, 2024 12:06
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.

4 participants