fix: wrap revert reason with a keccak (#244)

Cargo.toml

@@ -21,6 +21,7 @@ once_cell = "1.17.1"
 primitive-types = { version = "0.12.1", features = ["serde"] }
 serde = { version = "1.0.130", features = ["derive", "rc"] }
 serde_json = "1.0.81"
+sha3 = "0.10.8"
 starknet-crypto = "0.5.1"
 starknet-types-core = { version = "0.0.11", features = ["hash"] }
 strum = "0.24.1"

src/block_hash/receipt_commitment.rs

@@ -1,11 +1,10 @@
 use crate::core::ReceiptCommitment;
 use crate::crypto::patricia_hash::calculate_root;
 use crate::crypto::utils::HashChain;
-use crate::hash::{HashFunction, StarkFelt};
+use crate::hash::{starknet_keccak_hash, HashFunction, StarkFelt};
 use crate::transaction::{
     ExecutionResources, MessageToL1, TransactionExecutionStatus, TransactionReceipt,
 };
-use crate::transaction_hash::ascii_as_felt;
 
 #[cfg(test)]
 #[path = "receipt_commitment_test.rs"]
@@ -33,7 +32,7 @@ fn calculate_receipt_hash(transaction_receipt: &TransactionReceipt) -> StarkFelt
         .chain(&transaction_receipt.transaction_hash)
         .chain(&transaction_receipt.output.actual_fee().0.into())
         .chain(&calculate_messages_sent_hash(transaction_receipt.output.messages_sent()))
-        .chain(&get_revert_reason(transaction_receipt.output.execution_status()));
+        .chain(&get_revert_reason_hash(transaction_receipt.output.execution_status()));
     chain_execution_resources(hash_chain, transaction_receipt.output.execution_resources())
         .get_poseidon_hash()
 }
@@ -55,12 +54,11 @@ fn calculate_messages_sent_hash(messages_sent: &Vec<MessageToL1>) -> StarkFelt {
 }
 
 // Returns starknet-keccak of the revert reason ASCII string, or 0 if the transaction succeeded.
-fn get_revert_reason(execution_status: &TransactionExecutionStatus) -> StarkFelt {
+fn get_revert_reason_hash(execution_status: &TransactionExecutionStatus) -> StarkFelt {
     match execution_status {
         TransactionExecutionStatus::Succeeded => StarkFelt::ZERO,
-        // TODO(yoav): Wrap the ASCII with a keccak.
         TransactionExecutionStatus::Reverted(reason) => {
-            ascii_as_felt(&reason.revert_reason).expect("ascii_as_felt failed for revert reason")
+            starknet_keccak_hash(reason.revert_reason.as_bytes())
         }
     }
 }

src/block_hash/receipt_commitment_test.rs

@@ -5,7 +5,7 @@ use primitive_types::H160;
 use super::calculate_messages_sent_hash;
 use crate::block::{BlockHash, BlockNumber};
 use crate::block_hash::receipt_commitment::{
-    calculate_receipt_commitment, calculate_receipt_hash, get_revert_reason,
+    calculate_receipt_commitment, calculate_receipt_hash, get_revert_reason_hash,
 };
 use crate::core::{ContractAddress, EthAddress, ReceiptCommitment};
 use crate::hash::{PoseidonHashCalculator, StarkFelt};
@@ -43,12 +43,12 @@ fn test_receipt_hash_regression() {
     };
 
     let expected_hash =
-        StarkFelt::try_from("0x0144fc581761cdaa8327d6d32d4120cddda36be0fda68e463efe0d49f141ccc7")
+        StarkFelt::try_from("0x06720e8f1cd4543ae25714f0c79e592d98b16747a92962406ab08b6d46e10fd2")
             .unwrap();
     assert_eq!(calculate_receipt_hash(&transaction_receipt), expected_hash);
 
     let expected_root = ReceiptCommitment(
-        StarkFelt::try_from("0x03db8b2c479130ff1f1d718c593ee164849f89686e1a4c82e8a6abbf506852e8")
+        StarkFelt::try_from("0x035481a28b3ea40ddfbc80f3eabc924c9c5edf64f1dd7467d80c5c5290cf48ad")
             .unwrap(),
     );
     assert_eq!(
@@ -76,12 +76,15 @@ fn generate_message_to_l1(seed: u64) -> MessageToL1 {
 }
 
 #[test]
-fn test_revert_reason_regression() {
+fn test_revert_reason_hash_regression() {
     let execution_succeeded = TransactionExecutionStatus::Succeeded;
-    assert_eq!(get_revert_reason(&execution_succeeded), StarkFelt::ZERO);
+    assert_eq!(get_revert_reason_hash(&execution_succeeded), StarkFelt::ZERO);
     let execution_reverted =
         TransactionExecutionStatus::Reverted(RevertedTransactionExecutionStatus {
             revert_reason: "ABC".to_string(),
         });
-    assert_eq!(get_revert_reason(&execution_reverted), StarkFelt::from(0x414243_u32));
+    let expected_hash =
+        StarkFelt::try_from("0x01629b9dda060bb30c7908346f6af189c16773fa148d3366701fbaa35d54f3c8")
+            .unwrap();
+    assert_eq!(get_revert_reason_hash(&execution_reverted), expected_hash);
 }

src/hash.rs

@@ -6,6 +6,7 @@ use std::fmt::{Debug, Display};
 use std::io::Error;
 
 use serde::{Deserialize, Serialize};
+use sha3::{Digest, Keccak256};
 use starknet_crypto::FieldElement;
 use starknet_types_core::felt::Felt;
 use starknet_types_core::hash::{Pedersen, Poseidon, StarkHash as StarknetTypesStarkHash};
@@ -74,6 +75,15 @@ pub trait HashFunction {
     fn hash_array(stark_felts: &[StarkFelt]) -> StarkHash;
 }
 
+/// Computes the first 250 bits of the Keccak256 hash, in order to fit into a field element.
+pub fn starknet_keccak_hash(input: &[u8]) -> StarkFelt {
+    let mut keccak = Keccak256::default();
+    keccak.update(input);
+    let mut hashed_bytes: [u8; 32] = keccak.finalize().into();
+    hashed_bytes[0] &= 0b00000011_u8; // Discard the six MSBs.
+    StarkFelt::new_unchecked(hashed_bytes)
+}
+
 pub struct PoseidonHashCalculator;
 
 impl HashFunction for PoseidonHashCalculator {

src/transaction.rs

@@ -637,7 +637,8 @@ pub enum TransactionExecutionStatus {
 /// A reverted transaction execution status.
 #[derive(Debug, Clone, Eq, PartialEq, Hash, Deserialize, Serialize, PartialOrd, Ord)]
 pub struct RevertedTransactionExecutionStatus {
-    pub revert_reason: String, // Validate it's an ASCII string
+    // TODO: Validate it's an ASCII string.
+    pub revert_reason: String,
 }
 
 /// A fee.