Skip to content

Commit

Permalink
checking incoming class hash in compute_hash_impl
Browse files Browse the repository at this point in the history
  • Loading branch information
@vbar
vbar committed Nov 6, 2024
1 parent ae7d62a commit 3705dec
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 18 deletions.
12 changes: 8 additions & 4 deletions crates/p2p/src/client/peer_agnostic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,25 +513,27 @@ impl BlockClient for Client {
Ok(ClassesResponse::Class(p2p_proto::class::Class::Cairo0 {
class,
domain: _,
class_hash: _,
class_hash,
})) => {
let definition = CairoDefinition::try_from_dto(class)
.map_err(|_| ClassDefinitionsError::CairoDefinitionError(peer))?;
class_definitions.push(ClassDefinition::Cairo {
block_number: block,
definition: definition.0,
hash: ClassHash(class_hash.0),
});
}
Ok(ClassesResponse::Class(p2p_proto::class::Class::Cairo1 {
class,
domain: _,
class_hash: _,
class_hash,
})) => {
let definition = SierraDefinition::try_from_dto(class)
.map_err(|_| ClassDefinitionsError::SierraDefinitionError(peer))?;
class_definitions.push(ClassDefinition::Sierra {
block_number: block,
sierra_definition: definition.0,
hash: SierraHash(class_hash.0),
});
}
Ok(ClassesResponse::Fin) => {
Expand Down Expand Up @@ -1277,7 +1279,7 @@ mod class_definition_stream {
Ok(ClassesResponse::Class(p2p_proto::class::Class::Cairo0 {
class,
domain: _,
class_hash: _,
class_hash,
})) => {
let Ok(CairoDefinition(definition)) = CairoDefinition::try_from_dto(class) else {
// TODO punish the peer
Expand All @@ -1288,12 +1290,13 @@ mod class_definition_stream {
Some(ClassDefinition::Cairo {
block_number,
definition,
hash: ClassHash(class_hash.0),
})
}
Ok(ClassesResponse::Class(p2p_proto::class::Class::Cairo1 {
class,
domain: _,
class_hash: _,
class_hash,
})) => {
let Ok(SierraDefinition(definition)) = SierraDefinition::try_from_dto(class) else {
// TODO punish the peer
Expand All @@ -1304,6 +1307,7 @@ mod class_definition_stream {
Some(ClassDefinition::Sierra {
block_number,
sierra_definition: definition,
hash: SierraHash(class_hash.0),
})
}
Ok(ClassesResponse::Fin) => {
Expand Down
17 changes: 13 additions & 4 deletions crates/p2p/src/client/peer_agnostic/fixtures.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use pathfinder_common::{
TransactionHash,
TransactionIndex,
};
use pathfinder_crypto::Felt;
use tagged::Tagged;
use tagged_debug_derive::TaggedDebug;
use tokio::sync::Mutex;
Expand Down Expand Up @@ -288,7 +287,7 @@ pub fn class_resp(tag: i32) -> ClassesResponse {
ClassDefinition::Cairo(c) => Class::Cairo0 {
class: c.to_dto(),
domain: 0,
class_hash: Hash(Felt::default()),
class_hash: Faker.fake(),
},
}
})
Expand All @@ -300,18 +299,28 @@ pub fn class_resp(tag: i32) -> ClassesResponse {
pub fn class(tag: i32, block_number: u64) -> ClassDefinition {
let block_number = BlockNumber::new_or_panic(block_number);
match class_resp(tag) {
ClassesResponse::Class(Class::Cairo0 { class, .. }) => {
ClassesResponse::Class(Class::Cairo0 {
class,
domain: _,
class_hash,
}) => {
Tagged::get(format!("class {tag}"), || ClassDefinition::Cairo {
block_number,
definition: CairoDefinition::try_from_dto(class).unwrap().0,
hash: ClassHash(class_hash.0),
})
.unwrap()
.data
}
ClassesResponse::Class(Class::Cairo1 { class, .. }) => {
ClassesResponse::Class(Class::Cairo1 {
class,
domain: _,
class_hash,
}) => {
Tagged::get(format!("class {tag}"), || ClassDefinition::Sierra {
block_number,
sierra_definition: SierraDefinition::try_from_dto(class).unwrap().0,
hash: SierraHash(class_hash.0),
})
.unwrap()
.data
Expand Down
4 changes: 4 additions & 0 deletions crates/p2p/src/client/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ use pathfinder_common::{
BlockNumber,
BlockTimestamp,
ClassCommitment,
ClassHash,
EventCommitment,
Fee,
GasPrice,
ReceiptCommitment,
SequencerAddress,
SierraHash,
SignedBlockHeader,
StateCommitment,
StateDiffCommitment,
Expand All @@ -35,10 +37,12 @@ pub enum ClassDefinition {
Cairo {
block_number: BlockNumber,
definition: Vec<u8>,
hash: ClassHash,
},
Sierra {
block_number: BlockNumber,
sierra_definition: Vec<u8>,
hash: SierraHash,
},
}

Expand Down
7 changes: 6 additions & 1 deletion crates/pathfinder/src/sync/checkpoint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1520,14 +1520,17 @@ mod tests {
Ok(PeerData::for_tests(ClassDefinition::Cairo {
block_number: BlockNumber::GENESIS + 1,
definition: CAIRO.to_vec(),
hash: cairo_hash,
})),
Ok(PeerData::for_tests(ClassDefinition::Sierra {
block_number: BlockNumber::GENESIS + 1,
sierra_definition: SIERRA0.to_vec(),
hash: sierra0_hash,
})),
Ok(PeerData::for_tests(ClassDefinition::Sierra {
block_number: BlockNumber::GENESIS + 1,
sierra_definition: SIERRA2.to_vec(),
hash: sierra2_hash,
})),
];

Expand Down Expand Up @@ -1618,11 +1621,13 @@ mod tests {
#[rstest::rstest]
#[case::cairo(ClassDefinition::Cairo {
block_number: BlockNumber::GENESIS + 1,
definition: Default::default()
definition: Default::default(),
hash: Default::default(),
})]
#[case::sierra(ClassDefinition::Sierra {
block_number: BlockNumber::GENESIS + 1,
sierra_definition: Default::default(),
hash: Default::default(),
})]
#[tokio::test]
async fn bad_layout(#[case] class: ClassDefinition) {
Expand Down
23 changes: 17 additions & 6 deletions crates/pathfinder/src/sync/class_definitions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub struct ClassWithLayout {
pub block_number: BlockNumber,
pub definition: ClassDefinition,
pub layout: GwClassDefinition<'static>,
pub hash: ClassHash,
}

#[derive(Debug)]
Expand Down Expand Up @@ -137,6 +138,7 @@ fn verify_layout_impl(def: P2PClassDefinition) -> anyhow::Result<ClassWithLayout
P2PClassDefinition::Cairo {
block_number,
definition,
hash,
} => {
let layout = GwClassDefinition::Cairo(
serde_json::from_slice::<Cairo<'_>>(&definition).inspect_err(
Expand All @@ -147,11 +149,13 @@ fn verify_layout_impl(def: P2PClassDefinition) -> anyhow::Result<ClassWithLayout
block_number,
definition: ClassDefinition::Cairo(definition),
layout,
hash,
})
}
P2PClassDefinition::Sierra {
block_number,
sierra_definition,
hash,
} => {
let layout = GwClassDefinition::Sierra(
serde_json::from_slice::<Sierra<'_>>(&sierra_definition).inspect_err(
Expand All @@ -162,6 +166,7 @@ fn verify_layout_impl(def: P2PClassDefinition) -> anyhow::Result<ClassWithLayout
block_number,
definition: ClassDefinition::Sierra(sierra_definition),
layout,
hash: ClassHash(hash.0),
})
}
}
Expand Down Expand Up @@ -204,9 +209,10 @@ fn compute_hash_impl(input: ClassWithLayout) -> anyhow::Result<Class> {
block_number,
definition,
layout,
hash,
} = input;

let hash = match layout {
let computed_hash = match layout {
GwClassDefinition::Cairo(c) => compute_cairo_class_hash(
c.abi.as_ref().get().as_bytes(),
c.program.as_ref().get().as_bytes(),
Expand All @@ -222,11 +228,16 @@ fn compute_hash_impl(input: ClassWithLayout) -> anyhow::Result<Class> {
),
}?;

Ok(Class {
block_number,
definition,
hash,
})
if computed_hash != hash {
tracing::debug!(input_hash=%hash, actual_hash=%computed_hash, "Class hash mismatch");
Err(SyncError2::BadClassHash.into())
} else {
Ok(Class {
block_number,
definition,
hash,
})
}
}

pub struct VerifyDeclaredAt {
Expand Down
6 changes: 6 additions & 0 deletions crates/pathfinder/src/sync/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ pub(super) enum SyncError {
StateRootMismatch(PeerId),
#[error("Transaction hash verification failed")]
BadTransactionHash(PeerId),
#[error("Class hash verification failed")]
BadClassHash(PeerId),
}

impl PartialEq for SyncError {
Expand Down Expand Up @@ -62,6 +64,7 @@ impl SyncError {
}
SyncError::StateRootMismatch(x) => PeerData::new(x, SyncError2::StateRootMismatch),
SyncError::BadTransactionHash(x) => PeerData::new(x, SyncError2::BadTransactionHash),
SyncError::BadClassHash(x) => PeerData::new(x, SyncError2::BadClassHash),
}
}

Expand All @@ -83,6 +86,7 @@ impl SyncError {
}
SyncError2::StateRootMismatch => SyncError::StateRootMismatch(peer),
SyncError2::BadTransactionHash => SyncError::BadTransactionHash(peer),
SyncError2::BadClassHash => SyncError::BadClassHash(peer),
other => SyncError::Other(other.into()),
}
}
Expand Down Expand Up @@ -142,6 +146,8 @@ pub(super) enum SyncError2 {
StateRootMismatch,
#[error("Transaction hash verification failed")]
BadTransactionHash,
#[error("Class hash verification failed")]
BadClassHash,
}

impl PartialEq for SyncError2 {
Expand Down
7 changes: 4 additions & 3 deletions crates/pathfinder/src/sync/track.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use p2p::client::types::{
TransactionData,
};
use p2p::PeerData;
use pathfinder_common::class_definition::ClassDefinition;
use pathfinder_common::event::Event;
use pathfinder_common::receipt::Receipt;
use pathfinder_common::state_update::{DeclaredClasses, StateUpdateData};
Expand Down Expand Up @@ -1087,16 +1086,18 @@ mod tests {
let defs = b
.cairo_defs
.iter()
.map(|(_, x)| ClassDefinition::Cairo {
.map(|(h, x)| ClassDefinition::Cairo {
block_number: block,
definition: x.clone(),
hash: *h,
})
.chain(
b.sierra_defs
.iter()
.map(|(_, x, _)| ClassDefinition::Sierra {
.map(|(h, x, _)| ClassDefinition::Sierra {
block_number: block,
sierra_definition: x.clone(),
hash: *h,
}),
)
.collect::<Vec<ClassDefinition>>();
Expand Down

0 comments on commit 3705dec

Please sign in to comment.