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

storage: refactor and rocksdb implementation #123

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
586 changes: 232 additions & 354 deletions Cargo.lock

Large diffs are not rendered by default.

17 changes: 14 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,22 @@ keywords = ["crypto", "key-transparency"]
readme = "README.md"

[workspace]
default-members = [
"crates/prism",
"crates/common",
"crates/errors",
"crates/storage",
"crates/da",
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
]
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved

members = [
"crates/prism",
"crates/common",
"crates/nova",
"crates/groth16",
"crates/errors",
"crates/sp1",
"crates/storage",
"crates/da",
]
default-members = ["crates/prism", "crates/common", "crates/errors"]
resolver = "2"

[workspace.dependencies]
Expand Down Expand Up @@ -72,10 +79,14 @@ secp256k1 = "0.29.0"
sp1-zkvm = { version = "1.2.0" }
sp1-sdk = { version = "1.2.0" }
prism-common = { path = "crates/common" }
prism-storage = { path = "crates/storage" }
prism-nova = { path = "crates/nova" }
prism-da = { path = "crates/da" }
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
prism-errors = { path = "crates/errors" }
prism-main = { path = "crates/prism" }
prism-groth16 = { path = "crates/groth16" }
rocksdb = { version = "0.21.0", features = ["multi-threaded-cf"] }


