Skip to content

fix: Fix ISS reporting mechanism #20370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 29, 2025
Merged

fix: Fix ISS reporting mechanism #20370

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b1cb8a6
fix: Fix ISS reporting mechanism
thenswan Jul 25, 2025
8664549
fix IssDetectorTests
thenswan Jul 25, 2025
9202869
remove stateJson from the IssPayload
thenswan Jul 29, 2025
9867a18
remove stateJson from the IssPayload
thenswan Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...form-sdk/swirlds-logging/src/main/java/com/swirlds/logging/legacy/payload/IssPayload.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void setCatastrophic(final boolean catastrophic) {
*/
@JsonRawValue
public String getStateJson() {
return stateJson;
return (stateJson == null || stateJson.isEmpty()) ? null : stateJson;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions ...ds-platform-core/src/main/java/com/swirlds/platform/builder/PlatformComponentBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,8 +783,7 @@ public IssDetector buildIssDetector() {
blocks.rosterHistory().getCurrentRoster(),
ignorePreconsensusSignatures,
roundToIgnore,
latestFreezeRound,
blocks.swirldStateManager());
latestFreezeRound);
}
return issDetector;
}
Expand Down
29 changes: 13 additions & 16 deletions ...wirlds-platform-core/src/main/java/com/swirlds/platform/state/iss/DefaultIssDetector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.swirlds.platform.config.StateConfig;
import com.swirlds.platform.consensus.ConsensusConfig;
import com.swirlds.platform.metrics.IssMetrics;
import com.swirlds.platform.state.SwirldStateManager;
import com.swirlds.platform.state.iss.internal.ConsensusHashFinder;
import com.swirlds.platform.state.iss.internal.HashValidityStatus;
import com.swirlds.platform.state.iss.internal.RoundHashValidator;
Expand Down Expand Up @@ -107,16 +106,11 @@ public class DefaultIssDetector implements IssDetector {
*/
private final long latestFreezeRound;

/**
* The state manager that is used to get the current consensus state.
*/
private final SwirldStateManager swirldStateManager;

/**
* Create an object that tracks reported hashes and detects ISS events.
*
* @param platformContext the platform context
* @param roster the current roster
* @param roster the current roster
* @param ignorePreconsensusSignatures If true, ignore signatures from the preconsensus event stream, otherwise
* validate them like normal.
* @param ignoredRound a round that should not be validated. Set to {@link #DO_NOT_IGNORE_ROUNDS} if
Expand All @@ -127,9 +121,7 @@ public DefaultIssDetector(
@NonNull final Roster roster,
final boolean ignorePreconsensusSignatures,
final long ignoredRound,
final long latestFreezeRound,
@NonNull final SwirldStateManager swirldStateManager) {
this.swirldStateManager = swirldStateManager;
final long latestFreezeRound) {
Objects.requireNonNull(platformContext);
markerFileWriter = new MarkerFileWriter(platformContext);

Expand Down Expand Up @@ -193,10 +185,11 @@ private IssNotification maybeCreateIssNotification(final long roundNumber, @NonN
* force a decision on the hash, and handle any ISS events that result.
*
* @param roundNumber the round that was just completed
* @param stateJson the JSON string containing information about the current state
* @return a list of ISS notifications, which may be empty, but will not contain null
*/
@NonNull
private List<IssNotification> shiftRoundDataWindow(final long roundNumber) {
private List<IssNotification> shiftRoundDataWindow(final long roundNumber, @NonNull final String stateJson) {
if (roundNumber <= previousRound) {
throw new IllegalArgumentException(
"previous round was " + previousRound + ", can't decrease round to " + roundNumber);
Expand All @@ -216,7 +209,8 @@ private List<IssNotification> shiftRoundDataWindow(final long roundNumber) {

previousRound = roundNumber;
roundData.put(
roundNumber, new RoundHashValidator(roundNumber, RosterUtils.computeTotalWeight(roster), issMetrics));
roundNumber,
new RoundHashValidator(roundNumber, RosterUtils.computeTotalWeight(roster), stateJson, issMetrics));

return removedRounds.stream()
.map(this::handleRemovedRound)
Expand Down Expand Up @@ -267,8 +261,10 @@ public List<IssNotification> handleState(@NonNull final ReservedSignedState rese
try (reservedSignedState) {
final SignedState state = reservedSignedState.get();
final long roundNumber = state.getRound();
final String stateJson = state.getState().getInfoJson();

final List<IssNotification> issNotifications = new ArrayList<>(shiftRoundDataWindow(roundNumber));
final List<IssNotification> issNotifications =
new ArrayList<>(shiftRoundDataWindow(roundNumber, stateJson));

// Apply any signatures we collected previously that are for this round
issNotifications.addAll(applySignaturesAndShiftWindow(roundNumber));
Expand Down Expand Up @@ -446,10 +442,11 @@ private IssNotification checkSelfStateHash(final long round, @NonNull final Hash
public List<IssNotification> overridingState(@NonNull final ReservedSignedState state) {
try (state) {
final long roundNumber = state.get().getRound();
final String stateJson = state.get().getState().getInfoJson();
// this is not practically possible for an ISS to occur for hashes before the state provided
// in this method. Even if it were to happen, on a reconnect, we are receiving a new state that is fully
// signed, so any ISSs in the past should be ignored. so we will ignore any ISSs from removed rounds
shiftRoundDataWindow(roundNumber);
shiftRoundDataWindow(roundNumber, stateJson);

// Apply any signatures we collected previously that are for this round. It is not practically
// possible for there to be any signatures stored up for this state, but there is no harm in
Expand Down Expand Up @@ -525,6 +522,7 @@ private void handleSelfIss(@NonNull final RoundHashValidator roundHashValidator)
final long round = roundHashValidator.getRound();
final Hash selfHash = roundHashValidator.getSelfStateHash();
final Hash consensusHash = roundHashValidator.getConsensusHash();
final String stateJson = roundHashValidator.getStateJson();

final long skipCount = selfIssRateLimiter.getDeniedRequests();
if (selfIssRateLimiter.requestAndTrigger()) {
Expand All @@ -537,7 +535,6 @@ private void handleSelfIss(@NonNull final RoundHashValidator roundHashValidator)
roundHashValidator.getHashFinder().writePartitionData(sb);
writeSkippedLogCount(sb, skipCount);

final String stateJson = swirldStateManager.getConsensusState().getInfoJson();
logger.fatal(
EXCEPTION.getMarker(),
new IssPayload(
Expand Down Expand Up @@ -573,7 +570,7 @@ private void handleCatastrophic(@NonNull final RoundHashValidator roundHashValid
writeSkippedLogCount(sb, skipCount);

final String mnemonic = selfHash == null ? "null" : Mnemonics.generateMnemonic(selfHash);
final String stateJson = swirldStateManager.getConsensusState().getInfoJson();
final String stateJson = roundHashValidator.getStateJson();
logger.fatal(EXCEPTION.getMarker(), new IssPayload(sb.toString(), round, mnemonic, "", true, stateJson));
}
}
Expand Down
17 changes: 16 additions & 1 deletion ...atform-core/src/main/java/com/swirlds/platform/state/iss/internal/RoundHashValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ public class RoundHashValidator {
*/
private final long round;

/**
* The JSON string containing information about the current state.
*/
private final String stateJson;

/**
* An object capable of determining the consensus hash.
*/
Expand All @@ -58,10 +63,13 @@ public class RoundHashValidator {
*
* @param round the round number
* @param roundWeight the total weight for this round
* @param stateJson the JSON string containing information about the current state
* @param issMetrics iss related metrics
*/
public RoundHashValidator(final long round, final long roundWeight, @NonNull final IssMetrics issMetrics) {
public RoundHashValidator(
final long round, final long roundWeight, @NonNull String stateJson, @NonNull final IssMetrics issMetrics) {
this.round = round;
this.stateJson = Objects.requireNonNull(stateJson);
hashFinder = new ConsensusHashFinder(round, roundWeight, Objects.requireNonNull(issMetrics));
}

Expand All @@ -72,6 +80,13 @@ public long getRound() {
return round;
}

/**
* Get the state info of the state that is being validated.
*/
public String getStateJson() {
return stateJson;
}

/**
* Get the hash that this node computed for the round if it is known, or null if it is not known.
*/
Expand Down
46 changes: 15 additions & 31 deletions ...-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/state/IssDetectorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.hiero.consensus.model.transaction.ScopedSystemTransaction;
import org.hiero.consensus.roster.RosterUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

Expand All @@ -57,16 +56,6 @@ class IssDetectorTests extends PlatformTest {
private static final WeightGenerator WEIGHT_GENERATOR = new GaussianWeightGenerator(100, 50);
private static final long GENESIS_LAST_FREEZE_ROUND = 0L;

private SwirldStateManager swirldStateManager;

@BeforeEach
void setUp() {
swirldStateManager = mock(SwirldStateManager.class);
final MerkleNodeState consensusState = mock(MerkleNodeState.class);
when(swirldStateManager.getConsensusState()).thenReturn(consensusState);
when(consensusState.getInfoJson()).thenReturn("");
}

@Test
@DisplayName("State reservation is released")
void stateReservationIsReleased() {
Expand All @@ -83,12 +72,7 @@ void stateReservationIsReleased() {

final PlatformContext platformContext = createDefaultPlatformContext();
final IssDetector issDetector = new DefaultIssDetector(
platformContext,
mock(Roster.class),
false,
DO_NOT_IGNORE_ROUNDS,
GENESIS_LAST_FREEZE_ROUND,
swirldStateManager);
platformContext, mock(Roster.class), false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND);

issDetector.handleState(stateWrapperForIssDetector);
assertTrue(stateWrapperForIssDetector.isClosed(), "State passed to the ISS Detector should be closed");
Expand All @@ -111,8 +95,8 @@ void noIss() {

final PlatformContext platformContext = createDefaultPlatformContext();

final IssDetector issDetector = new DefaultIssDetector(
platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND, swirldStateManager);
final IssDetector issDetector =
new DefaultIssDetector(platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND);
final IssDetectorTestHelper issDetectorTestHelper = new IssDetectorTestHelper(issDetector);

long currentRound = 0;
Expand Down Expand Up @@ -239,8 +223,8 @@ void mixedOrderTest() {
}
}

final IssDetector issDetector = new DefaultIssDetector(
platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND, swirldStateManager);
final IssDetector issDetector =
new DefaultIssDetector(platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND);
final IssDetectorTestHelper issDetectorTestHelper = new IssDetectorTestHelper(issDetector);

long currentRound = 0;
Expand Down Expand Up @@ -336,8 +320,8 @@ void decideForCatastrophicIss() {
.build();
final NodeId selfId = NodeId.of(roster.rosterEntries().getFirst().nodeId());

final IssDetector issDetector = new DefaultIssDetector(
platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND, swirldStateManager);
final IssDetector issDetector =
new DefaultIssDetector(platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND);
final IssDetectorTestHelper issDetectorTestHelper = new IssDetectorTestHelper(issDetector);

long currentRound = 0;
Expand Down Expand Up @@ -452,8 +436,8 @@ void catastrophicShiftBeforeCompleteTest() {
.build();
final NodeId selfId = NodeId.of(roster.rosterEntries().getFirst().nodeId());

final IssDetector issDetector = new DefaultIssDetector(
platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND, swirldStateManager);
final IssDetector issDetector =
new DefaultIssDetector(platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND);
final IssDetectorTestHelper issDetectorTestHelper = new IssDetectorTestHelper(issDetector);

long currentRound = 0;
Expand Down Expand Up @@ -538,8 +522,8 @@ void bigShiftTest() {
.build();
final NodeId selfId = NodeId.of(roster.rosterEntries().getFirst().nodeId());

final IssDetector issDetector = new DefaultIssDetector(
platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND, swirldStateManager);
final IssDetector issDetector =
new DefaultIssDetector(platformContext, roster, false, DO_NOT_IGNORE_ROUNDS, GENESIS_LAST_FREEZE_ROUND);
final IssDetectorTestHelper issDetectorTestHelper = new IssDetectorTestHelper(issDetector);

long currentRound = 0;
Expand Down Expand Up @@ -617,8 +601,8 @@ void ignoredRoundTest() {
.getConfigData(ConsensusConfig.class)
.roundsNonAncient();

final IssDetector issDetector = new DefaultIssDetector(
platformContext, roster, false, 1, GENESIS_LAST_FREEZE_ROUND, swirldStateManager);
final IssDetector issDetector =
new DefaultIssDetector(platformContext, roster, false, 1, GENESIS_LAST_FREEZE_ROUND);
final IssDetectorTestHelper issDetectorTestHelper = new IssDetectorTestHelper(issDetector);

long currentRound = 0;
Expand Down Expand Up @@ -675,8 +659,7 @@ void previousVersionEventsTest() {
.roundsNonAncient();

final long latestFreezeRound = 5L;
final IssDetector issDetector =
new DefaultIssDetector(platformContext, roster, false, 1, latestFreezeRound, swirldStateManager);
final IssDetector issDetector = new DefaultIssDetector(platformContext, roster, false, 1, latestFreezeRound);
final IssDetectorTestHelper issDetectorTestHelper = new IssDetectorTestHelper(issDetector);

long currentRound = 5;
Expand Down Expand Up @@ -774,6 +757,7 @@ private static ReservedSignedState mockState(final long round, final Hash hash)
when(ss.getState()).thenReturn(s);
when(ss.getRound()).thenReturn(round);
when(s.getHash()).thenReturn(hash);
when(s.getInfoJson()).thenReturn("");
return rs;
}

Expand Down
27 changes: 16 additions & 11 deletions ...irlds-platform-core/src/test/java/com/swirlds/platform/state/RoundHashValidatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
class RoundHashValidatorTests {
private static final WeightGenerator WEIGHT_GENERATOR = new GaussianWeightGenerator(100, 50);

private static final String STATE_JSON = "{}";

static Stream<Arguments> args() {
return Stream.of(
Arguments.of(HashValidityStatus.VALID),
Expand Down Expand Up @@ -263,8 +265,8 @@ void selfSignatureLastTest(final HashValidityStatus expectedStatus) {
final NodeHashInfo thisNode = chooseSelfNode(random, hashGenerationData, expectedStatus);

final long round = random.nextInt(1000);
final RoundHashValidator validator =
new RoundHashValidator(round, RosterUtils.computeTotalWeight(roster), Mockito.mock(IssMetrics.class));
final RoundHashValidator validator = new RoundHashValidator(
round, RosterUtils.computeTotalWeight(roster), STATE_JSON, Mockito.mock(IssMetrics.class));

boolean decided = false;

Expand Down Expand Up @@ -309,8 +311,8 @@ void selfSignatureFirstTest(final HashValidityStatus expectedStatus) {
final NodeHashInfo thisNode = chooseSelfNode(random, hashGenerationData, expectedStatus);

final long round = random.nextInt(1000);
final RoundHashValidator validator =
new RoundHashValidator(round, RosterUtils.computeTotalWeight(roster), Mockito.mock(IssMetrics.class));
final RoundHashValidator validator = new RoundHashValidator(
round, RosterUtils.computeTotalWeight(roster), STATE_JSON, Mockito.mock(IssMetrics.class));

boolean decided = false;

Expand Down Expand Up @@ -353,8 +355,8 @@ void selfSignatureInMiddleTest(final HashValidityStatus expectedStatus) {
final NodeHashInfo thisNode = chooseSelfNode(random, hashGenerationData, expectedStatus);

final long round = random.nextInt(1000);
final RoundHashValidator validator =
new RoundHashValidator(round, RosterUtils.computeTotalWeight(roster), Mockito.mock(IssMetrics.class));
final RoundHashValidator validator = new RoundHashValidator(
round, RosterUtils.computeTotalWeight(roster), STATE_JSON, Mockito.mock(IssMetrics.class));

boolean decided = false;

Expand Down Expand Up @@ -402,8 +404,8 @@ void timeoutSelfHashTest() {
final HashGenerationData hashGenerationData = generateNodeHashes(random, roster, HashValidityStatus.VALID, 0);

final long round = random.nextInt(1000);
final RoundHashValidator validator =
new RoundHashValidator(round, RosterUtils.computeTotalWeight(roster), Mockito.mock(IssMetrics.class));
final RoundHashValidator validator = new RoundHashValidator(
round, RosterUtils.computeTotalWeight(roster), STATE_JSON, Mockito.mock(IssMetrics.class));

final Map<Long, RosterEntry> nodesById = RosterUtils.toMap(roster);
for (final NodeHashInfo nodeHashInfo : hashGenerationData.nodeList) {
Expand Down Expand Up @@ -435,7 +437,8 @@ void timeoutSelfHashAndSignaturesTest() {
final HashGenerationData hashGenerationData = generateNodeHashes(random, roster, HashValidityStatus.VALID, 0);

final long round = random.nextInt(1000);
final RoundHashValidator validator = new RoundHashValidator(round, totalWeight, Mockito.mock(IssMetrics.class));
final RoundHashValidator validator =
new RoundHashValidator(round, totalWeight, STATE_JSON, Mockito.mock(IssMetrics.class));

long addedWeight = 0;

Expand Down Expand Up @@ -475,7 +478,8 @@ void timeoutSignaturesTest() {
final NodeHashInfo thisNode = chooseSelfNode(random, hashGenerationData, HashValidityStatus.VALID);

final long round = random.nextInt(1000);
final RoundHashValidator validator = new RoundHashValidator(round, totalWeight, Mockito.mock(IssMetrics.class));
final RoundHashValidator validator =
new RoundHashValidator(round, totalWeight, STATE_JSON, Mockito.mock(IssMetrics.class));

assertFalse(validator.reportSelfHash(thisNode.nodeStateHash), "should not allow a decision");

Expand Down Expand Up @@ -519,7 +523,8 @@ void timeoutWithSuperMajorityTest() {
final NodeHashInfo thisNode = chooseSelfNode(random, hashGenerationData, HashValidityStatus.CATASTROPHIC_ISS);

final long round = random.nextInt(1000);
final RoundHashValidator validator = new RoundHashValidator(round, totalWeight, Mockito.mock(IssMetrics.class));
final RoundHashValidator validator =
new RoundHashValidator(round, totalWeight, STATE_JSON, Mockito.mock(IssMetrics.class));

assertFalse(validator.reportSelfHash(thisNode.nodeStateHash), "should not allow a decision");

Expand Down
Loading