Improve accuracy of userReserves calculations

[dib542]

Issue

At the moment it is not possible to guarantee that we know how many Duality Dex reserves a user's wallet holds.

this is because:

• the information required to calculate this resides on two different endpoints and I cannot guarantee that the data of both number are from the same block height
• this can be fixed by adding data to several different endpoints

Indicative user reserves calculations

We use indicativeReserves as a fundamental state to calculating a user's accurate userReserves in the app.

indicativeReserves can be calculated from userReserves = (sharesOwned / totalShares) * tickReserves at any point in time (but will only be "accurate" when totalShares and tickReserves are fetched from the same block height as they may change every/any block height, sharesOwned only changes with user actions).

These values can be currently collected from:

• sharesOwned from /cosmos/bank/v1beta1/balances/{address}
  • this could be fetched from /dualitylabs/duality/dex/user/deposits/{address} if poolID was returned on DepositRecord, but it is not so this is not recommend after the update of duality-labs/duality@9ede0f2
• totalShares from /cosmos/bank/v1beta1/supply/{denom}
• lowerReserves from /dualitylabs/duality/dex/pool_reserves/{pairID}/{tokenIn}/{tickIndex}/{fee}
• upperReserves from /dualitylabs/duality/dex/pool_reserves/{pairID}/{tokenIn}/{tickIndex}/{fee}

but if would be better if they could be collected from one place, allowing the app to have an "accurate" (numbers from the same height) version of the user's indicative reserves:

• The endpoint /duality/dex/user/deposits/{address}:
  • could add either a calculated indicativeReservesAsToken0 and indicativeReservesAsToken1 directly
  • or list all the properties required to calculated it all at once, i.e the endpoint should return for each pool all of
    • sharesOwned
    • totalShares
    • lowerReserves0
    • upperReserves1
  • the difference is that the first option returns the same values whenever called (if the user did not do an action), and the second option can change with the actions of other users trading on the pools that the user has deposits in. so the first option is more REST-like from the user's deposits point of view, and the second option is more REST-like from the specific pools point of view.
• the endpoints /duality/dex/pool/{poolID} and/or /duality/dex/pool/{pairID}/{tickIndex}/{fee}:
  • could return totalShares on QueryPoolResponse or the Pool type
  • this would provide enough information for each pool in question
  • but of course this mean that instead of fetching one just endpoint for all the user's deposit data, this data must be fetched for each user's deposit (which may be a significant number) after fetching the user's deposit pools from /duality/dex/user/deposits/{address}
    • /duality/dex/user/deposits/{address} should return poolID if only the /duality/dex/pool/{poolID} endpoint will contain the totalShares information.
    • it seems plausible to fetch all of a user's dex pools from the user's balances: /cosmos/bank/v1beta1/balances/{address} but this endpoint will no longer give an indication of which pools belong to which token pair (without fetching metadata for each pool)
• due to the number of possible network requests I think adding information to the /duality/dex/user/deposits/{address} would be more beneficial.
• A new endpoint /duality/dex/user/deposits/{address}/{poolID}
  • could allow the app to fetch and update in its store the indicativeReserves for a single pool. which would be a typical request after the users makes a Dex action that causes a bank transfer action to occur against their wallet address.
  • this is not really needed if we don't add indicativeReserves to the /duality/dex/user/deposits/{address} endpoint. If we add totalShares there instead the required data to update the app state for a single pool will be available on the /duality/dex/pool/{poolID} and/or /duality/dex/pool/{pairID}/{tickIndex}/{fee} endpoints if the totalShares information is added there.

Conclusion

Therefore I think my recommendation is to add

Option 1

• to the Pool type (https://github.com/neutron-org/neutron/blob/v2.0.0/proto/neutron/dex/pool.proto#L10-L14):
  • totalShares
  • it already has lowerReserves0 and upperReserves1
  • this will make the Pool type suitable for calculating accurate indicative reserves for the app
• to endpoint /duality/dex/user/deposits/{address} response type DepositRecord https://github.com/neutron-org/neutron/blob/v2.0.0/proto/neutron/dex/deposit_record.proto#L8-L20
  • pool (of Pool type with new totalShares info)

This allows the app to

• request /duality/dex/user/deposits/{address} on app start / address change
  • to get the entire user's indicativeReserves (and therefore future accurateUserReserves and estimatedUserReserves) in one network request.
• then request /duality/dex/pool/{poolID} upon any bank.transfer updates to the user's bank
  • to update relevant portions of the user's indicativeReserves

Option 2

or if we don't want to modify the common Pool type to add to the responses:

• response type DepositRecord https://github.com/neutron-org/neutron/blob/v2.0.0/proto/neutron/dex/deposit_record.proto#L8-L20
  • totalShares
  • pool (to add poolID, lowerReserves, and upperReserves all at once)
• response type QueryPoolResponse https://github.com/neutron-org/neutron/blob/v2.0.0/proto/neutron/dex/query.proto#L304-L306
  • totalShares
  • this will make endpoints /duality/dex/pool/{poolID} and/duality/dex/pool/{pairID}/{tickIndex}/{fee} suitable for calculating accurate indicative reserves on individual pools for the app

Option 3

or instead of modifying QueryPoolResponse (because of concerns for returning too much unrelated(?) information for these endpoints)

• creating a /duality/dex/user/deposits/{address}/{poolID} endpoint to return individual DepositRecords for individual Pool IDs would enable easy accurate user reserve queries for partial updates.

For all options

For applications other than the web app, adding totalShares, lowerReserves, and upperReserves to the user deposits response may be very helpful. The ability to continually fetch /duality/dex/user/deposits/{address} whenever to get an accurate total of the user's reserves at the current point in time could be very helpful in building simple applications.

[pr0n00gler]

the information required to calculate this resides on two different endpoints and I cannot guarantee that the data of both number are from the same block height

You can just do multiple queries to the same height, no?
https://docs.cosmos.network/main/user/run-node/interact-node#query-for-historical-state-using-grpcurl

[dib542]

the information required to calculate this resides on two different endpoints and I cannot guarantee that the data of both number are from the same block height

You can just do multiple queries to the same height, no? https://docs.cosmos.network/main/user/run-node/interact-node#query-for-historical-state-using-grpcurl

That's only true for backend applications, and if we can guarantee the API config of full nodes running the REST API (which I assumed we could not because we don't control all the nodes, but possibly we can just enforce the configuration on the node that we control?). The REST API nodes need to whitelist x-cosmos-block-height header in API response Access-Control-Allow-Headers headers. At the moment REST API https://rest-kralum.neutron-1.neutron.org does not allow that header to be sent, and results in a CORS error to any application not hosted at https://rest-kralum.neutron-1.neutron.org:

Access to fetch at 'https://rest-kralum.neutron-1.neutron.org/cosmos/bank/v1beta1/balances/1'  has been blocked by CORS policy: Request header field x-cosmos-block-height is not allowed by Access-Control-Allow-Headers in preflight response.

If we can guarantee that config for the API that the web app will use then yes, I could send those headers and avoid needing this data from the same endpoint.