Call price is inconsistent when MCR changed

[abitmore]

Bug Description
According to discussion in https://bitsharestalk.org/index.php?topic=26496.0

... in current system, we don't refresh the "call price" cache when MCR changed, that said, existing short positions' call prices can be inconsistent with latest MCR. In the meanwhile, call prices of new positions or updated positions will be always up-to-date as long as MCR didn't change. This may lead to unintended margin calls when MCR decreased, or unintended inability to call when MCR increased.

To solve the cache inconsistency issue, we have options:

1. totally drop the caching mechanism, calculate call prices on the fly (may impact order matching performance)

2. update "call price" cache immediately when MCR changed (may impact performance since possibly need to update lots of data at a time)

3. update "call price" cache at maintenance interval since maintenance interval is meant to be used to process mass calculation

3.1 if we don't change MCR update behavior, aka update "current effective" MCR directly when feed changed, then the call price cache will be still inconsistent before maintenance interval;

3.2 we can defer MCR update to maintenance interval, that said, when detected a MCR change due to feed change or asset option e.g. feed lifetime change, don't apply the MCR update immediately, but apply it in next maintenance interval. With this approach, the call price data will be always consistent to "current effective" MCR.

#941 is an attempt to implement 3.2.

Later discussion suggests it's better to go with option 1, and perhaps can use a cache on median price feed to improve performance (need test).

Update: finally went with option 1 and fixed by #1324.

Impacts
Describe which portion(s) of BitShares Core may be impacted by this bug. Please tick at least one box.

• API (the application programming interface)
• Build (the build process or something prior to compiled code)
• CLI (the command line wallet)
• Deployment (the deployment process after building such as Docker, Travis, etc.)
• DEX (the Decentralized EXchange, market engine, etc.)
• P2P (the peer-to-peer network for transaction/block propagation)
• Performance (system or user efficiency, etc.)
• Protocol (the blockchain logic, consensus, validation, etc.)
• Security (the security of system or user data, etc.)
• UX (the User Experience)
• Other (please add below)

CORE TEAM TASK LIST

• Evaluate / Prioritize Bug Report
• Refine User Stories / Requirements
• Define Test Cases
• Design / Develop Solution
• Perform QA/Testing
• Update Documentation

[abitmore]

Note: #460 makes the situation a bit more complicated, that said, MCR is now affecting PMs as well although it shouldn't. In principle we keep the behavior unchanged, but for better code maintenance-ability, it's better to still use same code for both PMs and non-PM bitAssets (that means we may change the behavior of PMs a bit).

[abitmore]

I think it's better to modify the median calculation a bit.

Given:

MCP  = settlement_price * MCR;
MSSP = settlement_price * MSSR;
feed = { settlement_price, MCR, MSSR, CER,
         (MCP), (MSSP) } // using `()` here because they are derivations

Explanation: for call orders, define CP = collateral_amount/debt_amount for call orders. When CP is smaller than MCP, the order enters margin call territory (will get margin called if can match a limit order on the opposite position). The worst price it will sell its collateral is MSSP.

Currently,

median_feed = { median(settlement_price), median(MCR), median(MSSR), median(CER),
                median(settlement_price) * median(MCR),
                median(settlement_price) * median(MSSR) }

When MCR and MSSR are static, curve of median MCP and MSSR would be smooth, and result would be expected; when MCR or MSSR is dynamic, we may get unexpected result. I think the formula below is better:

median_feed = { median(settlement_price), median(MCR), median(MSSR), median(CER),
                median(settlement_price * MCR),
                median(settlement_price * MSSR) }

For example, given this data set:

feed1 = { price=1.0, MCR=2.5 }
feed2 = { price=2.0, MCR=1.5 }
feed3 = { price=3.0, MCR=2.0 }

Result of current formula would be:

median_MCP = median(1.0, 2.0, 3.0) * median(2.5, 1.5, 2.0) = 2.0 * 2.0 = 4.0

Result of the revised formula would be:

median_MCP = median(1.0*2.5, 2.0*1.5, 3.0*2.0) = median(2.5, 3.0, 6.0) = 3.0

Thoughts?

[pmconrad]

I think it makes more sense to view SP, MCR and MSSR as independent values, and it is easier for market participants to think in terms of "I need 1.75 times collateral" and "on margin call I will lose 10%". With the new formula, behaviour would be much less predictable and consequently much more difficult to follow.

[abitmore]

@pmconrad I disagree. MCR and MSSR are subjective parameters, while settlement price was objective before BSIP42. For objective data, it makes sense to get the median input; but for subjective data, IMHO it's better to get the input as a whole to achieve combined effects. Combining subjective input from different sources will likely lead to unexpected consequences.

For better UX, we need to keep the numbers consistent, that said, if we use this formula:

median_feed = { median(settlement_price), median(MCR), median(MSSR), median(CER),
                median(settlement_price * MCR),
                median(settlement_price * MSSR) }

It's possible that median_feed.(MCP) != median_feed.settlement_price * median_feed.MCR.

To solve this inconsistency, we may change the algorithm to:

median_MCP = median((MCP)) = median(settlement_price * MCR);
accepted_feed = the corresponding feed whose (MCP) has been assigned to median_MCP;
median_feed = accepted_feed;

Note: it's possible that median(settlement_price * MCR) and median(settlement_price * MSSR) are from different witnesses. For consistency, we may need to use all data from one witness. However, if the other data is not median, it means it can be far off thus possibly lead to serious issues.

Thoughts?

[abitmore]

Fixed by #1324.