Fix ISS detection. (#10182)

Signed-off-by: Cody Littley <cody@swirldslabs.com>

platform-sdk/swirlds-common/src/main/java/com/swirlds/common/system/BasicSoftwareVersion.java

@@ -108,29 +108,9 @@ public int compareTo(final SoftwareVersion that) {
         }
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public boolean equals(final Object o) {
-        if (this == o) {
-            return true;
-        }
-
-        if (!(o instanceof BasicSoftwareVersion that)) {
-            return false;
-        }
-
-        return softwareVersion == that.softwareVersion;
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public int hashCode() {
-        return Long.hashCode(softwareVersion);
-    }
+    // Intentionally do not implement equals() or hashCode(). Although it is legal to do so, it is not required,
+    // and all platform operations should be functional without it. Since this class is used for platform testing,
+    // we should make sure this crutch is not available.
 
     /**
      * {@inheritDoc}

platform-sdk/swirlds-common/src/main/java/com/swirlds/common/system/events/BaseEventHashedData.java

@@ -276,7 +276,7 @@ public boolean equals(final Object o) {
                 && Objects.equals(otherParentHash, that.otherParentHash)
                 && Objects.equals(timeCreated, that.timeCreated)
                 && Arrays.equals(transactions, that.transactions)
-                && Objects.equals(softwareVersion, that.softwareVersion);
+                && (softwareVersion.compareTo(that.softwareVersion) == 0);
     }
 
     @Override

platform-sdk/swirlds-common/src/main/java/com/swirlds/common/utility/PlatformVersion.java

@@ -296,23 +296,6 @@ public String commit() {
         return commit;
     }
 
-    @Override
-    public boolean equals(final Object obj) {
-        if (obj == this) {
-            return true;
-        }
-        if (obj == null || obj.getClass() != this.getClass()) {
-            return false;
-        }
-        final var that = (PlatformVersion) obj;
-        return Objects.equals(this.versionNumber, that.versionNumber) && Objects.equals(this.commit, that.commit);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(versionNumber, commit);
-    }
-
     @Override
     public String toString() {
         return new ToStringBuilder(this)

platform-sdk/swirlds-common/src/test/java/com/swirlds/common/system/BasicSoftwareVersionTest.java

@@ -16,7 +16,6 @@
 
 package com.swirlds.common.system;
 
-import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -63,56 +62,4 @@ void testToString() {
         assertEquals("1", VERSION_ONE.toString(), "VERSION_ONE not reporting its version as 1.");
         assertEquals("2", VERSION_TWO.toString(), "VERSION_TWO not reporting its version as 2.");
     }
-
-    @Test
-    @Tag(TestTypeTags.FUNCTIONAL)
-    @DisplayName("A BasicSoftwareVersion is equal to itself")
-    void selfEquals() {
-        final var ver = new BasicSoftwareVersion(1);
-        assertThat(ver).isEqualTo(ver);
-    }
-
-    @Test
-    @Tag(TestTypeTags.FUNCTIONAL)
-    @DisplayName("Two BasicSoftwareVersion are equal if their versions are equal")
-    void equals() {
-        final var ver1 = new BasicSoftwareVersion(1);
-        final var ver2 = new BasicSoftwareVersion(1);
-        assertThat(ver1).isEqualTo(ver2);
-    }
-
-    @Test
-    @Tag(TestTypeTags.FUNCTIONAL)
-    @DisplayName("A BasicSoftwareVersion is not equal to null")
-    void notEqualToNull() {
-        final var ver = new BasicSoftwareVersion(1);
-        assertThat(ver).isNotEqualTo(null);
-    }
-
-    @Test
-    @Tag(TestTypeTags.FUNCTIONAL)
-    @DisplayName("Two BasicSoftwareVersion of different versions are not equal")
-    void unequal() {
-        final var ver1 = new BasicSoftwareVersion(1);
-        final var ver2 = new BasicSoftwareVersion(2);
-        assertThat(ver1).isNotEqualTo(ver2);
-    }
-
-    @Test
-    @Tag(TestTypeTags.FUNCTIONAL)
-    @DisplayName("Two equal BasicSoftwareVersion produce the same hashcode")
-    void hashCodeConsistentWithEquals() {
-        final var ver1 = new BasicSoftwareVersion(1);
-        final var ver2 = new BasicSoftwareVersion(1);
-        assertThat(ver1).hasSameHashCodeAs(ver2);
-    }
-
-    @Test
-    @Tag(TestTypeTags.FUNCTIONAL)
-    @DisplayName("Two different BasicSoftwareVersion probably produce different hash codes")
-    void differentHashCodesConsistentWithNotEquals() {
-        final var ver1 = new BasicSoftwareVersion(1);
-        final var ver2 = new BasicSoftwareVersion(2);
-        assertThat(ver1.hashCode()).isNotEqualTo(ver2.hashCode());
-    }
 }

platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/state/iss/ConsensusHashManager.java

@@ -129,10 +129,12 @@ public ConsensusHashManager(
             final DispatchBuilder dispatchBuilder,
             final AddressBook addressBook,
             final Hash currentEpochHash,
-            final SoftwareVersion currentSoftwareVersion,
+            @NonNull final SoftwareVersion currentSoftwareVersion,
             final boolean ignorePreconsensusSignatures,
             final long ignoredRound) {
 
+        Objects.requireNonNull(currentSoftwareVersion);
+
         final ConsensusConfig consensusConfig =
                 platformContext.getConfiguration().getConfigData(ConsensusConfig.class);
         final StateConfig stateConfig = platformContext.getConfiguration().getConfigData(StateConfig.class);
@@ -255,12 +257,17 @@ public void handlePostconsensusSignatureTransaction(
         Objects.requireNonNull(signerId);
         Objects.requireNonNull(signatureTransaction);
 
+        if (eventVersion == null) {
+            // Illegal event version, ignore.
+            return;
+        }
+
         if (ignorePreconsensusSignatures && replayingPreconsensusStream) {
             // We are still replaying preconsensus events and we are configured to ignore signatures during replay
             return;
         }
 
-        if (!Objects.equals(currentSoftwareVersion, eventVersion)) {
+        if (currentSoftwareVersion.compareTo(eventVersion) != 0) {
             // this is a signature from a different software version, ignore it
             return;
         }

platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/state/signed/StartupStateUtils.java

@@ -522,7 +522,7 @@ private static ReservedSignedState loadStateFile(
 
         if (oldHash.equals(newHash)) {
             logger.info(STARTUP.getMarker(), "Loaded state's hash is the same as when it was saved.");
-        } else if (loadedVersion.equals(currentSoftwareVersion)) {
+        } else if (loadedVersion.compareTo(currentSoftwareVersion) == 0) {
             logger.error(
                     EXCEPTION.getMarker(),
                     "The saved state file {} was created with the current version of the software, "

platform-sdk/swirlds-unit-tests/common/swirlds-common-test/src/test/java/com/swirlds/common/test/utility/PlatformVersionTest.java

@@ -212,7 +212,6 @@ void testSerialization(
         final PlatformVersion original =
                 new PlatformVersion(new SemanticVersion(major, minor, patch, prerelease, build), commitId);
         final PlatformVersion copy = SerializationUtils.serializeDeserialize(original);
-        assertEquals(original, copy, String.format(VALUES_NOT_EQUAL, "PlatformVersion"));
         assertEquals(0, original.compareTo(copy), String.format(VALUES_NOT_EQUAL, "PlatformVersion.compareTo(copy)"));
     }
 

platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/state/ConsensusHashManagerTests.java

@@ -38,8 +38,8 @@
 import com.swirlds.common.context.PlatformContext;
 import com.swirlds.common.crypto.Hash;
 import com.swirlds.common.crypto.Signature;
+import com.swirlds.common.system.BasicSoftwareVersion;
 import com.swirlds.common.system.NodeId;
-import com.swirlds.common.system.SoftwareVersion;
 import com.swirlds.common.system.address.Address;
 import com.swirlds.common.system.address.AddressBook;
 import com.swirlds.common.system.transaction.internal.StateSignatureTransaction;
@@ -66,8 +66,6 @@ class ConsensusHashManagerTests {
     /** the default epoch hash to use */
     private static final Hash DEFAULT_EPOCH_HASH = null;
 
-    private static final SoftwareVersion DEFAULT_SOFTWARE_VERSION = null;
-
     @Test
     @DisplayName("Valid Signatures After Hash Test")
     void validSignaturesAfterHashTest() {
@@ -89,7 +87,7 @@ void validSignaturesAfterHashTest() {
                 dispatchBuilder,
                 addressBook,
                 DEFAULT_EPOCH_HASH,
-                DEFAULT_SOFTWARE_VERSION,
+                new BasicSoftwareVersion(1),
                 false,
                 DO_NOT_IGNORE_ROUNDS);
 
@@ -114,7 +112,7 @@ void validSignaturesAfterHashTest() {
                 manager.handlePostconsensusSignatureTransaction(
                         address.getNodeId(),
                         new StateSignatureTransaction(round, mock(Signature.class), roundHash),
-                        null);
+                        new BasicSoftwareVersion(1));
             }
         }
 
@@ -203,7 +201,7 @@ void mixedOrderTest() {
                 dispatchBuilder,
                 addressBook,
                 DEFAULT_EPOCH_HASH,
-                DEFAULT_SOFTWARE_VERSION,
+                new BasicSoftwareVersion(1),
                 false,
                 DO_NOT_IGNORE_ROUNDS);
 
@@ -290,7 +288,7 @@ void mixedOrderTest() {
                     nodeId,
                     new StateSignatureTransaction(
                             nodeHashInfo.round(), mock(Signature.class), nodeHashInfo.nodeStateHash()),
-                    null);
+                    new BasicSoftwareVersion(1));
         }
 
         // Shifting after completion should have no side effects
@@ -360,7 +358,7 @@ void earlyAddTest() {
                 dispatchBuilder,
                 addressBook,
                 DEFAULT_EPOCH_HASH,
-                DEFAULT_SOFTWARE_VERSION,
+                new BasicSoftwareVersion(1),
                 false,
                 DO_NOT_IGNORE_ROUNDS);
 
@@ -393,7 +391,7 @@ void earlyAddTest() {
             manager.handlePostconsensusSignatureTransaction(
                     info.nodeId(),
                     new StateSignatureTransaction(targetRound, mock(Signature.class), info.nodeStateHash()),
-                    null);
+                    new BasicSoftwareVersion(1));
         }
 
         assertEquals(0, issCount.get(), "all data should have been ignored");
@@ -410,7 +408,7 @@ void earlyAddTest() {
             manager.handlePostconsensusSignatureTransaction(
                     info.nodeId(),
                     new StateSignatureTransaction(targetRound, mock(Signature.class), info.nodeStateHash()),
-                    null);
+                    new BasicSoftwareVersion(1));
         }
 
         assertEquals(1, issCount.get(), "data should not have been ignored");
@@ -442,7 +440,7 @@ void lateAddTest() {
                 dispatchBuilder,
                 addressBook,
                 DEFAULT_EPOCH_HASH,
-                DEFAULT_SOFTWARE_VERSION,
+                new BasicSoftwareVersion(1),
                 false,
                 DO_NOT_IGNORE_ROUNDS);
 
@@ -475,7 +473,7 @@ void lateAddTest() {
             manager.handlePostconsensusSignatureTransaction(
                     info.nodeId(),
                     new StateSignatureTransaction(targetRound, mock(Signature.class), info.nodeStateHash()),
-                    null);
+                    new BasicSoftwareVersion(1));
         }
 
         assertEquals(0, issCount.get(), "all data should have been ignored");
@@ -507,7 +505,7 @@ void shiftBeforeCompleteTest() {
                 dispatchBuilder,
                 addressBook,
                 DEFAULT_EPOCH_HASH,
-                DEFAULT_SOFTWARE_VERSION,
+                new BasicSoftwareVersion(1),
                 false,
                 DO_NOT_IGNORE_ROUNDS);
 
@@ -547,7 +545,7 @@ void shiftBeforeCompleteTest() {
             manager.handlePostconsensusSignatureTransaction(
                     info.nodeId(),
                     new StateSignatureTransaction(targetRound, mock(Signature.class), info.nodeStateHash()),
-                    null);
+                    new BasicSoftwareVersion(1));
         }
 
         // Shift the window even though we have not added enough data for a decision
@@ -612,7 +610,7 @@ void catastrophicShiftBeforeCompleteTest() {
                 dispatchBuilder,
                 addressBook,
                 DEFAULT_EPOCH_HASH,
-                DEFAULT_SOFTWARE_VERSION,
+                new BasicSoftwareVersion(1),
                 false,
                 DO_NOT_IGNORE_ROUNDS);
 
@@ -647,7 +645,7 @@ void catastrophicShiftBeforeCompleteTest() {
             manager.handlePostconsensusSignatureTransaction(
                     info.nodeId(),
                     new StateSignatureTransaction(targetRound, mock(Signature.class), info.nodeStateHash()),
-                    null);
+                    new BasicSoftwareVersion(1));
 
             // Stop once we have added >2/3. We should not have decided yet, but will
             // have gathered enough to declare a catastrophic ISS
@@ -690,7 +688,7 @@ void bigShiftTest() {
                 dispatchBuilder,
                 addressBook,
                 DEFAULT_EPOCH_HASH,
-                DEFAULT_SOFTWARE_VERSION,
+                new BasicSoftwareVersion(1),
                 false,
                 DO_NOT_IGNORE_ROUNDS);
 
@@ -725,7 +723,7 @@ void bigShiftTest() {
             manager.handlePostconsensusSignatureTransaction(
                     info.nodeId(),
                     new StateSignatureTransaction(targetRound, mock(Signature.class), info.nodeStateHash()),
-                    null);
+                    new BasicSoftwareVersion(1));
 
             // Stop once we have added >2/3. We should not have decided yet, but will
             // have gathered enough to declare a catastrophic ISS
@@ -762,7 +760,7 @@ void ignoredRoundTest() {
                 dispatchBuilder,
                 addressBook,
                 DEFAULT_EPOCH_HASH,
-                DEFAULT_SOFTWARE_VERSION,
+                new BasicSoftwareVersion(1),
                 false,
                 1);
 
@@ -789,12 +787,12 @@ void ignoredRoundTest() {
                     manager.handlePostconsensusSignatureTransaction(
                             address.getNodeId(),
                             new StateSignatureTransaction(round, mock(Signature.class), randomHash(random)),
-                            null);
+                            new BasicSoftwareVersion(1));
                 } else {
                     manager.handlePostconsensusSignatureTransaction(
                             address.getNodeId(),
                             new StateSignatureTransaction(round, mock(Signature.class), roundHash),
-                            null);
+                            new BasicSoftwareVersion(1));
                 }
             }
         }