From 94dacfb7644051173c41832310f6a3da8b30e600 Mon Sep 17 00:00:00 2001 From: philon- Date: Sat, 30 Aug 2025 20:40:41 +0200 Subject: [PATCH] messageStore tests, node PKI validation Implement node validation and improve merging logic - Added `validateIncomingNode` function to validate new nodes against existing nodes, checking for public key conflicts and ensuring proper handling of node updates. - Updated `nodeDBFactory` to utilize the new validation function when adding nodes. - Enhanced `getNodes` method to optionally include the current node in the results. - Removed the `mergeNodeInfo` utility as its functionality is now integrated into the validation and merging process. - Updated tests to cover new validation logic and ensure correct behavior during node addition and merging. - Cleaned up unused utility functions related to key comparison. --- .../DeleteMessagesDialog.test.tsx | 16 +- .../PageComponents/Connect/HTTP.test.tsx | 4 +- .../Messages/MessageInput.test.tsx | 7 +- .../web/src/core/stores/messageStore/index.ts | 1 - .../stores/messageStore/messageStore.test.ts | 478 ++++++++++++------ .../web/src/core/stores/nodeDBStore/index.ts | 90 ++-- .../stores/nodeDBStore/nodeDBStore.test.ts | 231 ++++++++- .../core/stores/nodeDBStore/nodeValidation.ts | 60 +++ .../web/src/core/stores/nodeDBStore/types.ts | 5 +- .../src/core/stores/utils/mergeNodeInfo.ts | 71 --- packages/web/src/core/utils/sameKey.ts | 20 - 11 files changed, 682 insertions(+), 301 deletions(-) create mode 100644 packages/web/src/core/stores/nodeDBStore/nodeValidation.ts delete mode 100644 packages/web/src/core/stores/utils/mergeNodeInfo.ts delete mode 100644 packages/web/src/core/utils/sameKey.ts diff --git a/packages/web/src/components/Dialog/DeleteMessagesDialog/DeleteMessagesDialog.test.tsx b/packages/web/src/components/Dialog/DeleteMessagesDialog/DeleteMessagesDialog.test.tsx index 581783d8..ce02e220 100644 --- a/packages/web/src/components/Dialog/DeleteMessagesDialog/DeleteMessagesDialog.test.tsx +++ b/packages/web/src/components/Dialog/DeleteMessagesDialog/DeleteMessagesDialog.test.tsx @@ -1,11 +1,13 @@ import { DeleteMessagesDialog } from "@components/Dialog/DeleteMessagesDialog/DeleteMessagesDialog.tsx"; import { fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; -// Ensure the path is correct for import -import { useMessageStore } from "@core/stores"; +import { useMessages, type MessageStore } from "@core/stores"; vi.mock("@core/stores", () => ({ - useMessageStore: vi.fn(() => ({ + CurrentDeviceContext: { + _currentValue: { deviceId: 1234 }, + }, + useMessages: vi.fn(() => ({ deleteAllMessages: vi.fn(), })), })); @@ -18,11 +20,11 @@ describe("DeleteMessagesDialog", () => { mockOnOpenChange.mockClear(); mockClearAllMessages.mockClear(); - const mockedUseMessageStore = vi.mocked(useMessageStore); - mockedUseMessageStore.mockImplementation(() => ({ + const mockedUseMessages = vi.mocked(useMessages); + mockedUseMessages.mockImplementation(() => ({ deleteAllMessages: mockClearAllMessages, - })); - mockedUseMessageStore.mockClear(); + } as unknown as MessageStore)); + mockedUseMessages.mockClear(); }); it("calls onOpenChange with false when the close button (X) is clicked", () => { diff --git a/packages/web/src/components/PageComponents/Connect/HTTP.test.tsx b/packages/web/src/components/PageComponents/Connect/HTTP.test.tsx index 8c0733f1..34040002 100644 --- a/packages/web/src/components/PageComponents/Connect/HTTP.test.tsx +++ b/packages/web/src/components/PageComponents/Connect/HTTP.test.tsx @@ -9,7 +9,9 @@ vi.mock("@core/stores", () => ({ useDeviceStore: vi.fn(() => ({ addDevice: vi.fn(() => ({ addConnection: vi.fn() })), })), - useMessageStore: vi.fn(), + useMessageStore: vi.fn(() => ({ + addMessageStore: vi.fn(), + })), useNodeDBStore: vi.fn(() => ({ addNodeDB: vi.fn(), })), diff --git a/packages/web/src/components/PageComponents/Messages/MessageInput.test.tsx b/packages/web/src/components/PageComponents/Messages/MessageInput.test.tsx index 9f3ca7ab..8d546428 100644 --- a/packages/web/src/components/PageComponents/Messages/MessageInput.test.tsx +++ b/packages/web/src/components/PageComponents/Messages/MessageInput.test.tsx @@ -40,8 +40,11 @@ const mockSetDraft = vi.fn(); const mockGetDraft = vi.fn(); const mockClearDraft = vi.fn(); -vi.mock("@core/stores/messageStore", () => ({ - useMessageStore: vi.fn(() => ({ +vi.mock("@core/stores", () => ({ + CurrentDeviceContext: { + _currentValue: { deviceId: 1234 }, + }, + useMessages: vi.fn(() => ({ setDraft: mockSetDraft, getDraft: mockGetDraft, clearDraft: mockClearDraft, diff --git a/packages/web/src/core/stores/messageStore/index.ts b/packages/web/src/core/stores/messageStore/index.ts index b1cb838b..accd6696 100644 --- a/packages/web/src/core/stores/messageStore/index.ts +++ b/packages/web/src/core/stores/messageStore/index.ts @@ -149,7 +149,6 @@ function messageStoreFactory( ); }, - // TODO: Cycle out old messages based on retention policy saveMessage: (message: Message) => { set( produce((draft) => { diff --git a/packages/web/src/core/stores/messageStore/messageStore.test.ts b/packages/web/src/core/stores/messageStore/messageStore.test.ts index 5cc8a9ee..caf2d9ef 100644 --- a/packages/web/src/core/stores/messageStore/messageStore.test.ts +++ b/packages/web/src/core/stores/messageStore/messageStore.test.ts @@ -1,30 +1,36 @@ +/** biome-ignore-all lint/style/noNonNullAssertion: */ import { Types } from "@meshtastic/core"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { - getConversationId, - MessageState, - MessageType, - useMessageStore, -} from "./index.ts"; -import type { - ChannelId, - ConversationId, - Message, - MessageLogMap, -} from "./types.ts"; - -vi.mock("../utils/indexDB.ts", () => { - const memoryStorage: Record = {}; +import { getConversationId, MessageState, MessageType } from "./index.ts"; +import type { ChannelId, Message } from "./types.ts"; + +const idbMem = new Map(); +vi.mock("idb-keyval", () => ({ + get: vi.fn((key: string) => Promise.resolve(idbMem.get(key))), + set: vi.fn((key: string, val: string) => { + idbMem.set(key, val); + return Promise.resolve(); + }), + del: vi.fn((k: string) => { + idbMem.delete(k); + return Promise.resolve(); + }), +})); + +async function freshStore() { + vi.resetModules(); + const mod = await import("../messageStore"); + return mod; +} + +vi.mock("@core/services/featureFlags", () => { return { - storageWithMapSupport: { - getItem: vi.fn(async (name: string): Promise => { - return (await memoryStorage[name]) ?? null; - }), - setItem: vi.fn(async (name: string, value: string): Promise => { - memoryStorage[name] = await value; - }), - removeItem: vi.fn(async (name: string): Promise => { - await delete memoryStorage[name]; + featureFlags: { + get: vi.fn((key: string) => { + if (key === "persistMessages") { + return true; + } + return false; }), }, }; @@ -90,53 +96,56 @@ const broadcastMessage2: Message = { message: "Broadcast message 2", }; -describe("useMessageStore", () => { - const initialState = useMessageStore.getState(); - +describe("MessageStore persistence & rehydrate", () => { beforeEach(() => { - useMessageStore.setState( - { - ...initialState, - messages: { - direct: new Map(), - broadcast: new Map(), - }, - draft: new Map(), - }, - true, - ); + idbMem.clear(); + vi.clearAllMocks(); }); - it("should have correct initial state", () => { + it("should have correct initial state", async () => { + const { useMessageStore } = await freshStore(); const state = useMessageStore.getState(); - expect(state.messages.direct).toBeInstanceOf(Map); - expect(state.messages.direct.size).toBe(0); - expect(state.messages.broadcast).toBeInstanceOf(Map); - expect(state.messages.broadcast.size).toBe(0); - expect(state.draft).toBeInstanceOf(Map); - expect(state.draft.size).toBe(0); - expect(state.nodeNum).toBe(0); - expect(state.activeChat).toBe(0); - expect(state.chatType).toBe(MessageType.Broadcast); + const store = state.addMessageStore(123); + + expect(store.messages.direct).toBeInstanceOf(Map); + expect(store.messages.direct.size).toBe(0); + expect(store.messages.broadcast).toBeInstanceOf(Map); + expect(store.messages.broadcast.size).toBe(0); + expect(store.drafts).toBeInstanceOf(Map); + expect(store.drafts.size).toBe(0); + expect(store.myNodeNum).toBe(undefined); + expect(store.activeChat).toBe(0); + expect(store.chatType).toBe(MessageType.Broadcast); }); - it("should set nodeNum", () => { - useMessageStore.getState().setNodeNum(myNodeNum); - expect(useMessageStore.getState().nodeNum).toBe(myNodeNum); + it("should set nodeNum", async () => { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + const db = state.addMessageStore(123); + + db.setNodeNum(myNodeNum); + expect(useMessageStore.getState().getMessageStore(123)?.myNodeNum).toBe( + myNodeNum, + ); }); - describe("saveMessage", () => { + describe("saveMessage", async () => { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + state.addMessageStore(123); + it("should save a direct message with correct Map structure", () => { - useMessageStore.getState().saveMessage(directMessageToOther1); - const state = useMessageStore.getState(); + state.getMessageStore(123)?.saveMessage(directMessageToOther1); const conversationId = getConversationId( directMessageToOther1.from, directMessageToOther1.to, ); + const store = state.getMessageStore(123)!; + // Check if the conversation Map exists - expect(state.messages.direct.has(conversationId)).toBe(true); - const conversationLog = state.messages.direct.get(conversationId); + expect(store.messages.direct.has(conversationId)).toBe(true); + const conversationLog = store.messages.direct.get(conversationId); // Check if the inner Map (MessageLogMap) exists and is a Map expect(conversationLog).toBeInstanceOf(Map); // Check if the message exists within the inner Map @@ -148,12 +157,12 @@ describe("useMessageStore", () => { }); it("should save a broadcast message with correct Map structure", () => { - useMessageStore.getState().saveMessage(broadcastMessage1); - const state = useMessageStore.getState(); + state.getMessageStore(123)?.saveMessage(broadcastMessage1); + const store = state.getMessageStore(123)!; const channelId = broadcastMessage1.channel; - expect(state.messages.broadcast.has(channelId)).toBe(true); - const channelLog = state.messages.broadcast.get(channelId); + expect(store.messages.broadcast.has(channelId)).toBe(true); + const channelLog = store.messages.broadcast.get(channelId); expect(channelLog).toBeInstanceOf(Map); expect(channelLog?.has(broadcastMessage1.messageId)).toBe(true); expect(channelLog?.get(broadcastMessage1.messageId)).toEqual( @@ -162,46 +171,50 @@ describe("useMessageStore", () => { }); it("should save multiple messages correctly", () => { - useMessageStore.getState().saveMessage(directMessageToOther1); - useMessageStore.getState().saveMessage(directMessageFromOther1); - useMessageStore.getState().saveMessage(broadcastMessage1); + state.getMessageStore(123)?.saveMessage(directMessageToOther1); + state.getMessageStore(123)?.saveMessage(directMessageFromOther1); + state.getMessageStore(123)?.saveMessage(broadcastMessage1); - const state = useMessageStore.getState(); + const store = state.getMessageStore(123)!; const convId1 = getConversationId(myNodeNum, otherNodeNum1); expect( - state.messages.direct + store.messages.direct .get(convId1) ?.get(directMessageToOther1.messageId), ).toEqual(directMessageToOther1); expect( - state.messages.direct + store.messages.direct .get(convId1) ?.get(directMessageFromOther1.messageId), ).toEqual(directMessageFromOther1); const channelId = broadcastMessage1.channel; expect( - state.messages.broadcast + store.messages.broadcast .get(channelId) ?.get(broadcastMessage1.messageId), ).toEqual(broadcastMessage1); }); }); - describe("getMessages", () => { + describe("getMessages", async () => { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + state.addMessageStore(123); + beforeEach(() => { - useMessageStore.getState().setNodeNum(myNodeNum); - useMessageStore.getState().saveMessage(directMessageToOther1); - useMessageStore.getState().saveMessage(directMessageFromOther1); - useMessageStore.getState().saveMessage(directMessageToOther2); - useMessageStore.getState().saveMessage(broadcastMessage1); - useMessageStore.getState().saveMessage(broadcastMessage2); + state.getMessageStore(123)?.setNodeNum(myNodeNum); + state.getMessageStore(123)?.saveMessage(directMessageToOther1); + state.getMessageStore(123)?.saveMessage(directMessageFromOther1); + state.getMessageStore(123)?.saveMessage(directMessageToOther2); + state.getMessageStore(123)?.saveMessage(broadcastMessage1); + state.getMessageStore(123)?.saveMessage(broadcastMessage2); }); it("should return broadcast messages for a channel, sorted by date", () => { - const messages = useMessageStore.getState().getMessages({ + const messages = state.getMessageStore(123)!.getMessages({ type: MessageType.Broadcast, channelId: broadcastChannel, }); @@ -211,7 +224,7 @@ describe("useMessageStore", () => { }); it("should return empty array for broadcast if channel has no messages", () => { - const messages = useMessageStore.getState().getMessages({ + const messages = state.getMessageStore(123)!.getMessages({ type: MessageType.Broadcast, channelId: Types.ChannelNumber.Channel1, }); @@ -219,7 +232,7 @@ describe("useMessageStore", () => { }); it("should return combined direct messages for a specific chat pair, sorted by date", () => { - const messages = useMessageStore.getState().getMessages({ + const messages = state.getMessageStore(123)!.getMessages({ type: MessageType.Direct, nodeA: myNodeNum, nodeB: otherNodeNum1, @@ -230,7 +243,7 @@ describe("useMessageStore", () => { }); it("should return only relevant direct messages for a different chat pair", () => { - const messages = useMessageStore.getState().getMessages({ + const messages = state.getMessageStore(123)!.getMessages({ type: MessageType.Direct, nodeA: myNodeNum, nodeB: otherNodeNum2, @@ -240,7 +253,7 @@ describe("useMessageStore", () => { }); it("should return empty array for direct chat if no messages exist", () => { - const messages = useMessageStore.getState().getMessages({ + const messages = state.getMessageStore(123)!.getMessages({ type: MessageType.Direct, nodeA: myNodeNum, nodeB: 999, @@ -249,16 +262,20 @@ describe("useMessageStore", () => { }); }); - describe("setMessageState", () => { + describe("setMessageState", async () => { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + state.addMessageStore(123); + beforeEach(() => { - useMessageStore.getState().setNodeNum(myNodeNum); - useMessageStore.getState().saveMessage(directMessageToOther1); - useMessageStore.getState().saveMessage(directMessageFromOther1); - useMessageStore.getState().saveMessage(broadcastMessage1); + state.getMessageStore(123)?.setNodeNum(myNodeNum); + state.getMessageStore(123)?.saveMessage(directMessageToOther1); + state.getMessageStore(123)?.saveMessage(directMessageFromOther1); + state.getMessageStore(123)?.saveMessage(broadcastMessage1); }); it("should update state for a direct message", () => { - useMessageStore.getState().setMessageState({ + state.getMessageStore(123)!.setMessageState({ type: MessageType.Direct, nodeA: directMessageToOther1.from, nodeB: directMessageToOther1.to, @@ -271,13 +288,14 @@ describe("useMessageStore", () => { ); const message = useMessageStore .getState() + .getMessageStore(123)! .messages.direct.get(conversationId) ?.get(directMessageToOther1.messageId); expect(message?.state).toBe(MessageState.Ack); }); it("should update state for another direct message in the same conversation", () => { - useMessageStore.getState().setMessageState({ + state.getMessageStore(123)!.setMessageState({ type: MessageType.Direct, nodeA: directMessageFromOther1.from, nodeB: directMessageFromOther1.to, @@ -290,13 +308,14 @@ describe("useMessageStore", () => { ); const message = useMessageStore .getState() + .getMessageStore(123)! .messages.direct.get(conversationId) ?.get(directMessageFromOther1.messageId); expect(message?.state).toBe(MessageState.Failed); }); it("should update state for a broadcast message", () => { - useMessageStore.getState().setMessageState({ + state.getMessageStore(123)!.setMessageState({ type: MessageType.Broadcast, channelId: broadcastChannel, messageId: broadcastMessage1.messageId, @@ -304,6 +323,7 @@ describe("useMessageStore", () => { }); const message = useMessageStore .getState() + .getMessageStore(123)! .messages.broadcast.get(broadcastChannel) ?.get(broadcastMessage1.messageId); expect(message?.state).toBe(MessageState.Ack); @@ -311,7 +331,7 @@ describe("useMessageStore", () => { it("should warn if message is not found (direct)", () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - useMessageStore.getState().setMessageState({ + state.getMessageStore(123)!.setMessageState({ type: MessageType.Direct, nodeA: myNodeNum, nodeB: otherNodeNum1, @@ -328,7 +348,7 @@ describe("useMessageStore", () => { it("should warn if message is not found (broadcast)", () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - useMessageStore.getState().setMessageState({ + state.getMessageStore(123)!.setMessageState({ type: MessageType.Broadcast, channelId: broadcastChannel, messageId: 999, @@ -344,7 +364,7 @@ describe("useMessageStore", () => { it("should warn if conversation/channel is not found", () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - useMessageStore.getState().setMessageState({ + state.getMessageStore(123)!.setMessageState({ type: MessageType.Direct, nodeA: myNodeNum, nodeB: 998, @@ -360,14 +380,18 @@ describe("useMessageStore", () => { }); }); - describe("clearMessageByMessageId", () => { + describe("clearMessageByMessageId", async () => { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + state.addMessageStore(123); + const extraDirectMessageId = 1011; beforeEach(() => { - useMessageStore.getState().setNodeNum(myNodeNum); - useMessageStore.getState().saveMessage(directMessageToOther1); - useMessageStore.getState().saveMessage(directMessageFromOther1); - useMessageStore.getState().saveMessage(broadcastMessage1); - useMessageStore.getState().saveMessage({ + state.getMessageStore(123)?.setNodeNum(myNodeNum); + state.getMessageStore(123)?.saveMessage(directMessageToOther1); + state.getMessageStore(123)?.saveMessage(directMessageFromOther1); + state.getMessageStore(123)?.saveMessage(broadcastMessage1); + state.getMessageStore(123)?.saveMessage({ ...directMessageToOther1, messageId: extraDirectMessageId, date: Date.now() + 50, @@ -380,21 +404,21 @@ describe("useMessageStore", () => { const nodeB = directMessageToOther1.to; const conversationId = getConversationId(nodeA, nodeB); - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Direct, nodeA: nodeA, nodeB: nodeB, messageId: messageIdToDelete, }); - const state = useMessageStore.getState(); - const conversationLog = state.messages.direct.get(conversationId); + const store = useMessageStore.getState().getMessageStore(123)!; + const conversationLog = store.messages.direct.get(conversationId); expect(conversationLog?.has(messageIdToDelete)).toBe(false); expect(conversationLog?.has(extraDirectMessageId)).toBe(true); expect(conversationLog?.has(directMessageFromOther1.messageId)).toBe( true, ); - expect(state.messages.direct.has(conversationId)).toBe(true); + expect(store.messages.direct.has(conversationId)).toBe(true); }); it("should delete another specific direct message", () => { @@ -403,15 +427,15 @@ describe("useMessageStore", () => { const nodeB = directMessageFromOther1.to; const conversationId = getConversationId(nodeA, nodeB); - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Direct, nodeA: nodeA, nodeB: nodeB, messageId: messageIdToDelete, }); - const state = useMessageStore.getState(); - const conversationLog = state.messages.direct.get(conversationId); + const store = useMessageStore.getState().getMessageStore(123)!; + const conversationLog = store.messages.direct.get(conversationId); expect(conversationLog?.has(messageIdToDelete)).toBe(false); expect(conversationLog?.has(directMessageToOther1.messageId)).toBe(true); expect(conversationLog?.has(extraDirectMessageId)).toBe(true); @@ -421,15 +445,15 @@ describe("useMessageStore", () => { const messageIdToDelete = broadcastMessage1.messageId; const channelId = broadcastMessage1.channel; - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Broadcast, channelId: channelId, messageId: messageIdToDelete, }); - const state = useMessageStore.getState(); + const store = useMessageStore.getState().getMessageStore(123)!; expect( - state.messages.broadcast.get(channelId)?.get(messageIdToDelete), + store.messages.broadcast.get(channelId)?.get(messageIdToDelete), ).toBeUndefined(); }); @@ -440,37 +464,37 @@ describe("useMessageStore", () => { ); const broadcastChanId = broadcastMessage1.channel; - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Direct, nodeA: directMessageToOther1.from, nodeB: directMessageToOther1.to, messageId: directMessageToOther1.messageId, }); - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Direct, nodeA: directMessageFromOther1.from, nodeB: directMessageFromOther1.to, messageId: directMessageFromOther1.messageId, }); - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Direct, nodeA: directMessageToOther1.from, nodeB: directMessageToOther1.to, messageId: extraDirectMessageId, }); - expect(useMessageStore.getState().messages.direct.has(directConvId)).toBe( - false, - ); + expect( + state.getMessageStore(123)?.messages.direct.has(directConvId), + ).toBe(false); - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Broadcast, channelId: broadcastChanId, messageId: broadcastMessage1.messageId, }); expect( - useMessageStore.getState().messages.broadcast.has(broadcastChanId), + state.getMessageStore(123)?.messages.broadcast.has(broadcastChanId), ).toBe(false); }); @@ -479,7 +503,7 @@ describe("useMessageStore", () => { const conversationId = getConversationId(myNodeNum, otherNodeNum1); expect(() => { - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Direct, nodeA: myNodeNum, nodeB: otherNodeNum1, @@ -487,8 +511,8 @@ describe("useMessageStore", () => { }); }).not.toThrow(); - const state = useMessageStore.getState(); - const conversationLog = state.messages.direct.get(conversationId); + const store = useMessageStore.getState().getMessageStore(123)!; + const conversationLog = store.messages.direct.get(conversationId); expect(conversationLog?.size).toBe(3); // 101, 102, 1011 expect(warnSpy).toHaveBeenCalledWith( expect.stringContaining("not found in direct chat"), @@ -500,7 +524,7 @@ describe("useMessageStore", () => { it("should not error when trying to delete from non-existent conversation/channel", () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); expect(() => { - useMessageStore.getState().clearMessageByMessageId({ + state.getMessageStore(123)?.clearMessageByMessageId({ type: MessageType.Direct, nodeA: myNodeNum, nodeB: 9998, @@ -518,63 +542,235 @@ describe("useMessageStore", () => { }); }); - describe("Drafts", () => { + describe("Drafts", async () => { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + state.addMessageStore(123); + const draftKeyDirect = otherNodeNum1; const draftKeyBroadcast = broadcastChannel; const draftMessage = "This is a draft"; it("should set and get a draft for direct chat", () => { - useMessageStore.getState().setDraft(draftKeyDirect, draftMessage); - expect(useMessageStore.getState().draft.get(draftKeyDirect)).toBe( + state.getMessageStore(123)?.setDraft(draftKeyDirect, draftMessage); + expect(state.getMessageStore(123)?.drafts.get(draftKeyDirect)).toBe( draftMessage, ); - expect(useMessageStore.getState().getDraft(draftKeyDirect)).toBe( + expect(state.getMessageStore(123)?.getDraft(draftKeyDirect)).toBe( draftMessage, ); }); it("should set and get a draft for broadcast chat", () => { - useMessageStore.getState().setDraft(draftKeyBroadcast, draftMessage); - expect(useMessageStore.getState().draft.get(draftKeyBroadcast)).toBe( + state.getMessageStore(123)?.setDraft(draftKeyBroadcast, draftMessage); + expect(state.getMessageStore(123)?.drafts.get(draftKeyBroadcast)).toBe( draftMessage, ); - expect(useMessageStore.getState().getDraft(draftKeyBroadcast)).toBe( + expect(state.getMessageStore(123)?.getDraft(draftKeyBroadcast)).toBe( draftMessage, ); }); it("should return empty string for non-existent draft", () => { - expect(useMessageStore.getState().getDraft(999)).toBe(""); + expect(state.getMessageStore(123)?.getDraft(999)).toBe(""); }); it("should clear a draft", () => { - useMessageStore.getState().setDraft(draftKeyDirect, draftMessage); - expect(useMessageStore.getState().draft.has(draftKeyDirect)).toBe(true); - useMessageStore.getState().clearDraft(draftKeyDirect); - expect(useMessageStore.getState().draft.has(draftKeyDirect)).toBe(false); - expect(useMessageStore.getState().getDraft(draftKeyDirect)).toBe(""); + state.getMessageStore(123)?.setDraft(draftKeyDirect, draftMessage); + expect(state.getMessageStore(123)?.drafts.has(draftKeyDirect)).toBe(true); + state.getMessageStore(123)?.clearDraft(draftKeyDirect); + expect(state.getMessageStore(123)?.drafts.has(draftKeyDirect)).toBe( + false, + ); + expect(state.getMessageStore(123)?.getDraft(draftKeyDirect)).toBe(""); }); }); - describe("deleteAllMessages", () => { + describe("deleteAllMessages", async () => { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + state.addMessageStore(123); + it("should clear all direct and broadcast messages, leaving empty Maps", () => { - useMessageStore.getState().saveMessage(directMessageToOther1); - useMessageStore.getState().saveMessage(broadcastMessage1); + state.getMessageStore(123)?.saveMessage(directMessageToOther1); + state.getMessageStore(123)?.saveMessage(broadcastMessage1); - expect(useMessageStore.getState().messages.direct.size).toBeGreaterThan( + expect(state.getMessageStore(123)?.messages.direct.size).toBeGreaterThan( 0, ); expect( - useMessageStore.getState().messages.broadcast.size, + state.getMessageStore(123)?.messages.broadcast.size, ).toBeGreaterThan(0); - useMessageStore.getState().deleteAllMessages(); + state.getMessageStore(123)?.deleteAllMessages(); + const store = useMessageStore.getState().getMessageStore(123)!; + expect(store.messages.direct).toBeInstanceOf(Map); + expect(store.messages.direct.size).toBe(0); + expect(store.messages.broadcast).toBeInstanceOf(Map); + expect(store.messages.broadcast.size).toBe(0); + }); + }); + + describe("persistence", () => { + it("partialize persists data; onRehydrateStorage rebuilds methods (messages + drafts survive)", async () => { + { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + + const store = state.addMessageStore(123); + store.setNodeNum(321); + + const convId = getConversationId(myNodeNum, otherNodeNum1); + store.saveMessage(directMessageToOther1); + store.saveMessage(broadcastMessage1); + store.setDraft( + otherNodeNum1 as unknown as Types.Destination, + "draft-text", + ); + + const store2 = state.addMessageStore(123); + + expect(store2.messages.direct.has(convId)).toBe(true); + expect(store2.messages.direct.get(convId)?.has(101)).toBe(true); + expect(store2.messages.broadcast.get(broadcastChannel)?.has(201)).toBe( + true, + ); + expect( + store2.getDraft(otherNodeNum1 as unknown as Types.Destination), + ).toBe("draft-text"); + } + { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + + const store = state.getMessageStore(123)!; // rebuilt instance + expect(store).toBeTruthy(); + + // Methods should work after rehydrate + const directMsgs = store.getMessages({ + type: MessageType.Direct, + nodeA: myNodeNum, + nodeB: otherNodeNum1, + }); + expect(directMsgs.map((m) => m.messageId)).toEqual([101]); + + const bMsgs = store.getMessages({ + type: MessageType.Broadcast, + channelId: broadcastChannel, + }); + expect(bMsgs.map((m) => m.messageId)).toEqual([201]); + + expect( + store.getDraft(otherNodeNum1 as unknown as Types.Destination), + ).toBe("draft-text"); + + store.saveMessage(directMessageFromOther1); + + const after = store.getMessages({ + type: MessageType.Direct, + nodeA: myNodeNum, + nodeB: otherNodeNum1, + }); + expect(after.map((m) => m.messageId)).toEqual([101, 102]); + expect(after[1]?.state).toBe(MessageState.Waiting); + } + }); + + it("removeMessageStore persists removal across reload", async () => { + { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + const store = state.addMessageStore(99); + + store.setNodeNum(42); + expect(state.getMessageStore(99)).toBeDefined(); + + state.removeMessageStore(99); + expect(state.getMessageStore(99)).toBeUndefined(); + } + { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + expect(state.getMessageStore(99)).toBeUndefined(); // still gone + } + }); + + it("rehydrate only rebuilds stores with myNodeNum set (orphans dropped)", async () => { + { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + + // Orphan (no myNodeNum) + const orphan = state.addMessageStore(500); + orphan.saveMessage(broadcastMessage1); + + // Proper store + const good = state.addMessageStore(501); + good.setNodeNum(777); + good.saveMessage(broadcastMessage2); + } + { + const { useMessageStore } = await freshStore(); + const state = useMessageStore.getState(); + + expect(state.getMessageStore(500)).toBeUndefined(); // orphan dropped + const kept = state.getMessageStore(501); + expect(kept).toBeDefined(); + + expect(kept?.messages.broadcast.get(broadcastChannel)?.has(202)).toBe( + true, + ); + } + }); + + it("evicts the earliest-added message store when exceeding cap of 10", async () => { + const { useMessageStore } = await freshStore(); const state = useMessageStore.getState(); - expect(state.messages.direct).toBeInstanceOf(Map); - expect(state.messages.direct.size).toBe(0); - expect(state.messages.broadcast).toBeInstanceOf(Map); - expect(state.messages.broadcast.size).toBe(0); + + for (let i = 1; i <= 10; i++) { + state.addMessageStore(i); + } + // Adding the 11th should evict the earliest (id=1) + state.addMessageStore(11); + + expect(state.getMessageStore(1)).toBeUndefined(); // evicted + expect(state.getMessageStore(2)).toBeDefined(); // still there + expect(state.getMessageStore(11)).toBeDefined(); // newest kept + }); + + it("keeps only the latest 1000 messages in a broadcast channel (oldest trimmed)", async () => { + const { useMessageStore, MessageType, MessageState } = await freshStore(); + const state = useMessageStore.getState(); + + const store = state.addMessageStore(123); + + const channelId = 0 as number; + const total = 1005; + + for (let i = 1; i <= total; i++) { + store.saveMessage({ + type: MessageType.Broadcast, + from: 123, + to: 0xffffffff, + channel: channelId, + date: Date.now() + i, + messageId: i, + state: MessageState.Waiting, + message: `m${i}`, + }); + } + + const fresh = useMessageStore.getState().getMessageStore(123)!; + const log = fresh.messages.broadcast.get(channelId)!; + + expect(log.size).toBe(1000); // capped + for (let i = 1; i <= 5; i++) { + expect(log.has(i)).toBe(false); // oldest removed + } + for (let i = 6; i <= 1005; i++) { + expect(log.has(i)).toBe(true); // newest kept + } }); }); }); diff --git a/packages/web/src/core/stores/nodeDBStore/index.ts b/packages/web/src/core/stores/nodeDBStore/index.ts index d0abf1b4..a5d633fe 100644 --- a/packages/web/src/core/stores/nodeDBStore/index.ts +++ b/packages/web/src/core/stores/nodeDBStore/index.ts @@ -1,7 +1,7 @@ +import { validateIncomingNode } from "@app/core/stores/nodeDBStore/nodeValidation"; import { create } from "@bufbuild/protobuf"; import { featureFlags } from "@core/services/featureFlags"; import { createStorage } from "@core/stores/utils/indexDB.ts"; -import { mergeNodeInfo } from "@core/stores/utils/mergeNodeInfo"; import { Protobuf, type Types } from "@meshtastic/core"; import { produce } from "immer"; import { create as createStore, type StateCreator } from "zustand"; @@ -34,6 +34,7 @@ export interface NodeDB { getNode: (nodeNum: number) => Protobuf.Mesh.NodeInfo | undefined; getNodes: ( filter?: (node: Protobuf.Mesh.NodeInfo) => boolean, + includeSelf?: boolean, ) => Protobuf.Mesh.NodeInfo[]; getMyNode: () => Protobuf.Mesh.NodeInfo; @@ -86,14 +87,20 @@ function nodeDBFactory( if (!nodeDB) { throw new Error(`No nodeDB found (id: ${id})`); } - const prev = - nodeDB.nodeMap.get(node.num) ?? - create(Protobuf.Mesh.NodeInfoSchema); // defaults if first time - - const next = mergeNodeInfo(prev, node, (num, err) => { - nodeDB.setNodeError(num, err); - }); - + // Use validation to check the new node before adding + const next = validateIncomingNode( + node, + (nodeNum: number, err: NodeErrorType) => { + nodeDB.setNodeError(nodeNum, err); + }, + (filter?: (node: Protobuf.Mesh.NodeInfo) => boolean) => + nodeDB.getNodes(filter, true), + ); + + if (!next) { + // Validation failed and error has been set inside validateIncomingNode + return; + } nodeDB.nodeMap.set(node.num, next); }), ), @@ -174,7 +181,7 @@ function nodeDBFactory( } const node = nodeDB.nodeMap.get(data.from); if (node) { - node.lastHeard = data.time; + node.lastHeard = data.time > 0 ? data.time : Date.now(); // fallback to now if time is 0 or negative node.snr = data.snr; nodeDB.nodeMap.set(data.from, node); } else { @@ -182,7 +189,7 @@ function nodeDBFactory( data.from, create(Protobuf.Mesh.NodeInfoSchema, { num: data.from, - lastHeard: data.time, + lastHeard: data.time > 0 ? data.time : Date.now(), // fallback to now if time is 0 or negative, snr: data.snr, }), ); @@ -234,38 +241,44 @@ function nodeDBFactory( for (const [key, oldDB] of draft.nodeDBs) { if (key === id) { + // short-circuit self continue; } if (oldDB.myNodeNum === nodeNum) { - // We found the oldDB (same myNodeNum). Merge node-by-node. - const mergedNodeMap = new Map( - oldDB.nodeMap, - ); - const mergedErrors = new Map(oldDB.nodeErrors); - - for (const [num, newNode] of newDB.nodeMap.entries()) { - const oldNode = mergedNodeMap.get(num); - if (!oldNode) { - // brand new node → add directly - mergedNodeMap.set(num, newNode); - continue; - } - // Merge fields; user conflicts will be flagged and old.user retained - const m = mergeNodeInfo(oldNode, newNode, (n, err) => - mergedErrors.set(n, { node: n, error: err }), + // We found the oldDB (same myNodeNum). Merge node-by-node as if the new nodes are added with addNode + + const mergedNodes = new Map(oldDB.nodeMap); + const mergedErrors = new Map(oldDB.nodeErrors); + + const getNodesProxy = ( + filter?: (node: Protobuf.Mesh.NodeInfo) => boolean, + ): Protobuf.Mesh.NodeInfo[] => { + const arr = Array.from(mergedNodes.values()); + return filter ? arr.filter(filter) : arr; + }; + + const setErrorProxy = (nodeNum: number, err: NodeErrorType) => { + mergedErrors.set(nodeNum, { error: err } as NodeError); + }; + + for (const [nodeNum, newNode] of newDB.nodeMap) { + const next = validateIncomingNode( + newNode, + setErrorProxy, + getNodesProxy, ); - mergedNodeMap.set(num, m); - } + if (next) { + mergedNodes.set(nodeNum, next); + } - // bring over any new errors that old didn't have - for (const [n, err] of newDB.nodeErrors.entries()) { - if (!mergedErrors.has(n)) { - mergedErrors.set(n, err); + const err = newDB.getNodeError(nodeNum); + if (err && !oldDB.hasNodeError(nodeNum)) { + mergedErrors.set(nodeNum, err); } } - // finalize: move merged data into newDB and drop oldDB entry - newDB.nodeMap = mergedNodeMap; + // finalize: move maps into newDB and drop oldDB entry + newDB.nodeMap = mergedNodes; newDB.nodeErrors = mergedErrors; draft.nodeDBs.delete(oldDB.id); } @@ -319,14 +332,15 @@ function nodeDBFactory( return nodeDB.nodeMap.get(nodeNum); }, - getNodes: (filter) => { + getNodes: (filter, includeSelf) => { const nodeDB = get().nodeDBs.get(id); if (!nodeDB) { throw new Error(`No nodeDB found (id: ${id})`); } - const all = Array.from(nodeDB.nodeMap.values()).filter( - (n) => n.num !== nodeDB.myNodeNum, + const all = Array.from(nodeDB.nodeMap.values()).filter((n) => + includeSelf ? true : n.num !== nodeDB.myNodeNum, ); + return filter ? all.filter(filter) : all; }, diff --git a/packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.ts b/packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.ts index 5f9702fa..ee59aa78 100644 --- a/packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.ts +++ b/packages/web/src/core/stores/nodeDBStore/nodeDBStore.test.ts @@ -1,3 +1,8 @@ +/** biome-ignore-all lint/suspicious/noExplicitAny: */ +/** biome-ignore-all lint/style/noNonNullAssertion: */ +import { create } from "@bufbuild/protobuf"; +import { Protobuf } from "@meshtastic/core"; +import { toByteArray } from "base64-js"; import { beforeEach, describe, expect, it, vi } from "vitest"; const idbMem = new Map(); @@ -34,7 +39,13 @@ vi.mock("@core/services/featureFlags", () => { }); function makeNode(num: number, extras: Record = {}) { - return { num, ...extras } as any; + return create(Protobuf.Mesh.NodeInfoSchema, { num, ...extras }); +} +function makeUser(fields: Record) { + return create(Protobuf.Mesh.UserSchema, fields); +} +function makePosition(fields: Record) { + return create(Protobuf.Mesh.PositionSchema, fields); } describe("NodeDB store", () => { @@ -85,6 +96,10 @@ describe("NodeDB store", () => { db.processPacket({ from: 50, time: 2222, snr: 9 } as any); expect(db.getNode(50)?.lastHeard).toBe(2222); expect(db.getNode(50)?.snr).toBe(9); + + db.processPacket({ from: 50, time: 0, snr: 9 } as any); + expect(db.getNode(50)?.lastHeard).toBeCloseTo(Date.now(), -1); // within 10ms + expect(db.getNode(50)?.snr).toBe(9); }); it("addUser and addPosition updates existing or creates new nodes", async () => { @@ -145,24 +160,6 @@ describe("NodeDB store", () => { expect(newDB.getNodeError(200)).toEqual({ node: 200, error: "ERROR" }); }); - it("setNodeNum does not merge when new DB already has nodes; old is removed", async () => { - const { useNodeDBStore } = await freshStore(); - const st = useNodeDBStore.getState(); - - const oldDB = st.addNodeDB(10); - oldDB.setNodeNum(999); - oldDB.addNode(makeNode(200)); // old has data - - const newDB = st.addNodeDB(11); - newDB.addNode(makeNode(300)); // new has data -> no merge - newDB.setNodeNum(999); - - expect(st.getNodeDB(10)).toBeUndefined(); // old removed - // new kept its own nodes; did not copy old's - expect(newDB.getNode(300)).toBeTruthy(); - expect(newDB.getNode(200)).toBeUndefined(); - }); - it("partialize persists only data, and onRehydrateStorage rebuilds methods", async () => { { const { useNodeDBStore } = await freshStore(); @@ -259,3 +256,199 @@ describe("NodeDB store", () => { expect(() => db.addNode(makeNode(1))).toThrow(/No nodeDB found/); }); }); + +describe("NodeDB – merge semantics, PKI checks & extras", () => { + const keyOld = toByteArray("40g5tLC6A+tXE92EyhwVwdiKsXwa1QUjZjkzEi0pCy4="); + const keyNew = toByteArray("osxYoEP43oDeWZyjyKx1wz/5cvwEOthHB6AhO2fXEQg="); + + it("upserts node", async () => { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + + const oldDB = st.addNodeDB(10); + oldDB.setNodeNum(999); + oldDB.addNode(makeNode(200, { position: { altitude: 100 } })); + + const newDB = st.addNodeDB(11); + newDB.addNode(makeNode(300)); + newDB.addNode(makeNode(200, { position: { altitude: 120 } })); + newDB.setNodeNum(999); + + expect(st.getNodeDB(10)).toBeUndefined(); // old db removed + expect(newDB.getNode(300)).toBeTruthy(); // node kept + const n200 = newDB.getNode(200)!; + expect(n200.position?.altitude).toBe(120); // replace existing + }); + + it("key conflict: keep old node, flag error", async () => { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + + const oldDB = st.addNodeDB(20); + oldDB.setNodeNum(42); + oldDB.addNode( + makeNode(7, { + user: makeUser({ publicKey: keyOld, longName: "old-7" }), + position: makePosition({ latitudeI: 11, longitudeI: 22 }), + }), + ); + const newDB = st.addNodeDB(21); + newDB.addNode( + makeNode(7, { + user: makeUser({ publicKey: keyNew, longName: "new-7" }), + position: makePosition({ latitudeI: 33 }), + }), + ); + newDB.setNodeNum(42); + + const n7 = newDB.getNode(7)!; + + // node from old + expect(n7.user?.longName).toBe("old-7"); + expect(n7.user?.publicKey).toEqual(keyOld); + expect(n7.position?.latitudeI).toBe(11); + expect(n7.position?.longitudeI).toBe(22); + + // error flagged + const err = newDB.getNodeError(7); + expect(err).toBeTruthy(); + expect(String(err!.error)).toMatch(/MISMATCH|PK/i); + }); + + it("empty new key; drop new node", async () => { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + + const oldDB = st.addNodeDB(30); + oldDB.setNodeNum(77); + oldDB.addNode( + makeNode(5, { user: { publicKey: keyOld, longName: "old-5" } }), + ); + + const newDB = st.addNodeDB(31); + newDB.addNode( + makeNode(5, { user: { publicKey: new Uint8Array(), longName: "new-5" } }), + ); + + newDB.setNodeNum(77); + + // node from old + const n5 = newDB.getNode(5)!; + expect(n5.user?.publicKey).toEqual(keyOld); // keep old PK + expect(n5.user?.longName).toBe("old-5"); + + // error flagged + const err = newDB!.getNodeError(5); + expect(String(err!.error)).toMatch(/MISMATCH|PK/i); + }); + + it("old key empty, new key present, store new node", async () => { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + + const oldDB = st.addNodeDB(40); + oldDB.setNodeNum(1001); + oldDB.addNode(makeNode(8, { user: { longName: "old-8" } })); // no key + + const newDB = st.addNodeDB(41); + newDB.addNode( + makeNode(8, { + user: { publicKey: keyNew, longName: "new-8" }, + position: { altitude: 555 }, + }), + ); + + newDB.setNodeNum(1001); + + // node from new + const n8 = newDB.getNode(8)!; + expect(n8.user?.longName).toBe("new-8"); + expect(n8.user?.publicKey).toEqual(keyNew); + expect(n8.position?.altitude).toBe(555); + + // no error + const err = newDB.getNodeError(8); + expect(err).toBeFalsy(); + }); + + it("unions nodeErrors: preserves old and new, respects existing-on-conflict", async () => { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + + const oldDB = st.addNodeDB(50); + oldDB.setNodeNum(2020); + oldDB.addNode(makeNode(1, { user: { longName: "old-1" } })); + oldDB.setNodeError(1, "OLD_ERR" as any); + + const newDB = st.addNodeDB(51); + newDB.addNode(makeNode(1, { user: { longName: "new-1" } })); + newDB.addNode(makeNode(2, { user: { longName: "new-2" } })); + newDB.setNodeError(2, "NEW_ERR" as any); + + // also set overlapping error + newDB.setNodeError(1, "SHOULD_NOT_OVERWRITE" as any); + + newDB.setNodeNum(2020); + + expect(newDB.getNodeError(1)!.error).toBe("OLD_ERR"); // old kept + expect(newDB.getNodeError(2)!.error).toBe("NEW_ERR"); // new added + }); + + it("eviction still honors cap after merge", async () => { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + + for (let i = 1; i <= 10; i++) { + st.addNodeDB(i); + } + const oldDB = st.addNodeDB(100); + oldDB.setNodeNum(12345); + oldDB.addNode(makeNode(2000)); + + const newDB = st.addNodeDB(101); + newDB.setNodeNum(12345); // merges + deletes 100 + + // adding another to trigger eviction of earliest non-merged entry (which was 1) + st.addNodeDB(102); + + expect(st.getNodeDB(1)).toBeUndefined(); // evicted + expect(st.getNodeDB(101)).toBeDefined(); // merged entry exists + expect(st.getNodeDB(101)!.getNode(2000)).toBeTruthy(); // carried over + }); + + it("removeAllNodes (optionally keeping my node) and removeAllNodeErrors persist across reload", async () => { + { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + const db = st.addNodeDB(1000); + db.setNodeNum(55); + db.addNode(makeNode(55, { user: { longName: "me" } })); + db.addNode(makeNode(56)); + db.setNodeError(56, "ERR" as any); + db.removeAllNodes(true); + db.removeAllNodeErrors(); + } + { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + const db = st.getNodeDB(1000)!; + expect(db.getNode(55)).toBeTruthy(); // kept me + expect(db.getNode(56)).toBeUndefined(); // cleared others + expect(db.getNodeError(56)).toBeUndefined(); // cleared errors + } + }); + + it("getMyNode works after merge establishes myNodeNum", async () => { + const { useNodeDBStore } = await freshStore(); + const st = useNodeDBStore.getState(); + + const oldDB = st.addNodeDB(1100); + oldDB.setNodeNum(4242); + oldDB.addNode(makeNode(4242)); + + const newDB = st.addNodeDB(1101); + newDB.setNodeNum(4242); + + expect(newDB.getMyNode().num).toBe(4242); + }); +}); diff --git a/packages/web/src/core/stores/nodeDBStore/nodeValidation.ts b/packages/web/src/core/stores/nodeDBStore/nodeValidation.ts new file mode 100644 index 00000000..1e6d1f66 --- /dev/null +++ b/packages/web/src/core/stores/nodeDBStore/nodeValidation.ts @@ -0,0 +1,60 @@ +import type { NodeErrorType } from "@core/stores"; +import type { Protobuf } from "@meshtastic/core"; + +// Validates a new incoming node against existing nodes. +// If valid, returns a node to store, else returns undefined. +export function validateIncomingNode( + newNode: Protobuf.Mesh.NodeInfo, + setNodeError: (nodeNum: number, error: NodeErrorType) => void, + getNodes: ( + filter?: (node: Protobuf.Mesh.NodeInfo) => boolean, + ) => Protobuf.Mesh.NodeInfo[], +): Protobuf.Mesh.NodeInfo | undefined { + const num = newNode.num; + const existingNodes = getNodes((node) => node.num === num); + + if (existingNodes.length === 0) { + // No existing node with this node number. + // Check if the new node's public key (if present and not empty) + // is already claimed by another existing node. + if (newNode.user?.publicKey !== undefined) { + const nodesWithSameKey = getNodes( + (node) => node.user?.publicKey === newNode.user?.publicKey, + ); + if (nodesWithSameKey.length > 0) { + // This is a potential impersonation attempt. + setNodeError(num, "DUPLICATE_PKI"); + return undefined; // drop newNode entirely + } + } + return newNode; // No conflicts, accept newNode + } else if (existingNodes.length === 1) { + // One existing node with this node number. + const oldNode = existingNodes[0]; + if (!oldNode) { + return undefined; + } + + // A public key is considered matching if the incoming key equals + // the existing key, OR if the existing key is empty. + const isKeyMatchingOrExistingEmpty = + oldNode.user?.publicKey === newNode.user?.publicKey || + oldNode.user?.publicKey === undefined || + oldNode.user?.publicKey.length === 0; + + if (isKeyMatchingOrExistingEmpty) { + // Keys match or existing key was empty: trust the incoming node data completely. + // This allows for legitimate updates to user info and other fields. + return newNode; + } else { + // Keys do not match and existing key was not empty: potential impersonation attempt. + setNodeError(num, "MISMATCH_PKI"); + return oldNode; // drop newNode fields and return old + } + } else { + // Multiple existing nodes with the same node number + // This should never happen, but if it does, we drop the new node entirely. + setNodeError(num, "DUPLICATE_PKI"); + return undefined; // drop newNode entirely + } +} diff --git a/packages/web/src/core/stores/nodeDBStore/types.ts b/packages/web/src/core/stores/nodeDBStore/types.ts index 105ddbf0..82523ec2 100644 --- a/packages/web/src/core/stores/nodeDBStore/types.ts +++ b/packages/web/src/core/stores/nodeDBStore/types.ts @@ -1,6 +1,9 @@ import type { Protobuf } from "@meshtastic/core"; -type NodeErrorType = Protobuf.Mesh.Routing_Error | "MISMATCH_PKI"; +type NodeErrorType = + | Protobuf.Mesh.Routing_Error + | "MISMATCH_PKI" + | "DUPLICATE_PKI"; type NodeError = { node: number; diff --git a/packages/web/src/core/stores/utils/mergeNodeInfo.ts b/packages/web/src/core/stores/utils/mergeNodeInfo.ts deleted file mode 100644 index 43d8af94..00000000 --- a/packages/web/src/core/stores/utils/mergeNodeInfo.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { create } from "@bufbuild/protobuf"; -import type { NodeErrorType } from "@core/stores"; -import { isEmptyKey, isSameKey } from "@core/utils/sameKey"; -import { Protobuf } from "@meshtastic/core"; - -function assignDefined( - destination: T, - source: Partial, - exclude?: readonly K[], -): T { - const ex = new Set(exclude ?? []); - for (const key of Object.keys(source) as Array) { - if (ex.has(key)) { - continue; - } - const val = source[key]; - if (val !== undefined) { - destination[key] = val as T[typeof key]; - } - } - return destination; -} - -export function mergeNodeInfo( - oldNode: Protobuf.Mesh.NodeInfo, - newNode: Protobuf.Mesh.NodeInfo, - setNodeError: (nodeNum: number, error: NodeErrorType) => void, -): Protobuf.Mesh.NodeInfo { - const merged = create(Protobuf.Mesh.NodeInfoSchema, oldNode); - - if (newNode.user !== undefined) { - const oldKey = oldNode.user?.publicKey; - const newKey = newNode.user.publicKey; - const ok = - isEmptyKey(newKey) || - (isEmptyKey(oldKey) && isEmptyKey(newKey)) || - (!isEmptyKey(oldKey) && !isEmptyKey(newKey) && isSameKey(oldKey, newKey)); - - if (!ok) { - setNodeError(merged.num, "MISMATCH_PKI"); - return merged; // drop all untrusted fields - } - - const baseUser = oldNode.user - ? create(Protobuf.Mesh.UserSchema, oldNode.user) - : create(Protobuf.Mesh.UserSchema); - merged.user = assignDefined(baseUser, newNode.user); - } - - if (newNode.position !== undefined) { - const basePosition = oldNode.position - ? create(Protobuf.Mesh.PositionSchema, oldNode.position) - : create(Protobuf.Mesh.PositionSchema); - merged.position = assignDefined(basePosition, newNode.position); - } - - if (newNode.deviceMetrics !== undefined) { - const baseMetrics = oldNode.deviceMetrics - ? create(Protobuf.Telemetry.DeviceMetricsSchema, oldNode.deviceMetrics) - : create(Protobuf.Telemetry.DeviceMetricsSchema); - merged.deviceMetrics = assignDefined(baseMetrics, newNode.deviceMetrics); - } - - assignDefined(merged, newNode, [ - "user", - "position", - "deviceMetrics", - ] as const); - - return merged; -} diff --git a/packages/web/src/core/utils/sameKey.ts b/packages/web/src/core/utils/sameKey.ts deleted file mode 100644 index 2dbc67fb..00000000 --- a/packages/web/src/core/utils/sameKey.ts +++ /dev/null @@ -1,20 +0,0 @@ -export function isEmptyKey(key?: Uint8Array): boolean { - return !key || key.length === 0; -} - -export function isSameKey(a?: Uint8Array, b?: Uint8Array): boolean { - if (isEmptyKey(a) && isEmptyKey(b)) { - return true; - } - if (!a || !b) { - return false; - } - - for (let i = 0; i < a.length; i++) { - if (a[i] !== b[i]) { - return false; - } - } - - return true; -}