feat: receive payments directly without Lightning Address#61
Open
Delgado74 wants to merge 4 commits intolachispame:mainfrom
Open
feat: receive payments directly without Lightning Address#61Delgado74 wants to merge 4 commits intolachispame:mainfrom
Delgado74 wants to merge 4 commits intolachispame:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds a Docker build-and-push workflow, updates release workflow to depend on it and include GHCR image info, modifies Docker runtime and nginx config, enhances the receive screen with invoice UI/logic and parsing, adds invoice-related localizations across 7 languages, and bumps SDK/dependency constraints. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Receive Screen
participant State as App State
participant Currency as Currency Service
participant Invoice as Invoice Generator
User->>UI: Open receive screen / request amount
UI->>State: initialize currencies
State->>Currency: fetch server & rates
Currency-->>State: currencies (or default)
State-->>UI: update UI
User->>UI: enter amount & create invoice
UI->>UI: check _isCheckingInvoice guard
UI->>UI: normalize localized number
UI->>Invoice: generate invoice (amount)
Invoice-->>UI: payment request / QR
UI->>UI: start 10-min timeout timer
UI->>User: render invoice card (QR, memo, copy/clear)
User->>UI: copy or clear invoice
UI->>UI: cancel timer, clear invoice, show confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docker/nginx.conf (1)
36-38:⚠️ Potential issue | 🔴 CriticalPort mismatch:
listen 80is incompatible withnginx-unprivileged.The Dockerfile was changed to use
nginxinc/nginx-unprivileged:alpine, which runs as a non-root user and cannot bind to privileged ports (below 1024). This config still specifieslisten 80, which will cause the container to fail on startup.🐛 Proposed fix: Update to port 8080
server { - listen 80; + listen 8080; server_name localhost;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/nginx.conf` around lines 36 - 38, The nginx config's server block currently uses the privileged port via the "listen 80;" directive which is incompatible with the nginx-unprivileged image; update the server block's listen directive from 80 to 8080 (i.e., change "listen 80;" to "listen 8080;") so the container can bind as a non-root user when using the nginx-unprivileged image.lib/screens/9receive_screen.dart (1)
1972-2047:⚠️ Potential issue | 🟠 MajorGuard the paid flow against stale polling results.
Canceling the timer does not cancel an in-flight
checkInvoiceStatus()call. If the user clears or replaces the invoice while that await is running, this callback can still navigate away for the old payment hash because the result is never revalidated against the current_generatedInvoice.Suggested guard
void _startInvoicePaymentMonitoring( LightningInvoice invoice, WalletInfo wallet, String serverUrl, ) { + final monitoredHash = invoice.paymentHash; + _invoicePaymentTimer = Timer.periodic(const Duration(seconds: 2), ( timer, ) async { if (_isCheckingInvoice) { return; @@ final isPaid = await _invoiceService.checkInvoiceStatus( serverUrl: serverUrl, adminKey: wallet.adminKey, - paymentHash: invoice.paymentHash, + paymentHash: monitoredHash, ); + + if (!mounted || _generatedInvoice?.paymentHash != monitoredHash) { + timer.cancel(); + return; + } if (isPaid) { print('[RECEIVE_SCREEN] Invoice paid! Starting celebration sequence'); timer.cancel(); + _invoicePaymentTimeoutTimer?.cancel(); if (mounted) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/screens/9receive_screen.dart` around lines 1972 - 2047, The paid-flow can act on stale results because the awaited _invoiceService.checkInvoiceStatus call isn't revalidated against the current invoice; capture the current invoice/paymentHash into a local variable (e.g., localInvoice or requestedPaymentHash) before calling _invoiceService.checkInvoiceStatus, then after awaiting isPaid verify that the current _generatedInvoice still matches that local identifier (compare paymentHash or object identity) and only proceed to cancel _invoicePaymentTimer, trigger _transactionDetector.triggerEventSpark('invoice_paid'), navigate, and show the SnackBar if they match; if they don't match, ignore the result and return, making sure to still reset _isCheckingInvoice in finally.
🧹 Nitpick comments (2)
.github/workflows/docker-release.yml (1)
34-37:latesttag will never be applied for tag-triggered builds.The condition
enable={{is_default_branch}}evaluates tofalsefor tag push events because tags don't have a branch context. If you wantlatestto track the most recent release tag, consider using a different approach.♻️ Option to always apply `latest` for version tags
tags: | type=ref,event=tag type=sha,prefix= - type=raw,value=latest,enable={{is_default_branch}} + type=raw,value=latestAlternatively, if you want
latestonly on stable releases (not pre-releases), you could use a more complex condition with a separate step.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-release.yml around lines 34 - 37, The current tags entry "type=raw,value=latest,enable={{is_default_branch}}" never becomes true for tag-triggered builds; update the tags logic so the "latest" tag is applied for the default branch and/or for tag pushes. Concretely, modify the tags block that contains "type=raw,value=latest,enable={{is_default_branch}}" so its enable expression also checks for tag refs (e.g., enable={{ startsWith(github.ref, 'refs/tags/') || is_default_branch }}), or split into two tag entries: one conditioned on is_default_branch and another conditioned on startsWith(github.ref,'refs/tags/'), ensuring the symbol "type=raw,value=latest" is used for the tag push case.lib/screens/9receive_screen.dart (1)
74-78: Avoid refreshing exchange rates through both paths.
CurrencySettingsProvider.updateServerUrl()already reloads currencies and exchange rates inlib/providers/currency_settings_provider.dart:359-372. When the server changes, the extraloadExchangeRates(forceRefresh: true)immediately repeats that work and slows screen initialization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/screens/9receive_screen.dart` around lines 74 - 78, The screen is redundantly forcing exchange-rate reload after calling CurrencySettingsProvider.updateServerUrl which already reloads currencies and rates; remove the extra await currencyProvider.loadExchangeRates(forceRefresh: true) call in the 9receive_screen (the block checking authProvider.currentServer) so the updateServerUrl(...) call alone handles reloading and avoids duplicate work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 77-80: The documentation's docker run command maps container port
80 but the Dockerfile and base image (nginx-unprivileged) expose port 8080;
update the docker run port mapping in the release workflow snippet by changing
the container side from :80 to :8080 (i.e., modify the docker run command
referenced in the diff to use -p 7777:8080) so the published port matches the
container's exposed port.
- Around line 3-7: The workflow_run trigger is using a branches filter that will
never match tag-triggered runs; remove the branches: [main] filter from the
workflow_run block so the trigger can fire for the referenced workflow, and
update the workflows reference to use the target workflow's name ("Build and
Push Docker Image") instead of the filename ("docker-release.yml"); specifically
modify the on.workflow_run section (workflows and branches keys) to only list
the workflow name and remove the branches key.
In `@docker/Dockerfile`:
- Around line 16-24: Update all references to the old port 80 to the new 8080 to
match the Docker image EXPOSE change: in docker/nginx.conf change the server
listen directive (e.g., "listen 80") to "listen 8080"; in docker-compose.yml
update the service port mapping (e.g., "7777:80") to "7777:8080", change the
NGINX_PORT env value from 80 to 8080, and update any Traefik label
loadbalancer.server.port values from 80 to 8080; in docker/README.md update any
documented Traefik label examples (loadbalancer.server.port=80) to 8080; and in
the GitHub Actions workflow replace docker run -p 7777:80 with docker run -p
7777:8080 so all runtime, docs, and labels are consistent.
In `@docker/README.md`:
- Line 191: Replace the obsolete anchor using the name attribute (<a
name="espanol-section"></a>) with an HTML element that uses an id attribute
(e.g., <a id="espanol-section"></a> or add id to the target heading) so markdown
renderers and GitHub link resolution work reliably; update any internal links
that referenced "#espanol-section" if needed to point to the same id, and prefer
adding the id to the section heading rather than a standalone empty anchor for
clarity.
In `@lib/l10n/app_it.arb`:
- Line 71: The localized string for key "create_lnaddress_label" is unidiomatic
Italian; replace the value with a native-speaker–approved CTA (for example an
idiomatic alternative such as "oppure crea un indirizzo:" or "oppure crea un:"),
ensuring tone/punctuation match the rest of the Italian file and confirm the
final phrasing with a native Italian reviewer before merging.
In `@lib/l10n/app_ru.arb`:
- Line 71: The localized string for key "create_lnaddress_label" is awkward and
has an extra space before the colon; update the value in app_ru.arb to a
natural, grammatically correct Russian CTA without the stray space (for example
rephrase to something like "или вы также можете создать адрес:" or another
idiomatic short CTA) so the text reads fluently and the punctuation is correct.
In `@lib/screens/9receive_screen.dart`:
- Around line 1736-1738: The hardcoded Spanish exception thrown when
amountInSats exceeds the 21M limit should be replaced with a localized message
from the app's l10n resources instead of inline text; in the guard where you
check amountInSats (> 2100000000000000) (around the code that currently throws
Exception('Monto muy grande. Máximo: 21M BTC')), replace that throw with one
that uses the localization API (e.g.,
AppLocalizations.of(context).maxAmountError or the project's equivalent key) or
throw a typed error carrying a localization key so the catch path can call the
l10n lookup before showing the snackbar. Ensure you reference the same unique
symbols (amountInSats and the throwing location in 9receive_screen /
ReceiveScreen) so the message is not language-locked.
- Around line 115-142: The current validation uses double.tryParse on the raw
controller text which rejects comma-decimal inputs before
_normalizeLocalizedNumber can run; update the validation logic that currently
parses controller.text directly to instead call
_normalizeLocalizedNumber(controller.text.trim()) and treat a non-null return as
valid (use the returned double value for further checks and storage), replacing
any direct double.tryParse(controller.text) checks so inputs like "1,5" or "1
000,50" are accepted.
---
Outside diff comments:
In `@docker/nginx.conf`:
- Around line 36-38: The nginx config's server block currently uses the
privileged port via the "listen 80;" directive which is incompatible with the
nginx-unprivileged image; update the server block's listen directive from 80 to
8080 (i.e., change "listen 80;" to "listen 8080;") so the container can bind as
a non-root user when using the nginx-unprivileged image.
In `@lib/screens/9receive_screen.dart`:
- Around line 1972-2047: The paid-flow can act on stale results because the
awaited _invoiceService.checkInvoiceStatus call isn't revalidated against the
current invoice; capture the current invoice/paymentHash into a local variable
(e.g., localInvoice or requestedPaymentHash) before calling
_invoiceService.checkInvoiceStatus, then after awaiting isPaid verify that the
current _generatedInvoice still matches that local identifier (compare
paymentHash or object identity) and only proceed to cancel _invoicePaymentTimer,
trigger _transactionDetector.triggerEventSpark('invoice_paid'), navigate, and
show the SnackBar if they match; if they don't match, ignore the result and
return, making sure to still reset _isCheckingInvoice in finally.
---
Nitpick comments:
In @.github/workflows/docker-release.yml:
- Around line 34-37: The current tags entry
"type=raw,value=latest,enable={{is_default_branch}}" never becomes true for
tag-triggered builds; update the tags logic so the "latest" tag is applied for
the default branch and/or for tag pushes. Concretely, modify the tags block that
contains "type=raw,value=latest,enable={{is_default_branch}}" so its enable
expression also checks for tag refs (e.g., enable={{ startsWith(github.ref,
'refs/tags/') || is_default_branch }}), or split into two tag entries: one
conditioned on is_default_branch and another conditioned on
startsWith(github.ref,'refs/tags/'), ensuring the symbol "type=raw,value=latest"
is used for the tag push case.
In `@lib/screens/9receive_screen.dart`:
- Around line 74-78: The screen is redundantly forcing exchange-rate reload
after calling CurrencySettingsProvider.updateServerUrl which already reloads
currencies and rates; remove the extra await
currencyProvider.loadExchangeRates(forceRefresh: true) call in the
9receive_screen (the block checking authProvider.currentServer) so the
updateServerUrl(...) call alone handles reloading and avoids duplicate work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 407cf2df-d830-4bd1-b1aa-5827eb6d67d5
⛔ Files ignored due to path filters (8)
lib/l10n/generated/app_localizations.dartis excluded by!**/generated/**lib/l10n/generated/app_localizations_de.dartis excluded by!**/generated/**lib/l10n/generated/app_localizations_en.dartis excluded by!**/generated/**lib/l10n/generated/app_localizations_es.dartis excluded by!**/generated/**lib/l10n/generated/app_localizations_fr.dartis excluded by!**/generated/**lib/l10n/generated/app_localizations_it.dartis excluded by!**/generated/**lib/l10n/generated/app_localizations_pt.dartis excluded by!**/generated/**lib/l10n/generated/app_localizations_ru.dartis excluded by!**/generated/**
📒 Files selected for processing (15)
.github/workflows/docker-release.yml.github/workflows/release.ymldocker/Dockerfiledocker/README.mddocker/nginx.confl10n.yamllib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_pt.arblib/l10n/app_ru.arblib/screens/9receive_screen.dartpubspec.yaml
This was referenced Mar 22, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/l10n/app_it.arb`:
- Line 159: The ARB entry "invoice_amount_label" includes a {amount} placeholder
but is missing its metadata; add a corresponding metadata block named
"@invoice_amount_label" that documents the message (brief description) and
defines the placeholders map with "amount" (specify its type, e.g. "String" or
"double" as used in source locales, and an example/format if applicable). Repeat
the same metadata addition for the other affected files (app_de.arb, app_fr.arb,
app_pt.arb, app_ru.arb) to match en/es and restore correct localization type
generation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2215ab1f-da9e-4434-8c48-c655a320fb96
⛔ Files ignored due to path filters (2)
lib/l10n/generated/app_localizations_it.dartis excluded by!**/generated/**lib/l10n/generated/app_localizations_ru.dartis excluded by!**/generated/**
📒 Files selected for processing (2)
lib/l10n/app_it.arblib/l10n/app_ru.arb
✅ Files skipped from review due to trivial changes (1)
- lib/l10n/app_ru.arb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Allow users to receive Lightning payments without requiring a Lightning Address by creating invoices directly from the wallet.
Changes
Motivation
Currently, users need a Lightning Address to receive payments. This change enables anyone with a LaChispa wallet to receive payments by simply generating an invoice, making the app more accessible and easier to use for new users.
Testing
Summary by CodeRabbit
New Features
Chores
Documentation