refactor(tvix/store): use strictly typed PathInfo struct r/8787

This switches the PathInfoService trait from using the proto-derived
PathInfo struct to a more restrictive struct, and updates all
implementations to use it.

It removes a lot of the previous conversion and checks, as invalid
states became nonrepresentable, and validations are expressed on the
type level.

PathInfoService implementations consuming protobuf need to convert and
do the verification internally, and can only return the strongly typed
variant.

The nix_compat::narinfo::NarInfo conversions for the proto PathInfo
are removed, we only keep a version showing a NarInfo representation for
the strong struct.

Converting back to a PathInfo requires the root node now, but is
otherwise trivial, so left to the users.

Co-Authored-By: Florian Klink <flokli@flokli.de>
Change-Id: I6fdfdb44063efebb44a8f0097b6b81a828717e03
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12588
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI

3 files changed, 309 insertions, 556 deletions

diff --git a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs
index 60da73012df7..5da3b23c269c 100644
--- a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs
+++ b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs
@@ -1,5 +1,5 @@
 use crate::nar::{NarCalculationService, RenderError};
-use crate::pathinfoservice::PathInfoService;
+use crate::pathinfoservice::{PathInfo, PathInfoService};
 use crate::proto;
 use futures::{stream::BoxStream, TryStreamExt};
 use std::ops::Deref;
