Merge pull request #29 from gattaca-com/sanity_check_fix

fix sanity check for withdrawals root

crates/api/src/builder/api.rs

@@ -1,8 +1,5 @@
 use std::{
- collections::HashMap,
- io::Read,
- sync::Arc,
- time::{Duration, SystemTime, UNIX_EPOCH},
+ collections::HashMap, io::Read, ops::Deref, sync::Arc, time::{Duration, SystemTime, UNIX_EPOCH}
 };
 
 use axum::{
@@ -1922,23 +1919,42 @@ fn sanity_check_block_submission(
  });
  }
 
- if has_reached_fork(payload.slot(), CAPELLA_FORK_EPOCH) && payload.is_full_payload() {
- let payload_withdrawals = match payload.withdrawals() {
- Some(w) => w,
- None => return Err(BuilderApiError::MissingWithdrawls),
- };
+ if has_reached_fork(payload.slot(), CAPELLA_FORK_EPOCH) {
 
- let withdrawals_root = calculate_withdrawals_root(payload_withdrawals);
- let expected_withdrawals_root = match payload_attributes.withdrawals_root {
- Some(wr) => wr,
- None => return Err(BuilderApiError::MissingWithdrawls),
- };
+ if payload.is_full_payload() {
+  let withdrawals_root = match payload.withdrawals_root() {
+  Some(w) => w,
+  None => return Err(BuilderApiError::MissingWithdrawls),
+  };
 
- if withdrawals_root != expected_withdrawals_root {
- return Err(BuilderApiError::WithdrawalsRootMismatch {
- got: withdrawals_root,
- expected: expected_withdrawals_root,
- });
+ let expected_withdrawals_root = match payload_attributes.withdrawals_root {
+ Some(wr) => wr,
+ None => return Err(BuilderApiError::MissingWithdrawls),
+ };
+
+ if withdrawals_root != expected_withdrawals_root {
+ return Err(BuilderApiError::WithdrawalsRootMismatch {
+ got: Hash32::try_from(withdrawals_root.as_ref()).unwrap(),
+ expected: Hash32::try_from(expected_withdrawals_root.as_ref()).unwrap(),
+ });
+ }
+ } else {
+ let expected_withdrawals_root = match payload_attributes.withdrawals_root {
+ Some(wr) => wr,
+ None => return Err(BuilderApiError::MissingWithdrawlsRoot),
+ };
+
+ let payload_withdrawals_root = match payload.withdrawals_root() {
+ Some(wr) => wr,
+ None => return Err(BuilderApiError::MissingWithdrawlsRoot),
+ };
+
+ if payload_withdrawals_root != expected_withdrawals_root {
+ return Err(BuilderApiError::WithdrawalsRootMismatch {
+ got: Hash32::try_from(payload_withdrawals_root.as_ref()).unwrap(),
+ expected: Hash32::try_from(expected_withdrawals_root.as_ref()).unwrap(),
+ });
+ }
  }
  }
 

crates/api/src/builder/error.rs

@@ -74,8 +74,14 @@ pub enum BuilderApiError {
  #[error("missing withdrawls")]
  MissingWithdrawls,
 
+ #[error("invalid withdrawls root")]
+ InvalidWithdrawlsRoot,
+
+ #[error("missing withdrawls root")]
+ MissingWithdrawlsRoot,
+
  #[error("withdrawls root mismatch. got: {got:?}, expected: {expected:?}")]
- WithdrawalsRootMismatch { got: [u8; 32], expected: [u8; 32] },
+ WithdrawalsRootMismatch { got: Hash32, expected: Hash32 },
 
  #[error("signature verification failed")]
  SignatureVerificationFailed,
@@ -219,6 +225,12 @@ impl IntoResponse for BuilderApiError {
  BuilderApiError::MissingWithdrawls => {
  (StatusCode::BAD_REQUEST, "missing withdrawals").into_response()
  },
+ BuilderApiError::InvalidWithdrawlsRoot => {
+ (StatusCode::BAD_REQUEST, "invalid withdrawals root").into_response()
+ },
+ BuilderApiError::MissingWithdrawlsRoot => {
+ (StatusCode::BAD_REQUEST, "missing withdrawals root").into_response()
+ },
  BuilderApiError::WithdrawalsRootMismatch { got, expected } => {
  (StatusCode::BAD_REQUEST, format!("Withdrawals root mismatch. got: {got:?}, expected: {expected:?}")).into_response()
  },

crates/api/src/builder/tests.rs

@@ -15,13 +15,7 @@ mod tests {
  use tokio_tungstenite::{connect_async, tungstenite::{self, Message}};
  use core::panic;
  use ethereum_consensus::{
- builder::{SignedValidatorRegistration, ValidatorRegistration},
- configs::mainnet::CAPELLA_FORK_EPOCH,
- phase0::mainnet::SLOTS_PER_EPOCH,
- primitives::{BlsPublicKey, BlsSignature},
- ssz::{self, prelude::*},
- types::mainnet::{ExecutionPayload, ExecutionPayloadHeader},
- Fork,
+ builder::{SignedValidatorRegistration, ValidatorRegistration}, configs::mainnet::CAPELLA_FORK_EPOCH, deneb::{Transaction, Withdrawal}, phase0::mainnet::SLOTS_PER_EPOCH, primitives::{BlsPublicKey, BlsSignature}, ssz::{self, prelude::*}, types::mainnet::{ExecutionPayload, ExecutionPayloadHeader}, Fork
  };
  use futures::{stream::FuturesOrdered, Future, SinkExt, StreamExt};
  use helix_beacon_client::types::PayloadAttributes;
@@ -39,14 +33,14 @@ mod tests {
  use helix_database::MockDatabaseService;
  use helix_datastore::MockAuctioneer;
  use helix_housekeeper::{ChainUpdate, PayloadAttributesUpdate, SlotUpdate};
- use helix_utils::request_encoding::Encoding;
+ use helix_utils::{calculate_withdrawals_root, request_encoding::Encoding};
  use rand::Rng;
  use reqwest::{Client, Response};
  use reth_primitives::hex;
  use serde_json::json;
  use serial_test::serial;
  use std::{
- convert::Infallible, future::pending, io::Write, net::IpAddr, pin::Pin, str::FromStr, sync::Arc, time::Duration
+ convert::Infallible, future::pending, io::Write, net::IpAddr, ops::Deref, pin::Pin, str::FromStr, sync::Arc, time::Duration
  };
  use tokio::sync::{
  mpsc::{Receiver, Sender},
@@ -172,9 +166,7 @@ mod tests {
  parent_hash: get_byte_vector_32_for_hex(
  "0xbd3291854dc822b7ec585925cda0e18f06af28fa2886e15f52d52dd4b6f94ed6",
  ),
- withdrawals_root: Some(hex_to_byte_arr_32(
- "0xb15ed76298ff84a586b1d875df08b6676c98dfe9c7cd73fab88450348d8e70c8",
- )),
+ withdrawals_root: None,
  payload_attributes: get_dummy_payload_attributes(),
  }
  }
@@ -1377,4 +1369,42 @@ mod tests {
 
  }
 
+ #[tokio::test]
+ async fn test_calculate_withdrawals_root() {
+
+ let json_str = r#"
+ [
+ {"index": "53516667", "validator_index": "226593", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18829431"},
+ {"index": "53516668", "validator_index": "226594", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18895995"},
+ {"index": "53516669", "validator_index": "226595", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18921948"},
+ {"index": "53516670", "validator_index": "226596", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18879996"},
+ {"index": "53516671", "validator_index": "226597", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18862058"},
+ {"index": "53516672", "validator_index": "226598", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18877682"},
+ {"index": "53516673", "validator_index": "226599", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18876362"},
+ {"index": "53516674", "validator_index": "226600", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18905370"},
+ {"index": "53516675", "validator_index": "226601", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18911572"},
+ {"index": "53516676", "validator_index": "226602", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18908681"},
+ {"index": "53516677", "validator_index": "226603", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18904531"},
+ {"index": "53516678", "validator_index": "226604", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18829228"},
+ {"index": "53516679", "validator_index": "226605", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18883181"},
+ {"index": "53516680", "validator_index": "226606", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "63784192"},
+ {"index": "53516681", "validator_index": "226607", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18875686"},
+ {"index": "53516682", "validator_index": "226608", "address": "0xb9d7934878b5fb9610b3fe8a5e441e8fad7e293f", "amount": "18924753"}
+ ]
+ "#;
+
+ let withdrawals: Vec<Withdrawal> = serde_json::from_str(json_str).unwrap();
+
+ let withdrawals_root = calculate_withdrawals_root(&withdrawals);
+
+ // calculate_withdrawals_root use MPT to calculate the root
+ assert_eq!(hex::encode(withdrawals_root), "25068b16d9a849006edff1fbe9bf96799ef524f0ba87199559d1f714719a8202");
+
+ let mut wlist : List<Withdrawal, 16> = withdrawals.try_into().unwrap();
+ let root = wlist.hash_tree_root().unwrap();
+
+ // hash_tree_root use SSZ to calculate the root
+ assert_eq!(hex::encode(root.deref()),"c4726ded906a1d6775eec5e83fa867cffb9f77c6da58b3ceb2e412df971b07f1");
+ }
+
 }

crates/common/src/bid_submission/mod.rs

@@ -58,6 +58,8 @@ pub trait BidSubmission {
 
  fn withdrawals(&self) -> Option<&[Withdrawal]>;
 
+ fn withdrawals_root(&self) -> Option<Node>;
+
  fn consensus_version(&self) -> Fork;
 
  /// True if full submission payload, false if not (e.g. Optimistic V2)

crates/common/src/bid_submission/submission.rs

@@ -183,6 +183,26 @@ impl BidSubmission for SignedBidSubmission {
  }
  }
 
+ fn withdrawals_root(&self) -> Option<Node> {
+ match &self.execution_payload() {
+ ExecutionPayload::Bellatrix(_) => None,
+ ExecutionPayload::Capella(payload) => {
+ let mut withdrawals = payload.withdrawals.clone();
+ match withdrawals.hash_tree_root() {
+ Ok(root) => Some(root),
+ Err(_) => None,
+ }
+ },
+ ExecutionPayload::Deneb(payload) => {
+ let mut withdrawals = payload.withdrawals.clone();
+ match withdrawals.hash_tree_root() {
+ Ok(root) => Some(root),
+ Err(_) => None,
+ }
+ },
+ }
+ }
+
  fn consensus_version(&self) -> Fork {
  match self {
  SignedBidSubmission::Deneb(_) => Fork::Deneb,

crates/common/src/bid_submission/v2/header_submission.rs

@@ -266,6 +266,17 @@ impl BidSubmission for SignedHeaderSubmission {
  None
  }
 
+ fn withdrawals_root(&self) -> Option<Node> {
+ match self {
+ Self::Capella(signed_header_submission) => {
+ Some(signed_header_submission.message.execution_payload_header.withdrawals_root)
+ }
+ Self::Deneb(signed_header_submission) => {
+ Some(signed_header_submission.message.execution_payload_header().withdrawals_root)
+ }
+ }
+ }
+
  fn consensus_version(&self) -> Fork {
  match self {
  Self::Capella(_) => Fork::Capella,

crates/housekeeper/src/chain_event_updater.rs

@@ -5,14 +5,13 @@ use std::{
 use std::collections::HashMap;
 
 use ethereum_consensus::{
- configs::{goerli::CAPELLA_FORK_EPOCH, mainnet::SECONDS_PER_SLOT},
- primitives::Bytes32,
+ configs::{goerli::CAPELLA_FORK_EPOCH, mainnet::SECONDS_PER_SLOT}, deneb::Withdrawal, primitives::Bytes32
 };
 use tokio::sync::{broadcast, mpsc};
 use tracing::{error, info, warn};
 
 use helix_beacon_client::types::{HeadEventData, PayloadAttributes, PayloadAttributesEvent};
-use helix_common::{api::builder_api::BuilderGetValidatorsResponseEntry, chain_info::ChainInfo};
+use helix_common::{api::builder_api::BuilderGetValidatorsResponseEntry, bellatrix::{List, Merkleized, Node}, chain_info::ChainInfo};
 use helix_database::DatabaseService;
 use helix_utils::{calculate_withdrawals_root, get_payload_attributes_key, has_reached_fork};
 
@@ -24,7 +23,7 @@ const MAX_DISTANCE_FOR_FUTURE_SLOT: u64 = 60;
 pub struct PayloadAttributesUpdate {
  pub slot: u64,
  pub parent_hash: Bytes32,
- pub withdrawals_root: Option<[u8; 32]>,
+ pub withdrawals_root: Option<Node>,
  pub payload_attributes: PayloadAttributes,
 }
 
@@ -203,8 +202,8 @@ impl<D: DatabaseService> ChainEventUpdater<D> {
 
  let mut withdrawals_root = None;
  if has_reached_fork(event.data.proposal_slot, CAPELLA_FORK_EPOCH) {
- withdrawals_root =
-  Some(calculate_withdrawals_root(&event.data.payload_attributes.withdrawals));
+ let mut withdrawals_list : List<Withdrawal, 16> = event.data.payload_attributes.withdrawals.clone().try_into().unwrap();
+ withdrawals_root = withdrawals_list.hash_tree_root().ok();
  }
 
  let update = ChainUpdate::PayloadAttributesUpdate(PayloadAttributesUpdate {