Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Materializer config loading to eagerly resolve optional fendermint config #1293

Open
4 tasks
karlem opened this issue Feb 18, 2025 · 1 comment
Open
4 tasks

Comments

@karlem
Copy link
Contributor

karlem commented Feb 18, 2025

Currently, the Materializer manifest configuration uses a lazy-loading approach for the optional fendermint_additional_config field. This field is represented using a single type (FendermintConfigSource) that can either be a file path or a fully loaded configuration. While we eventually trigger the loading (via load_all_fendermint_configs), a Node can exist in an intermediate state where the configuration is not yet loaded. This mixing of unresolved and resolved states in one type can lead to subtle bugs and makes it less clear when and where errors might occur.

Proposed Solution:

Separate Types:
Manifest Types: Create types that mirror the YAML structure (e.g., NodeManifest or NodeRaw). In these types,fendermint_additional_config remains as a file path (or is omitted).
Internal Types: Create internal types (e.g.,Node) where all configuration fields are fully resolved. In particular,fendermint_additional_config should be an optional FendermintConfig that is already loaded and parsed.

Conversion Step:
Implement conversion methods (or TryFrom implementations) that take a manifest type and a base_dir to resolve relative paths. This conversion should eagerly load and parse the TOML file, returning a fully-resolved Node, or an error if loading/parsing fails.

Integration:

Update the configuration loading pipeline so that after deserializing the YAML into the manifest types, we immediately convert all parts (rootnet, subnets, etc.) into their internal counterparts. This ensures that by the time our application logic uses the configuration, all nodes are in a consistent, fully loaded state.

Example Code Snippet:

// Manifest type: deserialized directly from YAML.
#[derive(Deserialize)]
pub struct NodeManifest {
    pub mode: NodeModeManifest,
    pub ethapi: bool,
    pub seed_nodes: Vec<String>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub parent_node: Option<String>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub fendermint_additional_config: Option<PathBuf>,
}

// Internal type: fully resolved configuration.
pub struct Node {
    pub mode: NodeMode,
    pub ethapi: bool,
    pub seed_nodes: Vec<String>,
    pub parent_node: Option<String>,
    pub fendermint_additional_config: Option<FendermintConfig>,
}

// Conversion method that eagerly loads the optional config.
impl NodeManifest {
    pub fn convert(self, base_dir: &Path) -> io::Result<Node> {
        let additional_config = if let Some(path) = self.fendermint_additional_config {
            let full_path = if path.is_relative() { base_dir.join(path) } else { path };
            let content = fs::read_to_string(full_path)?;
            let toml_value = toml::from_str(&content)
                .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?;
            Some(FendermintConfig(toml_value))
        } else {
            None
        };

        // Convert the node mode as needed.
        let mode = match self.mode {
            NodeModeManifest::Validator { validator } => NodeMode::Validator { validator },
            NodeModeManifest::FullNode => NodeMode::FullNode,
        };

        Ok(Node {
            mode,
            ethapi: self.ethapi,
            seed_nodes: self.seed_nodes,
            parent_node: self.parent_node,
            fendermint_additional_config: additional_config,
        })
    }
}

This change ensures that the conversion from raw (manifest) configuration to internal configuration is performed eagerly, avoiding any deferred/lazy loading of external files.

Acceptance Criteria:

  • Node configuration is split into manifest and internal types.
  • The conversion step eagerly loads all external configuration, returning errors immediately on failure.
  • The rest of the configuration pipeline (rootnet, subnets, etc.) is updated to use the new internal types.
  • All tests pass and configuration errors are caught during startup rather than at runtime.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant