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

Switch from sha256 to keccak256 #23

Merged
merged 8 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
487 changes: 253 additions & 234 deletions node/Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ rand = "0.8.0"
rocksdb = "0.21.0"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0.95"
sha2 = "0.10.6"
sha3 = "0.10.8"
snow = "0.9.3"
syn = "2.0.17"
tempfile = "3"
Expand Down
6 changes: 3 additions & 3 deletions node/actors/network/src/noise/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use concurrency::{
ctx, io,
io::{AsyncRead as _, AsyncWrite as _},
};
use crypto::{sha256::Sha256, ByteFmt};
use crypto::{keccak256::Keccak256, ByteFmt};
use std::{
pin::Pin,
task::{ready, Context, Poll},
Expand Down Expand Up @@ -75,7 +75,7 @@ impl Default for Buffer {
pub(crate) struct Stream<S = MeteredStream> {
/// Hash of the handshake messages.
/// Uniquely identifies the noise session.
id: Sha256,
id: Keccak256,
/// Underlying TCP stream.
#[pin]
inner: S,
Expand Down Expand Up @@ -138,7 +138,7 @@ where

/// Returns the noise session id.
/// See `Stream::id`.
pub(crate) fn id(&self) -> Sha256 {
pub(crate) fn id(&self) -> Keccak256 {
self.id
}

Expand Down
2 changes: 1 addition & 1 deletion node/deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ skip = [
{ name = "regex-syntax", version = "=0.6.29" },

# Old versions required by hyper.
{ name = "socket2", version = "=0.4.9" },
{ name = "socket2", version = "=0.4.10" },
moshababo marked this conversation as resolved.
Show resolved Hide resolved
]

[sources]
Expand Down
2 changes: 1 addition & 1 deletion node/libs/concurrency/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ anyhow.workspace = true
once_cell.workspace = true
pin-project.workspace = true
rand.workspace = true
sha2.workspace = true
sha3.workspace = true
thiserror.workspace = true
time.workspace = true
tokio.workspace = true
Expand Down
8 changes: 4 additions & 4 deletions node/libs/concurrency/src/ctx/rng.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rand::{
rngs::{OsRng, StdRng},
Rng, SeedableRng,
};
use sha2::{digest::Update as _, Digest as _};
use sha3::{digest::Update as _, Digest as _};
use std::sync::atomic::{AtomicU64, Ordering};

/// Splittable Pseudorandom Number Generators using Cryptographic Hashing
Expand All @@ -16,13 +16,13 @@ use std::sync::atomic::{AtomicU64, Ordering};
/// which is not crypto-secure, but for tests should be good enough.
pub(super) struct SplitProvider {
/// Hash builder of the prefix of the ID.
builder: sha2::Sha256,
builder: sha3::Keccak256,
/// Child branches constructed so far from this branch.
branch: AtomicU64,
}

impl SplitProvider {
fn next_builder(&self) -> sha2::Sha256 {
fn next_builder(&self) -> sha3::Keccak256 {
let branch = self.branch.fetch_add(1, Ordering::SeqCst);
self.builder.clone().chain(branch.to_le_bytes())
}
Expand All @@ -43,7 +43,7 @@ impl Provider {
pub(super) fn test() -> Provider {
Self::Split(
SplitProvider {
builder: sha2::Sha256::new().chain("Lzr81nDW8eSOMH".as_bytes()),
builder: sha3::Keccak256::new().chain("Lzr81nDW8eSOMH".as_bytes()),
Copy link
Contributor

Choose a reason for hiding this comment

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

this has nothing to do with evm. Is keccak faster than sha?

Copy link
Member

Choose a reason for hiding this comment

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

Depends. On hardware implementations Keccak/SHA3 is faster, but if there's no hardware support then it's slower than SHA2.
That being said, I just think it's easier for now to just use one hash algorithm for everything. If/when we do code profiling and realize that hashing is a bottleneck, we can change to a faster algorithm like BLAKE3 wherever we can.

branch: 0.into(),
}
.into(),
Expand Down
2 changes: 1 addition & 1 deletion node/libs/crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ blst.workspace = true
ed25519-dalek.workspace = true
hex.workspace = true
rand.workspace = true
sha2.workspace = true
sha3.workspace = true
thiserror.workspace = true
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
//! Wrappers for the SHA256 cryptographic hash algorithm.
//! Wrappers for the Keccak256 cryptographic hash algorithm.
use crate::ByteFmt;
use sha2::{digest::Update as _, Digest as _};
use sha3::{digest::Update as _, Digest as _};

mod test;
pub mod testonly;

Copy link
Contributor

Choose a reason for hiding this comment

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

Off-topic (?): Why does this module need to be public? AFAICT, it doesn't contain public items.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if you refer to line 5 (added) or 6 (already exists).

  • testonly: it needs to be public, although it works without the pub modifier, because visibility is bound to the trait implementation target type.
  • test: I forgot to add the #[cfg(test)] attribute. Added now.

/// SHA256 hash.
/// Keccak256 hash.
#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Sha256(pub(crate) [u8; 32]);
pub struct Keccak256(pub(crate) [u8; 32]);

impl Sha256 {
/// Computes a SHA256 hash of a message.
impl Keccak256 {
/// Computes a Keccak256 hash of a message.
pub fn new(msg: &[u8]) -> Self {
Self(sha2::Sha256::new().chain(msg).finalize().into())
Self(sha3::Keccak256::new().chain(msg).finalize().into())
}

/// Returns a reference to the bytes of this hash.
Expand All @@ -20,7 +21,7 @@ impl Sha256 {
}
}

impl ByteFmt for Sha256 {
impl ByteFmt for Keccak256 {
fn decode(bytes: &[u8]) -> anyhow::Result<Self> {
Ok(Self(bytes.try_into()?))
}
Expand Down
33 changes: 33 additions & 0 deletions node/libs/crypto/src/keccak256/test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#[test]
fn test_keccak256() -> Result<(), Box<dyn std::error::Error>> {
use crate::keccak256::Keccak256;

// Test vectors obtained from a trusted source
moshababo marked this conversation as resolved.
Show resolved Hide resolved
let test_vectors: Vec<(&[u8], [u8; 32])> = vec![
(
b"testing",
hex::decode("5f16f4c7f149ac4f9510d9cf8cf384038ad348b3bcdc01915f95de12df9d1b02")?
.try_into()
.unwrap(),
),
(
b"",
hex::decode("c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470")?
.try_into()
.unwrap(),
),
(
&[0x12, 0x34, 0x56],
hex::decode("6adf031833174bbe4c85eafe59ddb54e6584648c2c962c6f94791ab49caa0ad4")?
.try_into()
.unwrap(),
),
];

for (input, expected_hash) in &test_vectors {
let hash = Keccak256::new(input);
assert_eq!(hash.as_bytes(), expected_hash);
}

Ok(())
}
13 changes: 13 additions & 0 deletions node/libs/crypto/src/keccak256/testonly.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//! Random hash generation, intended for use in testing

use crate::keccak256::Keccak256;
use rand::{
distributions::{Distribution, Standard},
Rng,
};

impl Distribution<Keccak256> for Standard {
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> Keccak256 {
Keccak256(rng.gen())
}
}
2 changes: 1 addition & 1 deletion node/libs/crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
pub mod bls12_381;
pub mod ed25519;
mod fmt;
pub mod sha256;
pub mod keccak256;

pub use fmt::*;
13 changes: 0 additions & 13 deletions node/libs/crypto/src/sha256/testonly.rs

This file was deleted.

12 changes: 7 additions & 5 deletions node/libs/roles/src/node/messages.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::node;
use crypto::{sha256, ByteFmt, Text, TextFmt};
use crypto::{keccak256, ByteFmt, Text, TextFmt};
use utils::enum_util::{BadVariantError, Variant};

/// The ID for an authentication session.
Expand All @@ -17,7 +17,7 @@ pub enum Msg {
impl Msg {
/// Get the hash of this message.
pub fn hash(&self) -> MsgHash {
MsgHash(sha256::Sha256::new(&schema::canonical(self)))
MsgHash(keccak256::Keccak256::new(&schema::canonical(self)))
}
}

Expand Down Expand Up @@ -52,7 +52,7 @@ impl<V: Variant<Msg> + Clone> Signed<V> {
}

/// The hash of a message.
pub struct MsgHash(pub(super) sha256::Sha256);
pub struct MsgHash(pub(super) keccak256::Keccak256);

impl ByteFmt for MsgHash {
fn encode(&self) -> Vec<u8> {
Expand All @@ -66,11 +66,13 @@ impl ByteFmt for MsgHash {
impl TextFmt for MsgHash {
fn encode(&self) -> String {
format!(
"validator_msg:sha256:{}",
"validator_msg:keccak256:{}",
hex::encode(ByteFmt::encode(&self.0))
)
}
fn decode(text: Text) -> anyhow::Result<Self> {
text.strip("validator_msg:sha256:")?.decode_hex().map(Self)
text.strip("validator_msg:keccak256:")?
.decode_hex()
.map(Self)
}
}
12 changes: 6 additions & 6 deletions node/libs/roles/src/validator/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,23 @@ use utils::enum_util::Variant;
impl ProtoFmt for BlockHeaderHash {
type Proto = proto::BlockHeaderHash;
fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self(ByteFmt::decode(required(&r.sha256)?)?))
Ok(Self(ByteFmt::decode(required(&r.keccak256)?)?))
}
fn build(&self) -> Self::Proto {
Self::Proto {
sha256: Some(self.0.encode()),
keccak256: Some(self.0.encode()),
}
}
}

impl ProtoFmt for PayloadHash {
type Proto = proto::PayloadHash;
fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self(ByteFmt::decode(required(&r.sha256)?)?))
Ok(Self(ByteFmt::decode(required(&r.keccak256)?)?))
}
fn build(&self) -> Self::Proto {
Self::Proto {
sha256: Some(self.0.encode()),
keccak256: Some(self.0.encode()),
}
}
}
Expand Down Expand Up @@ -322,12 +322,12 @@ impl ProtoFmt for MsgHash {
type Proto = proto::MsgHash;

fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self(ByteFmt::decode(required(&r.sha256)?)?))
Ok(Self(ByteFmt::decode(required(&r.keccak256)?)?))
}

fn build(&self) -> Self::Proto {
Self::Proto {
sha256: Some(self.0.encode()),
keccak256: Some(self.0.encode()),
}
}
}
Expand Down
23 changes: 13 additions & 10 deletions node/libs/roles/src/validator/messages/block.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Messages related to blocks.

use super::CommitQC;
use crypto::{sha256, ByteFmt, Text, TextFmt};
use crypto::{keccak256, ByteFmt, Text, TextFmt};
use std::fmt;

/// Payload of the block. Consensus algorithm does not interpret the payload
Expand All @@ -13,14 +13,17 @@ pub struct Payload(pub Vec<u8>);

/// Hash of the Payload.
#[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct PayloadHash(pub(crate) sha256::Sha256);
pub struct PayloadHash(pub(crate) keccak256::Keccak256);

impl TextFmt for PayloadHash {
fn encode(&self) -> String {
format!("payload:sha256:{}", hex::encode(ByteFmt::encode(&self.0)))
format!(
"payload:keccak256:{}",
hex::encode(ByteFmt::encode(&self.0))
)
}
fn decode(text: Text) -> anyhow::Result<Self> {
text.strip("payload:sha256:")?.decode_hex().map(Self)
text.strip("payload:keccak256:")?.decode_hex().map(Self)
}
}

Expand All @@ -33,7 +36,7 @@ impl fmt::Debug for PayloadHash {
impl Payload {
/// Hash of the payload.
pub fn hash(&self) -> PayloadHash {
PayloadHash(sha256::Sha256::new(&self.0))
PayloadHash(keccak256::Keccak256::new(&self.0))
}
}

Expand Down Expand Up @@ -63,17 +66,17 @@ impl fmt::Display for BlockNumber {

/// Hash of the block.
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct BlockHeaderHash(pub(crate) sha256::Sha256);
pub struct BlockHeaderHash(pub(crate) keccak256::Keccak256);

impl TextFmt for BlockHeaderHash {
fn encode(&self) -> String {
format!(
"block_hash:sha256:{}",
"block_hash:keccak256:{}",
hex::encode(ByteFmt::encode(&self.0))
)
}
fn decode(text: Text) -> anyhow::Result<Self> {
text.strip("block_hash:sha256:")?.decode_hex().map(Self)
text.strip("block_hash:keccak256:")?.decode_hex().map(Self)
}
}

Expand All @@ -97,13 +100,13 @@ pub struct BlockHeader {
impl BlockHeader {
/// Returns the hash of the block.
pub fn hash(&self) -> BlockHeaderHash {
BlockHeaderHash(sha256::Sha256::new(&schema::canonical(self)))
BlockHeaderHash(keccak256::Keccak256::new(&schema::canonical(self)))
}

/// Creates a genesis block.
pub fn genesis(payload: PayloadHash) -> Self {
Self {
parent: BlockHeaderHash(sha256::Sha256::default()),
parent: BlockHeaderHash(keccak256::Keccak256::default()),
number: BlockNumber(0),
payload,
}
Expand Down
Loading
Loading