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

Change consolidated balance calculation + small refactor #8908

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,7 @@ public class ReferenceTestFinder {
Path.of("src", "referenceTest", "resources", "consensus-spec-tests", "tests");
private static final List<String> SUPPORTED_FORKS =
List.of(
TestFork.PHASE0,
TestFork.ALTAIR,
TestFork.BELLATRIX,
TestFork.CAPELLA,
TestFork.DENEB,
TestFork.ELECTRA);
TestFork.PHASE0, TestFork.ALTAIR, TestFork.BELLATRIX, TestFork.CAPELLA, TestFork.DENEB);

@MustBeClosed
public static Stream<TestDefinition> findReferenceTests() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,4 @@ public class WithdrawalPrefixes {
public static final byte ETH1_ADDRESS_WITHDRAWAL_BYTE = 0x01;
public static final byte COMPOUNDING_WITHDRAWAL_BYTE = 0x02;
public static final Bytes ETH1_ADDRESS_WITHDRAWAL_PREFIX = Bytes.of(ETH1_ADDRESS_WITHDRAWAL_BYTE);
public static final Bytes COMPOUNDING_WITHDRAWAL_PREFIX = Bytes.of(COMPOUNDING_WITHDRAWAL_BYTE);
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,22 +148,6 @@ public Supplier<ValidatorExitContext> createValidatorExitContextSupplier(
return Suppliers.memoize(() -> createValidatorExitContext(state));
}

/**
* <a
* href="https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#deposits">add_validator_to_registry</a>
*/
public void addValidatorToRegistry(
final MutableBeaconState state,
final BLSPublicKey pubkey,
final Bytes32 withdrawalCredentials,
final UInt64 amount) {
final Validator validator =
miscHelpers.getValidatorFromDeposit(pubkey, withdrawalCredentials, amount);
LOG.debug("Adding new validator with index {} to state", state.getValidators().size());
state.getValidators().append(validator);
state.getBalances().appendElement(amount);
}

/**
* This function implements an optimized version of exitQueueEpoch and exitQueueChurn calculation,
* compared to the `initiate_validator_exit` reference implementation.
Expand Down Expand Up @@ -239,6 +223,22 @@ public void setExitQueueChurn(final UInt64 exitQueueChurn) {
}
}

/**
* <a
* href="https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#deposits">add_validator_to_registry</a>
*/
public void addValidatorToRegistry(
final MutableBeaconState state,
final BLSPublicKey pubkey,
final Bytes32 withdrawalCredentials,
final UInt64 amount) {
final Validator validator =
miscHelpers.getValidatorFromDeposit(pubkey, withdrawalCredentials, amount);
LOG.debug("Adding new validator with index {} to state", state.getValidators().size());
state.getValidators().append(validator);
state.getBalances().appendElement(amount);
}

public void slashValidator(
final MutableBeaconState state,
final int slashedIndex,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,38 @@ private Bytes32 computeForkDataRoot(
return new ForkData(currentVersion, genesisValidatorsRoot).hashTreeRoot();
}

/** is_valid_deposit_signature */
public boolean isValidDepositSignature(
final BLSPublicKey pubkey,
final Bytes32 withdrawalCredentials,
final UInt64 amount,
final BLSSignature signature) {
try {
return depositSignatureVerifier.verify(
pubkey, computeDepositSigningRoot(pubkey, withdrawalCredentials, amount), signature);
} catch (final BlsException e) {
return false;
}
}

/** get_validator_from_deposit */
public Validator getValidatorFromDeposit(
final BLSPublicKey pubkey, final Bytes32 withdrawalCredentials, final UInt64 amount) {
final UInt64 effectiveBalance =
amount
.minus(amount.mod(specConfig.getEffectiveBalanceIncrement()))
.min(specConfig.getMaxEffectiveBalance());
return new Validator(
pubkey,
withdrawalCredentials,
effectiveBalance,
false,
FAR_FUTURE_EPOCH,
FAR_FUTURE_EPOCH,
FAR_FUTURE_EPOCH,
FAR_FUTURE_EPOCH);
}

public boolean isMergeTransitionComplete(final BeaconState state) {
return false;
}
Expand Down Expand Up @@ -414,38 +446,6 @@ public boolean isFormerDepositMechanismDisabled(final BeaconState state) {
return false;
}

/** is_valid_deposit_signature */
public boolean isValidDepositSignature(
final BLSPublicKey pubkey,
final Bytes32 withdrawalCredentials,
final UInt64 amount,
final BLSSignature signature) {
try {
return depositSignatureVerifier.verify(
pubkey, computeDepositSigningRoot(pubkey, withdrawalCredentials, amount), signature);
} catch (final BlsException e) {
return false;
}
}

/** get_validator_from_deposit */
public Validator getValidatorFromDeposit(
final BLSPublicKey pubkey, final Bytes32 withdrawalCredentials, final UInt64 amount) {
final UInt64 effectiveBalance =
amount
.minus(amount.mod(specConfig.getEffectiveBalanceIncrement()))
.min(specConfig.getMaxEffectiveBalance());
return new Validator(
pubkey,
withdrawalCredentials,
effectiveBalance,
false,
FAR_FUTURE_EPOCH,
FAR_FUTURE_EPOCH,
FAR_FUTURE_EPOCH,
FAR_FUTURE_EPOCH);
}

public Optional<MiscHelpersDeneb> toVersionDeneb() {
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ protected void processOperationsNoValidation(
final ExecutionRequests executionRequests =
BeaconBlockBodyElectra.required(body).getExecutionRequests();

this.processDepositRequests(state, executionRequests.getDeposits());
this.processWithdrawalRequests(
processDepositRequests(state, executionRequests.getDeposits());
processWithdrawalRequests(
state, executionRequests.getWithdrawals(), validatorExitContextSupplier);
this.processConsolidationRequests(state, executionRequests.getConsolidations());
processConsolidationRequests(state, executionRequests.getConsolidations());
});
}

Expand Down Expand Up @@ -210,6 +210,36 @@ public void processWithdrawals(
specConfigElectra);
}

/*
Implements process_deposit_request from consensus-specs (EIP-6110)
*/
@Override
public void processDepositRequests(
final MutableBeaconState state, final List<DepositRequest> depositRequests) {
final MutableBeaconStateElectra electraState = MutableBeaconStateElectra.required(state);
final SszMutableList<PendingDeposit> pendingDeposits =
MutableBeaconStateElectra.required(state).getPendingDeposits();
for (DepositRequest depositRequest : depositRequests) {
// process_deposit_request
if (electraState
.getDepositRequestsStartIndex()
.equals(SpecConfigElectra.UNSET_DEPOSIT_REQUESTS_START_INDEX)) {
electraState.setDepositRequestsStartIndex(depositRequest.getIndex());
}

final PendingDeposit deposit =
schemaDefinitionsElectra
.getPendingDepositSchema()
.create(
new SszPublicKey(depositRequest.getPubkey()),
SszBytes32.of(depositRequest.getWithdrawalCredentials()),
SszUInt64.of(depositRequest.getAmount()),
new SszSignature(depositRequest.getSignature()),
SszUInt64.of(state.getSlot()));
pendingDeposits.append(deposit);
}
}

/** Implements process_withdrawal_request from consensus-specs (EIP-7002 & EIP-7251). */
@Override
public void processWithdrawalRequests(
Expand Down Expand Up @@ -360,36 +390,6 @@ public void processWithdrawalRequests(
});
}

/*
Implements process_deposit_request from consensus-specs (EIP-6110)
*/
Comment on lines -363 to -365
Copy link
Contributor

Choose a reason for hiding this comment

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

moving all of these fns does make it a lot harder to determine if there were changes in githubs ui...

@Override
public void processDepositRequests(
final MutableBeaconState state, final List<DepositRequest> depositRequests) {
final MutableBeaconStateElectra electraState = MutableBeaconStateElectra.required(state);
final SszMutableList<PendingDeposit> pendingDeposits =
MutableBeaconStateElectra.required(state).getPendingDeposits();
for (DepositRequest depositRequest : depositRequests) {
// process_deposit_request
if (electraState
.getDepositRequestsStartIndex()
.equals(SpecConfigElectra.UNSET_DEPOSIT_REQUESTS_START_INDEX)) {
electraState.setDepositRequestsStartIndex(depositRequest.getIndex());
}

final PendingDeposit deposit =
schemaDefinitionsElectra
.getPendingDepositSchema()
.create(
new SszPublicKey(depositRequest.getPubkey()),
SszBytes32.of(depositRequest.getWithdrawalCredentials()),
SszUInt64.of(depositRequest.getAmount()),
new SszSignature(depositRequest.getSignature()),
SszUInt64.of(state.getSlot()));
pendingDeposits.append(deposit);
}
}

/**
* Implements process_consolidation_request from consensus-spec (EIP-7251)
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import tech.pegasys.teku.spec.config.SpecConfig;
import tech.pegasys.teku.spec.config.SpecConfigDeneb;
import tech.pegasys.teku.spec.config.SpecConfigElectra;
import tech.pegasys.teku.spec.datastructures.state.Validator;
import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState;
import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.electra.BeaconStateElectra;
import tech.pegasys.teku.spec.datastructures.state.versions.electra.PendingPartialWithdrawal;
Expand Down Expand Up @@ -52,19 +51,6 @@ public UInt64 getActivationExitChurnLimit(final BeaconStateElectra state) {
return getBalanceChurnLimit(state).min(configElectra.getMaxPerEpochActivationExitChurnLimit());
}

/**
* get_active_balance
*
* @param state The state to get the effective balance from
* @param validatorIndex the index of the validator
*/
public UInt64 getActiveBalance(final BeaconState state, final int validatorIndex) {
final Validator validator = state.getValidators().get(validatorIndex);
final UInt64 maxEffectiveBalance = miscHelpers.getMaxEffectiveBalance(validator);
final UInt64 validatorBalance = state.getBalances().get(validatorIndex).get();
return validatorBalance.min(maxEffectiveBalance);
}

/**
* get_pending_balance_to_withdraw
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,6 @@ public int computeProposerIndex(
SpecConfigElectra.required(specConfig).getMaxEffectiveBalanceElectra());
}

@Override
public UInt64 getMaxEffectiveBalance(final Validator validator) {
return predicatesElectra.hasCompoundingWithdrawalCredential(validator)
? specConfigElectra.getMaxEffectiveBalanceElectra()
: specConfigElectra.getMinActivationBalance();
}

@Override
public Validator getValidatorFromDeposit(
final BLSPublicKey pubkey, final Bytes32 withdrawalCredentials, final UInt64 amount) {
Expand All @@ -99,8 +92,10 @@ public Validator getValidatorFromDeposit(
}

@Override
public Optional<MiscHelpersElectra> toVersionElectra() {
return Optional.of(this);
public UInt64 getMaxEffectiveBalance(final Validator validator) {
return predicatesElectra.hasCompoundingWithdrawalCredential(validator)
? specConfigElectra.getMaxEffectiveBalanceElectra()
: specConfigElectra.getMinActivationBalance();
}

@Override
Expand All @@ -112,4 +107,9 @@ public boolean isFormerDepositMechanismDisabled(final BeaconState state) {
.getEth1DepositIndex()
.equals(BeaconStateElectra.required(state).getDepositRequestsStartIndex());
}

@Override
public Optional<MiscHelpersElectra> toVersionElectra() {
return Optional.of(this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,18 @@ public void processPendingConsolidations(final MutableBeaconState state) {
break;
}

final UInt64 activeBalance =
stateAccessorsElectra.getActiveBalance(state, pendingConsolidation.getSourceIndex());
// Calculate the consolidated balance
final UInt64 sourceEffectiveBalance =
state
.getBalances()
.get(pendingConsolidation.getSourceIndex())
.get()
.min(sourceValidator.getEffectiveBalance());
// Move active balance to target. Excess balance is withdrawable.
beaconStateMutators.decreaseBalance(
state, pendingConsolidation.getSourceIndex(), activeBalance);
state, pendingConsolidation.getSourceIndex(), sourceEffectiveBalance);
beaconStateMutators.increaseBalance(
state, pendingConsolidation.getTargetIndex(), activeBalance);
state, pendingConsolidation.getTargetIndex(), sourceEffectiveBalance);
Comment on lines -325 to +336
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make it easier for others to review. These are the spec changes:

image

And these changes track those properly 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

yep next time i think this can be a PR and refactor can be separate for sanity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, noted for next time.


nextPendingBalanceConsolidation++;
}
Expand Down
Loading