[Altair] Return error when total balance is zero

[paulhauner]

Issue Addressed

NA

Proposed Changes

Returns an (implicit) divide-by-zero error whenever total balances are zero. This is a non-substantive change since this code is unreachable due to total_active_balance always being involved in a std::cmp::max with spec.effective_balance_increment:

lighthouse/consensus/state_processing/src/per_epoch_processing/validator_statuses.rs

Lines 141 to 148 in 443793b

	// Generate a safe accessor for a balance in `TotalBalances`, as per spec `get_total_balance`.
	macro_rules! balance_accessor {
	($field_name:ident) => {
	pub fn $field_name(&self) -> u64 {
	std::cmp::max(self.effective_balance_increment, self.$field_name)
	}
	};
	}

lighthouse/consensus/types/src/beacon_state.rs

Lines 1194 to 1210 in 443793b

	/// Implementation of `get_total_balance`, matching the spec.
	///
	/// Returns minimum `EFFECTIVE_BALANCE_INCREMENT`, to avoid div by 0.
	pub fn get_total_balance(
	&self,
	validator_indices: &[usize],
	spec: &ChainSpec,
	) -> Result<u64, Error> {
	let total_balance = validator_indices.iter().try_fold(0_u64, |acc, i| {
	self.get_effective_balance(*i)
	.and_then(|bal| Ok(acc.safe_add(bal)?))
	})?;
	Ok(std::cmp::max(
	total_balance,
	spec.effective_balance_increment,
	))
	}

I can't see any actual issue with the current implementation, however it just seems safer to follow the spec in this scenario. Interested in thoughts.

Additional Info

References:

• Base: get_base_reward
• Altair: get_base_reward

[michaelsproul]

Seems like a change worth making. I think the if total_balance == 0 { } must be a relic from some ancient version of the spec

[paulhauner]

Seems like a change worth making. I think the if total_balance == 0 { } must be a relic from some ancient version of the spec

I agree. It passed CI so it's ready for review!

[michaelsproul]

Perfect.