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

teku-6320 Capella helper functions #6403

Merged
merged 2 commits into from
Nov 9, 2022

Conversation

lucassaldanha
Copy link
Member

@lucassaldanha lucassaldanha commented Nov 8, 2022

PR Description

Implemented helper predicate functions:

  • has_eth1_withdrawal_credential
  • has_eth1_withdrawal_credential
  • is_partially_withdrawable_validator

Reference: Cappella beacon chain spec

Fixed Issue(s)

fixes #6320

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@lucassaldanha lucassaldanha marked this pull request as ready for review November 9, 2022 00:27
Comment on lines +47 to +59
@Test
public void isFullyWithdrawableValidatorShouldReturnTrueForEligibleValidator() {
final Validator validator =
dataStructureUtil
.randomValidator()
.withWithdrawalCredentials(eth1WithdrawalsCredentials)
.withWithdrawableEpoch(UInt64.ZERO);

final UInt64 balance = UInt64.ONE;
final UInt64 epoch = UInt64.ONE;

assertThat(miscHelpersCapella.isFullyWithdrawableValidator(validator, balance, epoch)).isTrue();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this does look like an excellent use case for @ParameterizedTest
if we drove it via balance, epoch, withdrawableEpoch, expectation, then you'd have 1 set of code...

Comment on lines +1595 to +1598
public Bytes32 randomEth1WithdrawalCredentials() {
return Bytes32.wrap(
Bytes.concatenate(Bytes.fromHexString("0x01"), randomBytes32().slice(0, 31)));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

eth1 withdrawal credentials are slightly different...

  private static final Bytes ETH1_WITHDRAWAL_PREFIX = Bytes.fromHexString("0x01");
  private static final Bytes WITHDRAWAL_BUFFER = Bytes.repeat((byte) 0x00, 11);
  public static Bytes32 withdrawalAddressFromEth1Address(final Bytes20 toExecutionAddress) {
    return Bytes32.wrap(
        Bytes.concatenate(
            ETH1_WITHDRAWAL_PREFIX, WITHDRAWAL_BUFFER, toExecutionAddress.getWrappedBytes()));
  }

above is a snippet from my current work, where I'll create an eth1 address, we can probably migrate this into helpers after, but maybe for this one just make the Bytes32.wrap(Bytes.fromHexString("0x010000000000000000000000"), randomEth1Address())) or something...

Copy link
Contributor

@rolfyone rolfyone left a comment

Choose a reason for hiding this comment

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

some nits, overall lgtm, and i disagree with the codeQL... i'm not sure what it was scanning...

import tech.pegasys.teku.spec.datastructures.state.Validator;
import tech.pegasys.teku.spec.util.DataStructureUtil;

class MiscHelpersCapellaTest {

Check notice

Code scanning / CodeQL

Unused classes and interfaces

Unused class: MiscHelpersCapellaTest is not referenced within this codebase. If not used as an external API it should be removed.
@lucassaldanha lucassaldanha enabled auto-merge (squash) November 9, 2022 01:58
@lucassaldanha lucassaldanha merged commit 902e20e into Consensys:master Nov 9, 2022
@lucassaldanha lucassaldanha deleted the capella-helpers branch November 9, 2022 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Capella Helpers functions
2 participants