Skip to content

Conversation

@compumike
Copy link
Contributor

Clients are now sending an add_contact AdminMessage before basically every text message DM (because clients may hold a larger node database with public keys than the radio holds).

This has two unintended side effects:

  1. The destination node's last_heard is getting incorrectly updated to the current time even when we just send them a DM.
  2. For CLIENT_BASE, the auto-favoriting in NodeDB::addFromContact may be causing unintentional routing issues. (Discussion) For CLIENT_BASE we should only do explicit favoriting.

This PR makes the following changes:

  • NodeDB::addFromContact
    • don't auto-favorite if we're CLIENT_BASE
    • don't update last_heard (unless we're CLIENT_BASE -- see comment in code below)
    • if should_ignore then also clear the is_favorite flag
  • AdminModule::handleReceivedProtobuf
    • don't auto-favorite if we're CLIENT_BASE

Client Possible Issues

I have tested this, and think this should be generally safe, but there are two tiny issues that clients may want to take a look at:

  1. Will any clients break if they see a NodeInfo with last_heard == 0? This could happen now if they do an add_contact for a node that's been evicted.
  2. Clients may assume that whenever they send add_contact that the node will become favorited, and may update their local database accordingly. (This appears to be true in the iOS client, at least -- taking a look now.)

Future follow-ups

  • a few small PRs in clients:
    • make sure clients don't do their own auto-favoriting while in CLIENT_BASE
    • make it so favoriting while in CLIENT_BASE is more explicit, possibly a Yes/No confirm dialog.
    • make it so that when switching from any other role to CLIENT_BASE, we give some sort of message about removing any favorited nodes that are not in their control

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3

Copy link
Member

@GUVWAF GUVWAF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me from a firmware point-of-view, but indeed might need some additional checks in the clients.

@compumike
Copy link
Contributor Author

Looks good to me from a firmware point-of-view, but indeed might need some additional checks in the clients.

@GUVWAF Excellent, thanks. I just created a feature request issue here: "[Feature Request]: CLIENT_BASE: make it harder to 'accidentally' favorite other nodes" #8514 to track the broader topic, including revisions to client apps!

@fifieldt fifieldt added the enhancement New feature or request label Nov 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants