Skip to content

Commit

Permalink
fix: remove extra typing
Browse files Browse the repository at this point in the history
  • Loading branch information
BelfordZ authored and mhanson-github committed Dec 27, 2024
1 parent 2ddd46e commit 1975b89
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/config/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ const SERVER_CONFIG: StrictServerConfiguration = {
maxRotatedPerCycle: 1,
flexibleRotationDelta: 1,
flexibleRotationEnabled: false,
enableProblematicNodeRemoval: true,
enableProblematicNodeRemovalOnCycle: 10,
enableProblematicNodeRemoval: false,
enableProblematicNodeRemovalOnCycle: 20000,
maxProblematicNodeRemovalsPerCycle: 1,
problematicNodeConsecutiveRefuteThreshold: 6,
problematicNodeRefutePercentageThreshold: 0.1,
Expand Down
6 changes: 3 additions & 3 deletions src/debug/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { nestedCountersInstance } from '../utils/nestedCounters'
import { logFlags } from '../logger'
import { currentCycle } from '../p2p/CycleCreator'
import * as ProblemNodeHandler from '../p2p/ProblemNodeHandler'

import { nodes, NodeWithRefuteCycles } from '../p2p/NodeList'
import { Node } from '@shardus/types/build/src/p2p/NodeListTypes'
import { nodes } from '../p2p/NodeList'
const tar = require('tar-fs')
const fs = require('fs')

Expand Down Expand Up @@ -150,7 +150,7 @@ class Debug {
const dump: Record<string, any> = {}

// Collect data for all nodes that have any refute history
for (const [nodeId, node] of nodes as Map<string, NodeWithRefuteCycles>) {
for (const [nodeId, node] of nodes as Map<string, Node>) {
if (node.refuteCycles?.size > 0) {
const refuteCycles = Array.from(node.refuteCycles).sort((a, b) => a - b)
dump[nodeId] = {
Expand Down
5 changes: 1 addition & 4 deletions src/p2p/NodeList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,15 +307,12 @@ export function removeNodes(
for (const id of ids) removeNode(id, raiseEvents, cycle)
}

// create shorthand type for node with refuteCycles
export type NodeWithRefuteCycles = P2P.NodeListTypes.Node

export function updateNode(
update: P2P.NodeListTypes.Update,
raiseEvents: boolean,
cycle: P2P.CycleCreatorTypes.CycleRecord | null
) {
const node = nodes.get(update.id) as NodeWithRefuteCycles
const node = nodes.get(update.id)
if (node) {
// Initialize refuteCycles if it doesn't exist
if (config.p2p.enableProblematicNodeRemoval) {
Expand Down
10 changes: 5 additions & 5 deletions src/p2p/ProblemNodeHandler.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { P2P } from '@shardus/types'
import * as NodeList from './NodeList'
import { config } from './Context'
import { NodeWithRefuteCycles } from './NodeList';
import { Node } from '@shardus/types/build/src/p2p/NodeListTypes';

export function isNodeProblematic(node: NodeWithRefuteCycles, currentCycle: number): boolean {
export function isNodeProblematic(node: Node, currentCycle: number): boolean {
if (!node.refuteCycles) return false;

// Check consecutive refutes
Expand Down Expand Up @@ -42,15 +42,15 @@ export function getProblematicNodes(prevRecord: P2P.CycleCreatorTypes.CycleRecor
const problematicNodes = new Set<string>();

for (const node of NodeList.activeByIdOrder) {
if (isNodeProblematic(node as NodeWithRefuteCycles, prevRecord.counter)) {
if (isNodeProblematic(node as Node, prevRecord.counter)) {
problematicNodes.add(node.id);
}
}

// Sort by refute percentage
return Array.from(problematicNodes).sort((a, b) => {
const nodeA = NodeList.nodes.get(a) as NodeWithRefuteCycles;
const nodeB = NodeList.nodes.get(b) as NodeWithRefuteCycles;
const nodeA = NodeList.nodes.get(a) as Node;
const nodeB = NodeList.nodes.get(b) as Node;
const percentageA = getRefutePercentage(nodeA.refuteCycles, prevRecord.counter);
const percentageB = getRefutePercentage(nodeB.refuteCycles, prevRecord.counter);
return percentageB - percentageA;
Expand Down
3 changes: 1 addition & 2 deletions src/p2p/Rotation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ export function updateRecord(
} else {
const { expired, removed, problematic } = getExpiredRemovedV3(prev, lastLoggedCycle, txs, info)
nestedCountersInstance.countEvent('p2p', `results of getExpiredRemovedV2: expired: ${expired} removed: ${removed.length} problematic: ${problematic}`, 1)
if (logFlags && logFlags.verbose) console.log(`results of getExpiredRemovedV2: expired: ${expired} removed: ${removed.length} array: ${removed} problematic: ${problematic}`)

/* prettier-ignore */ if(logFlags?.node_rotation_debug) logger.mainLog_debug('GETEXPIREDREMOVEDV3_STATS', `results of getExpiredRemovedV2: expired: ${expired} removed: ${removed.length} problematic: ${problematic}`)
// record.problematic = problematic // may want to write this to cycle record for
record.expired = expired
record.removed = removed // already sorted
Expand Down
74 changes: 48 additions & 26 deletions test/unit/src/p2p/ProblemNodeHandler.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { isNodeProblematic, getConsecutiveRefutes, getRefutePercentage, getProblematicNodes } from '../../../../src/p2p/ProblemNodeHandler'
import { P2P } from '@shardus/types'
import { NodeWithRefuteCycles } from '../../../../src/p2p/NodeList'
import * as NodeList from '../../../../src/p2p/NodeList'
import * as Context from '../../../../src/p2p/Context'

import { Node } from '@shardus/types/build/src/p2p/NodeListTypes';
// Mock NodeList module
jest.mock('../../../../src/p2p/NodeList', () => ({
activeByIdOrder: [],
Expand All @@ -21,8 +20,31 @@ jest.mock('../../../../src/p2p/Context', () => ({
}
}))

const baseMockNode = {
id: 'node1',
refuteCycles: new Set<number>(),
curvePublicKey: 'mockKey',
status: P2P.P2PTypes.NodeStatus.SELECTED,
cycleJoined: '0',
counterRefreshed: 0,
activeTimestamp: Date.now(),
externalIp: '127.0.0.1',
externalPort: 9001,
internalIp: '127.0.0.1',
internalPort: 9001,
publicKey: 'mockPublicKey',
timestamp: Date.now(),
version: '1.0.0',
nodeId: 'mockNodeId',
address: '127.0.0.1:9001',
joinRequestTimestamp: Date.now(),
activeCycle: 0,
syncingTimestamp: Date.now(),
readyTimestamp: Date.now()
}

describe('ProblemNodeHandler', () => {
let mockNode: NodeWithRefuteCycles
let mockNode: Node

beforeEach(() => {
// Reset config values before each test
Expand All @@ -32,9 +54,9 @@ describe('ProblemNodeHandler', () => {

// Create a mock node for testing
mockNode = {
id: 'node1',
...baseMockNode,
refuteCycles: new Set<number>(),
} as NodeWithRefuteCycles
}

// Clear NodeList mocks before each test
NodeList.activeByIdOrder.length = 0
Expand Down Expand Up @@ -141,10 +163,10 @@ describe('ProblemNodeHandler', () => {

it('should return empty array when no problematic nodes exist', () => {
// Add a non-problematic node
const node: NodeWithRefuteCycles = {
id: 'node1',
const node: Node = {
...baseMockNode,
refuteCycles: new Set([90]), // Only 1% refute rate
} as NodeWithRefuteCycles
}

NodeList.activeByIdOrder.push(node)
NodeList.nodes.set(node.id, node)
Expand All @@ -155,20 +177,20 @@ describe('ProblemNodeHandler', () => {

it('should sort problematic nodes by refute percentage', () => {
// Create nodes with different refute percentages
const node1: NodeWithRefuteCycles = {
id: 'node1',
const node1: Node = {
...baseMockNode,
refuteCycles: new Set([2, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100]), // 11% refute rate
} as NodeWithRefuteCycles
}

const node2: NodeWithRefuteCycles = {
id: 'node2',
const node2: Node = {
...baseMockNode,
refuteCycles: new Set([90, 92, 94, 96, 98, 100]), // 6% refute rate
} as NodeWithRefuteCycles
}

const node3: NodeWithRefuteCycles = {
id: 'node3',
const node3: Node = {
...baseMockNode,
refuteCycles: new Set([98, 99, 100]), // 3% refute rate but 3 consecutive
} as NodeWithRefuteCycles
}

// Add nodes to NodeList mock
NodeList.activeByIdOrder.push(node1, node2, node3)
Expand All @@ -188,10 +210,10 @@ describe('ProblemNodeHandler', () => {
})

it('should identify nodes with consecutive refutes', () => {
const node: NodeWithRefuteCycles = {
id: 'node1',
const node: Node = {
...baseMockNode,
refuteCycles: new Set([98, 99, 100]), // 3 consecutive refutes
} as NodeWithRefuteCycles
}

NodeList.activeByIdOrder.push(node)
NodeList.nodes.set(node.id, node)
Expand All @@ -201,10 +223,10 @@ describe('ProblemNodeHandler', () => {
})

it('should identify nodes with high refute percentage', () => {
const node: NodeWithRefuteCycles = {
id: 'node1',
const node: Node = {
...baseMockNode,
refuteCycles: new Set(), // Will add 11 refutes (11%)
} as NodeWithRefuteCycles
}

for (let i = 90; i <= 100; i++) {
node.refuteCycles.add(i)
Expand All @@ -218,10 +240,10 @@ describe('ProblemNodeHandler', () => {
})

it('should not include nodes that are neither consecutive nor percentage problematic', () => {
const node: NodeWithRefuteCycles = {
id: 'node1',
const node: Node = {
...baseMockNode,
refuteCycles: new Set([95, 97, 99]), // Non-consecutive and only 3%
} as NodeWithRefuteCycles
}

NodeList.activeByIdOrder.push(node)
NodeList.nodes.set(node.id, node)
Expand Down

0 comments on commit 1975b89

Please sign in to comment.