(draft) feat: onchain #161
Conversation
WalkthroughThis pull request updates the LDKNodeMonday Xcode project configuration and dependency management. The project file’s object version and framework references have been revised, and a remote Swift package dependency (“ldk-node”) has been replaced with a local reference. Several Swift source files are modified: the Event enum now features updated case signatures and a new case; methods in the LightningNodeService and JITInvoiceViewModel now require a refined invoice description type; and a new error case is added for “InvalidCustomTlvs.” Additionally, new properties have been introduced in the channel detail view. Changes
Sequence Diagram(s)sequenceDiagram
participant V as JITInvoiceView
participant VM as JITInvoiceViewModel
participant LS as LightningNodeService
V ->> VM: initiate receivePaymentViaJitChannel(description: .direct("Monday Wallet"))
VM ->> LS: call receiveViaJitChannel(..., Bolt11InvoiceDescription, ...)
LS -->> VM: return Bolt11Invoice
VM -->> V: update UI with Bolt11Invoice
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
LDKNodeMonday/Service/Lightning Service/LightningServiceError.swift (1)
167-169: LGTM! Consider adding documentation for the new error case.The implementation of the
InvalidCustomTlvserror case follows the established pattern and maintains consistency with other error cases.Consider adding a documentation comment explaining what constitutes invalid TLV records to help developers better understand and handle this error case.
+ /// Represents an error that occurs when custom TLV (Type-Length-Value) records are invalid. + /// This could happen when the TLV records don't conform to the Lightning Network specifications. case .InvalidCustomTlvs(let message): return .init(title: "InvalidCustomTlvs", detail: message)LDKNodeMonday/Extensions/Event+Extensions.swift (1)
39-52: Enhance the payment forwarded description with relevant details.The case contains rich information about the forwarded payment (fees, amounts, channels) but returns a simple static string. Consider enhancing the description to include key details that would be valuable for users or debugging.
Example enhancement:
- return "Payment Forwarded" + let formattedFee = totalFeeEarnedMsat.formattedAmount() + let formattedAmount = outboundAmountForwardedMsat.formattedAmount() + return "Payment Forwarded: Amount \(formattedAmount) sats, Fee earned \(formattedFee) sats"LDKNodeMonday/View/Home/Receive/JITInvoiceView.swift (1)
47-47: Consider using a constant for the wallet description.The hardcoded string "Monday Wallet" would be better defined as a constant to maintain consistency and ease future updates.
+private let WALLET_DESCRIPTION = "Monday Wallet" + // In the view body: -description: .direct(description: "Monday Wallet"), +description: .direct(description: WALLET_DESCRIPTION),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
LDKNodeMonday.xcodeproj/project.pbxproj(7 hunks)LDKNodeMonday.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved(1 hunks)LDKNodeMonday/Extensions/Event+Extensions.swift(2 hunks)LDKNodeMonday/Service/Lightning Service/LightningNodeService.swift(3 hunks)LDKNodeMonday/Service/Lightning Service/LightningServiceError.swift(1 hunks)LDKNodeMonday/View Model/Home/Receive/JITInvoiceViewModel.swift(1 hunks)LDKNodeMonday/View/Home/Receive/JITInvoiceView.swift(1 hunks)LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift(1 hunks)
🧰 Additional context used
📓 Learnings (1)
LDKNodeMonday.xcodeproj/project.pbxproj (1)
Learnt from: danielnordh
PR: reez/Monday#131
File: LDKNodeMonday.xcodeproj/project.pbxproj:180-180
Timestamp: 2024-12-05T17:54:34.328Z
Learning: Multiple LDKNode framework references in `LDKNodeMonday.xcodeproj/project.pbxproj` may be intentional when using custom forks or multiple versions of LDKNode, and should not be flagged as duplicates.
🔇 Additional comments (10)
LDKNodeMonday/Extensions/Event+Extensions.swift (3)
17-18: Verify if new parameters should be included in the description.The case now includes two additional parameters, but they are not reflected in the description string. Consider if these parameters contain relevant information that should be displayed to users.
24-26: Verify if the new parameter should be included in the description.The case now includes an additional parameter, but it is not reflected in the description string. Consider if this parameter contains relevant information that should be displayed to users.
53-60: LGTM! Good security practice.The description appropriately excludes the newly added
customRecordsparameter, which is a good practice as these records might contain sensitive data.LDKNodeMonday.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1)
1-43: LGTM! Package dependencies are properly configured.The removal of the
ldk-nodepackage from Package.resolved is consistent with switching to a local package reference. All other dependencies are properly pinned to specific versions.LDKNodeMonday.xcodeproj/project.pbxproj (2)
6-6: Verify Xcode version compatibility.The project's
objectVersionhas been updated from 56 to 60, which requires a newer version of Xcode. Please ensure all team members have compatible Xcode versions installed.
182-188: Review multiple LDKNode framework references.There are 3 identical LDKNode framework references in the PBXBuildFile section. While multiple references may be intentional based on past feedback, having 3 identical references is unusual and could lead to linking issues.
Run the following script to check for any potential linking issues:
LDKNodeMonday/View Model/Home/Receive/JITInvoiceViewModel.swift (1)
27-27: LGTM! Type safety improvement.The change from
StringtoBolt11InvoiceDescriptionenhances type safety and aligns with the domain model.LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (1)
127-129: LGTM! Preview data updated.The new channel properties are correctly added with consistent mock values.
LDKNodeMonday/Service/Lightning Service/LightningNodeService.swift (2)
266-279: LGTM! Type safety improvement in service layer.The change from
StringtoBolt11InvoiceDescriptionenhances type safety and is consistently applied across the service layer.
364-365: LGTM! Client interface updated.The client interface is correctly updated to match the service layer changes.
|
I have code comments in for the log stuff but i |
|
FeeRate Options 
|
Description
Onchain per lightningdevkit/ldk-node#432 and locally built bindings
Notes to the reviewers
Not doing anything like moving transaction list to the main screen, just updating code per onchain transactions being exposed in ldk-node now.
I'm also going to add some additional comments below of interesting things I saw as of my local build's newest commit b0f4443
Changelog notice
Checklists
All Submissions:
.swift-formatfileNew Features:
Bugfixes: