Skip to content

Commit

Permalink
refactor: Increase code reuse in hashchain and tree (#146)
Browse files Browse the repository at this point in the history 
* fix: correct default value when checking for revoked keys

* fix: correct error message for account deletion

* refactor: remove unused iter methods

* refactor: Replace hashchain's create_account with from_operation

* refactor: Replace hashchain's register_service with from_operation

* refactor: correct name for variables of type VerifyingKey

* refactor: use a single way of adding operations to a hashchain

* refactor: avoid duplicate signature verification when creating account

* refactor: better naming for some funcs and vars

* fix: no newlines in error messages

* fix: re-add missing debug log

* fix: unbox update proofs after rebasing onto main

* chore: Incorporate changes in zkvm elf
  • Loading branch information
@jns-ps
jns-ps authored Nov 1, 2024
1 parent 46f19a8 commit 41a0898
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 277 deletions.
215 changes: 38 additions & 177 deletions crates/common/src/hashchain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,7 @@ use std::{
ops::{Deref, DerefMut},
};

use crate::{
digest::Digest,
hasher::Hasher,
keys::VerifyingKey,
operation::{
CreateAccountArgs, Operation, RegisterServiceArgs, ServiceChallenge, ServiceChallengeInput,
},
};
use crate::{digest::Digest, hasher::Hasher, keys::VerifyingKey, operation::Operation};

#[derive(Clone, Serialize, Deserialize, Debug, PartialEq)]
pub struct Hashchain {
Expand Down Expand Up @@ -69,138 +62,13 @@ impl Hashchain {
Ok(hc)
}

pub fn create_account(
id: String,
value: VerifyingKey,
service_id: String,
challenge: ServiceChallengeInput,
prev_hash: Digest,
signature: Vec<u8>,
) -> Result<Hashchain> {
let mut hc = Hashchain::empty(id.clone());
let operation = Operation::CreateAccount(CreateAccountArgs {
id,
value,
service_id,
challenge,
prev_hash,
signature,
});
hc.perform_operation(operation)?;
Ok(hc)
}

pub fn register_service(id: String, challenge: ServiceChallenge) -> Result<Hashchain> {
let mut hc = Hashchain::empty(id.clone());
let operation = Operation::RegisterService(RegisterServiceArgs {
id,
creation_gate: challenge,
prev_hash: Digest::zero(),
});
hc.perform_operation(operation)?;
Ok(hc)
}

pub fn empty(id: String) -> Self {
Self {
id,
entries: Vec::new(),
}
}

pub fn verify_last_entry(&self) -> Result<()> {
if self.entries.is_empty() {
return Ok(());
}

let mut valid_keys: HashSet<VerifyingKey> = HashSet::new();

for (index, entry) in self.entries.iter().enumerate().take(self.entries.len() - 1) {
match &entry.operation {
Operation::RegisterService(_) => {
if index != 0 {
bail!("RegisterService operation must be the first entry");
}
}
Operation::CreateAccount(args) => {
if index != 0 {
bail!("CreateAccount operation must be the first entry");
}
valid_keys.insert(args.value.clone());
}
Operation::AddKey(args) => {
valid_keys.insert(args.value.clone());
}
Operation::RevokeKey(args) => {
valid_keys.remove(&args.value);
}
Operation::AddData(_) => {}
}
}

let last_entry = self.entries.last().unwrap();
let last_index = self.entries.len() - 1;
if last_index > 0 {
let prev_entry = &self.entries[last_index - 1];
if last_entry.previous_hash != prev_entry.hash {
bail!("Previous hash mismatch for the last entry");
}
}

match &last_entry.operation {
// TODO: RegisterService should not be permissionless at first, until we have state bloat metrics
Operation::RegisterService(_) => {
if last_index != 0 {
bail!("RegisterService operation must be the first entry");
}
}
Operation::CreateAccount(args) => {
if last_index != 0 {
bail!("CreateAccount operation must be the first entry");
}
args.value.verify_signature(
&bincode::serialize(
&last_entry.operation.without_signature().without_challenge(),
)?,
&args.signature,
)?;
}
Operation::AddKey(args) | Operation::RevokeKey(args) => {
self.verify_signature_at_key_idx(
&last_entry.operation,
&args.signature.signature,
args.signature.key_idx,
&valid_keys,
)?;
}
Operation::AddData(args) => {
self.verify_signature_at_key_idx(
&last_entry.operation,
&args.op_signature.signature,
args.op_signature.key_idx,
&valid_keys,
)?;

let Some(value_signature) = &args.value_signature else {
return Ok(());
};

// If data to be added is signed, also validate its signature
value_signature
.verifying_key
.verify_signature(&args.value, &value_signature.signature)?;
}
}

Ok(())
}

pub(crate) fn insert_unsafe(&self, new_entry: HashchainEntry) -> Hashchain {
let mut new = self.clone();
new.entries.push(new_entry);
new
}

pub fn get_key_at_index(&self, idx: usize) -> Result<&VerifyingKey> {
self.entries
.get(idx)
Expand Down Expand Up @@ -228,7 +96,7 @@ impl Hashchain {
valid_keys
}

pub fn is_key_revoked(&self, key: VerifyingKey) -> bool {
pub fn is_key_invalid(&self, key: VerifyingKey) -> bool {
self.iter()
.rev()
.find_map(|entry| match entry.operation.clone() {
Expand All @@ -237,35 +105,7 @@ impl Hashchain {
Operation::CreateAccount(args) if args.value == key => Some(false),
_ => None,
})
.unwrap_or(false)
}

fn verify_signature_at_key_idx(
&self,
operation: &Operation,
signature: &[u8],
idx: usize,
valid_keys: &HashSet<VerifyingKey>,
) -> Result<()> {
let verifying_key = self.get_key_at_index(idx)?;
if !valid_keys.contains(verifying_key) {
bail!(
"Key intended to verify signature {:?} is not in valid keys {:?}",
verifying_key,
valid_keys
);
}

let message = bincode::serialize(&operation.without_signature())?;
verifying_key.verify_signature(&message, signature)
}

pub fn iter(&self) -> std::slice::Iter<'_, HashchainEntry> {
self.entries.iter()
}

pub fn iter_mut(&mut self) -> std::slice::IterMut<'_, HashchainEntry> {
self.entries.iter_mut()
.unwrap_or(true)
}

pub fn get(&self, idx: usize) -> &HashchainEntry {
Expand Down Expand Up @@ -303,40 +143,61 @@ impl Hashchain {
}

if args.prev_hash != Digest::zero() {
bail!("Previous hash for initial operation must be zero")
bail!(
"Previous hash for initial operation must be zero, but was {}",
args.prev_hash
)
}

Ok(())
}
Operation::AddKey(args) | Operation::RevokeKey(args) => {
if args.prev_hash != self.last_hash() {
bail!("Previous hash for key operation must be the last hash")
let last_hash = self.last_hash();
if args.prev_hash != last_hash {
bail!(
"Previous hash for key operation must be the last hash - prev: {}, last: {}",
args.prev_hash,
last_hash
)
}

let signing_key = self.get_key_at_index(args.signature.key_idx)?;
let key_idx = args.signature.key_idx;
let verifying_key = self.get_key_at_index(key_idx)?;

if self.is_key_revoked(signing_key.clone()) {
bail!("The signing key is revoked");
if self.is_key_invalid(verifying_key.clone()) {
bail!(
"The key at index {}, intended to verify this operation, is invalid",
key_idx
);
}

operation.verify_user_signature(signing_key)
operation.verify_user_signature(verifying_key)
}
Operation::AddData(args) => {
if args.prev_hash != self.last_hash() {
bail!("Previous hash for add-data operation must be the last hash")
let last_hash = self.last_hash();
if args.prev_hash != last_hash {
bail!(
"Previous hash for add-data operation is not equal to the last hash - prev: {}, last: {}",
args.prev_hash,
last_hash
)
}

let signing_key = self.get_key_at_index(args.op_signature.key_idx)?;
let key_idx = args.op_signature.key_idx;
let verifying_key = self.get_key_at_index(key_idx)?;

if self.is_key_revoked(signing_key.clone()) {
bail!("The signing key is revoked");
if self.is_key_invalid(verifying_key.clone()) {
bail!(
"The key at index {}, intended to verify this operation, is invalid",
key_idx
);
}

operation.verify_user_signature(signing_key)
operation.verify_user_signature(verifying_key)
}
Operation::CreateAccount(args) => {
if !self.entries.is_empty() {
bail!("RegisterService operation must be the first entry");
bail!("CreateAccount operation must be the first entry");
}

if args.prev_hash != Digest::zero() {
Expand Down
35 changes: 25 additions & 10 deletions crates/common/src/test_utils.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::{
digest::Digest,
hashchain::Hashchain,
hasher::Hasher,
keys::{SigningKey, VerifyingKey},
operation::{Operation, ServiceChallenge, SignatureBundle},
tree::{
Expand Down Expand Up @@ -50,10 +52,10 @@ impl TestTreeState {
pub fn register_service(&mut self, service_id: String) -> Service {
let service_key = create_mock_signing_key();

let hashchain = Hashchain::register_service(
let hashchain = Hashchain::from_operation(Operation::new_register_service(
service_id.clone(),
ServiceChallenge::from(service_key.clone()),
)
))
.unwrap();

let key_hash = hashchain.get_keyhash();
Expand Down Expand Up @@ -189,12 +191,20 @@ pub fn create_random_insert(state: &mut TestTreeState, rng: &mut StdRng) -> Inse
let (_, service) =
state.services.iter().nth(rng.gen_range(0..state.services.len())).unwrap();

let hc = create_new_hashchain(random_string.as_str(), &sk, service.clone()); //Hashchain::new(random_string);
let key = hc.get_keyhash();
let operation = Operation::new_create_account(
random_string.clone(),
&sk,
service.id.clone(),
&service.sk,
)
.expect("Creating account operation should succeed");

let hashed_id = Digest::hash(&random_string);
let key_hash = KeyHash::with::<Hasher>(hashed_id);

if !state.inserted_keys.contains(&key) {
let proof = state.tree.insert(key, hc).expect("Insert should succeed");
state.inserted_keys.insert(key);
if !state.inserted_keys.contains(&key_hash) {
let proof = state.tree.insert(key_hash, operation).expect("Insert should succeed");
state.inserted_keys.insert(key_hash);
state.signing_keys.insert(random_string, sk);
return proof;
}
Expand All @@ -208,7 +218,7 @@ pub fn create_random_update(state: &mut TestTreeState, rng: &mut StdRng) -> Upda

let key = *state.inserted_keys.iter().nth(rng.gen_range(0..state.inserted_keys.len())).unwrap();

let Found(mut hc, _) = state.tree.get(key).unwrap() else {
let Found(hc, _) = state.tree.get(key).unwrap() else {
panic!("No response found for key. Cannot perform update.");
};

Expand All @@ -229,9 +239,14 @@ pub fn create_random_update(state: &mut TestTreeState, rng: &mut StdRng) -> Upda
0,
)
.unwrap();
hc.perform_operation(operation).expect("Adding to hashchain should succeed");

state.tree.update(key, hc.last().unwrap().clone()).expect("Update should succeed")
let Proof::Update(update_proof) =
state.tree.process_operation(&operation).expect("Processing operation should succeed")
else {
panic!("No update proof returned.");
};

*update_proof
}

#[cfg(not(feature = "secp256k1"))]
Expand Down
Loading

0 comments on commit 41a0898

Please sign in to comment.