From a0dd88a64409cbb357359ad1ac09ac2365146c18 Mon Sep 17 00:00:00 2001 From: Austin Littley <102969658+alittley@users.noreply.github.com> Date: Tue, 7 Nov 2023 12:29:11 -0500 Subject: [PATCH] 09674 Rework intake components for framework compatibility (#9706) Signed-off-by: Austin Littley Signed-off-by: Ivo Yankov --- .../deduplication/EventDeduplicator.java | 26 +++--- .../platform/event/linking/InOrderLinker.java | 25 ++---- .../validation/EventSignatureValidator.java | 21 ++--- .../validation/InternalEventValidator.java | 20 ++--- .../EventSignatureValidatorScheduler.java | 2 +- .../InternalEventValidatorScheduler.java | 2 +- .../event/EventDeduplicatorTests.java | 46 ++++++---- .../event/linking/InOrderLinkerTests.java | 47 +++------- .../EventSignatureValidatorTests.java | 89 +++++++------------ .../InternalEventValidatorTests.java | 77 +++++++--------- 10 files changed, 141 insertions(+), 214 deletions(-) diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/deduplication/EventDeduplicator.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/deduplication/EventDeduplicator.java index 806649fa57b8..317173c44b80 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/deduplication/EventDeduplicator.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/deduplication/EventDeduplicator.java @@ -26,11 +26,11 @@ import com.swirlds.platform.event.GossipEvent; import com.swirlds.platform.gossip.IntakeEventCounter; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import java.nio.ByteBuffer; import java.util.HashSet; import java.util.Objects; import java.util.Set; -import java.util.function.Consumer; import java.util.function.Function; /** @@ -50,11 +50,6 @@ public class EventDeduplicator { */ private static final int INITIAL_CAPACITY = 1024; - /** - * Deduplicated events are passed to this consumer. - */ - private final Consumer eventConsumer; - /** * The current minimum generation required for an event to be non-ancient. */ @@ -88,15 +83,11 @@ public class EventDeduplicator { * Constructor * * @param platformContext the platform context - * @param eventConsumer deduplicated events are passed to this consumer * @param intakeEventCounter keeps track of the number of events in the intake pipeline from each peer */ public EventDeduplicator( - @NonNull final PlatformContext platformContext, - @NonNull final Consumer eventConsumer, - @NonNull final IntakeEventCounter intakeEventCounter) { + @NonNull final PlatformContext platformContext, @NonNull final IntakeEventCounter intakeEventCounter) { - this.eventConsumer = Objects.requireNonNull(eventConsumer); this.intakeEventCounter = Objects.requireNonNull(intakeEventCounter); this.duplicateEventAccumulator = platformContext.getMetrics().getOrCreate(DUPLICATE_EVENT_CONFIG); @@ -106,16 +97,17 @@ public EventDeduplicator( /** * Handle a potentially duplicate event *

- * Ancient events are ignored. If the input event has not already been observed by this deduplicator, it is passed - * to the event consumer. + * Ancient events are ignored. If the input event has not already been observed by this deduplicator, it is returned. * * @param event the event to handle + * @return the event if it is not a duplicate, or null if it is a duplicate */ - public void handleEvent(@NonNull final GossipEvent event) { + @Nullable + public GossipEvent handleEvent(@NonNull final GossipEvent event) { if (event.getGeneration() < minimumGenerationNonAncient) { // Ancient events can be safely ignored. intakeEventCounter.eventExitedIntakePipeline(event.getSenderId()); - return; + return null; } final Set signatures = observedEvents.computeIfAbsent(event.getDescriptor(), NEW_HASH_SET); @@ -125,11 +117,13 @@ public void handleEvent(@NonNull final GossipEvent event) { disparateSignatureAccumulator.update(1); } - eventConsumer.accept(event); + return event; } else { // duplicate descriptor and signature duplicateEventAccumulator.update(1); intakeEventCounter.eventExitedIntakePipeline(event.getSenderId()); + + return null; } } diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/linking/InOrderLinker.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/linking/InOrderLinker.java index 1df19676b0fe..5248cd5e7e89 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/linking/InOrderLinker.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/linking/InOrderLinker.java @@ -28,10 +28,10 @@ import com.swirlds.platform.gossip.IntakeEventCounter; import com.swirlds.platform.internal.EventImpl; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.function.Consumer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -68,11 +68,6 @@ public class InOrderLinker { */ private final Map parentHashMap = new HashMap<>(INITIAL_CAPACITY); - /** - * Linked events are passed to this consumer. - */ - private final Consumer eventConsumer; - /** * The current minimum generation required for an event to be non-ancient. */ @@ -86,13 +81,9 @@ public class InOrderLinker { /** * Constructor * - * @param eventConsumer the consumer that successfully linked events are passed to * @param intakeEventCounter keeps track of the number of events in the intake pipeline from each peer */ - public InOrderLinker( - @NonNull final Consumer eventConsumer, @NonNull final IntakeEventCounter intakeEventCounter) { - - this.eventConsumer = Objects.requireNonNull(eventConsumer); + public InOrderLinker(@NonNull final IntakeEventCounter intakeEventCounter) { this.intakeEventCounter = Objects.requireNonNull(intakeEventCounter); } @@ -100,12 +91,14 @@ public InOrderLinker( * Find and link the parents of the given event. * * @param event the event to link + * @return the linked event, or null if linking fails */ - public void linkEvent(@NonNull final GossipEvent event) { + @Nullable + public EventImpl linkEvent(@NonNull final GossipEvent event) { if (event.getGeneration() < minimumGenerationNonAncient) { // This event is ancient, so we don't need to link it. this.intakeEventCounter.eventExitedIntakePipeline(event.getSenderId()); - return; + return null; } final BaseEventHashedData hashedData = event.getHashedData(); @@ -124,7 +117,7 @@ public void linkEvent(@NonNull final GossipEvent event) { selfParentHash, selfParentGen); this.intakeEventCounter.eventExitedIntakePipeline(event.getSenderId()); - return; + return null; } } else { // ancient parents don't need to be linked @@ -145,7 +138,7 @@ public void linkEvent(@NonNull final GossipEvent event) { otherParentHash, otherParentGen); this.intakeEventCounter.eventExitedIntakePipeline(event.getSenderId()); - return; + return null; } } else { // ancient parents don't need to be linked @@ -159,7 +152,7 @@ public void linkEvent(@NonNull final GossipEvent event) { parentDescriptorMap.put(eventDescriptor, linkedEvent); parentHashMap.put(eventDescriptor.getHash(), linkedEvent); - eventConsumer.accept(linkedEvent); + return linkedEvent; } /** diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/validation/EventSignatureValidator.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/validation/EventSignatureValidator.java index dae81aa400cd..5468157450fc 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/validation/EventSignatureValidator.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/validation/EventSignatureValidator.java @@ -36,7 +36,6 @@ import java.security.PublicKey; import java.time.Duration; import java.util.Objects; -import java.util.function.Consumer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -59,11 +58,6 @@ public class EventSignatureValidator { */ private final SignatureVerifier signatureVerifier; - /** - * Events with valid signature are passed to this consumer. - */ - private final Consumer eventConsumer; - /** * The previous address book. May be null. */ @@ -109,7 +103,6 @@ public class EventSignatureValidator { * @param currentSoftwareVersion the current software version * @param previousAddressBook the previous address book * @param currentAddressBook the current address book - * @param eventConsumer validated events are passed to this consumer * @param intakeEventCounter keeps track of the number of events in the intake pipeline from each peer */ public EventSignatureValidator( @@ -119,7 +112,6 @@ public EventSignatureValidator( @NonNull final SoftwareVersion currentSoftwareVersion, @Nullable final AddressBook previousAddressBook, @NonNull final AddressBook currentAddressBook, - @NonNull final Consumer eventConsumer, @NonNull final IntakeEventCounter intakeEventCounter) { Objects.requireNonNull(time); @@ -128,7 +120,6 @@ public EventSignatureValidator( this.currentSoftwareVersion = Objects.requireNonNull(currentSoftwareVersion); this.previousAddressBook = previousAddressBook; this.currentAddressBook = Objects.requireNonNull(currentAddressBook); - this.eventConsumer = Objects.requireNonNull(eventConsumer); this.intakeEventCounter = Objects.requireNonNull(intakeEventCounter); this.rateLimitedLogger = new RateLimitedLogger(logger, time, MINIMUM_LOG_PERIOD); @@ -224,22 +215,26 @@ private boolean isSignatureValid(@NonNull final GossipEvent event) { } /** - * Verify event signature + * Validate event signature * * @param event the event to verify the signature of + * @return the event if the signature is valid, otherwise null */ - public void validateEventSignature(@NonNull final GossipEvent event) { + @Nullable + public GossipEvent validateSignature(@NonNull final GossipEvent event) { if (event.getGeneration() < minimumGenerationNonAncient) { // ancient events can be safely ignored intakeEventCounter.eventExitedIntakePipeline(event.getSenderId()); - return; + return null; } if (isSignatureValid(event)) { - eventConsumer.accept(event); + return event; } else { intakeEventCounter.eventExitedIntakePipeline(event.getSenderId()); validationFailedAccumulator.update(1); + + return null; } } diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/validation/InternalEventValidator.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/validation/InternalEventValidator.java index 8232b03d7659..aa1e2e6db020 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/validation/InternalEventValidator.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/validation/InternalEventValidator.java @@ -31,9 +31,9 @@ import com.swirlds.platform.event.GossipEvent; import com.swirlds.platform.gossip.IntakeEventCounter; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import java.time.Duration; import java.util.Objects; -import java.util.function.Consumer; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -53,11 +53,6 @@ public class InternalEventValidator { */ private final boolean singleNodeNetwork; - /** - * Valid events are passed to this consumer. - */ - private final Consumer eventConsumer; - /** * Keeps track of the number of events in the intake pipeline from each peer */ @@ -87,20 +82,17 @@ public class InternalEventValidator { * @param platformContext the platform context * @param time a time object, for rate limiting logging * @param singleNodeNetwork true if this node is in a single-node network, otherwise false - * @param eventConsumer validated events are passed to this consumer * @param intakeEventCounter keeps track of the number of events in the intake pipeline from each peer */ public InternalEventValidator( @NonNull final PlatformContext platformContext, @NonNull final Time time, final boolean singleNodeNetwork, - @NonNull final Consumer eventConsumer, @NonNull final IntakeEventCounter intakeEventCounter) { Objects.requireNonNull(time); this.singleNodeNetwork = singleNodeNetwork; - this.eventConsumer = Objects.requireNonNull(eventConsumer); this.intakeEventCounter = Objects.requireNonNull(intakeEventCounter); this.transactionConfig = platformContext.getConfiguration().getConfigData(TransactionConfig.class); @@ -271,18 +263,22 @@ private boolean isEventGenerationValid(@NonNull final GossipEvent event) { /** * Validate the internal data integrity of an event. *

- * If the event is determined to be valid, it is passed to the event consumer. + * If the event is determined to be valid, it is returned. * * @param event the event to validate + * @return the event if it is valid, otherwise null */ - public void handleEvent(@NonNull final GossipEvent event) { + @Nullable + public GossipEvent validateEvent(@NonNull final GossipEvent event) { if (areRequiredFieldsNonNull(event) && isTransactionByteCountValid(event) && areParentsInternallyConsistent(event) && isEventGenerationValid(event)) { - eventConsumer.accept(event); + return event; } else { intakeEventCounter.eventExitedIntakePipeline(event.getSenderId()); + + return null; } } } diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/EventSignatureValidatorScheduler.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/EventSignatureValidatorScheduler.java index 1e8b0a630318..ed89c2369e75 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/EventSignatureValidatorScheduler.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/EventSignatureValidatorScheduler.java @@ -88,7 +88,7 @@ public OutputWire getEventOutput() { * @param eventSignatureValidator the event signature validator to bind */ public void bind(@NonNull final EventSignatureValidator eventSignatureValidator) { - eventInput.bind(eventSignatureValidator::validateEventSignature); + eventInput.bind(eventSignatureValidator::validateSignature); minimumGenerationNonAncientInput.bind(eventSignatureValidator::setMinimumGenerationNonAncient); } } diff --git a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/InternalEventValidatorScheduler.java b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/InternalEventValidatorScheduler.java index 9f405d7aa785..0dfafc9a2603 100644 --- a/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/InternalEventValidatorScheduler.java +++ b/platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/InternalEventValidatorScheduler.java @@ -74,6 +74,6 @@ public OutputWire getEventOutput() { * @param internalEventValidator the validator to bind */ public void bind(@NonNull final InternalEventValidator internalEventValidator) { - eventInput.bind(internalEventValidator::handleEvent); + eventInput.bind(internalEventValidator::validateEvent); } } diff --git a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/EventDeduplicatorTests.java b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/EventDeduplicatorTests.java index e8dac07b95f8..2974373e5f12 100644 --- a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/EventDeduplicatorTests.java +++ b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/EventDeduplicatorTests.java @@ -35,13 +35,13 @@ import com.swirlds.platform.gossip.IntakeEventCounter; import com.swirlds.test.framework.context.TestPlatformContextBuilder; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Random; import java.util.Set; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -95,23 +95,27 @@ private GossipEvent createGossipEvent( return event; } + private static void validateEmittedEvent( + @Nullable final GossipEvent event, + final long minimumGenerationNonAncient, + @NonNull final Set emittedEvents) { + if (event != null) { + assertFalse(event.getGeneration() < minimumGenerationNonAncient, "Ancient events shouldn't be emitted"); + + assertTrue(emittedEvents.add(event), "Event was emitted twice"); + } + } + @Test @DisplayName("Test standard event deduplicator operation") void standardOperation() { - final AtomicLong minimumGenerationNonAncient = new AtomicLong(0); + long minimumGenerationNonAncient = 0; // events that have been emitted from the deduplicator final Set emittedEvents = new HashSet<>(); // events that have been submitted to the deduplicator final List submittedEvents = new ArrayList<>(); - final Consumer eventConsumer = event -> { - assertFalse( - event.getGeneration() < minimumGenerationNonAncient.get(), "Ancient events shouldn't be emitted"); - - assertTrue(emittedEvents.add(event), "Event was emitted twice"); - }; - final AtomicLong eventsExitedIntakePipeline = new AtomicLong(0); final IntakeEventCounter intakeEventCounter = mock(IntakeEventCounter.class); doAnswer(invocation -> { @@ -122,7 +126,7 @@ void standardOperation() { .eventExitedIntakePipeline(any()); final EventDeduplicator deduplicator = - new EventDeduplicator(TestPlatformContextBuilder.create().build(), eventConsumer, intakeEventCounter); + new EventDeduplicator(TestPlatformContextBuilder.create().build(), intakeEventCounter); int duplicateEventCount = 0; int ancientEventCount = 0; @@ -132,9 +136,9 @@ void standardOperation() { // submit a brand new event half the time final Hash eventHash = randomHash(random); final NodeId creatorId = new NodeId(random.nextInt(NODE_ID_COUNT)); - final long eventGeneration = Math.max(0, minimumGenerationNonAncient.get() + random.nextInt(-1, 10)); + final long eventGeneration = Math.max(0, minimumGenerationNonAncient + random.nextInt(-1, 10)); - if (eventGeneration < minimumGenerationNonAncient.get()) { + if (eventGeneration < minimumGenerationNonAncient) { ancientEventCount++; } @@ -144,13 +148,17 @@ void standardOperation() { eventGeneration, randomSignature(random).getSignatureBytes()); - deduplicator.handleEvent(newEvent); + validateEmittedEvent(deduplicator.handleEvent(newEvent), minimumGenerationNonAncient, emittedEvents); + submittedEvents.add(newEvent); } else if (random.nextBoolean()) { // submit a duplicate event 25% of the time duplicateEventCount++; - deduplicator.handleEvent(submittedEvents.get(random.nextInt(submittedEvents.size()))); + validateEmittedEvent( + deduplicator.handleEvent(submittedEvents.get(random.nextInt(submittedEvents.size()))), + minimumGenerationNonAncient, + emittedEvents); } else { // submit a duplicate event with a different signature 25% of the time final GossipEvent duplicateEvent = submittedEvents.get(random.nextInt(submittedEvents.size())); @@ -160,15 +168,19 @@ void standardOperation() { duplicateEvent.getDescriptor().getGeneration(), randomSignature(random).getSignatureBytes()); - if (duplicateEvent.getDescriptor().getGeneration() < minimumGenerationNonAncient.get()) { + if (duplicateEvent.getDescriptor().getGeneration() < minimumGenerationNonAncient) { ancientEventCount++; } - deduplicator.handleEvent(eventWithDisparateSignature); + validateEmittedEvent( + deduplicator.handleEvent(eventWithDisparateSignature), + minimumGenerationNonAncient, + emittedEvents); } if (random.nextBoolean()) { - deduplicator.setMinimumGenerationNonAncient(minimumGenerationNonAncient.addAndGet(1)); + minimumGenerationNonAncient++; + deduplicator.setMinimumGenerationNonAncient(minimumGenerationNonAncient); } } diff --git a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/linking/InOrderLinkerTests.java b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/linking/InOrderLinkerTests.java index 3838730309b7..edd37b342ec0 100644 --- a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/linking/InOrderLinkerTests.java +++ b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/linking/InOrderLinkerTests.java @@ -20,6 +20,8 @@ import static com.swirlds.common.test.fixtures.RandomUtils.randomHash; import static com.swirlds.platform.event.EventConstants.GENERATION_UNDEFINED; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -31,13 +33,10 @@ import com.swirlds.common.system.events.EventDescriptor; import com.swirlds.platform.event.GossipEvent; import com.swirlds.platform.gossip.IntakeEventCounter; -import com.swirlds.platform.internal.EventImpl; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import java.util.Random; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -46,12 +45,6 @@ * Tests the {@link InOrderLinker} class */ class InOrderLinkerTests { - /** - * How many events have been consumed by the consumer. This value includes to 2 genesis events submitted to the - * linker in the {@link #setup()} method. - */ - private AtomicInteger consumedEventCount; - private AtomicLong exitedIntakePipelineCount; private Random random; @@ -60,11 +53,6 @@ class InOrderLinkerTests { private GossipEvent genesisSelfParent; private GossipEvent genesisOtherParent; - /** - * The number of events handled in the setup method. Included as a constant for assertion clarity - */ - private final int genesisEventCount = 2; - /** * Generates a mock event with the given parameters * @@ -76,7 +64,7 @@ class InOrderLinkerTests { * @param otherParentGeneration the generation of the other parent of the event * @return the mock event */ - private GossipEvent generateMockEvent( + private static GossipEvent generateMockEvent( @NonNull final Hash selfHash, final long selfGeneration, @Nullable final Hash selfParentHash, @@ -125,12 +113,7 @@ void setup() { .when(intakeEventCounter) .eventExitedIntakePipeline(any()); - consumedEventCount = new AtomicInteger(0); - final Consumer eventConsumer = event -> { - consumedEventCount.incrementAndGet(); - }; - - inOrderLinker = new InOrderLinker(eventConsumer, intakeEventCounter); + inOrderLinker = new InOrderLinker(intakeEventCounter); genesisSelfParent = generateMockEvent(randomHash(random), 0, null, GENERATION_UNDEFINED, null, GENERATION_UNDEFINED); @@ -153,7 +136,7 @@ void standardOperation() { genesisSelfParent.getGeneration(), genesisOtherParent.getHashedData().getHash(), genesisOtherParent.getGeneration()); - inOrderLinker.linkEvent(child1); + assertNotEquals(null, inOrderLinker.linkEvent(child1)); inOrderLinker.setMinimumGenerationNonAncient(2); @@ -167,14 +150,13 @@ void standardOperation() { child1Generation, genesisOtherParent.getHashedData().getHash(), genesisOtherParent.getGeneration()); - inOrderLinker.linkEvent(child2); - assertEquals(genesisEventCount + 2, consumedEventCount.get()); + assertNotEquals(null, inOrderLinker.linkEvent(child2)); assertEquals(0, exitedIntakePipelineCount.get()); } @Test - @DisplayName("Events with ancient parents should still be passed to the consumer") + @DisplayName("Events with ancient parents should still be linkable") void parentBecomesAncient() { // this will cause the genesis parents to be purged, since they are now ancient inOrderLinker.setMinimumGenerationNonAncient(3); @@ -186,9 +168,8 @@ void parentBecomesAncient() { genesisSelfParent.getGeneration(), genesisOtherParent.getHashedData().getHash(), genesisOtherParent.getGeneration()); - inOrderLinker.linkEvent(child); - assertEquals(genesisEventCount + 1, consumedEventCount.get()); + assertNotEquals(null, inOrderLinker.linkEvent(child)); assertEquals(0, exitedIntakePipelineCount.get()); } @@ -202,9 +183,8 @@ void missingSelfParent() { genesisSelfParent.getGeneration(), genesisOtherParent.getHashedData().getHash(), genesisOtherParent.getGeneration()); - inOrderLinker.linkEvent(child); - assertEquals(genesisEventCount, consumedEventCount.get()); + assertNull(inOrderLinker.linkEvent(child)); assertEquals(1, exitedIntakePipelineCount.get()); } @@ -218,9 +198,8 @@ void missingOtherParent() { genesisSelfParent.getGeneration(), null, genesisOtherParent.getGeneration()); - inOrderLinker.linkEvent(child); - assertEquals(genesisEventCount, consumedEventCount.get()); + assertNull(inOrderLinker.linkEvent(child)); assertEquals(1, exitedIntakePipelineCount.get()); } @@ -236,7 +215,8 @@ void ancientEvent() { genesisSelfParent.getGeneration(), genesisOtherParent.getHashedData().getHash(), genesisOtherParent.getGeneration()); - inOrderLinker.linkEvent(child1); + + assertNull(inOrderLinker.linkEvent(child1)); // barely ancient final GossipEvent child2 = generateMockEvent( @@ -246,9 +226,8 @@ void ancientEvent() { genesisSelfParent.getGeneration(), genesisOtherParent.getHashedData().getHash(), genesisOtherParent.getGeneration()); - inOrderLinker.linkEvent(child2); - assertEquals(genesisEventCount, consumedEventCount.get()); + assertNull(inOrderLinker.linkEvent(child2)); assertEquals(2, exitedIntakePipelineCount.get()); } } diff --git a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/validation/EventSignatureValidatorTests.java b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/validation/EventSignatureValidatorTests.java index 2d045f633a54..87db149a79ff 100644 --- a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/validation/EventSignatureValidatorTests.java +++ b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/validation/EventSignatureValidatorTests.java @@ -19,6 +19,8 @@ import static com.swirlds.common.test.fixtures.RandomUtils.getRandomPrintSeed; import static com.swirlds.common.test.fixtures.RandomUtils.randomHash; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -43,9 +45,7 @@ import java.security.PublicKey; import java.util.List; import java.util.Random; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -54,12 +54,9 @@ class EventSignatureValidatorTests { private Random random; private PlatformContext platformContext; private FakeTime time; - private AtomicInteger consumedEventCount; - private Consumer eventConsumer; private AtomicLong exitedIntakePipelineCount; private IntakeEventCounter intakeEventCounter; - private AddressBook previousAddressBook; private AddressBook currentAddressBook; /** @@ -93,7 +90,7 @@ class EventSignatureValidatorTests { * @param nodeId the node id to use for the address * @return a mock address */ - private Address generateMockAddress(final @NonNull NodeId nodeId) { + private static Address generateMockAddress(final @NonNull NodeId nodeId) { final PublicKey publicKey = mock(PublicKey.class); final SerializablePublicKey serializablePublicKey = mock(SerializablePublicKey.class); when(serializablePublicKey.getPublicKey()).thenReturn(publicKey); @@ -109,7 +106,7 @@ private Address generateMockAddress(final @NonNull NodeId nodeId) { * @param creatorId the creator id to use for the event * @return a mock event */ - final GossipEvent generateMockEvent( + private static GossipEvent generateMockEvent( @NonNull final SoftwareVersion version, @NonNull final Hash hash, @NonNull final NodeId creatorId) { final BaseEventHashedData hashedData = mock(BaseEventHashedData.class); when(hashedData.getSoftwareVersion()).thenReturn(version); @@ -123,42 +120,12 @@ final GossipEvent generateMockEvent( return event; } - /** - * Assert either a passing or failing validation for a given event - * - * @param event the event to validate - * @param expectPass whether the event is expected to pass validation - */ - private void assertValidationResult( - final boolean useTrueVerifier, @NonNull final GossipEvent event, final boolean expectPass) { - int expectedConsumedEventCount = consumedEventCount.get(); - long expectedExitedIntakePipelineCount = exitedIntakePipelineCount.get(); - - if (expectPass) { - expectedConsumedEventCount++; - } else { - expectedExitedIntakePipelineCount++; - } - - final EventSignatureValidator validator = - useTrueVerifier ? validatorWithTrueVerifier : validatorWithFalseVerifier; - validator.validateEventSignature(event); - - assertEquals(expectedConsumedEventCount, consumedEventCount.get()); - assertEquals(expectedExitedIntakePipelineCount, exitedIntakePipelineCount.get()); - } - @BeforeEach void setup() { random = getRandomPrintSeed(); platformContext = TestPlatformContextBuilder.create().build(); time = new FakeTime(); - consumedEventCount = new AtomicInteger(0); - eventConsumer = event -> { - consumedEventCount.incrementAndGet(); - }; - exitedIntakePipelineCount = new AtomicLong(0); intakeEventCounter = mock(IntakeEventCounter.class); doAnswer(invocation -> { @@ -172,7 +139,7 @@ void setup() { previousNodeAddress = generateMockAddress(new NodeId(66)); currentNodeAddress = generateMockAddress(new NodeId(77)); - previousAddressBook = new AddressBook(List.of(previousNodeAddress)); + final AddressBook previousAddressBook = new AddressBook(List.of(previousNodeAddress)); currentAddressBook = new AddressBook(List.of(currentNodeAddress)); defaultVersion = new BasicSoftwareVersion(2); @@ -184,7 +151,6 @@ void setup() { defaultVersion, previousAddressBook, currentAddressBook, - eventConsumer, intakeEventCounter); validatorWithFalseVerifier = new EventSignatureValidator( @@ -194,7 +160,6 @@ void setup() { defaultVersion, previousAddressBook, currentAddressBook, - eventConsumer, intakeEventCounter); } @@ -204,27 +169,20 @@ void irreconcilableVersions() { final GossipEvent event = generateMockEvent(new BasicSoftwareVersion(3), randomHash(random), currentNodeAddress.getNodeId()); - assertValidationResult(true, event, false); + assertNull(validatorWithTrueVerifier.validateSignature(event)); + assertEquals(1, exitedIntakePipelineCount.get()); } @Test @DisplayName("Lower version event with missing previous address book") void versionMismatchWithNullPreviousAddressBook() { final EventSignatureValidator signatureValidator = new EventSignatureValidator( - platformContext, - time, - trueVerifier, - defaultVersion, - null, - currentAddressBook, - eventConsumer, - intakeEventCounter); + platformContext, time, trueVerifier, defaultVersion, null, currentAddressBook, intakeEventCounter); final GossipEvent event = generateMockEvent(new BasicSoftwareVersion(1), randomHash(random), previousNodeAddress.getNodeId()); - signatureValidator.validateEventSignature(event); - assertEquals(0, consumedEventCount.get()); + assertNull(signatureValidator.validateSignature(event)); assertEquals(1, exitedIntakePipelineCount.get()); } @@ -235,7 +193,8 @@ void applicableAddressBookMissingNode() { final GossipEvent event = generateMockEvent(defaultVersion, randomHash(random), previousNodeAddress.getNodeId()); - assertValidationResult(true, event, false); + assertNull(validatorWithTrueVerifier.validateSignature(event)); + assertEquals(1, exitedIntakePipelineCount.get()); } @Test @@ -251,7 +210,8 @@ void missingPublicKey() { final GossipEvent event = generateMockEvent(defaultVersion, randomHash(random), nodeId); - assertValidationResult(true, event, false); + assertNull(validatorWithTrueVerifier.validateSignature(event)); + assertEquals(1, exitedIntakePipelineCount.get()); } @Test @@ -260,12 +220,16 @@ void validSignature() { // both the event and the app have the same version, so the currentAddressBook will be selected final GossipEvent event1 = generateMockEvent(defaultVersion, randomHash(random), currentNodeAddress.getNodeId()); - assertValidationResult(true, event1, true); + + assertNotEquals(null, validatorWithTrueVerifier.validateSignature(event1)); + assertEquals(0, exitedIntakePipelineCount.get()); // event2 is from a previous version, so the previous address book will be selected final GossipEvent event2 = generateMockEvent(new BasicSoftwareVersion(1), randomHash(random), previousNodeAddress.getNodeId()); - assertValidationResult(true, event2, true); + + assertNotEquals(null, validatorWithTrueVerifier.validateSignature(event2)); + assertEquals(0, exitedIntakePipelineCount.get()); } @Test @@ -273,8 +237,11 @@ void validSignature() { void verificationFails() { final GossipEvent event = generateMockEvent(defaultVersion, randomHash(random), currentNodeAddress.getNodeId()); - assertValidationResult(true, event, true); - assertValidationResult(false, event, false); + assertNotEquals(null, validatorWithTrueVerifier.validateSignature(event)); + assertEquals(0, exitedIntakePipelineCount.get()); + + assertNull(validatorWithFalseVerifier.validateSignature(event)); + assertEquals(1, exitedIntakePipelineCount.get()); } @Test @@ -282,8 +249,12 @@ void verificationFails() { void ancientEvent() { final GossipEvent event = generateMockEvent(defaultVersion, randomHash(random), currentNodeAddress.getNodeId()); - assertValidationResult(true, event, true); + assertNotEquals(null, validatorWithTrueVerifier.validateSignature(event)); + assertEquals(0, exitedIntakePipelineCount.get()); + validatorWithTrueVerifier.setMinimumGenerationNonAncient(100L); - assertValidationResult(true, event, false); + + assertNull(validatorWithTrueVerifier.validateSignature(event)); + assertEquals(1, exitedIntakePipelineCount.get()); } } diff --git a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/validation/InternalEventValidatorTests.java b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/validation/InternalEventValidatorTests.java index 4715e3c44cd2..4cf5a0ab630d 100644 --- a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/validation/InternalEventValidatorTests.java +++ b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/event/validation/InternalEventValidatorTests.java @@ -19,6 +19,8 @@ import static com.swirlds.common.test.fixtures.RandomUtils.getRandomPrintSeed; import static com.swirlds.common.test.fixtures.RandomUtils.randomHash; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -36,18 +38,15 @@ import com.swirlds.test.framework.context.TestPlatformContextBuilder; import edu.umd.cs.findbugs.annotations.Nullable; import java.util.Random; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; /** - * Tests for {@link LinkedEventValidator} + * Tests for {@link InternalEventValidator} */ class InternalEventValidatorTests { - private AtomicInteger consumedEventCount; private AtomicLong exitedIntakePipelineCount; private Random random; private InternalEventValidator multinodeValidator; @@ -69,18 +68,13 @@ void setup() { final PlatformContext platformContext = TestPlatformContextBuilder.create().build(); - consumedEventCount = new AtomicInteger(0); - final Consumer eventConsumer = event -> consumedEventCount.incrementAndGet(); - final Time time = new FakeTime(); - multinodeValidator = - new InternalEventValidator(platformContext, time, false, eventConsumer, intakeEventCounter); - singleNodeValidator = - new InternalEventValidator(platformContext, time, true, eventConsumer, intakeEventCounter); + multinodeValidator = new InternalEventValidator(platformContext, time, false, intakeEventCounter); + singleNodeValidator = new InternalEventValidator(platformContext, time, true, intakeEventCounter); } - private GossipEvent generateEvent( + private static GossipEvent generateEvent( @Nullable final Hash selfParentHash, @Nullable final Hash otherParentHash, final long eventGeneration, @@ -117,10 +111,9 @@ void nullHashedData() { final GossipEvent event = generateEvent(randomHash(random), randomHash(random), 7, 5, 6, 1111); when(event.getHashedData()).thenReturn(null); - multinodeValidator.handleEvent(event); - singleNodeValidator.handleEvent(event); + assertNull(multinodeValidator.validateEvent(event)); + assertNull(singleNodeValidator.validateEvent(event)); - assertEquals(0, consumedEventCount.get()); assertEquals(2, exitedIntakePipelineCount.get()); } @@ -130,10 +123,9 @@ void nullUnhashedData() { final GossipEvent event = generateEvent(randomHash(random), randomHash(random), 7, 5, 6, 1111); when(event.getUnhashedData()).thenReturn(null); - multinodeValidator.handleEvent(event); - singleNodeValidator.handleEvent(event); + assertNull(multinodeValidator.validateEvent(event)); + assertNull(singleNodeValidator.validateEvent(event)); - assertEquals(0, consumedEventCount.get()); assertEquals(2, exitedIntakePipelineCount.get()); } @@ -143,10 +135,9 @@ void tooManyTransactionBytes() { // default max is 245_760 bytes final GossipEvent event = generateEvent(randomHash(random), randomHash(random), 7, 5, 6, 500_000); - multinodeValidator.handleEvent(event); - singleNodeValidator.handleEvent(event); + assertNull(multinodeValidator.validateEvent(event)); + assertNull(singleNodeValidator.validateEvent(event)); - assertEquals(0, consumedEventCount.get()); assertEquals(2, exitedIntakePipelineCount.get()); } @@ -164,17 +155,16 @@ void inconsistentParents() { final GossipEvent invalidOtherParentGeneration = generateEvent(randomHash(random), randomHash(random), 6, 5, -1, 1111); - multinodeValidator.handleEvent(nullSelfParentHash); - multinodeValidator.handleEvent(invalidSelfParentGeneration); - multinodeValidator.handleEvent(nullOtherParentHash); - multinodeValidator.handleEvent(invalidOtherParentGeneration); + assertNull(multinodeValidator.validateEvent(nullSelfParentHash)); + assertNull(multinodeValidator.validateEvent(invalidSelfParentGeneration)); + assertNull(multinodeValidator.validateEvent(nullOtherParentHash)); + assertNull(multinodeValidator.validateEvent(invalidOtherParentGeneration)); - singleNodeValidator.handleEvent(nullSelfParentHash); - singleNodeValidator.handleEvent(invalidSelfParentGeneration); - singleNodeValidator.handleEvent(nullOtherParentHash); - singleNodeValidator.handleEvent(invalidOtherParentGeneration); + assertNull(singleNodeValidator.validateEvent(nullSelfParentHash)); + assertNull(singleNodeValidator.validateEvent(invalidSelfParentGeneration)); + assertNull(singleNodeValidator.validateEvent(nullOtherParentHash)); + assertNull(singleNodeValidator.validateEvent(invalidOtherParentGeneration)); - assertEquals(0, consumedEventCount.get()); assertEquals(8, exitedIntakePipelineCount.get()); } @@ -184,10 +174,9 @@ void identicalParents() { final Hash sharedHash = randomHash(random); final GossipEvent event = generateEvent(sharedHash, sharedHash, 7, 5, 6, 1111); - multinodeValidator.handleEvent(event); - singleNodeValidator.handleEvent(event); + assertNull(multinodeValidator.validateEvent(event)); + assertNotEquals(null, singleNodeValidator.validateEvent(event)); - assertEquals(1, consumedEventCount.get()); assertEquals(1, exitedIntakePipelineCount.get()); } @@ -197,12 +186,11 @@ void invalidGeneration() { final GossipEvent highGeneration = generateEvent(randomHash(random), randomHash(random), 8, 5, 6, 1111); final GossipEvent lowGeneration = generateEvent(randomHash(random), randomHash(random), 4, 5, 6, 1111); - multinodeValidator.handleEvent(highGeneration); - multinodeValidator.handleEvent(lowGeneration); - singleNodeValidator.handleEvent(highGeneration); - singleNodeValidator.handleEvent(lowGeneration); + assertNull(multinodeValidator.validateEvent(highGeneration)); + assertNull(multinodeValidator.validateEvent(lowGeneration)); + assertNull(singleNodeValidator.validateEvent(highGeneration)); + assertNull(singleNodeValidator.validateEvent(lowGeneration)); - assertEquals(0, consumedEventCount.get()); assertEquals(4, exitedIntakePipelineCount.get()); } @@ -213,15 +201,14 @@ void successfulValidation() { final GossipEvent missingSelfParent = generateEvent(null, randomHash(random), 7, -1, 6, 1111); final GossipEvent missingOtherParent = generateEvent(randomHash(random), null, 6, 5, -1, 1111); - multinodeValidator.handleEvent(normalEvent); - multinodeValidator.handleEvent(missingSelfParent); - multinodeValidator.handleEvent(missingOtherParent); + assertNotEquals(null, multinodeValidator.validateEvent(normalEvent)); + assertNotEquals(null, multinodeValidator.validateEvent(missingSelfParent)); + assertNotEquals(null, multinodeValidator.validateEvent(missingOtherParent)); - singleNodeValidator.handleEvent(normalEvent); - singleNodeValidator.handleEvent(missingSelfParent); - singleNodeValidator.handleEvent(missingOtherParent); + assertNotEquals(null, singleNodeValidator.validateEvent(normalEvent)); + assertNotEquals(null, singleNodeValidator.validateEvent(missingSelfParent)); + assertNotEquals(null, singleNodeValidator.validateEvent(missingOtherParent)); - assertEquals(6, consumedEventCount.get()); assertEquals(0, exitedIntakePipelineCount.get()); } }