cluster: definition v1.8 #2823
cluster: definition v1.8 #2823
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2823 +/- ##
==========================================
+ Coverage 53.14% 53.34% +0.20%
==========================================
Files 200 200
Lines 27705 28004 +299
==========================================
+ Hits 14723 14939 +216
- Misses 11151 11204 +53
- Partials 1831 1861 +30 ☔ View full report in Codecov by Sentry. |
| @@ -685,6 +723,37 @@ func unmarshalDefinitionV1x5(data []byte) (def Definition, err error) { | |||
| }, nil | |||
| } | |||
|
|
|||
| func unmarshalDefinitionV1x8(data []byte) (def Definition, err error) { | |||
There was a problem hiding this comment.
shall we verify deposit amounts here only? 🤔
But that would be more implicit rather than doing it in the run function after unmarshallling definition.
There was a problem hiding this comment.
I am going to revisit verification end-to-end in the "final" integrating PR. The codebase of this PR does not have the verification method accessible yet.
cluster/distvalidator.go
Outdated
| type distValidatorJSONv1x8 struct { | ||
| PubKey ethHex `json:"distributed_public_key"` | ||
| PubShares []ethHex `json:"public_shares,omitempty"` | ||
| DepositData depositDataJSON `json:"deposit_data,omitempty"` |
There was a problem hiding this comment.
why do we have DepositData field here? this isn't used in v1.8.0?
There was a problem hiding this comment.
You are correct, removed.
| "deposit_data": { | ||
| "pubkey": "", | ||
| "withdrawal_credentials": "", | ||
| "amount": "0", | ||
| "signature": "" | ||
| }, |
There was a problem hiding this comment.
This shouldn't be in v1.8.0
There was a problem hiding this comment.
You are very right! My bad, now fixed.
| { | ||
| "pubkey": "0x1814be823350eab13935f31d84484517e924aef78ae151c00755925836b7075885650c30ec29a3703934bf50a28da102", | ||
| "withdrawal_credentials": "0x76b0620556304a3e3eae14c28d0cea39d2901a52720da85ca1e4b38eaf3f44c6", | ||
| "amount": "5919415281453547599", |
There was a problem hiding this comment.
this amount doesn't look right?
There was a problem hiding this comment.
This is randomly generated amount. It does not have to be 32ETH for example for the purpose of the test.
|
pinebit
added
the
merge when ready
Introducing cluster definition v1.8 with partial deposits.
category: feature
ticket: none