-
Notifications
You must be signed in to change notification settings - Fork 1
feat: bitkit-core serializable types #442
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
feat: bitkit-core serializable types #442
Conversation
28652c5 to
a0717bb
Compare
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.
Pull Request Overview
Updates bitkit-core dependency to 0.1.18 (introducing nullable / serializable payment-related fields) and refactors the developer Channel Orders UI to a card + section header style while adding null-safe accessors. Key changes:
- Dependency bump: bitkit-core from 0.1.10 to 0.1.18 and widespread adjustments for newly nullable fields (payment, lspNode, etc.).
- Refactor of Channel Orders / Order & CJIT detail screens: structural renaming, extraction of reusable composables (InfoCard, *Card variants), added section headers, and removal of ad‑hoc copy/animation code in favor of utilities.
- Added null-safe access (e.g., payment?, lspNode?) but introduced a few silent fallbacks that may surface incorrect or misleading UI values.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Bumps bitkit-core library version. |
| TransferViewModel.kt | Adjusts on-chain send to handle nullable payment but risks sending empty address silently. |
| ChannelStatusView.kt | Makes payment state access nullable; adds suppression for cyclomatic complexity. |
| LightningConnectionsViewModel.kt | Null-safe access to lspNode pubkey. |
| ChannelDetailScreen.kt | Null-safe payment state checks for channel status logic. |
| ChannelOrdersScreen.kt | Major UI refactor, new composables, null-safe data access; some new string and data handling issues introduced. |
| SectionHeader.kt | Adds configurability (padding, height) and expanded preview. |
| lightningRepo | ||
| .sendOnChain( | ||
| address = order.payment.onchain.address, | ||
| address = order.payment?.onchain?.address.orEmpty(), |
Copilot
AI
Oct 16, 2025
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.
Replacing a previously non-null address with order.payment?.onchain?.address.orEmpty() risks invoking sendOnChain with an empty string, producing an invalid on-chain send attempt and hiding the underlying null condition. Prefer an early return or explicit error state when the address is null instead of silently coercing to "". Example: val address = order.payment?.onchain?.address ?: return /* or emit error */.
| text = model.id, | ||
| maxLines = 1, | ||
| overflow = TextOverflow.MiddleEllipsis, | ||
| modifier = Modifier.clickableAlpha(onClick = copyToClipboard(model.id)) |
Copilot
AI
Oct 16, 2025
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.
Passing copyToClipboard(model.id) directly as the onClick argument assumes copyToClipboard returns a () -> Unit; if it performs the copy immediately (returns Unit) this will execute during composition and result in a type mismatch or unintended side effects. Ensure copyToClipboard returns a lambda; otherwise wrap: modifier = Modifier.clickableAlpha { copyToClipboard(model.id) }.
| modifier = Modifier.clickableAlpha(onClick = copyToClipboard(model.id)) | |
| modifier = Modifier.clickableAlpha(onClick = { copyToClipboard(model.id) }) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
jvsena42
left a 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.
Tested perform transfers on a fresh wallet
This PR updates bitkit-core dependency to the version 0.1.18 which includes serializable types.
It also adds polishing to the channel orders screen from dev settings.
Preview
Just randomly doing transfers and checking activity still works.
Screen_recording_20251016_161824.mp4
QA Notes
Regression Test: transfer to savings & back to spending, validate activity list and detail views data.