Skip to content

[WIP] feat(sdk): domain specific hashing for signature #26

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
15 changes: 11 additions & 4 deletions types/src/consensus/certificate.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
use crate::cryptography::{ecdsa::SignatureECDSA, hash::Hashable, Hash, SigHashable};
use serde::{Deserialize, Serialize};

use crate::cryptography::{ecdsa::SignatureECDSA, hash::Hashable};

// Certificate represents a proof on an agreement by the committee
#[derive(Clone, Serialize, Deserialize, Debug)]
pub struct Certificate<T: Hashable> {
pub struct Certificate<T>
where
T: Hashable + SigHashable,
{
pub signatures: Vec<SignatureECDSA>,
pub certified: T,
}

impl<T: Hashable> Certificate<T> {
impl<T: Hashable + SigHashable> Certificate<T> {
pub fn new(signatures: Vec<SignatureECDSA>, certified: T) -> Self {
Certificate {
signatures,
certified,
}
}
}
impl<T: Hashable + SigHashable> SigHashable for Certificate<T> {
fn hash_for_signature(&self) -> Hash {
self.certified.hash_for_signature()
}
}
26 changes: 19 additions & 7 deletions types/src/consensus/committee.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use super::{Attestation, Certificate};
use crate::cryptography::hash::{DomainDigest, MessageDigest};
use crate::sig_hash::SigHashable;
use crate::{
cryptography::{ecdsa::AddressECDSA, hash::Hashable},
ecdsa::SignatureECDSA,
Expand Down Expand Up @@ -42,15 +44,22 @@ impl Committee {
self.validator_set.contains_key(address)
}

pub fn verify_attestation<T: Hashable>(&self, attestation: &Attestation<T>) -> Result<bool> {
pub fn verify_attestation<T: Hashable>(
&self,
attestation: &Attestation<T>,
domain: DomainDigest,
) -> Result<bool> {
if !self.is_in_committee(&attestation.public_key) {
return Err(anyhow!("Validator not in committee"));
}

attestation.public_key.verify(
attestation.attested.hash_custom().as_slice(),
&attestation.signature,
)
let digest = MessageDigest {
domain,
message: attestation.attested.hash_custom(),
};
attestation
.public_key
.verify(digest.hash_custom().as_slice(), &attestation.signature)
}

// utility function that does aggregate verification over an arbitrary hash
Expand Down Expand Up @@ -94,9 +103,12 @@ impl Committee {
Ok(true)
}

pub fn verify_certificate<C: Hashable>(&self, certificate: &Certificate<C>) -> Result<bool> {
pub fn verify_certificate<C: Hashable + SigHashable>(
&self,
certificate: &Certificate<C>,
) -> Result<bool> {
self.verify_aggregate_attestation(
certificate.certified.hash_custom(),
certificate.certified.hash_for_signature(),
&certificate.signatures,
)
}
Expand Down
32 changes: 32 additions & 0 deletions types/src/cryptography/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,38 @@ pub use alloy_primitives::{keccak256 as hash, B256 as Hash};
use bytes::Bytes;
use std::hash::{DefaultHasher, Hash as StdHash, Hasher};

pub struct DomainDigest {
pub prefix: &'static [u8],
pub version: u8,
}

impl DomainDigest {
pub fn new(prefix: &'static [u8], version: u8) -> DomainDigest {
Self { prefix, version }
}
}
pub struct MessageDigest {
pub domain: DomainDigest,
pub message: Hash,
}

impl Hashable for MessageDigest {
fn hash_custom(&self) -> Hash {
let mut v = Vec::with_capacity(self.domain.prefix.len() + 1 + 32);
v.extend_from_slice(self.domain.prefix);
v.push(self.domain.version);
v.extend_from_slice(self.message.as_slice());

hash(v.as_slice())
}
}

impl MessageDigest {
pub fn new(domain: DomainDigest, message: Hash) -> MessageDigest {
Self { domain, message }
}
}

pub trait Hashable {
fn hash_custom(&self) -> Hash;
}
Expand Down
2 changes: 2 additions & 0 deletions types/src/cryptography/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
pub mod ecdsa;
pub mod hash;
pub mod merkle_tree;
pub mod sig_hash;
pub mod signer;

pub use hash::{Hash, Hashable};
pub use merkle_tree::{MerkleMultiProof, MerkleTree, Merkleizable};
pub use sig_hash::SigHashable;
pub use signer::Signer;
12 changes: 12 additions & 0 deletions types/src/cryptography/sig_hash.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use crate::cryptography::Hash;

pub const SIG_PREFIX_GAS_PRICE_ATTESTATION: &[u8] = b"ATT_GAS_PRICE";
pub const SIG_VERSION_GAS_PRICE_ATTESTATION: u8 = 1;
Comment on lines +3 to +4
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to separate prefix and version? Isn't it equal to have the version in the prefix like:

  • "ATT_GAS_PRICE" for version 0
  • "ATT_GAS_PRICE_1" for version 1

and so on?

pub const SIG_PREFIX_TX_ATTESTATION: &[u8] = b"ATT_TX";
pub const SIG_VERSION_TX_ATTESTATION: u8 = 1;
pub const SIG_PREFIX_RECEIPT_ATTESTATION: &[u8] = b"ATT_RECEIPT";
pub const SIG_VERSION_RECEIPT_ATTESTATION: u8 = 1;

pub trait SigHashable {
fn hash_for_signature(&self) -> Hash;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it also take a network-specifc prefix like this

Suggested change
fn hash_for_signature(&self) -> Hash;
fn hash_for_signature(&self, network_prefix: &[u8]) -> Hash;

where this prefix would be different on each network (e.g. devnet, testnet, mainnet etc.) so that signatures signed for devnet cannot be re-used on other networks?

}
23 changes: 23 additions & 0 deletions types/src/ledger/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@ use serde::{Deserialize, Serialize};
use std::convert::TryInto;

use super::{log, Transaction};
use crate::cryptography::hash::{DomainDigest, MessageDigest};
use crate::cryptography::{
hash::{Hash, Hashable},
merkle_tree::{index_prefix, MerkleBuilder, MerkleMultiProof, MerkleProof, Merkleizable},
signer::{Signed, UncheckedSigned},
};
use crate::sig_hash::{
SigHashable, SIG_PREFIX_RECEIPT_ATTESTATION, SIG_VERSION_RECEIPT_ATTESTATION,
};

#[derive(Clone, Serialize, Deserialize, Debug, Eq, PartialEq)]
pub struct Receipt {
Expand Down Expand Up @@ -93,6 +97,19 @@ impl Hashable for Receipt {
}
}

impl SigHashable for Receipt {
fn hash_for_signature(&self) -> Hash {
let digest = MessageDigest {
domain: DomainDigest {
prefix: SIG_PREFIX_RECEIPT_ATTESTATION,
version: SIG_VERSION_RECEIPT_ATTESTATION,
},
message: self.hash_custom(),
};
digest.hash_custom()
}
}

impl TryInto<TransactionReceipt> for Receipt {
type Error = anyhow::Error;

Expand Down Expand Up @@ -234,6 +251,12 @@ impl Hashable for UncheckedReceipt {
}
}

impl SigHashable for UncheckedReceipt {
fn hash_for_signature(&self) -> Hash {
Hash::default()
}
}

#[cfg(test)]
mod test {
use alloy_primitives::{Log, LogData, TxKind, U256};
Expand Down
22 changes: 22 additions & 0 deletions types/src/ledger/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ use alloy_consensus::{transaction::RlpEcdsaTx, SignableTransaction, TxLegacy};
use alloy_primitives::Address;
use alloy_sol_types::SolValue;

use crate::cryptography::hash::{DomainDigest, MessageDigest};
use crate::cryptography::{
hash::{Hash, Hashable},
merkle_tree::{MerkleBuilder, Merkleizable},
signer::{Signed, UncheckedSigned},
SigHashable,
};
use crate::sig_hash::{SIG_PREFIX_TX_ATTESTATION, SIG_VERSION_TX_ATTESTATION};

pub type Transaction = TxLegacy;

Expand Down Expand Up @@ -34,8 +37,27 @@ impl Hashable for Signed<Transaction> {
}
}

impl SigHashable for Signed<Transaction> {
fn hash_for_signature(&self) -> Hash {
let digest = MessageDigest {
domain: DomainDigest {
prefix: SIG_PREFIX_TX_ATTESTATION,
version: SIG_VERSION_TX_ATTESTATION,
},
message: self.hash_custom(),
};
digest.hash_custom()
}
}

impl<T: Hashable + Clone> Hashable for UncheckedSigned<T> {
fn hash_custom(&self) -> Hash {
self.signed.hash_custom()
}
}

impl<T: Hashable + Clone> SigHashable for UncheckedSigned<T> {
fn hash_for_signature(&self) -> Hash {
Hash::default()
}
}
1 change: 1 addition & 0 deletions types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub use crate::{
cryptography::{
ecdsa,
hash::std_hash,
sig_hash,
signer::{Signed, Signer},
Hashable, MerkleTree, Merkleizable,
},
Expand Down