Skip to content

chore: create withdrawal request if sig verify for deposit fails #87

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.

Sign up for GitHub

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

Merged
matthias-wright merged 2 commits into from
Jan 7, 2026
Merged

chore: create withdrawal request if sig verify for deposit fails #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e40c265
chore: create immediate withdrawal request if sig verify for deposits…
matthias-wright Jan 6, 2026
06bba92
chore: check deposit request amount when parsing + update tests
matthias-wright Jan 7, 2026
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
198 changes: 120 additions & 78 deletions finalizer/src/actor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -878,6 +878,7 @@ async fn execute_block<
epoch_num_of_blocks,
protocol_version_digest,
validator_withdrawal_num_epochs,
validator_minimum_stake,
)
.await;

Expand All @@ -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,
)
Expand Down Expand Up @@ -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!(
"<pubkey>{}</pubkey>_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!(
"<pubkey>{}</pubkey>_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
Expand Down Expand Up @@ -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,
) {
Expand All @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -1298,6 +1270,76 @@ async fn process_execution_requests<
}
}

fn verify_deposit_request<R: Storage + Metrics + Clock + Spawner + governor::clock::Clock + Rng>(
context: &ContextCell<R>,
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!(
"<pubkey>{}</pubkey>_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!(
"<pubkey>{}</pubkey>_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,
Expand Down
29 changes: 25 additions & 4 deletions node/src/tests/execution_requests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand All @@ -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);

Expand Down Expand Up @@ -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;
}
Expand All @@ -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
Expand Down