Skip to content

Initialize persistent stores regardless of enable_transport - #75

Closed
wet-bulb wants to merge 1 commit into
attermann:masterfrom
wet-bulb:fix/transport-store-init
Closed

wet-bulb wants to merge 1 commit into
attermann:masterfrom
wet-bulb:fix/transport-store-init

Conversation

@wet-bulb

@wet-bulb wet-bulb commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Fixes announce handling when transport is disabled

Transport::start() only initialises the path table, known-destinations store, and packet hashlist inside if (Reticulum::transport_enabled()). So if you run an instance with transport disabled, those stores never get set up, and every inbound announce fails:

[ERR] remember: failed to store identity for <hash>
[ERR] Failed to add destination <hash> to path table!

No path ever gets added. The instance announces fine, logs nothing else, and just can't address a peer.

Easy to reproduce: with transport disabled, all four test_interop scenarios (packet, link, request, resource) fail the same way. (Worth noting the test_interop senders all set transport_enabled(false) themselves.) With the stores initialised, they pass bidirectionally against Python RNS 1.4.2 and 1.1.9. It's not version-specific either, 1.2.9 reproduces it too.

The fix: every instance needs an identity cache, a path table, and a packet hashlist to address anything, so those three initialisations move out of the gate. The tunnel table read and the probe destination stay inside it, since those really are transport-only.

Nothing changes for transport-enabled instances. The same three initialisations run in the same relative order, just before the if instead of inside it.

Tested on a native host build (pio -e native17, Ubuntu, Python RNS from pip). Haven't run it on hardware yet.

Opened at the request in #74. The diff looks bigger than it is, most of it is re-indentation from dropping a nesting level. With whitespace hidden (?w=1) it's about fourteen lines.

Transport::start() initialises the path table, known-destinations and packet
hashlist stores only inside if (Reticulum::transport_enabled()). An instance
configured with transport disabled therefore starts with those stores
uninitialised, and Identity::remember() fails for every inbound announce:

    [ERR] remember: failed to store identity for <hash>
    [ERR] Failed to add destination <hash> to path table!

so no path is ever added. The instance announces, reports no errors beyond
these two, and cannot address a peer.

Reproduction: with transport disabled, all four test_interop scenarios
(packet, link, request, resource) fail identically. With the stores
initialised they pass bidirectionally against Python RNS 1.4.2 and 1.1.9. Not
version-specific; also reproduces against 1.2.9. Note that all four
test_interop senders themselves set transport_enabled(false).

Every instance needs an identity cache, a path table and a packet hashlist to
address anything at all, so these three initialisations move out of the gate.
The tunnel table read and the probe destination stay inside it.
@attermann

attermann commented Aug 2, 2026 via email

Copy link
Copy Markdown
Owner

@wet-bulb

wet-bulb commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

three places, all on current master:

known destinations: Reticulum.py:344 calls RNS.Identity.load_known_destinations() in Reticulum.init, two lines before RNS.Transport.start(self). not gated on anything.

path table: Transport.py:115, path_table = {} as a class attribute, so it always exists. only the disk restore is gated, at :306, under the same # Load transport-related data comment this code has.

packet hashlist: same shape. :107 packet_hashlist = set() unconditional, disk restore gated at :243.

so Python's split is that the structures always exist and only restoring their contents from storage is transport gated.

that split doesn't map cleanly onto microStore. FileStore::init() attaches the store and makes persisted records available in one step, and its third parameter is clearOnInit rather than anything that would let you attach without reading. the store is the structure, so there's no equivalent of "empty dict now, fill it later".

so this PR matches Python exactly for the first: known destinations. and for the other two it always restores where Python restores only when transport is on. that difference seemed acceptable to me on a constrained radio node (transport disabled scenario), where reloading known paths after a reboot saves announce and path request traffic, but it is a difference and you may not want it.

if you'd rather the port mirror the reference precisely, that needs a microStore change to separate attach from load. happy to do it that way instead.

@drupol

drupol commented Aug 2, 2026

Copy link
Copy Markdown

It would be nice to link to the source on GitHub.

@wet-bulb

wet-bulb commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

good point

permalinks, pinned to b48b96e:

known destinations: Reticulum.py:344
path table: Transport.py:115, disk restore at :306
packet hashlist: :107, disk restore at :243

@attermann

Copy link
Copy Markdown
Owner

Thanks for the PR, and for tracing the Python reference so carefully. That comparison is useful.

Making these stores always persisted, including when transport is disabled, isn't an ideal trade-off for this library, so I don't want to take this approach as written.

Today the library can already be built with -DRNS_PERSIST_PATHS=0. That makes the path table heap-backed instead of flash-backed. The known-destinations store and the packet hashlist have their own flags, -DRNS_PERSIST_KNOWN_DESTINATIONS=0 and -DRNS_PERSIST_HASHLIST=0. A node that never runs transport mode can build that way and avoid persisting these stores.

Choosing at runtime is a different matter. The store types are fixed at compile time (PathStore, KnownStore and BytesStore resolve to either BasicFileStore or BasicHeapStore). Supporting nodes that pick transport mode at runtime would need a new dynamic container in microStore that can be backed by either FLASH or HEAP. That is a larger change than this PR, and it belongs in microStore.

@attermann attermann closed this Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants