feat(crypto): Add extra types and conversions #517
Conversation
AndrewWestberg
force-pushed
the
amw/improvements
branch
from
7fa8938 to
4caa94e
Compare
AndrewWestberg
force-pushed
the
amw/improvements
branch
from
4caa94e to
5db91a6
Compare
| pub enum VrfDerivation { | ||
| Leader, | ||
| Nonce, | ||
| } | ||
|
|
||
| pub fn derive_tagged_vrf_output( | ||
| block_vrf_output_bytes: &[u8], | ||
| derivation: VrfDerivation, | ||
| ) -> Vec<u8> { | ||
| let mut tagged_vrf: Vec<u8> = match derivation { | ||
| VrfDerivation::Leader => vec![0x4C_u8], /* "L" */ | ||
| VrfDerivation::Nonce => vec![0x4E_u8], /* "N" */ | ||
| }; | ||
|
|
||
| tagged_vrf.extend(block_vrf_output_bytes); | ||
| Hasher::<256>::hash(&tagged_vrf).to_vec() | ||
| } | ||
|
|
||
| impl HeaderBody { | ||
| pub fn leader_vrf_output(&self) -> Vec<u8> { | ||
| derive_tagged_vrf_output(&self.vrf_result.0, VrfDerivation::Leader) | ||
| } | ||
|
|
||
| pub fn nonce_vrf_output(&self) -> Vec<u8> { | ||
| derive_tagged_vrf_output(&self.vrf_result.0, VrfDerivation::Nonce) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm not sure if we should put this logic in primitives. Why are we moving it from traverse to here?
There was a problem hiding this comment.
I moved it here because we said we didn't want to use pallas-traverse in Amaru. I believe it belongs in primitives because a leader_vrf and nonce_vrf exist in Alonzo and earlier headers. In babbage and beyond, these values are calculated instead of included in the header itself. I would consider these values as part of the header even though they're not explicitly in the header.
There was a problem hiding this comment.
I approved and merged the PR to move along, but I think that adding business logic to a specific era in the primitives crate is a slippery slope. I admit that this particular case is pretty constrained and harmless, but I just want to state that the original scope of the primitives crate is to hold data structures matching the CDDL and their corresponding encoding / decoding logic.
@KtorZ advocates for leaving pallas-traverse outside of the shared kernel, I'm still on the fence. What I don't like is slowly merging business logic into primitives just to accommodate for this decision.
Pallas implementors need an api layer that is era-agnostic and more ergonomic than CDDL structures. If Amaru doesn't want to use pallas-traverse, then I believe this api layer should be Amaru-specific and live within it's own repo.
AndrewWestberg
force-pushed
the
amw/improvements
branch
from
5db91a6 to
252714d
Compare
Adding some ergonomic improvements to make Amaru code less ugly.