@@ -44,7 +44,7 @@ where
                     .map_err(|_e| Status::invalid_argument("invalid output digest length"))?;
                 match self.path_info_service.get(digest).await {
                     Ok(None) => Err(Status::not_found("PathInfo not found")),
-                    Ok(Some(path_info)) => Ok(Response::new(path_info)),
+                    Ok(Some(path_info)) => Ok(Response::new(proto::PathInfo::from(path_info))),
                     Err(e) => {
                         warn!(err = %e, "failed to get PathInfo");
                         Err(e.into())
@@ -56,12 +56,15 @@ where
 
     #[instrument(skip_all)]
     async fn put(&self, request: Request<proto::PathInfo>) -> Result<Response<proto::PathInfo>> {
-        let path_info = request.into_inner();
+        let path_info_proto = request.into_inner();
+
+        let path_info = PathInfo::try_from(path_info_proto)
+            .map_err(|e| Status::invalid_argument(format!("Invalid path info: {e}")))?;
 
         // Store the PathInfo in the client. Clients MUST validate the data
         // they receive, so we don't validate additionally here.
         match self.path_info_service.put(path_info).await {
-            Ok(path_info_new) => Ok(Response::new(path_info_new)),
+            Ok(path_info_new) => Ok(Response::new(proto::PathInfo::from(path_info_new))),
             Err(e) => {
                 warn!(err = %e, "failed to put PathInfo");
                 Err(e.into())
@@ -99,6 +102,7 @@ where
         let stream = Box::pin(
             self.path_info_service
                 .list()
+                .map_ok(proto::PathInfo::from)
                 .map_err(|e| Status::internal(e.to_string())),
         );
 
diff --git a/tvix/store/src/proto/mod.rs b/tvix/store/src/proto/mod.rs
index f3ea4b196946..807f03854ddc 100644
--- a/tvix/store/src/proto/mod.rs
+++ b/tvix/store/src/proto/mod.rs
@@ -4,7 +4,7 @@ use bytes::Bytes;
 use data_encoding::BASE64;
 // https://github.com/hyperium/tonic/issues/1056
 use nix_compat::{
-    narinfo::Flags,
+    narinfo::{Signature, SignatureError},
     nixhash::{CAHash, NixHash},
     store_path::{self, StorePathRef},
 };
@@ -17,6 +17,8 @@ pub use grpc_pathinfoservice_wrapper::GRPCPathInfoServiceWrapper;
 
 tonic::include_proto!("tvix.store.v1");
 
+use tvix_castore::proto as castorepb;
+
 #[cfg(feature = "tonic-reflection")]
 /// Compiled file descriptors for implementing [gRPC
 /// reflection](https://github.com/grpc/grpc/blob/master/doc/server-reflection.md) with e.g.
@@ -70,183 +72,18 @@ pub enum ValidatePathInfoError {
     /// The deriver field is invalid.
     #[error("deriver field is invalid: {0}")]
     InvalidDeriverField(store_path::Error),
-}
 
-/// Parses a root node name.
-///
-/// On success, this returns the parsed [store_path::StorePathRef].
-/// On error, it returns an error generated from the supplied constructor.
-fn parse_node_name_root<E>(
-    name: &[u8],
-    err: fn(Vec<u8>, store_path::Error) -> E,
-) -> Result<store_path::StorePathRef<'_>, E> {
-    store_path::StorePathRef::from_bytes(name).map_err(|e| err(name.to_vec(), e))
-}
+    /// The narinfo field is missing
+    #[error("The narinfo field is missing")]
+    NarInfoFieldMissing,
 
-impl PathInfo {
-    /// validate performs some checks on the PathInfo struct,
-    /// Returning either a [store_path::StorePath] of the root node, or a
-    /// [ValidatePathInfoError].
-    pub fn validate(&self) -> Result<store_path::StorePath<String>, ValidatePathInfoError> {
-        // ensure the references have the right number of bytes.
-        for (i, reference) in self.references.iter().enumerate() {
-            if reference.len() != store_path::DIGEST_SIZE {
-                return Err(ValidatePathInfoError::InvalidReferenceDigestLen(
-                    i,
-                    reference.len(),
-                ));
-            }
-        }
+    /// The ca field is invalid
+    #[error("The ca field is invalid: {0}")]
+    InvalidCaField(ConvertCAError),
 
-        // If there is a narinfo field populated…
-        if let Some(narinfo) = &self.narinfo {
-            // ensure the nar_sha256 digest has the correct length.
-            if narinfo.nar_sha256.len() != 32 {
-                return Err(ValidatePathInfoError::InvalidNarSha256DigestLen(
-                    narinfo.nar_sha256.len(),
-                ));
-            }
-
-            // ensure the number of references there matches PathInfo.references count.
-            if narinfo.reference_names.len() != self.references.len() {
-                return Err(ValidatePathInfoError::InconsistentNumberOfReferences(
-                    self.references.len(),
-                    narinfo.reference_names.len(),
-                ));
-            }
-
-            // parse references in reference_names.
-            for (i, reference_name_str) in narinfo.reference_names.iter().enumerate() {
-                // ensure thy parse as (non-absolute) store path
-                let reference_names_store_path = store_path::StorePathRef::from_bytes(
-                    reference_name_str.as_bytes(),
-                )
-                .map_err(|_| {
-                    ValidatePathInfoError::InvalidNarinfoReferenceName(
-                        i,
-                        reference_name_str.to_owned(),
-                    )
-                })?;
-
-                // ensure their digest matches the one at self.references[i].
-                {
-                    // This is safe, because we ensured the proper length earlier already.
-                    let reference_digest = self.references[i].to_vec().try_into().unwrap();
-
-                    if reference_names_store_path.digest() != &reference_digest {
-                        return Err(
-                            ValidatePathInfoError::InconsistentNarinfoReferenceNameDigest(
-                                i,
-                                reference_digest,
-                                *reference_names_store_path.digest(),
-                            ),
-                        );
-                    }
-                }
-
-                // If the Deriver field is populated, ensure it parses to a
-                // [store_path::StorePath].
-                // We can't check for it to *not* end with .drv, as the .drv files produced by
-                // recursive Nix end with multiple .drv suffixes, and only one is popped when
-                // converting to this field.
-                if let Some(deriver) = &narinfo.deriver {
-                    store_path::StorePathRef::from_name_and_digest(&deriver.name, &deriver.digest)
-                        .map_err(ValidatePathInfoError::InvalidDeriverField)?;
-                }
-            }
-        }
-
-        // Ensure there is a (root) node present, and it properly parses to a [store_path::StorePath].
-        let root_nix_path = match &self.node {
-            None => Err(ValidatePathInfoError::NoNodePresent)?,
-            Some(node) => {
-                // NOTE: We could have some PathComponent not allocating here,
-                // so this can return StorePathRef.
-                // However, as this will get refactored away to stricter types
-                // soon anyways, there's no point.
-                let (name, _node) = node
-                    .clone()
-                    .into_name_and_node()
-                    .map_err(ValidatePathInfoError::InvalidRootNode)?;
-
-                // parse the name of the node itself and return
-                parse_node_name_root(name.as_ref(), ValidatePathInfoError::InvalidNodeName)?
-                    .to_owned()
-            }
-        };
-
-        // return the root nix path
-        Ok(root_nix_path)
-    }
-
-    /// With self and its store path name, this reconstructs a
-    /// [nix_compat::narinfo::NarInfo<'_>].
-    /// It can be used to validate Signatures, or get back a (sparse) NarInfo
-    /// struct to prepare writing it out.
-    ///
-    /// It assumes self to be validated first, and will only return None if the
-    /// `narinfo` field is unpopulated.
-    ///
-    /// It does very little allocation (a Vec each for `signatures` and
-    /// `references`), the rest points to data owned elsewhere.
-    ///
-    /// Keep in mind this is not able to reconstruct all data present in the
-    /// NarInfo<'_>, as some of it is not stored at all:
-    /// - the `system`, `file_hash` and `file_size` fields are set to `None`.
-    /// - the URL is set to an empty string.
-    /// - Compression is set to "none"
-    ///
-    /// If you want to render it out to a string and be able to parse it back
-    /// in, at least URL *must* be set again.
-    pub fn to_narinfo<'a>(
-        &'a self,
-        store_path: store_path::StorePathRef<'a>,
-    ) -> Option<nix_compat::narinfo::NarInfo<'_>> {
-        let narinfo = &self.narinfo.as_ref()?;
-
-        Some(nix_compat::narinfo::NarInfo {
-            flags: Flags::empty(),
-            store_path,
-            nar_hash: narinfo
-                .nar_sha256
-                .as_ref()
-                .try_into()
-                .expect("invalid narhash"),
-            nar_size: narinfo.nar_size,
-            references: narinfo
-                .reference_names
-                .iter()
-                .map(|ref_name| {
-                    // This shouldn't pass validation
-                    StorePathRef::from_bytes(ref_name.as_bytes()).expect("invalid reference")
-                })
-                .collect(),
-            signatures: narinfo
-                .signatures
-                .iter()
-                .map(|sig| {
-                    nix_compat::narinfo::SignatureRef::new(
-                        &sig.name,
-                        // This shouldn't pass validation
-                        sig.data[..].try_into().expect("invalid signature len"),
-                    )
-                })
-                .collect(),
-            ca: narinfo
-                .ca
-                .as_ref()
-                .map(|ca| ca.try_into().expect("invalid ca")),
-            system: None,
-            deriver: narinfo.deriver.as_ref().map(|deriver| {
-                StorePathRef::from_name_and_digest(&deriver.name, &deriver.digest)
-                    .expect("invalid deriver")
-            }),
-            url: "",
-            compression: Some("none"),
-            file_hash: None,
-            file_size: None,
-        })
-    }
+    /// The signature at position is invalid
+    #[error("The signature at position {0} is invalid: {1}")]
+    InvalidSignature(usize, SignatureError),
 }
 
 /// Errors that can occur when converting from a [nar_info::Ca] to a (stricter)
@@ -341,45 +178,154 @@ impl From<&nix_compat::nixhash::CAHash> for nar_info::Ca {
     }
 }
 
-impl From<&nix_compat::narinfo::NarInfo<'_>> for NarInfo {
-    /// Converts from a NarInfo (returned from the NARInfo parser) to the proto-
-    /// level NarInfo struct.
-    fn from(value: &nix_compat::narinfo::NarInfo<'_>) -> Self {
-        let signatures = value
-            .signatures
-            .iter()
-            .map(|sig| nar_info::Signature {
-                name: sig.name().to_string(),
-                data: Bytes::copy_from_slice(sig.bytes()),
-            })
-            .collect();
-
-        NarInfo {
-            nar_size: value.nar_size,
-            nar_sha256: Bytes::copy_from_slice(&value.nar_hash),
-            signatures,
-            reference_names: value.references.iter().map(|r| r.to_string()).collect(),
-            deriver: value.deriver.as_ref().map(|sp| StorePath {
-                name: (*sp.name()).to_owned(),
-                digest: Bytes::copy_from_slice(sp.digest()),
-            }),
-            ca: value.ca.as_ref().map(|ca| ca.into()),
-        }
-    }
-}
-
-impl From<&nix_compat::narinfo::NarInfo<'_>> for PathInfo {
-    /// Converts from a NarInfo (returned from the NARInfo parser) to a PathInfo
-    /// struct with the node set to None.
-    fn from(value: &nix_compat::narinfo::NarInfo<'_>) -> Self {
+impl From<crate::pathinfoservice::PathInfo> for PathInfo {
+    fn from(value: crate::pathinfoservice::PathInfo) -> Self {
         Self {
-            node: None,
+            node: Some(castorepb::Node::from_name_and_node(
+                value.store_path.to_string().into_bytes().into(),
+                value.node,
+            )),
             references: value
                 .references
                 .iter()
-                .map(|x| Bytes::copy_from_slice(x.digest()))
+                .map(|reference| Bytes::copy_from_slice(reference.digest()))
                 .collect(),
-            narinfo: Some(value.into()),
+            narinfo: Some(NarInfo {
+                nar_size: value.nar_size,
+                nar_sha256: Bytes::copy_from_slice(&value.nar_sha256),
+                signatures: value
+                    .signatures
+                    .iter()
+                    .map(|sig| nar_info::Signature {
+                        name: sig.name().to_string(),
+                        data: Bytes::copy_from_slice(sig.bytes()),
+                    })
+                    .collect(),
+                reference_names: value.references.iter().map(|r| r.to_string()).collect(),
+                deriver: value.deriver.as_ref().map(|sp| StorePath {
+                    name: (*sp.name()).to_owned(),
+                    digest: Bytes::copy_from_slice(sp.digest()),
+                }),
+                ca: value.ca.as_ref().map(|ca| ca.into()),
+            }),
+        }
+    }
+}
+
+impl TryFrom<PathInfo> for crate::pathinfoservice::PathInfo {
+    type Error = ValidatePathInfoError;
+    fn try_from(value: PathInfo) -> Result<Self, Self::Error> {
+        let narinfo = value
+            .narinfo
+            .ok_or_else(|| ValidatePathInfoError::NarInfoFieldMissing)?;
+
+        // ensure the references have the right number of bytes.
+        for (i, reference) in value.references.iter().enumerate() {
+            if reference.len() != store_path::DIGEST_SIZE {
+                return Err(ValidatePathInfoError::InvalidReferenceDigestLen(
+                    i,
+                    reference.len(),
+                ));
+            }
+        }
+
+        // ensure the number of references there matches PathInfo.references count.
+        if narinfo.reference_names.len() != value.references.len() {
+            return Err(ValidatePathInfoError::InconsistentNumberOfReferences(
+                value.references.len(),
+                narinfo.reference_names.len(),
+            ));
+        }
+
+        // parse references in reference_names.
+        let mut references = vec![];
+        for (i, reference_name_str) in narinfo.reference_names.iter().enumerate() {
+            // ensure thy parse as (non-absolute) store path
+            let reference_names_store_path =
+                StorePathRef::from_bytes(reference_name_str.as_bytes()).map_err(|_| {
+                    ValidatePathInfoError::InvalidNarinfoReferenceName(
+                        i,
+                        reference_name_str.to_owned(),
+                    )
+                })?;
+
+            // ensure their digest matches the one at self.references[i].
+            {
+                // This is safe, because we ensured the proper length earlier already.
+                let reference_digest = value.references[i].to_vec().try_into().unwrap();
+
+                if reference_names_store_path.digest() != &reference_digest {
+                    return Err(
+                        ValidatePathInfoError::InconsistentNarinfoReferenceNameDigest(
+                            i,
+                            reference_digest,
+                            *reference_names_store_path.digest(),
+                        ),
+                    );
+                } else {
+                    references.push(reference_names_store_path.to_owned());
+                }
+            }
         }
+
+        let nar_sha256_length = narinfo.nar_sha256.len();
+
+        // split value.node into the name and node components
+        let (name, node) = value
+            .node
+            .ok_or_else(|| ValidatePathInfoError::NoNodePresent)?
+            .into_name_and_node()
+            .map_err(ValidatePathInfoError::InvalidRootNode)?;
+
+        Ok(Self {
+            // value.node has a valid name according to the castore model but might not parse to a
+            // [StorePath]
+            store_path: nix_compat::store_path::StorePath::from_bytes(name.as_ref()).map_err(
+                |err| ValidatePathInfoError::InvalidNodeName(name.as_ref().to_vec(), err),
+            )?,
+            node,
+            references,
+            nar_size: narinfo.nar_size,
+            nar_sha256: narinfo.nar_sha256.to_vec()[..]
+                .try_into()
+                .map_err(|_| ValidatePathInfoError::InvalidNarSha256DigestLen(nar_sha256_length))?,
+            // If the Deriver field is populated, ensure it parses to a
+            // [StorePath].
+            // We can't check for it to *not* end with .drv, as the .drv files produced by
+            // recursive Nix end with multiple .drv suffixes, and only one is popped when
+            // converting to this field.
+            deriver: narinfo
+                .deriver
+                .map(|deriver| {
+                    nix_compat::store_path::StorePath::from_name_and_digest(
+                        &deriver.name,
+                        &deriver.digest,
+                    )
+                    .map_err(ValidatePathInfoError::InvalidDeriverField)
+                })
+                .transpose()?,
+            signatures: narinfo
+                .signatures
+                .into_iter()
+                .enumerate()
+                .map(|(i, signature)| {
+                    signature.data.to_vec()[..]
+                        .try_into()
+                        .map_err(|_| {
+                            ValidatePathInfoError::InvalidSignature(
+                                i,
+                                SignatureError::InvalidSignatureLen(signature.data.len()),
+                            )
+                        })
+                        .map(|signature_data| Signature::new(signature.name, signature_data))
+                })
+                .collect::<Result<Vec<_>, ValidatePathInfoError>>()?,
+            ca: narinfo
+                .ca
+                .as_ref()
+                .map(TryFrom::try_from)
+                .transpose()
+                .map_err(ValidatePathInfoError::InvalidCaField)?,
+        })
     }
 }
diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs
index f5ecc798bbfb..320f419b6c59 100644
--- a/tvix/store/src/proto/tests/pathinfo.rs
+++ b/tvix/store/src/proto/tests/pathinfo.rs
@@ -1,274 +1,226 @@
-use crate::proto::{nar_info::Signature, NarInfo, PathInfo, ValidatePathInfoError};
-use crate::tests::fixtures::*;
+use crate::pathinfoservice::PathInfo;
+use crate::proto::{self, ValidatePathInfoError};
+use crate::tests::fixtures::{DUMMY_PATH, DUMMY_PATH_DIGEST, DUMMY_PATH_STR};
 use bytes::Bytes;
-use data_encoding::BASE64;
-use nix_compat::nixbase32;
-use nix_compat::store_path::{self, StorePath, StorePathRef};
+use lazy_static::lazy_static;
+use nix_compat::store_path;
 use rstest::rstest;
+use tvix_castore::fixtures::DUMMY_DIGEST;
 use tvix_castore::proto as castorepb;
 use tvix_castore::{DirectoryError, ValidateNodeError};
 
-#[rstest]
-#[case::no_node(None, Err(ValidatePathInfoError::NoNodePresent))]
-#[case::no_node_2(Some(castorepb::Node { node: None}), Err(ValidatePathInfoError::InvalidRootNode(DirectoryError::NoNodeSet)))]
+lazy_static! {
+    /// A valid PathInfo message
+    /// The references in `narinfo.reference_names` aligns with what's in
+    /// `references`.
+    static ref PROTO_PATH_INFO : proto::PathInfo = proto::PathInfo {
+        node: Some(castorepb::Node {
+            node: Some(castorepb::node::Node::Directory(castorepb::DirectoryNode {
+                name: DUMMY_PATH_STR.into(),
+                digest: DUMMY_DIGEST.clone().into(),
+                size: 0,
+            })),
+        }),
+        references: vec![DUMMY_PATH_DIGEST.as_slice().into()],
+        narinfo: Some(proto::NarInfo {
+            nar_size: 0,
+            nar_sha256: DUMMY_DIGEST.clone().into(),
+            signatures: vec![],
+            reference_names: vec![DUMMY_PATH_STR.to_string()],
+            deriver: None,
+            ca: Some(proto::nar_info::Ca { r#type: proto::nar_info::ca::Hash::NarSha256.into(), digest:  DUMMY_DIGEST.clone().into() })
+        }),
+    };
+}
+
+#[test]
+fn convert_valid() {
+    let path_info = PROTO_PATH_INFO.clone();
+    PathInfo::try_from(path_info).expect("must succeed");
+}
+
+/// Create a PathInfo with a correct deriver field and ensure it succeeds.
+#[test]
+fn convert_valid_deriver() {
+    let mut path_info = PROTO_PATH_INFO.clone();
+
+    // add a valid deriver
+    let narinfo = path_info.narinfo.as_mut().unwrap();
+    narinfo.deriver = Some(crate::proto::StorePath {
+        name: DUMMY_PATH.name().to_string(),
+        digest: Bytes::from(DUMMY_PATH_DIGEST.as_slice()),
+    });
 
-fn validate_pathinfo(
+    let path_info = PathInfo::try_from(path_info).expect("must succeed");
+    assert_eq!(DUMMY_PATH.clone(), path_info.deriver.unwrap())
+}
+
+#[rstest]
+#[case::no_node(None, ValidatePathInfoError::NoNodePresent)]
+#[case::no_node_2(Some(castorepb::Node { node: None}), ValidatePathInfoError::InvalidRootNode(DirectoryError::NoNodeSet))]
+fn convert_pathinfo_wrong_nodes(
     #[case] node: Option<castorepb::Node>,
-    #[case] exp_result: Result<StorePath<String>, ValidatePathInfoError>,
+    #[case] exp_err: ValidatePathInfoError,
 ) {
     // construct the PathInfo object
-    let p = PathInfo {
-        node,
-        ..Default::default()
-    };
+    let mut path_info = PROTO_PATH_INFO.clone();
+    path_info.node = node;
 
-    assert_eq!(exp_result, p.validate());
+    assert_eq!(
+        exp_err,
+        PathInfo::try_from(path_info).expect_err("must fail")
+    );
 }
 
+/// Constructs a [proto::PathInfo] with root nodes that have wrong data in
+/// various places, causing the conversion to [PathInfo] to fail.
 #[rstest]
-#[case::ok(castorepb::DirectoryNode {
-        name: DUMMY_PATH.into(),
-        digest: DUMMY_DIGEST.clone().into(),
-        size: 0,
-}, Ok(StorePath::from_bytes(DUMMY_PATH.as_bytes()).unwrap()))]
-#[case::invalid_digest_length(castorepb::DirectoryNode {
-        name: DUMMY_PATH.into(),
+#[case::directory_invalid_digest_length(
+    castorepb::node::Node::Directory(castorepb::DirectoryNode {
+        name: DUMMY_PATH_STR.into(),
         digest: Bytes::new(),
         size: 0,
-}, Err(ValidatePathInfoError::InvalidRootNode(DirectoryError::InvalidNode(DUMMY_PATH.into(), ValidateNodeError::InvalidDigestLen(0)))))]
-#[case::invalid_node_name_no_storepath(castorepb::DirectoryNode {
+    }),
+    ValidatePathInfoError::InvalidRootNode(DirectoryError::InvalidNode(DUMMY_PATH_STR.into(), ValidateNodeError::InvalidDigestLen(0)))
+)]
+#[case::directory_invalid_node_name_no_storepath(
+    castorepb::node::Node::Directory(castorepb::DirectoryNode {
         name: "invalid".into(),
         digest: DUMMY_DIGEST.clone().into(),
         size: 0,
-}, Err(ValidatePathInfoError::InvalidNodeName(
-        "invalid".into(),
-        store_path::Error::InvalidLength
-)))]
-fn validate_directory(
-    #[case] directory_node: castorepb::DirectoryNode,
-    #[case] exp_result: Result<StorePath<String>, ValidatePathInfoError>,
-) {
-    // construct the PathInfo object
-    let p = PathInfo {
-        node: Some(castorepb::Node {
-            node: Some(castorepb::node::Node::Directory(directory_node)),
-        }),
-        ..Default::default()
-    };
-    assert_eq!(exp_result, p.validate());
-}
-
-#[rstest]
-#[case::ok(
-    castorepb::FileNode {
-        name: DUMMY_PATH.into(),
-        digest: DUMMY_DIGEST.clone().into(),
-        size: 0,
-        executable: false,
-    },
-    Ok(StorePath::from_bytes(DUMMY_PATH.as_bytes()).unwrap())
+    }),
+    ValidatePathInfoError::InvalidNodeName("invalid".into(), store_path::Error::InvalidLength)
 )]
