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

cmd: create dkg cli to support partial deposits #2887

Merged
merged 3 commits into from
Feb 19, 2024

Conversation

pinebit
Copy link
Contributor

@pinebit pinebit commented Feb 15, 2024

Added --deposit-amounts flag to create dkg command. DKG procedure itself does not respect the partial amounts yet.
This PR only introduces the flag, validation with a bug fix, plus refactoring of writing deposit datas to avoid code duplication.

category: feature
ticket: #2888

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (3237887) 53.57% compared to head (6221766) 53.66%.

Files Patch % Lines
eth2util/deposit/deposit.go 76.31% 6 Missing and 3 partials ⚠️
cmd/createcluster.go 0.00% 0 Missing and 1 partial ⚠️
dkg/dkg.go 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2887      +/-   ##
==========================================
+ Coverage   53.57%   53.66%   +0.09%     
==========================================
  Files         200      200              
  Lines       28122    28131       +9     
==========================================
+ Hits        15065    15096      +31     
+ Misses      11190    11173      -17     
+ Partials     1867     1862       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

sonarcloud bot commented Feb 16, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue

Measures
0 Security Hotspots
No data about Coverage
0.9% Duplication on New Code

See analysis details on SonarCloud


// WriteDepositDataFile writes deposit-data-*eth.json file for the provided depositDatas.
// The amount will be reflected in the filename in ETH.
// All depositDatas amounts shall have equal values.
Copy link
Contributor

Choose a reason for hiding this comment

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

All depositDatas amounts shall have equal values.
I think it should say All depositDatas amounts should sum up to 32 ETH., right? It is not mandatory to have equal values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not in this case. This function writes the single deposit file for a node. In case of partial deposits this function is called multiple times per each partial amount x each node. The input slice here is the input for the single file. All amounts shall be equal.

@pinebit pinebit added the merge when ready Indicates bulldozer bot may merge when all checks pass label Feb 19, 2024
@obol-bulldozer obol-bulldozer bot merged commit 6ca08f3 into main Feb 19, 2024
12 checks passed
@obol-bulldozer obol-bulldozer bot deleted the pinebit/partial-deposits-create-dkg-cli branch February 19, 2024 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants