-
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 theLndNode
interface.
SinceLndConfig
already hasurl
andmacaroon
, including them again inLndNode
may 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
, andisPrivate
do for improved clarity and maintainability.
226-239
: Unify node classes under a common baseline interface.
PhoenixdNode
,ClnNode
, andLndNode
largely 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
, andlimit
are 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.
LndNode
appears structurally similar toPhoenixdNode
andClnNode
. 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_private
would benefit from short descriptive statements on their purpose.crates/lni/lnd/types.rs (6)
1-3
: Consider derivingSerialize
as well
Currently, you only deriveDeserialize
. If you plan to send these structures back to clients or log them in JSON, derivingSerialize
can be beneficial.
4-8
: Use a typed enum for the network if possible
TheChain
struct storeschain
andnetwork
as 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
:FetchInvoiceResponse
rename consideration
invoice
could be renamed to a more descriptive field, likeencoded_invoice
orbolt11
, to distinguish it from other invoice fields in the codebase.
39-49
: Checkcreated_at
type inPayResponse
created_at
is 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 ofcreated
inBolt12Resp
With fields likeactive
,single_use
, andused
, the booleancreated
could be confusing. Consider a more descriptive name likeis_newly_created
or 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_with
or custom deserializers?crates/lni/lnd/lib.rs (5)
9-19
: Consider doc comments forLndConfig
Adding doc comments to fieldsurl
andmacaroon
clarifies their intended usage and valid formats. This is especially helpful for new team members or external consumers unfamiliar with LND.
21-27
: ValidateLndNode::new
parameters
IfLndConfig
allows empty or invalidurl
andmacaroon
, consider validating or returning an error if they don't match expected patterns.
33-38
: Review concurrency approach increate_invoice
Using.await
ensures 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
from
andlimit
are 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 theLndNode
struct
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 thatconfig
is valid. If you anticipate malicious or invalid input from external calls, consider validatingconfig.url
andconfig.macaroon
.
16-19
: Consider customizingget_url
get_url
trivially 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_invoice
Node 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_offers
usage
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_transactions
filtering
This function does not acceptpayment_hash
here (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_request
for 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 appropriateApiError
instead.- 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.,
log
crate) 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 theApiError
instead 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
cln
tolnd
branch 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
hex
andsha2
crates 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
LndNode
class is now properly extracted from the native binding, consistent with the other node types (PhoenixdNode, ClnNode).
318-318
: LndNode properly exported from moduleThe
LndNode
class 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 theCreateInvoiceParams
struct. 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_invoices
are 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
Clone
trait to theTransaction
struct 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_preimage
isNone
since 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_transactions
API 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_invoice
function 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
from
orlimit
parameters.
Sincefrom
andlimit
are 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
from
andlimit
Parameters
Please verify that negative values forfrom
andlimit
are either safely handled withinlookup_invoices
(defined incrates/lni/cln/api.rs
around 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 inGetInfoResponse
exist across LND versions
Some older or customized LND versions may not provide fields such asrequire_htlc_interceptor
orstore_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 thetxid
field 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_offer
andlist_offers
disclaimers
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_invoice
error 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
decode
is 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 importLndConfig
fromlni::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_offer
referencesBolt12
, 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_invoice
return validation
Ensure that the resultingTransaction
object 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,
Bolt11Resp
may differ from the fields (payment_request
andr_hash
) used here. Ensure the localBolt11Resp
struct 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
str
as a parameter name may overshadow thestr
type reference in Rust. Consider something likeencoded_str
orpayload_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.
PhoenixPayInvoiceResp
aligns with existing naming patterns. If consistent with the rest of your codebase, all good. Otherwise, consider a more generic name likePayInvoiceResp
for reuse.
34-37
: Consider flagging macaroon as a secret.You might mark
macaroon
with 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_transactions
is 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
status
field inPayResponse
is 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
LndPayInvoiceResponse
structure contains several fields that could benefit from stronger typing:
status
field should be an enum instead of a string- Using more semantic types for values, timestamps, and identifiers would improve code clarity
failure_reason
field 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
status
field in theHtlc
struct 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
, andf64
when 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::Value
Several fields use
serde_json::Value
for 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
ListInvoiceResponse
struct 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
lnd
module follows the same structure as the existingphoenixd
andcln
modules, with appropriate sub-modules and exports. This maintains consistency in the codebase.
47-48
: LGTM - New utils module exported.The introduction of a
utils
module 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
PhoenixPayInvoiceResp
struct 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_msats
function 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_invoice
method 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
fee
tofee_msats
makes 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_msat
could yield a negative fee in edge cases. Please confirm that real-world scenarios never allowamount_sent_msat
to 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 thetransactions
vector 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_invoice
method for PhoenixdNode looks fine.
48-50
: Newpay_invoice
method 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
:PayCode
dictionary updates look good.
324-335
:PayInvoiceParams
dictionary 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
:LndConfig
struct is straightforward.
16-20
:LndNode
struct definitions look good.
22-28
: Constructor properly assigns config fields.
30-32
:get_info
method delegates logic appropriately.
34-39
:create_invoice
method usage looks correct.
41-46
:pay_invoice
method aligns with desired usage.
48-50
:get_offer
method is consistent with the LND API.
52-54
:list_offers
method is acceptable.
56-70
:pay_offer
implementation delegates to the API correctly.
72-77
:lookup_invoice
is straightforward.
79-89
:list_transactions
usage is consistent with the rest of the code.
91-93
:decode
method 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
LndNode
is 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_info
which 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_invoice
calls an underlying blockingreqwest::Client
. Reiterating the suggestion to switch toreqwest
’s async API or remove theasync
signature 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
: MinimalLndNode
interface.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
, andisPrivate
usage 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
feeLimitMsat
andfeeLimitPercentage
optional 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
PayInvoiceParams
in the Rust backend.
239-246
: CLN methods are consistent.Adding
payInvoice
,getOffer
,listOffers
,payOffer
,lookupInvoice
,listTransactions
, anddecode
broadens 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 removingasync
to prevent blocking the async executor. The same pattern appears increate_invoice
.crates/lni/lnd/types.rs (1)
57-76
: Use enums for invoice statusThe
status
field 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_invoice
to facilitate invoice payments across various nodes.Documentation
Enhancements