Skip to content

Commit

Permalink
Revert "Refactor NodeList module to improve performance and remove un…
Browse files Browse the repository at this point in the history
…necessary node lists code"

This reverts commit 41c86d5.
  • Loading branch information
jairajdev authored May 16, 2024
1 parent 51f6469 commit 250ac3d
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/API.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export function registerRoutes(server: FastifyInstance<Server, IncomingMessage,

if (config.experimentalSnapshot) {
const data = {
nodeList: [firstNode],
nodeList: NodeList.getList(),
}
if (cycleRecordWithShutDownMode) {
// For restore network to start the network from the 'restart' mode
Expand All @@ -136,7 +136,7 @@ export function registerRoutes(server: FastifyInstance<Server, IncomingMessage,
res = Crypto.sign<P2P.FirstNodeResponse>(data)
} else {
res = Crypto.sign<P2P.FirstNodeResponse>({
nodeList: [firstNode],
nodeList: NodeList.getList(),
joinRequest: P2P.createArchiverJoinRequest(),
dataRequestCycle: Data.createDataRequest<P2PTypes.CycleCreatorTypes.CycleRecord>(
P2PTypes.SnapshotTypes.TypeNames.CYCLE,
Expand Down
4 changes: 2 additions & 2 deletions src/Data/AccountDataProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export const validateAccountDataRequest = (
return { success: false, error: 'Invalid sign object attached' }
}
const nodePublicKey = sign.owner
if (!NodeList.byPublicKey.has(nodePublicKey)) {
if (!Object.prototype.hasOwnProperty.call(NodeList.byPublicKey, nodePublicKey)) {
return { success: false, error: 'This node is not found in the nodelist!' }
}
if (!servingValidators.has(nodePublicKey) && servingValidators.size >= config.maxValidatorsToServe) {
Expand Down Expand Up @@ -143,7 +143,7 @@ export const validateAccountDataByListRequest = (
return { success: false, error: 'Invalid sign object attached' }
}
const nodePublicKey = sign.owner
if (!NodeList.byPublicKey.has(nodePublicKey)) {
if (!Object.prototype.hasOwnProperty.call(NodeList.byPublicKey, nodePublicKey)) {
return { success: false, error: 'This node is not found in the nodelist!' }
}
// TODO: Add max limit check for accountIds list query
Expand Down
2 changes: 1 addition & 1 deletion src/Data/Cycles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export async function processCycles(cycles: P2PTypes.CycleCreatorTypes.CycleData
if (currentNetworkMode === 'shutdown') {
Logger.mainLogger.debug(Date.now(), `❌ Shutdown Cycle Record received at Cycle #: ${cycle.counter}`)
await Utils.sleep(currentCycleDuration)
NodeList.clearNodeLists()
NodeList.clearNodeListCache()
await clearDataSenders()
setShutdownCycleRecord(cycle)
NodeList.toggleFirstNode()
Expand Down
6 changes: 3 additions & 3 deletions src/Data/Data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ export async function replaceDataSender(publicKey: NodeList.ConsensusNodeInfo['p
}
unsubscribeDataSender(publicKey)
// eslint-disable-next-line security/detect-object-injection
const node = NodeList.byPublicKey.get(publicKey)
const node = NodeList.byPublicKey[publicKey]
if (node) {
const nodeIndex = NodeList.activeListByIdSorted.findIndex((node) => node.publicKey === publicKey)
if (nodeIndex > -1) {
Expand Down Expand Up @@ -537,7 +537,7 @@ export function addDataSender(sender: DataSender): void {

async function getConsensusRadius(): Promise<number> {
// If there is no node, return existing currentConsensusRadius
if (NodeList.isEmpty()) return currentConsensusRadius
if (NodeList.getList().length === 0) return currentConsensusRadius

// Define the query function to get the network config from a node
const queryFn = async (node): Promise<object> => {
Expand All @@ -564,7 +564,7 @@ async function getConsensusRadius(): Promise<number> {

// Get the list of 10 max random active nodes or the first node if no active nodes are available
const nodes =
NodeList.getActiveNodeCount() > 0 ? NodeList.getRandomActiveNodes(10) : [NodeList.getFirstNode()]
NodeList.getActiveNodeCount() > 0 ? NodeList.getRandomActiveNodes(10) : NodeList.getList().slice(0, 1)

// Use robustQuery to get the consensusRadius from multiple nodes
const tallyItem = await robustQuery(
Expand Down
98 changes: 64 additions & 34 deletions src/NodeList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,15 @@ export const fromP2PTypesNode = (node: P2PTypes.NodeListTypes.Node): JoinedConse

// STATE

const list: ConsensusNodeInfo[] = []
const standbyList: Map<string, ConsensusNodeInfo> = new Map()
const syncingList: Map<string, ConsensusNodeInfo> = new Map()
const activeList: Map<string, ConsensusNodeInfo> = new Map()

// Map to get node public key by node Id
const byId: Map<string, string> = new Map()

// Map to get node info by public key, stores all nodes
export const byPublicKey: Map<string, ConsensusNodeInfo> = new Map()

// Array of active nodes sorted by id
export let activeListByIdSorted: ConsensusNodeInfo[] = []

export let byPublicKey: { [publicKey: string]: ConsensusNodeInfo } = {}
let byIpPort: { [ipPort: string]: ConsensusNodeInfo } = {}
export let byId: { [id: string]: ConsensusNodeInfo } = {}
let publicKeyToId: { [publicKey: string]: string } = {}
export let foundFirstNode = false

export type SignedNodeList = {
Expand All @@ -90,21 +86,31 @@ export const realUpdatedTimes: Map<string, number> = new Map()

// METHODS

function getIpPort(node: Node): string {
if (node.ip && node.port) {
return node.ip + ':' + node.port
} else if (node.externalIp && node.externalPort) {
return node.externalIp + ':' + node.externalPort
}
return ''
}

export function isEmpty(): boolean {
return byPublicKey.size === 0
return list.length <= 0
}

type Node = ConsensusNodeInfo | JoinedConsensor

export function addNodes(status: NodeStatus, nodes: Node[]): void {
if (nodes.length === 0) return
Logger.mainLogger.debug(`Adding ${status} nodes to the list`, nodes.length, nodes)

for (const node of nodes) {
const ipPort = getIpPort(node)

// If node not in lists, add it
// eslint-disable-next-line security/detect-object-injection
if (!byPublicKey.has(node.publicKey)) {
if (byPublicKey[node.publicKey] === undefined && byIpPort[ipPort] === undefined) {
list.push(node)
const key = node.publicKey
switch (status) {
case NodeStatus.SYNCING:
Expand All @@ -129,16 +135,18 @@ export function addNodes(status: NodeStatus, nodes: Node[]): void {
break
}
/* eslint-disable security/detect-object-injection */
byPublicKey.set(node.publicKey, node)
byPublicKey[node.publicKey] = node

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
A property name to write to depends on a
user-provided value
.
byIpPort[ipPort] = node
/* eslint-enable security/detect-object-injection */
}

// If an id is given, update its id
if (node.id) {
const entry = byPublicKey.get(node.publicKey)
const entry = byPublicKey[node.publicKey]
if (entry) {
entry.id = node.id
byId.set(node.id, node.publicKey)
publicKeyToId[node.publicKey] = node.id

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
A property name to write to depends on a
user-provided value
.
byId[node.id] = node

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
}
}
}
Expand All @@ -147,11 +155,13 @@ export function refreshNodes(status: NodeStatus, nodes: ConsensusNodeInfo[] | Jo
if (nodes.length === 0) return
Logger.mainLogger.debug('Refreshing nodes', nodes.length, nodes)
for (const node of nodes) {
const ipPort = getIpPort(node)

// If node not in lists, add it
// eslint-disable-next-line security/detect-object-injection
if (!byPublicKey.has(node.publicKey)) {
if (byPublicKey[node.publicKey] === undefined && byIpPort[ipPort] === undefined) {
Logger.mainLogger.debug('adding new node during refresh', node.publicKey)
list.push(node)
switch (status) {
case NodeStatus.SYNCING:
syncingList.set(node.publicKey, node)
Expand All @@ -166,16 +176,18 @@ export function refreshNodes(status: NodeStatus, nodes: ConsensusNodeInfo[] | Jo
break
}

byPublicKey.set(node.publicKey, node)
byPublicKey[node.publicKey] = node

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
// eslint-disable-next-line security/detect-object-injection
byIpPort[ipPort] = node
}

// If an id is given, update its id
if (node.id) {
const entry = byPublicKey.get(node.publicKey)
const entry = byPublicKey[node.publicKey]
if (entry) {
entry.id = node.id
byId.set(node.id, node.publicKey)
publicKeyToId[node.publicKey] = node.id

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
byId[node.id] = node

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
}
}
}
Expand All @@ -184,23 +196,38 @@ export function refreshNodes(status: NodeStatus, nodes: ConsensusNodeInfo[] | Jo
export function removeNodes(publicKeys: string[]): void {
if (publicKeys.length > 0) Logger.mainLogger.debug('Removing nodes', publicKeys)
// Efficiently remove nodes from nodelist
const keysToDelete: Map<ConsensusNodeInfo['publicKey'], boolean> = new Map()

for (const key of publicKeys) {
// eslint-disable-next-line security/detect-object-injection
if (!byPublicKey.has(key)) {
if (byPublicKey[key] === undefined) {
console.warn(`removeNodes: publicKey ${key} not in nodelist`)
continue
}
keysToDelete.set(key, true)
/* eslint-disable security/detect-object-injection */
syncingList.delete(key)
activeList.delete(key)
standbyList.delete(key)
const id = byPublicKey.get(key).id
activeListByIdSorted = activeListByIdSorted.filter((node) => node.id !== byPublicKey.get(key).id)
byId.delete(id)
byPublicKey.delete(key)
delete byIpPort[getIpPort(byPublicKey[key])]
delete byPublicKey[key]
const id = publicKeyToId[key]
activeListByIdSorted = activeListByIdSorted.filter((node) => node.id !== id)
delete byId[id]

Check failure

Code scanning / CodeQL

Remote property injection High

A property name to write to depends on a
user-provided value
.
delete publicKeyToId[key]
/* eslint-enable security/detect-object-injection */
}

if (keysToDelete.size > 0) {
let key: string
for (let i = list.length - 1; i > -1; i--) {
// eslint-disable-next-line security/detect-object-injection
key = list[i].publicKey
if (keysToDelete.has(key)) {
list.splice(i, 1)
if (syncingList.has(key)) syncingList.delete(key)
else if (activeList.has(key)) activeList.delete(key)
else if (standbyList.has(key)) standbyList.delete(key)
}
}
}
}

export const addStandbyNodes = (nodes: ConsensusNodeInfo[]): void => {
Expand All @@ -224,7 +251,7 @@ export function setStatus(status: Exclude<NodeStatus, NodeStatus.STANDBY>, publi
Logger.mainLogger.debug(`Updating status ${status} for nodes`, publicKeys)
for (const key of publicKeys) {
// eslint-disable-next-line security/detect-object-injection
const node = byPublicKey.get(key)
const node = byPublicKey[key]
if (node === undefined) {
console.warn(`setStatus: publicKey ${key} not in nodelist`)
continue
Expand Down Expand Up @@ -254,8 +281,8 @@ export function setStatus(status: Exclude<NodeStatus, NodeStatus.STANDBY>, publi
}
}

export function getFirstNode(): ConsensusNodeInfo | undefined {
return byPublicKey.values().next().value;
export function getList(): ConsensusNodeInfo[] {
return list
}

export function getActiveList(id_sorted = true): ConsensusNodeInfo[] {
Expand Down Expand Up @@ -290,7 +317,7 @@ export const getCachedNodeList = (): SignedNodeList => {

for (let index = 0; index < config.N_RANDOM_NODELIST_BUCKETS; index++) {
// If we dont have any active nodes, send back the first node in our list
const nodeList = nodeCount < 1 ? [getFirstNode()] : getRandomActiveNodes(nodeCount)
const nodeList = nodeCount < 1 ? getList().slice(0, 1) : getRandomActiveNodes(nodeCount)
const sortedNodeList = [...nodeList].sort(byAscendingNodeId)
const signedSortedNodeList = Crypto.sign({
nodeList: sortedNodeList,
Expand Down Expand Up @@ -400,7 +427,7 @@ export function changeNodeListInRestore(): void {
}

/** Resets/Cleans all the NodeList associated Maps and Array variables/caches */
export function clearNodeLists(): void {
export function clearNodeListCache(): void {
try {
activeNodescache.clear()
fullNodesCache.clear()
Expand All @@ -409,8 +436,11 @@ export function clearNodeLists(): void {
realUpdatedTimes.clear()
cacheUpdatedTimes.clear()

byId.clear()
byPublicKey.clear()
list.length = 0
byId = {}
byIpPort = {}
byPublicKey = {}
publicKeyToId = {}
activeListByIdSorted = []
} catch (e) {
Logger.mainLogger.error('Error thrown in clearNodeListCache', e)
Expand Down

0 comments on commit 250ac3d

Please sign in to comment.