Skip to content

[Issue 4023] Migrate altair validator constants #4058

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 3, 2021
Merged

[Issue 4023] Migrate altair validator constants #4058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a35097
Move TARGET_AGGREGATORS_PER_SYNC_SUBCOMMITTEE to ValidatorConstants
mbaxter Jun 3, 2021
4248094
Fix test
mbaxter Jun 3, 2021
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
3 changes: 2 additions & 1 deletion data/provider/src/main/java/tech/pegasys/teku/api/ConfigProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ public GetSpecResponse getConfig() {
"DOMAIN_CONTRIBUTION_AND_PROOF", Domain.CONTRIBUTION_AND_PROOF.toHexString());
configAttributes.put(
"TARGET_AGGREGATORS_PER_SYNC_SUBCOMMITTEE",
Integer.toString(altairConfig.getTargetAggregatorsPerSyncSubcommittee(), 10));
Integer.toString(
ValidatorConstants.TARGET_AGGREGATORS_PER_SYNC_SUBCOMMITTEE, 10));
});

return new GetSpecResponse(configAttributes);
Expand Down
13 changes: 2 additions & 11 deletions ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigAltair.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@

public class SpecConfigAltair extends DelegatingSpecConfig {

// Validator constants
private final int targetAggregatorsPerSyncSubcommittee = 4;

// Updated penalties
private final UInt64 inactivityPenaltyQuotientAltair;
private final int minSlashingPenaltyQuotientAltair;
Expand Down Expand Up @@ -114,10 +111,6 @@ public int getEpochsPerSyncCommitteePeriod() {
return epochsPerSyncCommitteePeriod;
}

public int getTargetAggregatorsPerSyncSubcommittee() {
return targetAggregatorsPerSyncSubcommittee;
}

public int getMinSyncCommitteeParticipants() {
return minSyncCommitteeParticipants;
}
Expand All @@ -142,8 +135,7 @@ public boolean equals(final Object o) {
&& minSyncCommitteeParticipants == that.minSyncCommitteeParticipants
&& Objects.equals(inactivityPenaltyQuotientAltair, that.inactivityPenaltyQuotientAltair)
&& Objects.equals(altairForkVersion, that.altairForkVersion)
&& Objects.equals(altairForkEpoch, that.altairForkEpoch)
&& targetAggregatorsPerSyncSubcommittee == that.targetAggregatorsPerSyncSubcommittee;
&& Objects.equals(altairForkEpoch, that.altairForkEpoch);
}

@Override
Expand All @@ -159,7 +151,6 @@ public int hashCode() {
epochsPerSyncCommitteePeriod,
altairForkVersion,
altairForkEpoch,
minSyncCommitteeParticipants,
targetAggregatorsPerSyncSubcommittee);
minSyncCommitteeParticipants);
}
}
2 changes: 2 additions & 0 deletions ethereum/spec/src/main/java/tech/pegasys/teku/spec/constants/ValidatorConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@ public class ValidatorConstants {
public static final int TARGET_AGGREGATORS_PER_COMMITTEE = 16;
public static final int RANDOM_SUBNETS_PER_VALIDATOR = 1;
public static final int EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION = 256;
// Altair
public static final int TARGET_AGGREGATORS_PER_SYNC_SUBCOMMITTEE = 4;
}
3 changes: 2 additions & 1 deletion ethereum/spec/src/main/java/tech/pegasys/teku/spec/logic/common/util/SyncCommitteeUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
import tech.pegasys.teku.spec.config.SpecConfigAltair;
import tech.pegasys.teku.spec.constants.Domain;
import tech.pegasys.teku.spec.constants.ValidatorConstants;
import tech.pegasys.teku.spec.datastructures.blocks.blockbody.versions.altair.BeaconBlockBodySchemaAltair;
import tech.pegasys.teku.spec.datastructures.blocks.blockbody.versions.altair.SyncAggregate;
import tech.pegasys.teku.spec.datastructures.blocks.blockbody.versions.altair.SyncAggregateSchema;
Expand Down Expand Up @@ -180,7 +181,7 @@ public boolean isSyncCommitteeAggregator(final BLSSignature signature) {
1,
specConfig.getSyncCommitteeSize()
/ SYNC_COMMITTEE_SUBNET_COUNT
/ specConfig.getTargetAggregatorsPerSyncSubcommittee());
/ ValidatorConstants.TARGET_AGGREGATORS_PER_SYNC_SUBCOMMITTEE);
return bytesToUInt64(Hash.sha2_256(signature.toSSZBytes()).slice(0, 8)).mod(modulo).isZero();
}

