Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FX - Unnecessary participant currency validations in FX transfer #3932

Closed
9 of 11 tasks
vijayg10 opened this issue May 24, 2024 · 3 comments
Closed
9 of 11 tasks

FX - Unnecessary participant currency validations in FX transfer #3932

vijayg10 opened this issue May 24, 2024 · 3 comments
Assignees
Labels
bug Something isn't working or it has wrong behavior on a Mojaloop Core service core-dev-squad oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it

Comments

@vijayg10
Copy link

vijayg10 commented May 24, 2024

Summary:
Observed that some validations are not needed for FX transfer but are happening in the central services.
For example:

  • Payer doesn't need an account in target currency
  • Payee doesn't need an account in source currency

Severity:
High

Priority:
Medium

Expected Behavior
We should be able to make an FX transfer without onboarding both source and target currencies for payer.

Steps to Reproduce

  1. Observe the integration tests for fxTimeout (test/integration-override/handlers/transfers/fxTimeout.test.js)
  2. In function prepareFxTestData, remove target currency for payer by changing the line
const payer = await ParticipantHelper.prepareData(dataObj.payer.name, dataObj.sourceAmount.currency, dataObj.targetAmount.currency)
----to----
const payer = await ParticipantHelper.prepareData(dataObj.payer.name, dataObj.sourceAmount.currency)
  1. Execute this integration test and it fails
  2. Observe the logs in central ledger services and we can find some error like 'Participant currency' doesn't exist

Specifications

  • Component (if known): Central Ledger FX branch
  • Type of testing: Integration tests
  • Bug found/raised by: @vijayg10

Tasks

  • Investigate the issue by going through different FX uses cases sequence diagrams
  • Discuss the observations and agree on the logic for validations
  • Refactor prepare handler for validations
  • Unit tests
  • Integration tests
  • Fix the issue with table transferParticipant related to the column participantCurrencyId
    • Change participantCurrencyId as nullable
    • Add participantId to the table transferParticipant
    • Refactor the queries using participantCurrencyId from the table transferParticipant
    • Fix unit tests

Pull Requests:

@vijayg10 vijayg10 added bug Something isn't working or it has wrong behavior on a Mojaloop Core service oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it core-dev-squad labels May 24, 2024
@vijayg10
Copy link
Author

vijayg10 commented May 29, 2024

I have gone through the sequence diagrams and discussed with @PaulGregoryBaker and came up with the following solution for this problem.

The participant account validations to be done based on the cyril result. i.e, In prepare handler, cyril should return a list of participant accounts (participantCurrencyIds) to be validated and then the prepare handler function iterates through the list and validate the accounts of existence and active status.

The following are the validations based on different FX use cases.

Payer side currency conversion

  • POST /fxTransfer

    • InitiatingFSP - Source Currency
    • CounterPartyFSP - Source Currency
    • CounterPartyFSP - Target Currency
  • POST /transfers

    • Payee - Currency

Payee side currency conversion

  • POST /transfers

    • Payer - Currency
    • Payee - Currency (Optional based on config param, this is tricky use case where we can't differentiate whether a transfer request involves currency conversion or not. Need to discuss with FX design authors)
  • POST /fxTransfer

    • InitiatingFSP - Target Currency
    • CounterPartyFSP - Source Currency
    • CounterPartyFSP - Target Currency

@PaulGregoryBaker
Copy link

2 pts left
Integration tests still outstanding.

@MichaelJBRichards
Copy link

In every case, whether currency conversion is initiated by the debtor or the creditor, the following reservations should be made by the switch:

  • For fxTransfers:
    • If there is already an entry in the Cyril for the determining transfer, then currency conversion is being performed by the creditor. Check and reserve against the FXP in the target currency.
    • Otherwise, currency conversion is being performed by the debtor. Check and reserve against the debtor FSP in the source currency.
  • for transfers:
    • If there is an entry in the Cyril, then currency conversion is being performed by the debtor. Check and reserve against the FXP in the target currency.
    • Otherwise, currency conversion is being performed by the creditor. Check and reserve against the debtor in the source currency.

The same obligations are always created, no matter who is doing the currency conversion; and they are always created after the transfer has completed:

  • An obligation from the debtor to the FXP in the source currency.
  • An obligation from the FXP to the creditor in the target currency.

Under no circumstances should any DFSP have an account at the switch in any currency other than its own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working or it has wrong behavior on a Mojaloop Core service core-dev-squad oss-core This is an issue - story or epic related to a feature on a Mojaloop core service or related to it
Projects
None yet
Development

No branches or pull requests

4 participants