diff --git a/docs/staking-and-participating.md b/docs/staking-and-participating.md index 2cff80c..3e6f31c 100644 --- a/docs/staking-and-participating.md +++ b/docs/staking-and-participating.md @@ -32,3 +32,12 @@ As defined in EIP-7002, the calldata for this transaction is 56 bytes Note that the validator pubkey is the ED25519 key (left-padded with zeros), and not the BLS key. When depositing funds into the staking contract (see above), an Ethereum address was specified (withdrawal_credentials). A valid withdrawal transaction has to be signed by the private key associated with this Ethereum address. + +## Invariants +- A validator will join the committee `VALIDATOR_NUM_WARM_UP_EPOCHS` epochs after submitting a valid deposit request. The phase after submitting the deposit request, and before joining the committee is called the `onboarding phase`. +- If a withdrawal request is submitted in epoch `n`, then the validator will be removed from the committee at the end of epoch `n`. The withdrawal will be processed in epoch `n + VALIDATOR_WITHDRAWAL_NUM_EPOCHS`. +- A validator can only submit one withdrawal request at a time. If another withdrawal request is submitted, while a withdrawal request is pending, then the second withdrawal request will be ignored. +- If a withdrawal request is submitted while a validator is in the onboarding phase, then the onboarding phase is aborted, and the withdrawal request will be processed `VALIDATOR_WITHDRAWAL_NUM_EPOCHS` epochs later. +- No partial withdrawals. If the validator balance is `balance`, and a withdrawal request with amount `amount < balance` is submitted, then the withdrawal request will be processed for the amount of `balance`. +- A validator can only have a balance of `VALIDATOR_MINIMUM_STAKE`. If a deposit request with `amount` is submitted, where `amount != VALIDATOR_MINIMUM_STAKE`, then the deposit request will be skipped, and a withdrawal request will be initiated immediately. +- No top up deposits. If a validator already has a balance of `VALIDATOR_MINIMUM_STAKE`, then it cannot submit another deposit request with amount `VALIDATOR_MINIMUM_STAKE`. \ No newline at end of file diff --git a/finalizer/src/actor.rs b/finalizer/src/actor.rs index bc9bd4f..ab259d1 100644 --- a/finalizer/src/actor.rs +++ b/finalizer/src/actor.rs @@ -2,7 +2,6 @@ use crate::archive::backup_with_enclave; use crate::db::{Config as StateConfig, FinalizerState}; use crate::{FinalizerConfig, FinalizerMailbox, FinalizerMessage}; use alloy_eips::eip4895::Withdrawal; -use alloy_primitives::Address; use alloy_rpc_types_engine::ForkchoiceState; #[allow(unused)] use commonware_codec::{DecodeExt as _, ReadExt as _}; @@ -33,10 +32,12 @@ 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; +use summit_types::execution_request::{ExecutionRequest, WithdrawalRequest}; use summit_types::network_oracle::NetworkOracle; use summit_types::scheme::EpochTransition; -use summit_types::utils::{is_last_block_of_epoch, is_penultimate_block_of_epoch}; +use summit_types::utils::{ + is_last_block_of_epoch, is_penultimate_block_of_epoch, parse_withdrawal_credentials, +}; use summit_types::{Block, BlockAuxData, Digest, FinalizedHeader, PublicKey, Signature}; use summit_types::{EngineClient, consensus_state::ConsensusState}; use tokio_util::sync::CancellationToken; @@ -78,9 +79,10 @@ pub struct Finalizer< validator_max_withdrawals_per_block: usize, epoch_num_of_blocks: u64, protocol_version_digest: Digest, - validator_minimum_stake: u64, // in gwei - validator_withdrawal_period: u64, // in blocks + validator_minimum_stake: u64, // in gwei + validator_withdrawal_num_epochs: u64, // in epochs validator_onboarding_limit_per_block: usize, + validator_num_warm_up_epochs: u64, oracle: O, orchestrator_mailbox: summit_orchestrator::Mailbox, node_public_key: PublicKey, @@ -155,8 +157,9 @@ impl< genesis_hash: cfg.genesis_hash, protocol_version_digest: Sha256::hash(&cfg.protocol_version.to_le_bytes()), validator_minimum_stake: cfg.validator_minimum_stake, - validator_withdrawal_period: cfg.validator_withdrawal_period, + validator_withdrawal_num_epochs: cfg.validator_withdrawal_num_epochs, validator_onboarding_limit_per_block: cfg.validator_onboarding_limit_per_block, + validator_num_warm_up_epochs: cfg.validator_num_warm_up_epochs, node_public_key: cfg.node_public_key, validator_exit: false, cancellation_token: cfg.cancellation_token, @@ -314,8 +317,9 @@ impl< self.validator_max_withdrawals_per_block, self.protocol_version_digest, self.validator_minimum_stake, - self.validator_withdrawal_period, self.validator_onboarding_limit_per_block, + self.validator_num_warm_up_epochs, + self.validator_withdrawal_num_epochs, ) .await; } @@ -409,21 +413,28 @@ impl< } // Add and remove validators for the next epoch - if !self.canonical_state.added_validators.is_empty() + let next_epoch = self.canonical_state.epoch + 1; + if self + .canonical_state + .added_validators + .contains_key(&next_epoch) || !self.canonical_state.removed_validators.is_empty() { - // TODO(matthias): we can probably find a way to do this without iterating over the joining validators - // Activate validators that staked this epoch. - for key in self.canonical_state.added_validators.iter() { - let key_bytes: [u8; 32] = key.as_ref().try_into().unwrap(); - let account = self - .canonical_state - .validator_accounts - .get_mut(&key_bytes) - .expect( - "only validators with accounts are added to the added_validators queue", - ); - account.status = ValidatorStatus::Active; + // Activate validators for the coming epoch. + if let Some(added_validators) = + self.canonical_state.added_validators.get(&next_epoch) + { + for key in added_validators { + let key_bytes: [u8; 32] = key.as_ref().try_into().unwrap(); + let account = self + .canonical_state + .validator_accounts + .get_mut(&key_bytes) + .expect( + "only validators with accounts are added to the added_validators queue", + ); + account.status = ValidatorStatus::Active; + } } for key in self.canonical_state.removed_validators.iter() { @@ -450,13 +461,12 @@ impl< if self.archive_mode { // Should always be there - if let Some(checkpoint) = &self.canonical_state.pending_checkpoint { - if let Err(e) = + if let Some(checkpoint) = &self.canonical_state.pending_checkpoint + && let Err(e) = backup_with_enclave(self.canonical_state.epoch, checkpoint.clone()) - { - // This shouldnt be critical but it should be logged - error!("Unable to backup with enclave: {}", e); - } + { + // This shouldn't be critical but it should be logged + error!("Unable to backup with enclave: {}", e); } } @@ -489,8 +499,9 @@ impl< histogram!("database_operations_duration_millis").record(db_operations_duration); } - // Create the list of validators for the new epoch - let active_validators = self.canonical_state.get_active_validators(); + // Create the list of validators for the p2p network for the next epoch. + // We also include the validators that already staked and are waiting to join the committee. + let active_validators = self.canonical_state.get_active_or_joining_validators(); let network_keys = active_validators .iter() .map(|(node_key, _)| node_key.clone()) @@ -511,9 +522,7 @@ impl< epoch_change = true; // Only clear the added and removed validators after saving the state to disk - if !self.canonical_state.added_validators.is_empty() { - self.canonical_state.added_validators.clear(); - } + self.canonical_state.added_validators.remove(&next_epoch); if !self.canonical_state.removed_validators.is_empty() { self.canonical_state.removed_validators.clear(); } @@ -614,8 +623,9 @@ impl< self.validator_max_withdrawals_per_block, self.protocol_version_digest, self.validator_minimum_stake, - self.validator_withdrawal_period, self.validator_onboarding_limit_per_block, + self.validator_num_warm_up_epochs, + self.validator_withdrawal_num_epochs, ) .await; @@ -710,12 +720,18 @@ impl< // Only submit withdrawals at the end of an epoch let ready_withdrawals = state.get_next_ready_withdrawals(height, self.validator_max_withdrawals_per_block); + let next_epoch = state.epoch; BlockAuxData { epoch: state.epoch, withdrawals: ready_withdrawals, checkpoint_hash: Some(checkpoint_hash), header_hash: prev_header_hash, - added_validators: state.added_validators.clone(), + // The block proposer needs the validators that will be added in the next epoch + added_validators: state + .added_validators + .get(&next_epoch) + .cloned() + .unwrap_or_default(), removed_validators: state.removed_validators.clone(), forkchoice: state.forkchoice, } @@ -762,6 +778,17 @@ impl< .map(|account| account.balance); let _ = sender.send(ConsensusStateResponse::ValidatorBalance(balance)); } + ConsensusStateRequest::GetValidatorAccount(public_key) => { + let mut key_bytes = [0u8; 32]; + key_bytes.copy_from_slice(&public_key); + + let account = self + .canonical_state + .validator_accounts + .get(&key_bytes) + .cloned(); + let _ = sender.send(ConsensusStateResponse::ValidatorAccount(account)); + } } } } @@ -792,8 +819,9 @@ async fn execute_block< validator_max_withdrawals_per_block: usize, protocol_version_digest: Digest, validator_minimum_stake: u64, - validator_withdrawal_period: u64, validator_onboarding_limit_per_block: usize, + validator_num_warm_up_epochs: u64, + validator_withdrawal_num_epochs: u64, ) { #[cfg(feature = "prom")] let block_processing_start = Instant::now(); @@ -843,9 +871,9 @@ async fn execute_block< block, new_height, state, + epoch_num_of_blocks, protocol_version_digest, - validator_minimum_stake, - validator_withdrawal_period, + validator_withdrawal_num_epochs, ) .await; @@ -866,6 +894,8 @@ async fn execute_block< epoch_num_of_blocks, validator_onboarding_limit_per_block, validator_minimum_stake, + validator_num_warm_up_epochs, + validator_withdrawal_num_epochs, ) .await; #[cfg(feature = "prom")] @@ -919,6 +949,7 @@ async fn execute_block< } } +#[allow(clippy::too_many_arguments)] async fn parse_execution_requests< S: Signer, V: Variant, @@ -928,9 +959,9 @@ async fn parse_execution_requests< block: &Block, new_height: u64, state: &mut ConsensusState, + epoch_num_of_blocks: u64, protocol_version_digest: Digest, - validator_minimum_stake: u64, - validator_withdrawal_period: u64, + validator_withdrawal_num_epochs: u64, ) { for request_bytes in &block.execution_requests { match ExecutionRequest::try_from_eth_bytes(request_bytes.as_ref()) { @@ -1002,6 +1033,7 @@ async fn parse_execution_requests< ); continue; // Skip this deposit request } + state.push_deposit(deposit_request); } ExecutionRequest::Withdrawal(mut withdrawal_request) => { @@ -1010,20 +1042,18 @@ async fn parse_execution_requests< .get_account(&withdrawal_request.validator_pubkey) .cloned() { - // If the validator already submitted an exit request, we skip this withdrawal request - if matches!(account.status, ValidatorStatus::SubmittedExitRequest) { + // If the validator already has a pending withdrawal request, we skip this withdrawal request + if account.has_pending_withdrawal { info!( - "Failed to parse withdrawal request because the validator already submitted a request: {withdrawal_request:?}" + "Skipping withdrawal request because the validator already has a pending withdrawal request: {withdrawal_request:?}" ); continue; // Skip this withdrawal request } // The balance minus any pending withdrawals have to be larger than the amount of the withdrawal request - if account.balance - account.pending_withdrawal_amount - < withdrawal_request.amount - { + if account.balance < withdrawal_request.amount { info!( - "Failed to parse withdrawal request due to insufficient balance: {withdrawal_request:?}" + "Skipping withdrawal request due to insufficient balance: {withdrawal_request:?}" ); continue; // Skip this withdrawal request } @@ -1031,29 +1061,59 @@ async fn parse_execution_requests< // The source address must match the validators withdrawal address if withdrawal_request.source_address != account.withdrawal_credentials { info!( - "Failed to parse withdrawal request because the source address doesn't match the withdrawal credentials: {withdrawal_request:?}" + "Skipping withdrawal request because the source address doesn't match the withdrawal credentials: {withdrawal_request:?}" ); continue; // Skip this withdrawal request } - // If after this withdrawal the validator balance would be less than the - // minimum stake, then the full validator balance is withdrawn. - if account.balance - - account.pending_withdrawal_amount - - withdrawal_request.amount - < validator_minimum_stake - { - // Check the remaining balance and set the withdrawal amount accordingly - let remaining_balance = - account.balance - account.pending_withdrawal_amount; - withdrawal_request.amount = remaining_balance; + + // Skip the request if the public key is malformatted + let Ok(public_key) = + PublicKey::decode(&withdrawal_request.validator_pubkey[..]) + else { + info!( + "Skipping withdrawal request because the public key is malformatted: {withdrawal_request:?}" + ); + continue; // Skip this withdrawal request + }; + + // We don't support partial withdrawals, so the withdrawal amount will be + // set to the entire balance + let remaining_balance = account.balance; + withdrawal_request.amount = remaining_balance; + + // If the validator is in the warm-up phase after depositing the stake + // and before joining the committee, then the onboarding is aborted + if account.joining_epoch > state.epoch { + // Cancel validator's pending activation + if let Some(validators) = + state.added_validators.get_mut(&account.joining_epoch) + && let Some(pos) = + validators.iter().position(|v| v == &public_key) + { + validators.remove(pos); + info!( + validator = ?public_key, + activation_epoch = account.joining_epoch, + current_epoch = state.epoch, + "cancelled pending validator activation due to withdrawal request" + ); + } + } else { + // Validator is already active - add to removed_validators + state.removed_validators.push(public_key); account.status = ValidatorStatus::SubmittedExitRequest; } - account.pending_withdrawal_amount += withdrawal_request.amount; + account.has_pending_withdrawal = true; state.set_account(withdrawal_request.validator_pubkey, account); + + // The withdrawal will be completed in `validator_withdrawal_num_epochs` epochs + let withdrawal_height = new_height + + validator_withdrawal_num_epochs * epoch_num_of_blocks + - 1; state.push_withdrawal_request( withdrawal_request.clone(), - new_height + validator_withdrawal_period, + withdrawal_height, ); } } @@ -1066,6 +1126,7 @@ async fn parse_execution_requests< } } +#[allow(clippy::too_many_arguments)] async fn process_execution_requests< S: Signer, V: Variant, @@ -1078,92 +1139,100 @@ async fn process_execution_requests< 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, ) { if is_penultimate_block_of_epoch(epoch_num_of_blocks, new_height) { for _ in 0..validator_onboarding_limit_per_block { if let Some(request) = state.pop_deposit() { - let mut validator_balance = 0; - let mut account_exists = false; - if let Some(mut account) = state - .get_account(request.node_pubkey.as_ref().try_into().unwrap()) - .cloned() - { - if request.index > account.last_deposit_index { - account.balance += request.amount; - account.last_deposit_index = request.index; - #[allow(unused)] - #[cfg(debug_assertions)] - { - validator_balance = account.balance; - } - account.last_deposit_index = request.index; - validator_balance = account.balance; - state - .set_account(request.node_pubkey.as_ref().try_into().unwrap(), account); - account_exists = true; - } - } else { - // Validate the withdrawal credentials format - // Eth1 withdrawal credentials: 0x01 + 11 zero bytes + 20 bytes Ethereum address - if request.withdrawal_credentials.len() != 32 { - warn!( - "Invalid withdrawal credentials length: {} bytes, expected 32", - request.withdrawal_credentials.len() + // If the amount is less or more than the minimum stake, or the validator already has a balance + // 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:?}" ); - continue; // Skip this deposit - } - // Check prefix is 0x01 (Eth1 withdrawal) - if request.withdrawal_credentials[0] != 0x01 { - warn!( - "Invalid withdrawal credentials prefix: 0x{:02x}, expected 0x01", - request.withdrawal_credentials[0] + } else { + info!( + "Received deposit request for an existing account, initiating immediate withdrawal: {request:?}" ); - continue; // Skip this deposit - } - // Check 11 zero bytes after the prefix - if !request.withdrawal_credentials[1..12] - .iter() - .all(|&b| b == 0) - { - warn!( - "Invalid withdrawal credentials format: non-zero bytes in positions 1-11" - ); - continue; // Skip this deposit } + let withdrawal_credentials = + match parse_withdrawal_credentials(request.withdrawal_credentials) { + Ok(withdrawal_credentials) => withdrawal_credentials, + Err(e) => { + warn!("Failed to parse withdrawal credentials: {e}"); + continue; + } + }; - // Create new ValidatorAccount from DepositRequest - let new_account = ValidatorAccount { - consensus_public_key: request.consensus_pubkey.clone(), - withdrawal_credentials: Address::from_slice( - &request.withdrawal_credentials[12..32], - ), // Take last 20 bytes - balance: request.amount, - pending_withdrawal_amount: 0, - status: ValidatorStatus::Inactive, - last_deposit_index: request.index, + let validator_pubkey: [u8; 32] = + request.node_pubkey.as_ref().try_into().unwrap(); + let withdrawal_request = WithdrawalRequest { + source_address: withdrawal_credentials, + validator_pubkey, + amount: request.amount, }; - state.set_account( - request.node_pubkey.as_ref().try_into().unwrap(), - new_account, - ); - validator_balance = request.amount; - } - if !account_exists && validator_balance >= validator_minimum_stake { - // If the node shuts down, before the account changes are committed, - // then everything should work normally, because the registry is not persisted to disk - state.added_validators.push(request.node_pubkey.clone()); + let withdrawal_height = + new_height + validator_withdrawal_num_epochs * epoch_num_of_blocks - 1; + + // 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; + } + + state.push_withdrawal_request(withdrawal_request.clone(), withdrawal_height); + continue; } + + // Create new validator account + let withdrawal_credentials = + match parse_withdrawal_credentials(request.withdrawal_credentials) { + Ok(withdrawal_credentials) => withdrawal_credentials, + Err(e) => { + warn!("Failed to parse withdrawal credentials: {e}"); + continue; + } + }; + + // Create new ValidatorAccount from DepositRequest + let activation_epoch = state.epoch + validator_num_warm_up_epochs; + let new_account = ValidatorAccount { + consensus_public_key: request.consensus_pubkey.clone(), + withdrawal_credentials, + balance: request.amount, + pending_withdrawal_amount: 0, + // This will add them to the p2p network for the next epoch + status: ValidatorStatus::Joining, + has_pending_withdrawal: false, + joining_epoch: activation_epoch, + last_deposit_index: request.index, + }; + + state.set_account(node_pubkey_bytes, new_account); + + // If the node shuts down, before the account changes are committed, + // then everything should work normally, because the registry is not persisted to disk + let activation_epoch = state.epoch + validator_num_warm_up_epochs; + state.add_validator(activation_epoch, request.node_pubkey.clone()); + #[cfg(debug_assertions)] { use commonware_codec::Encode; let gauge: Gauge = Gauge::default(); - gauge.set(validator_balance as i64); + gauge.set(request.amount as i64); context.register( - format!("{}{}{}_deposit_validator_balance", - !account_exists && validator_balance >= validator_minimum_stake, - hex::encode(request.withdrawal_credentials), hex::encode(request.node_pubkey.encode())), + format!( + "{}{}_deposit_validator_balance", + hex::encode(request.withdrawal_credentials), + hex::encode(request.node_pubkey.encode()) + ), "Validator balance", - gauge + gauge, ); } } @@ -1179,14 +1248,22 @@ async fn process_execution_requests< assert_eq!(pending_withdrawal.inner, *withdrawal); if let Some(mut account) = state.get_account(&pending_withdrawal.pubkey).cloned() - && account.balance >= withdrawal.amount + && account.balance + account.pending_withdrawal_amount >= withdrawal.amount { - // This check should never fail, because we checked the balance when + // The above balance check should never fail, because we checked the balance when // adding the pending withdrawal to the queue - account.balance = account.balance.saturating_sub(withdrawal.amount); - account.pending_withdrawal_amount = account - .pending_withdrawal_amount - .saturating_sub(withdrawal.amount); + + // If `pending_withdrawal_amount` is >= withdrawal.amount, then this is an immediate withdrawal + // request that was triggered by a invalid deposit request. + if account.pending_withdrawal_amount >= withdrawal.amount { + account.pending_withdrawal_amount = account + .pending_withdrawal_amount + .saturating_sub(withdrawal.amount); + } else { + // For a normal withdrawal request, we decrement the balance directly. + account.balance = account.balance.saturating_sub(withdrawal.amount); + account.has_pending_withdrawal = false; + } #[cfg(debug_assertions)] { @@ -1206,13 +1283,14 @@ async fn process_execution_requests< // If the remaining balance is 0, remove the validator account from the state. if account.balance == 0 { state.remove_account(&pending_withdrawal.pubkey); - state - .removed_validators - .push(PublicKey::decode(&pending_withdrawal.pubkey[..]).unwrap()); // todo(dalton) remove unwrap } else { state.set_account(pending_withdrawal.pubkey, account); } } + // Note: if a deposit request with amount less than the minimum stake was submitted, + // a withdrawal request will be initiated immediately, without creating a validator account. + // This is the only case where we process a withdrawal request, without having a validator account + // stored in the consensus state. } } diff --git a/finalizer/src/config.rs b/finalizer/src/config.rs index 47a9589..7484fcc 100644 --- a/finalizer/src/config.rs +++ b/finalizer/src/config.rs @@ -16,9 +16,11 @@ pub struct FinalizerConfig, V: Vari pub epoch_num_of_blocks: u64, pub validator_max_withdrawals_per_block: usize, pub validator_minimum_stake: u64, // in gwei - pub validator_withdrawal_period: u64, + pub validator_withdrawal_num_epochs: u64, /// The maximum number of validators that will be onboarded at the same time pub validator_onboarding_limit_per_block: usize, + /// Number of epochs to wait before activating validators after deposit + pub validator_num_warm_up_epochs: u64, pub buffer_pool: PoolRef, pub genesis_hash: [u8; 32], /// Optional initial state to initialize the finalizer with diff --git a/finalizer/src/ingress.rs b/finalizer/src/ingress.rs index 90c0b3b..5c91689 100644 --- a/finalizer/src/ingress.rs +++ b/finalizer/src/ingress.rs @@ -6,6 +6,7 @@ use futures::{ channel::{mpsc, oneshot}, }; use summit_syncer::Update; +use summit_types::account::ValidatorAccount; use summit_types::{ Block, BlockAuxData, Digest, PublicKey, checkpoint::Checkpoint, @@ -166,6 +167,25 @@ impl FinalizerMailbox { }; balance } + + // Added for testing + pub async fn get_validator_account(&self, public_key: PublicKey) -> Option { + let (response, rx) = oneshot::channel(); + let request = ConsensusStateRequest::GetValidatorAccount(public_key); + let _ = self + .sender + .clone() + .send(FinalizerMessage::QueryState { request, response }) + .await; + + let res = rx + .await + .expect("consensus state query response sender dropped"); + let ConsensusStateResponse::ValidatorAccount(account) = res else { + unreachable!("request and response variants must match"); + }; + account + } } impl Reporter for FinalizerMailbox { diff --git a/node/src/args.rs b/node/src/args.rs index 666a748..5b7420a 100644 --- a/node/src/args.rs +++ b/node/src/args.rs @@ -665,6 +665,8 @@ fn get_initial_state( balance: VALIDATOR_MINIMUM_STAKE, pending_withdrawal_amount: 0, status: ValidatorStatus::Active, + has_pending_withdrawal: false, + joining_epoch: 0, // TODO(matthias): this index is comes from the deposit contract. // Since there is no deposit transaction for the genesis nodes, the index will still be // 0 for the deposit contract. Right now we only use this index to avoid counting the same deposit request twice. diff --git a/node/src/bin/withdraw_and_exit.rs b/node/src/bin/withdraw_and_exit.rs index d4cb958..b7c93ae 100644 --- a/node/src/bin/withdraw_and_exit.rs +++ b/node/src/bin/withdraw_and_exit.rs @@ -30,7 +30,7 @@ use std::{ thread::JoinHandle, }; use summit::args::{RunFlags, run_node_local}; -use summit::engine::{BLOCKS_PER_EPOCH, VALIDATOR_MINIMUM_STAKE}; +use summit::engine::{BLOCKS_PER_EPOCH, VALIDATOR_MINIMUM_STAKE, VALIDATOR_WITHDRAWAL_NUM_EPOCHS}; use summit_types::PublicKey; use summit_types::reth::Reth; use tokio::sync::mpsc; @@ -238,10 +238,10 @@ fn main() -> Result<(), Box> { .expect("failed to send deposit transaction"); // Wait for all nodes to continue making progress - let epoch_end = BLOCKS_PER_EPOCH; + let end_height = BLOCKS_PER_EPOCH * (VALIDATOR_WITHDRAWAL_NUM_EPOCHS + 1); println!( "Waiting for all {} nodes to reach height {}", - NUM_NODES, epoch_end + NUM_NODES, end_height ); loop { let mut all_ready = true; @@ -249,7 +249,7 @@ fn main() -> Result<(), Box> { let rpc_port = get_node_flags(idx as usize).rpc_port; match get_latest_height(rpc_port).await { Ok(height) => { - if height < epoch_end { + if height < end_height { all_ready = false; println!("Node {} at height {}", idx, height); } @@ -261,7 +261,7 @@ fn main() -> Result<(), Box> { } } if all_ready { - println!("All nodes have reached height {}", epoch_end); + println!("All nodes have reached height {}", end_height); break; } context.sleep(Duration::from_secs(2)).await; @@ -275,7 +275,6 @@ fn main() -> Result<(), Box> { let node0_provider = ProviderBuilder::new().connect_http(node0_url.parse().expect("Invalid URL")); // Check - let balance_after = node0_provider.get_balance(withdrawal_credentials).await.expect("Failed to get balance after withdrawal"); println!("Withdrawal credentials balance after: {} wei", balance_after); diff --git a/node/src/config.rs b/node/src/config.rs index c1468ad..78734f7 100644 --- a/node/src/config.rs +++ b/node/src/config.rs @@ -61,6 +61,7 @@ pub struct EngineConfig> EngineConfig { + #[allow(clippy::too_many_arguments)] pub fn get_engine_config( engine_client: C, oracle: O, diff --git a/node/src/engine.rs b/node/src/engine.rs index edc44e0..6ee87cf 100644 --- a/node/src/engine.rs +++ b/node/src/engine.rs @@ -52,13 +52,10 @@ const MAX_REPAIR: NonZero = NZUsize!(10); const VALIDATOR_ONBOARDING_LIMIT_PER_BLOCK: usize = 3; pub const VALIDATOR_MINIMUM_STAKE: u64 = 32_000_000_000; // in gwei - -#[cfg(feature = "e2e")] -pub const VALIDATOR_WITHDRAWAL_PERIOD: u64 = 10; -#[cfg(all(debug_assertions, not(feature = "e2e")))] -pub const VALIDATOR_WITHDRAWAL_PERIOD: u64 = 5; -#[cfg(all(not(debug_assertions), not(feature = "e2e")))] -const VALIDATOR_WITHDRAWAL_PERIOD: u64 = 100; +// Number of epochs after a deposit until a validator joins the committee +pub const VALIDATOR_NUM_WARM_UP_EPOCHS: u64 = 2; +// Number of epochs after a withdrawal request until the payout +pub const VALIDATOR_WITHDRAWAL_NUM_EPOCHS: u64 = 2; #[cfg(all(feature = "e2e", not(debug_assertions)))] pub const BLOCKS_PER_EPOCH: u64 = 50; #[cfg(debug_assertions)] @@ -280,8 +277,9 @@ where epoch_num_of_blocks: BLOCKS_PER_EPOCH, validator_max_withdrawals_per_block: VALIDATOR_MAX_WITHDRAWALS_PER_BLOCK, validator_minimum_stake: VALIDATOR_MINIMUM_STAKE, - validator_withdrawal_period: VALIDATOR_WITHDRAWAL_PERIOD, + validator_withdrawal_num_epochs: VALIDATOR_WITHDRAWAL_NUM_EPOCHS, validator_onboarding_limit_per_block: VALIDATOR_ONBOARDING_LIMIT_PER_BLOCK, + validator_num_warm_up_epochs: VALIDATOR_NUM_WARM_UP_EPOCHS, buffer_pool: buffer_pool.clone(), genesis_hash: cfg.genesis_hash, initial_state: cfg.initial_state, diff --git a/node/src/test_harness/common.rs b/node/src/test_harness/common.rs index 1045b45..3c137cf 100644 --- a/node/src/test_harness/common.rs +++ b/node/src/test_harness/common.rs @@ -317,7 +317,9 @@ pub fn get_initial_state( balance, pending_withdrawal_amount: 0, status: ValidatorStatus::Active, - // TODO(matthias): this index is comes from the deposit contract. + has_pending_withdrawal: false, + joining_epoch: 0, + // TODO(matthias): this index comes from the deposit contract. // Since there is no deposit transaction for the genesis nodes, the index will still be // 0 for the deposit contract. Right now we only use this index to avoid counting the same deposit request twice. // Since we set the index to 0 here, we cannot rely on the uniqueness. The first actual deposit request will have diff --git a/node/src/tests/execution_requests.rs b/node/src/tests/execution_requests.rs index 7e7aaf8..af0784b 100644 --- a/node/src/tests/execution_requests.rs +++ b/node/src/tests/execution_requests.rs @@ -13,11 +13,11 @@ use commonware_runtime::{Clock, Metrics, Runner as _, deterministic}; use commonware_utils::from_hex_formatted; use std::collections::{HashMap, HashSet}; use std::time::Duration; -use summit_types::PrivateKey; use summit_types::execution_request::ExecutionRequest; use summit_types::keystore::KeyStore; +use summit_types::{PrivateKey, utils}; -use crate::engine::VALIDATOR_WITHDRAWAL_PERIOD; +use crate::engine::VALIDATOR_WITHDRAWAL_NUM_EPOCHS; #[test_traced("INFO")] fn test_deposit_request_single() { @@ -77,7 +77,7 @@ fn test_deposit_request_single() { // Create a single deposit request using the helper let (test_deposit, _, _) = common::create_deposit_request( - 1, + 10, VALIDATOR_MINIMUM_STAKE, common::get_domain(), None, @@ -90,7 +90,7 @@ fn test_deposit_request_single() { // Create execution requests map (add deposit to block 5) let deposit_block_height = 5; - let stop_height = deposit_block_height + 7; + let stop_height = BLOCKS_PER_EPOCH + 1; let mut execution_requests_map = HashMap::new(); execution_requests_map.insert(deposit_block_height, requests); @@ -168,8 +168,6 @@ fn test_deposit_request_single() { if metric.ends_with("finalizer_height") { let height = value.parse::().unwrap(); if height == stop_height { - println!("############################"); - println!("{metric}: {}", height_reached.len()); height_reached.insert(metric.to_string()); } } @@ -214,8 +212,8 @@ fn test_deposit_request_single() { #[test_traced("INFO")] fn test_deposit_request_top_up() { - // Adds two deposit requests to blocks at different heights, and makes sure that the - // validator balance is the sum of the amounts of both deposit requests. + // Adds two deposit requests to blocks at different heights, and makes sure that only + // the first request is processed. let n = 10; let link = Link { latency: Duration::from_millis(80), @@ -272,34 +270,58 @@ fn test_deposit_request_top_up() { // Create a single deposit request using the helper let (test_deposit1, private_key, _) = common::create_deposit_request( - 1, + 10, VALIDATOR_MINIMUM_STAKE, common::get_domain(), None, None, ); let (test_deposit2, _, _) = common::create_deposit_request( - 2, - 10_000_000_000, + 10, + VALIDATOR_MINIMUM_STAKE, + common::get_domain(), + Some(private_key.clone()), + Some(test_deposit1.withdrawal_credentials), + ); + let (test_deposit3, _, _) = common::create_deposit_request( + 10, + VALIDATOR_MINIMUM_STAKE, common::get_domain(), Some(private_key), Some(test_deposit1.withdrawal_credentials), ); + let validator_node_key = test_deposit1.node_pubkey.clone(); + // Convert to ExecutionRequest and then to Requests - let execution_requests1 = vec![ExecutionRequest::Deposit(test_deposit1.clone())]; + let execution_requests1 = vec![ + ExecutionRequest::Deposit(test_deposit1.clone()), + ExecutionRequest::Deposit(test_deposit2.clone()), + ]; let requests1 = common::execution_requests_to_requests(execution_requests1); - let execution_requests2 = vec![ExecutionRequest::Deposit(test_deposit2.clone())]; - let requests2 = common::execution_requests_to_requests(execution_requests2); + let execution_requests3 = vec![ExecutionRequest::Deposit(test_deposit3.clone())]; + let requests3 = common::execution_requests_to_requests(execution_requests3); // Create execution requests map (add deposit to block 5) let deposit_block_height1 = 5; - let deposit_block_height2 = 10; - let stop_height = deposit_block_height2 + VALIDATOR_WITHDRAWAL_PERIOD + 5; + let deposit_block_height2 = 5; + let deposit_block_height3 = 10; + + let deposit_process_height2 = + utils::last_block_in_epoch(BLOCKS_PER_EPOCH, deposit_block_height2 / BLOCKS_PER_EPOCH); + let withdrawal_height2 = + deposit_process_height2 + VALIDATOR_WITHDRAWAL_NUM_EPOCHS * BLOCKS_PER_EPOCH; + + let deposit_process_height3 = + utils::last_block_in_epoch(BLOCKS_PER_EPOCH, deposit_block_height3 / BLOCKS_PER_EPOCH); + let withdrawal_height3 = + deposit_process_height3 + VALIDATOR_WITHDRAWAL_NUM_EPOCHS * BLOCKS_PER_EPOCH; + + let stop_height = withdrawal_height3 + 1; let mut execution_requests_map = HashMap::new(); execution_requests_map.insert(deposit_block_height1, requests1); - execution_requests_map.insert(deposit_block_height2, requests2); + execution_requests_map.insert(deposit_block_height3, requests3); let engine_client_network = MockEngineNetworkBuilder::new(genesis_hash) .with_execution_requests(execution_requests_map) @@ -351,7 +373,6 @@ fn test_deposit_request_top_up() { // Poll metrics let mut height_reached = HashSet::new(); - let mut processed_requests = HashSet::new(); loop { let metrics = context.encode(); @@ -381,23 +402,7 @@ fn test_deposit_request_top_up() { } } - if metric.ends_with("validator_balance") { - let balance = value.parse::().unwrap(); - if balance == test_deposit1.amount { - continue; - } - // Parse the pubkey from the metric name using helper function - let ed_pubkey_hex = - common::parse_metric_substring(metric, "pubkey").expect("pubkey missing"); - let creds = - common::parse_metric_substring(metric, "creds").expect("creds missing"); - assert_eq!(creds, hex::encode(test_deposit1.withdrawal_credentials)); - assert_eq!(ed_pubkey_hex, test_deposit1.node_pubkey.to_string()); - // The amount from both deposits should be added to the validator balance - assert_eq!(balance, test_deposit1.amount + test_deposit2.amount); - processed_requests.insert(metric.to_string()); - } - if processed_requests.len() as u32 >= n && height_reached.len() as u32 == n { + if height_reached.len() as u32 == n { success = true; break; } @@ -410,6 +415,38 @@ fn test_deposit_request_top_up() { context.sleep(Duration::from_secs(1)).await; } + // Assert that the validator account data is consistent with the request + let state_query = consensus_state_queries.get(&0).unwrap(); + let account = state_query + .get_validator_account(validator_node_key) + .await + .unwrap(); + assert_eq!( + account.withdrawal_credentials, + utils::parse_withdrawal_credentials(test_deposit1.withdrawal_credentials).unwrap() + ); + assert_eq!(account.consensus_public_key, test_deposit1.consensus_pubkey); + assert_eq!(account.balance, test_deposit1.amount); + + let withdrawals = engine_client_network.get_withdrawals(); + assert_eq!(withdrawals.len(), 2); + + // check test_deposit2 + let epoch_withdrawals = withdrawals.get(&withdrawal_height2).unwrap(); + assert_eq!(epoch_withdrawals[0].amount, test_deposit2.amount); + + let address = + utils::parse_withdrawal_credentials(test_deposit2.withdrawal_credentials).unwrap(); + assert_eq!(epoch_withdrawals[0].address, address); + + // check test_deposit3 + let epoch_withdrawals = withdrawals.get(&withdrawal_height3).unwrap(); + assert_eq!(epoch_withdrawals[0].amount, test_deposit2.amount); + + let address = + utils::parse_withdrawal_credentials(test_deposit2.withdrawal_credentials).unwrap(); + assert_eq!(epoch_withdrawals[0].address, address); + // Check that all nodes have the same canonical chain assert!( engine_client_network @@ -625,7 +662,7 @@ fn test_deposit_and_withdrawal_request_single() { let withdrawals = engine_client_network.get_withdrawals(); assert_eq!(withdrawals.len(), 1); let withdrawal_epoch = - (withdrawal_block_height + VALIDATOR_WITHDRAWAL_PERIOD) / BLOCKS_PER_EPOCH; + (withdrawal_block_height / BLOCKS_PER_EPOCH) + VALIDATOR_WITHDRAWAL_NUM_EPOCHS; let withdrawal_height = (withdrawal_epoch + 1) * BLOCKS_PER_EPOCH - 1; let withdrawals = withdrawals .get(&(withdrawal_height)) @@ -864,7 +901,7 @@ fn test_partial_withdrawal_balance_below_minimum_stake() { // to the execution layer. assert_eq!(withdrawals.len(), 1); let withdrawal_epoch = - (withdrawal_block_height + VALIDATOR_WITHDRAWAL_PERIOD) / BLOCKS_PER_EPOCH; + (withdrawal_block_height / BLOCKS_PER_EPOCH) + VALIDATOR_WITHDRAWAL_NUM_EPOCHS; let withdrawal_height = (withdrawal_epoch + 1) * BLOCKS_PER_EPOCH - 1; let withdrawals = withdrawals .get(&withdrawal_height) @@ -886,12 +923,10 @@ fn test_partial_withdrawal_balance_below_minimum_stake() { } #[test_traced("INFO")] -fn test_deposit_less_than_min_stake_and_withdrawal() { - // Adds a deposit request to the block at height 5, and then adds a withdrawal request - // to the block at height 7. - // The deposit request is less than the minimum stake, so the validator should not be added - // to the registry. - // The balance should still increase and the withdrawal should work as well. +fn test_deposit_less_than_min_stake_rejected() { + // Adds a deposit request to the block at height 5. + // The deposit request should be skipped and a withdrawal request for the same amount + // should be initiated. let n = 10; let link = Link { latency: Duration::from_millis(80), @@ -955,31 +990,23 @@ fn test_deposit_less_than_min_stake_and_withdrawal() { None, ); - let withdrawal_address = Address::from_slice(&test_deposit.withdrawal_credentials[12..32]); - let test_withdrawal = common::create_withdrawal_request( - withdrawal_address, - test_deposit.node_pubkey.as_ref().try_into().unwrap(), - test_deposit.amount, - ); + let validator_node_key = test_deposit.node_pubkey.clone(); // Convert to ExecutionRequest and then to Requests let execution_requests1 = vec![ExecutionRequest::Deposit(test_deposit.clone())]; let requests1 = common::execution_requests_to_requests(execution_requests1); - let execution_requests2 = vec![ExecutionRequest::Withdrawal(test_withdrawal.clone())]; - let requests2 = common::execution_requests_to_requests(execution_requests2); - // Create execution requests map (add deposit to block 5) - // The deposit request will be processed after 10 blocks because `BLOCKS_PER_EPOCH` - // is set to 10 in debug mode. - // The withdrawal request should be added after block 10, otherwise it will be ignored, because - // the account doesn't exist yet. let deposit_block_height = 5; - let withdrawal_block_height = 11; - let stop_height = withdrawal_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, requests1); - execution_requests_map.insert(withdrawal_block_height, requests2); let engine_client_network = MockEngineNetworkBuilder::new(genesis_hash) .with_execution_requests(execution_requests_map) @@ -1031,7 +1058,6 @@ fn test_deposit_less_than_min_stake_and_withdrawal() { // Poll metrics let mut height_reached = HashSet::new(); - let mut processed_requests = HashSet::new(); loop { let metrics = context.encode(); @@ -1070,19 +1096,7 @@ fn test_deposit_less_than_min_stake_and_withdrawal() { assert_eq!(registry_flag, "false"); } - if metric.ends_with("withdrawal_validator_balance") { - let balance = value.parse::().unwrap(); - // Parse the pubkey from the metric name using helper function - let ed_pubkey_hex = common::parse_metric_substring(metric, "pubkey") - .expect(&format!("{}: {} (failed to parse pubkey)", metric, value)); - let creds = - common::parse_metric_substring(metric, "creds").expect("creds missing"); - assert_eq!(creds, hex::encode(test_withdrawal.source_address)); - assert_eq!(ed_pubkey_hex, test_deposit.node_pubkey.to_string()); - assert_eq!(balance, test_deposit.amount - test_withdrawal.amount); - processed_requests.insert(metric.to_string()); - } - if processed_requests.len() as u32 >= n && height_reached.len() as u32 == n { + if height_reached.len() as u32 == n { success = true; break; } @@ -1095,16 +1109,20 @@ fn test_deposit_less_than_min_stake_and_withdrawal() { 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 withdrawal_epoch = - (withdrawal_block_height + VALIDATOR_WITHDRAWAL_PERIOD) / BLOCKS_PER_EPOCH; - let withdrawal_height = (withdrawal_epoch + 1) * BLOCKS_PER_EPOCH - 1; - let withdrawals = withdrawals - .get(&withdrawal_height) - .expect("missing withdrawal"); - assert_eq!(withdrawals[0].amount, test_withdrawal.amount); - assert_eq!(withdrawals[0].address, test_withdrawal.source_address); + + 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!( diff --git a/types/src/account.rs b/types/src/account.rs index fc0db2e..25da7f5 100644 --- a/types/src/account.rs +++ b/types/src/account.rs @@ -8,6 +8,7 @@ pub enum ValidatorStatus { Active, Inactive, SubmittedExitRequest, + Joining, } impl ValidatorStatus { @@ -16,6 +17,7 @@ impl ValidatorStatus { ValidatorStatus::Active => 0, ValidatorStatus::Inactive => 1, ValidatorStatus::SubmittedExitRequest => 2, + ValidatorStatus::Joining => 3, } } @@ -24,6 +26,7 @@ impl ValidatorStatus { 0 => Ok(ValidatorStatus::Active), 1 => Ok(ValidatorStatus::Inactive), 2 => Ok(ValidatorStatus::SubmittedExitRequest), + 3 => Ok(ValidatorStatus::Joining), _ => Err("Invalid ValidatorStatus value"), } } @@ -36,6 +39,8 @@ pub struct ValidatorAccount { pub balance: u64, // Balance in gwei pub pending_withdrawal_amount: u64, // Sum of pending withdrawals in gwei pub status: ValidatorStatus, + pub has_pending_withdrawal: bool, + pub joining_epoch: u64, // Epoch when validator joined/will join (genesis validators = 0) pub last_deposit_index: u64, // Last deposit request index } @@ -43,11 +48,11 @@ impl TryFrom<&[u8]> for ValidatorAccount { type Error = &'static str; fn try_from(bytes: &[u8]) -> Result { - // ValidatorAccount data is exactly 93 bytes - // Format: consensus_public_key(48) + withdrawal_credentials(20) + balance(8) + pending_withdrawal_amount(8) + status(1) + last_deposit_index(8) = 93 bytes + // ValidatorAccount data is exactly 102 bytes + // Format: consensus_public_key(48) + withdrawal_credentials(20) + balance(8) + pending_withdrawal_amount(8) + status(1) + has_pending_withdrawal(1) + joining_epoch(8) + last_deposit_index(8) = 102 bytes - if bytes.len() != 93 { - return Err("ValidatorAccount must be exactly 93 bytes"); + if bytes.len() != 102 { + return Err("ValidatorAccount must be exactly 102 bytes"); } // Extract consensus_public_key (48 bytes) @@ -78,8 +83,21 @@ impl TryFrom<&[u8]> for ValidatorAccount { // Extract status (1 byte) let status = ValidatorStatus::from_u8(bytes[84])?; + // Extract has_pending_withdrawal (1 byte) + let has_pending_withdrawal = match bytes[85] { + 0x0 => Ok(false), + 0x1 => Ok(true), + _ => Err("Unknown validator status"), + }?; + + // Extract joining_epoch (8 bytes, little-endian u64) + let joining_epoch_bytes: [u8; 8] = bytes[86..94] + .try_into() + .map_err(|_| "Failed to parse joining_epoch")?; + let joining_epoch = u64::from_le_bytes(joining_epoch_bytes); + // Extract last_deposit_index (8 bytes, little-endian u64) - let last_deposit_index_bytes: [u8; 8] = bytes[85..93] + let last_deposit_index_bytes: [u8; 8] = bytes[94..102] .try_into() .map_err(|_| "Failed to parse last_deposit_index")?; let last_deposit_index = u64::from_le_bytes(last_deposit_index_bytes); @@ -90,6 +108,8 @@ impl TryFrom<&[u8]> for ValidatorAccount { balance, pending_withdrawal_amount, status, + has_pending_withdrawal, + joining_epoch, last_deposit_index, }) } @@ -102,19 +122,21 @@ impl Write for ValidatorAccount { buf.put(&self.balance.to_le_bytes()[..]); buf.put(&self.pending_withdrawal_amount.to_le_bytes()[..]); buf.put_u8(self.status.to_u8()); + buf.put_u8(self.has_pending_withdrawal as u8); + buf.put(&self.joining_epoch.to_le_bytes()[..]); buf.put(&self.last_deposit_index.to_le_bytes()[..]); } } impl FixedSize for ValidatorAccount { - const SIZE: usize = 93; // 48 + 20 + 8 + 8 + 1 + 8 + const SIZE: usize = 102; // 48 + 20 + 8 + 8 + 1 + 1 + 8 + 8 } impl Read for ValidatorAccount { type Cfg = (); fn read_cfg(buf: &mut impl Buf, _cfg: &Self::Cfg) -> Result { - if buf.remaining() < 93 { + if buf.remaining() < 102 { return Err(Error::Invalid("ValidatorAccount", "Insufficient bytes")); } @@ -139,6 +161,20 @@ impl Read for ValidatorAccount { let status = ValidatorStatus::from_u8(status_byte) .map_err(|_| Error::Invalid("ValidatorAccount", "Invalid status value"))?; + // Extract has_pending_withdrawal (1 byte) + let has_pending_withdrawal = match buf.get_u8() { + 0x0 => Ok(false), + 0x1 => Ok(true), + _ => Err(Error::Invalid( + "ValidatorAccount", + "Unknown validator status", + )), + }?; + + let mut joining_epoch_bytes = [0u8; 8]; + buf.copy_to_slice(&mut joining_epoch_bytes); + let joining_epoch = u64::from_le_bytes(joining_epoch_bytes); + let mut last_deposit_index_bytes = [0u8; 8]; buf.copy_to_slice(&mut last_deposit_index_bytes); let last_deposit_index = u64::from_le_bytes(last_deposit_index_bytes); @@ -149,6 +185,8 @@ impl Read for ValidatorAccount { balance, pending_withdrawal_amount, status, + has_pending_withdrawal, + joining_epoch, last_deposit_index, }) } @@ -170,6 +208,8 @@ mod tests { balance: 32000000000u64, // 32 ETH in gwei pending_withdrawal_amount: 1000000000u64, // 1 ETH in gwei status: ValidatorStatus::Active, + has_pending_withdrawal: false, + joining_epoch: 0, last_deposit_index: 42u64, }; @@ -192,6 +232,8 @@ mod tests { balance: 64000000000u64, // 64 ETH in gwei pending_withdrawal_amount: 2000000000u64, // 2 ETH in gwei status: ValidatorStatus::Inactive, + has_pending_withdrawal: true, + joining_epoch: 0, last_deposit_index: 100u64, }; @@ -207,7 +249,7 @@ mod tests { #[test] fn test_validator_account_insufficient_bytes() { let mut buf = BytesMut::new(); - buf.put(&[0u8; 92][..]); // One byte short + buf.put(&[0u8; 101][..]); // One byte short let result = ValidatorAccount::read(&mut buf.as_ref()); assert!(result.is_err()); @@ -221,23 +263,23 @@ mod tests { #[test] fn test_validator_account_try_from_insufficient_bytes() { - let buf = [0u8; 92]; // One byte short + let buf = [0u8; 101]; // One byte short let result = ValidatorAccount::try_from(buf.as_ref()); assert!(result.is_err()); assert_eq!( result.unwrap_err(), - "ValidatorAccount must be exactly 93 bytes" + "ValidatorAccount must be exactly 102 bytes" ); } #[test] fn test_validator_account_try_from_too_many_bytes() { - let buf = [0u8; 94]; // One byte too many + let buf = [0u8; 103]; // One byte too many let result = ValidatorAccount::try_from(buf.as_ref()); assert!(result.is_err()); assert_eq!( result.unwrap_err(), - "ValidatorAccount must be exactly 93 bytes" + "ValidatorAccount must be exactly 102 bytes" ); } @@ -251,6 +293,8 @@ mod tests { balance: 128000000000u64, // 128 ETH in gwei pending_withdrawal_amount: 4000000000u64, // 4 ETH in gwei status: ValidatorStatus::SubmittedExitRequest, + has_pending_withdrawal: false, + joining_epoch: 0, last_deposit_index: 500u64, }; @@ -270,7 +314,7 @@ mod tests { #[test] fn test_validator_account_fixed_size() { - assert_eq!(ValidatorAccount::SIZE, 93); + assert_eq!(ValidatorAccount::SIZE, 102); let consensus_key = bls12381::PrivateKey::from_seed(1); let account = ValidatorAccount { @@ -279,6 +323,8 @@ mod tests { balance: 0, pending_withdrawal_amount: 0, status: ValidatorStatus::Active, + has_pending_withdrawal: false, + joining_epoch: 0, last_deposit_index: 0, }; @@ -301,6 +347,8 @@ mod tests { balance: 0x0123456789abcdefu64, pending_withdrawal_amount: 0xfedcba9876543210u64, status: ValidatorStatus::SubmittedExitRequest, + has_pending_withdrawal: false, + joining_epoch: 0, last_deposit_index: 0xa1b2c3d4e5f60708u64, }; @@ -330,8 +378,14 @@ mod tests { // Check status (next 1 byte) assert_eq!(bytes[84], 2); // SubmittedExitRequest = 2 + // Check has_pending_withdrawal (next 1 byte) + assert_eq!(bytes[85], 0); // false = 0 + + // Check joining_epoch (next 8 bytes, little-endian) + assert_eq!(&bytes[86..94], &0u64.to_le_bytes()); + // Check last_deposit_index (last 8 bytes, little-endian) - assert_eq!(&bytes[85..93], &0xa1b2c3d4e5f60708u64.to_le_bytes()); + assert_eq!(&bytes[94..102], &0xa1b2c3d4e5f60708u64.to_le_bytes()); // Verify roundtrip let decoded = ValidatorAccount::read(&mut buf.as_ref()).unwrap(); @@ -357,9 +411,13 @@ mod tests { ValidatorStatus::from_u8(2).unwrap(), ValidatorStatus::SubmittedExitRequest ); + assert_eq!( + ValidatorStatus::from_u8(3).unwrap(), + ValidatorStatus::Joining + ); // Test invalid status - assert!(ValidatorStatus::from_u8(3).is_err()); + assert!(ValidatorStatus::from_u8(4).is_err()); assert!(ValidatorStatus::from_u8(255).is_err()); } @@ -374,6 +432,8 @@ mod tests { buf.put(&1000u64.to_le_bytes()[..]); // balance buf.put(&100u64.to_le_bytes()[..]); // pending_withdrawal_amount buf.put_u8(99); // invalid status + buf.put_u8(0); // has_pending_withdrawal + buf.put(&0u64.to_le_bytes()[..]); // joining_epoch buf.put(&42u64.to_le_bytes()[..]); // last_deposit_index let result = ValidatorAccount::read(&mut buf.as_ref()); diff --git a/types/src/checkpoint.rs b/types/src/checkpoint.rs index 4ded295..cc705b9 100644 --- a/types/src/checkpoint.rs +++ b/types/src/checkpoint.rs @@ -155,7 +155,7 @@ mod tests { withdrawal_queue: VecDeque::new(), validator_accounts: HashMap::new(), pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: HashMap::new(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], @@ -226,6 +226,8 @@ mod tests { balance: 32_000_000_000, // 32 ETH pending_withdrawal_amount: 0, status: ValidatorStatus::Active, + has_pending_withdrawal: false, + joining_epoch: 0, last_deposit_index: 100, }; @@ -236,6 +238,8 @@ mod tests { balance: 16_000_000_000, // 16 ETH pending_withdrawal_amount: 8_000_000_000, // 8 ETH pending status: ValidatorStatus::SubmittedExitRequest, + has_pending_withdrawal: true, + joining_epoch: 0, last_deposit_index: 101, }; @@ -261,7 +265,7 @@ mod tests { withdrawal_queue, validator_accounts, pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: HashMap::new(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], @@ -297,7 +301,7 @@ mod tests { withdrawal_queue: VecDeque::new(), validator_accounts: HashMap::new(), pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: HashMap::new(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], @@ -375,6 +379,8 @@ mod tests { balance: 32_000_000_000, // 32 ETH pending_withdrawal_amount: 0, status: ValidatorStatus::Active, + has_pending_withdrawal: false, + joining_epoch: 0, last_deposit_index: 100, }; @@ -385,6 +391,8 @@ mod tests { balance: 16_000_000_000, // 16 ETH pending_withdrawal_amount: 8_000_000_000, // 8 ETH pending status: ValidatorStatus::SubmittedExitRequest, + has_pending_withdrawal: true, + joining_epoch: 0, last_deposit_index: 101, }; @@ -410,7 +418,7 @@ mod tests { withdrawal_queue, validator_accounts, pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: HashMap::new(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], @@ -451,7 +459,7 @@ mod tests { withdrawal_queue: VecDeque::new(), validator_accounts: HashMap::new(), pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: HashMap::new(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], @@ -498,7 +506,7 @@ mod tests { withdrawal_queue: VecDeque::new(), validator_accounts: HashMap::new(), pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: HashMap::new(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], @@ -541,7 +549,7 @@ mod tests { withdrawal_queue: VecDeque::new(), validator_accounts: HashMap::new(), pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: HashMap::new(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], @@ -602,6 +610,8 @@ mod tests { balance: 32_000_000_000, // 32 ETH pending_withdrawal_amount: 0, status: ValidatorStatus::Active, + has_pending_withdrawal: false, + joining_epoch: 0, last_deposit_index: 100, }; @@ -625,7 +635,7 @@ mod tests { withdrawal_queue, validator_accounts, pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: HashMap::new(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], diff --git a/types/src/consensus_state.rs b/types/src/consensus_state.rs index fbbd53f..43ffd80 100644 --- a/types/src/consensus_state.rs +++ b/types/src/consensus_state.rs @@ -21,7 +21,7 @@ pub struct ConsensusState { pub withdrawal_queue: VecDeque, pub validator_accounts: HashMap<[u8; 32], ValidatorAccount>, pub pending_checkpoint: Option, - pub added_validators: Vec, + pub added_validators: HashMap>, pub removed_validators: Vec, pub forkchoice: ForkchoiceState, pub epoch_genesis_hash: [u8; 32], @@ -39,7 +39,7 @@ impl Default for ConsensusState { withdrawal_queue: Default::default(), validator_accounts: Default::default(), pending_checkpoint: None, - added_validators: Vec::new(), + added_validators: Default::default(), removed_validators: Vec::new(), forkchoice: Default::default(), epoch_genesis_hash: [0u8; 32], @@ -103,12 +103,15 @@ impl ConsensusState { self.pending_checkpoint = checkpoint; } - pub fn get_added_validators(&self) -> &Vec { - &self.added_validators + pub fn get_added_validators(&self, epoch: u64) -> Option<&Vec> { + self.added_validators.get(&epoch) } - pub fn set_added_validators(&mut self, validators: Vec) { - self.added_validators = validators; + pub fn add_validator(&mut self, epoch: u64, validator: PublicKey) { + self.added_validators + .entry(epoch) + .or_default() + .push(validator); } pub fn get_removed_validators(&self) -> &Vec { @@ -239,6 +242,25 @@ impl ConsensusState { peers } + pub fn get_active_or_joining_validators(&self) -> Vec<(PublicKey, bls12381::PublicKey)> { + let mut peers: Vec<(PublicKey, bls12381::PublicKey)> = self + .validator_accounts + .iter() + .filter(|(_, acc)| { + acc.status == ValidatorStatus::Active || acc.status == ValidatorStatus::Joining + }) + .map(|(v, acc)| { + let mut key_bytes = &v[..]; + let node_public_key = + PublicKey::read(&mut key_bytes).expect("failed to parse public key"); + let consensus_public_key = acc.consensus_public_key.clone(); + (node_public_key, consensus_public_key) + }) + .collect(); + peers.sort_by(|lhs, rhs| lhs.0.cmp(&rhs.0)); + peers + } + pub fn get_active_validators_as(&self) -> Vec<(PublicKey, BLS)> where bls12381::PublicKey: Into, @@ -265,7 +287,7 @@ impl EncodeSize for ConsensusState { + 1 // pending_checkpoint presence flag + self.pending_checkpoint.as_ref().map_or(0, |cp| cp.encode_size()) + 4 // added_validators length - + self.added_validators.iter().map(|pk| pk.encode_size()).sum::() + + self.added_validators.values().map(|validators| 8 + 4 + validators.iter().map(|pk| pk.encode_size()).sum::()).sum::() + 4 // removed_validators length + self.removed_validators.iter().map(|pk| pk.encode_size()).sum::() + 32 // forkchoice.head_block_hash @@ -316,9 +338,15 @@ impl Read for ConsensusState { // Read added_validators let added_validators_len = buf.get_u32() as usize; - let mut added_validators = Vec::with_capacity(added_validators_len); + let mut added_validators = HashMap::new(); for _ in 0..added_validators_len { - added_validators.push(PublicKey::read_cfg(buf, &())?); + let key = buf.get_u64(); + let validator_count = buf.get_u32() as usize; + let mut validators = Vec::with_capacity(validator_count); + for _ in 0..validator_count { + validators.push(PublicKey::read_cfg(buf, &())?); + } + added_validators.insert(key, validators); } // Read removed_validators @@ -400,8 +428,12 @@ impl Write for ConsensusState { // Write added_validators buf.put_u32(self.added_validators.len() as u32); - for validator in &self.added_validators { - validator.write(buf); + for (key, validators) in &self.added_validators { + buf.put_u64(*key); + buf.put_u32(validators.len() as u32); + for validator in validators { + validator.write(buf); + } } // Write removed_validators @@ -442,7 +474,7 @@ mod tests { use alloy_eips::eip4895::Withdrawal; use alloy_primitives::Address; use commonware_codec::{DecodeExt, Encode}; - use commonware_cryptography::{PrivateKeyExt, Signer, bls12381}; + use commonware_cryptography::{PrivateKeyExt, Signer, bls12381, ed25519}; fn create_test_deposit_request(index: u64, amount: u64) -> DepositRequest { let mut withdrawal_credentials = [0u8; 32]; @@ -488,6 +520,8 @@ mod tests { balance, pending_withdrawal_amount: 0, status: ValidatorStatus::Active, + has_pending_withdrawal: false, + joining_epoch: 0, last_deposit_index: index, } } @@ -551,6 +585,22 @@ mod tests { original_state.set_account(pubkey1, account1); original_state.set_account(pubkey2, account2); + // Add validators scheduled for future epochs + let validator1 = ed25519::PrivateKey::from_seed(10).public_key(); + let validator2 = ed25519::PrivateKey::from_seed(20).public_key(); + let validator3 = ed25519::PrivateKey::from_seed(30).public_key(); + let validator4 = ed25519::PrivateKey::from_seed(40).public_key(); + + // Schedule validators for epoch 9 (current epoch + 2) + original_state.add_validator(9, validator1.clone()); + original_state.add_validator(9, validator2.clone()); + + // Schedule validators for epoch 10 + original_state.add_validator(10, validator3.clone()); + + // Schedule validators for epoch 11 + original_state.add_validator(11, validator4.clone()); + let mut encoded = original_state.encode(); let decoded_state = ConsensusState::decode(&mut encoded).expect("Failed to decode"); @@ -585,6 +635,24 @@ mod tests { let decoded_account2 = decoded_state.validator_accounts.get(&pubkey2).unwrap(); assert_eq!(decoded_account2.balance, 64000000000); assert_eq!(decoded_account2.last_deposit_index, 2); + + // Verify added_validators + assert_eq!(decoded_state.added_validators.len(), 3); + + // Check epoch 9 has 2 validators + let epoch9_validators = decoded_state.get_added_validators(9).unwrap(); + assert_eq!(epoch9_validators.len(), 2); + + // Check epoch 10 has 1 validator + let epoch10_validators = decoded_state.get_added_validators(10).unwrap(); + assert_eq!(epoch10_validators.len(), 1); + + // Check epoch 11 has 1 validator + let epoch11_validators = decoded_state.get_added_validators(11).unwrap(); + assert_eq!(epoch11_validators.len(), 1); + + // Check that epoch 8 returns None (no validators scheduled) + assert!(decoded_state.get_added_validators(8).is_none()); } #[test] @@ -606,6 +674,15 @@ mod tests { let account = create_test_validator_account(1, 32000000000); state.set_account(pubkey, account); + // Add validators scheduled for future epochs + let validator1 = ed25519::PrivateKey::from_seed(10).public_key(); + let validator2 = ed25519::PrivateKey::from_seed(20).public_key(); + let validator3 = ed25519::PrivateKey::from_seed(30).public_key(); + + state.add_validator(5, validator1.clone()); + state.add_validator(6, validator2.clone()); + state.add_validator(6, validator3.clone()); + let predicted_size = state.encode_size(); let actual_encoded = state.encode(); let actual_size = actual_encoded.len(); diff --git a/types/src/consensus_state_query.rs b/types/src/consensus_state_query.rs index f925ba8..818e4cd 100644 --- a/types/src/consensus_state_query.rs +++ b/types/src/consensus_state_query.rs @@ -1,4 +1,5 @@ use crate::PublicKey; +use crate::account::ValidatorAccount; use crate::checkpoint::Checkpoint; use futures::SinkExt; use futures::channel::{mpsc, oneshot}; @@ -9,6 +10,7 @@ pub enum ConsensusStateRequest { GetCheckpoint(u64), GetLatestHeight, GetValidatorBalance(PublicKey), + GetValidatorAccount(PublicKey), } pub enum ConsensusStateResponse { @@ -16,6 +18,7 @@ pub enum ConsensusStateResponse { Checkpoint(Option), LatestHeight(u64), ValidatorBalance(Option), + ValidatorAccount(Option), } /// Used to send queries to the application finalizer to query the consensus state. diff --git a/types/src/utils.rs b/types/src/utils.rs index 1ccf2d4..354dbbd 100644 --- a/types/src/utils.rs +++ b/types/src/utils.rs @@ -1,4 +1,5 @@ -use anyhow::{Context, Result}; +use alloy_primitives::Address; +use anyhow::{Context, Result, anyhow}; use commonware_consensus::types::Epoch; use dirs::home_dir; use std::{path::PathBuf, str::FromStr}; @@ -31,6 +32,32 @@ pub fn is_penultimate_block_of_epoch(epoch_num_blocks: u64, height: u64) -> bool is_last_block_of_epoch(epoch_num_blocks, height + 1) } +pub fn parse_withdrawal_credentials(withdrawal_credentials: [u8; 32]) -> Result
{ + // Validate the withdrawal credentials format + // Eth1 withdrawal credentials: 0x01 + 11 zero bytes + 20 bytes Ethereum address + if withdrawal_credentials.len() != 32 { + return Err(anyhow!( + "Invalid withdrawal credentials length: {} bytes, expected 32", + withdrawal_credentials.len() + )); + } + // Check prefix is 0x01 (Eth1 withdrawal) + if withdrawal_credentials[0] != 0x01 { + return Err(anyhow!( + "Invalid withdrawal credentials prefix: 0x{:02x}, expected 0x01", + withdrawal_credentials[0] + )); + } + // Check 11 zero bytes after the prefix + if !withdrawal_credentials[1..12].iter().all(|&b| b == 0) { + return Err(anyhow!( + "Invalid withdrawal credentials format: non-zero bytes in positions 1-11" + )); + } + // Take last 20 bytes + Ok(Address::from_slice(&withdrawal_credentials[12..32])) +} + #[cfg(any(feature = "base-bench", feature = "bench"))] pub mod benchmarking { use alloy_primitives::B256;