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

Clap based improvements for the CLI #214

Open
hu55a1n1 opened this issue Sep 25, 2024 · 3 comments · May be fixed by #279
Open

Clap based improvements for the CLI #214

hu55a1n1 opened this issue Sep 25, 2024 · 3 comments · May be fixed by #279

Comments

@hu55a1n1
Copy link
Member

hu55a1n1 commented Sep 25, 2024

Summary

(Copying @dangush's comments from #150)

@piotr-roslaniec
Copy link

We have no access to the mock_sgx field present in the "higher contexts" where SgxConfiguration is used, so we have to rely on the MOCK_SGX environment variable directly. Using ArgGroup seems a bit superfluous here. We still need to condition fields based on the reverse logic of the MOCK_SGX environment variable. Please clarify if I misunderstood, otherwise I'd be happy to contribute:

/// SGX-specific configuration. Required if `MOCK_SGX` is not set.
#[derive(Debug, Parser, Clone, Serialize, Deserialize)]
pub struct SgxConfiguration {
    /// FMSPC (Family-Model-Stepping-Platform-Custom SKU)
    #[arg(long)]
    #[serde(skip_serializing_if = "Option::is_none")]
    pub fmspc: Option<Fmspc>,

    /// Address of the TcbInfo contract
    #[arg(long)]
    #[serde(skip_serializing_if = "Option::is_none")]
    pub tcbinfo_contract: Option<AccountId>,

    /// Address of the DCAP verifier contract
    #[arg(long)]
    #[serde(skip_serializing_if = "Option::is_none")]
    pub dcap_verifier_contract: Option<AccountId>,
}

impl SgxConfiguration {
    fn validate(&self) -> Result<(), String> {
        if std::env::var("MOCK_SGX").is_err() {
            self.check_required_field(&self.fmspc, "FMSPC")?;
            self.check_required_field(&self.tcbinfo_contract, "tcbinfo_contract")?;
            self.check_required_field(&self.dcap_verifier_contract, "dcap_verifier_contract")?;
        }
        Ok(())
    }

    fn check_required_field<T>(&self, field: &Option<T>, field_name: &str) -> Result<(), String> {
        if field.is_none() {
            return Err(format!("{} is required if MOCK_SGX isn't set", field_name));
        }
        Ok(())
    }
}

@dusterbloom
Copy link
Contributor

@piotr-roslaniec Thanks for you comment. I am not 100% clear on what you meant.
I have tried to imagine what you meant and built a quick test branch.
Can you please give it a look it out and/or just try it out?

piotr-roslaniec added a commit to piotr-roslaniec/cycles-quartz that referenced this issue Oct 31, 2024
piotr-roslaniec added a commit to piotr-roslaniec/cycles-quartz that referenced this issue Oct 31, 2024
@piotr-roslaniec
Copy link

Hi @dusterbloom, thanks for getting back to me. For details, please see this draft: #279

@hu55a1n1 hu55a1n1 linked a pull request Nov 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants