Repository navigation
Keep the pairing record store's writes and new records off the heap - #252
Merged
Merged
Conversation
The record store held each record's psk_id and server_id as heap strings, grew its record and dirty-slot vectors on the protocol task, and built a vector of string-keyed, vector-backed blobs for every flush, including the order-blob flush after each playback handoff to another paired server. Records are now held internally as PairingRecord with inline ids (src/pairing_record.h); the public SendspinPairingRecord and persistence codec are unchanged, sharing one layout helper with the internal encoder. The flush takes the owed slots as a bitset under the lock and encodes each write into one stack buffer saved outside it, and the store reserves max_records_ at construction. sendspin_alloc_tests gains windows for a handoff flush and a server/unpair flush (0) and a pairing (4, the pairing notes and the listener's strings).
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The zeroization suite lacks a PairingRecord destructor test, leaving erasure of its stored and pending long-term keys unverified.
3 open issues
What changed in this PR
Reduces heap allocation in pairing-record storage and persistence, supporting the library’s embedded memory constraints without changing the public API or persistence format.
Changes:
- Introduces inline-ID pairing records and reserves record storage to the configured capacity.
- Replaces heap-backed flush batches with a bitset and reusable stack buffer, keeping persistence calls outside the mutex.
- Adds allocation and capacity regression coverage and updates memory and stack documentation.
| File | Description |
|---|---|
| tools/stack_usage/README.md | Updates measured protocol-task stack bounds. |
| tests/test_record_store.cpp | Adds capacity and storage-stability checks; adapts record handling. |
| tests/test_pairing_state_machine.cpp | Adapts pending-record setup to PairingRecord. |
| tests/test_noise_transport.cpp | Converts handshake fixture records to the internal type. |
| tests/test_noise_rehandshake.cpp | Converts re-handshake fixture records to the internal type. |
| tests/test_encrypted_lifecycle.cpp | Adapts eviction and unpair scenarios. |
| tests/test_allocations.cpp | Adds handoff, unpair, and pairing allocation windows. |
| tests/loopback_connection.h | Adapts loopback record setup. |
| tests/CMakeLists.txt | Clarifies the white-box storage-address check rationale. |
| src/record_store.h | Defines bounded dirty-slot tracking and reusable write-buffer helpers. |
| src/record_store.cpp | Reserves storage, retires before insertion, and encodes writes individually. |
| src/persistence_codec.cpp | Shares the encoding layout between public and internal records. |
| src/pairing_record.h | Adds inline-ID records with destruction-time key wiping. |
| src/crypto/keys.h | Declares inline results for fallible PSK-ID derivation. |
| src/crypto/keys.cpp | Encodes fallible PSK-ID results without a heap-backed string. |
| src/connection.h | Stores pending pairings using the internal record type. |
| src/connection_manager_pairing.cpp | Passes inline records into pending pairing state. |
| src/client_dispatch.cpp | Passes finalized records through the revised store interface. |
| docs/internals.md | Updates the allocation model for records and flushes. |
| docs/integration-guide.md | Documents reserved RAM per configured record. |
| CLAUDE.md | Documents the expanded allocation-test windows. |
Review effort: Max
kahrendt
enabled auto-merge (squash)
November 5, 2026 13:53
kahrendt
disabled auto-merge
November 5, 2026 13:58
kahrendt
added a commit
that referenced
this pull request
Nov 5, 2026
## Summary After #252 the pairing exchange itself still allocated on the protocol task: CPace built a `std::vector` for every length-value piece and transcript and copied the sid and AD on each round, the PRS and the emitted code (and the 43-character QR token) were heap-backed, the AEAD that wraps the new long-term PSK allocated, and the pairing-note slot handed its capacity to the main loop at every drain. A digits-code pairing made 68 library allocations, a QR-code pairing 71. Part of Sendspin/backlog#92. No public API change. The listener's `std::string` callback arguments on the main loop remain, as the public callbacks require. ## Change - CPace (`src/crypto/cpace.*`) takes spans, holds the sid and AD in fixed arrays (size-checked, failing closed), and hashes the generator string, the ISK transcript and the MAC input piece by piece into SHA-512 instead of concatenating; `hmac_sha512_streamed()` feeds the HMAC's inner hash and wipes the key-derived buffers. `build_pake_sid()` returns a fixed array; the ADs are views of the constants. - The PRS is an inline 24-byte buffer wiped with the pairing session. `pairing_code_digits()` and the QR/pairing tokens return `InlineText` from one bit-stream base32 formatter; the public `format_pairing_token()` converts at its boundary and its output is unchanged. - `aead_oneshot_encrypt()` writes into the caller's buffer and wipes it on every failure; `wrap_value()` reads the sid from the CPace run. - The note slot and the main-loop drain each hold a vector reserved for one attempt's notes (8) and trade them through `InboxSlot::exchange()`, so a drain no longer takes the slot's capacity away. A drain reserves only when it takes notes, so a nested drain inside a listener's `stop()` allocates only when it delivers that stop's dismissals (listed in internals.md "Memory"). ## Tests - New `Allocations.CodePairingThroughItsRecordFlush` plays the server side with the library's CPace for both the digits and qr_code flows: 68 -> 2 and 71 -> 3 (the listener's server-id strings and the QR token). The `pairing_psk` window drops from 4 to 2. - All draft-irtf-cfrg-cpace-21 test vectors and the existing KATs pass. Mutants caught: a vector back in the MAC path or the note slot (window counts), a dropped length prefix in the ISK hash or the MAC input (vectors/KATs), the zero-pad length and the base32 tail (generator and token KATs). `StartRejectsAnOversizedSidOrAd` covers the size checks. - `ClientLifecycle.StopFromInsideAPairingCallbackDismissesThePromptOnce`: `stop()`, and `stop()` then `start()`, from `on_display_pairing_code()` deliver each owed dismissal once, abandon the rest of the batch, and drain a fresh batch after the restart (mutants: no batch abandonment, no dismissals in `stop()`). `InboxSlot.ExchangeAndEditKeepTheSlotsStorage` pins `exchange()` and `edit()` keeping the buffers' storage (mutant: `exchange()` moving instead of swapping). - Gap: nothing asserts that both note buffers keep their reserve after a re-entrant drain. `EventState` is private to `client.cpp`, so checking it would need a production seam; the allocation windows cover the normal drain. Stack (ESP-IDF 5.5.4, esp32), re-measured on the final tree: the protocol task is unchanged at 6,544 B -Os / 6,688 B -Og bounded and 5,920 / 6,144 reachable, under the 7,168 B default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
The pairing record store allocated on the heap after
start(): each record held itspsk_idandserver_idas 43-characterstd::strings, the record and dirty-slot vectors grew on the protocol task, and every flush built a vector of string-keyed, vector-backed blobs. That last one also runs after every playback handoff to another paired server, which rewrites the order blob. Part of Sendspin/backlog#92; the pairing exchange itself (CPace, the PRS, pairing notes) follows in a separate PR.No public API change:
SendspinPairingRecordand the persistence codec keep their signatures.Change
PairingRecord(new privatesrc/pairing_record.h) withIdTextids and a PSK wiped on destruction; the store,PairingOutcomeand the connection's pending record use it. Its encoder,encode_pairing_record_into(), shares one layout helper with the publicencode_pairing_record(), so the two cannot diverge. The falliblepsk_id_for()returnsstd::optional<IdText>.records_is reserved tomax_records_at construction (about 123 B of internal RAM per record, noted in the integration guide), and a re-pair at capacity retires the old record before inserting, so the store never holds cap + 1.Tests
sendspin_alloc_testsgains a playback-handoff flush window and aserver/unpairflush window, both 0 (were 2 and 4), and apairing_pskpairing window at 4 (was 11; what remains is the pairing note list and the listener'sstd::stringarguments).ChangesAfterConstructionKeepTheReservedRecordspins thatrecords_never reallocates; the cap table's ceiling row checks the 255-byte order blob.std::vectororder blob (fails both flush windows), dropping the reserve, inserting before the retire at re-pair, and a move-assigned reorder at load.Stack (ESP-IDF 5.5.4, esp32): the protocol task measures 6,544 B at -Os and 6,688 B at -Og bounded, 5,920 / 6,144 reachable, under the unchanged 7,168 B default. The main-loop flush frame is 384 B at -Os, on the consumer's own task.