-
Notifications
You must be signed in to change notification settings - Fork 2
Description
// Type-level number for 3100 bytes (signature size) = 31 * 100
pub type U3100 = Prod<U31, U100>;
// Type-level number for 3116 bytes
pub type U3116 = Sum<U3100, U16>;
// Type alias for Signature
pub type Signature = ByteVector<U3116>;I believe there is no need to expose U3100 and U3116 to end user, as well as making them as separate types at all. It is pretty obvious what is happening there, so you can just do:
type U3116 = Sum<Prod<U31, U100>, U16>;
pub type Signature = ByteVector<U3116>;But another thing, shouldn't this be some more complicated type, and not just byte vector? I mean, it should at least have something like verify()? For example, as far as I've seen, ream uses special wrapper type around Signature, that supports verification and conversions to low-level leanSig type:
https://github.com/ReamLabs/ream/blob/e26d87dfdd1d3eec01ee261e6b14a130122fb91b/crates/crypto/post_quantum/src/leansig/signature.rs#L14-L18
So you probably want something similar here.
Moving on, in the same file (./lean_client/containers/src/attestation.rs), use appears after type definitions:
// Type-level number for 4096 (validator registry limit)
use typenum::U4096;it has to be at the top of the file.
Next:
/// Validator specific attestation wrapping shared attestation data.
#[derive(Clone, Debug, PartialEq, Eq, Ssz, Default, Serialize, Deserialize)]
pub struct Attestation {
/// The index of the validator making the attestation.
pub validator_id: Uint64, // <-----
/// The attestation data produced by the validator.
pub data: AttestationData,
}Why you've created wrapper around u64 type? You can just use builtin types, no need to wrap everything into structs. If you want to add some additional meaning to type, like ValidatorIndex, then you also don't need to make custom struct - you can just create type alias:
type ValidatorIndex = u64;And enjoy the convenience of u64 type, while having semantics of ValidatorIndex struct. Wrapping something inside struct has meaning only if you make some additional checks, and/or if you need additional functionality.
So exactly the same goes for Bytes32 struct - not needed.
UPDATE:
Also don't forget to remove all unused functions, after removing structs - e.g., hash_tree_root function in file ./lean_client/containers/src/block.rs is no longer needed, if there is no Bytes32 struct.