core: add new SignedData interface and impls #699
Conversation
| _ SignedData = VersionedSignedBeaconBlock{} | ||
| _ SignedData = Attestation{} | ||
| _ SignedData = Signature{} | ||
| _ SignedData = SignedVoluntaryExit{} |
There was a problem hiding this comment.
The 4 implementations of the new SignedData interface
Codecov Report
@@ Coverage Diff @@
## main #699 +/- ##
==========================================
- Coverage 55.12% 54.32% -0.80%
==========================================
Files 102 103 +1
Lines 9896 10126 +230
==========================================
+ Hits 5455 5501 +46
- Misses 3653 3820 +167
- Partials 788 805 +17
Continue to review full report at Codecov.
|
core/types.go
Outdated
| // SignedData is a signed duty data. | ||
| type SignedData interface { | ||
| // Signature returns the partial signature. | ||
| Signature() Signature | ||
| // SetSignature returns a copy of signed duty data with the signature replaced. | ||
| SetSignature(Signature) (SignedData, error) | ||
| // Marshaler returns the json serialised signed duty data (including the signature). | ||
| json.Marshaler | ||
| } | ||
|
|
||
| // ParSignedData2 is a partially signed duty data only signed by a single threshold BLS share. | ||
| // TODO(corver): Rename and place ParSignedData. | ||
| type ParSignedData2 struct { | ||
| SignedData | ||
| // ShareIdx returns the threshold BLS share index. | ||
| ShareIdx int | ||
| } | ||
|
|
There was a problem hiding this comment.
these are the two new types (one interface and one struct)
corverroos
changed the title
corverroos
changed the title
|
|
||
| // Attestation is a signed attestation and implements SignedData. | ||
| type Attestation struct { | ||
| eth2p0.Attestation |
There was a problem hiding this comment.
cannot subtype since eth2p0.Attestation's field Signature clashes with interface method Signature
There was a problem hiding this comment.
can we make this attestation private since we already have getter and setter methods(Signature and SetSignature)?
There was a problem hiding this comment.
Although I don't have any strong opinion on this.
There was a problem hiding this comment.
it cannot be made private since some components will cast and use the embedded type like bcast and aggsigdb. Also it is a value, and value can be public.
core/types.go
Outdated
| } | ||
| } | ||
|
|
||
| // Signature is a BLS12-381 Signature. It implements SignedData. | ||
| type Signature []byte |
There was a problem hiding this comment.
If this is an implementation of SignedData then why it is not present in signeddata.go?
There was a problem hiding this comment.
it was already present here, but sure can move it.
| var ( | ||
| _ SignedData = VersionedSignedBeaconBlock{} | ||
| _ SignedData = Attestation{} | ||
| _ SignedData = Signature{} |
There was a problem hiding this comment.
I assume Signature{} is to used by Randao and exchanger in dkg. Can we be more specific about those types and also accomodate those cases as well? Since in exchanger we require to share some data(like cluster-lock and deposit-data) not just signature.
There was a problem hiding this comment.
Yeah, it depends if if the data also needs to be shared with the signature, then we need another type, not just Signature which is ONLY a signature with the data being out-of-band / implicit. This is how Randao and DKG lockhash+depositdata is implemeneted, as signatures only.
For randao, the data is the epoch, it is implicit in the duty slot, so not shared in the data at this point.
For DKG, it is the lock hash and the depositdatahash, those datas are implicit in DKg instance so not shared at this point.
We can include that data if we want, but we do not do this at this point. So that would be another refactor I think.
|
|
||
| // Attestation is a signed attestation and implements SignedData. | ||
| type Attestation struct { | ||
| eth2p0.Attestation |
There was a problem hiding this comment.
can we make this attestation private since we already have getter and setter methods(Signature and SetSignature)?
|
|
||
| // Attestation is a signed attestation and implements SignedData. | ||
| type Attestation struct { | ||
| eth2p0.Attestation |
There was a problem hiding this comment.
Although I don't have any strong opinion on this.
corverroos
added
the
merge when ready
Integrates new `SignedData` interface and `ParSignedData` type. category: refactor ticket: #699
Step 1 of N: Adds the new interface and implementations and helper functions. Nothing wired or integrated yet.
category: refactor
ticket: #698