Expand Down
31 changes: 0 additions & 31 deletions .../spec/src/testFixtures/java/tech/pegasys/teku/spec/config/DelegatingSpecConfigAltair.java
View file
Open in desktop

This file was deleted.

26 changes: 8 additions & 18 deletions .../pegasys/teku/validator/client/duties/synccommittee/SyncCommitteeAggregationDutyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@
import tech.pegasys.teku.infrastructure.unsigned.UInt64;
import tech.pegasys.teku.spec.Spec;
import tech.pegasys.teku.spec.TestSpecFactory;
import tech.pegasys.teku.spec.config.DelegatingSpecConfigAltair;
import tech.pegasys.teku.spec.config.SpecConfig;
import tech.pegasys.teku.spec.config.SpecConfigAltair;
import tech.pegasys.teku.spec.config.TestConfigLoader;
import tech.pegasys.teku.spec.datastructures.operations.versions.altair.ContributionAndProof;
import tech.pegasys.teku.spec.datastructures.operations.versions.altair.SignedContributionAndProof;
Expand Down Expand Up @@ -68,7 +66,7 @@ class SyncCommitteeAggregationDutyTest {
private final BLSSignature nonAggregatorSignature =
BLSSignature.fromBytesCompressed(
Bytes.fromHexString(
"0xae401f767ab1917f925fe299ad51a57d52f7cc80deb1cc20fa2b3aa983e4e4d23056d79f01f3c97e29c8905da17e70e30c2a3f6bdd83dbc4ddf530e02e8f4d7ba22260e12e5f5fe7875b48e79660615b275597e87b2d33e076664b3da1737852"));
"0xce401f767ab1917f925fe299ad51a57d52f7cc80deb1cc20fa2b3aa983e4e4d23056d79f01f3c97e29c8905da17e70e30c2a3f6bdd83dbc4ddf530e02e8f4d7ba22260e12e5f5fe7875b48e79660615b275597e87b2d33e076664b3da1737852"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The modulus math changed when I removed the customized value for TARGET_AGGREGATORS_PER_SYNC_SUBCOMMITTEE - just tweaked this variable until I found a value that would fail the SyncCommitteeUtil.isSyncCommitteeAggregator() check.

private final BLSSignature aggregatorSignature =
BLSSignature.fromBytesCompressed(
Bytes.fromHexString(
Expand Down Expand Up @@ -97,21 +95,13 @@ void setUp() {
}

private static SpecConfig createSpecConfig() {
final SpecConfigAltair baseConfig =
TestConfigLoader.loadConfig(
"minimal",
modifier ->
modifier.altairBuilder(
altairModifier -> altairModifier.altairForkEpoch(UInt64.ZERO)))
.toVersionAltair()
.orElseThrow();

return new DelegatingSpecConfigAltair(baseConfig) {
@Override
public int getTargetAggregatorsPerSyncSubcommittee() {
return 1;
}
};
return TestConfigLoader.loadConfig(
"minimal",
modifier ->
modifier.altairBuilder(
altairModifier -> altairModifier.altairForkEpoch(UInt64.ZERO)))
.toVersionAltair()
.orElseThrow();
}

@Test
Expand Down