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.

Sign up for GitHub

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

refactor: Increase code reuse in hashchain and tree #146

Merged
merged 13 commits into from
Nov 1, 2024
Merged

refactor: Increase code reuse in hashchain and tree #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6357943
fix: correct default value when checking for revoked keys
jns-ps Oct 17, 2024
17caaa3
fix: correct error message for account deletion
jns-ps Oct 17, 2024
4eea49b
refactor: remove unused iter methods
jns-ps Oct 17, 2024
728d6f4
refactor: Replace hashchain's create_account with from_operation
jns-ps Oct 28, 2024
df2aa72
refactor: Replace hashchain's register_service with from_operation
jns-ps Oct 28, 2024
54bc54b
refactor: correct name for variables of type VerifyingKey
jns-ps Oct 30, 2024
578bd81
refactor: use a single way of adding operations to a hashchain
jns-ps Oct 31, 2024
57f78de
refactor: avoid duplicate signature verification when creating account
jns-ps Oct 31, 2024
6a3d4bc
refactor: better naming for some funcs and vars
jns-ps Oct 31, 2024
d40cafa
fix: no newlines in error messages
jns-ps Oct 31, 2024
ddefee9
fix: re-add missing debug log
jns-ps Oct 31, 2024
1bc736a
fix: unbox update proofs after rebasing onto main
jns-ps Oct 31, 2024
5c55fef
chore: Incorporate changes in zkvm elf
jns-ps Oct 31, 2024
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
215 changes: 38 additions & 177 deletions crates/common/src/hashchain.rs
View file
Open in desktop
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)
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
jns-ps marked this conversation as resolved.
Show resolved Hide resolved
}

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)
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
}
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
Open in desktop
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
distractedm1nd marked this conversation as resolved.
Show resolved Hide resolved
}

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