x/oracle/abci_test.go (3)

323-325: Redundant error check.

The require.NoError(t, err) on line 324 checks an err variable that was already checked on line 317. This is harmless but adds noise.

🧹 Suggested cleanup

 		// Validator should now be jailed
-		require.NoError(t, err)
 		require.True(t, validator.IsJailed())

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@x/oracle/abci_test.go` around lines 323 - 325, The test contains a redundant
require.NoError(t, err) immediately before asserting validator.IsJailed();
remove the duplicate error check (the earlier require.NoError already validated
err) so the test only asserts require.True(t, validator.IsJailed()) and avoids
the noisy duplicate require.NoError call referencing err.

---

422-451: Test may not fully verify the expected behavior.

The test checks that MissCount and AbstainCount remain 0, but doesn't explicitly verify that SuccessCount increased. When no votes are submitted and voteTargets becomes empty, validators should receive a success (since WinCount >= len(voteTargets) where both are 0).

Consider adding an explicit assertion for SuccessCount:

✅ Suggested enhancement

 		// Check if all three are not getting miss counts
 		for i := 0; i < 3; i++ {
 			counter, err := oracleKeeper.VotePenaltyCounter.Get(ctx, keeper.ValAddrs[i])
 			require.NoError(t, err)
+			require.EqualValues(t, uint64(1), counter.SuccessCount)
 			require.EqualValues(t, uint64(0), counter.AbstainCount)
 			require.EqualValues(t, uint64(0), counter.MissCount)
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@x/oracle/abci_test.go` around lines 422 - 451, The test "No votes
submitted..." currently asserts MissCount and AbstainCount remain 0 but omits
verifying SuccessCount; update the test after calling EndBlocker(ctx,
oracleKeeper) to fetch each validator's VotePenaltyCounter via
oracleKeeper.VotePenaltyCounter.Get(ctx, keeper.ValAddrs[i]) and add an
assertion that counter.SuccessCount has incremented as expected (e.g., equals
uint64(1) or the expected value given WinCount and voteTargets being empty).
Ensure you reference the same loop and variables (ctx, oracleKeeper,
keeper.ValAddrs) and place the SuccessCount assertion alongside the existing
AbstainCount and MissCount checks.

---

571-577: Comment is slightly misleading.

The comment states "validator 0 gets WinCount=4", but with the discardable map change in abci.go, the WinCount in validatorClaimMap only includes above-threshold wins. Validator 0's actual WinCount would be 3 (from atom, eth, kii), which still satisfies WinCount >= len(voteTargets) (3 >= 3).

📝 Suggested comment fix

-		// Validator 0 voted for all 4 denoms. Usdc went to belowThresholdVoteMap
-		// and still runs through Tally, so validator 0 gets WinCount=4 and
-		// should also get a success
+		// Validator 0 voted for all 4 denoms. Usdc went to belowThresholdVoteMap
+		// and uses a discardable claim map, so validator 0 gets WinCount=3 from
+		// above-threshold denoms, which matches len(voteTargets)=3, so they get success

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@x/oracle/abci_test.go` around lines 571 - 577, The test comment is
inaccurate: due to the discardable map change in abci.go the
validatorClaimMap.WinCount only counts above-threshold wins, so validator 0's
WinCount is 3 (atom, eth, kii) not 4; update the comment above the
VotePenaltyCounter assertion to state that WinCount=3 (and that 3 >=
len(voteTargets) so the validator still succeeds), and keep the existing
assertions using oracleKeeper.VotePenaltyCounter.Get(ctx, keeper.ValAddrs[0])
unchanged.

x/oracle/abci.go (2)

238-238: Comment is misleading.

The comment "Add jail back in repeated offence" suggests jail is being added at this location, but the actual jail implementation is in slash.go within SlashAndResetCounters. Consider clarifying that jailing happens inside SlashAndResetCounters or removing this comment.

📝 Suggested fix

-		// Add jail back in repeated offence
+		// Slash and jail validators who failed to meet the minimum valid vote threshold
		err = k.SlashAndResetCounters(ctx) // slash validator and reset voting counter

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@x/oracle/abci.go` at line 238, The comment "Add jail back in repeated
offence" in abci.go is misleading because the actual jailing logic lives in
SlashAndResetCounters (slash.go); update the comment at that location to either
remove the misleading line or replace it with a clear note such as "Jailing
handled in SlashAndResetCounters (see slash.go)" and include the function name
SlashAndResetCounters to guide readers to the implementation.

---

159-159: Consider if >= is intentional given the discardable map change.

With the discardable map for below-threshold tallies (lines 141-147), validatorClaimMap should only accumulate WinCount from above-threshold denoms. This means WinCount should at most equal len(voteTargets) for validators who voted correctly on all above-threshold denoms.

The >= operator makes this check more permissive, which is safer but may mask unexpected behavior. If WinCount ever exceeds len(voteTargets), it could indicate a logic error elsewhere. Consider whether == would be more appropriate to catch such cases, or add a debug log when WinCount > len(voteTargets).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@x/oracle/abci.go` at line 159, The condition if int(claim.WinCount) >=
len(voteTargets) on validatorClaimMap should not silently allow WinCount to
exceed the number of voteTargets; change the comparison to == to ensure a
validator is counted only when their WinCount exactly matches the number of
voteTargets, and add a debug log (or warn/error) that emits the validator ID and
the unexpected values when claim.WinCount > len(voteTargets) to catch logic
errors elsewhere; update the check around validatorClaimMap/WinCount (the if
using int(claim.WinCount) >= len(voteTargets)) and add the logging branch for
the > case.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

x/oracle/abci.go (1)

225-235: Minor: float32 precision loss for very large counts.

Converting uint64 counters to float32 loses precision for values above ~16.7 million. While unlikely for vote counts in a slash window, consider using float64 for consistency with other telemetry patterns if high precision is desired.

♻️ Optional: Use float64 for higher precision

-		telemetry.SetGaugeWithLabels([]string{types.ModuleName, "miss_count"}, float32(counter.MissCount), labels)
-		telemetry.SetGaugeWithLabels([]string{types.ModuleName, "abstain_count"}, float32(counter.AbstainCount), labels)
-		telemetry.SetGaugeWithLabels([]string{types.ModuleName, "success_count"}, float32(counter.SuccessCount), labels)
+		telemetry.SetGaugeWithLabels([]string{types.ModuleName, "miss_count"}, float32(float64(counter.MissCount)), labels)
+		telemetry.SetGaugeWithLabels([]string{types.ModuleName, "abstain_count"}, float32(float64(counter.AbstainCount)), labels)
+		telemetry.SetGaugeWithLabels([]string{types.ModuleName, "success_count"}, float32(float64(counter.SuccessCount)), labels)

Note: The Cosmos SDK telemetry API uses float32, so this is mostly academic unless counts regularly exceed millions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@x/oracle/abci.go` around lines 225 - 235, The telemetry gauges in
writeValidatorPenaltyMetrics cast uint64 counters to float32 causing precision
loss; change the casts so counter.MissCount, counter.AbstainCount and
counter.SuccessCount are converted to float64 (e.g. assign to a float64 var) and
pass that to the telemetry call (or use the telemetry float64 variant) instead
of float32; update the calls around VotePenaltyCounter.Walk and
telemetry.SetGaugeWithLabels to accept the float64 values so the counts retain
precision.