From 688c4e143e03d54742b9bfa1fe8a9e1f9d2117b0 Mon Sep 17 00:00:00 2001 From: Clive Blackledge Date: Tue, 11 Aug 2026 23:48:39 -0700 Subject: [PATCH 1/3] fix(pki): re-derive NodeNum when setting a region mints the identity key A node's mesh address is derived from its identity key: my_node_num == crc32Buffer(config.security.public_key.bytes, 32) NodeDB::createNewIdentity() is what establishes that, and NodeDB:: generateCryptoKeyPair() is the only thing that called it. CryptoEngine::ensurePkiKeys() generates or re-derives the keypair and writes security.public_key, security.private_key and user.public_key - but never re-derives my_node_num. Boot-time keygen is suppressed while the LoRa region is UNSET (generateCryptoKeyPair()'s regionBlocksKeygen guard), so on a fresh device my_node_num is still the MAC-derived value from pickNewNodeNum(). The user then sets the region - the stock onboarding flow - ensurePkiKeys() mints a key, and the invariant is broken. The node then signs its broadcasts (Router.cpp signs when !pki_encrypted && (owner.is_licensed || isBroadcast(p->to))). Every receiver runs verifyFirstContactNodeInfo, fails crc32Buffer(user.public_key) != p->from, and drops the NodeInfo. The node's identity beacons are invisible to the mesh. Nothing reboots to repair it: AdminModule sets requiresReboot = false for LoRa changes ("All LoRa radio changes apply live via configChanged observer") and MenuHandler ends at service->reloadConfig(changes). Four call sites reached ensurePkiKeys(): 1. AdminModule set_config LORA, region first set (phone app - the common path) 2. MenuHandler applyLoraRegion (on-device region picker) 3. InkHUD MenuApplet applyLoRaRegion (schedules a reboot, so it self-healed at next boot) 4. portduino wasm wasm_set_region The reference implementation was already in the tree: the *licensed* branch of call site 1, thirteen lines below the broken unlicensed one, calls nodeDB->generateCryptoKeyPair() (which reaches createNewIdentity()) and widens the persisted mask with SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE. Rather than repeat that at four call sites, the key-mint is routed through one chokepoint that owns both halves of the identity: NodeDB::ensurePkiIdentity() calls crypto->ensurePkiKeys() and then createNewIdentity(). It lives in NodeDB because createNewIdentity() operates on the devicestate/node-DB globals, which CryptoEngine deliberately does not touch - ensurePkiKeys() takes the security config and user by reference precisely so it stays free of that dependency, and it is unit-tested against a standalone CryptoEngine. ensurePkiIdentity() returns true only when my_node_num actually moved (createNewIdentity() early-returns when the key is unchanged, so a repeat region change does not disturb the self entry or force a needless flash write). Callers use that to widen their save mask; my_node_num lives in devicestate and the self row moves in the node DB, so both segments must be persisted or the fix would revert at the next boot. SEGMENT_CONFIG, which carries the key itself, is already unconditional on all four paths. The InkHUD reboot is left as-is. It is now redundant for this invariant, but it covers the rest of that menu's behaviour and a redundant reboot is not a bug. Adds test_handleSetConfig_persistsUnlicensedFirstRegionIdentity, the unlicensed twin of the existing licensed test, asserting both the segment mask and my_node_num == crc32(public_key). Co-Authored-By: Claude Opus 5 --- src/graphics/draw/MenuHandler.cpp | 6 ++-- .../InkHUD/Applets/System/Menu/MenuApplet.cpp | 7 +++-- src/mesh/NodeDB.cpp | 18 +++++++++++ src/mesh/NodeDB.h | 9 ++++++ src/modules/AdminModule.cpp | 6 ++-- .../portduino/wasm/portduino_glue_wasm.cpp | 14 +++++---- test/test_admin_radio/test_main.cpp | 30 +++++++++++++++++++ 7 files changed, 78 insertions(+), 12 deletions(-) diff --git a/src/graphics/draw/MenuHandler.cpp b/src/graphics/draw/MenuHandler.cpp index f5cd21e1a52..f3f97a7a0d2 100644 --- a/src/graphics/draw/MenuHandler.cpp +++ b/src/graphics/draw/MenuHandler.cpp @@ -239,8 +239,10 @@ static void applyLoraRegion(meshtastic_Config_LoRaConfig_RegionCode region, bool } auto changes = SEGMENT_CONFIG; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (crypto) { - crypto->ensurePkiKeys(config.security, owner); + // Minting the key moves our node num with it (my_node_num == crc32(public_key)), so persist + // devicestate + the node DB too - nothing reboots on this path to repair it later. + if (nodeDB->ensurePkiIdentity()) { + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; } #endif initRegion(); diff --git a/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp b/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp index 5e8a08e7511..0b3ec985082 100644 --- a/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp +++ b/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp @@ -324,8 +324,11 @@ static void applyLoRaRegion(meshtastic_Config_LoRaConfig_RegionCode region) auto changes = SEGMENT_CONFIG; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (crypto) { - crypto->ensurePkiKeys(config.security, owner); + // Minting the key moves our node num with it (my_node_num == crc32(public_key)), so persist + // devicestate + the node DB too. The reboot below would re-derive it anyway, but only after the + // save - leaving the invariant to be repaired rather than simply never broken. + if (nodeDB->ensurePkiIdentity()) { + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; } #endif diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index e6c1ac67e66..d7bc98c20ad 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -4349,6 +4349,24 @@ bool NodeDB::createNewIdentity() return true; } +bool NodeDB::ensurePkiIdentity() +{ +#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) + // ensurePkiKeys() declines licensed operation (that path goes through generateCryptoKeyPair()), and a + // failed keygen leaves the existing key - and therefore the existing node num - untouched. + if (!crypto || !crypto->ensurePkiKeys(config.security, owner)) + return false; + + // ensurePkiKeys() writes key material only. Boot-time keygen is suppressed until a LoRa region is set, + // so at first region set my_node_num is still the MAC-derived value from pickNewNodeNum(). Re-derive it + // here, or the node signs broadcasts that every receiver drops (crc32(user.public_key) != from). + // createNewIdentity() early-returns when the key, and so the node num, did not actually change. + return createNewIdentity(); +#else + return false; +#endif +} + bool NodeDB::backupPreferences(meshtastic_AdminMessage_BackupLocation location) { bool success = false; diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index 0d45d1e1e4b..2fa65ac3390 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -572,6 +572,15 @@ class NodeDB bool createNewIdentity(); + /// Mint (or re-derive) the identity keypair *outside* the boot path - i.e. when the user sets a LoRa + /// region, which is what unblocks keygen - and re-seat the identity that key defines. A node's mesh + /// address is its key (my_node_num == crc32Buffer(config.security.public_key)), so + /// CryptoEngine::ensurePkiKeys() must never be called on its own: it writes key material only. Call + /// this instead, so the invariant can't be left broken. + /// @return true if my_node_num moved. The caller must then persist SEGMENT_DEVICESTATE | + /// SEGMENT_NODEDATABASE alongside the SEGMENT_CONFIG that holds the new key. + bool ensurePkiIdentity(); + bool backupPreferences(meshtastic_AdminMessage_BackupLocation location); bool restorePreferences(meshtastic_AdminMessage_BackupLocation location, int restoreWhat = SEGMENT_CONFIG | SEGMENT_MODULECONFIG | SEGMENT_DEVICESTATE | SEGMENT_CHANNELS); diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 80bb7990366..55b029f031b 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1034,8 +1034,10 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers) // If we're setting region for the first time, init the region and regenerate the keys if (isRegionUnset && validatedLora.region > meshtastic_Config_LoRaConfig_RegionCode_UNSET) { #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (crypto && !owner.is_licensed) { - crypto->ensurePkiKeys(config.security, owner); + // Minting the key moves our node num with it (my_node_num == crc32(public_key)), so + // persist devicestate + the node DB too - exactly as the licensed branch below does. + if (!owner.is_licensed && nodeDB->ensurePkiIdentity()) { + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; } #endif // new region is valid and we're coming from an unset region, so enable tx diff --git a/src/platform/portduino/wasm/portduino_glue_wasm.cpp b/src/platform/portduino/wasm/portduino_glue_wasm.cpp index a4b4a31e9d4..6e8be5939e5 100644 --- a/src/platform/portduino/wasm/portduino_glue_wasm.cpp +++ b/src/platform/portduino/wasm/portduino_glue_wasm.cpp @@ -12,10 +12,9 @@ // - exec() short-circuits to "" (no popen/shell in the browser). // Downstream is unchanged: Ch341Hal -> libpinedio_webusb.c -> WebUSB. -#include "CryptoEngine.h" // crypto->ensurePkiKeys() #include "MeshRadio.h" // initRegion() #include "MeshService.h" // service->reloadConfig() -#include "NodeDB.h" // config, owner globals + SEGMENT_CONFIG +#include "NodeDB.h" // config globals, SEGMENT_*, nodeDB->ensurePkiIdentity() #include "PhoneAPI.h" // the transport-agnostic client API seam #include "PortduinoFS.h" // portduinoVFS #include "PortduinoGlue.h" // declares `portduino_config` + Ch341Hal @@ -260,11 +259,14 @@ extern "C" EMSCRIPTEN_KEEPALIVE int wasm_set_region(int region) if (!(RadioInterface::validateConfigRegion(validated) && RadioInterface::validateConfigLora(validated))) return -1; + int changes = SEGMENT_CONFIG; bool wasUnset = (config.lora.region == meshtastic_Config_LoRaConfig_RegionCode_UNSET); if (wasUnset && newRegion > meshtastic_Config_LoRaConfig_RegionCode_UNSET) { #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - if (crypto) - crypto->ensurePkiKeys(config.security, owner); // first real region -> generate keys + // First real region -> generate keys. That moves our node num with them + // (my_node_num == crc32(public_key)), so persist devicestate + the node DB too. + if (nodeDB->ensurePkiIdentity()) + changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; #endif validated.tx_enabled = true; } @@ -274,8 +276,8 @@ extern "C" EMSCRIPTEN_KEEPALIVE int wasm_set_region(int region) config.lora = validated; initRegion(); // repoint myRegion at the new region table if (service) - service->reloadConfig(SEGMENT_CONFIG); // reconfigure radio (new freq) + persist - wasm_fs_sync(); // browser: flush config.proto to IndexedDB + service->reloadConfig(changes); // reconfigure radio (new freq) + persist + wasm_fs_sync(); // browser: flush config.proto to IndexedDB return 0; } diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index f5237b05552..edb4883d7fe 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -23,6 +23,7 @@ #include "mesh/Channels.h" #include "modules/AdminModule.h" #include "modules/NodeInfoModule.h" +#include // crc32Buffer(), for the my_node_num == crc32(public_key) invariant #include #include #include @@ -1116,6 +1117,34 @@ static void test_handleSetConfig_persistsLicensedFirstRegionIdentity() TEST_ASSERT_EQUAL(32, owner.public_key.size); } +// The unlicensed twin of the test above: setting a region for the first time is what unblocks keygen, and +// the minted key *is* the node's mesh address. If the node num is not re-derived with it, the node signs +// broadcasts that every receiver drops (verifyFirstContactNodeInfo: crc32(user.public_key) != from), and if +// the widened segment mask is not persisted the repair is lost at the next boot. +static void test_handleSetConfig_persistsUnlicensedFirstRegionIdentity() +{ + owner = meshtastic_User_init_zero; + owner.is_licensed = false; + config.security = meshtastic_Config_SecurityConfig_init_zero; + config.lora = meshtastic_Config_LoRaConfig_init_zero; + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_UNSET; + initRegion(); + + testAdmin->deferSaves(); + const meshtastic_Config c = + makeLoraSetConfig(meshtastic_Config_LoRaConfig_RegionCode_US, true, meshtastic_Config_LoRaConfig_ModemPreset_LONG_FAST); + testAdmin->handleSetConfig(c, false); + + const int expectedSegments = SEGMENT_CONFIG | SEGMENT_MODULECONFIG | SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; + TEST_ASSERT_EQUAL_INT(expectedSegments, testAdmin->savedSegments()); + TEST_ASSERT_EQUAL(32, config.security.private_key.size); + TEST_ASSERT_EQUAL(32, config.security.public_key.size); + TEST_ASSERT_EQUAL(32, owner.public_key.size); + // The invariant: a node's mesh address is derived from its identity key. + TEST_ASSERT_EQUAL_UINT32(crc32Buffer(config.security.public_key.bytes, config.security.public_key.size), + nodeDB->getNodeNum()); +} + static void test_handleSetConfig_fromOthers_invalidPresetRejected() { // Set up a known-good baseline in the global config @@ -1886,6 +1915,7 @@ void setup() // getRegion() RUN_TEST(test_handleSetOwner_persistsLicensedChannelSanitation); RUN_TEST(test_handleSetConfig_persistsLicensedFirstRegionIdentity); + RUN_TEST(test_handleSetConfig_persistsUnlicensedFirstRegionIdentity); RUN_TEST(test_bootDefense_sanitizesStaleLicensedChannelsOnce); RUN_TEST(test_restorePreferences_sanitizesLicensedBackupBeforeReturn); RUN_TEST(test_getRegion_returnsCorrectRegion_US); From f9e16440f787d452d053898d9bb3a6563d7875c0 Mon Sep 17 00:00:00 2001 From: Clive Blackledge Date: Tue, 18 Aug 2026 21:09:07 -0700 Subject: [PATCH 2/3] style(NodeDB): trim identity-recovery comments and guard the WASM nodeDB deref Two review asks, no behaviour change on any built target. Copilot flagged the unguarded nodeDB deref in the WASM region setter; it is the only ensurePkiIdentity() call site that did not check the pointer first. The rest is comment length. AGENTS.md:83 caps code comments at two lines, and the identity-recovery comments across the four call sites plus the NodeDB.h doc block ran to four and six lines. The rationale they carried is in the commit messages and the PR body, which is where AGENTS.md says it belongs. The PR's own fix in AdminModule.cpp is deliberately untouched. --- src/graphics/draw/MenuHandler.cpp | 3 +-- .../niche/InkHUD/Applets/System/Menu/MenuApplet.cpp | 4 +--- src/mesh/NodeDB.cpp | 7 ++----- src/mesh/NodeDB.h | 9 ++------- src/platform/portduino/wasm/portduino_glue_wasm.cpp | 5 ++--- test/test_admin_radio/test_main.cpp | 6 ++---- 6 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/graphics/draw/MenuHandler.cpp b/src/graphics/draw/MenuHandler.cpp index f3f97a7a0d2..6d49ec55167 100644 --- a/src/graphics/draw/MenuHandler.cpp +++ b/src/graphics/draw/MenuHandler.cpp @@ -239,8 +239,7 @@ static void applyLoraRegion(meshtastic_Config_LoRaConfig_RegionCode region, bool } auto changes = SEGMENT_CONFIG; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - // Minting the key moves our node num with it (my_node_num == crc32(public_key)), so persist - // devicestate + the node DB too - nothing reboots on this path to repair it later. + // Minting the key moves our node num with it, and nothing reboots on this path to repair it later. if (nodeDB->ensurePkiIdentity()) { changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; } diff --git a/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp b/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp index 0b3ec985082..863c1e85d4c 100644 --- a/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp +++ b/src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp @@ -324,9 +324,7 @@ static void applyLoRaRegion(meshtastic_Config_LoRaConfig_RegionCode region) auto changes = SEGMENT_CONFIG; #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - // Minting the key moves our node num with it (my_node_num == crc32(public_key)), so persist - // devicestate + the node DB too. The reboot below would re-derive it anyway, but only after the - // save - leaving the invariant to be repaired rather than simply never broken. + // Minting the key moves our node num with it, and the reboot below only re-derives after the save. if (nodeDB->ensurePkiIdentity()) { changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; } diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index d7bc98c20ad..85a639b2975 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -4352,14 +4352,11 @@ bool NodeDB::createNewIdentity() bool NodeDB::ensurePkiIdentity() { #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - // ensurePkiKeys() declines licensed operation (that path goes through generateCryptoKeyPair()), and a - // failed keygen leaves the existing key - and therefore the existing node num - untouched. + // A failed or declined keygen leaves the existing key, and so the existing node num, untouched. if (!crypto || !crypto->ensurePkiKeys(config.security, owner)) return false; - // ensurePkiKeys() writes key material only. Boot-time keygen is suppressed until a LoRa region is set, - // so at first region set my_node_num is still the MAC-derived value from pickNewNodeNum(). Re-derive it - // here, or the node signs broadcasts that every receiver drops (crc32(user.public_key) != from). + // ensurePkiKeys() writes key material only, so my_node_num is still the stale MAC-derived value. // createNewIdentity() early-returns when the key, and so the node num, did not actually change. return createNewIdentity(); #else diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index 2fa65ac3390..8762d04003c 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -572,13 +572,8 @@ class NodeDB bool createNewIdentity(); - /// Mint (or re-derive) the identity keypair *outside* the boot path - i.e. when the user sets a LoRa - /// region, which is what unblocks keygen - and re-seat the identity that key defines. A node's mesh - /// address is its key (my_node_num == crc32Buffer(config.security.public_key)), so - /// CryptoEngine::ensurePkiKeys() must never be called on its own: it writes key material only. Call - /// this instead, so the invariant can't be left broken. - /// @return true if my_node_num moved. The caller must then persist SEGMENT_DEVICESTATE | - /// SEGMENT_NODEDATABASE alongside the SEGMENT_CONFIG that holds the new key. + /// Mint the identity keypair outside the boot path and re-seat my_node_num == crc32(public_key). + /// @return true if my_node_num moved; the caller must then also persist SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE. bool ensurePkiIdentity(); bool backupPreferences(meshtastic_AdminMessage_BackupLocation location); diff --git a/src/platform/portduino/wasm/portduino_glue_wasm.cpp b/src/platform/portduino/wasm/portduino_glue_wasm.cpp index 6e8be5939e5..65d6320849e 100644 --- a/src/platform/portduino/wasm/portduino_glue_wasm.cpp +++ b/src/platform/portduino/wasm/portduino_glue_wasm.cpp @@ -263,9 +263,8 @@ extern "C" EMSCRIPTEN_KEEPALIVE int wasm_set_region(int region) bool wasUnset = (config.lora.region == meshtastic_Config_LoRaConfig_RegionCode_UNSET); if (wasUnset && newRegion > meshtastic_Config_LoRaConfig_RegionCode_UNSET) { #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI) - // First real region -> generate keys. That moves our node num with them - // (my_node_num == crc32(public_key)), so persist devicestate + the node DB too. - if (nodeDB->ensurePkiIdentity()) + // Minting the key moves our node num with it, so persist devicestate + the node DB too. + if (nodeDB && nodeDB->ensurePkiIdentity()) changes |= SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE; #endif validated.tx_enabled = true; diff --git a/test/test_admin_radio/test_main.cpp b/test/test_admin_radio/test_main.cpp index edb4883d7fe..5abbcd0b282 100644 --- a/test/test_admin_radio/test_main.cpp +++ b/test/test_admin_radio/test_main.cpp @@ -1117,10 +1117,8 @@ static void test_handleSetConfig_persistsLicensedFirstRegionIdentity() TEST_ASSERT_EQUAL(32, owner.public_key.size); } -// The unlicensed twin of the test above: setting a region for the first time is what unblocks keygen, and -// the minted key *is* the node's mesh address. If the node num is not re-derived with it, the node signs -// broadcasts that every receiver drops (verifyFirstContactNodeInfo: crc32(user.public_key) != from), and if -// the widened segment mask is not persisted the repair is lost at the next boot. +// Unlicensed twin of the test above. Without the re-derivation the node signs broadcasts every receiver +// drops (verifyFirstContactNodeInfo: crc32(user.public_key) != from). static void test_handleSetConfig_persistsUnlicensedFirstRegionIdentity() { owner = meshtastic_User_init_zero; From 9d7ec3963b937a70f31f032cc8824349e21951ed Mon Sep 17 00:00:00 2001 From: Clive Blackledge Date: Tue, 18 Aug 2026 22:00:51 -0700 Subject: [PATCH 3/3] fix(NodeDB): keep the identity move authoritative when the self record cannot be created createNewIdentity() removes the old node entry and assigns myNodeInfo.my_node_num before it tries to create the row for the new number. If getOrCreateMeshNode() came back null it returned false, so the first-region callers left SEGMENT_DEVICESTATE and SEGMENT_NODEDATABASE out of the save mask. The number had already moved in RAM at that point, and the freshly minted key goes to flash under SEGMENT_CONFIG regardless. The next boot therefore reloads the old number alongside the new key, which is exactly the crc32(public_key) != my_node_num break this path exists to prevent, reached through the error branch instead of the happy one. Rolling the number back is not an option either, since the key has already been replaced by the time this runs. So the move is now reported as the fact it is and the missing self record is logged separately; getOrCreateMeshNode() will recreate that row on the next contact. Reachable when the self record is absent and the table is full of protected nodes. Reported by CodeRabbit on #11426. --- src/mesh/NodeDB.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 85a639b2975..c30ee575dac 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -4341,10 +4341,13 @@ bool NodeDB::createNewIdentity() myNodeInfo.my_node_num = newNodeNum; + // The number has moved, so the caller must persist it whatever happens next. Returning false here + // would leave the new key saved against the old number, which is the break this exists to prevent. meshtastic_NodeInfoLite *info = getOrCreateMeshNode(getNodeNum()); - if (!info) - return false; - TypeConversions::CopyUserToNodeInfoLite(info, owner); + if (info) + TypeConversions::CopyUserToNodeInfoLite(info, owner); + else + LOG_ERROR("No room for our own node 0x%08x, identity moved without a self record", newNodeNum); return true; }