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

feat(mater): add unixfs wrapping support #690

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 10 additions & 8 deletions maat/tests/real_world.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this file related to mater?

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::BTreeSet;
use std::{collections::BTreeSet, time::Duration};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Konrad is currently working on fixing the maat. Do you think we should remove the changes so that he won't have conflicts when merging? The thing is that there are more changes needed to fix the tests. There are not only compile time problems.


use cid::Cid;
use maat::*;
Expand Down Expand Up @@ -35,7 +35,7 @@ where
let result = client
.register_storage_provider(
charlie,
peer_id.clone(),
peer_id.clone().parse().expect("invalid peer_id"),
primitives::proofs::RegisteredPoStProof::StackedDRGWindow2KiBV1P1,
true,
)
Expand Down Expand Up @@ -251,7 +251,7 @@ where
assert_eq!(event.sectors.0, sectors_pre_commit_info);
}

result.height
client.height(true).await.unwrap()
}

async fn prove_commit_sector<Keypair>(
Expand Down Expand Up @@ -325,10 +325,10 @@ where
SubmitWindowedPoStParams {
deadline: 0,
partitions: vec![0],
proof: storagext::types::storage_provider::PoStProof {
proofs: vec![storagext::types::storage_provider::PoStProof {
post_proof: primitives::proofs::RegisteredPoStProof::StackedDRGWindow2KiBV1P1,
proof_bytes: "beef".as_bytes().to_vec(),
},
}],
},
true,
)
Expand Down Expand Up @@ -408,9 +408,11 @@ async fn real_world_use_case() {

tracing::debug!("base dir: {:?}", network.base_dir());

let collator = network.get_node(COLLATOR_NAME).unwrap();
let client =
storagext::Client::from(collator.wait_client::<PolkaStorageConfig>().await.unwrap());
let client = storagext::Client::new(
"ws://127.0.0.1:9944",
5,
Duration::from_secs(2)
).await.unwrap();

let alice_kp = pair_signer_from_str::<Sr25519Pair>("//Alice");
let charlie_kp = pair_signer_from_str::<Sr25519Pair>("//Charlie");
Expand Down
82 changes: 56 additions & 26 deletions mater/cli/src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,30 @@ use std::path::PathBuf;
use mater::{create_filestore, Cid, Config};
use tokio::fs::File;

use crate::error::Error;
use crate::{error::Error, ConvertConfig, WrapMode};

/// Converts a file at location `input_path` to a CARv2 file at `output_path`
pub(crate) async fn convert_file_to_car(
input_path: &PathBuf,
output_path: &PathBuf,
overwrite: bool,
config: ConvertConfig,
) -> Result<Cid, Error> {
let source_file = File::open(input_path).await?;
let output_file = if overwrite {
let output_file = if config.overwrite {
File::create(output_path).await
} else {
File::create_new(output_path).await
}?;
let cid = create_filestore(source_file, output_file, Config::default()).await?;

let mater_config = match config.wrap_mode {
WrapMode::Raw => Config::default(),
WrapMode::UnixFS => Config::Balanced {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How were the chunk_size and tree_width here determined?

chunk_size: 64 * 1024,
tree_width: 2,
wrap_mode: WrapMode::UnixFS,
},
Comment on lines +23 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just create a UnixFS default config + these are, once more, magic values

};
let cid = create_filestore(source_file, output_file, mater_config).await?;
Ok(cid)
}

Expand All @@ -27,16 +35,14 @@ pub(crate) async fn convert_file_to_car(
#[cfg(test)]
mod tests {
use std::str::FromStr;

use anyhow::Result;
use mater::Cid;
use tempfile::tempdir;
use tokio::{fs::File, io::AsyncWriteExt};

use crate::{convert::convert_file_to_car, error::Error};
use crate::{convert::convert_file_to_car, error::Error, ConvertConfig, WrapMode};

#[tokio::test]
async fn convert_file_to_car_success() -> Result<()> {
async fn convert_file_to_car_raw_success() -> Result<()> {
// Setup: Create a dummy input file
let temp_dir = tempdir()?;
let input_path = temp_dir.path().join("test_input.txt");
Expand All @@ -49,18 +55,42 @@ mod tests {
// Define output path
let output_path = temp_dir.path().join("test_output.car");

// Call the function under test
let result = convert_file_to_car(&input_path, &output_path, false).await;
// Configure for raw mode
let config = ConvertConfig {
wrap_mode: WrapMode::Raw,
overwrite: false,
};

// Assert the result is Ok
// Call the function under test
let result = convert_file_to_car(&input_path, &output_path, config).await;
assert!(result.is_ok());

// Verify that the CID is as expected
assert_eq!(result?, expected_cid);

// Close temporary directory
temp_dir.close()?;
Ok(())
}

#[tokio::test]
async fn convert_file_to_car_unixfs_success() -> Result<()> {
// Setup: Create a dummy input file
let temp_dir = tempdir()?;
let input_path = temp_dir.path().join("test_input.txt");
let mut input_file = File::create(&input_path).await?;
input_file.write_all(b"test data").await?;

// Define output path
let output_path = temp_dir.path().join("test_output.car");

// Configure for UnixFS mode
let config = ConvertConfig {
wrap_mode: WrapMode::UnixFS,
overwrite: false,
};

let result = convert_file_to_car(&input_path, &output_path, config).await;
assert!(result.is_ok());

temp_dir.close()?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to manually close the temp_dir. Lets remove this in other tests too.

Ok(())
}

Expand All @@ -69,20 +99,20 @@ mod tests {
// Define non-existent input path
let temp_dir = tempdir()?;
let input_path = temp_dir.path().join("non_existent_input.txt");

// Define output path
let output_path = temp_dir.path().join("test_output.car");

// Call the function under test
let result = convert_file_to_car(&input_path, &output_path, false).await;
let config = ConvertConfig {
wrap_mode: WrapMode::Raw,
overwrite: false,
};

// Assert the result is an error
// Call the function under test
let result = convert_file_to_car(&input_path, &output_path, config).await;
assert!(result.is_err());
assert!(matches!(result, Err(Error::IoError(..))));

// Close temporary directory
temp_dir.close()?;

Ok(())
}

Expand All @@ -97,18 +127,18 @@ mod tests {
// Create output file
let output_path = temp_dir.path().join("output_file");
File::create_new(&output_path).await?;
println!("gets here");

// Call the function under test
let result = convert_file_to_car(&input_path, &output_path, false).await;
let config = ConvertConfig {
wrap_mode: WrapMode::Raw,
overwrite: false,
};

// Assert the result is an error
// Call the function under test
let result = convert_file_to_car(&input_path, &output_path, config).await;
assert!(result.is_err());
assert!(matches!(result, Err(Error::IoError(..))));

// Close temporary directory
temp_dir.close()?;

Ok(())
}
}
14 changes: 13 additions & 1 deletion mater/cli/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::path::PathBuf;

use clap::Parser;
use mater::ConvertConfig;
use mater::WrapMode;

use crate::{convert::convert_file_to_car, error::Error, extract::extract_file_from_car};

Expand Down Expand Up @@ -29,6 +31,11 @@ enum MaterCli {
/// If enabled, the output will overwrite any existing files.
#[arg(long, action)]
overwrite: bool,

/// Determines how content should be wrapped in the CAR file.
/// 'raw' stores content directly, 'unixfs' wraps it in UnixFS format (default: raw)
#[arg(long, value_enum, default_value = "raw")]
wrap: WrapMode,
Comment on lines +37 to +38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's either wrapped or not. This is a bool

},
/// Convert a CARv2 file to its original format
Extract {
Expand All @@ -47,13 +54,18 @@ async fn main() -> Result<(), Error> {
output_path,
quiet,
overwrite,
wrap
} => {
let output_path = output_path.unwrap_or_else(|| {
let mut new_path = input_path.clone();
new_path.set_extension("car");
new_path
});
let cid = convert_file_to_car(&input_path, &output_path, overwrite).await?;
let config = ConvertConfig {
wrap_mode: wrap,
overwrite,
};
let cid = convert_file_to_car(&input_path, &output_path, config).await?;

if quiet {
println!("{}", cid);
Expand Down
1 change: 1 addition & 0 deletions mater/lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ thiserror.workspace = true
tokio = { workspace = true, features = ["fs", "macros", "rt-multi-thread"] }
tokio-stream.workspace = true
tokio-util = { workspace = true, features = ["io"] }
clap = { workspace = true, features = ["derive"] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be removed. The mater crate has nothing to do with the CLI

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should hide the clap behind a feature flag. This was one of the reasons why we split the cli and lib.


# Optional dependencies
blockstore = { workspace = true, optional = true }
Expand Down
6 changes: 5 additions & 1 deletion mater/lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ mod unixfs;
mod v1;
mod v2;

pub use stores::WrapMode;
pub use stores::ConvertConfig;
pub use stores::filestore::create_filestore;

// We need to re-expose this because `read_block` returns `(Cid, Vec<u8>)`.
pub use ipld_core::cid::Cid;
pub use multicodec::{DAG_PB_CODE, IDENTITY_CODE, RAW_CODE};
pub use stores::{create_filestore, Blockstore, Config, FileBlockstore};
pub use stores::{Blockstore, Config, FileBlockstore};
pub use v1::{Header as CarV1Header, Reader as CarV1Reader, Writer as CarV1Writer};
pub use v2::{
verify_cid, Characteristics, Header as CarV2Header, Index, IndexEntry, IndexSorted,
Expand Down
Loading