Skip to content

feat: accept allowance offers automatically on the allowee's wallet - #841

Open
ovitrif wants to merge 22 commits into
feat/paykit-allowancesfrom
feat/allowance-auto-accept
Open

ovitrif wants to merge 22 commits into
feat/paykit-allowancesfrom
feat/allowance-auto-accept

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #807
Twin: synonymdev/bitkit-android#1385
Stacked on #799
Refs: #805

This PR makes the allowee's wallet accept an allowance the allower offers, so the allowee taps nothing and the allowance is active as soon as the offer arrives.

Description

  • Adds automatic acceptance of a received allowance proposal that makes this wallet the allowee, so the allower's decision alone activates the allowance.
  • Adds an "Allowance Added" notification, "{name} set up an allowance for you", with a toast fallback, so the allowee learns about the new allowance.
  • Keeps the review sheet with Accept and Decline for an allowee's ask, because there the money is this wallet's.
  • Retries a failed acceptance on the next refresh, and never touches a proposal this wallet sent or one already answered.
  • Accepts and announces an offer only for the profile session that found it, and never while the profile is being deleted.

Out of Scope

  • Bitkit/Resources/Localization: the new strings are English only until the next translation sync.
  • Allowee-authored asks: this PR only changes what the allowee's wallet does with a received offer; the iOS app proposes allowances only as allower, so the review sheet for an ask is covered by unit tests, not on a device.

Design

N/A — no design available.

Preview

Payer waits Payee: Allowance Added Payee: Active Payer: Active
Recording
set-and-accept-ios.mp4

QA Notes

Journeys

  • updated set-and-accept.xml
  • updated pin-gates-allowance-changes.xml: the payee's wallet accepts the offer by itself once the payer's PIN is entered

Manual Tests

  • 1. Payer: Allowances → Add Allowance → contact → Save: the payee's wallet shows no review sheet, posts "Allowance Added", and both rows turn Active — the first private link between two new contacts can delay the accept by a minute or two.
  • 2. regression: payee: allowance detail → Swipe To End Allowance: ends on both sides.
  • Allowee-authored ask → review sheet on the allower — not reachable on iOS, which proposes only as allower; covered by PaykitAllowanceExecutorTests.swift.

Automated Checks

  • added PaykitAllowanceTests.swift — only a received proposal from the allower counts as an offer
  • added PaykitAllowanceExecutorTests.swift — an allower's offer is accepted without the sheet and not announced twice; an allowee's ask still goes to the sheet; sent and answered proposals are never accepted; a failed accept is retried and announced once; nothing is accepted or announced once the session changes; a background accept leaves Save disabled while a proposal is in flight; accepting one contact's offer leaves another contact's same-id ask on the review sheet
  • ran xcodebuild test -only-testing:BitkitTests on an iPhone 17 simulator (iOS 26.5) with Paykit 0.1.0 after merging the updated feat: add allowances for automatic paykit payments #799 — 2,770 tests, 12 skipped; the only failures are SendConfirmationLifecycleTests.testCustomFeeWalletSwitchSurvivesFeeScreenNavigation, which fails the same way on master
  • ran the allowance and payment proof suites again after merging the updated feat: add allowances for automatic paykit payments #799 (PIN gate, Lightning-only automatic payments, payer-side proposal checks) and the review fixes, with warnings as errors — 151 tests, 0 failures
  • ran set-and-accept.xml on two simulators against the staging regtest backend on the final head: the payee's wallet accepted by itself 14 s after the payer published the proposal, no review sheet opened, the toast "Allowance Added" showed, and both rows turned Active (the system notification was not exercised, notifications were not granted)

@ovitrif
ovitrif force-pushed the feat/allowance-auto-accept branch from b0c583e to 80f5c0f Compare October 1, 2026 21:52
@ovitrif

ovitrif commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 0b8e352: this PR now sits on the updated #799 (merge 8f0449c, Paykit SDK rc59 with identity-wide shared state).

  • Rebased only this PR's two feature commits onto the new feat: add allowances for automatic paykit payments #799, whose old base commits are gone, and resolved the conflicts in AppScene.swift and the two allowance test files. Allowances are bound to identities now, so the offer rule is unchanged: a received proposal is accepted automatically only when this wallet is the allowee.
  • README: the journeys README no longer promises a one-second accept; the offer is accepted once it has arrived, which can take a minute or two between contacts that have never exchanged a private message.
  • Local verification: BitkitTests on an iPhone 17 simulator, 1,625 tests, 12 skipped, 0 failures; set-and-accept.xml on two simulators against staging regtest passed, the payee turned Active with no tap and no review sheet.
  • The description now carries the screenshots and a recording, and its QA Notes record that the allowee-ask review sheet is not reachable on iOS.

@ovitrif

ovitrif commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 14307a0: merged the updated #799 (a19ea87: the request-presentation and covered-request fixes and more of Ben's shared-runtime merges) into this PR; the merge was clean and nothing in the auto-accept code changed.

Local verification: the Paykit and allowance suites on an iPhone 17 simulator, 382 tests, 0 failures.

@jvsena42
jvsena42 added this pull request to stack #866 October 2, 2026 16:44
@ovitrif
ovitrif removed this pull request from stack #866 October 5, 2026 15:39
@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 5f65a58: merged the current #799 head (2073233) into this PR, which brings the shared Paykit runtime merges and the subscription-notification and contact-loading fixes. The merge was clean.

  • xcodebuild test -only-testing:BitkitTests on an iPhone 17 simulator (iOS 26.5) with Paykit rc59: 1,685 tests, 12 skipped, 0 failures.
  • feat: add allowances for automatic paykit payments #799 still has to take the latest master (Paykit 0.1.0); this PR follows once it does, and set-and-accept.xml is rerun on that head.

@ovitrif ovitrif added this to the 3.1.0 milestone Oct 19, 2026
@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed cebcc55: merged the updated #799 (ea65e76, now level with master and Paykit 0.1.0) into this PR.

  • One conflict, in AppScene.swift: the incoming request refresh now runs automatic work only for the current automatic Paykit session and never while the profile is being deleted. Accepting an allower's offer and posting "Allowance Added" now sit inside that same guard, right after the allowance refresh, so an offer is never accepted for a stale session or during profile deletion.
  • xcodebuild test -only-testing:BitkitTests on an iPhone 17 simulator (iOS 26.5): 2,770 tests, 12 skipped, 2 failing, neither in allowance code:
    • SendConfirmationLifecycleTests.testCustomFeeWalletSwitchSurvivesFeeScreenNavigation fails the same way on master.
    • PaymentFlowTests.testchannelPurchaseFlow failed at the regtest deposit step (the balance never rose after funding); it passed on the previous head and gets a rerun.
  • set-and-accept.xml is rerun on this head next.

@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 8de56f4: merged the updated #799 (47ed839, which counts an automatic on-chain allowance payment only once it is accepted). The merge was clean; this PR's offer acceptance does not touch the changed payer interface.

  • xcodebuild test for PaykitAllowanceTests, PaykitAllowanceExecutorTests, PaykitPaymentProofServiceTests and PaymentFlowTests.testchannelPurchaseFlow: 144 tests, 0 failures; the channel purchase test that failed at its regtest deposit step on cebcc55 passes.

@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 5143187: merged the updated #799 (10abf2e: the payment PIN gate for granting and ending an allowance, automatic allowance payments over Lightning only, and both limits required), plus a51aca5 for pin-gates-allowance-changes.xml.

  • The PIN journey now expects the payee's wallet to accept the offer by itself: no "Allowance Added" while the payer's PIN is cancelled, and once the PIN is entered, "Allowance Added" arrives with no review sheet, no tap and no PIN. It matches the Android copy word for word apart from the identifier annotations.
  • xcodebuild test for PaykitAllowanceTests, PaykitAllowanceExecutorTests and PaykitPaymentProofServiceTests: 144 tests, 0 failures.

@ovitrif

ovitrif commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator Author

QA: set-and-accept.xml on 5143187

Two iPhone 17 simulators (iOS 26.5), fresh wallets with Pubky profiles on staging, saved as each other's contacts, regtest Debug build. Pass on every action.

  • The payer saved $5 a payment and $50 a month; its row read "Waiting for an answer" with "$ 50.00" over "Monthly limit".
  • The payee's wallet accepted by itself 14 s after the payer published the proposal: no review sheet opened and nothing was tapped. Notifications were not granted, so the "Allowance Added" toast showed: "Alwpayer807 set up an allowance for you".
  • The payee's row reads "Active · $5.00 a payment" with no Accept or Decline step, and the payer's row turned "Active · $5 a payment".

Payer on the left, payee on the right, at 2x:

set-and-accept-ios.mp4

@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 8e476fc: merged the updated #799 (b18cb79: master with the USDT feedback fixes, capped routing fees on automatic allowance payments, and dropping proposals whose limits the payer can't show). The merge was clean. Dropping such proposals applies only when this wallet is the payer, so the payee's automatic accept is unchanged.

  • xcodebuild test for PaykitAllowanceTests, PaykitAllowanceExecutorTests and PaykitPaymentProofServiceTests: 146 tests, 0 failures.
  • set-and-accept.xml passed on 5143187, one merge earlier (results above); none of the commits merged since touch the offer or its acceptance.

@ovitrif
ovitrif marked this pull request as ready for review October 19, 2026 19:21
@ovitrif
ovitrif requested review from a team and ben-kaufman October 19, 2026 19:21
@ovitrif
ovitrif requested a review from coreyphillips October 19, 2026 19:21
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-19T19:25:51.073289Z 8e476fc Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

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

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e476fc97f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

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

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

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Bitkit/Services/PaykitAllowanceManager.swift Outdated
@greptile-apps

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The change affects the allowance agreement flow between two wallets.

The PR appears safe to merge, with non-blocking improvements for session checks and retry tests.

Findings

  1. P2 Old sessions still announce allowances ▶
  2. P2 Failed acceptance lacks a test ▶

Summary

Received allowance offers now accept automatically. Requests asking this wallet to pay still go to the review sheet.

  • The payee’s wallet accepts allowance offers from the payer automatically.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Received proposal] --> B{This wallet receives money?}
    B -->|Yes| C[Accept automatically]
    C --> D{Acceptance succeeds?}
    D -->|Yes| E[Announce Allowance Added]
    D -->|No| F[Retry on next refresh]
    B -->|No| G[Show review sheet]
Loading

Reviews (1) · Last reviewed commit: "chore: merge feat/paykit-allowances" · Reviewed by Greptile

Comment thread Bitkit/AppScene.swift Outdated
Comment thread BitkitTests/PaykitAllowanceExecutorTests.swift
@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed c5ac982: the automatic offer accept now runs only for the session that found the offer.

  • acceptOffersFromAllowers pins the identity it started with and takes the refresh's session check (current automatic Paykit session, no profile deletion). It checks both before each accept and before returning offers to announce, so a sign-out, profile switch or profile deletion mid-loop accepts and announces nothing more.
  • Answering a proposal records it as presented only under the identity that answered it.
  • New tests cover a failed accept that is retried on the next refresh and announced once, and the session check.
  • xcodebuild test for PaykitAllowanceTests, PaykitAllowanceExecutorTests and PaykitPaymentProofServiceTests: 148 tests, 0 failures.

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No findings. Makes the allowee's wallet accept an allower's offer by itself and announce it, while an allowee's ask still goes to the review sheet. Reviewed c5ac982, full tier.

What I checked, and 4 candidates I ruled out

Read in full: the diff against feat/paykit-allowances (10 files), PaykitAllowance.swift, PaykitAllowanceManager.swift (accept, respond, presentation, automatic payments), the refreshPaykit path in AppScene.swift, the row routing in AllowancesView.swift
Call sites traced: acceptOffersFromAllowers, isOfferFromAllower, isAnswerable, proposalForPresentation
CI: Run Tests still running at review time; Greptile passed. Not built here: Swift does not build on this box, so this review is reasoned from the code and the tests.

Ruled out

  • An allowee's ask accepted without the user: isOfferFromAllower needs role == .allowee on a received proposal, so a proposal that makes this wallet the payer never qualifies; testManagerLeavesAnAskFromTheAlloweeForTheReviewSheet covers it.
  • A mixed entry auto-accepted: an entry is taken only when every answerable allowance in it is an offer (allSatisfy), and anything else stays on the sheet path.
  • Accepting or announcing for a stale session: c5ac982 pins the starting identity, rechecks it with the caller's session check before each accept and before returning, and respond records presented ids only under the identity it started with; both bot threads are answered by that commit and its two tests.
  • Double accept or double announcement: isAcceptingOffers drops a re-entrant pass, and an accepted offer leaves the answerable set after respond refreshes, so the second pass finds nothing; testManagerAcceptsAnOfferFromTheAllowerWithoutAReviewSheet and the retry test assert one accept and one announcement.

Merge confidence: 4/5, the change is covered by the new manager tests but Run Tests had not finished at review time.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan: no HIGH/MEDIUM. One LOW is inline, with a failing test.

Checked and clean:

  • This wallet never ends up paying. isOfferFromAllower requires role == .allowee on a received proposal, and localRole comes from the SDK (recipient_role()).
  • Terms can't change after the check. The accept passes expectedProposalStreamItemId, so a re-sent or replaced proposal is refused.
  • The allowee side has no automatic effects. Every payment path checks isAllower.
  • Removed contacts. Offers arrive only over an Encrypted Link, list_allowances drops blocked peers, and deleting a contact blocks the peer, so a removed contact's offer isn't auto-accepted.

Parity: the session/identity fence fixed in c5ac982 needs porting to the Android draft synonymdev/bitkit-android#1385, whose acceptOffersFromAllowers() takes no canContinue.

Perf (repeated work): an offer whose accept keeps failing (recoveryRequired, SDK NotFound) retries on every poll, costing one acceptAllowance call and a warn line each time. I'll measure that on device together with #799.

Device gate (partial): not run yet. Pending: the updated allowances/set-and-accept.xml and pin-gates-allowance-changes.xml, cross-device.

Comment thread Bitkit/Services/PaykitAllowanceManager.swift Outdated
@ovitrif

ovitrif commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed d2c0ca9: an offer accepted in the background no longer clears the allowance sheet's progress state, so Save stays disabled while the user's own proposal is in flight and a second tap can't send a duplicate proposal.

  • xcodebuild test for PaykitAllowanceTests, PaykitAllowanceExecutorTests and PaykitPaymentProofServiceTests: 149 tests, 0 failures, including the new testAutomaticOfferAcceptanceKeepsAManualProposalWorking.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 84d9ccc: merged the updated #799 (2e7c71d, which brings master with its SwiftFormat lint job and warnings-as-errors builds), then formatted this PR's Swift with SwiftFormat 0.64.0 and dropped an unneeded try in a test that the stricter build rejects.

  • swiftformat --lint passes on every Swift file this PR changes; the only remaining lint error on the branch is in ActivityRow.swift, which comes from master and is fixed by chore: make master pass the swiftformat check #1035.
  • xcodebuild test with warnings as errors for PaykitAllowanceTests, PaykitAllowanceExecutorTests and PaykitPaymentProofServiceTests: 149 tests, 0 failures.

@ovitrif
ovitrif requested a review from jvsena42 October 20, 2026 04:53

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan at 84d9ccc: no issues found in d2c0ca9 and 84d9ccc. Reviewed as a funds change. My earlier LOW is fixed:

  • the background pass calls accept(offer, showsProgress: false) (:268);
  • respond touches isWorking only when showsProgress is set (:283-290);
  • covered by testAutomaticOfferAcceptanceKeepsAManualProposalWorking.

Checked and clean:

  • showsProgress defaults to true, so the review sheet and reject keep the isWorking guard.
  • 84d9ccc is formatting only.
  • Parity with synonymdev/bitkit-android#1385: both pin the checked entry, check the identity, take canContinue, and record presented ids only for the starting identity.

Inherited from the base: the shared-id issue posted on #799 (review). This PR's auto-accept leaves offer-plus-ask entries for manual review, and that is the shape the issue produces. The fix belongs in #799 and needs nothing here.

Perf: not relevant.

Device gate (partial): 84d9ccc — not run yet. Pending: allowances/set-and-accept.xml and allowances/pin-gates-allowance-changes.xml cross-device with the Android twin. Device results will follow in a separate review.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed e1f6cc8: merged the updated #799 (407c225: allowance entries and presented proposals are keyed by contact as well as allowance id, so two contacts' proposals that share an id stay apart).

  • Two conflicts, resolved by keeping both sides: the SDK mock in the tests matches the contact and still records the accept, and the review sheet keeps leaving out an allower's offer while it now tracks presented proposals by contact.
  • feat: add allowances for automatic paykit payments #799's two new same-id tests are adapted to this PR, where an allower's offer never reaches the review sheet: accepting C's offer by itself leaves M's same-id ask waiting for the user, and two contacts' same-id asks are each presented.
  • xcodebuild test with warnings as errors for PaykitAllowanceTests, PaykitAllowanceExecutorTests and PaykitPaymentProofServiceTests: 151 tests, 0 failures.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 488cba4: merged the updated #799 (a844c40: the allowance journeys name a row by its entry id). The one conflict, in the allowances journey README, keeps the new row identifier together with this PR's offer toast and review-sheet notes. Docs only; no Swift changed.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 86d52d3: merged the updated #799 (b059b8e: each contact's automatic payments count only toward that contact's allowance). The merge was clean.

  • xcodebuild test with warnings as errors for PaykitAllowanceTests, PaykitAllowanceExecutorTests and PaykitPaymentProofServiceTests: 152 tests, 0 failures.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Delta at e1f6cc8: no issues found. The merge of feat/paykit-allowances replaces 80c05bc's change with the scoped-id version from #799. This PR's production diff over its base is unchanged, so only tests moved: the merged test asserts that only C's offer is auto-accepted and M's ask stays presentable.

set-and-accept.xml:20 still says AllowanceRow-<allowanceId>. #799 changed it to <entryId> in a844c40, so this line needs the same wording once you merge the base.

Device gate (partial): e1f6cc8 — not run yet. Pending: allowances/set-and-accept.xml and allowances/pin-gates-allowance-changes.xml cross-device with the Android twin. Device results will follow in a separate review.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@jvsena42 set-and-accept.xml:20 already reads AllowanceRow-<entryId>: merging a844c40 in 488cba4 brought that wording in, and no journey on the head (86d52d3) still names the row by <allowanceId>.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 97d86f8: merged the updated #799 (b005f8f, which carries master with #1040, so the branch now passes SwiftFormat). The merge was clean.

  • swiftformat --lint . (0.64.0) passes on the whole branch.
  • xcodebuild test with warnings as errors for PaykitAllowanceTests, PaykitAllowanceExecutorTests and PaykitPaymentProofServiceTests: 152 tests, 0 failures.

@ovitrif
ovitrif requested a review from pwltr October 20, 2026 23:26
Sign up for free to 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