-#[case::invalid_digest_len(
-    castorepb::FileNode {
-        name: DUMMY_PATH.into(),
+#[case::file_invalid_digest_len(
+    castorepb::node::Node::File(castorepb::FileNode {
+        name: DUMMY_PATH_STR.into(),
         digest: Bytes::new(),
         ..Default::default()
-    },
-    Err(ValidatePathInfoError::InvalidRootNode(DirectoryError::InvalidNode(DUMMY_PATH.into(), ValidateNodeError::InvalidDigestLen(0))))
+    }),
+    ValidatePathInfoError::InvalidRootNode(DirectoryError::InvalidNode(DUMMY_PATH_STR.into(), ValidateNodeError::InvalidDigestLen(0)))
 )]
-#[case::invalid_node_name(
-    castorepb::FileNode {
+#[case::file_invalid_node_name(
+    castorepb::node::Node::File(castorepb::FileNode {
         name: "invalid".into(),
         digest: DUMMY_DIGEST.clone().into(),
         ..Default::default()
-    },
-    Err(ValidatePathInfoError::InvalidNodeName(
+    }),
+    ValidatePathInfoError::InvalidNodeName(
         "invalid".into(),
         store_path::Error::InvalidLength
-    ))
-)]
-fn validate_file(
-    #[case] file_node: castorepb::FileNode,
-    #[case] exp_result: Result<StorePath<String>, ValidatePathInfoError>,
-) {
-    // construct the PathInfo object
-    let p = PathInfo {
-        node: Some(castorepb::Node {
-            node: Some(castorepb::node::Node::File(file_node)),
-        }),
-        ..Default::default()
-    };
-    assert_eq!(exp_result, p.validate());
-}
-
-#[rstest]
-#[case::ok(
-    castorepb::SymlinkNode {
-        name: DUMMY_PATH.into(),
-        target: "foo".into(),
-    },
-    Ok(StorePath::from_bytes(DUMMY_PATH.as_bytes()).unwrap())
+    )
 )]
