diff --git a/finalizer/src/actor.rs b/finalizer/src/actor.rs
index cd60276..2cf9d6f 100644
--- a/finalizer/src/actor.rs
+++ b/finalizer/src/actor.rs
@@ -32,7 +32,7 @@ use summit_syncer::Update;
use summit_types::account::{ValidatorAccount, ValidatorStatus};
use summit_types::checkpoint::Checkpoint;
use summit_types::consensus_state_query::{ConsensusStateRequest, ConsensusStateResponse};
-use summit_types::execution_request::{ExecutionRequest, WithdrawalRequest};
+use summit_types::execution_request::{DepositRequest, ExecutionRequest, WithdrawalRequest};
use summit_types::network_oracle::NetworkOracle;
use summit_types::scheme::EpochTransition;
use summit_types::utils::{
@@ -878,6 +878,7 @@ async fn execute_block<
epoch_num_of_blocks,
protocol_version_digest,
validator_withdrawal_num_epochs,
+ validator_minimum_stake,
)
.await;
@@ -897,7 +898,6 @@ async fn execute_block<
state,
epoch_num_of_blocks,
validator_onboarding_limit_per_block,
- validator_minimum_stake,
validator_num_warm_up_epochs,
validator_withdrawal_num_epochs,
)
@@ -966,79 +966,60 @@ async fn parse_execution_requests<
epoch_num_of_blocks: u64,
protocol_version_digest: Digest,
validator_withdrawal_num_epochs: u64,
+ validator_minimum_stake: u64,
) {
for request_bytes in &block.execution_requests {
match ExecutionRequest::try_from_eth_bytes(request_bytes.as_ref()) {
Ok(execution_request) => {
match execution_request {
ExecutionRequest::Deposit(deposit_request) => {
- let message = deposit_request.as_message(protocol_version_digest);
+ if verify_deposit_request(
+ context,
+ &deposit_request,
+ protocol_version_digest,
+ new_height,
+ validator_minimum_stake,
+ ) {
+ state.push_deposit(deposit_request);
+ } else {
+ // If the signatures fail, we create an immediate withdrawal request for the deposited amount.
+ // Since the signatures are invalid, the validator cannot be added to the committee.
+ // However, the deposited funds are still burned in the deposit contract, so we have to withdraw them.
+ let withdrawal_credentials = match parse_withdrawal_credentials(
+ deposit_request.withdrawal_credentials,
+ ) {
+ Ok(withdrawal_credentials) => withdrawal_credentials,
+ Err(e) => {
+ warn!("Failed to parse withdrawal credentials: {e}");
+ continue;
+ }
+ };
- let mut node_signature_bytes = &deposit_request.node_signature[..];
- let Ok(node_signature) = Signature::read(&mut node_signature_bytes) else {
- info!(
- "Failed to parse node signature from deposit request: {deposit_request:?}"
- );
- continue; // Skip this deposit request
- };
- if !deposit_request
- .node_pubkey
- .verify(&[], &message, &node_signature)
- {
- #[cfg(debug_assertions)]
- {
- let gauge: Gauge = Gauge::default();
- gauge.set(new_height as i64);
- context.register(
- format!(
- "{}_deposit_request_invalid_node_sig",
- hex::encode(&deposit_request.node_pubkey)
- ),
- "height",
- gauge,
- );
- }
- info!(
- "Failed to verify node signature from deposit request: {deposit_request:?}"
- );
- continue; // Skip this deposit request
- }
+ let validator_pubkey: [u8; 32] =
+ deposit_request.node_pubkey.as_ref().try_into().unwrap();
+ let withdrawal_request = WithdrawalRequest {
+ source_address: withdrawal_credentials,
+ validator_pubkey,
+ amount: deposit_request.amount,
+ };
+ let withdrawal_height = new_height
+ + validator_withdrawal_num_epochs * epoch_num_of_blocks
+ - 1;
- let mut consensus_signature_bytes =
- &deposit_request.consensus_signature[..];
- let Ok(consensus_signature) =
- bls12381::Signature::read(&mut consensus_signature_bytes)
- else {
- info!(
- "Failed to parse consensus signature from deposit request: {deposit_request:?}"
- );
- continue; // Skip this deposit request
- };
- if !deposit_request.consensus_pubkey.verify(
- &[],
- &message,
- &consensus_signature,
- ) {
- #[cfg(debug_assertions)]
+ // If an account exists, we have to temporary increase the `pending_withdrawal_amount`,
+ // otherwise this withdrawal request will decrement the actual account balance.
+
+ if let Some(account) =
+ state.validator_accounts.get_mut(&validator_pubkey)
{
- let gauge: Gauge = Gauge::default();
- gauge.set(new_height as i64);
- context.register(
- format!(
- "{}_deposit_request_invalid_consensus_sig",
- hex::encode(&deposit_request.consensus_pubkey)
- ),
- "height",
- gauge,
- );
+ account.pending_withdrawal_amount += deposit_request.amount;
}
- info!(
- "Failed to verify consensus signature from deposit request: {deposit_request:?}"
+
+ state.push_withdrawal_request(
+ withdrawal_request.clone(),
+ withdrawal_height,
);
- continue; // Skip this deposit request
}
-
- state.push_deposit(deposit_request);
}
ExecutionRequest::Withdrawal(mut withdrawal_request) => {
// Only add the withdrawal request if the validator exists and has sufficient balance
@@ -1142,7 +1123,6 @@ async fn process_execution_requests<
state: &mut ConsensusState,
epoch_num_of_blocks: u64,
validator_onboarding_limit_per_block: usize,
- validator_minimum_stake: u64,
validator_num_warm_up_epochs: u64,
validator_withdrawal_num_epochs: u64,
) {
@@ -1153,17 +1133,11 @@ async fn process_execution_requests<
// then the deposit will not be processed and a withdrawal is initiated immediately
let node_pubkey_bytes = request.node_pubkey.as_ref().try_into().unwrap();
- let maybe_account = state.validator_accounts.get_mut(&node_pubkey_bytes);
- if maybe_account.is_some() || request.amount != validator_minimum_stake {
- if request.amount != validator_minimum_stake {
- info!(
- "Received deposit request with amount != minimum stake, initiating immediate withdrawal: {request:?}"
- );
- } else {
- info!(
- "Received deposit request for an existing account, initiating immediate withdrawal: {request:?}"
- );
- }
+ if let Some(account) = state.validator_accounts.get_mut(&node_pubkey_bytes) {
+ // We already check that the amount equals the minimum stake when the request is parsed
+ info!(
+ "Received deposit request for an existing account, initiating immediate withdrawal: {request:?}"
+ );
let withdrawal_credentials =
match parse_withdrawal_credentials(request.withdrawal_credentials) {
Ok(withdrawal_credentials) => withdrawal_credentials,
@@ -1185,9 +1159,7 @@ async fn process_execution_requests<
// If an account exists, we have to temporary increase the `pending_withdrawal_amount`,
// otherwise this withdrawal request will decrement the actual account balance.
- if let Some(account) = maybe_account {
- account.pending_withdrawal_amount += request.amount;
- }
+ account.pending_withdrawal_amount += request.amount;
state.push_withdrawal_request(withdrawal_request.clone(), withdrawal_height);
continue;
@@ -1298,6 +1270,76 @@ async fn process_execution_requests<
}
}
+fn verify_deposit_request(
+ context: &ContextCell,
+ deposit_request: &DepositRequest,
+ protocol_version_digest: Digest,
+ new_height: u64,
+ validator_minimum_stake: u64,
+) -> bool {
+ if deposit_request.amount != validator_minimum_stake {
+ info!(
+ "Received deposit request with amount != minimum stake, initiating immediate withdrawal: {deposit_request:?}"
+ );
+ return false;
+ }
+
+ let message = deposit_request.as_message(protocol_version_digest);
+
+ let mut node_signature_bytes = &deposit_request.node_signature[..];
+ let Ok(node_signature) = Signature::read(&mut node_signature_bytes) else {
+ info!("Failed to parse node signature from deposit request: {deposit_request:?}");
+ return false;
+ };
+ if !deposit_request
+ .node_pubkey
+ .verify(&[], &message, &node_signature)
+ {
+ #[cfg(debug_assertions)]
+ {
+ let gauge: Gauge = Gauge::default();
+ gauge.set(new_height as i64);
+ context.register(
+ format!(
+ "{}_deposit_request_invalid_node_sig",
+ hex::encode(&deposit_request.node_pubkey)
+ ),
+ "height",
+ gauge,
+ );
+ }
+ info!("Failed to verify node signature from deposit request: {deposit_request:?}");
+ return false;
+ }
+
+ let mut consensus_signature_bytes = &deposit_request.consensus_signature[..];
+ let Ok(consensus_signature) = bls12381::Signature::read(&mut consensus_signature_bytes) else {
+ info!("Failed to parse consensus signature from deposit request: {deposit_request:?}");
+ return false;
+ };
+ if !deposit_request
+ .consensus_pubkey
+ .verify(&[], &message, &consensus_signature)
+ {
+ #[cfg(debug_assertions)]
+ {
+ let gauge: Gauge = Gauge::default();
+ gauge.set(new_height as i64);
+ context.register(
+ format!(
+ "{}_deposit_request_invalid_consensus_sig",
+ hex::encode(&deposit_request.consensus_pubkey)
+ ),
+ "height",
+ gauge,
+ );
+ }
+ info!("Failed to verify consensus signature from deposit request: {deposit_request:?}");
+ return false;
+ }
+ true
+}
+
impl<
R: Storage + Metrics + Clock + Spawner + governor::clock::Clock + Rng,
C: EngineClient,
diff --git a/node/src/tests/execution_requests.rs b/node/src/tests/execution_requests.rs
index af0784b..c78402c 100644
--- a/node/src/tests/execution_requests.rs
+++ b/node/src/tests/execution_requests.rs
@@ -1392,7 +1392,7 @@ fn test_deposit_and_withdrawal_request_multiple() {
#[test_traced("INFO")]
fn test_deposit_request_invalid_signature() {
// Adds a deposit request with an invalid signature to the block at height 5, and then
- // verifies that the request is rejected.
+ // verifies that the request is rejected and a withdrawal request is submitted for the same amount.
let n = 10;
let link = Link {
latency: Duration::from_millis(80),
@@ -1446,7 +1446,7 @@ fn test_deposit_request_invalid_signature() {
// Create a single deposit request using the helper
let (mut test_deposit, _, _) = common::create_deposit_request(
- 1,
+ n,
VALIDATOR_MINIMUM_STAKE,
common::get_domain(),
None,
@@ -1464,13 +1464,19 @@ fn test_deposit_request_invalid_signature() {
test_deposit.node_signature = test_deposit2.node_signature;
test_deposit.consensus_signature = test_deposit2.consensus_signature;
+ let validator_node_key = test_deposit.node_pubkey.clone();
+
// Convert to ExecutionRequest and then to Requests
let execution_requests = vec![ExecutionRequest::Deposit(test_deposit.clone())];
let requests = common::execution_requests_to_requests(execution_requests);
// Create execution requests map (add deposit to block 5)
let deposit_block_height = 5;
- let stop_height = deposit_block_height + BLOCKS_PER_EPOCH + 1;
+ let deposit_process_height =
+ utils::last_block_in_epoch(BLOCKS_PER_EPOCH, deposit_block_height / BLOCKS_PER_EPOCH);
+ let withdrawal_height =
+ deposit_process_height + VALIDATOR_WITHDRAWAL_NUM_EPOCHS * BLOCKS_PER_EPOCH;
+ let stop_height = withdrawal_height + 1;
let mut execution_requests_map = HashMap::new();
execution_requests_map.insert(deposit_block_height, requests);
@@ -1565,7 +1571,7 @@ fn test_deposit_request_invalid_signature() {
println!("{}: {} (failed to parse pubkey)", metric, value);
}
}
- if processed_requests.len() as u32 >= n && height_reached.len() as u32 >= n {
+ if processed_requests.len() as u64 >= n && height_reached.len() as u64 >= n {
success = true;
break;
}
@@ -1578,6 +1584,21 @@ fn test_deposit_request_invalid_signature() {
context.sleep(Duration::from_secs(1)).await;
}
+ let state_query = consensus_state_queries.get(&0).unwrap();
+ let balance = state_query.get_validator_balance(validator_node_key).await;
+ // Assert that no validator account was created
+ assert!(balance.is_none());
+
+ let withdrawals = engine_client_network.get_withdrawals();
+ assert_eq!(withdrawals.len(), 1);
+
+ let epoch_withdrawals = withdrawals.get(&withdrawal_height).unwrap();
+ assert_eq!(epoch_withdrawals[0].amount, test_deposit.amount);
+
+ let address =
+ utils::parse_withdrawal_credentials(test_deposit.withdrawal_credentials).unwrap();
+ assert_eq!(epoch_withdrawals[0].address, address);
+
// Check that all nodes have the same canonical chain
assert!(
engine_client_network