Skip to content

Commit

Permalink
fix gossip subscriptions (#8896)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tbenr
tbenr authored Dec 9, 2024
1 parent 16fcabd commit 4ac3018
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 37 deletions.
10 changes: 7 additions & 3 deletions .../src/main/java/tech/pegasys/teku/networking/eth2/gossip/topics/Eth2GossipTopicFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import tech.pegasys.teku.networking.p2p.libp2p.gossip.GossipTopicFilter;
import tech.pegasys.teku.spec.Spec;
import tech.pegasys.teku.spec.SpecMilestone;
import tech.pegasys.teku.spec.SpecVersion;
import tech.pegasys.teku.spec.datastructures.state.ForkInfo;
import tech.pegasys.teku.storage.client.RecentChainData;

Expand Down Expand Up @@ -57,15 +58,18 @@ private Set<String> computeRelevantTopics(
recentChainData.getMilestoneByForkDigest(forkDigest).orElseThrow();
final Set<String> topics = getAllTopics(gossipEncoding, forkDigest, spec, specMilestone);
spec.getForkSchedule().getForks().stream()
.filter(fork -> fork.getEpoch().isGreaterThanOrEqualTo(forkInfo.getFork().getEpoch()))
.filter(fork -> fork.getEpoch().isGreaterThan(forkInfo.getFork().getEpoch()))
.forEach(
futureFork -> {
final SpecVersion futureSpecVersion = spec.atEpoch(futureFork.getEpoch());
final Bytes4 futureForkDigest =
spec.atEpoch(futureFork.getEpoch())
futureSpecVersion
.miscHelpers()
.computeForkDigest(
futureFork.getCurrentVersion(), forkInfo.getGenesisValidatorsRoot());
topics.addAll(getAllTopics(gossipEncoding, futureForkDigest, spec, specMilestone));
topics.addAll(
getAllTopics(
gossipEncoding, futureForkDigest, spec, futureSpecVersion.getMilestone()));
});
return topics;
}
Expand Down
87 changes: 53 additions & 34 deletions .../test/java/tech/pegasys/teku/networking/eth2/gossip/topics/Eth2GossipTopicFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,20 @@
package tech.pegasys.teku.networking.eth2.gossip.topics;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assumptions.assumeThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.spy;
import static tech.pegasys.teku.networking.eth2.gossip.encoding.GossipEncoding.SSZ_SNAPPY;
import static tech.pegasys.teku.networking.eth2.gossip.topics.GossipTopicName.getAttestationSubnetTopicName;
import static tech.pegasys.teku.networking.eth2.gossip.topics.GossipTopicName.getBlobSidecarSubnetTopicName;
import static tech.pegasys.teku.networking.eth2.gossip.topics.GossipTopicName.getSyncCommitteeSubnetTopicName;
import static tech.pegasys.teku.spec.SpecMilestone.DENEB;
import static tech.pegasys.teku.spec.SpecMilestone.ELECTRA;
import static tech.pegasys.teku.spec.constants.NetworkConstants.SYNC_COMMITTEE_SUBNET_COUNT;

import java.util.List;
import java.util.Optional;
import org.apache.tuweni.bytes.Bytes32;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.TestTemplate;
import tech.pegasys.teku.infrastructure.bytes.Bytes4;
Expand All @@ -34,58 +36,60 @@
import tech.pegasys.teku.spec.SpecMilestone;
import tech.pegasys.teku.spec.TestSpecContext;
import tech.pegasys.teku.spec.TestSpecFactory;
import tech.pegasys.teku.spec.TestSpecInvocationContextProvider;
import tech.pegasys.teku.spec.TestSpecInvocationContextProvider.SpecContext;
import tech.pegasys.teku.spec.config.SpecConfig;
import tech.pegasys.teku.spec.config.SpecConfigDeneb;
import tech.pegasys.teku.spec.datastructures.state.Fork;
import tech.pegasys.teku.spec.datastructures.state.ForkInfo;
import tech.pegasys.teku.spec.util.DataStructureUtil;
import tech.pegasys.teku.storage.client.RecentChainData;
import tech.pegasys.teku.storage.storageSystem.InMemoryStorageSystemBuilder;
import tech.pegasys.teku.storage.storageSystem.StorageSystem;

@TestSpecContext(milestone = {SpecMilestone.DENEB, SpecMilestone.ELECTRA})
@TestSpecContext(milestone = {DENEB, ELECTRA})
class Eth2GossipTopicFilterTest {

private final RecentChainData recentChainData = mock(RecentChainData.class);
private final UInt64 currentForkEpoch = UInt64.valueOf(10);
private final UInt64 nextMilestoneForkEpoch = UInt64.valueOf(10);
private RecentChainData recentChainData;
private Spec spec;
private SpecMilestone specMilestone;
private DataStructureUtil dataStructureUtil;
private ForkInfo forkInfo;
private SpecMilestone currentSpecMilestone;
private SpecMilestone nextSpecMilestone;
private ForkInfo currentForkInfo;
private Eth2GossipTopicFilter filter;
private Bytes4 currentForkDigest;
private Bytes4 nextForkDigest;

@BeforeEach
void setUp(final TestSpecInvocationContextProvider.SpecContext specContext) {
specMilestone = specContext.getSpecMilestone();
void setUp(final SpecContext specContext) {
// we set up a spec that will be transitioning to specContext.getSpecMilestone() in
// currentForkEpoch epochs
// current milestone is actually the previous milestone
currentSpecMilestone = specContext.getSpecMilestone().getPreviousMilestone();
nextSpecMilestone = specContext.getSpecMilestone();
spec =
switch (specContext.getSpecMilestone()) {
switch (nextSpecMilestone) {
case PHASE0 -> throw new IllegalArgumentException("Phase0 is an unsupported milestone");
case ALTAIR -> throw new IllegalArgumentException("Altair is an unsupported milestone");
case BELLATRIX ->
throw new IllegalArgumentException("Bellatrix is an unsupported milestone");
case CAPELLA -> throw new IllegalArgumentException("Capella is an unsupported milestone");
case DENEB -> TestSpecFactory.createMinimalWithDenebForkEpoch(currentForkEpoch);
case ELECTRA -> TestSpecFactory.createMinimalWithElectraForkEpoch(currentForkEpoch);
case DENEB -> TestSpecFactory.createMinimalWithDenebForkEpoch(nextMilestoneForkEpoch);
case ELECTRA -> TestSpecFactory.createMinimalWithElectraForkEpoch(nextMilestoneForkEpoch);
};
dataStructureUtil = new DataStructureUtil(spec);

final StorageSystem storageSystem = InMemoryStorageSystemBuilder.buildDefault(spec);
storageSystem.chainUpdater().initializeGenesis();

recentChainData = spy(storageSystem.recentChainData());
filter = new Eth2GossipTopicFilter(recentChainData, SSZ_SNAPPY, spec);

final Bytes32 genesisValidatorsRoot = dataStructureUtil.randomBytes32();
final List<Fork> forks = spec.getForkSchedule().getForks();
forkInfo = new ForkInfo(forks.get(0), genesisValidatorsRoot);
currentForkDigest = forkInfo.getForkDigest(spec);
currentForkInfo = recentChainData.getCurrentForkInfo().orElseThrow();

final Fork nextFork = forks.get(1);
nextForkDigest =
spec.atEpoch(nextFork.getEpoch())
.miscHelpers()
.computeForkDigest(nextFork.getCurrentVersion(), genesisValidatorsRoot);

when(recentChainData.getCurrentForkInfo()).thenReturn(Optional.of(forkInfo));
when(recentChainData.getNextFork(forkInfo.getFork())).thenReturn(Optional.of(nextFork));
when(recentChainData.getMilestoneByForkDigest(currentForkDigest))
.thenReturn(Optional.of(specMilestone));
.computeForkDigest(
nextFork.getCurrentVersion(),
recentChainData.getGenesisData().orElseThrow().getGenesisValidatorsRoot());
}

@TestTemplate
Expand All @@ -95,7 +99,7 @@ void shouldNotAllowIrrelevantTopics() {

@TestTemplate
void shouldNotRequireNextForkToBePresent() {
when(recentChainData.getNextFork(any())).thenReturn(Optional.empty());
doAnswer(invocation -> Optional.empty()).when(recentChainData).getNextFork(any());
assertThat(filter.isRelevantTopic(getTopicName(GossipTopicName.BEACON_BLOCK))).isTrue();
}

Expand Down Expand Up @@ -129,18 +133,33 @@ void shouldConsiderAllSyncCommitteeSubnetsRelevant() {
}

@TestTemplate
void shouldConsiderAllBlobSidecarSubnetsRelevant() {
final SpecConfig config = spec.forMilestone(SpecMilestone.DENEB).getConfig();
void shouldConsiderAllBlobSidecarSubnetsRelevantForCurrentMilestone() {
final SpecConfig config = spec.forMilestone(currentSpecMilestone).getConfig();
assumeThat(config.toVersionDeneb()).isPresent();
final SpecConfigDeneb specConfigDeneb = SpecConfigDeneb.required(config);
for (int i = 0; i < specConfigDeneb.getBlobSidecarSubnetCount(); i++) {
assertThat(filter.isRelevantTopic(getTopicName(getBlobSidecarSubnetTopicName(i)))).isTrue();
}
}

@TestTemplate
void shouldConsiderAllBlobSidecarSubnetsRelevantForNextMilestone() {
final SpecConfig config = spec.forMilestone(nextSpecMilestone).getConfig();
assumeThat(config.toVersionDeneb()).isPresent();
final SpecConfigDeneb specConfigDeneb = SpecConfigDeneb.required(config);
for (int i = 0; i < specConfigDeneb.getBlobSidecarSubnetCount(); i++) {
assertThat(filter.isRelevantTopic(getNextForkTopicName(getBlobSidecarSubnetTopicName(i))))
.isTrue();
}
}

@TestTemplate
void shouldNotConsiderBlobSidecarWithIncorrectSubnetIdRelevant() {
final int blobSidecarSubnetCount =
SpecConfigDeneb.required(spec.forMilestone(specMilestone).getConfig())
spec.forMilestone(nextSpecMilestone)
.getConfig()
.toVersionDeneb()
.orElseThrow()
.getBlobSidecarSubnetCount();
assertThat(
filter.isRelevantTopic(
Expand All @@ -162,11 +181,11 @@ void shouldNotAllowTopicsWithUnknownForkDigest() {
}

private String getTopicName(final GossipTopicName name) {
return GossipTopics.getTopic(forkInfo.getForkDigest(spec), name, SSZ_SNAPPY);
return GossipTopics.getTopic(currentForkInfo.getForkDigest(spec), name, SSZ_SNAPPY);
}

private String getTopicName(final String name) {
return GossipTopics.getTopic(forkInfo.getForkDigest(spec), name, SSZ_SNAPPY);
return GossipTopics.getTopic(currentForkInfo.getForkDigest(spec), name, SSZ_SNAPPY);
}

private String getNextForkTopicName(final GossipTopicName name) {
Expand Down

0 comments on commit 4ac3018

Please sign in to comment.