Electra alpha8 spec updates

[pawanjay176]

Issue Addressed

N/A

Proposed Changes

Implements changes for electra from https://github.com/ethereum/consensus-specs/releases/tag/v1.5.0-alpha.7 and https://github.com/ethereum/consensus-specs/releases/tag/v1.5.0-alpha.8

1. Fixed partial withdrawals count eip7251: Fix partial withdrawals count ethereum/consensus-specs#3943
2. Remove get_active_balance EIP-7251: Flatten get_active_balance ethereum/consensus-specs#3949
3. Remove queue_entire_balance_and_reset_validator Remove queue_entire_balance_and_reset_validator ethereum/consensus-specs#3951
4. Switch to compounding when consolidating with source==target eip7251: Switch to compounding when consolidating with source==target ethereum/consensus-specs#3918
5. Queue deposit requests and apply them during epoch processing eip6110: Queue deposit requests and apply them during epoch processing ethereum/consensus-specs#3818

Other changes from the releases are mostly cosmetic/ do not require changes on our end.
Can be reviewed per commit for each change. However, b28d010 contains bug fixes from all the changes

[pawanjay176]

Should we change the type to SignatureBytes for the other request types as well for consistency?

[ethDreamer]

curious why we did this?

[pawanjay176]

Because we need to allow for invalid bls signature bytes to be sent over the engine api. Otherwise, it becomes a deserialization error when we get the payload from the EL instead of an invalid deposit during processing

[pawanjay176]

Great find 🙌 Turns out there was a spec test to catch this specific case, but it was written incorrectly. This has been fixed in the linked consensus specs PR above

[michaelsproul]

Nice! Thanks for following that up!

[michaelsproul]

Some of the tests seem to be failing due to builder stuff. Did you mean to include mock-builder changes in this PR?

[pawanjay176]

Ahh I didn't realise I pushed the mock builder changes to this PR. I'll revert them and fix the tests.
Will make a separate PR for the mock builder changes

[michaelsproul]

I think I've fixed another consensus bug in this commit:

• ebabd0f

The spec iterates over post.validators, but we were iterating over a clone of validators taken prior to modifying post.validators in the pre_activation loop. So if there are any validators that are pre-activation with compounding withdrawal credentials, we would double credit them by pushing a PendingDeposit in the pre_activation loop here:

lighthouse/consensus/state_processing/src/upgrade/electra.rs

Lines 68 to 76 in ebabd0f

	post.pending_deposits_mut()?
	.push(PendingDeposit {
	pubkey,
	withdrawal_credentials,
	amount: balance_copy,
	signature: Signature::infinity()?.into(),
	slot: spec.genesis_slot,
	})
	.map_err(Error::MilhouseError)?;

And then another deposit if they meet the conditions has_compounding_withdrawal_credential (true) and balance > spec.min_activation_balance (true):

lighthouse/consensus/state_processing/src/upgrade/electra.rs

Lines 81 to 85 in ebabd0f

	for (index, validator) in validators.iter().enumerate() {
	if validator.has_compounding_withdrawal_credential(spec) {
	post.queue_excess_active_balance(index, spec)?;
	}
	}

lighthouse/consensus/types/src/beacon_state.rs

Lines 2170 to 2192 in ebabd0f

	pub fn queue_excess_active_balance(
	&mut self,
	validator_index: usize,
	spec: &ChainSpec,
	) -> Result<(), Error> {
	let balance = self
	.balances_mut()
	.get_mut(validator_index)
	.ok_or(Error::UnknownValidator(validator_index))?;
	if *balance > spec.min_activation_balance {
	let excess_balance = balance.safe_sub(spec.min_activation_balance)?;
	*balance = spec.min_activation_balance;
	let validator = self.get_validator(validator_index)?.clone();
	self.pending_deposits_mut()?.push(PendingDeposit {
	pubkey: validator.pubkey,
	withdrawal_credentials: validator.withdrawal_credentials,
	amount: excess_balance,
	signature: Signature::infinity()?.into(),
	slot: spec.genesis_slot,
	})?;
	}
	Ok(())
	}

Additionally, in this case, the balance of the validator would be set to spec.min_activation_balance rather than 0 (the correct value). So their total balance including deposits would end up being balance (first deposit) + excess_balance (second deposit) + min_activation_balance (state) = 2 * balance.

[michaelsproul]

I also simplified some sorting in this commit (non-semantic change):

• 75efd4f

[michaelsproul]

Looks good. Let's merge this so we can get other Electra PRs into unstable on top

[pawanjay176]

Great find with the other accounting bug as well! 🙏 I looked through it and also agree with your solution.

[michaelsproul]

@mergify queue

[mergify]

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

[mergify]

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

• ☑️ test-suite-success

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

[michaelsproul]

@mergify requeue

[mergify]

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at d74b2d9

[michaelsproul]

Yay!