[kwsantiago]

Summary

Documents the stack requirement the mbedTLS CTR-DRBG introduced on ESP targets, and ignores the build byproducts the QEMU smoke test generates.

The hazard

#63 moved the ESP RNG from esp_fill_random(), which has no state, onto a cached CTR-DRBG so a failed draw can be reported instead of silently returning zeros. Seeding costs stack in whichever task calls first: ctr_drbg_reseed_internal() puts a 384-byte MBEDTLS_CTR_DRBG_MAX_SEED_INPUT buffer on the caller's stack, plus the entropy gather.

Verified against the ESP-IDF sources rather than quoted from memory: mbedtls/library/ctr_drbg.c:452 declares the buffer, and ctr_drbg.h confirms MAX_SEED_INPUT 384, MAX_REQUEST 1024, RESEED_INTERVAL 10000.

Measured, not estimated

Fixed cost, read from a built ESP32-S3 image with nm:

symbol	bytes
rng_entropy	420
rng_ctr_drbg	76
rng_initialized	4
total .bss	500

My original estimate on the issue was ~450 B, so this corrects it upward.

Stack behaviour, run under QEMU:

first caller	result
2 KB task drawing from an already-seeded DRBG	passes
2 KB task that seeds the DRBG itself	Guru Meditation Error: Core 1 panic’ed (LoadProhibited)
8 KB app_main seeding, then 2 KB tasks drawing	passes

That middle row is the point, and it corrected my first experiment. I initially just shrank the smoke test's task stack to 2 KB and it passed, which looked like the hazard was not real. It passed because app_main had already seeded with 8 KB of stack, so the task never paid the seed cost. Re-running with a 2 KB task as the genuine first caller crashes. The hazard is specifically the first caller, which is why the README recommends seeding deliberately from a task whose depth you control.

.gitignore

Executing action: all (aliases: build)
Running cmake in directory /home/kyle/Documents/GitHub/libnostr-c/build
Executing "cmake -G Ninja -DPYTHON_DEPS_CHECKED=1 -DPYTHON=/home/kyle/.espressif/python_env/idf5.4_py3.12_env/bin/python -DESP_PLATFORM=1 -DCCACHE_ENABLE=0 /home/kyle/Documents/GitHub/libnostr-c"... and the QEMU step generate sdkconfig, dependencies.lock and managed_components/ in test/esp-idf. None were ignored, so they were one git add -A away from being committed. sdkconfig in particular is per-developer. sdkconfig.defaults stays tracked, since that is what CI and a fresh checkout use.

Test plan

• Figures verified against ESP-IDF's own mbedTLS sources
• .bss measured from a built ELF, not estimated
• Stack claims reproduced under QEMU, including the crash
• Smoke test restored and passing after the experiments: SMOKE-PASS
• Generated files now ignored; sdkconfig.defaults still tracked
• CI

Closes libnostr-c-j31.

Summary by CodeRabbit

• Documentation

  • Added ESP-IDF platform guidance covering stack requirements for random number generation, key generation, and signing.
  • Documented cached cryptographic random generator behavior, initialization, memory usage, measured QEMU results, and CI task configuration.

• Chores

  • Updated ignore rules to exclude generated ESP-IDF configuration, dependency lock, and managed component files.

[coderabbitai]

Warning

Review limit reached

@kwsantiago, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d0613ab-1a69-4988-8a02-3e51553b12ba

📥 Commits

Reviewing files that changed from the base of the PR and between 9d6b317 and a42085f.

📒 Files selected for processing (1)

• README.md

Walkthrough

The PR documents ESP-IDF CTR-DRBG stack requirements, initialization behavior, memory usage, measurements, and CI configuration. It also ignores ESP-IDF-generated configuration, lock, and managed component files.

Changes

ESP-IDF support

Layer / File(s)	Summary
ESP-IDF documentation and generated-file exclusions README.md, .gitignore	The README documents CTR-DRBG stack requirements and ESP32-S3 QEMU measurements. .gitignore excludes ESP-IDF-generated files under test/esp-idf.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• privkeyio/libnostr-c#63: Moves ESP-IDF RNG operations to CTR-DRBG, whose stack usage this PR documents.
• privkeyio/libnostr-c#67: Adds ESP-IDF smoke tests for the documented RNG and keypair operations.

Poem

A rabbit checks the stack with care,
While CTR-DRBG seeds the air.
Generated files now hide,
QEMU measurements stand beside.
Eight kilobytes keep tasks aware.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: documenting the ESP stack cost of initial DRBG seeding.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch document-esp-rng-cost

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 49-50: Update the README guidance to replace nostr_key_generate()
with the exported nostr_keypair_generate() API name, while preserving the
surrounding stack-size and signing-entry-point guidance.
- Line 41: Update the ESP-IDF row in the compatibility table to remove the
completed “✅ Tested” status, describe the coverage as an ESP32-S3 QEMU smoke
test, and explicitly indicate that CI is still pending.
- Around line 50-56: Update the README guidance around the documented caller
stack requirement so it does not present 4 KB as guaranteed safe: either add a
QEMU test covering first-call seeding from a 4 KB task, or revise the affected
requirements to mandate the measured 8 KB stack size used by CI.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f2c9e2d7-7ebf-4519-840c-f0df124e0522

📥 Commits

Reviewing files that changed from the base of the PR and between 3a61e60 and 9d6b317.

📒 Files selected for processing (2)

• .gitignore
• README.md

[kwsantiago]

All three looked at. Two were valid, one had a premise that has since gone stale.

Stack guidance (line 50-56), valid and the important one. I wrote "at least 4 KB" having tested only 2 KB (crashes) and 8 KB (passes). 4 KB was an asserted number, not a measured one, which is exactly the habit the rest of this PR was written to avoid. Measured it:

first caller	result
2 KB seeding	crashes
3 KB seeding	passes
4 KB seeding	passes
8 KB seeding	passes

The threshold is between 2 KB and 3 KB on this build. The README now says that, and says 4 KB is recommended for margin above the seed rather than because 3 KB was seen to fail.

API name (line 50), valid but not for the stated reason. The comment says nostr_key_generate() "does not compile". It does: both nostr_key_generate (nostr.h:299) and nostr_keypair_generate (nostr.h:313) are exported. nostr_keypair_generate delegates to nostr_key_generate at key.c, and only the latter contains the RNG draws, so the paragraph naming it was defensible. Still worth fixing, since a reader could reasonably call either: both are now named, with the delegation noted.

Platform table (line 41), premise now stale. The suggestion is to say CI is pending. It is not: #67 merged, and build-esp-idf on main reports success with the QEMU step running two SMOKE-PASS assertions. Verified on run 30778286937 rather than assumed. Leaving the row as is.

Smoke test restored and passing after the experiments: 11 assertions, SMOKE-PASS.