Reject invalid parameters instead of silently dropping them#182
Reject invalid parameters instead of silently dropping them#182benthecarman wants to merge 2 commits intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
| }, | ||
| Commands::OnchainSend { address, amount, send_all, fee_rate_sat_per_vb } => { | ||
| if let Some(rate) = fee_rate_sat_per_vb { | ||
| // Mirrors FeeRate::from_sat_per_vb: sat/vb * 1000/4 = sat/kwu |
There was a problem hiding this comment.
If we get into that territory, let's just take a dependency on bitcoin and use bitcoin::FeeRate?
There was a problem hiding this comment.
Just removed the cli side validation. Too hit this would have to enter something greater than u64::MAX / 250 which is super unlikely a user enters anyways and don't want to add the whole rust-bitcoin dep tree to the cli just for that.
| Commands::Bolt12Receive { description, amount, expiry_secs, quantity } => { | ||
| if quantity.is_some() && amount.is_none() { | ||
| handle_error_msg( | ||
| "quantity can only be set for fixed-amount offers (amount must be provided)", |
There was a problem hiding this comment.
Debatable, @TheBlueMatt previously wanted to drop quantity from the LDK Node API (lightningdevkit/ldk-node#642, but was closed as abandoned). Matt, do you still think we should?
Previously, an invalid fee_rate_sat_per_vb (overflows) was silently collapsed into None via .and_then(), causing the node to substitute its default fee estimation without informing the user. Now the server returns an InvalidRequestError if the fee rate overflows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously, setting quantity without amount_msat silently produced a variable-amount offer with no quantity constraint. Now both the server and CLI reject this misconfiguration with a clear error message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
benthecarman
force-pushed
the
cli-silent
branch
from
d86f651 to
d9b4876
Compare
3 participants
Previously,
onchain_sendsilently fell back to the node's default fee estimation when given an invalidfee_rate_sat_per_vb(e.g., overflows), andbolt12_receivesilently dropped thequantityparameter whenamount_msatwas not set. Both cases now return an error instead.Matching CLI-side validation added for both cases.