-
Notifications
You must be signed in to change notification settings - Fork 0
lnd #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe update integrates native support for LND nodes across multiple layers. New interfaces and classes for LND configuration and operations (such as getting node info, creating invoices, handling offers, and decoding invoices) have been introduced in TypeScript, JavaScript, and Rust. The changes streamline existing CLN node functions and update configuration, dependencies, and documentation. Additionally, bindings for React Native and UniFFI now expose the LND APIs, reinforcing a consistent interface for Lightning node interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application (main)
participant LN as LndNode
participant API as LND API (Rust)
App->>LN: Instantiate LndNode(config)
LN->>API: get_info()
API-->>LN: Return node info
LN-->>App: Deliver node info
App->>LN: create_invoice(params)
LN->>API: create_invoice(params)
API-->>LN: Invoice created
LN-->>App: Deliver invoice
App->>LN: lookup_invoice(payment_hash)
LN->>API: lookup_invoice(payment_hash)
API-->>LN: Return transaction
LN-->>App: Deliver transaction
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (34)
readme.md (2)
269-269: Fix the markdown linting issue with the bare URL.The bare URL for Tor Socks5 fetch doesn't follow proper markdown formatting guidelines.
-- [ ] Tor Socks5 fetch https://tpo.pages.torproject.net/core/arti/guides/starting-arti +- [ ] Tor Socks5 fetch [Arti Guide](https://tpo.pages.torproject.net/core/arti/guides/starting-arti)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
269-269: Bare URL used
null(MD034, no-bare-urls)
271-274: Fix indentation for list items.The list items are indented with 4 spaces, but the markdown linter expects 2 spaces for proper nesting.
- [ ] implement lightning nodes - - [X] phoenixd - - [X] cln - - [X] lnd + - [X] phoenixd + - [X] cln + - [X] lndGreat job completing the LND implementation! This matches perfectly with the code changes in the other files.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
271-271: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
272-272: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
273-273: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
274-274: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
bindings/lni_nodejs/index.d.ts (4)
27-30: Consider secure handling of LND macaroon.
Storing macaroons as plain strings can be risky if they are sensitive credentials. Consider ensuring that these fields are handled securely (e.g., encrypted at rest, masked in logs) to prevent inadvertent leaks.
31-34: Avoid repeating configuration fields in theLndNodeinterface.
SinceLndConfigalready hasurlandmacaroon, including them again inLndNodemay introduce duplication. Consider relying upon an internal config object or using a getter to avoid scattered references.
178-182: Add doc comments for the new optional invoice parameters.
It would be helpful to document whatrPreimage,isBlinded,isKeysend,isAmp, andisPrivatedo for improved clarity and maintainability.
226-239: Unify node classes under a common baseline interface.
PhoenixdNode,ClnNode, andLndNodelargely share similar methods. Consider extracting a shared interface or abstract class to streamline code maintenance.crates/lni/cln/api.rs (1)
350-356: Consider a single parameter struct for paging fields.
payment_hash,from, andlimitare commonly passed around together. You could streamline the code by encapsulating these into a single struct (e.g.,LookupParams) to reduce repetition and improve clarity.bindings/lni_uniffi/src/lni.udl (3)
65-68: Handle the LND macaroon securely.
Similar to the TypeScript interface, take care to handle sensitive macaroons safely (avoid logging them, consider encryption in storage, etc.).
70-97: Consider a shared approach for repeating node interfaces.
LndNodeappears structurally similar toPhoenixdNodeandClnNode. Extracting a common interface or base type might reduce duplication across node implementations.
143-147: Add descriptive comments for new CreateInvoiceParams fields.
r_preimage,is_blinded,is_keysend,is_amp, andis_privatewould benefit from short descriptive statements on their purpose.crates/lni/lnd/types.rs (6)
1-3: Consider derivingSerializeas well
Currently, you only deriveDeserialize. If you plan to send these structures back to clients or log them in JSON, derivingSerializecan be beneficial.
4-8: Use a typed enum for the network if possible
TheChainstruct storeschainandnetworkas plain strings. If the set of valid networks is known (e.g.,mainnet,testnet,regtest), consider using a typed enum to reduce errors and improve clarity.
34-37:FetchInvoiceResponserename consideration
invoicecould be renamed to a more descriptive field, likeencoded_invoiceorbolt11, to distinguish it from other invoice fields in the codebase.
39-49: Checkcreated_attype inPayResponse
created_atis af64, which might be prone to floating-point inaccuracies when handling timestamps. Consider using an integer type (e.g.,i64) or a specialized time library if precision is important.
83-99: Clarify meaning ofcreatedinBolt12Resp
With fields likeactive,single_use, andused, the booleancreatedcould be confusing. Consider a more descriptive name likeis_newly_createdor adding doc comments.
101-149: Ensure proper numeric parsing inListInvoiceResponse
Many fields (e.g.,value,value_msat,creation_date,settle_date, etc.) areOption<String>. Make sure any conversions to numeric types (e.g.,i64) are sufficiently validated. This helps avoid runtime panics from invalid strings.Do you want me to provide a helper or code snippet for safely parsing these fields using a library like
serde_withor custom deserializers?crates/lni/lnd/lib.rs (5)
9-19: Consider doc comments forLndConfig
Adding doc comments to fieldsurlandmacaroonclarifies their intended usage and valid formats. This is especially helpful for new team members or external consumers unfamiliar with LND.
21-27: ValidateLndNode::newparameters
IfLndConfigallows empty or invalidurlandmacaroon, consider validating or returning an error if they don't match expected patterns.
33-38: Review concurrency approach increate_invoice
Using.awaitensures the call is asynchronous. Verify that concurrent invoice creation is safe on the LND side, especially if multiple invoices are created rapidly.
75-85: Paginated results inlist_transactions
fromandlimitare used for pagination. Validate that subsequent or large queries won’t silently omit transactions if LND returns partial data or changes the indexing rules.Can provide a verification script if you want to confirm the offset logic with various test scenarios.
92-316: Improve test coverage
Your test module coversget_info,create_invoice,lookup_invoice,list_transactions, anddecode. Consider also testing error scenarios (e.g., invalid macaroon, unreachable URL) to ensure robust error handling.bindings/lni_nodejs/src/lnd.rs (7)
4-7: Add doc comments to theLndNodestruct
Because this struct is exposed to Node.js developers via N-API, clarifying usage and constraints in doc comments helps consumers use it more effectively.
11-14: Constructor input validation
new(config: LndConfig)trusts thatconfigis valid. If you anticipate malicious or invalid input from external calls, consider validatingconfig.urlandconfig.macaroon.
16-19: Consider customizingget_url
get_urltrivially returns the stored URL. If there's any requirement to sanitize or mask sensitive parts, do it here before exposing it to consumers.
41-55:create_invoiceNode API usage
This function is asynchronous and returns anapi::Result. Confirm that Node.js consumers handle potential rejections appropriately, especially if the system cannot reach the LND or if invoice creation fails.
65-72:list_offersusage
Likeget_offer, this might return partial or unimplemented data from LND if Bolt12 is incomplete. Decide whether you prefer to gate these calls behind a feature flag for clarity.
104-117:list_transactionsfiltering
This function does not acceptpayment_hashhere (unlike the Rust method signature in the crate). If you need payment-hash filtering, ensure the TypeScript definitions match.
119-125: Clarify usage ofdecode
Method name is generic. If it’s intended only for BOLT 11 or BOLT 12 strings, consider naming itdecode_payment_requestfor clarity.crates/lni/lnd/api.rs (6)
30-47: Prevent potential runtime panics by handling errors explicitly.Calls to
send().unwrap()andtext().unwrap()will panic on network or parsing errors. Return an appropriateApiErrorinstead.- let response = client.get(&req_url).send().unwrap(); - let response_text = response.text().unwrap(); + let response = client.get(&req_url).send().map_err(|e| ApiError::Network { + reason: e.to_string(), + })?; + let response_text = response.text().map_err(|e| ApiError::Network { + reason: e.to_string(), + })?;
76-79: Avoid printing debug statements to stdout in production code.Use a structured logger (e.g.,
logcrate) or remove theseprintln!calls to prevent cluttering output logs.- println!("Status: {}", response.status()); - println!("Bolt11 {}", &invoice_str.to_string()); + // log::debug!("Status: {}", response.status()); + // log::debug!("Bolt11 {}", &invoice_str);
109-118: Use an async client or handle blocking appropriately.Similar to
create_invoice(), this function is async but uses blocking reqwest calls. Consider using the async client or removingasync.
120-129: Implement or remove stubbed Bolt12 functions.All these functions currently return an error stating Bolt12 is not implemented. If you plan to support Bolt12, consider outlining a roadmap or removing them for now to avoid confusion.
Do you want help prototyping these Bolt12 methods?
Also applies to: 131-139, 141-151, 153-163, 165-175
177-233: Handle potential parsing or network failures without panicking.Calls such as
send().unwrap(),text().unwrap(), andserde_json::from_str(...).unwrap()can panic. Map these errors to theApiErrorinstead of unwrapping.
235-298: Refactor to safely handle JSON deserialization failures.Parsing invoice fields using
.unwrap_or_default()on parsed integers can mask errors. Consider logging or returning relevant error information to troubleshoot malformed responses or unexpected data.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
bindings/lni_nodejs/index.d.ts(3 hunks)bindings/lni_nodejs/index.js(1 hunks)bindings/lni_nodejs/main.mjs(2 hunks)bindings/lni_nodejs/src/cln.rs(1 hunks)bindings/lni_nodejs/src/lib.rs(1 hunks)bindings/lni_nodejs/src/lnd.rs(1 hunks)bindings/lni_react_native/ubrn.config.yaml(1 hunks)bindings/lni_uniffi/src/lib.rs(1 hunks)bindings/lni_uniffi/src/lni.udl(2 hunks)crates/lni/Cargo.toml(1 hunks)crates/lni/cln/api.rs(2 hunks)crates/lni/cln/lib.rs(2 hunks)crates/lni/lib.rs(1 hunks)crates/lni/lnd/api.rs(1 hunks)crates/lni/lnd/lib.rs(1 hunks)crates/lni/lnd/types.rs(1 hunks)crates/lni/phoenixd/lib.rs(1 hunks)crates/lni/types.rs(3 hunks)readme.md(1 hunks)
🧰 Additional context used
🧬 Code Definitions (8)
bindings/lni_nodejs/index.js (1)
bindings/lni_nodejs/index.d.ts (1) (1)
LndNode(31-34)
crates/lni/phoenixd/lib.rs (1)
crates/lni/types.rs (1) (1)
default(228-242)
bindings/lni_nodejs/src/lib.rs (1)
bindings/lni_nodejs/index.d.ts (1) (1)
LndNode(31-34)
crates/lni/lib.rs (1)
bindings/lni_nodejs/index.d.ts (2) (2)
LndConfig(27-30)LndNode(31-34)
crates/lni/lnd/types.rs (1)
bindings/lni_nodejs/index.d.ts (2) (2)
PayCode(184-191)Bolt11Resp(14-18)
crates/lni/lnd/api.rs (1)
bindings/lni_nodejs/index.d.ts (5) (5)
Bolt11Resp(14-18)NodeInfo(43-50)CreateInvoiceParams(171-183)PayCode(184-191)PayInvoiceResponse(150-154)
crates/lni/cln/lib.rs (3)
crates/lni/cln/api.rs (2) (2)
lookup_invoice(337-348)list_transactions(418-428)crates/lni/phoenixd/api.rs (2) (2)
lookup_invoice(165-193)list_transactions(195-324)bindings/lni_nodejs/index.d.ts (2) (2)
ListTransactionsParams(166-170)Transaction(51-65)
crates/lni/lnd/lib.rs (3)
bindings/lni_nodejs/index.d.ts (5) (5)
NodeInfo(43-50)CreateInvoiceParams(171-183)ListTransactionsParams(166-170)PayCode(184-191)Transaction(51-65)bindings/lni_nodejs/src/lnd.rs (6) (6)
get_info(35-39)create_invoice(42-55)get_offer(58-63)lookup_invoice(94-102)list_transactions(105-117)decode(120-125)crates/lni/types.rs (1) (1)
default(228-242)
🪛 markdownlint-cli2 (0.17.2)
readme.md
269-269: Bare URL used
null
(MD034, no-bare-urls)
271-271: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
272-272: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
273-273: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (30)
bindings/lni_react_native/ubrn.config.yaml (1)
4-4: Branch update aligns with LND integration focusThe change from
clntolndbranch correctly updates the configuration to target the Lightning Network Daemon implementation branch instead of Core Lightning.crates/lni/Cargo.toml (1)
29-30: Appropriate cryptographic dependencies added for LND integrationThe addition of
hexandsha2crates is necessary for handling cryptographic operations commonly used in Lightning Network implementations, such as encoding/decoding hexadecimal data and computing SHA-2 hashes for payment verification.bindings/lni_nodejs/index.js (2)
313-313: LndNode class correctly added to native binding importsThe
LndNodeclass is now properly extracted from the native binding, consistent with the other node types (PhoenixdNode, ClnNode).
318-318: LndNode properly exported from moduleThe
LndNodeclass is now correctly exported, making it available for use by consumers of this module.bindings/lni_uniffi/src/lib.rs (1)
4-4: LND module properly exposed through UniFFIThe addition of
pub use lni::lnd::*;correctly exposes all public items from the LND module, following the same pattern used for other Lightning implementations (phoenixd, cln).bindings/lni_nodejs/src/lib.rs (1)
14-16: Successfully added LND module.The Lightning Network Daemon (LND) node implementation has been correctly added to the library exports, following the same pattern as the existing PhoenixdNode and ClnNode.
crates/lni/phoenixd/lib.rs (1)
150-150: Good practice using Default trait.Using
..Default::default()is a clean way to handle the additional parameters that were added to theCreateInvoiceParamsstruct. This ensures that the test code won't break when new fields are added to the struct in the future.crates/lni/lib.rs (1)
37-42: Correctly implemented LND module structure.The LND module follows the same structure as the existing modules (phoenixd, cln) with appropriate submodules for lib, api, and types. This consistent architecture makes the codebase more maintainable and easier to understand.
bindings/lni_nodejs/main.mjs (2)
1-1: Clean import update to include LndNode.The import has been updated to include LndNode along with the existing node types, maintaining a consistent pattern.
132-132: Main function properly updated to include the LND node.The LND integration is correctly enabled in the main function, ensuring it runs alongside the existing node implementations.
bindings/lni_nodejs/src/cln.rs (1)
107-107:✅ Verification successful
Code simplification in lookup_invoice.
The implementation has been simplified to directly return the transaction result from the API call. This eliminates unnecessary processing and aligns with the pattern seen in the LND implementation.
Let's verify that error handling is still robust by checking if any error cases are still properly handled:
🏁 Script executed:
#!/bin/bash # Check if the error handling has been moved to the API level grep -A 10 "lookup_invoice" crates/lni/cln/api.rs | grep -A 5 "Result<"Length of output: 479
Error handling verification in lookup_invoice is robust
The simplified implementation now directly returns the transaction result from the API call, and error conditions are appropriately handled at the API level. The verification confirms that any errors from
lookup_invoicesare correctly propagated usingErr(e), which safeguards against unhandled error cases.crates/lni/types.rs (3)
32-32: Added Clone trait to Transaction struct.Adding the
Clonetrait to theTransactionstruct allows creating copies of transactions, which is necessary for operations like returning transactions from lookup functions.
220-225: Added LND-specific invoice parameters.The additional fields support LND-specific features:
r_preimage: Optional payment preimage for verificationis_blinded: For blinded payments hiding routing informationis_keysend: For keysend payments (without requiring invoice)is_amp: For Atomic Multi-path Paymentsis_private: For private invoices not broadcasted to the networkThese provide the necessary flexibility for working with LND while maintaining compatibility with other implementations.
236-240: Default values for new invoice parameters.The default implementation correctly initializes the new fields with sensible defaults:
r_preimageisNonesince it's typically generated- Boolean flags are
Some(false)rather thanNone, making the intent explicitcrates/lni/cln/lib.rs (3)
91-96: Updated list_transactions to use correct API.The implementation now correctly calls
list_transactionsAPI with direct parameters rather than wrapping them inSome(). This is consistent with the API definition shown in the relevant code snippets.
213-213: Updated Default usage in tests.The test correctly uses
..Default::default()to leverage the default values for the new invoice parameters, ensuring the tests remain compatible with the updated struct.
78-84:✅ Verification successful
Simplified lookup_invoice implementation.
The lookup_invoice method has been simplified to directly return the API result. This aligns with the pattern seen in the LND implementation and reduces complexity.
Looking at the relevant code snippets, the error handling appears to have been moved to the API layer. Let's verify:
🏁 Script executed:
#!/bin/bash # Look for error handling in the updated lookup_invoice API function grep -A 10 "pub fn lookup_invoice" crates/lni/cln/api.rsLength of output: 403
Simplified lookup_invoice Implementation Verified
- The
lookup_invoicefunction incrates/lni/cln/lib.rs(lines 78-84) now directly returns the result from the API call.- Error handling has been effectively moved into the API layer. As seen in
crates/lni/cln/api.rs, the function properly matches on the result oflookup_invoices, returning an error when necessary.- This approach aligns well with patterns used in the LND implementation and reduces code complexity.
crates/lni/cln/api.rs (1)
424-424:❓ Verification inconclusive
Verify behavior for negative
fromorlimitparameters.
Sincefromandlimitare i64, ensure the service can handle negative values safely or add validation to fail early.Run this script to locate parameter references and confirm appropriate validation or usage patterns:
🏁 Script executed:
#!/bin/bash # Searches for references of 'list_transactions' to confirm negative param handling. rg 'list_transactions'Length of output: 1753
🏁 Script executed:
#!/bin/bash # Searching for the definition of lookup_invoices to inspect its handling of parameters. rg "fn lookup_invoices" -nLength of output: 74
Validation Check for Negative
fromandlimitParameters
Please verify that negative values forfromandlimitare either safely handled withinlookup_invoices(defined incrates/lni/cln/api.rsaround line 351) or trigger an early validation error. Cross-check this behavior against similar endpoints (likelist_transactions) to ensure consistency and avoid unintended behavior with pagination or related queries.crates/lni/lnd/types.rs (2)
10-32: Confirm that all fields inGetInfoResponseexist across LND versions
Some older or customized LND versions may not provide fields such asrequire_htlc_interceptororstore_final_htlc_resolutions. If using varied LND backends, consider making these fields optional to gracefully handle missing fields.
51-55: Validate TXID endianness inPaidOutpoint
LND typically returns TXIDs in big-endian hexadecimal. Confirm that thetxidfield is interpreted correctly, as different APIs can invert endianness.crates/lni/lnd/lib.rs (5)
1-8: Check feature gating
You conditionally compile with#[cfg(feature = "napi_rs")]in some places. Verify whether you need consistent gating for other items (e.g.,use napi_derive::napi). Inconsistent gating can cause build issues if features differ.
29-31: Check potential network errors inget_info
Make sure you handle potential timeouts, connection issues, or 4xx/5xx HTTP responses when calling the LND. Currently, the call is forwarded to the underlying API.
40-46:get_offerandlist_offersdisclaimers
Given that Bolt12 is not fully implemented in many LND versions, confirm whether these calls will always return “not implemented” or partial data.
64-73:lookup_invoiceerror handling
Invoices not found raise an error with"not found". Confirm that the presence of an invoice with partial data or unusual statuses (e.g., “canceled”) is also handled.
87-89: Confirm decode usage
decodeis often used for BOLT 11 payment requests. If used with BOLT 12 or custom strings, confirm that an appropriate error or fallback is provided.bindings/lni_nodejs/src/lnd.rs (4)
1-3: Validate cross-crate references
You importLndConfigfromlni::lnd::lib::LndConfig. Confirm that version mismatches or reorganization won't break these references, particularly when the crates evolve.
34-39: Check concurrency forget_info
While.map_err(...)wraps errors, ensure that concurrency is safe if multiple Node.js threads use this method simultaneously.
57-63: Confirm partial Bolt12 support
get_offerreferencesBolt12, which many LND builds might not fully support. The error path returns “Bolt12 not implemented” in some placeholders. Ensure docs note that limitation.Likely an incorrect or invalid review comment.
93-102:lookup_invoicereturn validation
Ensure that the resultingTransactionobject is consistent with the Node.js type definitions. Check for mismatches in field naming or type conversions.crates/lni/lnd/api.rs (1)
85-99: Check for discrepancies in the Bolt11 response structure.According to the related TypeScript snippet,
Bolt11Respmay differ from the fields (payment_requestandr_hash) used here. Ensure the localBolt11Respstruct is up to date.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
crates/lni/lnd/types.rs (1)
1-223: 🛠️ Refactor suggestionAdd documentation for structs and fields
The file lacks documentation comments for structs and their fields. Adding documentation would improve code readability and maintainability, especially since this is a public API.
Consider adding:
- Doc comments (
///) for each struct explaining its purpose- Doc comments for fields, especially those with special formats or constraints
- Examples where appropriate
Example:
+/// Represents a blockchain with chain name and network information #[derive(Debug, Deserialize)] pub struct Chain { + /// The name of the blockchain (e.g., "bitcoin") pub chain: String, + /// The network type (e.g., "mainnet", "testnet") pub network: String, }
🧹 Nitpick comments (21)
readme.md (4)
269-269: Add proper formatting for the URL.The bare URL should be formatted using Markdown link syntax for better readability and to follow Markdown best practices.
-Tor Socks5 fetch https://tpo.pages.torproject.org/core/arti/guides/starting-arti +Tor Socks5 fetch [https://tpo.pages.torproject.org/core/arti/guides/starting-arti](https://tpo.pages.torproject.org/core/arti/guides/starting-arti)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
269-269: Bare URL used
null(MD034, no-bare-urls)
271-279: Fix unordered list indentation.The markdown linter has flagged inconsistent indentation in the unordered list. The expected indentation is 2 spaces, but 4 spaces are used.
- [ ] implement lightning nodes - - [X] phoenixd - - [X] cln - - [X] lnd - - [ ] lndk - - [ ] ldk_node - - [ ] eclair - - [ ] strike?? (BOLT 12 support, BOLT 11 blinded path support?) - - [ ] nwc?? (AlbyHub - blinded path support?) https://github.com/rust-nostr/nostr/blob/master/crates/nwc/examples/nwc.rs - - [ ] LNURL? + [ ] implement lightning nodes + - [X] phoenixd + - [X] cln + - [X] lnd + - [ ] lndk + - [ ] ldk_node + - [ ] eclair + - [ ] strike?? (BOLT 12 support, BOLT 11 blinded path support?) + - [ ] nwc?? (AlbyHub - blinded path support?) [https://github.com/rust-nostr/nostr/blob/master/crates/nwc/examples/nwc.rs](https://github.com/rust-nostr/nostr/blob/master/crates/nwc/examples/nwc.rs) + - [ ] LNURL?🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
271-271: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
272-272: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
273-273: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
274-274: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
275-275: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
276-276: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
277-277: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
278-278: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
278-278: Bare URL used
null(MD034, no-bare-urls)
279-279: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
278-278: Add proper formatting for the URL.Similar to the previous URL, this bare URL should be formatted using Markdown link syntax.
-nwc?? (AlbyHub - blinded path support?) https://github.com/rust-nostr/nostr/blob/master/crates/nwc/examples/nwc.rs +nwc?? (AlbyHub - blinded path support?) [https://github.com/rust-nostr/nostr/blob/master/crates/nwc/examples/nwc.rs](https://github.com/rust-nostr/nostr/blob/master/crates/nwc/examples/nwc.rs)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
278-278: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
278-278: Bare URL used
null(MD034, no-bare-urls)
279-279: Consider adding a URL for LNURL.For consistency with the other items in the list, consider adding a reference URL for LNURL as well.
-LNURL? +LNURL? [https://github.com/fiatjaf/lnurl-rfc](https://github.com/fiatjaf/lnurl-rfc)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
279-279: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
crates/lni/utils.rs (2)
14-18: Consider removing debug println statements.These debug print statements are useful during development but should typically be replaced with proper logging before merging to production code.
- println!("invoice: {:?}", invoice); - - // Get the amount from the invoice - let invoice_amount_msats = invoice.amount_milli_satoshis().unwrap_or(0); - println!("invoice amount: {:?}", invoice_amount_msats); + // Get the amount from the invoice + let invoice_amount_msats = invoice.amount_milli_satoshis().unwrap_or(0);
30-33: Consider removing debug println statements.Similar to the previous comment, these debug print statements should be replaced with proper logging.
- println!( - "calculated fee_limit_msat {} from percentage, {} for the total amount {}", - fee_msats, fee_percentage, amount_msats - );bindings/lni_nodejs/main.mjs (1)
46-50: Commented out code for payInvoice is a good placeholder for future implementation.The commented code hints at how to implement the payInvoice functionality in the phoenixd function, but isn't actively used yet.
Consider adding a TODO comment explaining when this code will be uncommented and used.
bindings/lni_nodejs/src/lnd.rs (3)
1-3: Consider avoiding wildcard imports if possible.Replacing:
use lni::{lnd::lib::LndConfig, CreateInvoiceParams, PayInvoiceParams};with more selective imports or distinct namespaces can improve clarity and help maintain a structured import hierarchy.
9-14: Constructor security check.When storing the entire
macaroon, you may want to confirm that you actually need persisted plaintext credentials. Consider storing sensitive data more securely or adding usage disclaimers.
128-135: Rename “str” parameter to avoid naming confusion.Using
stras a parameter name may overshadow thestrtype reference in Rust. Consider something likeencoded_strorpayload_str:-pub async fn decode(&self, str: String) -> Result<String> { - ... +pub async fn decode(&self, encoded_str: String) -> Result<String> { + ...bindings/lni_nodejs/index.d.ts (3)
19-25: Validate naming consistency with other pay responses.
PhoenixPayInvoiceRespaligns with existing naming patterns. If consistent with the rest of your codebase, all good. Otherwise, consider a more generic name likePayInvoiceRespfor reuse.
34-37: Consider flagging macaroon as a secret.You might mark
macaroonwith doc comments warning developers it’s sensitive. This helps communicate the potential security considerations when handling this field.
247-261: LndNode class interface.This interface reflects the Rust definitions. Double-check that returning the entire macaroon (via
getMacaroon) is truly intended.crates/lni/lnd/api.rs (2)
232-242: Partial support for decode.Currently, decoding only handles Bolt11. Document or handle Bolt12 decode if you plan to expand. Errors are straightforward, but call out that partial coverage is by design.
358-421: Incoming transaction listing sorts only invoices.
list_transactionsis restricted to invoices. If you require outgoing payments, you may need a separate call or unify them. Confirm it aligns with your design for a complete transaction list.crates/lni/lnd/types.rs (6)
39-49: Use enums for payment statusThe
statusfield inPayResponseis using a string type. Similar to the invoice status, consider using an enum for better type safety and improved code readability.+#[derive(Debug, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum PaymentStatus { + Succeeded, + Failed, + InFlight, + // Add other possible statuses +} + #[derive(Debug, Deserialize)] pub struct PayResponse { pub destination: String, pub payment_hash: String, pub created_at: f64, pub parts: i32, pub amount_msat: i64, pub amount_sent_msat: i64, pub payment_preimage: String, - pub status: String, + pub status: PaymentStatus, }
150-168: Consider using enums and dedicated types for payment responsesThe
LndPayInvoiceResponsestructure contains several fields that could benefit from stronger typing:
statusfield should be an enum instead of a string- Using more semantic types for values, timestamps, and identifiers would improve code clarity
failure_reasonfield could be an enum of possible failure types+#[derive(Debug, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum PaymentStatus { + Succeeded, + Failed, + InFlight, + // Add other possible statuses +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum FailureReason { + None, + Timeout, + NoRoute, + Error, + IncorrectPaymentDetails, + InsufficientBalance, + // Add other failure reasons +} + #[derive(Debug, Deserialize)] pub struct LndPayInvoiceResponse { pub payment_hash: String, pub value: String, pub creation_date: String, pub fee: String, pub payment_preimage: String, pub value_sat: String, pub value_msat: String, pub payment_request: String, - pub status: String, + pub status: PaymentStatus, pub fee_sat: String, pub fee_msat: String, pub creation_time_ns: String, pub htlcs: Option<Vec<Htlc>>, pub payment_index: String, - pub failure_reason: String, + pub failure_reason: FailureReason, pub first_hop_custom_records: Option<serde_json::Value>, }
213-222: Use enums for HTLC statusThe
statusfield in theHtlcstruct is a string. Similar to other status fields, consider using an enum for better type safety.+#[derive(Debug, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum HtlcStatus { + InFlight, + Succeeded, + Failed, + // Add other possible statuses +} + #[derive(Debug, Deserialize)] pub struct Htlc { pub attempt_id: String, - pub status: String, + pub status: HtlcStatus, pub route: Route, pub attempt_time_ns: String, pub resolve_time_ns: String, pub failure: Option<serde_json::Value>, pub preimage: String, }
1-223: Consider using stronger types for numeric values and timestampsMany fields in these structs use primitive types like
String,i64, andf64when more semantic types would be clearer:
- Use specialized types for amounts (satoshis/millisatoshis)
- Use chrono types for timestamps
- Use appropriate types for cryptographic hashes and keys
This would make the code more self-documenting and add type safety.
Example implementation:
use chrono::{DateTime, Utc}; use std::fmt; pub struct SatAmount(pub u64); pub struct MsatAmount(pub u64); // For cryptographic hashes pub struct Hash(pub [u8; 32]); impl fmt::Debug for Hash { /* implementation */ } impl<'de> Deserialize<'de> for Hash { /* implementation */ } // Then in your structs: // pub amount_msat: MsatAmount, // pub creation_date: DateTime<Utc>, // pub payment_hash: Hash,
131-147: Consider using more specific types instead ofserde_json::ValueSeveral fields use
serde_json::Valuefor complex JSON structures. While this works, it leaves the specific structure of these values undocumented and bypasses type checking. If the structure of these fields is known, consider defining proper types.For example:
+#[derive(Debug, Deserialize)] +pub struct RouteHint { + // Fields for route hint structure +} + +#[derive(Debug, Deserialize)] +pub struct Feature { + // Fields for feature structure +} + #[derive(Debug, Deserialize)] pub struct ListInvoiceResponse { // ...existing fields - pub route_hints: Option<serde_json::Value>, + pub route_hints: Option<Vec<RouteHint>>, pub private: Option<bool>, pub add_index: Option<String>, pub settle_index: Option<String>, pub amt_paid: Option<String>, pub amt_paid_sat: Option<String>, pub amt_paid_msat: Option<String>, pub state: Option<String>, - pub htlcs: Option<serde_json::Value>, - pub features: Option<serde_json::Value>, + pub htlcs: Option<Vec<Htlc>>, + pub features: Option<HashMap<String, Feature>>, // ...remaining fields
114-148: Consider grouping related fields in ListInvoiceResponseThe
ListInvoiceResponsestruct has many fields that could be organized into logical groups by creating nested structs. This would improve readability and make the structure more maintainable.For example:
+#[derive(Debug, Deserialize)] +pub struct InvoiceAmount { + pub value: Option<String>, + pub value_msat: Option<String>, + pub amt_paid: Option<String>, + pub amt_paid_sat: Option<String>, + pub amt_paid_msat: Option<String>, +} + +#[derive(Debug, Deserialize)] +pub struct InvoiceDescription { + pub memo: Option<String>, + pub description: Option<String>, + pub description_hash_hex: Option<String>, + pub description_hash_b64: Option<String>, + pub description_hash: Option<String>, +} + +#[derive(Debug, Deserialize)] +pub struct InvoiceTiming { + pub creation_date: Option<String>, + pub settle_date: Option<String>, + pub expiry: Option<String>, + pub cltv_expiry: Option<String>, +} + #[derive(Debug, Deserialize)] pub struct ListInvoiceResponse { - pub memo: Option<String>, pub r_preimage: Option<String>, pub r_hash: Option<String>, - pub value: Option<String>, - pub value_msat: Option<String>, + pub amount: InvoiceAmount, + pub description: InvoiceDescription, + pub timing: InvoiceTiming, pub settled: Option<bool>, - pub creation_date: Option<String>, - pub settle_date: Option<String>, pub payment_request: Option<String>, - pub description: Option<String>, - pub description_hash_hex: Option<String>, - pub description_hash_b64: Option<String>, - pub description_hash: Option<String>, - pub expiry: Option<String>, pub fallback_addr: Option<String>, - pub cltv_expiry: Option<String>, // ... remaining fields
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
bindings/lni_nodejs/index.d.ts(7 hunks)bindings/lni_nodejs/main.mjs(3 hunks)bindings/lni_nodejs/src/cln.rs(3 hunks)bindings/lni_nodejs/src/lib.rs(1 hunks)bindings/lni_nodejs/src/lnd.rs(1 hunks)bindings/lni_nodejs/src/phoenixd.rs(2 hunks)bindings/lni_uniffi/src/lni.udl(5 hunks)crates/lni/Cargo.toml(1 hunks)crates/lni/cln/api.rs(4 hunks)crates/lni/cln/lib.rs(5 hunks)crates/lni/lib.rs(1 hunks)crates/lni/lnd/api.rs(1 hunks)crates/lni/lnd/lib.rs(1 hunks)crates/lni/lnd/types.rs(1 hunks)crates/lni/phoenixd/api.rs(3 hunks)crates/lni/phoenixd/lib.rs(4 hunks)crates/lni/phoenixd/types.rs(1 hunks)crates/lni/types.rs(5 hunks)crates/lni/utils.rs(1 hunks)readme.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- bindings/lni_nodejs/src/lib.rs
- crates/lni/Cargo.toml
- crates/lni/types.rs
- bindings/lni_nodejs/src/cln.rs
- crates/lni/cln/lib.rs
🧰 Additional context used
🧬 Code Definitions (6)
crates/lni/phoenixd/types.rs (1)
bindings/lni_nodejs/index.d.ts (1)
PhoenixPayInvoiceResp(19-25)
crates/lni/lib.rs (1)
bindings/lni_nodejs/index.d.ts (2)
LndConfig(34-37)LndNode(38-41)
crates/lni/cln/api.rs (6)
crates/lni/utils.rs (1)
calculate_fee_msats(6-36)bindings/lni_nodejs/index.d.ts (3)
PayInvoiceParams(199-210)PayInvoiceResponse(157-161)Transaction(58-72)crates/lni/cln/lib.rs (2)
pay_invoice(51-56)lookup_invoice(82-93)crates/lni/lnd/api.rs (4)
pay_invoice(112-230)client(15-31)params(188-191)lookup_invoice(300-356)crates/lni/lnd/lib.rs (2)
pay_invoice(41-46)lookup_invoice(72-77)crates/lni/phoenixd/api.rs (2)
pay_invoice(100-129)lookup_invoice(198-226)
crates/lni/lnd/lib.rs (3)
bindings/lni_nodejs/index.d.ts (8)
NodeInfo(50-57)CreateInvoiceParams(178-190)ListTransactionsParams(173-177)PayCode(191-198)PayInvoiceParams(199-210)PayInvoiceResponse(157-161)Transaction(58-72)LndConfig(34-37)bindings/lni_nodejs/src/lnd.rs (8)
new(12-14)get_info(35-39)create_invoice(42-51)pay_invoice(54-63)get_offer(66-72)lookup_invoice(103-111)list_transactions(114-126)decode(129-134)crates/lni/types.rs (2)
default(228-242)default(273-287)
bindings/lni_nodejs/src/lnd.rs (2)
bindings/lni_nodejs/index.d.ts (8)
LndConfig(34-37)CreateInvoiceParams(178-190)PayInvoiceParams(199-210)NodeInfo(50-57)Transaction(58-72)PayInvoiceResponse(157-161)PayCode(191-198)ListTransactionsParams(173-177)crates/lni/lnd/lib.rs (10)
new(23-28)get_info(30-32)create_invoice(34-39)pay_invoice(41-46)get_offer(48-50)list_offers(52-54)pay_offer(56-70)lookup_invoice(72-77)list_transactions(79-89)decode(91-93)
crates/lni/lnd/api.rs (5)
bindings/lni_nodejs/index.d.ts (6)
Bolt11Resp(14-18)NodeInfo(50-57)CreateInvoiceParams(178-190)PayInvoiceParams(199-210)PayInvoiceResponse(157-161)Transaction(58-72)crates/lni/utils.rs (1)
calculate_fee_msats(6-36)crates/lni/lnd/lib.rs (7)
new(23-28)get_info(30-32)create_invoice(34-39)pay_invoice(41-46)decode(91-93)lookup_invoice(72-77)list_transactions(79-89)crates/lni/phoenixd/api.rs (6)
get_info(16-34)create_invoice(36-98)pay_invoice(100-129)decode(133-135)lookup_invoice(198-226)list_transactions(228-357)crates/lni/lib.rs (1)
from(16-20)
🪛 markdownlint-cli2 (0.17.2)
readme.md
269-269: Bare URL used
null
(MD034, no-bare-urls)
271-271: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
272-272: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
273-273: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
274-274: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
275-275: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
276-276: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
277-277: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
278-278: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
278-278: Bare URL used
null
(MD034, no-bare-urls)
279-279: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (59)
readme.md (1)
273-273: Great job implementing LND support!I see you've completed the LND implementation task which aligns with the PR objectives. The implementation appears to be consistent with the other Lightning node implementations.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
273-273: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
crates/lni/lib.rs (2)
37-42: LGTM - LND module structure is consistent with existing patterns.The new
lndmodule follows the same structure as the existingphoenixdandclnmodules, with appropriate sub-modules and exports. This maintains consistency in the codebase.
47-48: LGTM - New utils module exported.The introduction of a
utilsmodule is a good addition for shared functionality like fee calculations that can be used across different node implementations.crates/lni/phoenixd/types.rs (1)
105-118: LGTM - Response struct is consistently structured with other types.The new
PhoenixPayInvoiceRespstruct follows the same pattern as other response types in the file, with appropriate annotations for serialization/deserialization and field renaming. The field naming is consistent with the existing codebase style.crates/lni/utils.rs (1)
6-36: LGTM - Utility function for fee calculation.The
calculate_fee_msatsfunction provides a useful utility for calculating fees based on invoice amounts and specified percentage rates. This will ensure consistent fee calculation logic across different node implementations.crates/lni/phoenixd/lib.rs (3)
51-56: The newpay_invoicemethod is well-structured and follows existing patterns.The implementation correctly passes the required parameters to the API function and maintains consistency with the other methods in the struct.
160-161: Good use of Default trait.Using
..Default::default()for the remaining fields is a clean approach that makes the code more maintainable as new fields are added to the struct.
4-7: Import changes correctly support the new functionality.The updated imports properly include the new types needed for the pay invoice functionality.
bindings/lni_nodejs/main.mjs (2)
1-1: Import statement correctly updated to include LndNode.The import statement has been properly updated to include the new LndNode class.
98-132: LND integration follows the consistent pattern established by other node types.The implementation of the
lnd()function follows the same structure as the existingphoenixd()andcln()functions, providing a consistent interface for users.However, there is a concern regarding the environment variable used in line 123:
#!/bin/bash # Check if the LND_TEST_PAYMENT_HASH environment variable is defined if grep -q "LND_TEST_PAYMENT_HASH" .env 2>/dev/null; then echo "LND_TEST_PAYMENT_HASH is defined in .env file" else echo "Warning: LND_TEST_PAYMENT_HASH not found in .env file" echo "This will cause lookupInvoice() to fail in the lnd() function" ficrates/lni/phoenixd/api.rs (2)
1-7: Import statements properly updated to include new types.The imports are correctly updated to include
PhoenixPayInvoiceResp,PayInvoiceParams, andPayInvoiceResponse, which are needed for the new functionality.
191-192: Variable rename improves code clarity.Changing
feetofee_msatsmakes the code clearer by explicitly indicating the unit of measurement, which is especially important given the conversion from satoshis to millisatoshis.bindings/lni_nodejs/src/phoenixd.rs (3)
1-1: Import statement correctly updated to include PayInvoiceParams.The import has been properly updated to include the PayInvoiceParams type needed for the new pay_invoice method.
61-71: Well-implemented pay_invoice method.The method follows the established pattern of other methods in the struct, correctly passing the node's URL and password along with the provided parameters to the API function and handling errors appropriately.
74-79: Improved code formatting enhances readability.The reformatting of the get_offer method improves code consistency with other methods in the file.
crates/lni/cln/api.rs (6)
6-9: Imports look good.
418-418: Validate that fee calculation cannot be negative.The expression
pay_resp.amount_sent_msat - pay_resp.amount_msatcould yield a negative fee in edge cases. Please confirm that real-world scenarios never allowamount_sent_msatto be less thanamount_msat.
422-434: Prevent potential panic when no invoices are found.This repeats a prior review concern:
.first().unwrap()will panic if thetransactionsvector is empty. Handle the empty case gracefully:match lookup_invoices(url, rune, payment_hash, from, limit) { Ok(transactions) => { - Ok(transactions.first().unwrap().clone()) + if let Some(t) = transactions.first() { + Ok(t.clone()) + } else { + Err(ApiError::Api { + reason: "No matching invoice found".to_string(), + }) + } } Err(e) => Err(e), }
435-442: No issues found.
503-503: No concerns here.
509-509: Looks good.bindings/lni_uniffi/src/lni.udl (9)
17-19: Newpay_invoicemethod for PhoenixdNode looks fine.
48-50: Newpay_invoicemethod for ClnNode is consistent with the rest of the interface.
51-68: Additional ClnNode methods are coherent and align with existing interface patterns.
69-69: Interface block termination is fine.
71-74: LndConfig dictionary fields look straightforward.
76-87: LndNode interface additions are consistent and no issues.
244-244: Dictionary end bracket is fine.
316-322:PayCodedictionary updates look good.
324-335:PayInvoiceParamsdictionary is well-defined and matches the code usage.crates/lni/lnd/lib.rs (15)
1-3: Conditional N-API imports look fine.
4-9: Imports are correct and consistent with usage.
10-14:LndConfigstruct is straightforward.
16-20:LndNodestruct definitions look good.
22-28: Constructor properly assigns config fields.
30-32:get_infomethod delegates logic appropriately.
34-39:create_invoicemethod usage looks correct.
41-46:pay_invoicemethod aligns with desired usage.
48-50:get_offermethod is consistent with the LND API.
52-54:list_offersmethod is acceptable.
56-70:pay_offerimplementation delegates to the API correctly.
72-77:lookup_invoiceis straightforward.
79-89:list_transactionsusage is consistent with the rest of the code.
91-93:decodemethod usage is standard.
96-341: Test coverage is impressive.Tests rely on environment variables, which can affect reproducibility, but overall the coverage for invoice creation, payment, lookups, and decoding is thorough.
bindings/lni_nodejs/src/lnd.rs (6)
5-7: Structure is straightforward.Encapsulating the config in
LndNodeis clean. However, ensure there's no accidental exposure of internal fields in external modules unless intentional.
21-24: Returning the entire macaroon might be a security concern.A similar point was raised in previous reviews. Directly exposing the macaroon can be risky if logs or UI inadvertently disclose it. Consider obfuscating or restricting direct access.
26-32: Returning the full config includes sensitive macaroon.As previously discussed (lines 26-32 in past reviews), returning sensitive fields (like
macaroon) may not be ideal. Verify you truly want to expose them.
34-40: Synchronous blocking in an async method.This method uses
lni::lnd::api::get_infowhich in turn creates areqwest::blocking::Client. Past reviews recommended using async clients or making the function synchronous to avoid blocking the runtime.
41-51: Blocking calls in an async function.
create_invoicecalls an underlying blockingreqwest::Client. Reiterating the suggestion to switch toreqwest’s async API or remove theasyncsignature to avoid potential runtime blocking.
83-100: Check partial success in offer payment.If LND enters a partial payment state or partial success is returned, ensure you handle it properly. Right now, any error triggers a direct rejection. This is correct for fully failing states—but confirm partial states are also surfaced.
Do you want me to generate a script to scan for partial success scenarios or discuss them with upstream docs?
bindings/lni_nodejs/index.d.ts (5)
38-41: MinimalLndNodeinterface.While it matches the Rust struct shape, ensure you expose any additional fields or methods that the code uses internally (if needed by consumers). Otherwise, looks good.
185-189: New invoice params look valid.
rPreimage,isBlinded,isKeysend,isAmp, andisPrivateusage lines up with the Rust code. Make sure each new param is tested or documented if it’s not fully supported.
199-210: Check optional fee fields.Having both
feeLimitMsatandfeeLimitPercentageoptional is good. You already handle conflicts in Rust code by returning errors if both are set. Good consistency.
226-226: Ensure PhoenixdNode payInvoice coverage.No immediate issues found. Confirm it behaves consistently with the newly introduced
PayInvoiceParamsin the Rust backend.
239-246: CLN methods are consistent.Adding
payInvoice,getOffer,listOffers,payOffer,lookupInvoice,listTransactions, anddecodebroadens parity across nodes. Looks aligned with your LND/CLN bridging.crates/lni/lnd/api.rs (2)
15-31: Invalid cert acceptance in production.As previously noted, using
.danger_accept_invalid_certs(true)is risky. Consider removing or gating it behind a dev/test config to maintain secure TLS in production.
52-110: Async function with blocking HTTP client.Past reviewers recommended using
reqwest’s async API or removingasyncto prevent blocking the async executor. The same pattern appears increate_invoice.crates/lni/lnd/types.rs (1)
57-76: Use enums for invoice statusThe
statusfield is a string that can only be"paid","unpaid", or"expired". Consider using a Rust enum to ensure type safety, reduce runtime string checks, and prevent accidental typos.+#[derive(Debug, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum InvoiceStatus { + Paid, + Unpaid, + Expired, +} + #[derive(Debug, Deserialize)] pub struct Invoice { pub label: String, pub bolt11: Option<String>, pub bolt12: Option<String>, pub payment_hash: String, - pub status: String, // "paid" "unpaid" "expired" + pub status: InvoiceStatus, pub pay_index: Option<i32>, // ...rest of the fields remain the same
Summary by CodeRabbit
New Features
pay_invoiceto facilitate invoice payments across various nodes.Documentation
Enhancements