Configurable nip40 expiration time via settings.toml #513
Conversation
WalkthroughAdds a new Mostro configuration setting Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant P as Publisher
participant N as order_to_tags
participant S as Settings
participant C as Config Store
participant T as Tags
P->>N: order_to_tags(order)
N->>S: Settings::get_mostro()
S->>C: read MostroSettings
C-->>S: { nip40_expiration_days, ... }
S-->>N: MostroSettings
Note over N: Compute expiration = order.expires_at + days(nip40_expiration_days)
N->>T: add ["expiration", computed_seconds]
N-->>P: Tags
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (4)
src/lightning/invoice.rs (1)
286-291: Zero-amount test uses a hard-coded BOLT11 that can expire over timeThe new invoice string is embedded with a fixed timestamp; once expired, this test will start failing intermittently. Also, the other tests use regtest invoices (
lnbcrt...), while this one is mainnet (lnbc...), which is inconsistent.Consider making the test time-agnostic or at least guarding against accidental expiry.
Apply this guard so failures clearly indicate an expired vector and not a logic regression:
async fn test_zero_amount_invoice() { init_settings_test(); - let payment_request = "lnbc1p52v0v9pp5scdu5kh9gkgp63r7k72pugffkut4uqdv98d7vruvanklt700rmrsdq5g9kxy7fqd9h8vmmfvdjscqzzsxqyz5vqsp5tw8kcvr22mlmdffrtfta6xxhhl5cdtnpg02qygvr2yhm3vva9zas9qxpqysgqecamkem2pcz8savcv8j7p3uj6lxlx5dnecc26ax5uk005aqfdszsdxfuchsph6tej8j08eps3np5ukvv6t6nfldkge76qyn3qs0p23gqy2jj0z".to_string(); + let payment_request = "lnbc1p52v0v9pp5scdu5kh9gkgp63r7k72pugffkut4uqdv98d7vruvanklt700rmrsdq5g9kxy7fqd9h8vmmfvdjscqzzsxqyz5vqsp5tw8kcvr22mlmdffrtfta6xxhhl5cdtnpg02qygvr2yhm3vva9zas9qxpqysgqecamkem2pcz8savcv8j7p3uj6lxlx5dnecc26ax5uk005aqfdszsdxfuchsph6tej8j08eps3np5ukvv6t6nfldkge76qyn3qs0p23gqy2jj0z".to_string(); + // Defensive check so failures are self-explanatory if the static vector ever expires. + let inv = decode_invoice(&payment_request).expect("invoice must decode"); + assert!(!inv.is_expired(), "zero-amount test vector expired; update payment_request"); let zero_amount_err = is_valid_invoice(payment_request, Some(100), None); assert_eq!(Ok(()), zero_amount_err.await); }If feasible, prefer generating a fresh zero-amount invoice at runtime (or injecting “now” into validation) to fully eliminate time-coupling. I can draft that if desired.
src/config/mod.rs (1)
78-79: Tests updated for new key; add a regression test for missing-key defaultThe assertions for
nip40_expiration_days = 15look good. To prevent future regressions and to cover migration scenarios where existing configs lack this key, add a test that omits it and validates the deserialized default (ideally 1 day per the compatibility suggestion).You can add the following test:
#[test] fn test_mostro_settings_default_nip40_expiration_days() { // Same MOSTRO_SETTINGS TOML but without `nip40_expiration_days` const MOSTRO_SETTINGS_NO_NIP40: &str = r#"[mostro] fee = 0 max_routing_fee = 0.001 max_order_amount = 1000000 min_payment_amount = 100 expiration_hours = 24 max_expiration_days = 15 expiration_seconds = 900 user_rates_sent_interval_seconds = 3600 publish_relays_interval = 60 pow = 0 publish_mostro_info_interval = 300 bitcoin_price_api_url = "https://api.yadio.io""#; #[derive(Debug, Deserialize)] struct Stub { mostro: MostroSettings } let settings: Stub = toml::from_str(MOSTRO_SETTINGS_NO_NIP40).expect("Failed to deserialize"); // Expect 1 if we preserve prior 24h behavior via serde default. assert_eq!(settings.mostro.nip40_expiration_days, 1); }Also applies to: 165-166
settings.tpl.toml (1)
38-39: Clarify semantics and consider a backward-compatible defaultGood addition. To minimize surprises for operators, set the template’s default to the prior implicit value (e.g., 1) and document precisely how it’s applied.
Apply:
-# Nip40 expiration days -nip40_expiration_days = 15 +# NIP-40 expiration grace in days (added to `order.expires_at` when generating the expiration tag). +# This controls how long the event remains on relays after the order itself expires. +# Set to 0 to disable any extra grace period. +nip40_expiration_days = 1If you want 15 as the new recommended value, keep 15 here but ensure serde default is 1 to preserve legacy behavior for configs missing this key.
src/nip33.rs (1)
246-250: Dynamic NIP-40 expiration computation — confirm desired default and consider saturating addThe computation is correct. Be mindful that with the new field defaulting to 0, events will have no extra grace unless operators set the key. If you aim to preserve the prior 24h behavior by default, couple this with a serde default of 1 day in
MostroSettings.Optionally, make the addition explicit and saturating to guard against extreme configuration values (defense-in-depth; harmless overhead):
- vec![(order.expires_at - + Duration::days(mostro_settings.nip40_expiration_days as i64).num_seconds()) - .to_string()], + vec![ + (order.expires_at.saturating_add( + Duration::days(mostro_settings.nip40_expiration_days as i64).num_seconds(), + )) + .to_string() + ],Also consider exposing
nip40_expiration_daysininfo_to_tagsso clients can introspect runtime policy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
settings.tpl.toml(1 hunks)src/config/mod.rs(2 hunks)src/config/types.rs(1 hunks)src/lightning/invoice.rs(1 hunks)src/nip33.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-17T13:51:17.338Z
Learnt from: arkanoider
PR: MostroP2P/mostro#499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.
Applied to files:
src/lightning/invoice.rs
🧬 Code graph analysis (2)
src/nip33.rs (1)
src/config/settings.rs (1)
get_mostro(58-63)
src/config/types.rs (1)
src/config/settings.rs (1)
Settings(23-34)
🔇 Additional comments (1)
src/nip33.rs (1)
199-201: Fetching Mostro settings here is appropriateUsing
Settings::get_mostro()locally inorder_to_tagskeeps concerns contained. No issues.
@grunch ,
since it comes to zero effort, i will drop this PR and let you decide if it is worth to merge it, as I said in tg chat this allow with new config parameter called
nip40_expiration_daysto define the days before nip40 on relay is triggered so now total life time of the order event is:lifetime = now() + expires_at + nip40_expiration_days
now nip40_expiration_days is basically hardcoded to 24h.
Summary by CodeRabbit
New Features
Tests