[patch.crates-io]
sha2-v0-10-8 = { git = "https://github.com/sp1-patches/RustCrypto-hashes", package = "sha2", branch = "patch-sha2-v0.10.8" }
Expand Down
13 changes: 6 additions & 7 deletions crates/common/src/hashchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::{

use crate::{
operation::{CreateAccountArgs, Operation, PublicKey, ServiceChallengeInput},
tree::{hash, Digest, Hasher},
tree::{Digest, Hasher},
};

#[derive(Clone, Serialize, Deserialize, Debug, PartialEq)]
Expand Down Expand Up @@ -280,12 +280,11 @@ pub struct HashchainEntry {

impl HashchainEntry {
pub fn new(operation: Operation, previous_hash: Digest) -> Self {
let hash = {
let mut data = Vec::new();
data.extend_from_slice(operation.to_string().as_bytes());
data.extend_from_slice(previous_hash.as_ref());
hash(&data)
};
let mut data = Vec::new();
data.extend_from_slice(operation.to_string().as_bytes());
data.extend_from_slice(previous_hash.as_ref());
let hash: Digest = data.as_slice().into();

Self {
hash,
previous_hash,
Expand Down
23 changes: 13 additions & 10 deletions crates/common/src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use jmt::{
};
use prism_errors::DatabaseError;
use serde::{ser::SerializeTupleStruct, Deserialize, Serialize};
use std::convert::{From, Into};
use std::sync::Arc;

use crate::{
Expand Down Expand Up @@ -70,17 +71,19 @@ impl SimpleHasher for Hasher {
}
}

pub fn hash(data: &[u8]) -> Digest {
let mut hasher = Hasher::new();
hasher.update(data);
Digest(hasher.finalize())
}

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Copy)]
pub struct Digest([u8; 32]);

impl Digest {
pub fn to_bytes(&self) -> [u8; 32] {
impl From<&[u8]> for Digest {
fn from(data: &[u8]) -> Self {
let mut hasher = Hasher::new();
hasher.update(data);
Self(hasher.finalize())
}
}

impl Into<[u8; 32]> for &Digest {
fn into(self) -> [u8; 32] {
self.0
}
}
Expand Down Expand Up @@ -316,7 +319,7 @@ where
match operation {
Operation::AddKey(KeyOperationArgs { id, .. })
| Operation::RevokeKey(KeyOperationArgs { id, .. }) => {
let hashed_id = hash(id.as_bytes());
let hashed_id: Digest = id.as_bytes().into();
let key_hash = KeyHash::with::<Hasher>(hashed_id);

let mut current_chain = self
Expand All @@ -337,7 +340,7 @@ where
service_id,
challenge,
}) => {
let hashed_id = hash(id.as_bytes());
let hashed_id: Digest = id.as_bytes().into();
let key_hash = KeyHash::with::<Hasher>(hashed_id);

match &challenge {
Expand Down
52 changes: 52 additions & 0 deletions crates/da/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
[package]
name = "prism-da"
version.workspace = true
authors.workspace = true
edition.workspace = true
description.workspace = true
homepage.workspace = true
repository.workspace = true
license.workspace = true
keywords.workspace = true
readme.workspace = true
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved

[dependencies]
axum = { workspace = true }
borsh = { workspace = true }
tower-http = { workspace = true }
utoipa = { workspace = true }
utoipa-swagger-ui = { workspace = true }
async-trait = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
redis = { workspace = true }
ed25519-dalek = { workspace = true }
base64 = { workspace = true }
tokio = { workspace = true }
bellman = { workspace = true }
bincode = { workspace = true }
bls12_381 = { workspace = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

lets run cargo-udeps on this PR to see which deps we can remove

rand = { workspace = true }
hex = { workspace = true }
ff = { workspace = true }
log = { workspace = true }
pretty_env_logger = { workspace = true }
clap = { workspace = true }
config = { workspace = true }
thiserror = { workspace = true }
indexed-merkle-tree = { workspace = true }
dotenvy = { workspace = true }
celestia-rpc = { workspace = true }
celestia-types = { workspace = true }
mockall = { workspace = true }
keystore-rs = { workspace = true }
toml = { workspace = true }
dirs = { workspace = true }
anyhow = { workspace = true }
jmt = { workspace = true }
sha2 = { workspace = true }
auto_impl = { workspace = true }
prism-common = { workspace = true }
prism-errors = { workspace = true }
prism-groth16 = { workspace = true }
sp1-sdk = { workspace = true }
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
26 changes: 22 additions & 4 deletions crates/prism/src/da/celestia.rs → crates/da/src/celestia.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use crate::{
cfg::CelestiaConfig,
da::{DataAvailabilityLayer, FinalizedEpoch},
};
use crate::{DataAvailabilityLayer, FinalizedEpoch};
use anyhow::{anyhow, Context, Result};
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
use async_trait::async_trait;
use celestia_rpc::{BlobClient, Client, HeaderClient};
use celestia_types::{blob::GasPrice, nmt::Namespace, Blob};
use log::{debug, error, trace, warn};
use prism_common::operation::Operation;
use prism_errors::{DataAvailabilityError, GeneralError};
use serde::{Deserialize, Serialize};
use std::{
self,
sync::{
Expand All @@ -29,6 +28,25 @@ impl TryFrom<&Blob> for FinalizedEpoch {
}
}

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct CelestiaConfig {
pub connection_string: String,
pub start_height: u64,
pub snark_namespace_id: String,
pub operation_namespace_id: Option<String>,
}

distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
impl Default for CelestiaConfig {
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
fn default() -> Self {
CelestiaConfig {
connection_string: "ws://localhost:26658".to_string(),
start_height: 0,
snark_namespace_id: "00000000000000de1008".to_string(),
operation_namespace_id: Some("00000000000000de1009".to_string()),
}
}
}

distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
pub struct CelestiaConnection {
pub client: celestia_rpc::Client,
pub snark_namespace: Namespace,
Expand Down
File renamed without changes.
75 changes: 75 additions & 0 deletions crates/da/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
use anyhow::Result;
use async_trait::async_trait;
use ed25519_dalek::{Signature, Signer, SigningKey, VerifyingKey};
use prism_common::{operation::Operation, tree::Digest};
use serde::{Deserialize, Serialize};
use sp1_sdk::SP1ProofWithPublicValues;
use tokio::sync::broadcast;

pub mod celestia;
pub mod consts;
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
pub mod memory;

// FinalizedEpoch is the data structure that represents the finalized epoch data, and is posted to the DA layer.
#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct FinalizedEpoch {
pub height: u64,
pub prev_commitment: Digest,
pub current_commitment: Digest,
pub proof: SP1ProofWithPublicValues,
pub signature: Option<String>,
}

impl FinalizedEpoch {
pub fn insert_signature(&mut self, key: &SigningKey) {
let plaintext = bincode::serialize(&self).unwrap();
Copy link

@coderabbitai coderabbitai bot Sep 23, 2024

Choose a reason for hiding this comment

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

Handle potential serialization errors to prevent panics

Using unwrap() on the result of bincode::serialize can cause a panic if serialization fails. It's better to handle this error to prevent unexpected crashes and improve the robustness of the code.

Apply this diff to handle the error:

-    let plaintext = bincode::serialize(&epoch_without_signature).unwrap();
+    let plaintext = match bincode::serialize(&epoch_without_signature) {
+        Ok(data) => data,
+        Err(e) => {
+            return Err(anyhow::anyhow!("Failed to serialize epoch: {}", e));
+        }
+    };

Alternatively, if the function returns a Result, you can use the ? operator:

-    let plaintext = bincode::serialize(&epoch_without_signature).unwrap();
+    let plaintext = bincode::serialize(&epoch_without_signature)
+        .map_err(|e| anyhow::anyhow!("Failed to serialize epoch: {}", e))?;

Committable suggestion was skipped due to low confidence.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you create an issue for this? It needs to be fixed but not in this PR

Copy link

Choose a reason for hiding this comment

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

@distractedm1nd, the GitHub issue has been created successfully: Issue #127.

Analysis chain

@distractedm1nd, I'll create an issue to track this.


@distractedm1nd, thanks for pointing out the issue. The error occurs because backticks in the shell script are interpreted as command substitutions. Here's the corrected script with escaped backticks:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Create a GitHub issue to handle serialization errors in FinalizedEpoch

gh issue create \
  --title "Handle potential serialization errors in `FinalizedEpoch::insert_signature`" \
  --body "The method `FinalizedEpoch::insert_signature` uses `unwrap()` on the result of `bincode::serialize`, which can cause a panic if serialization fails. It's better to handle this error to prevent unexpected crashes and improve the robustness of the code.

This issue was identified during discussion in PR [#123](https://github.com/deltadevsde/prism/pull/123) by [@coderabbitai[bot]](https://github.com/coderabbitai). Please refer to the comment [here](https://github.com/deltadevsde/prism/pull/123#discussion_r1770704323) for more details.

Requested by @distractedm1nd." \
  --assignee distractedm1nd

Length of output: 1147


Script:

#!/bin/bash
# Create a GitHub issue to handle serialization errors in FinalizedEpoch

gh issue create \
  --title "Handle potential serialization errors in \`FinalizedEpoch::insert_signature\`" \
  --body "The method \`FinalizedEpoch::insert_signature\` uses \`unwrap()\` on the result of \`bincode::serialize\`, which can cause a panic if serialization fails. It's better to handle this error to prevent unexpected crashes and improve the robustness of the code.

This issue was identified during discussion in PR [#123](https://github.com/deltadevsde/prism/pull/123) by [@coderabbitai[bot]](https://github.com/coderabbitai). Please refer to the comment [here](https://github.com/deltadevsde/prism/pull/123#discussion_r1770704323) for more details.

Requested by @distractedm1nd." \
  --assignee distractedm1nd

Length of output: 758

let signature = key.sign(&plaintext);
self.signature = Some(hex::encode(signature.to_bytes()));
}
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved

pub fn verify_signature(&self, vk: VerifyingKey) -> Result<()> {
let epoch_without_signature = FinalizedEpoch {
height: self.height,
prev_commitment: self.prev_commitment,
current_commitment: self.current_commitment,
proof: self.proof.clone(),
signature: None,
};

let message = bincode::serialize(&epoch_without_signature)
.map_err(|e| anyhow::anyhow!("Failed to serialize epoch: {}", e))?;

let signature = self
.signature
.as_ref()
.ok_or_else(|| anyhow::anyhow!("No signature present"))?;

let signature_bytes = hex::decode(signature)
.map_err(|e| anyhow::anyhow!("Failed to decode signature: {}", e))?;

if signature_bytes.len() != 64 {
return Err(anyhow::anyhow!("Invalid signature length"));
}

let signature: Signature = signature_bytes
.as_slice()
.try_into()
.map_err(|_| anyhow::anyhow!("Invalid signature length"))?;

vk.verify_strict(&message, &signature)
.map_err(|e| anyhow::anyhow!("Signature verification failed: {}", e))?;
Ok(())
}
}

#[async_trait]
pub trait DataAvailabilityLayer: Send + Sync {
async fn get_latest_height(&self) -> Result<u64>;
async fn initialize_sync_target(&self) -> Result<u64>;
async fn get_finalized_epoch(&self, height: u64) -> Result<Option<FinalizedEpoch>>;
async fn submit_finalized_epoch(&self, epoch: FinalizedEpoch) -> Result<u64>;
async fn get_operations(&self, height: u64) -> Result<Vec<Operation>>;
async fn submit_operations(&self, operations: Vec<Operation>) -> Result<u64>;
async fn start(&self) -> Result<()>;
fn subscribe_to_heights(&self) -> broadcast::Receiver<u64>;
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
}
3 changes: 2 additions & 1 deletion crates/prism/src/da/memory.rs → crates/da/src/memory.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::da::{DataAvailabilityLayer, FinalizedEpoch};
use crate::{DataAvailabilityLayer, FinalizedEpoch};
use anyhow::Result;
use async_trait::async_trait;
use log::debug;
use prism_common::operation::Operation;
use std::{collections::VecDeque, sync::Arc};
use tokio::{
Expand Down
2 changes: 2 additions & 0 deletions crates/prism/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ jmt = { workspace = true }
sha2 = { workspace = true }
auto_impl = { workspace = true }
prism-common = { workspace = true, features = ["test_utils"] }
prism-storage = { workspace = true }
prism-errors = { workspace = true }
prism-da = { workspace = true }
prism-groth16 = { workspace = true }
sp1-sdk = { workspace = true }

Expand Down
47 changes: 9 additions & 38 deletions crates/prism/src/cfg.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
use crate::{
consts::{DA_RETRY_COUNT, DA_RETRY_INTERVAL},
da::memory::InMemoryDataAvailabilityLayer,
};
use anyhow::{anyhow, Context, Result};
use clap::{Parser, Subcommand};
use config::{builder::DefaultState, ConfigBuilder, File};
use dirs::home_dir;
use dotenvy::dotenv;
use log::{error, warn};
use prism_errors::{DataAvailabilityError, GeneralError, PrismError};
use prism_storage::redis::RedisConfig;
use serde::{Deserialize, Serialize};
use std::{fs, path::Path, sync::Arc};

use crate::da::{celestia::CelestiaConnection, DataAvailabilityLayer};
use prism_da::{
celestia::{CelestiaConfig, CelestiaConnection},
consts::{DA_RETRY_COUNT, DA_RETRY_INTERVAL},
memory::InMemoryDataAvailabilityLayer,
DataAvailabilityLayer,
};

#[derive(Clone, Debug, Subcommand, Deserialize)]
pub enum Commands {
Expand Down Expand Up @@ -97,38 +100,6 @@ impl Default for WebServerConfig {
}
}

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct RedisConfig {
pub connection_string: String,
}

impl Default for RedisConfig {
fn default() -> Self {
RedisConfig {
connection_string: "redis://127.0.0.1/".to_string(),
}
}
}

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct CelestiaConfig {
pub connection_string: String,
pub start_height: u64,
pub snark_namespace_id: String,
pub operation_namespace_id: Option<String>,
}

impl Default for CelestiaConfig {
fn default() -> Self {
CelestiaConfig {
connection_string: "ws://localhost:26658".to_string(),
start_height: 0,
snark_namespace_id: "00000000000000de1008".to_string(),
operation_namespace_id: Some("00000000000000de1009".to_string()),
}
}
}

impl Default for Config {
fn default() -> Self {
Config {
Expand Down Expand Up @@ -300,7 +271,7 @@ pub async fn initialize_da_layer(
unreachable!() // This line should never be reached due to the return in the last iteration
}
DALayerOption::InMemory => {
let (da_layer, _height_rx, _block_rx) = InMemoryDataAvailabilityLayer::new(1);
let (da_layer, _height_rx, _block_rx) = InMemoryDataAvailabilityLayer::new(30);
Ok(Arc::new(da_layer) as Arc<dyn DataAvailabilityLayer + 'static>)
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
}
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
DALayerOption::None => Err(anyhow!(PrismError::ConfigError(
Expand Down
3 changes: 0 additions & 3 deletions crates/prism/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
pub mod cfg;
pub mod consts;
pub mod da;
pub mod node_types;
pub mod storage;
pub mod utils;
pub mod webserver;
#[macro_use]
Expand Down
Loading
Loading