e710bc80 - Keep buy-bound IBANs out of the user-level personal IBAN lookups - #4515
e710bc80 - Keep buy-bound IBANs out of the user-level personal IBAN lookups#4515TaprootFreak wants to merge 1 commit into
Conversation
|
One review pass, clean on both dimensions — this PR is itself the fix for a finding raised during the Verified before opening: format, lint and type-check clean; the five affected suites pass (272 tests); The behavioural test was checked by mutation rather than by inspection: with |
Follow-up to #4511. A logic review there found that the user-level lookup does not filter on
buy IS NULL, so the generic deposit path can hand a customer an IBAN bound to a different purchase.BankTxServicethen assigns incoming funds on that IBAN to its stored purchase:The customer pays for purchase A and the money is booked against purchase B.
Why this is safe to change now
The risk is latent, not active. Checked against production:
virtual_ibanrows ever created, none has abuyId.wallet.buySpecificIbanEnabledis off on all 66 wallets;asset.personalIbanEnabledis off on all430 assets. Those two flags gate the only path that creates a buy-bound row, so it has never run.
No existing row changes meaning, no customer is affected, and no migration is needed. The moment
someone enables those flags there would be rows to reason about — this is the cheapest point to close
it.
Why this does not contradict #4384
The test this replaces was named
retains merge-base behavior by reusing a buy-bound Yapeal IBAN in the generic lookup. Its commit message explains the intent: the Bank Frick work had changed Yapealbehaviour as a side effect, and that was rolled back so every Yapeal-reachable path matched the
merge base again.
That was a statement about the scope of that PR, not a finding that buy-bound IBANs should satisfy a
user-level lookup. This PR makes the change deliberately, on its own merits, in isolation — which is
the route #4384 left open.
The change
buy: IsNull()on both user-level lookups:getActiveReceivingForUserAndCurrencyandgetActiveSendingCandidatesForUserAndCurrency. The privatefindActiveForUserCurrencyAndBankalreadycarried it, so all three user-related lookups now agree on which rows count as generally assigned to
the customer. Buy-specific callers keep using
getActiveForBuyAndCurrency.Both directions are filtered on purpose. An asymmetric condition between deposits and payouts is
exactly the kind of inconsistency that produced the previous round of findings.
The test
The behavioural test stores the generally assigned row first, so it carries the lower id, and the
buy-bound row second. Under the existing newest-first ordering the buy-bound row would win, which
means only the new predicate can make the test return the free row. Removing
buy: IsNull()fails it.