Skip to content
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
3 changes: 3 additions & 0 deletions settings.mainnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ dobs_cache_directory = "cache/dobs"
# expiration time indicator for cleaning whole dobs cache, zero means never clean
dobs_cache_expiration_sec = 300

# expiration time in minutes dedicated for type_id and type_script decoder cache, zero means always update
decoders_cache_expiration_minutes = 1440

# all deployed on-chain Spore contracts binary hash (order from new to old)
# refer to: https://github.com/sporeprotocol/spore-contract/blob/master/docs/VERSIONS.md
[[available_spores]]
Expand Down
5 changes: 4 additions & 1 deletion settings.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ ckb_rpc = "https://testnet.ckbapp.dev/"
rpc_server_address = "0.0.0.0:8090"

# directory that stores decoders on hard-disk, including on-chain and off-chain binary files
decoders_cache_directory = "cache/decoders"
decoders_cache_directory = "cache/decoders/testnet"

Choose a reason for hiding this comment

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

medium

It's a good practice to namespace the cache directory for testnet to avoid conflicts. For consistency and to prevent any potential confusion when switching between environments, you might consider also namespacing the mainnet cache directory in settings.mainnet.toml (e.g., to cache/decoders/mainnet).

Choose a reason for hiding this comment

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

medium

Adding a network-specific path for the testnet cache is a good improvement to prevent conflicts. To be consistent and avoid potential issues, consider also updating settings.mainnet.toml to use a mainnet-specific path, for example cache/decoders/mainnet, instead of the generic cache/decoders.


# directory that stores DOBs rendering results on hard-disk
dobs_cache_directory = "cache/dobs"

# expiration time indicator for cleaning whole dobs cache, zero means never clean
dobs_cache_expiration_sec = 300

# expiration time in minutes dedicated for type_id and type_script decoder cache, zero means always update
decoders_cache_expiration_minutes = 1440

# all deployed on-chain Spore contracts binary hash (order from new to old)
# refer to: https://github.com/sporeprotocol/spore-contract/blob/master/docs/VERSIONS.md
[[available_spores]]
Expand Down
24 changes: 22 additions & 2 deletions src/decoder/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,26 @@ fn build_type_script_search_option(type_script: Script) -> CellQueryOptions {
CellQueryOptions::new_type(type_script)
}

fn file_older_than_minutes(file_path: &PathBuf, minutes: u64) -> bool {
if minutes == 0 {
return true;
}
let metadata = match std::fs::metadata(file_path) {
Ok(m) => m,
Err(_) => return true, // File doesn't exist or we can't access it, so it's "old".
};

let file_time = match metadata.modified().or_else(|_| metadata.created()) {
Ok(time) => time,
Err(_) => return true, // Can't get a timestamp, assume it's old.
};

match file_time.elapsed() {
Ok(elapsed) => elapsed.as_secs() >= minutes * 60,
Err(_) => true, // System clock is earlier than file time, assume it's old to be safe.
}
Comment on lines +39 to +52

Choose a reason for hiding this comment

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

medium

The error handling in this function consistently returns true while ignoring the underlying error. This is a safe fallback, but it makes debugging potential filesystem issues (like permissions) very difficult because the errors are completely silent. It would be beneficial to log these errors before returning true to provide insight into why a file couldn't be accessed.

}
Comment on lines +35 to +53

Choose a reason for hiding this comment

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

medium

This function is logically correct. However, it can be made more concise and idiomatic by chaining Result and Option combinators. This approach reduces nesting and can be more readable for developers familiar with functional-style error handling in Rust. The unwrap_or(true) pattern elegantly handles all error cases (file not found, timestamp unavailable, clock skew) by defaulting to the safe behavior of treating the cache as expired.

fn file_older_than_minutes(file_path: &PathBuf, minutes: u64) -> bool {
    if minutes == 0 {
        return true;
    }
    let threshold = std::time::Duration::from_secs(minutes * 60);
    // Treat the file as "old" (return true) if metadata can't be read,
    // timestamps are unavailable, or if the system clock is behind the file time.
    std::fs::metadata(file_path)
        .and_then(|m| m.modified().or_else(|_| m.created()))
        .map(|time| {
            time.elapsed()
                .map(|elapsed| elapsed >= threshold)
                .unwrap_or(true) // System clock is earlier than file time, assume old.
        })
        .unwrap_or(true) // File doesn't exist or timestamps unavailable, assume old.
}


fn build_batch_search_options(
type_args: &[u8; 32],
available_script_ids: &[ScriptId],
Expand Down Expand Up @@ -267,7 +287,7 @@ pub async fn parse_decoder_path(
DecoderLocationType::TypeId => {
let hash = decoder.hash.as_ref().ok_or(Error::DecoderHashNotFound)?;
decoder_path.push(format!("type_id_{}.bin", hex::encode(hash)));
if !decoder_path.exists() {
if file_older_than_minutes(&decoder_path, settings.decoders_cache_expiration_minutes) {
let decoder_search_option = build_type_id_search_option(hash.clone().into());
let decoder_binary = fetch_decoder_binary(rpc, decoder_search_option).await?;
std::fs::write(decoder_path.clone(), decoder_binary)
Expand All @@ -284,7 +304,7 @@ pub async fn parse_decoder_path(
"type_script_{}.bin",
hex::encode(script.calc_script_hash().raw_data())
));
if !decoder_path.exists() {
if file_older_than_minutes(&decoder_path, settings.decoders_cache_expiration_minutes) {
let decoder_search_option = build_type_script_search_option(script);
let decoder_binary = fetch_decoder_binary(rpc, decoder_search_option).await?;
std::fs::write(decoder_path.clone(), decoder_binary)
Expand Down
12 changes: 12 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ async fn main() {
"server settings: {}",
serde_json::to_string_pretty(&settings).unwrap()
);

tracing::info!("ensuring cache directories exist");
fs::create_dir_all(&settings.decoders_cache_directory)
.expect("failed to create decoders cache directory");
fs::create_dir_all(&settings.dobs_cache_directory)
.expect("failed to create DOBs cache directory");
Comment on lines +30 to +33

Choose a reason for hiding this comment

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

medium

Using .expect() will cause the application to panic if it fails to create the directories. While this is a fatal startup error, it's generally better to handle the Result explicitly, log a helpful error message, and exit the process cleanly. This avoids a panic and provides more context to the user running the application.

Comment on lines +30 to +33

Choose a reason for hiding this comment

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

medium

Using .expect() is suitable for fatal startup errors. However, the panic messages can be more helpful for debugging. By including the problematic path and the underlying I/O error, you can more quickly diagnose issues related to file permissions or incorrect path configurations.

Suggested change
fs::create_dir_all(&settings.decoders_cache_directory)
.expect("failed to create decoders cache directory");
fs::create_dir_all(&settings.dobs_cache_directory)
.expect("failed to create DOBs cache directory");
fs::create_dir_all(&settings.decoders_cache_directory)
.unwrap_or_else(|e| panic!("failed to create decoders cache directory at {:?}: {}", &settings.decoders_cache_directory, e));
fs::create_dir_all(&settings.dobs_cache_directory)
.unwrap_or_else(|e| panic!("failed to create DOBs cache directory at {:?}: {}", &settings.dobs_cache_directory, e));

tracing::info!(
"decoders cache directory: {:?}",
settings.decoders_cache_directory
);
tracing::info!("DOBs cache directory: {:?}", settings.dobs_cache_directory);

let rpc_server_address = settings.rpc_server_address.clone();
let cache_expiration = settings.dobs_cache_expiration_sec;
let decoder = decoder::DOBDecoder::new(settings);
Expand Down
2 changes: 2 additions & 0 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ impl From<Error> for ErrorObjectOwned {
#[derive(Deserialize)]
#[cfg_attr(test, derive(serde::Serialize, PartialEq, Debug))]
pub struct ClusterDescriptionField {
#[allow(dead_code)]
pub description: String,
pub dob: DOBClusterFormat,
}
Expand Down Expand Up @@ -257,6 +258,7 @@ pub struct Settings {
pub decoders_cache_directory: PathBuf,
pub dobs_cache_directory: PathBuf,
pub dobs_cache_expiration_sec: u64,
pub decoders_cache_expiration_minutes: u64,
pub onchain_decoder_deployment: Vec<OnchainDecoderDeployment>,
pub available_spores: Vec<ScriptId>,
pub available_clusters: Vec<ScriptId>,
Expand Down