fix: harden Paykit request handling - #684

Open
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes
Open

fix: harden Paykit request handling#684
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

This PR hardens incoming Paykit contact-payment handling after QA on the cross-platform payment-request flow.

Description

  1. Keeps one contact's malformed private receiver marker from blocking endpoint publication for otherwise valid contacts, while continuing to reject the malformed marker itself.
  2. Validates incoming request amounts during initial invoice routing, so unaffordable requests show the standard insufficient Spending or Savings balance message instead of reaching coin selection with a generic error.

No Paykit SDK or binding change is required. Companion Android fix: synonymdev/bitkit-android#1172

Linked Issues/Tasks

Screenshot / Video

N/A — this corrects error handling in the existing payment-request flow.

QA Notes

Manual Tests

  • 1. Saved contacts include one malformed remote Paykit marker → enable Contact Payments: valid contacts publish their private payment details and the malformed marker remains unusable.
  • 2a. Incoming LN payment request exceeds Spending balance → open request: the insufficient Spending message appears instead of coin selection.
    • 2b. Incoming on-chain payment request exceeds Savings balance → open request: the insufficient Savings message appears instead of coin selection.

Automated Checks

  • PrivatePaykitServiceTests: 15 tests passed on an iPhone 16 simulator.
  • The Debug simulator test build, SwiftFormat lint, and git diff --check passed.

@greptile-apps

Copy link
Copy Markdown

Greptile Summary

This PR hardens incoming Paykit contact-payment handling by isolating malformed receiver markers and validating request amounts against the balance before entering the send flow.

  • Allows valid contacts’ private endpoints to publish when another contact has a malformed receiver marker.
  • Uses the incoming Paykit request amount for Lightning and on-chain preflight balance checks.
  • Adds a changelog entry describing the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed preflight checks aligned to the amount ultimately paid and malformed markers safely isolated from valid contacts.

Private Paykit endpoints are intentionally amountless, the incoming request amount is injected into payment execution and checked again during confirmation, and valid receiver paths continue through publication while malformed paths remain rejected and logged.

Important Files Changed

FilenameOverview
Bitkit/Services/PrivatePaykitService+Contacts.swiftStops a malformed receiver marker from becoming the aggregate publication error while retaining logging and processing valid receiver paths.
Bitkit/ViewModels/AppViewModel.swiftUses the incoming request amount for initial Lightning and on-chain affordability checks, matching the amount later injected into payment execution.
changelog.d/next/paykit-request-qa.fixed.mdAccurately documents the malformed-marker isolation and insufficient-balance handling fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Paykit request] --> B[Resolve private amountless endpoint]
B --> C[Decode Lightning, on-chain, or unified target]
C --> D[Read amount from incoming request]
D --> E{Preferred route has sufficient balance?}
E -->|Yes| F[Open shared send flow]
E -->|No, unified request| G{On-chain balance sufficient?}
G -->|Yes| F
G -->|No| H[Show insufficient balance message]
E -->|No, single route| H
F --> I[Inject request amount]
I --> J[Confirm exact request amount]
J --> K[Pay]
Loading

Reviews (1): Last reviewed commit: "fix: harden Paykit contact payments" | Re-trigger Greptile

@piotr-iohk

piotr-iohk commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Retested after 97033fe.

Enable — still fails. ❌

Toggle Enable payments with contactsPrivate Paykit is not available, toggle does not stay on. Same staging contact as the Android QA (pubky1jy146y751hkgqk9p69ds5cqf461aj5x7c8ptbca5rw6m9asotso). Recording + logs attached.

This is not the malformed-marker isolation path. enable() still calls preparePrivateEndpoints(..., requireImmediate: true) before setting publishingEnabledKey. canPublishPrivateEndpoints requires that flag already true, so any saved contact fail-closes as privateUnavailable.

Same surface on Android #1172 (toast there is Unknown error).

Steps:

  1. Paykit UI on. Profile exists. Staging pubky above saved as a contact (need not be live).
  2. Payments with contacts off.
  3. Enable payments with contacts.
  4. Toast is Private Paykit is not available; toggle stays off.
Screen.Recording.2026-08-27.at.10.25.35.-.ios.mov

bitkit_logs_2026-08-27_08-28-24.zip

Over-balance — copy is fixed, toast loops. ❌

Pay on a request larger than the wallet now shows the right message (Insufficient Savings, e.g. ₿ 26,717 more needed) instead of coin selection. On Android that toast fires once. On iOS it keeps coming back. Tapping Pay again then shows The payment request is already being processed. Recording attached.

validateOnchainBalance toasts and returns; presentNextIncomingPaykitPaymentRequest then deferPresentation (2s × 14). Each retry runs handleScannedData again and toasts again. Android’s equivalent is clearActiveContactPaymentContext(retryIncomingRequest = false) after the insufficient toast — no retry.

Steps:

  1. Two live wallets, payments with contacts on. Payer under the request amount (split savings + spending is fine).
  2. Other wallet requests more than that total.
  3. On iOS, tap Pay on the incoming request / See All row.
  4. Insufficient Savings toast repeats. A second Pay can show already being processed.
Screen.Recording.2026-08-27.at.11.14.21-os.overbalance.mov

bitkit_logs_2026-08-27_09-15-42-overbalance.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed both retest findings in 5ba7ee7. Private publication is now enabled before immediate endpoint preparation, with rollback still handled by the existing failure path. An unaffordable request is marked surfaced after the single balance toast, so it remains in the bell/history queue without the 2-second automatic retry loop or already-processing follow-up. The focused iOS suites pass 56/56 tests.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unaffordable incoming request path and the malformed-marker isolation both lack tests that would fail if those behaviors regressed.

  1. handleScannedData now uses incomingPaymentRequest.amountSats and the insufficient-balance path marks the request presented so it does not retry. No test drives an unaffordable request through that routing.
  2. A malformed private receiver marker no longer assigns firstError, so other contacts can still publish. No fixture would fail if that assignment returned.

Inline comments have the specific coverage to add.

Comment threadBitkit/ViewModels/AppViewModel.swift
Comment threadBitkit/Services/PrivatePaykitService+Contacts.swift
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Retested 112fb70.

Over-balance: fixed. One insufficient-capacity toast, no retry loop. ✅

Enable: still broken. Five toggles failed with privateUnavailable ❌
(09:41:49, 09:41:57, 09:46:04, 09:46:17, 09:46:25). Setting
publishingEnabledKey first did not help.

Logs never reach inspect/prepare/queue/deliver. 09:46:04 is
pubky_secret_key loaded → enable ERROR in 2ms, so this is still
canPublishPrivateEndpoints == false + requireImmediate + a saved
contact, then restore() rolls the toggle off.

Also still had pubky8u997gg after deleting pubky1jy146y, so this
is not specific to that staging contact.

Need the failing canPublish clause logged, and enable should not
fail-close when publication never actually ran.

Screen.Recording.2026-09-01.at.11.58.31.mov

bitkit_logs_2026-09-01_09-46-59.zip

@ben-kaufman
ben-kaufmanforce-pushed the codex/paykit-request-qa-fixes branch from 112fb70 to 154b3d6CompareSeptember 1, 2026 12:32
@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed the remaining enable failure in 2d90e0e. Private endpoint preparation during enable is now best effort, so temporary wallet, node, app, or Pubky unreadiness defers publication instead of returning privateUnavailable and rolling the toggle back. Real errors after publication starts still fail and restore the previous state. The readiness check now logs the exact unavailable condition. ContactPaymentsServiceTests and PrivatePaykitServiceTests pass 24 of 24. @piotr-iohk please retest the Enable case, and @ovitrif please re-review the updated head.

@piotr-iohk

piotr-iohk commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Retested latest revision (2d90e0e), iPhone 17 sim.

Enable still fail-closes. Toggle → Failed to update contact payments: privateUnavailable (GeneralSettingsView.updateContactPayments line 180) → “Private Paykit is not available.”

2d90e0e is on this build — startup logged Deferring private Paykit endpoint publication during prepare: private publication is disabled (flag off, 0 contacts). On the enable attempt itself there was no second defer: flag was set, canPublish passed, then it still threw. Sequence: loaded 1 contact (pubkypwuhkhj…), then the error ~11s later. No Failed to inspect/prepare/sync WARN around the fail.

Hypothesis: requireImmediatePublication: false on enable is bypassed by the unguarded return error after receiverPathSelection in syncLocalEndpointPublicationLocked. Other failures on that path are supposed to defer.

Android enable stays on after the matching defer change.

Logs:
bitkit_logs_2026-09-01_15-09-55.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

You were right about the remaining fail-closed path. The receiverPathSelection catch returned immediately before the deferred-publication policy was applied. Signed commit ab3308b8 now records/logs that per-contact error, continues reconciliation, and returns it only for immediate publication.

The regression test covers this exact selection failure, and the focused private/contact/request suites pass 74/74. I also installed this exact head over the saved-contact QA state, enabled Paykit UI, and verified the toggle stayed enabled past 15 seconds with the success toast and no privateUnavailable rollback. Please retest ab3308b8 after the fresh checks finish.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ben-kaufman@piotr-iohk@ovitrif
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: harden Paykit request handling - #684

Open
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes
Open

fix: harden Paykit request handling#684
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

This PR hardens incoming Paykit contact-payment handling after QA on the cross-platform payment-request flow.

Description

  1. Keeps one contact's malformed private receiver marker from blocking endpoint publication for otherwise valid contacts, while continuing to reject the malformed marker itself.
  2. Validates incoming request amounts during initial invoice routing, so unaffordable requests show the standard insufficient Spending or Savings balance message instead of reaching coin selection with a generic error.

No Paykit SDK or binding change is required. Companion Android fix: synonymdev/bitkit-android#1172

Linked Issues/Tasks

Screenshot / Video

N/A — this corrects error handling in the existing payment-request flow.

QA Notes

Manual Tests

  • 1. Saved contacts include one malformed remote Paykit marker → enable Contact Payments: valid contacts publish their private payment details and the malformed marker remains unusable.
  • 2a. Incoming LN payment request exceeds Spending balance → open request: the insufficient Spending message appears instead of coin selection.
    • 2b. Incoming on-chain payment request exceeds Savings balance → open request: the insufficient Savings message appears instead of coin selection.

Automated Checks

  • PrivatePaykitServiceTests: 15 tests passed on an iPhone 16 simulator.
  • The Debug simulator test build, SwiftFormat lint, and git diff --check passed.

@greptile-apps

Copy link
Copy Markdown

Greptile Summary

This PR hardens incoming Paykit contact-payment handling by isolating malformed receiver markers and validating request amounts against the balance before entering the send flow.

  • Allows valid contacts’ private endpoints to publish when another contact has a malformed receiver marker.
  • Uses the incoming Paykit request amount for Lightning and on-chain preflight balance checks.
  • Adds a changelog entry describing the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed preflight checks aligned to the amount ultimately paid and malformed markers safely isolated from valid contacts.

Private Paykit endpoints are intentionally amountless, the incoming request amount is injected into payment execution and checked again during confirmation, and valid receiver paths continue through publication while malformed paths remain rejected and logged.

Important Files Changed

FilenameOverview
Bitkit/Services/PrivatePaykitService+Contacts.swiftStops a malformed receiver marker from becoming the aggregate publication error while retaining logging and processing valid receiver paths.
Bitkit/ViewModels/AppViewModel.swiftUses the incoming request amount for initial Lightning and on-chain affordability checks, matching the amount later injected into payment execution.
changelog.d/next/paykit-request-qa.fixed.mdAccurately documents the malformed-marker isolation and insufficient-balance handling fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Paykit request] --> B[Resolve private amountless endpoint]
B --> C[Decode Lightning, on-chain, or unified target]
C --> D[Read amount from incoming request]
D --> E{Preferred route has sufficient balance?}
E -->|Yes| F[Open shared send flow]
E -->|No, unified request| G{On-chain balance sufficient?}
G -->|Yes| F
G -->|No| H[Show insufficient balance message]
E -->|No, single route| H
F --> I[Inject request amount]
I --> J[Confirm exact request amount]
J --> K[Pay]
Loading

Reviews (1): Last reviewed commit: "fix: harden Paykit contact payments" | Re-trigger Greptile

@piotr-iohk

piotr-iohk commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Retested after 97033fe.

Enable — still fails. ❌

Toggle Enable payments with contactsPrivate Paykit is not available, toggle does not stay on. Same staging contact as the Android QA (pubky1jy146y751hkgqk9p69ds5cqf461aj5x7c8ptbca5rw6m9asotso). Recording + logs attached.

This is not the malformed-marker isolation path. enable() still calls preparePrivateEndpoints(..., requireImmediate: true) before setting publishingEnabledKey. canPublishPrivateEndpoints requires that flag already true, so any saved contact fail-closes as privateUnavailable.

Same surface on Android #1172 (toast there is Unknown error).

Steps:

  1. Paykit UI on. Profile exists. Staging pubky above saved as a contact (need not be live).
  2. Payments with contacts off.
  3. Enable payments with contacts.
  4. Toast is Private Paykit is not available; toggle stays off.
Screen.Recording.2026-08-27.at.10.25.35.-.ios.mov

bitkit_logs_2026-08-27_08-28-24.zip

Over-balance — copy is fixed, toast loops. ❌

Pay on a request larger than the wallet now shows the right message (Insufficient Savings, e.g. ₿ 26,717 more needed) instead of coin selection. On Android that toast fires once. On iOS it keeps coming back. Tapping Pay again then shows The payment request is already being processed. Recording attached.

validateOnchainBalance toasts and returns; presentNextIncomingPaykitPaymentRequest then deferPresentation (2s × 14). Each retry runs handleScannedData again and toasts again. Android’s equivalent is clearActiveContactPaymentContext(retryIncomingRequest = false) after the insufficient toast — no retry.

Steps:

  1. Two live wallets, payments with contacts on. Payer under the request amount (split savings + spending is fine).
  2. Other wallet requests more than that total.
  3. On iOS, tap Pay on the incoming request / See All row.
  4. Insufficient Savings toast repeats. A second Pay can show already being processed.
Screen.Recording.2026-08-27.at.11.14.21-os.overbalance.mov

bitkit_logs_2026-08-27_09-15-42-overbalance.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed both retest findings in 5ba7ee7. Private publication is now enabled before immediate endpoint preparation, with rollback still handled by the existing failure path. An unaffordable request is marked surfaced after the single balance toast, so it remains in the bell/history queue without the 2-second automatic retry loop or already-processing follow-up. The focused iOS suites pass 56/56 tests.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unaffordable incoming request path and the malformed-marker isolation both lack tests that would fail if those behaviors regressed.

  1. handleScannedData now uses incomingPaymentRequest.amountSats and the insufficient-balance path marks the request presented so it does not retry. No test drives an unaffordable request through that routing.
  2. A malformed private receiver marker no longer assigns firstError, so other contacts can still publish. No fixture would fail if that assignment returned.

Inline comments have the specific coverage to add.

Comment threadBitkit/ViewModels/AppViewModel.swift
Comment threadBitkit/Services/PrivatePaykitService+Contacts.swift
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Retested 112fb70.

Over-balance: fixed. One insufficient-capacity toast, no retry loop. ✅

Enable: still broken. Five toggles failed with privateUnavailable ❌
(09:41:49, 09:41:57, 09:46:04, 09:46:17, 09:46:25). Setting
publishingEnabledKey first did not help.

Logs never reach inspect/prepare/queue/deliver. 09:46:04 is
pubky_secret_key loaded → enable ERROR in 2ms, so this is still
canPublishPrivateEndpoints == false + requireImmediate + a saved
contact, then restore() rolls the toggle off.

Also still had pubky8u997gg after deleting pubky1jy146y, so this
is not specific to that staging contact.

Need the failing canPublish clause logged, and enable should not
fail-close when publication never actually ran.

Screen.Recording.2026-09-01.at.11.58.31.mov

bitkit_logs_2026-09-01_09-46-59.zip

@ben-kaufman
ben-kaufmanforce-pushed the codex/paykit-request-qa-fixes branch from 112fb70 to 154b3d6CompareSeptember 1, 2026 12:32
@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed the remaining enable failure in 2d90e0e. Private endpoint preparation during enable is now best effort, so temporary wallet, node, app, or Pubky unreadiness defers publication instead of returning privateUnavailable and rolling the toggle back. Real errors after publication starts still fail and restore the previous state. The readiness check now logs the exact unavailable condition. ContactPaymentsServiceTests and PrivatePaykitServiceTests pass 24 of 24. @piotr-iohk please retest the Enable case, and @ovitrif please re-review the updated head.

@piotr-iohk

piotr-iohk commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Retested latest revision (2d90e0e), iPhone 17 sim.

Enable still fail-closes. Toggle → Failed to update contact payments: privateUnavailable (GeneralSettingsView.updateContactPayments line 180) → “Private Paykit is not available.”

2d90e0e is on this build — startup logged Deferring private Paykit endpoint publication during prepare: private publication is disabled (flag off, 0 contacts). On the enable attempt itself there was no second defer: flag was set, canPublish passed, then it still threw. Sequence: loaded 1 contact (pubkypwuhkhj…), then the error ~11s later. No Failed to inspect/prepare/sync WARN around the fail.

Hypothesis: requireImmediatePublication: false on enable is bypassed by the unguarded return error after receiverPathSelection in syncLocalEndpointPublicationLocked. Other failures on that path are supposed to defer.

Android enable stays on after the matching defer change.

Logs:
bitkit_logs_2026-09-01_15-09-55.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

You were right about the remaining fail-closed path. The receiverPathSelection catch returned immediately before the deferred-publication policy was applied. Signed commit ab3308b8 now records/logs that per-contact error, continues reconciliation, and returns it only for immediate publication.

The regression test covers this exact selection failure, and the focused private/contact/request suites pass 74/74. I also installed this exact head over the saved-contact QA state, enabled Paykit UI, and verified the toggle stayed enabled past 15 seconds with the success toast and no privateUnavailable rollback. Please retest ab3308b8 after the fresh checks finish.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ben-kaufman@piotr-iohk@ovitrif
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: harden Paykit request handling - #684

Open
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes
Open

fix: harden Paykit request handling#684
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

This PR hardens incoming Paykit contact-payment handling after QA on the cross-platform payment-request flow.

Description

  1. Keeps one contact's malformed private receiver marker from blocking endpoint publication for otherwise valid contacts, while continuing to reject the malformed marker itself.
  2. Validates incoming request amounts during initial invoice routing, so unaffordable requests show the standard insufficient Spending or Savings balance message instead of reaching coin selection with a generic error.

No Paykit SDK or binding change is required. Companion Android fix: synonymdev/bitkit-android#1172

Linked Issues/Tasks

Screenshot / Video

N/A — this corrects error handling in the existing payment-request flow.

QA Notes

Manual Tests

  • 1. Saved contacts include one malformed remote Paykit marker → enable Contact Payments: valid contacts publish their private payment details and the malformed marker remains unusable.
  • 2a. Incoming LN payment request exceeds Spending balance → open request: the insufficient Spending message appears instead of coin selection.
    • 2b. Incoming on-chain payment request exceeds Savings balance → open request: the insufficient Savings message appears instead of coin selection.

Automated Checks

  • PrivatePaykitServiceTests: 15 tests passed on an iPhone 16 simulator.
  • The Debug simulator test build, SwiftFormat lint, and git diff --check passed.

@greptile-apps

Copy link
Copy Markdown

Greptile Summary

This PR hardens incoming Paykit contact-payment handling by isolating malformed receiver markers and validating request amounts against the balance before entering the send flow.

  • Allows valid contacts’ private endpoints to publish when another contact has a malformed receiver marker.
  • Uses the incoming Paykit request amount for Lightning and on-chain preflight balance checks.
  • Adds a changelog entry describing the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed preflight checks aligned to the amount ultimately paid and malformed markers safely isolated from valid contacts.

Private Paykit endpoints are intentionally amountless, the incoming request amount is injected into payment execution and checked again during confirmation, and valid receiver paths continue through publication while malformed paths remain rejected and logged.

Important Files Changed

FilenameOverview
Bitkit/Services/PrivatePaykitService+Contacts.swiftStops a malformed receiver marker from becoming the aggregate publication error while retaining logging and processing valid receiver paths.
Bitkit/ViewModels/AppViewModel.swiftUses the incoming request amount for initial Lightning and on-chain affordability checks, matching the amount later injected into payment execution.
changelog.d/next/paykit-request-qa.fixed.mdAccurately documents the malformed-marker isolation and insufficient-balance handling fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Paykit request] --> B[Resolve private amountless endpoint]
B --> C[Decode Lightning, on-chain, or unified target]
C --> D[Read amount from incoming request]
D --> E{Preferred route has sufficient balance?}
E -->|Yes| F[Open shared send flow]
E -->|No, unified request| G{On-chain balance sufficient?}
G -->|Yes| F
G -->|No| H[Show insufficient balance message]
E -->|No, single route| H
F --> I[Inject request amount]
I --> J[Confirm exact request amount]
J --> K[Pay]
Loading

Reviews (1): Last reviewed commit: "fix: harden Paykit contact payments" | Re-trigger Greptile

@piotr-iohk

piotr-iohk commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Retested after 97033fe.

Enable — still fails. ❌

Toggle Enable payments with contactsPrivate Paykit is not available, toggle does not stay on. Same staging contact as the Android QA (pubky1jy146y751hkgqk9p69ds5cqf461aj5x7c8ptbca5rw6m9asotso). Recording + logs attached.

This is not the malformed-marker isolation path. enable() still calls preparePrivateEndpoints(..., requireImmediate: true) before setting publishingEnabledKey. canPublishPrivateEndpoints requires that flag already true, so any saved contact fail-closes as privateUnavailable.

Same surface on Android #1172 (toast there is Unknown error).

Steps:

  1. Paykit UI on. Profile exists. Staging pubky above saved as a contact (need not be live).
  2. Payments with contacts off.
  3. Enable payments with contacts.
  4. Toast is Private Paykit is not available; toggle stays off.
Screen.Recording.2026-08-27.at.10.25.35.-.ios.mov

bitkit_logs_2026-08-27_08-28-24.zip

Over-balance — copy is fixed, toast loops. ❌

Pay on a request larger than the wallet now shows the right message (Insufficient Savings, e.g. ₿ 26,717 more needed) instead of coin selection. On Android that toast fires once. On iOS it keeps coming back. Tapping Pay again then shows The payment request is already being processed. Recording attached.

validateOnchainBalance toasts and returns; presentNextIncomingPaykitPaymentRequest then deferPresentation (2s × 14). Each retry runs handleScannedData again and toasts again. Android’s equivalent is clearActiveContactPaymentContext(retryIncomingRequest = false) after the insufficient toast — no retry.

Steps:

  1. Two live wallets, payments with contacts on. Payer under the request amount (split savings + spending is fine).
  2. Other wallet requests more than that total.
  3. On iOS, tap Pay on the incoming request / See All row.
  4. Insufficient Savings toast repeats. A second Pay can show already being processed.
Screen.Recording.2026-08-27.at.11.14.21-os.overbalance.mov

bitkit_logs_2026-08-27_09-15-42-overbalance.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed both retest findings in 5ba7ee7. Private publication is now enabled before immediate endpoint preparation, with rollback still handled by the existing failure path. An unaffordable request is marked surfaced after the single balance toast, so it remains in the bell/history queue without the 2-second automatic retry loop or already-processing follow-up. The focused iOS suites pass 56/56 tests.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unaffordable incoming request path and the malformed-marker isolation both lack tests that would fail if those behaviors regressed.

  1. handleScannedData now uses incomingPaymentRequest.amountSats and the insufficient-balance path marks the request presented so it does not retry. No test drives an unaffordable request through that routing.
  2. A malformed private receiver marker no longer assigns firstError, so other contacts can still publish. No fixture would fail if that assignment returned.

Inline comments have the specific coverage to add.

Comment threadBitkit/ViewModels/AppViewModel.swift
Comment threadBitkit/Services/PrivatePaykitService+Contacts.swift
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Retested 112fb70.

Over-balance: fixed. One insufficient-capacity toast, no retry loop. ✅

Enable: still broken. Five toggles failed with privateUnavailable ❌
(09:41:49, 09:41:57, 09:46:04, 09:46:17, 09:46:25). Setting
publishingEnabledKey first did not help.

Logs never reach inspect/prepare/queue/deliver. 09:46:04 is
pubky_secret_key loaded → enable ERROR in 2ms, so this is still
canPublishPrivateEndpoints == false + requireImmediate + a saved
contact, then restore() rolls the toggle off.

Also still had pubky8u997gg after deleting pubky1jy146y, so this
is not specific to that staging contact.

Need the failing canPublish clause logged, and enable should not
fail-close when publication never actually ran.

Screen.Recording.2026-09-01.at.11.58.31.mov

bitkit_logs_2026-09-01_09-46-59.zip

@ben-kaufman
ben-kaufmanforce-pushed the codex/paykit-request-qa-fixes branch from 112fb70 to 154b3d6CompareSeptember 1, 2026 12:32
@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed the remaining enable failure in 2d90e0e. Private endpoint preparation during enable is now best effort, so temporary wallet, node, app, or Pubky unreadiness defers publication instead of returning privateUnavailable and rolling the toggle back. Real errors after publication starts still fail and restore the previous state. The readiness check now logs the exact unavailable condition. ContactPaymentsServiceTests and PrivatePaykitServiceTests pass 24 of 24. @piotr-iohk please retest the Enable case, and @ovitrif please re-review the updated head.

@piotr-iohk

piotr-iohk commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Retested latest revision (2d90e0e), iPhone 17 sim.

Enable still fail-closes. Toggle → Failed to update contact payments: privateUnavailable (GeneralSettingsView.updateContactPayments line 180) → “Private Paykit is not available.”

2d90e0e is on this build — startup logged Deferring private Paykit endpoint publication during prepare: private publication is disabled (flag off, 0 contacts). On the enable attempt itself there was no second defer: flag was set, canPublish passed, then it still threw. Sequence: loaded 1 contact (pubkypwuhkhj…), then the error ~11s later. No Failed to inspect/prepare/sync WARN around the fail.

Hypothesis: requireImmediatePublication: false on enable is bypassed by the unguarded return error after receiverPathSelection in syncLocalEndpointPublicationLocked. Other failures on that path are supposed to defer.

Android enable stays on after the matching defer change.

Logs:
bitkit_logs_2026-09-01_15-09-55.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

You were right about the remaining fail-closed path. The receiverPathSelection catch returned immediately before the deferred-publication policy was applied. Signed commit ab3308b8 now records/logs that per-contact error, continues reconciliation, and returns it only for immediate publication.

The regression test covers this exact selection failure, and the focused private/contact/request suites pass 74/74. I also installed this exact head over the saved-contact QA state, enabled Paykit UI, and verified the toggle stayed enabled past 15 seconds with the success toast and no privateUnavailable rollback. Please retest ab3308b8 after the fresh checks finish.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ben-kaufman@piotr-iohk@ovitrif
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: harden Paykit request handling - #684

Open
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes
Open

fix: harden Paykit request handling#684
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

This PR hardens incoming Paykit contact-payment handling after QA on the cross-platform payment-request flow.

Description

  1. Keeps one contact's malformed private receiver marker from blocking endpoint publication for otherwise valid contacts, while continuing to reject the malformed marker itself.
  2. Validates incoming request amounts during initial invoice routing, so unaffordable requests show the standard insufficient Spending or Savings balance message instead of reaching coin selection with a generic error.

No Paykit SDK or binding change is required. Companion Android fix: synonymdev/bitkit-android#1172

Linked Issues/Tasks

Screenshot / Video

N/A — this corrects error handling in the existing payment-request flow.

QA Notes

Manual Tests

  • 1. Saved contacts include one malformed remote Paykit marker → enable Contact Payments: valid contacts publish their private payment details and the malformed marker remains unusable.
  • 2a. Incoming LN payment request exceeds Spending balance → open request: the insufficient Spending message appears instead of coin selection.
    • 2b. Incoming on-chain payment request exceeds Savings balance → open request: the insufficient Savings message appears instead of coin selection.

Automated Checks

  • PrivatePaykitServiceTests: 15 tests passed on an iPhone 16 simulator.
  • The Debug simulator test build, SwiftFormat lint, and git diff --check passed.

@greptile-apps

Copy link
Copy Markdown

Greptile Summary

This PR hardens incoming Paykit contact-payment handling by isolating malformed receiver markers and validating request amounts against the balance before entering the send flow.

  • Allows valid contacts’ private endpoints to publish when another contact has a malformed receiver marker.
  • Uses the incoming Paykit request amount for Lightning and on-chain preflight balance checks.
  • Adds a changelog entry describing the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed preflight checks aligned to the amount ultimately paid and malformed markers safely isolated from valid contacts.

Private Paykit endpoints are intentionally amountless, the incoming request amount is injected into payment execution and checked again during confirmation, and valid receiver paths continue through publication while malformed paths remain rejected and logged.

Important Files Changed

FilenameOverview
Bitkit/Services/PrivatePaykitService+Contacts.swiftStops a malformed receiver marker from becoming the aggregate publication error while retaining logging and processing valid receiver paths.
Bitkit/ViewModels/AppViewModel.swiftUses the incoming request amount for initial Lightning and on-chain affordability checks, matching the amount later injected into payment execution.
changelog.d/next/paykit-request-qa.fixed.mdAccurately documents the malformed-marker isolation and insufficient-balance handling fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Paykit request] --> B[Resolve private amountless endpoint]
B --> C[Decode Lightning, on-chain, or unified target]
C --> D[Read amount from incoming request]
D --> E{Preferred route has sufficient balance?}
E -->|Yes| F[Open shared send flow]
E -->|No, unified request| G{On-chain balance sufficient?}
G -->|Yes| F
G -->|No| H[Show insufficient balance message]
E -->|No, single route| H
F --> I[Inject request amount]
I --> J[Confirm exact request amount]
J --> K[Pay]
Loading

Reviews (1): Last reviewed commit: "fix: harden Paykit contact payments" | Re-trigger Greptile

@piotr-iohk

piotr-iohk commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Retested after 97033fe.

Enable — still fails. ❌

Toggle Enable payments with contactsPrivate Paykit is not available, toggle does not stay on. Same staging contact as the Android QA (pubky1jy146y751hkgqk9p69ds5cqf461aj5x7c8ptbca5rw6m9asotso). Recording + logs attached.

This is not the malformed-marker isolation path. enable() still calls preparePrivateEndpoints(..., requireImmediate: true) before setting publishingEnabledKey. canPublishPrivateEndpoints requires that flag already true, so any saved contact fail-closes as privateUnavailable.

Same surface on Android #1172 (toast there is Unknown error).

Steps:

  1. Paykit UI on. Profile exists. Staging pubky above saved as a contact (need not be live).
  2. Payments with contacts off.
  3. Enable payments with contacts.
  4. Toast is Private Paykit is not available; toggle stays off.
Screen.Recording.2026-08-27.at.10.25.35.-.ios.mov

bitkit_logs_2026-08-27_08-28-24.zip

Over-balance — copy is fixed, toast loops. ❌

Pay on a request larger than the wallet now shows the right message (Insufficient Savings, e.g. ₿ 26,717 more needed) instead of coin selection. On Android that toast fires once. On iOS it keeps coming back. Tapping Pay again then shows The payment request is already being processed. Recording attached.

validateOnchainBalance toasts and returns; presentNextIncomingPaykitPaymentRequest then deferPresentation (2s × 14). Each retry runs handleScannedData again and toasts again. Android’s equivalent is clearActiveContactPaymentContext(retryIncomingRequest = false) after the insufficient toast — no retry.

Steps:

  1. Two live wallets, payments with contacts on. Payer under the request amount (split savings + spending is fine).
  2. Other wallet requests more than that total.
  3. On iOS, tap Pay on the incoming request / See All row.
  4. Insufficient Savings toast repeats. A second Pay can show already being processed.
Screen.Recording.2026-08-27.at.11.14.21-os.overbalance.mov

bitkit_logs_2026-08-27_09-15-42-overbalance.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed both retest findings in 5ba7ee7. Private publication is now enabled before immediate endpoint preparation, with rollback still handled by the existing failure path. An unaffordable request is marked surfaced after the single balance toast, so it remains in the bell/history queue without the 2-second automatic retry loop or already-processing follow-up. The focused iOS suites pass 56/56 tests.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unaffordable incoming request path and the malformed-marker isolation both lack tests that would fail if those behaviors regressed.

  1. handleScannedData now uses incomingPaymentRequest.amountSats and the insufficient-balance path marks the request presented so it does not retry. No test drives an unaffordable request through that routing.
  2. A malformed private receiver marker no longer assigns firstError, so other contacts can still publish. No fixture would fail if that assignment returned.

Inline comments have the specific coverage to add.

Comment threadBitkit/ViewModels/AppViewModel.swift
Comment threadBitkit/Services/PrivatePaykitService+Contacts.swift
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Retested 112fb70.

Over-balance: fixed. One insufficient-capacity toast, no retry loop. ✅

Enable: still broken. Five toggles failed with privateUnavailable ❌
(09:41:49, 09:41:57, 09:46:04, 09:46:17, 09:46:25). Setting
publishingEnabledKey first did not help.

Logs never reach inspect/prepare/queue/deliver. 09:46:04 is
pubky_secret_key loaded → enable ERROR in 2ms, so this is still
canPublishPrivateEndpoints == false + requireImmediate + a saved
contact, then restore() rolls the toggle off.

Also still had pubky8u997gg after deleting pubky1jy146y, so this
is not specific to that staging contact.

Need the failing canPublish clause logged, and enable should not
fail-close when publication never actually ran.

Screen.Recording.2026-09-01.at.11.58.31.mov

bitkit_logs_2026-09-01_09-46-59.zip

@ben-kaufman
ben-kaufmanforce-pushed the codex/paykit-request-qa-fixes branch from 112fb70 to 154b3d6CompareSeptember 1, 2026 12:32
@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed the remaining enable failure in 2d90e0e. Private endpoint preparation during enable is now best effort, so temporary wallet, node, app, or Pubky unreadiness defers publication instead of returning privateUnavailable and rolling the toggle back. Real errors after publication starts still fail and restore the previous state. The readiness check now logs the exact unavailable condition. ContactPaymentsServiceTests and PrivatePaykitServiceTests pass 24 of 24. @piotr-iohk please retest the Enable case, and @ovitrif please re-review the updated head.

@piotr-iohk

piotr-iohk commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Retested latest revision (2d90e0e), iPhone 17 sim.

Enable still fail-closes. Toggle → Failed to update contact payments: privateUnavailable (GeneralSettingsView.updateContactPayments line 180) → “Private Paykit is not available.”

2d90e0e is on this build — startup logged Deferring private Paykit endpoint publication during prepare: private publication is disabled (flag off, 0 contacts). On the enable attempt itself there was no second defer: flag was set, canPublish passed, then it still threw. Sequence: loaded 1 contact (pubkypwuhkhj…), then the error ~11s later. No Failed to inspect/prepare/sync WARN around the fail.

Hypothesis: requireImmediatePublication: false on enable is bypassed by the unguarded return error after receiverPathSelection in syncLocalEndpointPublicationLocked. Other failures on that path are supposed to defer.

Android enable stays on after the matching defer change.

Logs:
bitkit_logs_2026-09-01_15-09-55.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

You were right about the remaining fail-closed path. The receiverPathSelection catch returned immediately before the deferred-publication policy was applied. Signed commit ab3308b8 now records/logs that per-contact error, continues reconciliation, and returns it only for immediate publication.

The regression test covers this exact selection failure, and the focused private/contact/request suites pass 74/74. I also installed this exact head over the saved-contact QA state, enabled Paykit UI, and verified the toggle stayed enabled past 15 seconds with the success toast and no privateUnavailable rollback. Please retest ab3308b8 after the fresh checks finish.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ben-kaufman@piotr-iohk@ovitrif
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: harden Paykit request handling - #684

Open
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes
Open

fix: harden Paykit request handling#684
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

This PR hardens incoming Paykit contact-payment handling after QA on the cross-platform payment-request flow.

Description

  1. Keeps one contact's malformed private receiver marker from blocking endpoint publication for otherwise valid contacts, while continuing to reject the malformed marker itself.
  2. Validates incoming request amounts during initial invoice routing, so unaffordable requests show the standard insufficient Spending or Savings balance message instead of reaching coin selection with a generic error.

No Paykit SDK or binding change is required. Companion Android fix: synonymdev/bitkit-android#1172

Linked Issues/Tasks

Screenshot / Video

N/A — this corrects error handling in the existing payment-request flow.

QA Notes

Manual Tests

  • 1. Saved contacts include one malformed remote Paykit marker → enable Contact Payments: valid contacts publish their private payment details and the malformed marker remains unusable.
  • 2a. Incoming LN payment request exceeds Spending balance → open request: the insufficient Spending message appears instead of coin selection.
    • 2b. Incoming on-chain payment request exceeds Savings balance → open request: the insufficient Savings message appears instead of coin selection.

Automated Checks

  • PrivatePaykitServiceTests: 15 tests passed on an iPhone 16 simulator.
  • The Debug simulator test build, SwiftFormat lint, and git diff --check passed.

@greptile-apps

Copy link
Copy Markdown

Greptile Summary

This PR hardens incoming Paykit contact-payment handling by isolating malformed receiver markers and validating request amounts against the balance before entering the send flow.

  • Allows valid contacts’ private endpoints to publish when another contact has a malformed receiver marker.
  • Uses the incoming Paykit request amount for Lightning and on-chain preflight balance checks.
  • Adds a changelog entry describing the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed preflight checks aligned to the amount ultimately paid and malformed markers safely isolated from valid contacts.

Private Paykit endpoints are intentionally amountless, the incoming request amount is injected into payment execution and checked again during confirmation, and valid receiver paths continue through publication while malformed paths remain rejected and logged.

Important Files Changed

FilenameOverview
Bitkit/Services/PrivatePaykitService+Contacts.swiftStops a malformed receiver marker from becoming the aggregate publication error while retaining logging and processing valid receiver paths.
Bitkit/ViewModels/AppViewModel.swiftUses the incoming request amount for initial Lightning and on-chain affordability checks, matching the amount later injected into payment execution.
changelog.d/next/paykit-request-qa.fixed.mdAccurately documents the malformed-marker isolation and insufficient-balance handling fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Paykit request] --> B[Resolve private amountless endpoint]
B --> C[Decode Lightning, on-chain, or unified target]
C --> D[Read amount from incoming request]
D --> E{Preferred route has sufficient balance?}
E -->|Yes| F[Open shared send flow]
E -->|No, unified request| G{On-chain balance sufficient?}
G -->|Yes| F
G -->|No| H[Show insufficient balance message]
E -->|No, single route| H
F --> I[Inject request amount]
I --> J[Confirm exact request amount]
J --> K[Pay]
Loading

Reviews (1): Last reviewed commit: "fix: harden Paykit contact payments" | Re-trigger Greptile

@piotr-iohk

piotr-iohk commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Retested after 97033fe.

Enable — still fails. ❌

Toggle Enable payments with contactsPrivate Paykit is not available, toggle does not stay on. Same staging contact as the Android QA (pubky1jy146y751hkgqk9p69ds5cqf461aj5x7c8ptbca5rw6m9asotso). Recording + logs attached.

This is not the malformed-marker isolation path. enable() still calls preparePrivateEndpoints(..., requireImmediate: true) before setting publishingEnabledKey. canPublishPrivateEndpoints requires that flag already true, so any saved contact fail-closes as privateUnavailable.

Same surface on Android #1172 (toast there is Unknown error).

Steps:

  1. Paykit UI on. Profile exists. Staging pubky above saved as a contact (need not be live).
  2. Payments with contacts off.
  3. Enable payments with contacts.
  4. Toast is Private Paykit is not available; toggle stays off.
Screen.Recording.2026-08-27.at.10.25.35.-.ios.mov

bitkit_logs_2026-08-27_08-28-24.zip

Over-balance — copy is fixed, toast loops. ❌

Pay on a request larger than the wallet now shows the right message (Insufficient Savings, e.g. ₿ 26,717 more needed) instead of coin selection. On Android that toast fires once. On iOS it keeps coming back. Tapping Pay again then shows The payment request is already being processed. Recording attached.

validateOnchainBalance toasts and returns; presentNextIncomingPaykitPaymentRequest then deferPresentation (2s × 14). Each retry runs handleScannedData again and toasts again. Android’s equivalent is clearActiveContactPaymentContext(retryIncomingRequest = false) after the insufficient toast — no retry.

Steps:

  1. Two live wallets, payments with contacts on. Payer under the request amount (split savings + spending is fine).
  2. Other wallet requests more than that total.
  3. On iOS, tap Pay on the incoming request / See All row.
  4. Insufficient Savings toast repeats. A second Pay can show already being processed.
Screen.Recording.2026-08-27.at.11.14.21-os.overbalance.mov

bitkit_logs_2026-08-27_09-15-42-overbalance.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed both retest findings in 5ba7ee7. Private publication is now enabled before immediate endpoint preparation, with rollback still handled by the existing failure path. An unaffordable request is marked surfaced after the single balance toast, so it remains in the bell/history queue without the 2-second automatic retry loop or already-processing follow-up. The focused iOS suites pass 56/56 tests.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unaffordable incoming request path and the malformed-marker isolation both lack tests that would fail if those behaviors regressed.

  1. handleScannedData now uses incomingPaymentRequest.amountSats and the insufficient-balance path marks the request presented so it does not retry. No test drives an unaffordable request through that routing.
  2. A malformed private receiver marker no longer assigns firstError, so other contacts can still publish. No fixture would fail if that assignment returned.

Inline comments have the specific coverage to add.

Comment threadBitkit/ViewModels/AppViewModel.swift
Comment threadBitkit/Services/PrivatePaykitService+Contacts.swift
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Retested 112fb70.

Over-balance: fixed. One insufficient-capacity toast, no retry loop. ✅

Enable: still broken. Five toggles failed with privateUnavailable ❌
(09:41:49, 09:41:57, 09:46:04, 09:46:17, 09:46:25). Setting
publishingEnabledKey first did not help.

Logs never reach inspect/prepare/queue/deliver. 09:46:04 is
pubky_secret_key loaded → enable ERROR in 2ms, so this is still
canPublishPrivateEndpoints == false + requireImmediate + a saved
contact, then restore() rolls the toggle off.

Also still had pubky8u997gg after deleting pubky1jy146y, so this
is not specific to that staging contact.

Need the failing canPublish clause logged, and enable should not
fail-close when publication never actually ran.

Screen.Recording.2026-09-01.at.11.58.31.mov

bitkit_logs_2026-09-01_09-46-59.zip

@ben-kaufman
ben-kaufmanforce-pushed the codex/paykit-request-qa-fixes branch from 112fb70 to 154b3d6CompareSeptember 1, 2026 12:32
@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed the remaining enable failure in 2d90e0e. Private endpoint preparation during enable is now best effort, so temporary wallet, node, app, or Pubky unreadiness defers publication instead of returning privateUnavailable and rolling the toggle back. Real errors after publication starts still fail and restore the previous state. The readiness check now logs the exact unavailable condition. ContactPaymentsServiceTests and PrivatePaykitServiceTests pass 24 of 24. @piotr-iohk please retest the Enable case, and @ovitrif please re-review the updated head.

@piotr-iohk

piotr-iohk commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Retested latest revision (2d90e0e), iPhone 17 sim.

Enable still fail-closes. Toggle → Failed to update contact payments: privateUnavailable (GeneralSettingsView.updateContactPayments line 180) → “Private Paykit is not available.”

2d90e0e is on this build — startup logged Deferring private Paykit endpoint publication during prepare: private publication is disabled (flag off, 0 contacts). On the enable attempt itself there was no second defer: flag was set, canPublish passed, then it still threw. Sequence: loaded 1 contact (pubkypwuhkhj…), then the error ~11s later. No Failed to inspect/prepare/sync WARN around the fail.

Hypothesis: requireImmediatePublication: false on enable is bypassed by the unguarded return error after receiverPathSelection in syncLocalEndpointPublicationLocked. Other failures on that path are supposed to defer.

Android enable stays on after the matching defer change.

Logs:
bitkit_logs_2026-09-01_15-09-55.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

You were right about the remaining fail-closed path. The receiverPathSelection catch returned immediately before the deferred-publication policy was applied. Signed commit ab3308b8 now records/logs that per-contact error, continues reconciliation, and returns it only for immediate publication.

The regression test covers this exact selection failure, and the focused private/contact/request suites pass 74/74. I also installed this exact head over the saved-contact QA state, enabled Paykit UI, and verified the toggle stayed enabled past 15 seconds with the success toast and no privateUnavailable rollback. Please retest ab3308b8 after the fresh checks finish.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ben-kaufman@piotr-iohk@ovitrif
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: harden Paykit request handling - #684

Open
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes
Open

fix: harden Paykit request handling#684
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

This PR hardens incoming Paykit contact-payment handling after QA on the cross-platform payment-request flow.

Description

  1. Keeps one contact's malformed private receiver marker from blocking endpoint publication for otherwise valid contacts, while continuing to reject the malformed marker itself.
  2. Validates incoming request amounts during initial invoice routing, so unaffordable requests show the standard insufficient Spending or Savings balance message instead of reaching coin selection with a generic error.

No Paykit SDK or binding change is required. Companion Android fix: synonymdev/bitkit-android#1172

Linked Issues/Tasks

Screenshot / Video

N/A — this corrects error handling in the existing payment-request flow.

QA Notes

Manual Tests

  • 1. Saved contacts include one malformed remote Paykit marker → enable Contact Payments: valid contacts publish their private payment details and the malformed marker remains unusable.
  • 2a. Incoming LN payment request exceeds Spending balance → open request: the insufficient Spending message appears instead of coin selection.
    • 2b. Incoming on-chain payment request exceeds Savings balance → open request: the insufficient Savings message appears instead of coin selection.

Automated Checks

  • PrivatePaykitServiceTests: 15 tests passed on an iPhone 16 simulator.
  • The Debug simulator test build, SwiftFormat lint, and git diff --check passed.

@greptile-apps

Copy link
Copy Markdown

Greptile Summary

This PR hardens incoming Paykit contact-payment handling by isolating malformed receiver markers and validating request amounts against the balance before entering the send flow.

  • Allows valid contacts’ private endpoints to publish when another contact has a malformed receiver marker.
  • Uses the incoming Paykit request amount for Lightning and on-chain preflight balance checks.
  • Adds a changelog entry describing the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed preflight checks aligned to the amount ultimately paid and malformed markers safely isolated from valid contacts.

Private Paykit endpoints are intentionally amountless, the incoming request amount is injected into payment execution and checked again during confirmation, and valid receiver paths continue through publication while malformed paths remain rejected and logged.

Important Files Changed

FilenameOverview
Bitkit/Services/PrivatePaykitService+Contacts.swiftStops a malformed receiver marker from becoming the aggregate publication error while retaining logging and processing valid receiver paths.
Bitkit/ViewModels/AppViewModel.swiftUses the incoming request amount for initial Lightning and on-chain affordability checks, matching the amount later injected into payment execution.
changelog.d/next/paykit-request-qa.fixed.mdAccurately documents the malformed-marker isolation and insufficient-balance handling fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Paykit request] --> B[Resolve private amountless endpoint]
B --> C[Decode Lightning, on-chain, or unified target]
C --> D[Read amount from incoming request]
D --> E{Preferred route has sufficient balance?}
E -->|Yes| F[Open shared send flow]
E -->|No, unified request| G{On-chain balance sufficient?}
G -->|Yes| F
G -->|No| H[Show insufficient balance message]
E -->|No, single route| H
F --> I[Inject request amount]
I --> J[Confirm exact request amount]
J --> K[Pay]
Loading

Reviews (1): Last reviewed commit: "fix: harden Paykit contact payments" | Re-trigger Greptile

@piotr-iohk

piotr-iohk commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Retested after 97033fe.

Enable — still fails. ❌

Toggle Enable payments with contactsPrivate Paykit is not available, toggle does not stay on. Same staging contact as the Android QA (pubky1jy146y751hkgqk9p69ds5cqf461aj5x7c8ptbca5rw6m9asotso). Recording + logs attached.

This is not the malformed-marker isolation path. enable() still calls preparePrivateEndpoints(..., requireImmediate: true) before setting publishingEnabledKey. canPublishPrivateEndpoints requires that flag already true, so any saved contact fail-closes as privateUnavailable.

Same surface on Android #1172 (toast there is Unknown error).

Steps:

  1. Paykit UI on. Profile exists. Staging pubky above saved as a contact (need not be live).
  2. Payments with contacts off.
  3. Enable payments with contacts.
  4. Toast is Private Paykit is not available; toggle stays off.
Screen.Recording.2026-08-27.at.10.25.35.-.ios.mov

bitkit_logs_2026-08-27_08-28-24.zip

Over-balance — copy is fixed, toast loops. ❌

Pay on a request larger than the wallet now shows the right message (Insufficient Savings, e.g. ₿ 26,717 more needed) instead of coin selection. On Android that toast fires once. On iOS it keeps coming back. Tapping Pay again then shows The payment request is already being processed. Recording attached.

validateOnchainBalance toasts and returns; presentNextIncomingPaykitPaymentRequest then deferPresentation (2s × 14). Each retry runs handleScannedData again and toasts again. Android’s equivalent is clearActiveContactPaymentContext(retryIncomingRequest = false) after the insufficient toast — no retry.

Steps:

  1. Two live wallets, payments with contacts on. Payer under the request amount (split savings + spending is fine).
  2. Other wallet requests more than that total.
  3. On iOS, tap Pay on the incoming request / See All row.
  4. Insufficient Savings toast repeats. A second Pay can show already being processed.
Screen.Recording.2026-08-27.at.11.14.21-os.overbalance.mov

bitkit_logs_2026-08-27_09-15-42-overbalance.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed both retest findings in 5ba7ee7. Private publication is now enabled before immediate endpoint preparation, with rollback still handled by the existing failure path. An unaffordable request is marked surfaced after the single balance toast, so it remains in the bell/history queue without the 2-second automatic retry loop or already-processing follow-up. The focused iOS suites pass 56/56 tests.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unaffordable incoming request path and the malformed-marker isolation both lack tests that would fail if those behaviors regressed.

  1. handleScannedData now uses incomingPaymentRequest.amountSats and the insufficient-balance path marks the request presented so it does not retry. No test drives an unaffordable request through that routing.
  2. A malformed private receiver marker no longer assigns firstError, so other contacts can still publish. No fixture would fail if that assignment returned.

Inline comments have the specific coverage to add.

Comment threadBitkit/ViewModels/AppViewModel.swift
Comment threadBitkit/Services/PrivatePaykitService+Contacts.swift
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Retested 112fb70.

Over-balance: fixed. One insufficient-capacity toast, no retry loop. ✅

Enable: still broken. Five toggles failed with privateUnavailable ❌
(09:41:49, 09:41:57, 09:46:04, 09:46:17, 09:46:25). Setting
publishingEnabledKey first did not help.

Logs never reach inspect/prepare/queue/deliver. 09:46:04 is
pubky_secret_key loaded → enable ERROR in 2ms, so this is still
canPublishPrivateEndpoints == false + requireImmediate + a saved
contact, then restore() rolls the toggle off.

Also still had pubky8u997gg after deleting pubky1jy146y, so this
is not specific to that staging contact.

Need the failing canPublish clause logged, and enable should not
fail-close when publication never actually ran.

Screen.Recording.2026-09-01.at.11.58.31.mov

bitkit_logs_2026-09-01_09-46-59.zip

@ben-kaufman
ben-kaufmanforce-pushed the codex/paykit-request-qa-fixes branch from 112fb70 to 154b3d6CompareSeptember 1, 2026 12:32
@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed the remaining enable failure in 2d90e0e. Private endpoint preparation during enable is now best effort, so temporary wallet, node, app, or Pubky unreadiness defers publication instead of returning privateUnavailable and rolling the toggle back. Real errors after publication starts still fail and restore the previous state. The readiness check now logs the exact unavailable condition. ContactPaymentsServiceTests and PrivatePaykitServiceTests pass 24 of 24. @piotr-iohk please retest the Enable case, and @ovitrif please re-review the updated head.

@piotr-iohk

piotr-iohk commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Retested latest revision (2d90e0e), iPhone 17 sim.

Enable still fail-closes. Toggle → Failed to update contact payments: privateUnavailable (GeneralSettingsView.updateContactPayments line 180) → “Private Paykit is not available.”

2d90e0e is on this build — startup logged Deferring private Paykit endpoint publication during prepare: private publication is disabled (flag off, 0 contacts). On the enable attempt itself there was no second defer: flag was set, canPublish passed, then it still threw. Sequence: loaded 1 contact (pubkypwuhkhj…), then the error ~11s later. No Failed to inspect/prepare/sync WARN around the fail.

Hypothesis: requireImmediatePublication: false on enable is bypassed by the unguarded return error after receiverPathSelection in syncLocalEndpointPublicationLocked. Other failures on that path are supposed to defer.

Android enable stays on after the matching defer change.

Logs:
bitkit_logs_2026-09-01_15-09-55.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

You were right about the remaining fail-closed path. The receiverPathSelection catch returned immediately before the deferred-publication policy was applied. Signed commit ab3308b8 now records/logs that per-contact error, continues reconciliation, and returns it only for immediate publication.

The regression test covers this exact selection failure, and the focused private/contact/request suites pass 74/74. I also installed this exact head over the saved-contact QA state, enabled Paykit UI, and verified the toggle stayed enabled past 15 seconds with the success toast and no privateUnavailable rollback. Please retest ab3308b8 after the fresh checks finish.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ben-kaufman@piotr-iohk@ovitrif
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: harden Paykit request handling - #684

Open
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes
Open

fix: harden Paykit request handling#684
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

This PR hardens incoming Paykit contact-payment handling after QA on the cross-platform payment-request flow.

Description

  1. Keeps one contact's malformed private receiver marker from blocking endpoint publication for otherwise valid contacts, while continuing to reject the malformed marker itself.
  2. Validates incoming request amounts during initial invoice routing, so unaffordable requests show the standard insufficient Spending or Savings balance message instead of reaching coin selection with a generic error.

No Paykit SDK or binding change is required. Companion Android fix: synonymdev/bitkit-android#1172

Linked Issues/Tasks

Screenshot / Video

N/A — this corrects error handling in the existing payment-request flow.

QA Notes

Manual Tests

  • 1. Saved contacts include one malformed remote Paykit marker → enable Contact Payments: valid contacts publish their private payment details and the malformed marker remains unusable.
  • 2a. Incoming LN payment request exceeds Spending balance → open request: the insufficient Spending message appears instead of coin selection.
    • 2b. Incoming on-chain payment request exceeds Savings balance → open request: the insufficient Savings message appears instead of coin selection.

Automated Checks

  • PrivatePaykitServiceTests: 15 tests passed on an iPhone 16 simulator.
  • The Debug simulator test build, SwiftFormat lint, and git diff --check passed.

@greptile-apps

Copy link
Copy Markdown

Greptile Summary

This PR hardens incoming Paykit contact-payment handling by isolating malformed receiver markers and validating request amounts against the balance before entering the send flow.

  • Allows valid contacts’ private endpoints to publish when another contact has a malformed receiver marker.
  • Uses the incoming Paykit request amount for Lightning and on-chain preflight balance checks.
  • Adds a changelog entry describing the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed preflight checks aligned to the amount ultimately paid and malformed markers safely isolated from valid contacts.

Private Paykit endpoints are intentionally amountless, the incoming request amount is injected into payment execution and checked again during confirmation, and valid receiver paths continue through publication while malformed paths remain rejected and logged.

Important Files Changed

FilenameOverview
Bitkit/Services/PrivatePaykitService+Contacts.swiftStops a malformed receiver marker from becoming the aggregate publication error while retaining logging and processing valid receiver paths.
Bitkit/ViewModels/AppViewModel.swiftUses the incoming request amount for initial Lightning and on-chain affordability checks, matching the amount later injected into payment execution.
changelog.d/next/paykit-request-qa.fixed.mdAccurately documents the malformed-marker isolation and insufficient-balance handling fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Paykit request] --> B[Resolve private amountless endpoint]
B --> C[Decode Lightning, on-chain, or unified target]
C --> D[Read amount from incoming request]
D --> E{Preferred route has sufficient balance?}
E -->|Yes| F[Open shared send flow]
E -->|No, unified request| G{On-chain balance sufficient?}
G -->|Yes| F
G -->|No| H[Show insufficient balance message]
E -->|No, single route| H
F --> I[Inject request amount]
I --> J[Confirm exact request amount]
J --> K[Pay]
Loading

Reviews (1): Last reviewed commit: "fix: harden Paykit contact payments" | Re-trigger Greptile

@piotr-iohk

piotr-iohk commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Retested after 97033fe.

Enable — still fails. ❌

Toggle Enable payments with contactsPrivate Paykit is not available, toggle does not stay on. Same staging contact as the Android QA (pubky1jy146y751hkgqk9p69ds5cqf461aj5x7c8ptbca5rw6m9asotso). Recording + logs attached.

This is not the malformed-marker isolation path. enable() still calls preparePrivateEndpoints(..., requireImmediate: true) before setting publishingEnabledKey. canPublishPrivateEndpoints requires that flag already true, so any saved contact fail-closes as privateUnavailable.

Same surface on Android #1172 (toast there is Unknown error).

Steps:

  1. Paykit UI on. Profile exists. Staging pubky above saved as a contact (need not be live).
  2. Payments with contacts off.
  3. Enable payments with contacts.
  4. Toast is Private Paykit is not available; toggle stays off.
Screen.Recording.2026-08-27.at.10.25.35.-.ios.mov

bitkit_logs_2026-08-27_08-28-24.zip

Over-balance — copy is fixed, toast loops. ❌

Pay on a request larger than the wallet now shows the right message (Insufficient Savings, e.g. ₿ 26,717 more needed) instead of coin selection. On Android that toast fires once. On iOS it keeps coming back. Tapping Pay again then shows The payment request is already being processed. Recording attached.

validateOnchainBalance toasts and returns; presentNextIncomingPaykitPaymentRequest then deferPresentation (2s × 14). Each retry runs handleScannedData again and toasts again. Android’s equivalent is clearActiveContactPaymentContext(retryIncomingRequest = false) after the insufficient toast — no retry.

Steps:

  1. Two live wallets, payments with contacts on. Payer under the request amount (split savings + spending is fine).
  2. Other wallet requests more than that total.
  3. On iOS, tap Pay on the incoming request / See All row.
  4. Insufficient Savings toast repeats. A second Pay can show already being processed.
Screen.Recording.2026-08-27.at.11.14.21-os.overbalance.mov

bitkit_logs_2026-08-27_09-15-42-overbalance.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed both retest findings in 5ba7ee7. Private publication is now enabled before immediate endpoint preparation, with rollback still handled by the existing failure path. An unaffordable request is marked surfaced after the single balance toast, so it remains in the bell/history queue without the 2-second automatic retry loop or already-processing follow-up. The focused iOS suites pass 56/56 tests.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unaffordable incoming request path and the malformed-marker isolation both lack tests that would fail if those behaviors regressed.

  1. handleScannedData now uses incomingPaymentRequest.amountSats and the insufficient-balance path marks the request presented so it does not retry. No test drives an unaffordable request through that routing.
  2. A malformed private receiver marker no longer assigns firstError, so other contacts can still publish. No fixture would fail if that assignment returned.

Inline comments have the specific coverage to add.

Comment threadBitkit/ViewModels/AppViewModel.swift
Comment threadBitkit/Services/PrivatePaykitService+Contacts.swift
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Retested 112fb70.

Over-balance: fixed. One insufficient-capacity toast, no retry loop. ✅

Enable: still broken. Five toggles failed with privateUnavailable ❌
(09:41:49, 09:41:57, 09:46:04, 09:46:17, 09:46:25). Setting
publishingEnabledKey first did not help.

Logs never reach inspect/prepare/queue/deliver. 09:46:04 is
pubky_secret_key loaded → enable ERROR in 2ms, so this is still
canPublishPrivateEndpoints == false + requireImmediate + a saved
contact, then restore() rolls the toggle off.

Also still had pubky8u997gg after deleting pubky1jy146y, so this
is not specific to that staging contact.

Need the failing canPublish clause logged, and enable should not
fail-close when publication never actually ran.

Screen.Recording.2026-09-01.at.11.58.31.mov

bitkit_logs_2026-09-01_09-46-59.zip

@ben-kaufman
ben-kaufmanforce-pushed the codex/paykit-request-qa-fixes branch from 112fb70 to 154b3d6CompareSeptember 1, 2026 12:32
@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed the remaining enable failure in 2d90e0e. Private endpoint preparation during enable is now best effort, so temporary wallet, node, app, or Pubky unreadiness defers publication instead of returning privateUnavailable and rolling the toggle back. Real errors after publication starts still fail and restore the previous state. The readiness check now logs the exact unavailable condition. ContactPaymentsServiceTests and PrivatePaykitServiceTests pass 24 of 24. @piotr-iohk please retest the Enable case, and @ovitrif please re-review the updated head.

@piotr-iohk

piotr-iohk commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Retested latest revision (2d90e0e), iPhone 17 sim.

Enable still fail-closes. Toggle → Failed to update contact payments: privateUnavailable (GeneralSettingsView.updateContactPayments line 180) → “Private Paykit is not available.”

2d90e0e is on this build — startup logged Deferring private Paykit endpoint publication during prepare: private publication is disabled (flag off, 0 contacts). On the enable attempt itself there was no second defer: flag was set, canPublish passed, then it still threw. Sequence: loaded 1 contact (pubkypwuhkhj…), then the error ~11s later. No Failed to inspect/prepare/sync WARN around the fail.

Hypothesis: requireImmediatePublication: false on enable is bypassed by the unguarded return error after receiverPathSelection in syncLocalEndpointPublicationLocked. Other failures on that path are supposed to defer.

Android enable stays on after the matching defer change.

Logs:
bitkit_logs_2026-09-01_15-09-55.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

You were right about the remaining fail-closed path. The receiverPathSelection catch returned immediately before the deferred-publication policy was applied. Signed commit ab3308b8 now records/logs that per-contact error, continues reconciliation, and returns it only for immediate publication.

The regression test covers this exact selection failure, and the focused private/contact/request suites pass 74/74. I also installed this exact head over the saved-contact QA state, enabled Paykit UI, and verified the toggle stayed enabled past 15 seconds with the success toast and no privateUnavailable rollback. Please retest ab3308b8 after the fresh checks finish.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ben-kaufman@piotr-iohk@ovitrif
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: harden Paykit request handling - #684

Open
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes
Open

fix: harden Paykit request handling#684
ben-kaufman wants to merge 6 commits into
masterfrom
codex/paykit-request-qa-fixes

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

This PR hardens incoming Paykit contact-payment handling after QA on the cross-platform payment-request flow.

Description

  1. Keeps one contact's malformed private receiver marker from blocking endpoint publication for otherwise valid contacts, while continuing to reject the malformed marker itself.
  2. Validates incoming request amounts during initial invoice routing, so unaffordable requests show the standard insufficient Spending or Savings balance message instead of reaching coin selection with a generic error.

No Paykit SDK or binding change is required. Companion Android fix: synonymdev/bitkit-android#1172

Linked Issues/Tasks

Screenshot / Video

N/A — this corrects error handling in the existing payment-request flow.

QA Notes

Manual Tests

  • 1. Saved contacts include one malformed remote Paykit marker → enable Contact Payments: valid contacts publish their private payment details and the malformed marker remains unusable.
  • 2a. Incoming LN payment request exceeds Spending balance → open request: the insufficient Spending message appears instead of coin selection.
    • 2b. Incoming on-chain payment request exceeds Savings balance → open request: the insufficient Savings message appears instead of coin selection.

Automated Checks

  • PrivatePaykitServiceTests: 15 tests passed on an iPhone 16 simulator.
  • The Debug simulator test build, SwiftFormat lint, and git diff --check passed.

@greptile-apps

Copy link
Copy Markdown

Greptile Summary

This PR hardens incoming Paykit contact-payment handling by isolating malformed receiver markers and validating request amounts against the balance before entering the send flow.

  • Allows valid contacts’ private endpoints to publish when another contact has a malformed receiver marker.
  • Uses the incoming Paykit request amount for Lightning and on-chain preflight balance checks.
  • Adds a changelog entry describing the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge, with the changed preflight checks aligned to the amount ultimately paid and malformed markers safely isolated from valid contacts.

Private Paykit endpoints are intentionally amountless, the incoming request amount is injected into payment execution and checked again during confirmation, and valid receiver paths continue through publication while malformed paths remain rejected and logged.

Important Files Changed

FilenameOverview
Bitkit/Services/PrivatePaykitService+Contacts.swiftStops a malformed receiver marker from becoming the aggregate publication error while retaining logging and processing valid receiver paths.
Bitkit/ViewModels/AppViewModel.swiftUses the incoming request amount for initial Lightning and on-chain affordability checks, matching the amount later injected into payment execution.
changelog.d/next/paykit-request-qa.fixed.mdAccurately documents the malformed-marker isolation and insufficient-balance handling fixes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming Paykit request] --> B[Resolve private amountless endpoint]
B --> C[Decode Lightning, on-chain, or unified target]
C --> D[Read amount from incoming request]
D --> E{Preferred route has sufficient balance?}
E -->|Yes| F[Open shared send flow]
E -->|No, unified request| G{On-chain balance sufficient?}
G -->|Yes| F
G -->|No| H[Show insufficient balance message]
E -->|No, single route| H
F --> I[Inject request amount]
I --> J[Confirm exact request amount]
J --> K[Pay]
Loading

Reviews (1): Last reviewed commit: "fix: harden Paykit contact payments" | Re-trigger Greptile

@piotr-iohk

piotr-iohk commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Retested after 97033fe.

Enable — still fails. ❌

Toggle Enable payments with contactsPrivate Paykit is not available, toggle does not stay on. Same staging contact as the Android QA (pubky1jy146y751hkgqk9p69ds5cqf461aj5x7c8ptbca5rw6m9asotso). Recording + logs attached.

This is not the malformed-marker isolation path. enable() still calls preparePrivateEndpoints(..., requireImmediate: true) before setting publishingEnabledKey. canPublishPrivateEndpoints requires that flag already true, so any saved contact fail-closes as privateUnavailable.

Same surface on Android #1172 (toast there is Unknown error).

Steps:

  1. Paykit UI on. Profile exists. Staging pubky above saved as a contact (need not be live).
  2. Payments with contacts off.
  3. Enable payments with contacts.
  4. Toast is Private Paykit is not available; toggle stays off.
Screen.Recording.2026-08-27.at.10.25.35.-.ios.mov

bitkit_logs_2026-08-27_08-28-24.zip

Over-balance — copy is fixed, toast loops. ❌

Pay on a request larger than the wallet now shows the right message (Insufficient Savings, e.g. ₿ 26,717 more needed) instead of coin selection. On Android that toast fires once. On iOS it keeps coming back. Tapping Pay again then shows The payment request is already being processed. Recording attached.

validateOnchainBalance toasts and returns; presentNextIncomingPaykitPaymentRequest then deferPresentation (2s × 14). Each retry runs handleScannedData again and toasts again. Android’s equivalent is clearActiveContactPaymentContext(retryIncomingRequest = false) after the insufficient toast — no retry.

Steps:

  1. Two live wallets, payments with contacts on. Payer under the request amount (split savings + spending is fine).
  2. Other wallet requests more than that total.
  3. On iOS, tap Pay on the incoming request / See All row.
  4. Insufficient Savings toast repeats. A second Pay can show already being processed.
Screen.Recording.2026-08-27.at.11.14.21-os.overbalance.mov

bitkit_logs_2026-08-27_09-15-42-overbalance.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed both retest findings in 5ba7ee7. Private publication is now enabled before immediate endpoint preparation, with rollback still handled by the existing failure path. An unaffordable request is marked surfaced after the single balance toast, so it remains in the bell/history queue without the 2-second automatic retry loop or already-processing follow-up. The focused iOS suites pass 56/56 tests.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unaffordable incoming request path and the malformed-marker isolation both lack tests that would fail if those behaviors regressed.

  1. handleScannedData now uses incomingPaymentRequest.amountSats and the insufficient-balance path marks the request presented so it does not retry. No test drives an unaffordable request through that routing.
  2. A malformed private receiver marker no longer assigns firstError, so other contacts can still publish. No fixture would fail if that assignment returned.

Inline comments have the specific coverage to add.

Comment threadBitkit/ViewModels/AppViewModel.swift
Comment threadBitkit/Services/PrivatePaykitService+Contacts.swift
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Retested 112fb70.

Over-balance: fixed. One insufficient-capacity toast, no retry loop. ✅

Enable: still broken. Five toggles failed with privateUnavailable ❌
(09:41:49, 09:41:57, 09:46:04, 09:46:17, 09:46:25). Setting
publishingEnabledKey first did not help.

Logs never reach inspect/prepare/queue/deliver. 09:46:04 is
pubky_secret_key loaded → enable ERROR in 2ms, so this is still
canPublishPrivateEndpoints == false + requireImmediate + a saved
contact, then restore() rolls the toggle off.

Also still had pubky8u997gg after deleting pubky1jy146y, so this
is not specific to that staging contact.

Need the failing canPublish clause logged, and enable should not
fail-close when publication never actually ran.

Screen.Recording.2026-09-01.at.11.58.31.mov

bitkit_logs_2026-09-01_09-46-59.zip

@ben-kaufman
ben-kaufmanforce-pushed the codex/paykit-request-qa-fixes branch from 112fb70 to 154b3d6CompareSeptember 1, 2026 12:32
@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

Fixed the remaining enable failure in 2d90e0e. Private endpoint preparation during enable is now best effort, so temporary wallet, node, app, or Pubky unreadiness defers publication instead of returning privateUnavailable and rolling the toggle back. Real errors after publication starts still fail and restore the previous state. The readiness check now logs the exact unavailable condition. ContactPaymentsServiceTests and PrivatePaykitServiceTests pass 24 of 24. @piotr-iohk please retest the Enable case, and @ovitrif please re-review the updated head.

@piotr-iohk

piotr-iohk commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Retested latest revision (2d90e0e), iPhone 17 sim.

Enable still fail-closes. Toggle → Failed to update contact payments: privateUnavailable (GeneralSettingsView.updateContactPayments line 180) → “Private Paykit is not available.”

2d90e0e is on this build — startup logged Deferring private Paykit endpoint publication during prepare: private publication is disabled (flag off, 0 contacts). On the enable attempt itself there was no second defer: flag was set, canPublish passed, then it still threw. Sequence: loaded 1 contact (pubkypwuhkhj…), then the error ~11s later. No Failed to inspect/prepare/sync WARN around the fail.

Hypothesis: requireImmediatePublication: false on enable is bypassed by the unguarded return error after receiverPathSelection in syncLocalEndpointPublicationLocked. Other failures on that path are supposed to defer.

Android enable stays on after the matching defer change.

Logs:
bitkit_logs_2026-09-01_15-09-55.zip

@ben-kaufman

Copy link
Copy Markdown
ContributorAuthor

You were right about the remaining fail-closed path. The receiverPathSelection catch returned immediately before the deferred-publication policy was applied. Signed commit ab3308b8 now records/logs that per-contact error, continues reconciliation, and returns it only for immediate publication.

The regression test covers this exact selection failure, and the focused private/contact/request suites pass 74/74. I also installed this exact head over the saved-contact QA state, enabled Paykit UI, and verified the toggle stayed enabled past 15 seconds with the success toast and no privateUnavailable rollback. Please retest ab3308b8 after the fresh checks finish.

@ovitrifovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ben-kaufman@piotr-iohk@ovitrif