-#[case::invalid_node_name(
-    castorepb::SymlinkNode {
+#[case::symlink_invalid_node_name(
+    castorepb::node::Node::Symlink(castorepb::SymlinkNode {
         name: "invalid".into(),
         target: "foo".into(),
-    },
-    Err(ValidatePathInfoError::InvalidNodeName(
+    }),
+    ValidatePathInfoError::InvalidNodeName(
         "invalid".into(),
         store_path::Error::InvalidLength
-    ))
+    )
 )]
-fn validate_symlink(
-    #[case] symlink_node: castorepb::SymlinkNode,
-    #[case] exp_result: Result<StorePath<String>, ValidatePathInfoError>,
-) {
-    // construct the PathInfo object
-    let p = PathInfo {
-        node: Some(castorepb::Node {
-            node: Some(castorepb::node::Node::Symlink(symlink_node)),
-        }),
-        ..Default::default()
-    };
-    assert_eq!(exp_result, p.validate());
-}
+fn convert_fail_node(#[case] node: castorepb::node::Node, #[case] exp_err: ValidatePathInfoError) {
+    // construct the proto::PathInfo object
+    let mut p = PROTO_PATH_INFO.clone();
+    p.node = Some(castorepb::Node { node: Some(node) });
 
-/// Ensure parsing a correct PathInfo without narinfo populated succeeds.
-#[test]
-fn validate_references_without_narinfo_ok() {
-    assert!(PATH_INFO_WITHOUT_NARINFO.validate().is_ok());
+    assert_eq!(exp_err, PathInfo::try_from(p).expect_err("must fail"));
 }
 
-/// Ensure parsing a correct PathInfo with narinfo populated succeeds.
+/// Ensure a PathInfo without narinfo populated fails converting!
 #[test]
-fn validate_references_with_narinfo_ok() {
-    assert!(PATH_INFO_WITH_NARINFO.validate().is_ok());
+fn convert_without_narinfo_fail() {
+    let mut path_info = PROTO_PATH_INFO.clone();
+    path_info.narinfo = None;
+
+    assert_eq!(
+        ValidatePathInfoError::NarInfoFieldMissing,
+        PathInfo::try_from(path_info).expect_err("must fail"),
+    );
 }
 
 /// Create a PathInfo with a wrong digest length in narinfo.nar_sha256, and
-/// ensure validation fails.
+/// ensure conversion fails.
 #[test]
-fn validate_wrong_nar_sha256() {
-    let mut path_info = PATH_INFO_WITH_NARINFO.clone();
+fn convert_wrong_nar_sha256() {
+    let mut path_info = PROTO_PATH_INFO.clone();
     path_info.narinfo.as_mut().unwrap().nar_sha256 = vec![0xbe, 0xef].into();
 
-    match path_info.validate().expect_err("must_fail") {
-        ValidatePathInfoError::InvalidNarSha256DigestLen(2) => {}
-        e => panic!("unexpected error: {:?}", e),
-    };
+    assert_eq!(
+        ValidatePathInfoError::InvalidNarSha256DigestLen(2),
+        PathInfo::try_from(path_info).expect_err("must fail")
+    );
 }
 
 /// Create a PathInfo with a wrong count of narinfo.reference_names,
 /// and ensure validation fails.
 #[test]
-fn validate_inconsistent_num_refs_fail() {
-    let mut path_info = PATH_INFO_WITH_NARINFO.clone();
+fn convert_inconsistent_num_refs_fail() {
+    let mut path_info = PROTO_PATH_INFO.clone();
     path_info.narinfo.as_mut().unwrap().reference_names = vec![];
 
-    match path_info.validate().expect_err("must_fail") {
-        ValidatePathInfoError::InconsistentNumberOfReferences(1, 0) => {}
-        e => panic!("unexpected error: {:?}", e),
-    };
+    assert_eq!(
+        ValidatePathInfoError::InconsistentNumberOfReferences(1, 0),
+        PathInfo::try_from(path_info).expect_err("must fail")
+    );
 }
 
 /// Create a PathInfo with a wrong digest length in references.
 #[test]
-fn validate_invalid_reference_digest_len() {
-    let mut path_info = PATH_INFO_WITHOUT_NARINFO.clone();
+fn convert_invalid_reference_digest_len() {
+    let mut path_info = PROTO_PATH_INFO.clone();
     path_info.references.push(vec![0xff, 0xff].into());
 
-    match path_info.validate().expect_err("must fail") {
+    assert_eq!(
         ValidatePathInfoError::InvalidReferenceDigestLen(
             1, // position
             2, // unexpected digest len
-        ) => {}
-        e => panic!("unexpected error: {:?}", e),
-    };
+        ),
+        PathInfo::try_from(path_info).expect_err("must fail")
+    );
 }
 
 /// Create a PathInfo with a narinfo.reference_name[1] that is no valid store path.
 #[test]
-fn validate_invalid_narinfo_reference_name() {
-    let mut path_info = PATH_INFO_WITH_NARINFO.clone();
+fn convert_invalid_narinfo_reference_name() {
+    let mut path_info = PROTO_PATH_INFO.clone();
 
     // This is invalid, as the store prefix is not part of reference_names.
     path_info.narinfo.as_mut().unwrap().reference_names[0] =
         "/nix/store/00000000000000000000000000000000-dummy".to_string();
 
-    match path_info.validate().expect_err("must fail") {
-        ValidatePathInfoError::InvalidNarinfoReferenceName(0, reference_name) => {
-            assert_eq!(
-                "/nix/store/00000000000000000000000000000000-dummy",
-                reference_name
-            );
-        }
-        e => panic!("unexpected error: {:?}", e),
-    }
+    assert_eq!(
+        ValidatePathInfoError::InvalidNarinfoReferenceName(
+            0,
+            "/nix/store/00000000000000000000000000000000-dummy".to_string()
+        ),
+        PathInfo::try_from(path_info).expect_err("must fail")
+    );
 }
 
 /// Create a PathInfo with a narinfo.reference_name[0] that doesn't match references[0].
 #[test]
-fn validate_inconsistent_narinfo_reference_name_digest() {
-    let mut path_info = PATH_INFO_WITH_NARINFO.clone();
+fn convert_inconsistent_narinfo_reference_name_digest() {
+    let mut path_info = PROTO_PATH_INFO.clone();
 
     // mutate the first reference, they were all zeroes before
     path_info.references[0] = vec![0xff; store_path::DIGEST_SIZE].into();
 
-    match path_info.validate().expect_err("must fail") {
-        ValidatePathInfoError::InconsistentNarinfoReferenceNameDigest(0, e_expected, e_actual) => {
-            assert_eq!(path_info.references[0][..], e_expected[..]);
-            assert_eq!(DUMMY_PATH_DIGEST, e_actual);
-        }
-        e => panic!("unexpected error: {:?}", e),
-    }
-}
-
-/// Create a node with an empty symlink target, and ensure it fails validation.
-#[test]
-fn validate_symlink_empty_target_invalid() {
-    castorepb::Node {
-        node: Some(castorepb::node::Node::Symlink(castorepb::SymlinkNode {
-            name: "foo".into(),
-            target: "".into(),
-        })),
-    }
-    .into_name_and_node()
-    .expect_err("must fail validation");
-}
-
-/// Create a node with a symlink target including null bytes, and ensure it
-/// fails validation.
-#[test]
-fn validate_symlink_target_null_byte_invalid() {
-    castorepb::Node {
-        node: Some(castorepb::node::Node::Symlink(castorepb::SymlinkNode {
-            name: "foo".into(),
-            target: "foo\0".into(),
-        })),
-    }
-    .into_name_and_node()
-    .expect_err("must fail validation");
-}
-
-/// Create a PathInfo with a correct deriver field and ensure it succeeds.
-#[test]
-fn validate_valid_deriver() {
-    let mut path_info = PATH_INFO_WITH_NARINFO.clone();
-
-    // add a valid deriver
-    let narinfo = path_info.narinfo.as_mut().unwrap();
-    narinfo.deriver = Some(crate::proto::StorePath {
-        name: "foo".to_string(),
-        digest: Bytes::from(DUMMY_PATH_DIGEST.as_slice()),
-    });
-
-    path_info.validate().expect("must validate");
+    assert_eq!(
+        ValidatePathInfoError::InconsistentNarinfoReferenceNameDigest(
+            0,
+            path_info.references[0][..].try_into().unwrap(),
+            DUMMY_PATH_DIGEST
+        ),
+        PathInfo::try_from(path_info).expect_err("must fail")
+    )
 }
 
 /// Create a PathInfo with a broken deriver field and ensure it fails.
 #[test]
-fn validate_invalid_deriver() {
-    let mut path_info = PATH_INFO_WITH_NARINFO.clone();
+fn convert_invalid_deriver() {
+    let mut path_info = PROTO_PATH_INFO.clone();
 
     // add a broken deriver (invalid digest)
     let narinfo = path_info.narinfo.as_mut().unwrap();
@@ -277,157 +229,8 @@ fn validate_invalid_deriver() {
         digest: vec![].into(),
     });
 
-    match path_info.validate().expect_err("must fail validation") {
-        ValidatePathInfoError::InvalidDeriverField(_) => {}
-        e => panic!("unexpected error: {:?}", e),
-    }
-}
-
-#[test]
-fn from_nixcompat_narinfo() {
-    let narinfo_parsed = nix_compat::narinfo::NarInfo::parse(
-        r#"StorePath: /nix/store/s66mzxpvicwk07gjbjfw9izjfa797vsw-hello-2.12.1
-URL: nar/1nhgq6wcggx0plpy4991h3ginj6hipsdslv4fd4zml1n707j26yq.nar.xz
-Compression: xz
-FileHash: sha256:1nhgq6wcggx0plpy4991h3ginj6hipsdslv4fd4zml1n707j26yq
-FileSize: 50088
-NarHash: sha256:0yzhigwjl6bws649vcs2asa4lbs8hg93hyix187gc7s7a74w5h80
-NarSize: 226488
-References: 3n58xw4373jp0ljirf06d8077j15pc4j-glibc-2.37-8 s66mzxpvicwk07gjbjfw9izjfa797vsw-hello-2.12.1
-Deriver: ib3sh3pcz10wsmavxvkdbayhqivbghlq-hello-2.12.1.drv
-Sig: cache.nixos.org-1:8ijECciSFzWHwwGVOIVYdp2fOIOJAfmzGHPQVwpktfTQJF6kMPPDre7UtFw3o+VqenC5P8RikKOAAfN7CvPEAg=="#).expect("must parse");
-
-    assert_eq!(
-        PathInfo {
-            node: None,
-            references: vec![
-                Bytes::copy_from_slice(&nixbase32::decode_fixed::<20>("3n58xw4373jp0ljirf06d8077j15pc4j").unwrap()),
-                Bytes::copy_from_slice(&nixbase32::decode_fixed::<20>("s66mzxpvicwk07gjbjfw9izjfa797vsw").unwrap()),
-            ],
-            narinfo: Some(
-                NarInfo {
-                    nar_size: 226488,
-                    nar_sha256: Bytes::copy_from_slice(
-                        &nixbase32::decode_fixed::<32>("0yzhigwjl6bws649vcs2asa4lbs8hg93hyix187gc7s7a74w5h80".as_bytes())
-                            .unwrap()
-                    ),
-                    signatures: vec![Signature {
-                        name: "cache.nixos.org-1".to_string(),
-                        data: BASE64.decode("8ijECciSFzWHwwGVOIVYdp2fOIOJAfmzGHPQVwpktfTQJF6kMPPDre7UtFw3o+VqenC5P8RikKOAAfN7CvPEAg==".as_bytes()).unwrap().into(),
-                    }],
-                    reference_names: vec![
-                        "3n58xw4373jp0ljirf06d8077j15pc4j-glibc-2.37-8".to_string(),
-                        "s66mzxpvicwk07gjbjfw9izjfa797vsw-hello-2.12.1".to_string()
-                    ],
-                    deriver: Some(crate::proto::StorePath {
-                        digest: Bytes::copy_from_slice(&nixbase32::decode_fixed::<20>("ib3sh3pcz10wsmavxvkdbayhqivbghlq").unwrap()),
-                        name: "hello-2.12.1".to_string(),
-                     }),
-                    ca: None,
-                }
-            )
-        },
-        (&narinfo_parsed).into(),
-    );
-}
-
-#[test]
-fn from_nixcompat_narinfo_fod() {
-    let narinfo_parsed = nix_compat::narinfo::NarInfo::parse(
-        r#"StorePath: /nix/store/pa10z4ngm0g83kx9mssrqzz30s84vq7k-hello-2.12.1.tar.gz
-URL: nar/1zjrhzhaizsrlsvdkqfl073vivmxcqnzkff4s50i0cdf541ary1r.nar.xz
-Compression: xz
-FileHash: sha256:1zjrhzhaizsrlsvdkqfl073vivmxcqnzkff4s50i0cdf541ary1r
-FileSize: 1033524
-NarHash: sha256:1lvqpbk2k1sb39z8jfxixf7p7v8sj4z6mmpa44nnmff3w1y6h8lh
-NarSize: 1033416
-References: 
-Deriver: dyivpmlaq2km6c11i0s6bi6mbsx0ylqf-hello-2.12.1.tar.gz.drv
-Sig: cache.nixos.org-1:ywnIG629nQZQhEr6/HLDrLT/mUEp5J1LC6NmWSlJRWL/nM7oGItJQUYWGLvYGhSQvHrhIuvMpjNmBNh/WWqCDg==
-CA: fixed:sha256:086vqwk2wl8zfs47sq2xpjc9k066ilmb8z6dn0q6ymwjzlm196cd"#
-    ).expect("must parse");
-
-    assert_eq!(
-        PathInfo {
-            node: None,
-            references: vec![],
-            narinfo: Some(
-                NarInfo {
-                    nar_size: 1033416,
-                    nar_sha256: Bytes::copy_from_slice(
-                        &nixbase32::decode_fixed::<32>(
-                            "1lvqpbk2k1sb39z8jfxixf7p7v8sj4z6mmpa44nnmff3w1y6h8lh"
-                        )
-                        .unwrap()
-                    ),
-                    signatures: vec![Signature {
-                        name: "cache.nixos.org-1".to_string(),
-                        data: BASE64
-                            .decode("ywnIG629nQZQhEr6/HLDrLT/mUEp5J1LC6NmWSlJRWL/nM7oGItJQUYWGLvYGhSQvHrhIuvMpjNmBNh/WWqCDg==".as_bytes())
-                            .unwrap()
-                            .into(),
-                    }],
-                    reference_names: vec![],
-                    deriver: Some(crate::proto::StorePath {
-                        digest: Bytes::copy_from_slice(
-                            &nixbase32::decode_fixed::<20>("dyivpmlaq2km6c11i0s6bi6mbsx0ylqf").unwrap()
-                        ),
-                        name: "hello-2.12.1.tar.gz".to_string(),
-                    }),
-                    ca: Some(crate::proto::nar_info::Ca {
-                        r#type: crate::proto::nar_info::ca::Hash::FlatSha256.into(),
-                        digest: Bytes::copy_from_slice(
-                            &nixbase32::decode_fixed::<32>(
-                                "086vqwk2wl8zfs47sq2xpjc9k066ilmb8z6dn0q6ymwjzlm196cd"
-                            )
-                            .unwrap()
-                        )
-                    }),
-                }
-            ),
-        },
-        (&narinfo_parsed).into()
-    );
-}
-
-/// Exercise .as_narinfo() on a PathInfo and ensure important fields are preserved..
-#[test]
-fn as_narinfo() {
-    let narinfo_parsed = nix_compat::narinfo::NarInfo::parse(
-        r#"StorePath: /nix/store/pa10z4ngm0g83kx9mssrqzz30s84vq7k-hello-2.12.1.tar.gz
-URL: nar/1zjrhzhaizsrlsvdkqfl073vivmxcqnzkff4s50i0cdf541ary1r.nar.xz
-Compression: xz
-FileHash: sha256:1zjrhzhaizsrlsvdkqfl073vivmxcqnzkff4s50i0cdf541ary1r
-FileSize: 1033524
-NarHash: sha256:1lvqpbk2k1sb39z8jfxixf7p7v8sj4z6mmpa44nnmff3w1y6h8lh
-NarSize: 1033416
-References: 
-Deriver: dyivpmlaq2km6c11i0s6bi6mbsx0ylqf-hello-2.12.1.tar.gz.drv
-Sig: cache.nixos.org-1:ywnIG629nQZQhEr6/HLDrLT/mUEp5J1LC6NmWSlJRWL/nM7oGItJQUYWGLvYGhSQvHrhIuvMpjNmBNh/WWqCDg==
-CA: fixed:sha256:086vqwk2wl8zfs47sq2xpjc9k066ilmb8z6dn0q6ymwjzlm196cd"#
-    ).expect("must parse");
-
-    let path_info: PathInfo = (&narinfo_parsed).into();
-
-    let mut narinfo_returned = path_info
-        .to_narinfo(
-            StorePathRef::from_bytes(b"pa10z4ngm0g83kx9mssrqzz30s84vq7k-hello-2.12.1.tar.gz")
-                .expect("invalid storepath"),
-        )
-        .expect("must be some");
-    narinfo_returned.url = "some.nar";
-
     assert_eq!(
-        r#"StorePath: /nix/store/pa10z4ngm0g83kx9mssrqzz30s84vq7k-hello-2.12.1.tar.gz
-URL: some.nar
-Compression: none
-NarHash: sha256:1lvqpbk2k1sb39z8jfxixf7p7v8sj4z6mmpa44nnmff3w1y6h8lh
-NarSize: 1033416
-References: 
-Deriver: dyivpmlaq2km6c11i0s6bi6mbsx0ylqf-hello-2.12.1.tar.gz.drv
-Sig: cache.nixos.org-1:ywnIG629nQZQhEr6/HLDrLT/mUEp5J1LC6NmWSlJRWL/nM7oGItJQUYWGLvYGhSQvHrhIuvMpjNmBNh/WWqCDg==
-CA: fixed:sha256:086vqwk2wl8zfs47sq2xpjc9k066ilmb8z6dn0q6ymwjzlm196cd
-"#,
-        narinfo_returned.to_string(),
-    );
+        ValidatePathInfoError::InvalidDeriverField(store_path::Error::InvalidLength),
+        PathInfo::try_from(path_info).expect_err("must fail")
+    )
 }