Parts of the Participant-Limit flow in central-ledger ignore the limit-type

[yosheeck]

Summary: Few functions ignore limit-type, which can lead to undefined behaviour when more than one limit-type is used by the participant.

Severity: Medium

Priority: Medium

Expected Behaviour The code should correctly handle limit-type for all the scenarios. For example adding new limit-type of type "B", when type "A" is already added should be possible. At the moment addLimitAndInitialPosition() will fail to recognize the difference between "B" and "A" and will return an error ("Internal server error - Participant Limit or Initial Position already set").

Steps to Reproduce - example

1. Start new participant
2. Configure limit "NET_DEBIT_CAP" for this participant - works ok
3. Configure limit "NET_DEBIT_CAP2" for this participant - that fails

The code analysis I didn't investigate all the possible scenarios, but those I found are related to the fact that ParticipantLimitModel.getByParticipantCurrencyId() function accepts/handles just one parameter "participantCurrencyId" and returns just one participant even if comment for the functions says "Returns the rows from participantLimit table if successful, or throws an error if failed" - which suggests that this function should return array of many participants (it doesn't, it returns just one).

Potential fix There is not much code that calls that function. Actually I found only 2 meaningful places and both use cases would NOT need the array of participants as then those callers would need to filter by limit-type anyway. So, potentially the best thing to do is:

1. Fix the function, so it does what the comment says (this will break the callers - look at point 3).
2. Write new function which accepts 2 params: participantCurrencyId and limitType and returns only perfect match.
3. Refactor callers to call the new function.

Specifications

• Component (if known): central-ledger
• Version: master (last release 10.1.0)
• Type of testing: code validation
• Bug found/raised by: @yosheeck

Notes:

• Severity when opened: Medium
• Priority when opened: Medium

[elnyry-sam-k]

@yosheeck - good catch. Thanks for this and good that we caught this before anyone else :-) .. Lets discuss when we can prioritize this!

Its understandable, given that our focus so far has been solely on NDC and no other limit, but addressing this will be critical as we expand and add more limit types / limits...

[shashi165]

@elnyry-sam-k, @mdebarros - addLimitAndInitialPosition is used not only for adding the limit but also for setting the initial position. So the endpoint POST /participants/{name}/initialPositionAndLimits needs to be called only once. If we want to add a limit of a different type, we should provide a separate endpoint: POST /participants/{name}/limits.

[mdebarros]

@elnyry-sam-k, @mdebarros - addLimitAndInitialPosition is used not only for adding the limit but also for setting the initial position. So the endpoint POST /participants/{name}/initialPositionAndLimits needs to be called only once. If we want to add a limit of a different type, we should provide a separate endpoint: POST /participants/{name}/limits.

Hi @shashi165, that is the case for each limit. I.e. addLimitAndInitialPosition adds a new limit and sets the initial position for that specific limit.

I assume that this issue is due to the addLimitAndInitialPosition ignoring the limit type & currency (note: the type + currency is the unique identifier). Instead, it tries to set the position of the NET_DEBIT_CAP limit which results in the failure described by @yosheeck.

For example:

First limit:

POST /participants/payerfsp/initialPositionAndLimits HTTP/1.1
Host: http://dev2-central-ledger.mojaloop.live
Content-Type: application/json
FSPIOP-Source: hub_operator
Authorization: Bearer {{HUB_OPERATOR_BEARER_TOKEN}}
Content-Length: 107

{
    "currency": "USD",
    "limit": {
      "type": "NET_DEBIT_CAP",
      "value": 10000
    },
    "initialPosition": 0
}

This should create a new limit called NET_DEBIT_CAP, and set its position to 0.

POST /participants/payerfsp/initialPositionAndLimits HTTP/1.1
Host: http://dev2-central-ledger.mojaloop.live
Content-Type: application/json
FSPIOP-Source: hub_operator
Authorization: Bearer {{HUB_OPERATOR_BEARER_TOKEN}}
Content-Length: 107

{
    "currency": "USD",
    "limit": {
      "type": "NET_DEBIT_CAP_2",
      "value": 10000
    },
    "initialPosition": 0
}

This should create a new limit called NET_DEBIT_CAP_2, and set its position to 0, but this instead tries to set the limit for NET_DEBIT_CAP instead which results in an internal server error - Participant Limit or Initial Position already set. <-- This should not happen, and it should be accepted.

Assuming that everything was working as intended, the following should also be valid. The only difference here to the first limit is a currency change from USD to GBP:

POST /participants/payerfsp/initialPositionAndLimits HTTP/1.1
Host: http://dev2-central-ledger.mojaloop.live
Content-Type: application/json
FSPIOP-Source: hub_operator
Authorization: Bearer {{HUB_OPERATOR_BEARER_TOKEN}}
Content-Length: 107

{
    "currency": "GBP", <-- Note the difference here!!!
    "limit": {
      "type": "NET_DEBIT_CAP",
      "value": 10000
    },
    "initialPosition": 0
}

Please confirm this understanding.

[shashi165]

@mdebarros , @elnyry-sam-k - Since there is no link between the participantLimit and participantPosition table this can be resolved in 2 ways

1. create the relationship in the database tables and then we can have a limit type and a position related to that limit type OR
2. deprecate the endpoint POST /participants/{name}/initialPositionAndLimits and provide 2 separate endpoints: a. POST /participants/{name}/limits -- for creating new limits b. POST /participants/{name}/position -- for setting the initial position

[elnyry-sam-k]

Since current version of Mojaloop (pending Reference Architecture implementation) has only a single limit - this can be turned into a requirement for future implementation of this aspect (in ref arch workstream)