From ec069827c01ae91d2b66dd03700f52fe9afee17d Mon Sep 17 00:00:00 2001 From: Dmitrii Shmatko Date: Wed, 9 Oct 2024 22:06:27 +0200 Subject: [PATCH 01/10] Interface preview --- .../registry/AbstractSchemaProvider.java | 111 +++++++++++------- .../registry/AttestationSchemaProvider.java | 49 +++----- .../AttnetsENRFieldSchemaProvider.java | 24 +--- .../ExecutionPayloadHeaderSchemaProvider.java | 57 +++++++++ .../spec/schemas/registry/SchemaTypes.java | 11 ++ 5 files changed, 157 insertions(+), 95 deletions(-) create mode 100644 ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/ExecutionPayloadHeaderSchemaProvider.java diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java index d7e023b76d5..2571f7ef189 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java @@ -15,69 +15,65 @@ import static com.google.common.base.Preconditions.checkArgument; -import java.util.Map; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; import java.util.NavigableMap; +import java.util.Optional; +import java.util.Set; import java.util.TreeMap; +import java.util.function.BiFunction; +import org.apache.commons.lang3.tuple.Triple; import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; abstract class AbstractSchemaProvider implements SchemaProvider { - private final NavigableMap milestoneToEffectiveMilestone = - new TreeMap<>(); + private final NavigableMap> + milestoneToSchemaCreator = new TreeMap<>(); private final SchemaId schemaId; - protected AbstractSchemaProvider(final SchemaId schemaId) { + protected AbstractSchemaProvider( + final SchemaId schemaId, + Triple, BiFunction>... + milestoneCreators) { this.schemaId = schemaId; - } - - protected void addMilestoneMapping( - final SpecMilestone milestone, final SpecMilestone untilMilestone) { + final List< + Triple< + SpecMilestone, Optional, BiFunction>> + creatorsList = Arrays.stream(milestoneCreators).toList(); + checkArgument(!creatorsList.isEmpty(), "There should be at least 1 creator"); checkArgument( - untilMilestone.isGreaterThanOrEqualTo(milestone), - "%s must be earlier then or equal to %s", - milestone, - untilMilestone); - - checkOverlappingVersionMappings(milestone, untilMilestone); - - SpecMilestone currentMilestone = untilMilestone; - while (currentMilestone.isGreaterThan(milestone)) { - milestoneToEffectiveMilestone.put(currentMilestone, milestone); - currentMilestone = currentMilestone.getPreviousMilestone(); - } - } + creatorsList.size() + == new HashSet<>(creatorsList.stream().map(Triple::getLeft).toList()).size(), + "There shouldn't be duplicated milestones in boundary start for creator, but were: %s", + creatorsList.stream().map(Triple::getLeft).toList()); + BiFunction lastCreator = null; + // TODO: add optional right bound SpecMilestone checks and logic + for (SpecMilestone specMilestone : SpecMilestone.values()) { + if (creatorsList.getFirst().getLeft() == specMilestone) { + lastCreator = creatorsList.removeFirst().getRight(); + milestoneToSchemaCreator.put(specMilestone, lastCreator); + } else { + milestoneToSchemaCreator.put(specMilestone, lastCreator); + } - private void checkOverlappingVersionMappings( - final SpecMilestone milestone, final SpecMilestone untilMilestone) { - final Map.Entry floorEntry = - milestoneToEffectiveMilestone.floorEntry(untilMilestone); - if (floorEntry != null && floorEntry.getValue().isGreaterThanOrEqualTo(milestone)) { - throw new IllegalArgumentException( - String.format( - "Milestone %s is already mapped to %s", - floorEntry.getKey(), getEffectiveMilestone(floorEntry.getValue()))); - } - final Map.Entry ceilingEntry = - milestoneToEffectiveMilestone.ceilingEntry(milestone); - if (ceilingEntry != null && ceilingEntry.getKey().isLessThanOrEqualTo(untilMilestone)) { - throw new IllegalArgumentException( - String.format( - "Milestone %s is already mapped to %s", - ceilingEntry.getKey(), getEffectiveMilestone(ceilingEntry.getValue()))); + if (!creatorsList.isEmpty()) { + throw new IllegalArgumentException("Overlapping creators detected: " + creatorsList); + } } } + // TODO: not needed @Override public SpecMilestone getEffectiveMilestone(final SpecMilestone milestone) { - return milestoneToEffectiveMilestone.getOrDefault(milestone, milestone); + return milestone; } @Override public T getSchema(final SchemaRegistry registry) { final SpecMilestone milestone = registry.getMilestone(); - final SpecMilestone effectiveMilestone = getEffectiveMilestone(milestone); - return createSchema(registry, effectiveMilestone, registry.getSpecConfig()); + return createSchema(registry, milestone, registry.getSpecConfig()); } @Override @@ -85,6 +81,35 @@ public SchemaId getSchemaId() { return schemaId; } - protected abstract T createSchema( - SchemaRegistry registry, SpecMilestone effectiveMilestone, SpecConfig specConfig); + static + Triple, BiFunction> + milestoneSchema( + SpecMilestone milestone, BiFunction creationSchema) { + return Triple.of(milestone, Optional.empty(), creationSchema); + } + + static + Triple, BiFunction> + milestoneSchema( + SpecMilestone milestone, + SpecMilestone untilMilestone, + BiFunction creationSchema) { + return Triple.of(milestone, Optional.of(untilMilestone), creationSchema); + } + + protected T createSchema( + SchemaRegistry registry, SpecMilestone effectiveMilestone, SpecConfig specConfig) { + final BiFunction maybeSchemaCreator = + milestoneToSchemaCreator.get(effectiveMilestone); + if (maybeSchemaCreator == null) { + throw new IllegalArgumentException( + "It is not supposed to create a specific version for " + effectiveMilestone); + } + return maybeSchemaCreator.apply(registry, specConfig); + } + + @Override + public Set getSupportedMilestones() { + return milestoneToSchemaCreator.navigableKeySet(); + } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java index 120d9d5ecbd..84903b549ea 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java @@ -14,13 +14,9 @@ package tech.pegasys.teku.spec.schemas.registry; import static tech.pegasys.teku.spec.SpecMilestone.DENEB; -import static tech.pegasys.teku.spec.SpecMilestone.ELECTRA; import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.ATTESTATION_SCHEMA; -import java.util.Set; -import tech.pegasys.teku.spec.SpecMilestone; -import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationSchema; import tech.pegasys.teku.spec.datastructures.operations.versions.electra.AttestationElectraSchema; @@ -28,36 +24,21 @@ public class AttestationSchemaProvider extends AbstractSchemaProvider> { - public AttestationSchemaProvider() { - super(ATTESTATION_SCHEMA); - addMilestoneMapping(PHASE0, DENEB); - addMilestoneMapping(ELECTRA, SpecMilestone.getHighestMilestone()); - } - - @Override - protected AttestationSchema createSchema( - final SchemaRegistry registry, - final SpecMilestone effectiveMilestone, - final SpecConfig specConfig) { - return switch (effectiveMilestone) { - case PHASE0 -> - new AttestationPhase0Schema(specConfig.getMaxValidatorsPerCommittee()) - .castTypeToAttestationSchema(); - case ELECTRA -> - new AttestationElectraSchema( - (long) specConfig.getMaxValidatorsPerCommittee() - * specConfig.getMaxCommitteesPerSlot(), - specConfig.getMaxCommitteesPerSlot()) - .castTypeToAttestationSchema(); - default -> - throw new IllegalArgumentException( - "It is not supposed to create a specific version for " + effectiveMilestone); - }; - } - - @Override - public Set getSupportedMilestones() { - return ALL_MILESTONES; + super( + ATTESTATION_SCHEMA, + milestoneSchema( + PHASE0, + (registry, specConfig) -> + new AttestationPhase0Schema(specConfig.getMaxValidatorsPerCommittee()) + .castTypeToAttestationSchema()), + milestoneSchema( + DENEB, + (registry, specConfig) -> + new AttestationElectraSchema( + (long) specConfig.getMaxValidatorsPerCommittee() + * specConfig.getMaxCommitteesPerSlot(), + specConfig.getMaxCommitteesPerSlot()) + .castTypeToAttestationSchema())); } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java index bd01161eec4..4a54ff5fbb1 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java @@ -16,29 +16,17 @@ import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.ATTNETS_ENR_FIELD_SCHEMA; -import java.util.Set; import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector; import tech.pegasys.teku.infrastructure.ssz.schema.collections.SszBitvectorSchema; -import tech.pegasys.teku.spec.SpecMilestone; -import tech.pegasys.teku.spec.config.SpecConfig; public class AttnetsENRFieldSchemaProvider extends AbstractSchemaProvider> { public AttnetsENRFieldSchemaProvider() { - super(ATTNETS_ENR_FIELD_SCHEMA); - addMilestoneMapping(PHASE0, SpecMilestone.getHighestMilestone()); - } - - @Override - protected SszBitvectorSchema createSchema( - final SchemaRegistry registry, - final SpecMilestone effectiveMilestone, - final SpecConfig specConfig) { - return SszBitvectorSchema.create(specConfig.getAttestationSubnetCount()); - } - - @Override - public Set getSupportedMilestones() { - return ALL_MILESTONES; + super( + ATTNETS_ENR_FIELD_SCHEMA, + milestoneSchema( + PHASE0, + (registry, specConfig) -> + SszBitvectorSchema.create(specConfig.getAttestationSubnetCount()))); } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/ExecutionPayloadHeaderSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/ExecutionPayloadHeaderSchemaProvider.java new file mode 100644 index 00000000000..ee8be178e0c --- /dev/null +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/ExecutionPayloadHeaderSchemaProvider.java @@ -0,0 +1,57 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.spec.schemas.registry; + +import static tech.pegasys.teku.spec.SpecMilestone.BELLATRIX; +import static tech.pegasys.teku.spec.SpecMilestone.CAPELLA; +import static tech.pegasys.teku.spec.SpecMilestone.DENEB; +import static tech.pegasys.teku.spec.SpecMilestone.ELECTRA; +import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.BEACON_STATE_SCHEMA; +import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.EXECUTION_PAYLOAD_HEADER_SCHEMA; + +import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeader; +import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeaderSchema; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.bellatrix.BeaconStateSchemaBellatrix; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.capella.BeaconStateSchemaCapella; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.deneb.BeaconStateSchemaDeneb; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.electra.BeaconStateSchemaElectra; + +public class ExecutionPayloadHeaderSchemaProvider + extends AbstractSchemaProvider> { + + public ExecutionPayloadHeaderSchemaProvider() { + super( + EXECUTION_PAYLOAD_HEADER_SCHEMA, + milestoneSchema( + BELLATRIX, + ((registry, specConfig) -> + BeaconStateSchemaBellatrix.required(registry.get(BEACON_STATE_SCHEMA)) + .getLastExecutionPayloadHeaderSchema())), + milestoneSchema( + CAPELLA, + ((registry, specConfig) -> + BeaconStateSchemaCapella.required(registry.get(BEACON_STATE_SCHEMA)) + .getLastExecutionPayloadHeaderSchema())), + milestoneSchema( + DENEB, + ((registry, specConfig) -> + BeaconStateSchemaDeneb.required(registry.get(BEACON_STATE_SCHEMA)) + .getLastExecutionPayloadHeaderSchema())), + milestoneSchema( + ELECTRA, + ((registry, specConfig) -> + BeaconStateSchemaElectra.required(registry.get(BEACON_STATE_SCHEMA)) + .getLastExecutionPayloadHeaderSchema()))); + } +} diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaTypes.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaTypes.java index 69e783dd28f..de164ca7f7a 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaTypes.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaTypes.java @@ -21,8 +21,13 @@ import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector; import tech.pegasys.teku.infrastructure.ssz.schema.collections.SszBitvectorSchema; import tech.pegasys.teku.spec.SpecMilestone; +import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeader; +import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeaderSchema; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.AttestationSchema; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconStateSchema; +import tech.pegasys.teku.spec.datastructures.state.beaconstate.MutableBeaconState; public class SchemaTypes { // PHASE0 @@ -32,6 +37,12 @@ public class SchemaTypes { public static final SchemaId> ATTESTATION_SCHEMA = create("ATTESTATION_SCHEMA"); + public static final SchemaId> + EXECUTION_PAYLOAD_HEADER_SCHEMA = create("EXECUTION_PAYLOAD_HEADER_SCHEMA"); + + public static final SchemaId> + BEACON_STATE_SCHEMA = create("BEACON_STATE_SCHEMA"); + // Altair // Bellatrix From bd2efef19bb85455890912f3fdf66e5bcb4830f1 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 10 Oct 2024 15:30:14 +0200 Subject: [PATCH 02/10] improved --- .../registry/AbstractSchemaProvider.java | 120 +++++++++++------ .../registry/AttestationSchemaProvider.java | 4 +- .../AttnetsENRFieldSchemaProvider.java | 2 +- .../ExecutionPayloadHeaderSchemaProvider.java | 57 -------- .../registry/BaseSchemaProviderTest.java | 125 +++++++++++++----- 5 files changed, 168 insertions(+), 140 deletions(-) delete mode 100644 ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/ExecutionPayloadHeaderSchemaProvider.java diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java index 2571f7ef189..67106989555 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java @@ -15,59 +15,71 @@ import static com.google.common.base.Preconditions.checkArgument; +import com.google.common.base.MoreObjects; import java.util.Arrays; -import java.util.HashSet; import java.util.List; -import java.util.NavigableMap; import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.BiFunction; -import org.apache.commons.lang3.tuple.Triple; +import java.util.stream.Stream; import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; abstract class AbstractSchemaProvider implements SchemaProvider { - private final NavigableMap> - milestoneToSchemaCreator = new TreeMap<>(); + private final TreeMap> milestoneToSchemaCreator = + new TreeMap<>(); private final SchemaId schemaId; + @SafeVarargs protected AbstractSchemaProvider( - final SchemaId schemaId, - Triple, BiFunction>... - milestoneCreators) { + final SchemaId schemaId, final SchemaProviderCreator... schemaProviderCreators) { this.schemaId = schemaId; - final List< - Triple< - SpecMilestone, Optional, BiFunction>> - creatorsList = Arrays.stream(milestoneCreators).toList(); + final List> creatorsList = Arrays.asList(schemaProviderCreators); checkArgument(!creatorsList.isEmpty(), "There should be at least 1 creator"); - checkArgument( - creatorsList.size() - == new HashSet<>(creatorsList.stream().map(Triple::getLeft).toList()).size(), - "There shouldn't be duplicated milestones in boundary start for creator, but were: %s", - creatorsList.stream().map(Triple::getLeft).toList()); - BiFunction lastCreator = null; - // TODO: add optional right bound SpecMilestone checks and logic - for (SpecMilestone specMilestone : SpecMilestone.values()) { - if (creatorsList.getFirst().getLeft() == specMilestone) { - lastCreator = creatorsList.removeFirst().getRight(); - milestoneToSchemaCreator.put(specMilestone, lastCreator); - } else { - milestoneToSchemaCreator.put(specMilestone, lastCreator); + + Arrays.stream(schemaProviderCreators) + .forEach( + creator -> + creator + .streamSupporterMilestones() + .forEach(milestone -> putCreatorAtMilestone(milestone, creator))); + } + + private void putCreatorAtMilestone( + final SpecMilestone milestone, final SchemaProviderCreator creator) { + if (!milestoneToSchemaCreator.isEmpty()) { + final SpecMilestone lastMilestone = milestoneToSchemaCreator.lastKey(); + if (lastMilestone.isGreaterThanOrEqualTo(milestone)) { + throw new IllegalArgumentException( + "Milestones ascending ordering error: from " + lastMilestone + " to " + milestone); } - if (!creatorsList.isEmpty()) { - throw new IllegalArgumentException("Overlapping creators detected: " + creatorsList); + if (lastMilestone != milestone.getPreviousMilestone()) { + throw new IllegalArgumentException( + "Milestones gap detected: from " + + lastMilestone + + " to " + + milestone.getPreviousMilestone()); } } + + if (milestoneToSchemaCreator.put(milestone, creator) != null) { + // this can't happen due to preceded checks + throw new IllegalArgumentException("Overlapping creators detected"); + } } - // TODO: not needed @Override public SpecMilestone getEffectiveMilestone(final SpecMilestone milestone) { - return milestone; + final SchemaProviderCreator maybeSchemaCreator = milestoneToSchemaCreator.get(milestone); + if (maybeSchemaCreator == null) { + throw new IllegalArgumentException( + "It is not supposed to create a specific version for " + milestone); + } + + return maybeSchemaCreator.milestone; } @Override @@ -81,35 +93,55 @@ public SchemaId getSchemaId() { return schemaId; } - static - Triple, BiFunction> - milestoneSchema( - SpecMilestone milestone, BiFunction creationSchema) { - return Triple.of(milestone, Optional.empty(), creationSchema); + static SchemaProviderCreator schemaCreator( + final SpecMilestone milestone, + final BiFunction creationSchema) { + return new SchemaProviderCreator<>(milestone, Optional.empty(), creationSchema); } - static - Triple, BiFunction> - milestoneSchema( - SpecMilestone milestone, - SpecMilestone untilMilestone, - BiFunction creationSchema) { - return Triple.of(milestone, Optional.of(untilMilestone), creationSchema); + static SchemaProviderCreator schemaCreator( + final SpecMilestone milestone, + final SpecMilestone untilMilestone, + final BiFunction creationSchema) { + checkArgument( + untilMilestone.isGreaterThan(milestone), "untilMilestone should be greater than milestone"); + return new SchemaProviderCreator<>(milestone, Optional.of(untilMilestone), creationSchema); } protected T createSchema( SchemaRegistry registry, SpecMilestone effectiveMilestone, SpecConfig specConfig) { - final BiFunction maybeSchemaCreator = + final SchemaProviderCreator maybeSchemaCreator = milestoneToSchemaCreator.get(effectiveMilestone); if (maybeSchemaCreator == null) { throw new IllegalArgumentException( "It is not supposed to create a specific version for " + effectiveMilestone); } - return maybeSchemaCreator.apply(registry, specConfig); + return maybeSchemaCreator.creator.apply(registry, specConfig); } @Override public Set getSupportedMilestones() { - return milestoneToSchemaCreator.navigableKeySet(); + return milestoneToSchemaCreator.keySet(); + } + + protected record SchemaProviderCreator( + SpecMilestone milestone, + Optional untilMilestone, + BiFunction creator) { + Stream streamSupporterMilestones() { + if (untilMilestone.isEmpty()) { + return Stream.of(milestone); + } + return SpecMilestone.getMilestonesUpTo(untilMilestone.get()).stream() + .filter(m -> m.isGreaterThanOrEqualTo(milestone)); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("milestone", milestone) + .add("untilMilestone", untilMilestone) + .toString(); + } } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java index 84903b549ea..3140a6844fb 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java @@ -27,12 +27,12 @@ public class AttestationSchemaProvider public AttestationSchemaProvider() { super( ATTESTATION_SCHEMA, - milestoneSchema( + schemaCreator( PHASE0, (registry, specConfig) -> new AttestationPhase0Schema(specConfig.getMaxValidatorsPerCommittee()) .castTypeToAttestationSchema()), - milestoneSchema( + schemaCreator( DENEB, (registry, specConfig) -> new AttestationElectraSchema( diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java index 4a54ff5fbb1..8eb23db98d0 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java @@ -24,7 +24,7 @@ public class AttnetsENRFieldSchemaProvider public AttnetsENRFieldSchemaProvider() { super( ATTNETS_ENR_FIELD_SCHEMA, - milestoneSchema( + schemaCreator( PHASE0, (registry, specConfig) -> SszBitvectorSchema.create(specConfig.getAttestationSubnetCount()))); diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/ExecutionPayloadHeaderSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/ExecutionPayloadHeaderSchemaProvider.java deleted file mode 100644 index ee8be178e0c..00000000000 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/ExecutionPayloadHeaderSchemaProvider.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright Consensys Software Inc., 2024 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.spec.schemas.registry; - -import static tech.pegasys.teku.spec.SpecMilestone.BELLATRIX; -import static tech.pegasys.teku.spec.SpecMilestone.CAPELLA; -import static tech.pegasys.teku.spec.SpecMilestone.DENEB; -import static tech.pegasys.teku.spec.SpecMilestone.ELECTRA; -import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.BEACON_STATE_SCHEMA; -import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.EXECUTION_PAYLOAD_HEADER_SCHEMA; - -import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeader; -import tech.pegasys.teku.spec.datastructures.execution.ExecutionPayloadHeaderSchema; -import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.bellatrix.BeaconStateSchemaBellatrix; -import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.capella.BeaconStateSchemaCapella; -import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.deneb.BeaconStateSchemaDeneb; -import tech.pegasys.teku.spec.datastructures.state.beaconstate.versions.electra.BeaconStateSchemaElectra; - -public class ExecutionPayloadHeaderSchemaProvider - extends AbstractSchemaProvider> { - - public ExecutionPayloadHeaderSchemaProvider() { - super( - EXECUTION_PAYLOAD_HEADER_SCHEMA, - milestoneSchema( - BELLATRIX, - ((registry, specConfig) -> - BeaconStateSchemaBellatrix.required(registry.get(BEACON_STATE_SCHEMA)) - .getLastExecutionPayloadHeaderSchema())), - milestoneSchema( - CAPELLA, - ((registry, specConfig) -> - BeaconStateSchemaCapella.required(registry.get(BEACON_STATE_SCHEMA)) - .getLastExecutionPayloadHeaderSchema())), - milestoneSchema( - DENEB, - ((registry, specConfig) -> - BeaconStateSchemaDeneb.required(registry.get(BEACON_STATE_SCHEMA)) - .getLastExecutionPayloadHeaderSchema())), - milestoneSchema( - ELECTRA, - ((registry, specConfig) -> - BeaconStateSchemaElectra.required(registry.get(BEACON_STATE_SCHEMA)) - .getLastExecutionPayloadHeaderSchema()))); - } -} diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java index 67b6a3d5f5d..c634eefa40f 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.spec.schemas.registry; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.mock; @@ -20,73 +21,125 @@ import static tech.pegasys.teku.spec.SpecMilestone.ALTAIR; import static tech.pegasys.teku.spec.SpecMilestone.BELLATRIX; import static tech.pegasys.teku.spec.SpecMilestone.CAPELLA; +import static tech.pegasys.teku.spec.SpecMilestone.DENEB; import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; +import static tech.pegasys.teku.spec.schemas.registry.AbstractSchemaProvider.schemaCreator; -import java.util.EnumSet; -import java.util.Set; import org.junit.jupiter.api.Test; -import tech.pegasys.teku.spec.SpecMilestone; -import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; class BaseSchemaProviderTest { @SuppressWarnings("unchecked") private static final SchemaId STRING_SCHEMA_ID = mock(SchemaId.class); - private final TestSchemaProvider provider = new TestSchemaProvider(); private final SchemaRegistry mockRegistry = mock(SchemaRegistry.class); @Test - void shouldGetEffectiveMilestone() { - provider.addMilestoneMapping(PHASE0, ALTAIR); - assertEquals(PHASE0, provider.getEffectiveMilestone(PHASE0)); - assertEquals(PHASE0, provider.getEffectiveMilestone(ALTAIR)); + void shouldSupportContinuousWithoutUntil() { + final TestSchemaProvider provider = + new TestSchemaProvider( + schemaCreator(ALTAIR, (r, c) -> "TestSchemaAltair"), + schemaCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix")); + assertEquals(ALTAIR, provider.getEffectiveMilestone(ALTAIR)); assertEquals(BELLATRIX, provider.getEffectiveMilestone(BELLATRIX)); - } - @Test - void shouldGetSchema() { - when(mockRegistry.getMilestone()).thenReturn(PHASE0); - String result = provider.getSchema(mockRegistry); - assertEquals("TestSchema", result); + when(mockRegistry.getMilestone()).thenReturn(ALTAIR); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaAltair"); + + when(mockRegistry.getMilestone()).thenReturn(BELLATRIX); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaBellatrix"); + + assertThat(provider.getSupportedMilestones()).containsExactly(ALTAIR, BELLATRIX); } @Test - void shouldGetNonOverlappingVersionMappings() { - provider.addMilestoneMapping(PHASE0, ALTAIR); - provider.addMilestoneMapping(BELLATRIX, CAPELLA); + void shouldSupportContinuousWithUntil() { + final TestSchemaProvider provider = + new TestSchemaProvider( + schemaCreator(PHASE0, ALTAIR, (r, c) -> "TestSchemaPhase0"), + schemaCreator(BELLATRIX, CAPELLA, (r, c) -> "TestSchemaBellatrix")); assertEquals(PHASE0, provider.getEffectiveMilestone(PHASE0)); assertEquals(PHASE0, provider.getEffectiveMilestone(ALTAIR)); assertEquals(BELLATRIX, provider.getEffectiveMilestone(BELLATRIX)); assertEquals(BELLATRIX, provider.getEffectiveMilestone(CAPELLA)); + + when(mockRegistry.getMilestone()).thenReturn(PHASE0); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaPhase0"); + + when(mockRegistry.getMilestone()).thenReturn(ALTAIR); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaPhase0"); + + when(mockRegistry.getMilestone()).thenReturn(BELLATRIX); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaBellatrix"); + + when(mockRegistry.getMilestone()).thenReturn(CAPELLA); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaBellatrix"); + + assertThat(provider.getSupportedMilestones()) + .containsExactly(PHASE0, ALTAIR, BELLATRIX, CAPELLA); + } + + @Test + void shouldThrowWhenNoCreators() { + assertThatThrownBy(TestSchemaProvider::new) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("There should be at least 1 creator"); } @Test - void testOverlappingVersionMappingsThrowsException() { - provider.addMilestoneMapping(PHASE0, ALTAIR); + void shouldThrowWhenNotAscendingMilestonesWithUntil() { + assertThatThrownBy( + () -> + new TestSchemaProvider( + schemaCreator(PHASE0, ALTAIR, (r, c) -> "TestSchema"), + schemaCreator(ALTAIR, (r, c) -> "TestSchema"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Milestones ascending ordering error: from ALTAIR to ALTAIR"); - assertThatThrownBy(() -> provider.addMilestoneMapping(ALTAIR, BELLATRIX)) + assertThatThrownBy( + () -> + new TestSchemaProvider( + schemaCreator(ALTAIR, BELLATRIX, (r, c) -> "TestSchema"), + schemaCreator(PHASE0, (r, c) -> "TestSchema"))) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Milestone ALTAIR is already mapped to PHASE0"); + .hasMessage("Milestones ascending ordering error: from BELLATRIX to PHASE0"); } - private static class TestSchemaProvider extends AbstractSchemaProvider { - TestSchemaProvider() { - super(STRING_SCHEMA_ID); - } + @Test + void shouldThrowWhenNotAscendingMilestonesWithoutUntil() { + assertThatThrownBy( + () -> + new TestSchemaProvider( + schemaCreator(PHASE0, (r, c) -> "TestSchema"), + schemaCreator(PHASE0, (r, c) -> "TestSchema"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Milestones ascending ordering error: from PHASE0 to PHASE0"); - @Override - protected String createSchema( - final SchemaRegistry registry, - final SpecMilestone effectiveMilestone, - final SpecConfig specConfig) { - return "TestSchema"; - } + assertThatThrownBy( + () -> + new TestSchemaProvider( + schemaCreator(DENEB, (r, c) -> "TestSchema"), + schemaCreator(ALTAIR, (r, c) -> "TestSchema"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Milestones ascending ordering error: from DENEB to ALTAIR"); + } - @Override - public Set getSupportedMilestones() { - return EnumSet.allOf(SpecMilestone.class); + @Test + void shouldThrowWhenDeclaringGaps() { + assertThatThrownBy( + () -> + new TestSchemaProvider( + schemaCreator(PHASE0, (r, c) -> "TestSchema"), + schemaCreator(BELLATRIX, (r, c) -> "TestSchema"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Milestones gap detected: from PHASE0 to ALTAIR"); + } + + private static class TestSchemaProvider extends AbstractSchemaProvider { + @SafeVarargs + TestSchemaProvider(final SchemaProviderCreator... schemaProviderCreators) { + super(STRING_SCHEMA_ID, schemaProviderCreators); } } } From 17f084a46a84f482a0ac0c534edd4197b9c305a4 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 10 Oct 2024 15:40:25 +0200 Subject: [PATCH 03/10] fix finals and improvements --- .../registry/AbstractSchemaProvider.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java index 67106989555..da96789f269 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java @@ -73,13 +73,7 @@ private void putCreatorAtMilestone( @Override public SpecMilestone getEffectiveMilestone(final SpecMilestone milestone) { - final SchemaProviderCreator maybeSchemaCreator = milestoneToSchemaCreator.get(milestone); - if (maybeSchemaCreator == null) { - throw new IllegalArgumentException( - "It is not supposed to create a specific version for " + milestone); - } - - return maybeSchemaCreator.milestone; + return getSchemaCreator(milestone).milestone; } @Override @@ -109,14 +103,19 @@ static SchemaProviderCreator schemaCreator( } protected T createSchema( - SchemaRegistry registry, SpecMilestone effectiveMilestone, SpecConfig specConfig) { - final SchemaProviderCreator maybeSchemaCreator = - milestoneToSchemaCreator.get(effectiveMilestone); + final SchemaRegistry registry, + final SpecMilestone effectiveMilestone, + final SpecConfig specConfig) { + return getSchemaCreator(effectiveMilestone).creator.apply(registry, specConfig); + } + + private SchemaProviderCreator getSchemaCreator(final SpecMilestone milestone) { + final SchemaProviderCreator maybeSchemaCreator = milestoneToSchemaCreator.get(milestone); if (maybeSchemaCreator == null) { throw new IllegalArgumentException( - "It is not supposed to create a specific version for " + effectiveMilestone); + "It is not supposed to create a specific version for " + milestone); } - return maybeSchemaCreator.creator.apply(registry, specConfig); + return maybeSchemaCreator; } @Override From 3bae3b5fc94b7c2762bcc251c54c833aec59cdeb Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Thu, 10 Oct 2024 21:41:22 +0200 Subject: [PATCH 04/10] improvements --- .../registry/AbstractSchemaProvider.java | 85 ++++++++++--------- .../spec/schemas/registry/SchemaProvider.java | 24 ------ .../registry/BaseSchemaProviderTest.java | 48 ++++++----- 3 files changed, 75 insertions(+), 82 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java index da96789f269..3152257173e 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java @@ -16,13 +16,13 @@ import static com.google.common.base.Preconditions.checkArgument; import com.google.common.base.MoreObjects; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.BiFunction; -import java.util.stream.Stream; import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; @@ -36,38 +36,57 @@ abstract class AbstractSchemaProvider implements SchemaProvider { protected AbstractSchemaProvider( final SchemaId schemaId, final SchemaProviderCreator... schemaProviderCreators) { this.schemaId = schemaId; - final List> creatorsList = Arrays.asList(schemaProviderCreators); - checkArgument(!creatorsList.isEmpty(), "There should be at least 1 creator"); - - Arrays.stream(schemaProviderCreators) - .forEach( - creator -> - creator - .streamSupporterMilestones() - .forEach(milestone -> putCreatorAtMilestone(milestone, creator))); - } - - private void putCreatorAtMilestone( - final SpecMilestone milestone, final SchemaProviderCreator creator) { - if (!milestoneToSchemaCreator.isEmpty()) { - final SpecMilestone lastMilestone = milestoneToSchemaCreator.lastKey(); - if (lastMilestone.isGreaterThanOrEqualTo(milestone)) { - throw new IllegalArgumentException( - "Milestones ascending ordering error: from " + lastMilestone + " to " + milestone); + validateCreators(schemaProviderCreators); + final List> creatorsList = + new ArrayList<>(Arrays.asList(schemaProviderCreators)); + + SchemaProviderCreator lastCreator = null; + for (final SpecMilestone milestone : SpecMilestone.values()) { + if (!creatorsList.isEmpty() && creatorsList.getFirst().milestone == milestone) { + lastCreator = creatorsList.removeFirst(); } - if (lastMilestone != milestone.getPreviousMilestone()) { - throw new IllegalArgumentException( - "Milestones gap detected: from " - + lastMilestone - + " to " - + milestone.getPreviousMilestone()); + if (lastCreator != null) { + milestoneToSchemaCreator.put(milestone, lastCreator); + + final boolean untilSpecReached = + lastCreator + .untilMilestone + .map(untilMilestone -> untilMilestone == milestone) + .orElse(false); + + if (untilSpecReached) { + break; + } } } + } - if (milestoneToSchemaCreator.put(milestone, creator) != null) { - // this can't happen due to preceded checks - throw new IllegalArgumentException("Overlapping creators detected"); + @SafeVarargs + private void validateCreators(final SchemaProviderCreator... schemaProviderCreators) { + checkArgument( + schemaProviderCreators.length > 0, "There should be at least 1 creator for %s", schemaId); + for (int i = 0; i < schemaProviderCreators.length; i++) { + final SchemaProviderCreator currentCreator = schemaProviderCreators[i]; + if (i > 0) { + checkArgument( + currentCreator.milestone.isGreaterThan(schemaProviderCreators[i - 1].milestone), + "Creator's milestones must be in order for %s", + schemaId); + } + final boolean isLast = i == schemaProviderCreators.length - 1; + if (isLast) { + checkArgument( + currentCreator.untilMilestone.isEmpty() + || currentCreator.untilMilestone.get().isGreaterThan(currentCreator.milestone), + "Last creator untilMilestone must be greater than milestone in %s", + schemaId); + } else { + checkArgument( + currentCreator.untilMilestone.isEmpty(), + "Only last creator is allowed to use until for %s", + schemaId); + } } } @@ -97,8 +116,6 @@ static SchemaProviderCreator schemaCreator( final SpecMilestone milestone, final SpecMilestone untilMilestone, final BiFunction creationSchema) { - checkArgument( - untilMilestone.isGreaterThan(milestone), "untilMilestone should be greater than milestone"); return new SchemaProviderCreator<>(milestone, Optional.of(untilMilestone), creationSchema); } @@ -127,14 +144,6 @@ protected record SchemaProviderCreator( SpecMilestone milestone, Optional untilMilestone, BiFunction creator) { - Stream streamSupporterMilestones() { - if (untilMilestone.isEmpty()) { - return Stream.of(milestone); - } - return SpecMilestone.getMilestonesUpTo(untilMilestone.get()).stream() - .filter(m -> m.isGreaterThanOrEqualTo(milestone)); - } - @Override public String toString() { return MoreObjects.toStringHelper(this) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java index 7c6efcafd5a..75516b08fe5 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java @@ -13,35 +13,11 @@ package tech.pegasys.teku.spec.schemas.registry; -import static tech.pegasys.teku.spec.SpecMilestone.BELLATRIX; -import static tech.pegasys.teku.spec.SpecMilestone.CAPELLA; -import static tech.pegasys.teku.spec.SpecMilestone.DENEB; -import static tech.pegasys.teku.spec.SpecMilestone.ELECTRA; - -import java.util.EnumSet; import java.util.Set; import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; interface SchemaProvider { - Set ALL_MILESTONES = EnumSet.allOf(SpecMilestone.class); - Set FROM_BELLATRIX = from(BELLATRIX); - Set FROM_CAPELLA = from(CAPELLA); - Set FROM_DENEB = from(DENEB); - Set FROM_ELECTRA = from(ELECTRA); - - static Set from(final SpecMilestone milestone) { - return EnumSet.copyOf(SpecMilestone.getAllMilestonesFrom(milestone)); - } - - static Set fromTo( - final SpecMilestone fromMilestone, final SpecMilestone toMilestone) { - return EnumSet.copyOf( - SpecMilestone.getAllMilestonesFrom(fromMilestone).stream() - .filter(toMilestone::isLessThanOrEqualTo) - .toList()); - } - T getSchema(SchemaRegistry registry); Set getSupportedMilestones(); diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java index c634eefa40f..caf47a764ef 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java @@ -26,6 +26,7 @@ import static tech.pegasys.teku.spec.schemas.registry.AbstractSchemaProvider.schemaCreator; import org.junit.jupiter.api.Test; +import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; class BaseSchemaProviderTest { @@ -49,14 +50,15 @@ void shouldSupportContinuousWithoutUntil() { when(mockRegistry.getMilestone()).thenReturn(BELLATRIX); assertEquals(provider.getSchema(mockRegistry), "TestSchemaBellatrix"); - assertThat(provider.getSupportedMilestones()).containsExactly(ALTAIR, BELLATRIX); + assertThat(provider.getSupportedMilestones()) + .containsAll(SpecMilestone.getAllMilestonesFrom(ALTAIR)); } @Test void shouldSupportContinuousWithUntil() { final TestSchemaProvider provider = new TestSchemaProvider( - schemaCreator(PHASE0, ALTAIR, (r, c) -> "TestSchemaPhase0"), + schemaCreator(PHASE0, (r, c) -> "TestSchemaPhase0"), schemaCreator(BELLATRIX, CAPELLA, (r, c) -> "TestSchemaBellatrix")); assertEquals(PHASE0, provider.getEffectiveMilestone(PHASE0)); @@ -84,7 +86,7 @@ void shouldSupportContinuousWithUntil() { void shouldThrowWhenNoCreators() { assertThatThrownBy(TestSchemaProvider::new) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("There should be at least 1 creator"); + .hasMessageStartingWith("There should be at least 1 creator"); } @Test @@ -92,18 +94,10 @@ void shouldThrowWhenNotAscendingMilestonesWithUntil() { assertThatThrownBy( () -> new TestSchemaProvider( - schemaCreator(PHASE0, ALTAIR, (r, c) -> "TestSchema"), - schemaCreator(ALTAIR, (r, c) -> "TestSchema"))) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Milestones ascending ordering error: from ALTAIR to ALTAIR"); - - assertThatThrownBy( - () -> - new TestSchemaProvider( - schemaCreator(ALTAIR, BELLATRIX, (r, c) -> "TestSchema"), - schemaCreator(PHASE0, (r, c) -> "TestSchema"))) + schemaCreator(ALTAIR, (r, c) -> "TestSchema"), + schemaCreator(PHASE0, BELLATRIX, (r, c) -> "TestSchema"))) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Milestones ascending ordering error: from BELLATRIX to PHASE0"); + .hasMessageStartingWith("Creator's milestones must be in order"); } @Test @@ -114,7 +108,7 @@ void shouldThrowWhenNotAscendingMilestonesWithoutUntil() { schemaCreator(PHASE0, (r, c) -> "TestSchema"), schemaCreator(PHASE0, (r, c) -> "TestSchema"))) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Milestones ascending ordering error: from PHASE0 to PHASE0"); + .hasMessageStartingWith("Creator's milestones must be in order"); assertThatThrownBy( () -> @@ -122,18 +116,32 @@ void shouldThrowWhenNotAscendingMilestonesWithoutUntil() { schemaCreator(DENEB, (r, c) -> "TestSchema"), schemaCreator(ALTAIR, (r, c) -> "TestSchema"))) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Milestones ascending ordering error: from DENEB to ALTAIR"); + .hasMessageStartingWith("Creator's milestones must be in order"); } @Test - void shouldThrowWhenDeclaringGaps() { + void shouldThrowWhenWithUntilNotAsLast() { assertThatThrownBy( () -> new TestSchemaProvider( - schemaCreator(PHASE0, (r, c) -> "TestSchema"), - schemaCreator(BELLATRIX, (r, c) -> "TestSchema"))) + schemaCreator(ALTAIR, BELLATRIX, (r, c) -> "TestSchema"), + schemaCreator(CAPELLA, (r, c) -> "TestSchema"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Only last creator is allowed to use until"); + } + + @Test + void shouldThrowWhenWithUntilIsPriorToMilestone() { + assertThatThrownBy( + () -> + new TestSchemaProvider(schemaCreator(BELLATRIX, BELLATRIX, (r, c) -> "TestSchema"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Last creator untilMilestone must be greater than milestone in"); + + assertThatThrownBy( + () -> new TestSchemaProvider(schemaCreator(CAPELLA, BELLATRIX, (r, c) -> "TestSchema"))) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Milestones gap detected: from PHASE0 to ALTAIR"); + .hasMessageStartingWith("Last creator untilMilestone must be greater than milestone in"); } private static class TestSchemaProvider extends AbstractSchemaProvider { From 2a39925098e8428b92a69758e268958da96ca120 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Fri, 11 Oct 2024 13:32:22 +0200 Subject: [PATCH 05/10] tmp --- .../registry/AbstractSchemaProvider.java | 36 +++++++++++++++++-- .../registry/AttestationSchemaProvider.java | 1 + .../registry/SchemaRegistryBuilder.java | 18 +++++++++- .../registry/BaseSchemaProviderTest.java | 26 ++++++++++---- 4 files changed, 71 insertions(+), 10 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java index 3152257173e..32d327889b2 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java @@ -27,7 +27,7 @@ import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; -abstract class AbstractSchemaProvider implements SchemaProvider { +class AbstractSchemaProvider implements SchemaProvider { private final TreeMap> milestoneToSchemaCreator = new TreeMap<>(); private final SchemaId schemaId; @@ -78,8 +78,8 @@ private void validateCreators(final SchemaProviderCreator... schemaProviderCr if (isLast) { checkArgument( currentCreator.untilMilestone.isEmpty() - || currentCreator.untilMilestone.get().isGreaterThan(currentCreator.milestone), - "Last creator untilMilestone must be greater than milestone in %s", + || currentCreator.untilMilestone.get().isGreaterThanOrEqualTo(currentCreator.milestone), + "Last creator untilMilestone must be greater or equal than milestone in %s", schemaId); } else { checkArgument( @@ -152,4 +152,34 @@ public String toString() { .toString(); } } + + static Builder providerBuilder(final SchemaId schemaId) { + return new Builder<>(schemaId); + } + + static class Builder { + private final SchemaId schemaId; + final List> schemaProviderCreators = new ArrayList<>(); + + private Builder(final SchemaId schemaId) { + this.schemaId = schemaId; + } + + public Builder withCreator(final SpecMilestone milestone, + final BiFunction creationSchema) { + schemaProviderCreators.add(new SchemaProviderCreator<>(milestone, Optional.empty(), creationSchema)); + return this; + } + + public Builder withCreator(final SpecMilestone milestone, + final SpecMilestone untilMilestone, + final BiFunction creationSchema) { + schemaProviderCreators.add(new SchemaProviderCreator<>(milestone, Optional.of(untilMilestone), creationSchema)); + return this; + } + + public AbstractSchemaProvider build() { + return new AbstractSchemaProvider<>(schemaId, schemaProviderCreators.toArray(new SchemaProviderCreator[0])); + } + } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java index 3140a6844fb..2bf95b09297 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java @@ -41,4 +41,5 @@ public AttestationSchemaProvider() { specConfig.getMaxCommitteesPerSlot()) .castTypeToAttestationSchema())); } + } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java index 81788ca7d99..90dc718f1b3 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java @@ -16,10 +16,16 @@ import com.google.common.annotations.VisibleForTesting; import java.util.HashSet; import java.util.Set; + +import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector; +import tech.pegasys.teku.infrastructure.ssz.schema.collections.SszBitvectorSchema; import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; +import static tech.pegasys.teku.spec.schemas.registry.AbstractSchemaProvider.providerBuilder; +import static tech.pegasys.teku.spec.schemas.registry.AbstractSchemaProvider.schemaCreator; + public class SchemaRegistryBuilder { private final Set> providers = new HashSet<>(); private final Set> schemaIds = new HashSet<>(); @@ -27,10 +33,20 @@ public class SchemaRegistryBuilder { public static SchemaRegistryBuilder create() { return new SchemaRegistryBuilder() - .addProvider(new AttnetsENRFieldSchemaProvider()) + .addProvider(createAttnetsENRFieldSchemaProvider()) .addProvider(new AttestationSchemaProvider()); } + private static SchemaProvider> createAttnetsENRFieldSchemaProvider() { + return providerBuilder(SchemaTypes.ATTNETS_ENR_FIELD_SCHEMA) + .withCreator( + SpecMilestone.PHASE0, + (registry, specConfig) -> + SszBitvectorSchema.create(specConfig.getAttestationSubnetCount())) + .build(); + } + + public SchemaRegistryBuilder() { this.cache = SchemaCache.createDefault(); } diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java index caf47a764ef..59370866624 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java @@ -82,6 +82,19 @@ void shouldSupportContinuousWithUntil() { .containsExactly(PHASE0, ALTAIR, BELLATRIX, CAPELLA); } + @Test + void shouldAllowSingleMilestone() { + final TestSchemaProvider provider = + new TestSchemaProvider( + schemaCreator(ALTAIR, (r, c) -> "TestSchemaAltair"), + schemaCreator(BELLATRIX,CAPELLA, (r, c) -> "TestSchemaBellatrix")); + + when(mockRegistry.getMilestone()).thenReturn(DENEB); + + assertThatThrownBy(() -> provider.getSchema(mockRegistry)).isInstanceOf(IllegalArgumentException.class) + .hasMessage("It is not supposed to create a specific version for DENEB"); + } + @Test void shouldThrowWhenNoCreators() { assertThatThrownBy(TestSchemaProvider::new) @@ -89,6 +102,13 @@ void shouldThrowWhenNoCreators() { .hasMessageStartingWith("There should be at least 1 creator"); } + @Test + void shouldThrowWhenAskingForAnUnsupportedMilestone() { + assertThatThrownBy(TestSchemaProvider::new) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("There should be at least 1 creator"); + } + @Test void shouldThrowWhenNotAscendingMilestonesWithUntil() { assertThatThrownBy( @@ -132,12 +152,6 @@ void shouldThrowWhenWithUntilNotAsLast() { @Test void shouldThrowWhenWithUntilIsPriorToMilestone() { - assertThatThrownBy( - () -> - new TestSchemaProvider(schemaCreator(BELLATRIX, BELLATRIX, (r, c) -> "TestSchema"))) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Last creator untilMilestone must be greater than milestone in"); - assertThatThrownBy( () -> new TestSchemaProvider(schemaCreator(CAPELLA, BELLATRIX, (r, c) -> "TestSchema"))) .isInstanceOf(IllegalArgumentException.class) From 04ac291e38e048f81605a33d3185eb40e16e4c30 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 14 Oct 2024 11:59:28 +0200 Subject: [PATCH 06/10] migrate to Builder --- .../registry/AbstractSchemaProvider.java | 185 ----------------- .../registry/AttestationSchemaProvider.java | 45 ---- .../AttnetsENRFieldSchemaProvider.java | 32 --- .../schemas/registry/BaseSchemaProvider.java | 196 ++++++++++++++++++ .../registry/SchemaRegistryBuilder.java | 48 +++-- .../registry/BaseSchemaProviderTest.java | 138 ++++++------ 6 files changed, 311 insertions(+), 333 deletions(-) delete mode 100644 ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java delete mode 100644 ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java delete mode 100644 ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java create mode 100644 ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java deleted file mode 100644 index 32d327889b2..00000000000 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java +++ /dev/null @@ -1,185 +0,0 @@ -/* - * Copyright Consensys Software Inc., 2024 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.spec.schemas.registry; - -import static com.google.common.base.Preconditions.checkArgument; - -import com.google.common.base.MoreObjects; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Optional; -import java.util.Set; -import java.util.TreeMap; -import java.util.function.BiFunction; -import tech.pegasys.teku.spec.SpecMilestone; -import tech.pegasys.teku.spec.config.SpecConfig; -import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; - -class AbstractSchemaProvider implements SchemaProvider { - private final TreeMap> milestoneToSchemaCreator = - new TreeMap<>(); - private final SchemaId schemaId; - - @SafeVarargs - protected AbstractSchemaProvider( - final SchemaId schemaId, final SchemaProviderCreator... schemaProviderCreators) { - this.schemaId = schemaId; - validateCreators(schemaProviderCreators); - final List> creatorsList = - new ArrayList<>(Arrays.asList(schemaProviderCreators)); - - SchemaProviderCreator lastCreator = null; - for (final SpecMilestone milestone : SpecMilestone.values()) { - if (!creatorsList.isEmpty() && creatorsList.getFirst().milestone == milestone) { - lastCreator = creatorsList.removeFirst(); - } - - if (lastCreator != null) { - milestoneToSchemaCreator.put(milestone, lastCreator); - - final boolean untilSpecReached = - lastCreator - .untilMilestone - .map(untilMilestone -> untilMilestone == milestone) - .orElse(false); - - if (untilSpecReached) { - break; - } - } - } - } - - @SafeVarargs - private void validateCreators(final SchemaProviderCreator... schemaProviderCreators) { - checkArgument( - schemaProviderCreators.length > 0, "There should be at least 1 creator for %s", schemaId); - for (int i = 0; i < schemaProviderCreators.length; i++) { - final SchemaProviderCreator currentCreator = schemaProviderCreators[i]; - if (i > 0) { - checkArgument( - currentCreator.milestone.isGreaterThan(schemaProviderCreators[i - 1].milestone), - "Creator's milestones must be in order for %s", - schemaId); - } - final boolean isLast = i == schemaProviderCreators.length - 1; - if (isLast) { - checkArgument( - currentCreator.untilMilestone.isEmpty() - || currentCreator.untilMilestone.get().isGreaterThanOrEqualTo(currentCreator.milestone), - "Last creator untilMilestone must be greater or equal than milestone in %s", - schemaId); - } else { - checkArgument( - currentCreator.untilMilestone.isEmpty(), - "Only last creator is allowed to use until for %s", - schemaId); - } - } - } - - @Override - public SpecMilestone getEffectiveMilestone(final SpecMilestone milestone) { - return getSchemaCreator(milestone).milestone; - } - - @Override - public T getSchema(final SchemaRegistry registry) { - final SpecMilestone milestone = registry.getMilestone(); - return createSchema(registry, milestone, registry.getSpecConfig()); - } - - @Override - public SchemaId getSchemaId() { - return schemaId; - } - - static SchemaProviderCreator schemaCreator( - final SpecMilestone milestone, - final BiFunction creationSchema) { - return new SchemaProviderCreator<>(milestone, Optional.empty(), creationSchema); - } - - static SchemaProviderCreator schemaCreator( - final SpecMilestone milestone, - final SpecMilestone untilMilestone, - final BiFunction creationSchema) { - return new SchemaProviderCreator<>(milestone, Optional.of(untilMilestone), creationSchema); - } - - protected T createSchema( - final SchemaRegistry registry, - final SpecMilestone effectiveMilestone, - final SpecConfig specConfig) { - return getSchemaCreator(effectiveMilestone).creator.apply(registry, specConfig); - } - - private SchemaProviderCreator getSchemaCreator(final SpecMilestone milestone) { - final SchemaProviderCreator maybeSchemaCreator = milestoneToSchemaCreator.get(milestone); - if (maybeSchemaCreator == null) { - throw new IllegalArgumentException( - "It is not supposed to create a specific version for " + milestone); - } - return maybeSchemaCreator; - } - - @Override - public Set getSupportedMilestones() { - return milestoneToSchemaCreator.keySet(); - } - - protected record SchemaProviderCreator( - SpecMilestone milestone, - Optional untilMilestone, - BiFunction creator) { - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("milestone", milestone) - .add("untilMilestone", untilMilestone) - .toString(); - } - } - - static Builder providerBuilder(final SchemaId schemaId) { - return new Builder<>(schemaId); - } - - static class Builder { - private final SchemaId schemaId; - final List> schemaProviderCreators = new ArrayList<>(); - - private Builder(final SchemaId schemaId) { - this.schemaId = schemaId; - } - - public Builder withCreator(final SpecMilestone milestone, - final BiFunction creationSchema) { - schemaProviderCreators.add(new SchemaProviderCreator<>(milestone, Optional.empty(), creationSchema)); - return this; - } - - public Builder withCreator(final SpecMilestone milestone, - final SpecMilestone untilMilestone, - final BiFunction creationSchema) { - schemaProviderCreators.add(new SchemaProviderCreator<>(milestone, Optional.of(untilMilestone), creationSchema)); - return this; - } - - public AbstractSchemaProvider build() { - return new AbstractSchemaProvider<>(schemaId, schemaProviderCreators.toArray(new SchemaProviderCreator[0])); - } - } -} diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java deleted file mode 100644 index 2bf95b09297..00000000000 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttestationSchemaProvider.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright Consensys Software Inc., 2024 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.spec.schemas.registry; - -import static tech.pegasys.teku.spec.SpecMilestone.DENEB; -import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; -import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.ATTESTATION_SCHEMA; - -import tech.pegasys.teku.spec.datastructures.operations.Attestation; -import tech.pegasys.teku.spec.datastructures.operations.AttestationSchema; -import tech.pegasys.teku.spec.datastructures.operations.versions.electra.AttestationElectraSchema; -import tech.pegasys.teku.spec.datastructures.operations.versions.phase0.AttestationPhase0Schema; - -public class AttestationSchemaProvider - extends AbstractSchemaProvider> { - public AttestationSchemaProvider() { - super( - ATTESTATION_SCHEMA, - schemaCreator( - PHASE0, - (registry, specConfig) -> - new AttestationPhase0Schema(specConfig.getMaxValidatorsPerCommittee()) - .castTypeToAttestationSchema()), - schemaCreator( - DENEB, - (registry, specConfig) -> - new AttestationElectraSchema( - (long) specConfig.getMaxValidatorsPerCommittee() - * specConfig.getMaxCommitteesPerSlot(), - specConfig.getMaxCommitteesPerSlot()) - .castTypeToAttestationSchema())); - } - -} diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java deleted file mode 100644 index 8eb23db98d0..00000000000 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AttnetsENRFieldSchemaProvider.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright Consensys Software Inc., 2024 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package tech.pegasys.teku.spec.schemas.registry; - -import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; -import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.ATTNETS_ENR_FIELD_SCHEMA; - -import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector; -import tech.pegasys.teku.infrastructure.ssz.schema.collections.SszBitvectorSchema; - -public class AttnetsENRFieldSchemaProvider - extends AbstractSchemaProvider> { - public AttnetsENRFieldSchemaProvider() { - super( - ATTNETS_ENR_FIELD_SCHEMA, - schemaCreator( - PHASE0, - (registry, specConfig) -> - SszBitvectorSchema.create(specConfig.getAttestationSubnetCount()))); - } -} diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java new file mode 100644 index 00000000000..674d96616af --- /dev/null +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java @@ -0,0 +1,196 @@ +/* + * Copyright Consensys Software Inc., 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package tech.pegasys.teku.spec.schemas.registry; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.common.base.MoreObjects; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.TreeMap; +import java.util.function.BiFunction; +import tech.pegasys.teku.spec.SpecMilestone; +import tech.pegasys.teku.spec.config.SpecConfig; +import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; + +class BaseSchemaProvider implements SchemaProvider { + private final TreeMap> milestoneToSchemaCreator = + new TreeMap<>(); + private final SchemaId schemaId; + + private BaseSchemaProvider( + final SchemaId schemaId, + final List> schemaProviderCreators, + final SpecMilestone untilMilestone, + final boolean isConstant) { + this.schemaId = schemaId; + // validateCreators(schemaProviderCreators); + final List> creatorsList = new ArrayList<>(schemaProviderCreators); + + SchemaProviderCreator lastCreator = null; + + for (final SpecMilestone milestone : SpecMilestone.getMilestonesUpTo(untilMilestone)) { + if (!creatorsList.isEmpty() && creatorsList.getFirst().milestone == milestone) { + lastCreator = creatorsList.removeFirst(); + } + + if (lastCreator != null) { + milestoneToSchemaCreator.put( + milestone, isConstant ? lastCreator : lastCreator.withMilestone(milestone)); + } + } + } + + // private void validateCreators(final List> schemaProviderCreators) { + // checkArgument( + // !schemaProviderCreators.isEmpty(), "There should be at least 1 creator for %s", + // schemaId); + // for (int i = 0; i < schemaProviderCreators.size(); i++) { + // final SchemaProviderCreator currentCreator = schemaProviderCreators.get(i); + // if (i > 0) { + // checkArgument( + // currentCreator.milestone.isGreaterThan(schemaProviderCreators.get(i - 1).milestone), + // "Creator's milestones must be in order for %s", + // schemaId); + // } + // final boolean isLast = i == schemaProviderCreators.size() - 1; + // if (isLast) { + // checkArgument( + // currentCreator.untilMilestone.isEmpty() + // || + // currentCreator.untilMilestone.get().isGreaterThanOrEqualTo(currentCreator.milestone), + // "Last creator untilMilestone must be greater or equal than milestone in %s", + // schemaId); + // } else { + // checkArgument( + // currentCreator.untilMilestone.isEmpty(), + // "Only last creator is allowed to use until for %s", + // schemaId); + // } + // } + // } + + @Override + public SpecMilestone getEffectiveMilestone(final SpecMilestone milestone) { + return getSchemaCreator(milestone).milestone; + } + + @Override + public T getSchema(final SchemaRegistry registry) { + final SpecMilestone milestone = registry.getMilestone(); + return createSchema(registry, milestone, registry.getSpecConfig()); + } + + @Override + public SchemaId getSchemaId() { + return schemaId; + } + + // static SchemaProviderCreator schemaCreator( + // final SpecMilestone milestone, + // final BiFunction creationSchema) { + // return new SchemaProviderCreator<>(milestone, Optional.empty(), creationSchema); + // } + // + // static SchemaProviderCreator schemaCreator( + // final SpecMilestone milestone, + // final SpecMilestone untilMilestone, + // final BiFunction creationSchema) { + // return new SchemaProviderCreator<>(milestone, Optional.of(untilMilestone), creationSchema); + // } + + protected T createSchema( + final SchemaRegistry registry, + final SpecMilestone effectiveMilestone, + final SpecConfig specConfig) { + return getSchemaCreator(effectiveMilestone).creator.apply(registry, specConfig); + } + + private SchemaProviderCreator getSchemaCreator(final SpecMilestone milestone) { + final SchemaProviderCreator maybeSchemaCreator = milestoneToSchemaCreator.get(milestone); + if (maybeSchemaCreator == null) { + throw new IllegalArgumentException( + "It is not supposed to create a specific version for " + milestone); + } + return maybeSchemaCreator; + } + + @Override + public Set getSupportedMilestones() { + return milestoneToSchemaCreator.keySet(); + } + + protected record SchemaProviderCreator( + SpecMilestone milestone, BiFunction creator) { + + private SchemaProviderCreator withMilestone(SpecMilestone milestone) { + return new SchemaProviderCreator<>(milestone, creator); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this).add("milestone", milestone).toString(); + } + } + + static Builder providerBuilder(final SchemaId schemaId) { + return new Builder<>(schemaId); + } + + static class Builder { + private final SchemaId schemaId; + final List> schemaProviderCreators = new ArrayList<>(); + private SpecMilestone untilMilestone = SpecMilestone.getHighestMilestone(); + private boolean isConstant = false; + + private Builder(final SchemaId schemaId) { + this.schemaId = schemaId; + } + + public Builder withCreator( + final SpecMilestone milestone, + final BiFunction creationSchema) { + checkArgument( + schemaProviderCreators.isEmpty() + || milestone.isGreaterThan(schemaProviderCreators.getLast().milestone), + "Creator's milestones must added in strict ascending order for %s", + schemaId); + + schemaProviderCreators.add(new SchemaProviderCreator<>(milestone, creationSchema)); + return this; + } + + public Builder until(final SpecMilestone untilMilestone) { + this.untilMilestone = untilMilestone; + return this; + } + + public Builder constant(final boolean isConstant) { + this.isConstant = isConstant; + return this; + } + + public BaseSchemaProvider build() { + checkArgument( + !schemaProviderCreators.isEmpty(), "There should be at least 1 creator for %s", schemaId); + + checkArgument( + untilMilestone.isGreaterThanOrEqualTo(schemaProviderCreators.getLast().milestone), + "until must be greater or equal than last creator milestone in %s", + schemaId); + return new BaseSchemaProvider<>(schemaId, schemaProviderCreators, untilMilestone, isConstant); + } + } +} diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java index 90dc718f1b3..34aeffc9e1c 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java @@ -13,19 +13,23 @@ package tech.pegasys.teku.spec.schemas.registry; +import static tech.pegasys.teku.spec.schemas.registry.BaseSchemaProvider.providerBuilder; +import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.ATTESTATION_SCHEMA; +import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.ATTNETS_ENR_FIELD_SCHEMA; + import com.google.common.annotations.VisibleForTesting; import java.util.HashSet; import java.util.Set; - import tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector; import tech.pegasys.teku.infrastructure.ssz.schema.collections.SszBitvectorSchema; import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.config.SpecConfig; +import tech.pegasys.teku.spec.datastructures.operations.Attestation; +import tech.pegasys.teku.spec.datastructures.operations.AttestationSchema; +import tech.pegasys.teku.spec.datastructures.operations.versions.electra.AttestationElectraSchema; +import tech.pegasys.teku.spec.datastructures.operations.versions.phase0.AttestationPhase0Schema; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; -import static tech.pegasys.teku.spec.schemas.registry.AbstractSchemaProvider.providerBuilder; -import static tech.pegasys.teku.spec.schemas.registry.AbstractSchemaProvider.schemaCreator; - public class SchemaRegistryBuilder { private final Set> providers = new HashSet<>(); private final Set> schemaIds = new HashSet<>(); @@ -33,19 +37,39 @@ public class SchemaRegistryBuilder { public static SchemaRegistryBuilder create() { return new SchemaRegistryBuilder() + // PHASE0 .addProvider(createAttnetsENRFieldSchemaProvider()) - .addProvider(new AttestationSchemaProvider()); + .addProvider(createAttestationSchemaProvider()); } - private static SchemaProvider> createAttnetsENRFieldSchemaProvider() { - return providerBuilder(SchemaTypes.ATTNETS_ENR_FIELD_SCHEMA) - .withCreator( - SpecMilestone.PHASE0, - (registry, specConfig) -> - SszBitvectorSchema.create(specConfig.getAttestationSubnetCount())) - .build(); + private static SchemaProvider> + createAttnetsENRFieldSchemaProvider() { + return providerBuilder(ATTNETS_ENR_FIELD_SCHEMA) + .constant(true) + .withCreator( + SpecMilestone.PHASE0, + (registry, specConfig) -> + SszBitvectorSchema.create(specConfig.getAttestationSubnetCount())) + .build(); } + private static SchemaProvider> createAttestationSchemaProvider() { + return providerBuilder(ATTESTATION_SCHEMA) + .withCreator( + SpecMilestone.PHASE0, + (registry, specConfig) -> + new AttestationPhase0Schema(specConfig.getMaxValidatorsPerCommittee()) + .castTypeToAttestationSchema()) + .withCreator( + SpecMilestone.DENEB, + (registry, specConfig) -> + new AttestationElectraSchema( + (long) specConfig.getMaxValidatorsPerCommittee() + * specConfig.getMaxCommitteesPerSlot(), + specConfig.getMaxCommitteesPerSlot()) + .castTypeToAttestationSchema()) + .build(); + } public SchemaRegistryBuilder() { this.cache = SchemaCache.createDefault(); diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java index 59370866624..6d03d40c7e5 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java @@ -23,7 +23,7 @@ import static tech.pegasys.teku.spec.SpecMilestone.CAPELLA; import static tech.pegasys.teku.spec.SpecMilestone.DENEB; import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; -import static tech.pegasys.teku.spec.schemas.registry.AbstractSchemaProvider.schemaCreator; +import static tech.pegasys.teku.spec.schemas.registry.BaseSchemaProvider.providerBuilder; import org.junit.jupiter.api.Test; import tech.pegasys.teku.spec.SpecMilestone; @@ -36,11 +36,13 @@ class BaseSchemaProviderTest { private final SchemaRegistry mockRegistry = mock(SchemaRegistry.class); @Test - void shouldSupportContinuousWithoutUntil() { - final TestSchemaProvider provider = - new TestSchemaProvider( - schemaCreator(ALTAIR, (r, c) -> "TestSchemaAltair"), - schemaCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix")); + void shouldSupportContinuousUntilHighestMilestone() { + final SchemaProvider provider = + providerBuilder(STRING_SCHEMA_ID) + .withCreator(ALTAIR, (r, c) -> "TestSchemaAltair") + .withCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix") + .build(); + assertEquals(ALTAIR, provider.getEffectiveMilestone(ALTAIR)); assertEquals(BELLATRIX, provider.getEffectiveMilestone(BELLATRIX)); @@ -55,11 +57,14 @@ void shouldSupportContinuousWithoutUntil() { } @Test - void shouldSupportContinuousWithUntil() { - final TestSchemaProvider provider = - new TestSchemaProvider( - schemaCreator(PHASE0, (r, c) -> "TestSchemaPhase0"), - schemaCreator(BELLATRIX, CAPELLA, (r, c) -> "TestSchemaBellatrix")); + void shouldSupportContinuousConstantWithUntil() { + final SchemaProvider provider = + providerBuilder(STRING_SCHEMA_ID) + .constant(true) + .withCreator(PHASE0, (r, c) -> "TestSchemaPhase0") + .withCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix") + .until(CAPELLA) + .build(); assertEquals(PHASE0, provider.getEffectiveMilestone(PHASE0)); assertEquals(PHASE0, provider.getEffectiveMilestone(ALTAIR)); @@ -82,86 +87,101 @@ void shouldSupportContinuousWithUntil() { .containsExactly(PHASE0, ALTAIR, BELLATRIX, CAPELLA); } + @Test + void shouldSupportContinuousDefaultVariable() { + final SchemaProvider provider = + providerBuilder(STRING_SCHEMA_ID) + .withCreator(PHASE0, (r, c) -> "TestSchema" + r.getMilestone()) + .until(CAPELLA) + .build(); + + // variable has effective milestone always equal to the milestone + SpecMilestone.getMilestonesUpTo(CAPELLA) + .forEach(milestone -> assertEquals(milestone, provider.getEffectiveMilestone(milestone))); + + when(mockRegistry.getMilestone()).thenReturn(PHASE0); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaPHASE0"); + + when(mockRegistry.getMilestone()).thenReturn(ALTAIR); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaALTAIR"); + + when(mockRegistry.getMilestone()).thenReturn(BELLATRIX); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaBELLATRIX"); + + when(mockRegistry.getMilestone()).thenReturn(CAPELLA); + assertEquals(provider.getSchema(mockRegistry), "TestSchemaCAPELLA"); + + assertThat(provider.getSupportedMilestones()) + .containsExactly(PHASE0, ALTAIR, BELLATRIX, CAPELLA); + } + @Test void shouldAllowSingleMilestone() { - final TestSchemaProvider provider = - new TestSchemaProvider( - schemaCreator(ALTAIR, (r, c) -> "TestSchemaAltair"), - schemaCreator(BELLATRIX,CAPELLA, (r, c) -> "TestSchemaBellatrix")); + final SchemaProvider provider = + providerBuilder(STRING_SCHEMA_ID) + .withCreator(ALTAIR, (r, c) -> "TestSchemaAltair") + .withCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix") + .until(CAPELLA) + .build(); when(mockRegistry.getMilestone()).thenReturn(DENEB); - assertThatThrownBy(() -> provider.getSchema(mockRegistry)).isInstanceOf(IllegalArgumentException.class) - .hasMessage("It is not supposed to create a specific version for DENEB"); + assertThatThrownBy(() -> provider.getSchema(mockRegistry)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("It is not supposed to create a specific version for DENEB"); } @Test void shouldThrowWhenNoCreators() { - assertThatThrownBy(TestSchemaProvider::new) + assertThatThrownBy(() -> providerBuilder(STRING_SCHEMA_ID).build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("There should be at least 1 creator"); } @Test void shouldThrowWhenAskingForAnUnsupportedMilestone() { - assertThatThrownBy(TestSchemaProvider::new) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("There should be at least 1 creator"); - } + final SchemaProvider provider = + providerBuilder(STRING_SCHEMA_ID) + .withCreator(ALTAIR, (r, c) -> "TestSchemaAltair") + .until(ALTAIR) + .build(); - @Test - void shouldThrowWhenNotAscendingMilestonesWithUntil() { - assertThatThrownBy( - () -> - new TestSchemaProvider( - schemaCreator(ALTAIR, (r, c) -> "TestSchema"), - schemaCreator(PHASE0, BELLATRIX, (r, c) -> "TestSchema"))) + when(mockRegistry.getMilestone()).thenReturn(DENEB); + + assertThatThrownBy(() -> provider.getSchema(mockRegistry)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Creator's milestones must be in order"); + .hasMessage("It is not supposed to create a specific version for DENEB"); } @Test - void shouldThrowWhenNotAscendingMilestonesWithoutUntil() { - assertThatThrownBy( - () -> - new TestSchemaProvider( - schemaCreator(PHASE0, (r, c) -> "TestSchema"), - schemaCreator(PHASE0, (r, c) -> "TestSchema"))) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Creator's milestones must be in order"); - + void shouldThrowWhenNotAscendingMilestones() { assertThatThrownBy( () -> - new TestSchemaProvider( - schemaCreator(DENEB, (r, c) -> "TestSchema"), - schemaCreator(ALTAIR, (r, c) -> "TestSchema"))) + providerBuilder(STRING_SCHEMA_ID) + .withCreator(PHASE0, (r, c) -> "TestSchema") + .withCreator(PHASE0, (r, c) -> "TestSchema")) .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Creator's milestones must be in order"); - } + .hasMessageStartingWith("Creator's milestones must added in strict ascending order"); - @Test - void shouldThrowWhenWithUntilNotAsLast() { assertThatThrownBy( () -> - new TestSchemaProvider( - schemaCreator(ALTAIR, BELLATRIX, (r, c) -> "TestSchema"), - schemaCreator(CAPELLA, (r, c) -> "TestSchema"))) + providerBuilder(STRING_SCHEMA_ID) + .withCreator(ALTAIR, (r, c) -> "TestSchema") + .withCreator(PHASE0, (r, c) -> "TestSchema")) .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Only last creator is allowed to use until"); + .hasMessageStartingWith("Creator's milestones must added in strict ascending order"); } @Test void shouldThrowWhenWithUntilIsPriorToMilestone() { assertThatThrownBy( - () -> new TestSchemaProvider(schemaCreator(CAPELLA, BELLATRIX, (r, c) -> "TestSchema"))) + () -> + providerBuilder(STRING_SCHEMA_ID) + .withCreator(PHASE0, (r, c) -> "TestSchema") + .withCreator(CAPELLA, (r, c) -> "TestSchema") + .until(ALTAIR) + .build()) .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Last creator untilMilestone must be greater than milestone in"); - } - - private static class TestSchemaProvider extends AbstractSchemaProvider { - @SafeVarargs - TestSchemaProvider(final SchemaProviderCreator... schemaProviderCreators) { - super(STRING_SCHEMA_ID, schemaProviderCreators); - } + .hasMessageStartingWith("until must be greater or equal than last creator milestone"); } } From 6c4fd402068996f022b88973f09fcf4aedb1c6ce Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 14 Oct 2024 12:04:08 +0200 Subject: [PATCH 07/10] cleanups --- .../schemas/registry/BaseSchemaProvider.java | 43 ------------------- 1 file changed, 43 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java index 674d96616af..91642b3e525 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java @@ -36,7 +36,6 @@ private BaseSchemaProvider( final SpecMilestone untilMilestone, final boolean isConstant) { this.schemaId = schemaId; - // validateCreators(schemaProviderCreators); final List> creatorsList = new ArrayList<>(schemaProviderCreators); SchemaProviderCreator lastCreator = null; @@ -53,35 +52,6 @@ private BaseSchemaProvider( } } - // private void validateCreators(final List> schemaProviderCreators) { - // checkArgument( - // !schemaProviderCreators.isEmpty(), "There should be at least 1 creator for %s", - // schemaId); - // for (int i = 0; i < schemaProviderCreators.size(); i++) { - // final SchemaProviderCreator currentCreator = schemaProviderCreators.get(i); - // if (i > 0) { - // checkArgument( - // currentCreator.milestone.isGreaterThan(schemaProviderCreators.get(i - 1).milestone), - // "Creator's milestones must be in order for %s", - // schemaId); - // } - // final boolean isLast = i == schemaProviderCreators.size() - 1; - // if (isLast) { - // checkArgument( - // currentCreator.untilMilestone.isEmpty() - // || - // currentCreator.untilMilestone.get().isGreaterThanOrEqualTo(currentCreator.milestone), - // "Last creator untilMilestone must be greater or equal than milestone in %s", - // schemaId); - // } else { - // checkArgument( - // currentCreator.untilMilestone.isEmpty(), - // "Only last creator is allowed to use until for %s", - // schemaId); - // } - // } - // } - @Override public SpecMilestone getEffectiveMilestone(final SpecMilestone milestone) { return getSchemaCreator(milestone).milestone; @@ -98,19 +68,6 @@ public SchemaId getSchemaId() { return schemaId; } - // static SchemaProviderCreator schemaCreator( - // final SpecMilestone milestone, - // final BiFunction creationSchema) { - // return new SchemaProviderCreator<>(milestone, Optional.empty(), creationSchema); - // } - // - // static SchemaProviderCreator schemaCreator( - // final SpecMilestone milestone, - // final SpecMilestone untilMilestone, - // final BiFunction creationSchema) { - // return new SchemaProviderCreator<>(milestone, Optional.of(untilMilestone), creationSchema); - // } - protected T createSchema( final SchemaRegistry registry, final SpecMilestone effectiveMilestone, From 4075bbad9a078cb54af76b7ae008bcd3d64af62d Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 14 Oct 2024 12:18:56 +0200 Subject: [PATCH 08/10] fix final --- .../pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java index 91642b3e525..543a9ff2d78 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java @@ -92,7 +92,7 @@ public Set getSupportedMilestones() { protected record SchemaProviderCreator( SpecMilestone milestone, BiFunction creator) { - private SchemaProviderCreator withMilestone(SpecMilestone milestone) { + private SchemaProviderCreator withMilestone(final SpecMilestone milestone) { return new SchemaProviderCreator<>(milestone, creator); } From 9361b6b5be3959f6aa50dad35ebd234c50dbca46 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 14 Oct 2024 13:40:54 +0200 Subject: [PATCH 09/10] add constant to attestation --- .../teku/spec/schemas/registry/SchemaRegistryBuilder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java index 34aeffc9e1c..07947be9225 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java @@ -55,6 +55,7 @@ public static SchemaRegistryBuilder create() { private static SchemaProvider> createAttestationSchemaProvider() { return providerBuilder(ATTESTATION_SCHEMA) + .constant(true) .withCreator( SpecMilestone.PHASE0, (registry, specConfig) -> From e69e1d6c4175625e6a2ba2d24e3d19b437535be1 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 14 Oct 2024 15:01:02 +0200 Subject: [PATCH 10/10] apply suggestions --- .../schemas/registry/BaseSchemaProvider.java | 4 ++-- .../registry/SchemaRegistryBuilder.java | 4 ++-- .../registry/BaseSchemaProviderTest.java | 18 +----------------- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java index 543a9ff2d78..95eeceda456 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java @@ -134,8 +134,8 @@ public Builder until(final SpecMilestone untilMilestone) { return this; } - public Builder constant(final boolean isConstant) { - this.isConstant = isConstant; + public Builder constant() { + this.isConstant = true; return this; } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java index 07947be9225..ddf5971ebf1 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java @@ -45,7 +45,7 @@ public static SchemaRegistryBuilder create() { private static SchemaProvider> createAttnetsENRFieldSchemaProvider() { return providerBuilder(ATTNETS_ENR_FIELD_SCHEMA) - .constant(true) + .constant() .withCreator( SpecMilestone.PHASE0, (registry, specConfig) -> @@ -55,7 +55,7 @@ public static SchemaRegistryBuilder create() { private static SchemaProvider> createAttestationSchemaProvider() { return providerBuilder(ATTESTATION_SCHEMA) - .constant(true) + .constant() .withCreator( SpecMilestone.PHASE0, (registry, specConfig) -> diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java index 6d03d40c7e5..54a5196219f 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java @@ -60,7 +60,7 @@ void shouldSupportContinuousUntilHighestMilestone() { void shouldSupportContinuousConstantWithUntil() { final SchemaProvider provider = providerBuilder(STRING_SCHEMA_ID) - .constant(true) + .constant() .withCreator(PHASE0, (r, c) -> "TestSchemaPhase0") .withCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix") .until(CAPELLA) @@ -115,22 +115,6 @@ void shouldSupportContinuousDefaultVariable() { .containsExactly(PHASE0, ALTAIR, BELLATRIX, CAPELLA); } - @Test - void shouldAllowSingleMilestone() { - final SchemaProvider provider = - providerBuilder(STRING_SCHEMA_ID) - .withCreator(ALTAIR, (r, c) -> "TestSchemaAltair") - .withCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix") - .until(CAPELLA) - .build(); - - when(mockRegistry.getMilestone()).thenReturn(DENEB); - - assertThatThrownBy(() -> provider.getSchema(mockRegistry)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("It is not supposed to create a specific version for DENEB"); - } - @Test void shouldThrowWhenNoCreators() { assertThatThrownBy(() -> providerBuilder(STRING_SCHEMA_ID).build())