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

Vc/multi shares #59

Merged
merged 9 commits into from
Dec 10, 2023
Merged

Vc/multi shares #59

merged 9 commits into from
Dec 10, 2023

Conversation

vadiminc
Copy link
Contributor

@vadiminc vadiminc commented Dec 5, 2023

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

  • Tests pass
  • Appropriate changes to README are included in PR

@vadiminc vadiminc changed the base branch from main to vc/improve-sanitaze December 5, 2023 10:04
Copy link

@alexstolr alexstolr left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -56,7 +57,18 @@ async function main() {
privateKey
});

const keyShares = new KeyShares();

Choose a reason for hiding this comment

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

You can add an argument of optional list of key share items to the constructor instead of adding after constructed.

Copy link
Contributor

Choose a reason for hiding this comment

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

For higher flexibility of SDK it can be implemented.
@vadiminc please add this possibility

Copy link
Contributor

@meshin-dev meshin-dev left a comment

Choose a reason for hiding this comment

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

Please check comments

README.md Outdated

- keystore-path (kp) = The validator keystore file/folder path, if a folder is provided all keystore files within the provided folder will be split according to the provided arguments
- keystore (ks) = The validator keystore file path. Only one keystore file can be specified using this argument
- keystore-path (kp) = The path to the folder containing validator keystore files. If a folder is provided, all keystore files within the provided folder will be split according to the provided arguments. This argument should not be used together with the `keystore`` argument
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove double ``

@@ -56,7 +57,18 @@ async function main() {
privateKey
});

const keyShares = new KeyShares();
Copy link
Contributor

Choose a reason for hiding this comment

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

For higher flexibility of SDK it can be implemented.
@vadiminc please add this possibility

@@ -1,6 +1,6 @@
{
"name": "ssv-keys",
"version": "1.0.5-dev",
"version": "1.0.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

Use 1.1.0 version, it's breaking changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not possible for now, as it won't support old files format anymore (single shares)

// Using a helper function to process each item
const processItem = async (item: any) => {
const keySharesItem = new KeySharesItem();
await keySharesItem.fromJson(item);
Copy link
Contributor

Choose a reason for hiding this comment

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

From json should be static class method and return class instance.

// Process each item in the array
for (const item of body.shares) {
const processedItem = await processItem(item);
this.shares.push(processedItem);
Copy link
Contributor

Choose a reason for hiding this comment

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

this.shares.push(KeySharesItem.fromJson(item));

@vadiminc vadiminc merged commit 34c074c into vc/improve-sanitaze Dec 10, 2023
9 checks passed
@vadiminc vadiminc deleted the vc/multi-shares branch December 10, 2023 14:37
vadiminc added a commit that referenced this pull request Dec 12, 2023
* improve sanitaze function

* update build files

* fix lint

* fix lint

* update help messages

* upda operator public key validation and unify the way how to do that

* fix readme

* forgotten commit

* tag 1.0.5-dev

* Vc/multi shares (#59)

* multi-shares logic and improvements in design

* new logic

* fix examples and tests

* import/export improvement

* update examples

* updated tests

* updated tests

* add keyshares construct init new flow

* update build

* Vc/more extended tests and errors (#61)

* add more covered tests and updated errors

* version up to 1.0.7
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.

5 participants