diff --git a/eth2/state_processing/src/per_block_processing/verify_transfer.rs b/eth2/state_processing/src/per_block_processing/verify_transfer.rs index c6388bebea..3c11290e00 100644 --- a/eth2/state_processing/src/per_block_processing/verify_transfer.rs +++ b/eth2/state_processing/src/per_block_processing/verify_transfer.rs @@ -8,9 +8,7 @@ use types::*; /// /// Returns `Ok(())` if the `Transfer` is valid, otherwise indicates the reason for invalidity. /// -/// Note: this function is incomplete. -/// -/// Spec v0.5.1 +/// Spec v0.6.1 pub fn verify_transfer( state: &BeaconState, transfer: &Transfer, @@ -20,6 +18,8 @@ pub fn verify_transfer( } /// Like `verify_transfer` but doesn't run checks which may become true in future states. +/// +/// Spec v0.6.1 pub fn verify_transfer_time_independent_only( state: &BeaconState, transfer: &Transfer, @@ -29,32 +29,41 @@ pub fn verify_transfer_time_independent_only( } /// Parametric version of `verify_transfer` that allows some checks to be skipped. +/// +/// When `time_independent_only == true`, time-specific parameters are ignored, including: +/// +/// - Balance considerations (e.g., adequate balance, not dust, etc). +/// - `transfer.slot` does not have to exactly match `state.slot`, it just needs to be in the +/// present or future. +/// - Validator transfer eligibility (e.g., is withdrawable) +/// +/// Spec v0.6.1 fn verify_transfer_parametric( state: &BeaconState, transfer: &Transfer, spec: &ChainSpec, time_independent_only: bool, ) -> Result<(), Error> { + // Load the sender balance from state. let sender_balance = *state - .validator_balances + .balances .get(transfer.sender as usize) .ok_or_else(|| Error::Invalid(Invalid::FromValidatorUnknown(transfer.sender)))?; + // Safely determine `amount + fee`. let total_amount = transfer .amount .checked_add(transfer.fee) .ok_or_else(|| Error::Invalid(Invalid::FeeOverflow(transfer.amount, transfer.fee)))?; + // Verify the sender has adequate balance. verify!( time_independent_only || sender_balance >= transfer.amount, Invalid::FromBalanceInsufficient(transfer.amount, sender_balance) ); - verify!( - time_independent_only || sender_balance >= transfer.fee, - Invalid::FromBalanceInsufficient(transfer.fee, sender_balance) - ); - + // Verify balances are not "dust" (i.e., greater than zero but less than the minimum deposit + // amount). verify!( time_independent_only || (sender_balance == total_amount) @@ -65,6 +74,8 @@ fn verify_transfer_parametric( ) ); + // If loosely enforcing `transfer.slot`, ensure the slot is not in the past. Otherwise, ensure + // the transfer slot equals the state slot. if time_independent_only { verify!( state.slot <= transfer.slot, @@ -77,19 +88,33 @@ fn verify_transfer_parametric( ); } + // Load the sender `Validator` record from the state. let sender_validator = state .validator_registry .get(transfer.sender as usize) .ok_or_else(|| Error::Invalid(Invalid::FromValidatorUnknown(transfer.sender)))?; + let epoch = state.slot.epoch(spec.slots_per_epoch); + // Ensure one of the following is met: + // + // - Time independent checks are being ignored. + // - The sender has not been activated. + // - The sender is withdrawable at the state's epoch. + // - The transfer will not reduce the sender below the max effective balance. verify!( time_independent_only + || sender_validator.activation_eligibility_epoch == spec.far_future_epoch || sender_validator.is_withdrawable_at(epoch) - || sender_validator.activation_epoch == spec.far_future_epoch, + || total_amount + spec.max_effective_balance <= sender_balance, Invalid::FromValidatorIneligableForTransfer(transfer.sender) ); + // Ensure the withdrawal credentials generated from the sender's pubkey match those stored in + // the validator registry. + // + // This ensures the validator can only perform a transfer when they are in control of the + // withdrawal address. let transfer_withdrawal_credentials = Hash256::from_slice( &get_withdrawal_credentials(&transfer.pubkey, spec.bls_withdrawal_prefix_byte)[..], ); @@ -101,13 +126,13 @@ fn verify_transfer_parametric( ) ); + // Verify the transfer signature. let message = transfer.signed_root(); let domain = spec.get_domain( transfer.slot.epoch(spec.slots_per_epoch), Domain::Transfer, &state.fork, ); - verify!( transfer .signature @@ -122,31 +147,31 @@ fn verify_transfer_parametric( /// /// Does not check that the transfer is valid, however checks for overflow in all actions. /// -/// Spec v0.5.1 +/// Spec v0.6.1 pub fn execute_transfer( state: &mut BeaconState, transfer: &Transfer, spec: &ChainSpec, ) -> Result<(), Error> { let sender_balance = *state - .validator_balances + .balances .get(transfer.sender as usize) .ok_or_else(|| Error::Invalid(Invalid::FromValidatorUnknown(transfer.sender)))?; let recipient_balance = *state - .validator_balances + .balances .get(transfer.recipient as usize) .ok_or_else(|| Error::Invalid(Invalid::ToValidatorUnknown(transfer.recipient)))?; let proposer_index = state.get_beacon_proposer_index(state.slot, RelativeEpoch::Current, spec)?; - let proposer_balance = state.validator_balances[proposer_index]; + let proposer_balance = state.balances[proposer_index]; let total_amount = transfer .amount .checked_add(transfer.fee) .ok_or_else(|| Error::Invalid(Invalid::FeeOverflow(transfer.amount, transfer.fee)))?; - state.validator_balances[transfer.sender as usize] = + state.balances[transfer.sender as usize] = sender_balance.checked_sub(total_amount).ok_or_else(|| { Error::Invalid(Invalid::FromBalanceInsufficient( total_amount, @@ -154,7 +179,7 @@ pub fn execute_transfer( )) })?; - state.validator_balances[transfer.recipient as usize] = recipient_balance + state.balances[transfer.recipient as usize] = recipient_balance .checked_add(transfer.amount) .ok_or_else(|| { Error::Invalid(Invalid::ToBalanceOverflow( @@ -163,7 +188,7 @@ pub fn execute_transfer( )) })?; - state.validator_balances[proposer_index] = + state.balances[proposer_index] = proposer_balance.checked_add(transfer.fee).ok_or_else(|| { Error::Invalid(Invalid::ProposerBalanceOverflow( proposer_balance, diff --git a/eth2/types/src/validator.rs b/eth2/types/src/validator.rs index ad65a8c604..f16225d7cc 100644 --- a/eth2/types/src/validator.rs +++ b/eth2/types/src/validator.rs @@ -49,7 +49,7 @@ impl Validator { /// Returns `true` if the validator is able to withdraw at some epoch. pub fn is_withdrawable_at(&self, epoch: Epoch) -> bool { - self.withdrawable_epoch <= epoch + epoch >= self.withdrawable_epoch } }