Skip to content

fix: await ln payment before gift card confetti#929

Open
jvsena42 wants to merge 9 commits intomasterfrom
fix/gift-card-falsse-positive
Open

fix: await ln payment before gift card confetti#929
jvsena42 wants to merge 9 commits intomasterfrom
fix/gift-card-falsse-positive

Conversation

@jvsena42
Copy link
Copy Markdown
Member

@jvsena42 jvsena42 commented May 5, 2026 •
edited
Loading

This PR:

  1. Fixes a false-positive confetti animation in the gift card redemption flow when the user already has an open Lightning channel.
  2. Fixes the gift sheet re-opening after an app language change re-delivers the launching intent.

Description

1. False-positive confetti

BlocktankRepo.claimGiftCodeWithLiquidity previously returned success as soon as Blocktank's giftPay HTTP RPC returned — but that response only confirms the LSP accepted the request, not that the LN payment actually landed. If Blocktank's background payer subsequently failed to route the HTLC, Bitkit still played confetti and surfaced a "received" sheet with the requested amount, while no incoming payment ever arrived.

The repo now subscribes to LightningRepo.nodeEvents for the Event.PaymentReceived matching the freshly-created invoice's payment hash before triggering giftPay, then awaits that event with a 45 second timeout. On timeout it throws ServiceError.GiftClaimPaymentNotReceived, which the existing error handler in GiftViewModel routes to the standard GiftRoute.Error screen. The success result reports the actual received sat amount instead of the requested amount.

The without-liquidity path (channel open + openChannel) is unchanged — that path already blocks on the funding transaction.

2. Intent re-delivery on Activity recreate

When the app language is changed, Android destroys and recreates the Activity, re-delivering the original launching intent. MainActivity.onCreate was unconditionally calling handleDeeplinkIntent(intent), so any deeplink-driven flow (gift sheet, send sheet, pubky auth, etc.) re-fired on the locale switch.

onCreate now only processes the launching intent on a fresh start (savedInstanceState == null). New deeplinks that arrive while the app is running are still handled via onNewIntent.

Preview

N/A

QA Notes

Reproducing the failure path

The originally-reported gUCdzXfm12V71xztGzDoiQ-3000 is now exhausted and returns GIFT_CODE_USED_UP (a separate, already-handled path), so it can no longer reproduce the false-positive. To exercise case 1, use one of:

  • Coordinate with the Blocktank team to provide a code whose LSP-side LN payer is in an unpayable state (accepts the HTTP request but cannot route the HTLC).
  • Issue a valid code via the regtest blocktank harness, then disconnect the LSP peer / cut network between scanning the QR and the HTLC settling, so the inbound HTLC never arrives. giftPay returns success but Event.PaymentReceived never fires.

Regtest helpers

Fire the gift deeplink directly without scanning a QR (replace the package id per flavor — to.bitkit.dev for dev/regtest, to.bitkit.tnet for testnet, to.bitkit for mainnet):

adb shell am start -a android.intent.action.VIEW -d "bitkit://gift-gUCdzXfm12V71xztGzDoiQ-3000" to.bitkit.dev

@jvsena42 jvsena42 self-assigned this May 5, 2026
@jvsena42
Copy link
Copy Markdown
Member Author

jvsena42 commented May 5, 2026

Looking for a reliable way to reproduce it

@jvsena42 jvsena42 marked this pull request as ready for review May 7, 2026 12:59
@jvsena42
Copy link
Copy Markdown
Member Author

jvsena42 commented May 7, 2026

@ovitrif the success paths worked for me, but couldn't reproduce the bug from master. we could do an internal release for @dzdidi

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed May 7, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25dd0c6052

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread app/src/main/java/to/bitkit/ui/MainActivity.kt Outdated
@claude

This comment has been minimized.

Sign in to view
@claude

This comment has been minimized.

Sign in to view
ovitrif
ovitrif previously approved these changes May 7, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

Comment thread app/src/main/java/to/bitkit/ui/MainActivity.kt Outdated
@ovitrif
Copy link
Copy Markdown
Collaborator

ovitrif commented May 7, 2026

@ovitrif the success paths worked for me, but couldn't reproduce the bug from master. we could do an internal release for @dzdidi

@jvsena42 The failed multi_address_local e2e check doesn't look related, I retriggered it; but please double check.

@jvsena42
Copy link
Copy Markdown
Member Author

jvsena42 commented May 7, 2026

@ovitrif the success paths worked for me, but couldn't reproduce the bug from master. we could do an internal release for @dzdidi

@jvsena42 The failed multi_address_local e2e check doesn't look related, I retriggered it; but please double check.

it is happening in all branches

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 7, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@piotr-iohk
Copy link
Copy Markdown
Collaborator

piotr-iohk commented May 11, 2026

Tested the gift false-positive failure path on regtest. ✅

Setup:

  • Wallet had an existing Lightning channel with enough inbound liquidity (maxInbound=744191, gift amount 3000 sats).
  • Blocked the LSP peer 34.65.86.104:9400 with a local pf rule while keeping HTTPS/Blocktank reachable.

Set up rule:

# 1. Block only the regtest LSP peer, keep HTTPS working
printf 'block drop out quick proto tcp to 34.65.86.104 port 9400\n' | sudo tee /tmp/bitkit-lsp-block.pf
sudo pfctl -a com.apple/bitkit-lsp-block -f /tmp/bitkit-lsp-block.pf
sudo pfctl -E

# 2. Verify rule is active
sudo pfctl -a com.apple/bitkit-lsp-block -sr

Remove after test:

sudo pfctl -a com.apple/bitkit-lsp-block -F rules
  • Claimed a fresh valid gift code via deeplink.

Observed:

  • App used the with-liquidity path.
  • Invoice was created.
  • giftPay returned successfully:
    Gift payment request completed ... awaiting LDK PaymentReceived
  • LSP peer connection failed due to the firewall rule.
  • No PaymentReceived arrived within the timeout.
  • App failed with GiftClaimPaymentNotReceived and showed the error state, with no confetti / received success sheet.

After removing the firewall rule, the delayed payment arrived, which confirms the test blocked settlement long enough to exercise the timeout path.

Also regression tested:

  • Fresh valid gift code with an existing channel and with no channel - payment complete ✅
  • Depleted gift code - no confetti, appropriate error message ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@piotr-iohk piotr-iohk piotr-iohk approved these changes

@ovitrif ovitrif ovitrif left review comments

Assignees

@jvsena42 jvsena42

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jvsena42 @ovitrif @piotr-iohk