SHARD-2475 - Problematic Fixes

[mhanson-github]

PR Type

Bug fix, Enhancement, Tests

---

Description

• Refactored consecutive refutes logic to track maximum streaks

• Updated parameter defaults for debug simulation endpoints

• Adjusted tests to validate new consecutive refutes calculation

• Replaced compressed JSON export with standard JSON for cache

---

Changes walkthrough 📝

	Relevant files
Enhancement	debug.ts Use max consecutive refutes and update debug simulation defaults src/debug/debug.ts Replaced getConsecutiveRefutes with getMaxConsecutiveRefutes for node metrics. Updated default values for debug simulation query parameters. +6/-6      ProblemNodeHandler.ts Refactor and rename consecutive refutes logic                        src/p2p/ProblemNodeHandler.ts Changed export to use toJSON instead of toCompressedJSON. Renamed and refactored consecutive refutes function to getMaxConsecutiveRefutes. Updated internal logic to use new function name. +3/-3
Bug fix	ProblematicNodeCache.ts Track maximum consecutive refutes in node cache                    src/p2p/ProblematicNodeCache.ts Replaced getConsecutiveRefutes with getMaxConsecutiveRefutes. Refactored logic to compute maximum consecutive refutes in history. Updated metric calculation to use new logic. +22/-24
Tests	index.test.ts Update mocks for renamed consecutive refutes function        test/unit/debug/index.test.ts Updated mocks to use getMaxConsecutiveRefutes instead of old function. +1/-1      debug.test.ts Adjust debug test mocks for refutes function rename            test/unit/src/debug/debug.test.ts Updated mock import to use getMaxConsecutiveRefutes. +1/-1      ProblematicNodeCache.test.ts Update tests for max consecutive refutes calculation          test/unit/src/p2p/ProblematicNodeCache.test.ts Updated tests to validate maximum consecutive refutes logic. Changed test expectations to reflect new behavior. +3/-3

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🏅 Score: 87

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Logic Change The consecutive refutes calculation now tracks the maximum streak in history rather than only the current streak. This is a significant behavioral change—ensure this aligns with intended business logic and does not break downstream consumers expecting the previous behavior. getMaxConsecutiveRefutes(refuteCycles: number[], currentCycle: number): number { if (refuteCycles.length === 0) return 0 // Filter to only include refutes up to current cycle const relevantRefutes = refuteCycles.filter((cycle) => cycle <= currentCycle) if (relevantRefutes.length === 0) return 0 // Find the maximum consecutive refutes in the retained cycle history const sortedRefutes = [...relevantRefutes].sort((a, b) => a - b) let maxConsecutive = 1 // At least 1 if we have any refutes let currentConsecutive = 1 // Iterate through sorted refutes to find all consecutive sequences for (let i = 1; i < sortedRefutes.length; i++) { const currentCycle = sortedRefutes[i] const prevCycle = sortedRefutes[i - 1] if (currentCycle === prevCycle + 1) { // Consecutive with previous cycle currentConsecutive++ } else { // Gap found, update max and reset current count maxConsecutive = Math.max(maxConsecutive, currentConsecutive) currentConsecutive = 1 } } // Update max with final sequence maxConsecutive = Math.max(maxConsecutive, currentConsecutive) return maxConsecutive } Parameter Defaults Change The debug simulation endpoint now defaults parameters to non-zero values (e.g., missConsensus=1, networkDelay=100). Confirm that these new defaults are safe for all environments and do not unintentionally disrupt test or debug flows. const missConsensus = req.query.missConsensus ? parseFloat(req.query.missConsensus as string) : 1 const networkDelay = req.query.networkDelay ? parseInt(req.query.networkDelay as string) : 100 const dropMessages = req.query.dropMessages ? parseFloat(req.query.dropMessages as string) : 1 const slowResponse = req.query.slowResponse ? parseFloat(req.query.slowResponse as string) : 1 const slowDelayMs = req.query.slowDelayMs ? parseInt(req.query.slowDelayMs as string) : 10000

[github-actions]

Suggestion: Initializing maxConsecutive and currentConsecutive to 1 can produce incorrect results when sortedRefutes has only one element, or when there are gaps at the start. Set both variables to 0 and increment only when a sequence is found to avoid overcounting. [possible issue, importance: 8]

Suggested change

	let maxConsecutive = 1 // At least 1 if we have any refutes
	let currentConsecutive = 1
	// Iterate through sorted refutes to find all consecutive sequences
	for (let i = 1; i < sortedRefutes.length; i++) {
	const currentCycle = sortedRefutes[i]
	const prevCycle = sortedRefutes[i - 1]
	if (currentCycle === prevCycle + 1) {
	// Consecutive with previous cycle
	currentConsecutive++
	} else {
	// Check if consecutive with next cycle
	const nextCycle = sortedRefutes[i + 1]
	if (cycle === nextCycle - 1) {
	count++
	} else {
	break
	}
	// Gap found, update max and reset current count
	maxConsecutive = Math.max(maxConsecutive, currentConsecutive)
	currentConsecutive = 1
	}
	}
	return count
	// Update max with final sequence
	maxConsecutive = Math.max(maxConsecutive, currentConsecutive)
	return maxConsecutive
	let maxConsecutive = 0
	let currentConsecutive = 0
	// Iterate through sorted refutes to find all consecutive sequences
	for (let i = 0; i < sortedRefutes.length; i++) {
	if (i === 0 || sortedRefutes[i] === sortedRefutes[i - 1] + 1) {
	currentConsecutive++
	} else {
	maxConsecutive = Math.max(maxConsecutive, currentConsecutive)
	currentConsecutive = 1
	}
	}
	// Update max with final sequence
	maxConsecutive = Math.max(maxConsecutive, currentConsecutive)
	return maxConsecutive