fix(pow): surface explicit PoW rejection instead of generic timeout#173
Conversation
When mostrod requires NIP-13 PoW and the client sends an event without
satisfying it, the daemon silently drops the event (logs "Not POW
verified event!" on its side) and never replies. The CLI was hiding
that behind a generic timeout message ("deadline has elapsed" /
"Timeout waiting for DM or gift wrap event"), which can mislead users
into suspecting network failures, relay issues, or daemon downtime.
After timing out, wait_for_dm now consults the daemon's kind-38385
info event ("pow" tag) and, if the required difficulty exceeds the
configured POW, returns the new PowRequirementUnmet error with an
actionable message pointing at --pow / POW=. Genuine timeouts keep
their existing WaitForDmTimeout error, so the add-bond-invoice
happy-path branch (which only matches WaitForDmTimeout) is unaffected.
Closes #172
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 32 minutes and 38 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR implements detection and user-facing reporting of proof-of-work rejection errors in ChangesPoW Rejection Error Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86804192e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/util/messaging.rs`:
- Around line 344-361: The timeout branch currently awaits
super::events::fetch_required_pow(ctx) which can itself block up to
FETCH_EVENTS_TIMEOUT and double real timeout latency; change this so you do a
non-blocking/short-timeout probe instead: after Err(_elapsed) call
fetch_required_pow with a very short tokio::time::timeout (or poll it without
awaiting) and only if it returns Some(required) within that probe window compare
required against parse_pow_env() and potentially return PowRequirementUnmet; if
the probe times out or errors, immediately return WaitForDmTimeout without
waiting the full fetch timeout.
🪄 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
Run ID: e2198022-0dc2-4934-8e30-216d195aaf22
📒 Files selected for processing (4)
docs/pow_error_handling.mdsrc/util/events.rssrc/util/messaging.rssrc/util/mod.rs
fetch_required_pow uses FETCH_EVENTS_TIMEOUT (15s) internally, so a slow/unreachable relay would let wait_for_dm wait 15s for the reply plus another 15s for the probe before producing an error. Wrap the probe in a short tokio::time::timeout (3s) and fall through to WaitForDmTimeout if it doesn't return in time — Ok(Some(required)) is the only case that escalates to PowRequirementUnmet. Addresses review feedback on #173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous attempt (commit 72ced79) capped the postflight info-event lookup at 3s, but it still ran sequentially after the 15s DM wait timed out, so the user-visible failure path could take up to 18s when the relay was slow. Run the probe concurrently with the DM wait instead: by the time the wait elapses the probe's answer is typically already in hand, so the timeout branch consumes a resolved JoinHandle with ~0s added latency. POW_PROBE_TIMEOUT is kept as a safety net for pathological relays that outlive the 15s wait. The probe needs a 'static future for tokio::spawn, so the work moves into a new fetch_required_pow_with(client, mostro_pubkey) — the existing fetch_required_pow(ctx) becomes a thin wrapper around it. On the happy path (DM arrives in time) the spawned probe is aborted so we don't leak a stray relay request. Addresses review feedback on #173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good to me. I reviewed the current head commit 2050955e919731722bbf50a0d70dc9b3a9413fa2.
This fixes the UX problem in the right place: wait_for_dm now keeps genuine timeouts distinct from silent PoW rejections, and the new PowRequirementUnmet error gives the user an explicit, actionable message instead of the misleading generic timeout.
I also checked the important add-bond-invoice interaction: it still only treats WaitForDmTimeout as the happy path, so PoW failures will correctly bubble up as errors instead of being misreported as success.
Checks run:
cargo test powcargo clippy --all-targets --all-features -- -D warnings
Both passed.
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
The PoW-rejection flow looks correct overall, and the test suite passes locally.
One non-blocking concern: on the timeout path, the code still waits up to 3s for the
PoW probe to finish before falling back to WaitForDmTimeout. That's much better than
the old sequential 15s+15s shape, but it can still add a small delay when the relay is
slow or the info event is stale. If you want the generic timeout to remain as close as
possible to 15s, consider shortening the probe budget further or serving a cached PoW
hint.
There was a problem hiding this comment.
Strict Review
Verdict: APPROVED
I don't see a blocking issue in this patch. The change addresses the PoW-rejection path
without regressing the existing timeout flow, and the local test suite passes.
Minor note: the timeout path still has a small extra wait for the concurrent PoW probe,
but that's not a blocker for this PR.
1 participant
Summary
mostrodrequires NIP-13 PoW and the CLI sends an event without satisfying it, the daemon silently drops the event (logsNot POW verified event!on its side) and never replies. The CLI surfaced this as a generic timeout (deadline has elapsed/Timeout waiting for DM or gift wrap event), which led users to suspect network/relay/daemon issues instead of the real PoW cause.wait_for_dmnow consults the daemon's kind-38385 info event (powtag) and, if the required difficulty exceeds the configuredPOW, returns the newPowRequirementUnmet { required, configured }error with an actionable message pointing at--pow/POW=.WaitForDmTimeout, so theadd-bond-invoicehappy-path branch (which only matchesWaitForDmTimeout) is unaffected; a PoW failure on that command propagates instead of being misreported as a successful submission.Design notes and alternatives considered are in
docs/pow_error_handling.md.Closes #172
Test plan
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo test(10 unit tests + integration tests, all green)pow_requirement_unmet_display_mentions_required_and_configuredpow_requirement_unmet_is_downcastable_through_anyhowread_info_tag_finds_pow_valueread_info_tag_returns_none_when_missingpow_tag_parses_as_u8mostrod:POW=0 mostro-cli neworder …→PowRequirementUnmeterror (not "deadline has elapsed")POW=<required> mostro-cli neworder …→ normal flow🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation