Skip to content
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.

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

feat: switch to compounding from consolidation requests #7122

Merged
merged 8 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
2 changes: 1 addition & 1 deletion packages/beacon-node/test/spec/specTestVersioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {DownloadTestsOptions} from "@lodestar/spec-test-util/downloadTests";
const __dirname = path.dirname(fileURLToPath(import.meta.url));

export const ethereumConsensusSpecsTests: DownloadTestsOptions = {
specVersion: "v1.5.0-alpha.6",
specVersion: "v1.5.0-alpha.7",
// Target directory is the host package root: 'packages/*/spec-tests'
outputDir: path.join(__dirname, "../../spec-tests"),
specTestsRepoUrl: "https://github.com/ethereum/consensus-spec-tests",
Expand Down
82 changes: 71 additions & 11 deletions packages/state-transition/src/block/processConsolidationRequest.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,41 @@
import {electra, ssz} from "@lodestar/types";
import {FAR_FUTURE_EPOCH, MIN_ACTIVATION_BALANCE, PENDING_CONSOLIDATIONS_LIMIT} from "@lodestar/params";

import {toHex} from "@lodestar/utils";
import {CachedBeaconStateElectra} from "../types.js";
import {getConsolidationChurnLimit, isActiveValidator} from "../util/validator.js";
import {hasExecutionWithdrawalCredential} from "../util/electra.js";
import {hasExecutionWithdrawalCredential, switchToCompoundingValidator} from "../util/electra.js";
import {computeConsolidationEpochAndUpdateChurn} from "../util/epoch.js";
import {hasEth1WithdrawalCredential} from "../util/capella.js";

// TODO Electra: Clean up necessary as there is a lot of overlap with isValidSwitchToCompoundRequest
export function processConsolidationRequest(
state: CachedBeaconStateElectra,
consolidationRequest: electra.ConsolidationRequest
): void {
// If the pending consolidations queue is full, consolidation requests are ignored
if (state.pendingConsolidations.length >= PENDING_CONSOLIDATIONS_LIMIT) {
const {sourcePubkey, targetPubkey, sourceAddress} = consolidationRequest;
const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey);
const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey);

if (sourceIndex === null || targetIndex === null) {
return;
}

// If there is too little available consolidation churn limit, consolidation requests are ignored
if (getConsolidationChurnLimit(state.epochCtx) <= MIN_ACTIVATION_BALANCE) {
if (isValidSwitchToCompoundRequest(state, consolidationRequest)) {
switchToCompoundingValidator(state, sourceIndex);
// Early return since we have already switched validator to compounding
return;
}

const {sourcePubkey, targetPubkey} = consolidationRequest;
const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey);
const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey);

if (sourceIndex === null || targetIndex === null) {
// If the pending consolidations queue is full, consolidation requests are ignored
if (state.pendingConsolidations.length >= PENDING_CONSOLIDATIONS_LIMIT) {
return;
}

// If there is too little available consolidation churn limit, consolidation requests are ignored
if (getConsolidationChurnLimit(state.epochCtx) <= MIN_ACTIVATION_BALANCE) {
return;
}
// Verify that source != target, so a consolidation cannot be used as an exit.
if (sourceIndex === targetIndex) {
return;
Expand All @@ -46,7 +54,7 @@ export function processConsolidationRequest(
return;
}

if (Buffer.compare(sourceWithdrawalAddress, consolidationRequest.sourceAddress) !== 0) {
if (Buffer.compare(sourceWithdrawalAddress, sourceAddress) !== 0) {
return;
}

Expand All @@ -70,4 +78,56 @@ export function processConsolidationRequest(
targetIndex,
});
state.pendingConsolidations.push(pendingConsolidation);

// Churn any target excess active balance of target and raise its max
if (hasEth1WithdrawalCredential(targetValidator.withdrawalCredentials)) {
switchToCompoundingValidator(state, targetIndex);
}
}

/**
* Determine if we should set consolidation target validator to compounding credential
*/
function isValidSwitchToCompoundRequest(
state: CachedBeaconStateElectra,
consolidationRequest: electra.ConsolidationRequest
): boolean {
const {sourcePubkey, targetPubkey, sourceAddress} = consolidationRequest;
const sourceIndex = state.epochCtx.getValidatorIndex(sourcePubkey);
Copy link
Member

@nflaig nflaig Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could save the index lookup if we compare the pubkeys first, not sure if it's worth though but would match more closely to the spec

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could save the index lookup if we compare the pubkeys first, not sure if it's worth though but would match more closely to the spec

Let's do the optimization later. This part of spec is sub-optimized and will very likely to be cleaned up in alpha 9 or 10. We can optimize when the spec gets refactored.

const targetIndex = state.epochCtx.getValidatorIndex(targetPubkey);

// Verify pubkey exists
if (sourceIndex === null) {
return false;
}

// Switch to compounding requires source and target be equal
if (sourceIndex !== targetIndex) {
return false;
}

const sourceValidator = state.validators.getReadonly(sourceIndex);
const addressStr = toHex(sourceValidator.withdrawalCredentials.subarray(12));
ensi321 marked this conversation as resolved.
Show resolved Hide resolved
const sourceAddressStr = toHex(sourceAddress);
// Verify request has been authorized
if (addressStr !== sourceAddressStr) {
return false;
}

// Verify source withdrawal credentials
if (!hasEth1WithdrawalCredential(sourceValidator.withdrawalCredentials)) {
return false;
}

// Verify the source is active
if (!isActiveValidator(sourceValidator, state.epochCtx.epoch)) {
return false;
}

// Verify exit for source has not been initiated
if (sourceValidator.exitEpoch !== FAR_FUTURE_EPOCH) {
return false;
}

return true;
}
17 changes: 1 addition & 16 deletions packages/state-transition/src/block/processDeposit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,7 @@ import {DepositData} from "@lodestar/types/lib/phase0/types.js";
import {DepositRequest} from "@lodestar/types/lib/electra/types.js";
import {BeaconConfig} from "@lodestar/config";
import {ZERO_HASH} from "../constants/index.js";
import {
computeDomain,
computeSigningRoot,
hasCompoundingWithdrawalCredential,
hasEth1WithdrawalCredential,
increaseBalance,
switchToCompoundingValidator,
} from "../util/index.js";
import {computeDomain, computeSigningRoot, increaseBalance} from "../util/index.js";
import {CachedBeaconStateAllForks, CachedBeaconStateAltair, CachedBeaconStateElectra} from "../types.js";

/**
Expand Down Expand Up @@ -80,14 +73,6 @@ export function applyDeposit(
amount: BigInt(amount),
});
stateElectra.pendingBalanceDeposits.push(pendingBalanceDeposit);

if (
hasCompoundingWithdrawalCredential(withdrawalCredentials) &&
hasEth1WithdrawalCredential(validators.getReadonly(cachedIndex).withdrawalCredentials) &&
isValidDepositSignature(config, pubkey, withdrawalCredentials, amount, deposit.signature)
) {
switchToCompoundingValidator(stateElectra, cachedIndex);
}
}
}
}
Expand Down
8 changes: 0 additions & 8 deletions packages/state-transition/src/cache/epochTransitionCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,6 @@ export interface EpochTransitionCache {
*/
validators: phase0.Validator[];

/**
* This is for electra only
* Validators that're switched to compounding during processPendingConsolidations(), not available in beforeProcessEpoch()
*/
newCompoundingValidators?: Set<ValidatorIndex>;

/**
* balances array will be populated by processRewardsAndPenalties() and consumed by processEffectiveBalanceUpdates().
* processRewardsAndPenalties() already has a regular Javascript array of balances.
Expand Down Expand Up @@ -518,8 +512,6 @@ export function beforeProcessEpoch(
inclusionDelays,
flags,
validators,
// will be assigned in processPendingConsolidations()
newCompoundingValidators: undefined,
// Will be assigned in processRewardsAndPenalties()
balances: undefined,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export function processEffectiveBalanceUpdates(
// so it's recycled here for performance.
const balances = cache.balances ?? state.balances.getAll();
const currentEpochValidators = cache.validators;
const newCompoundingValidators = cache.newCompoundingValidators ?? new Set();

let numUpdate = 0;
for (let i = 0, len = balances.length; i < len; i++) {
Expand All @@ -61,9 +60,9 @@ export function processEffectiveBalanceUpdates(
effectiveBalanceLimit = MAX_EFFECTIVE_BALANCE;
} else {
// from electra, effectiveBalanceLimit is per validator
const isCompoundingValidator =
hasCompoundingWithdrawalCredential(currentEpochValidators[i].withdrawalCredentials) ||
newCompoundingValidators.has(i);
const isCompoundingValidator = hasCompoundingWithdrawalCredential(
currentEpochValidators[i].withdrawalCredentials
);
effectiveBalanceLimit = isCompoundingValidator ? MAX_EFFECTIVE_BALANCE_ELECTRA : MIN_ACTIVATION_BALANCE;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import {ValidatorIndex} from "@lodestar/types";
import {CachedBeaconStateElectra, EpochTransitionCache} from "../types.js";
import {decreaseBalance, increaseBalance} from "../util/balance.js";
import {getActiveBalance} from "../util/validator.js";
import {switchToCompoundingValidator} from "../util/electra.js";

/**
* Starting from Electra:
Expand All @@ -22,7 +20,6 @@ export function processPendingConsolidations(state: CachedBeaconStateElectra, ca
let nextPendingConsolidation = 0;
const validators = state.validators;
const cachedBalances = cache.balances;
const newCompoundingValidators = new Set<ValidatorIndex>();

for (const pendingConsolidation of state.pendingConsolidations.getAllReadonly()) {
const {sourceIndex, targetIndex} = pendingConsolidation;
Expand All @@ -36,9 +33,6 @@ export function processPendingConsolidations(state: CachedBeaconStateElectra, ca
if (sourceValidator.withdrawableEpoch > nextEpoch) {
break;
}
// Churn any target excess active balance of target and raise its max
switchToCompoundingValidator(state, targetIndex);
newCompoundingValidators.add(targetIndex);
// Move active balance to target. Excess balance is withdrawable.
const activeBalance = getActiveBalance(state, sourceIndex);
decreaseBalance(state, sourceIndex, activeBalance);
Expand All @@ -51,6 +45,5 @@ export function processPendingConsolidations(state: CachedBeaconStateElectra, ca
nextPendingConsolidation++;
}

cache.newCompoundingValidators = newCompoundingValidators;
state.pendingConsolidations = state.pendingConsolidations.sliceFrom(nextPendingConsolidation);
}
16 changes: 7 additions & 9 deletions packages/state-transition/src/util/electra.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ export function hasExecutionWithdrawalCredential(withdrawalCredentials: Uint8Arr
export function switchToCompoundingValidator(state: CachedBeaconStateElectra, index: ValidatorIndex): void {
const validator = state.validators.get(index);

if (hasEth1WithdrawalCredential(validator.withdrawalCredentials)) {
// directly modifying the byte leads to ssz missing the modification resulting into
// wrong root compute, although slicing can be avoided but anyway this is not going
// to be a hot path so its better to clean slice and avoid side effects
const newWithdrawalCredentials = validator.withdrawalCredentials.slice();
newWithdrawalCredentials[0] = COMPOUNDING_WITHDRAWAL_PREFIX;
validator.withdrawalCredentials = newWithdrawalCredentials;
queueExcessActiveBalance(state, index);
}
// directly modifying the byte leads to ssz missing the modification resulting into
// wrong root compute, although slicing can be avoided but anyway this is not going
// to be a hot path so its better to clean slice and avoid side effects
const newWithdrawalCredentials = validator.withdrawalCredentials.slice();
newWithdrawalCredentials[0] = COMPOUNDING_WITHDRAWAL_PREFIX;
validator.withdrawalCredentials = newWithdrawalCredentials;
queueExcessActiveBalance(state, index);
}

export function queueExcessActiveBalance(state: CachedBeaconStateElectra, index: ValidatorIndex): void {
Expand Down
Loading