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

Shamir Secret Sharing #354

Merged
merged 7 commits into from
Jan 29, 2025
Merged

Shamir Secret Sharing #354

merged 7 commits into from
Jan 29, 2025

Conversation

aszepieniec
Copy link
Contributor

I think it is rather uncontroversial but it can't hurt to offer the opportunity for review.

@@ -49,14 +53,153 @@ pub const WALLET_DB_NAME: &str = "wallet";
pub const WALLET_OUTPUT_COUNT_DB_NAME: &str = "wallout_output_count_db";

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
struct SecretKeyMaterial(XFieldElement);
pub struct SecretKeyMaterial(XFieldElement);
Copy link
Member

Choose a reason for hiding this comment

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

Can we use pub(crate) here for SecretKeyMaterial and all its new associated functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that they are being used from within neptune-cli.rs, which lives in its own crate and therefore does not have access to structs and functions marked pub(crate).

Copy link
Member

@Sword-Smith Sword-Smith left a comment

Choose a reason for hiding this comment

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

Nice tests!

Consider factoring the new functionality into a new file/module, as we suffer from quite big files.

Can we use pub(crate) instead of pub for the new functions and for SecretKeyMaterial?

Also: I identified one typo.

@Sword-Smith Sword-Smith self-requested a review January 29, 2025 06:14
aszepieniec and others added 7 commits January 29, 2025 11:26
Add support for splitting secret key material into n shares such that:
 - combining *any* t < n of them is enough to reproduce the original
   secret; and
 - combining *fewer* than t yields no information about the original
   secret.

Addresses #325.
Add two CLI commands:
 - `shamir-share t n`: Split the wallet secret into n shares such
   that any t of them can reproduce the original secret with ...
 - `shamir-recombine t`: Combine t Shamir secret shares to
   reconstruct a wallet secret.

Closes #325.
Specifically, add chapter "User Guides" and page "installation".
The latter duplicates instructions from the README.md.
Use `println!` instead of `bail!` to inform user of absent wallet
file.

The wallet file being absent does not indicate a programmer or even
user error. The stack trace here is of no use to anyone.
Co-authored-by: Thorkil Schmidiger <[email protected]>
@aszepieniec aszepieniec merged commit afaae66 into master Jan 29, 2025
5 of 8 checks passed
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.

2 participants