Invalid oracle versions can cause desync of global and local positions making protocol lose funds and being unable to pay back all users
In more details, if there are 2 pending positions with timestamps different by 2 oracle versions and the first of them has invalid oracle version at its timestamp, then there are 2 different position flows possible depending on the time when the position is settled (update transaction called):\\nFor earlier update the flow is: previous position (oracle v1) -> position 1 (oracle v2) -> position 2 (oracle v3)\\nFor later update position 1 is skipped completely (the fees for the position are also not taken) and the flow is: previous position (oracle v1) -> invalidated position 1 (in the other words: previous position again) (oracle v2) -> position 2 (oracle v3)\\nWhile the end result (position 2) is the same, it's possible that pending global position is updated earlier (goes the 1st path), while the local position is updated later (goes the 2nd path). For a short time (between oracle versions 2 and 3), the global position will accumulate everything (including profit and loss) using the pending position 1 long/short/maker values, but local position will accumulate everything using the previous position with different values.\\nConsider the following scenario: Oracle uses granularity = 100. Initially user B opens position maker = 2 with collateral = 100. T=99: User A opens long = 1 with collateral = 100 (pending position long=1 timestamp=100) T=100: Oracle fails to commit this version, thus it becomes invalid T=201: At this point oracle version at timestamp 200 is not yet commited, but the new positions are added with the next timestamp = 300: User A closes his long position (update(0,0,0,0)) (pending position: long=1 timestamp=100; long=0 timestamp=300) At this point, current global long position is still 0 (pending the same as user A local pending positions)\\nT=215: Oracle commits version with timestamp = 200, price = $100 T=220: User B settles (update(2,0,0,0) - keeping the same position). At this point the latest oracle version is the one at timestamp = 200, so this update triggers update of global pending positions, and current latest global position is now long = 1.0 at timestamp = 200. T=315: Oracle commits version with timestamp = 300, price = $90 after settlement of both UserA and UserB, we have the following:\\nGlobal position settlement. It accumulates position [maker = 2.0, long = 1.0] from timestamp = 200 (price=$100) to timestamp = 300 (price=$90). In particular: longPnl = 1*($90-$100) = -$10 makerPnl = -longPnl = +$10\\nUser B local position settlement. It accumulates position [maker = 2.0] from timestamp = 200 to timestamp = 300, adding makerPnl ($10) to user B collateral. So user B collateral = $110\\nUser A local position settlement. When accumulating, pending position 1 (long = 1, timestamp = 100) is invalidated to previous position (long = 0) and also fees are set to 0 by invalidation. So user A local accumulates position [long = 0] from timestamp = 0 to timestamp = 300 (next pending position), this doesn't change collateral at all (remains $100). Then the next pending position [long = 0] becomes the latest position (basically position of long=1 was completely ignored as if it has not existed).\\nResult: User A deposited $100, User B deposited $100 (total $200 deposited) after the scenario above: User A has collateral $110, User B has collateral $100 (total $210 collateral withdrawable) However, protocol only has $200 deposited. This means that the last user will be unable to withdraw the last $10 since protocol doesn't have it, leading to a user loss of funds.\\nThe scenario above is demonstrated in the test, add this to test/unit/market/Market.test.ts:\\n```\\nit('panprog global-local desync', async () => {\\n    const positionMaker = parse6decimal('2.000')\\n    const positionLong = parse6decimal('1.000')\\n    const collateral = parse6decimal('100')\\n\\n    const oracleVersion = {\\n        price: parse6decimal('100'),\\n        timestamp: TIMESTAMP,\\n        valid: true,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion.timestamp).returns(oracleVersion)\\n    oracle.status.returns([oracleVersion, oracleVersion.timestamp + 100])\\n    oracle.request.returns()\\n\\n    dsu.transferFrom.whenCalledWith(userB.address, market.address, collateral.mul(1e12)).returns(true)\\n    await market.connect(userB).update(userB.address, positionMaker, 0, 0, collateral, false)\\n\\n    const oracleVersion2 = {\\n        price: parse6decimal('100'),\\n        timestamp: TIMESTAMP + 100,\\n        valid: true,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion2.timestamp).returns(oracleVersion2)\\n    oracle.status.returns([oracleVersion2, oracleVersion2.timestamp + 100])\\n    oracle.request.returns()\\n\\n    dsu.transferFrom.whenCalledWith(user.address, market.address, collateral.mul(1e12)).returns(true)\\n    await market.connect(user).update(user.address, 0, positionLong, 0, collateral, false)\\n\\n    var info = await market.locals(userB.address);\\n    console.log("collateral deposit maker: " + info.collateral);\\n    var info = await market.locals(user.address);\\n    console.log("collateral deposit long: " + info.collateral);\\n\\n    // invalid oracle version\\n    const oracleVersion3 = {\\n        price: 0,\\n        timestamp: TIMESTAMP + 200,\\n        valid: false,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion3.timestamp).returns(oracleVersion3)\\n\\n    // next oracle version is valid\\n    const oracleVersion4 = {\\n        price: parse6decimal('100'),\\n        timestamp: TIMESTAMP + 300,\\n        valid: true,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion4.timestamp).returns(oracleVersion4)\\n\\n    // still returns oracleVersion2, because nothing commited for version 3, and version 4 time has passed but not yet commited\\n    oracle.status.returns([oracleVersion2, oracleVersion4.timestamp + 100])\\n    oracle.request.returns()\\n\\n    // reset to 0\\n    await market.connect(user).update(user.address, 0, 0, 0, 0, false)\\n\\n    // oracleVersion4 commited\\n    oracle.status.returns([oracleVersion4, oracleVersion4.timestamp + 100])\\n    oracle.request.returns()\\n\\n    // settle\\n    await market.connect(userB).update(userB.address, positionMaker, 0, 0, 0, false)\\n\\n    const oracleVersion5 = {\\n        price: parse6decimal('90'),\\n        timestamp: TIMESTAMP + 400,\\n        valid: true,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion5.timestamp).returns(oracleVersion5)\\n    oracle.status.returns([oracleVersion5, oracleVersion5.timestamp + 100])\\n    oracle.request.returns()\\n\\n    // settle\\n    await market.connect(userB).update(userB.address, positionMaker, 0, 0, 0, false)\\n    await market.connect(user).update(user.address, 0, 0, 0, 0, false)\\n\\n    var info = await market.locals(userB.address);\\n    console.log("collateral maker: " + info.collateral);\\n    var info = await market.locals(user.address);\\n    console.log("collateral long: " + info.collateral);\\n})\\n```\\n\\nConsole output for the code:\\n```\\ncollateral deposit maker: 100000000\\ncollateral deposit long: 100000000\\ncollateral maker: 110000028\\ncollateral long: 100000000\\n```\\n\\nMaker has a bit more than $110 in the end, because he also earns funding and interest during the short time when ephemeral long position is active (but user A doesn't pay these fees).
The issue is that positions with invalid oracle versions are ignored until the first valid oracle version, however the first valid version can be different for global and local positions. One of the solutions I see is to introduce a map of position timestamp -> oracle version to settle, which will be filled by global position processing. Local position processing will follow the same path as global using this map, which should eliminate possibility of different paths for global and local positions.\\nIt might seem that the issue can only happen with exactly 1 oracle version between invalid and valid positions. However, it's also possible that some non-requested oracle versions are commited (at some random timestamps between normal oracle versions) and global position will go via the route like t100[pos0]->t125[pos1]->t144[pos1]->t200[pos2] while local one will go t100[pos0]->t200[pos2] OR it can also go straight to t300 instead of t200 etc. So the exact route can be anything, and local oracle will have to follow it, that's why I suggest a path map.\\nThere might be some other solutions possible.
Any time the oracle skips a version (invalid version), it's likely that global and local positions for different users who try to trade during this time will desync, leading to messed up accounting and loss of funds for users or protocol, potentially triggering a bank run with the last user being unable to withdraw all funds.\\nThe severity of this issue is high, because while invalid versions are normally a rare event, however in the current state of the codebase there is a bug that pyth oracle requests are done using this block timestamp instead of granulated future time (as positions do), which leads to invalid oracle versions almost for all updates (that bug is reported separately). Due to this other bug, the situation described in this issue will arise very often by itself in a normal flow of the user requests, so it's almost 100% that internal accounting for any semi-active market will be broken and total user collateral will deviate away from real deposited funds, meaning the user funds loss.\\nBut even with that other bug fixed, the invalid oracle version is a normal protocol event and even 1 such event might be enough to break internal market accounting.
```\\nit('panprog global-local desync', async () => {\\n    const positionMaker = parse6decimal('2.000')\\n    const positionLong = parse6decimal('1.000')\\n    const collateral = parse6decimal('100')\\n\\n    const oracleVersion = {\\n        price: parse6decimal('100'),\\n        timestamp: TIMESTAMP,\\n        valid: true,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion.timestamp).returns(oracleVersion)\\n    oracle.status.returns([oracleVersion, oracleVersion.timestamp + 100])\\n    oracle.request.returns()\\n\\n    dsu.transferFrom.whenCalledWith(userB.address, market.address, collateral.mul(1e12)).returns(true)\\n    await market.connect(userB).update(userB.address, positionMaker, 0, 0, collateral, false)\\n\\n    const oracleVersion2 = {\\n        price: parse6decimal('100'),\\n        timestamp: TIMESTAMP + 100,\\n        valid: true,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion2.timestamp).returns(oracleVersion2)\\n    oracle.status.returns([oracleVersion2, oracleVersion2.timestamp + 100])\\n    oracle.request.returns()\\n\\n    dsu.transferFrom.whenCalledWith(user.address, market.address, collateral.mul(1e12)).returns(true)\\n    await market.connect(user).update(user.address, 0, positionLong, 0, collateral, false)\\n\\n    var info = await market.locals(userB.address);\\n    console.log("collateral deposit maker: " + info.collateral);\\n    var info = await market.locals(user.address);\\n    console.log("collateral deposit long: " + info.collateral);\\n\\n    // invalid oracle version\\n    const oracleVersion3 = {\\n        price: 0,\\n        timestamp: TIMESTAMP + 200,\\n        valid: false,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion3.timestamp).returns(oracleVersion3)\\n\\n    // next oracle version is valid\\n    const oracleVersion4 = {\\n        price: parse6decimal('100'),\\n        timestamp: TIMESTAMP + 300,\\n        valid: true,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion4.timestamp).returns(oracleVersion4)\\n\\n    // still returns oracleVersion2, because nothing commited for version 3, and version 4 time has passed but not yet commited\\n    oracle.status.returns([oracleVersion2, oracleVersion4.timestamp + 100])\\n    oracle.request.returns()\\n\\n    // reset to 0\\n    await market.connect(user).update(user.address, 0, 0, 0, 0, false)\\n\\n    // oracleVersion4 commited\\n    oracle.status.returns([oracleVersion4, oracleVersion4.timestamp + 100])\\n    oracle.request.returns()\\n\\n    // settle\\n    await market.connect(userB).update(userB.address, positionMaker, 0, 0, 0, false)\\n\\n    const oracleVersion5 = {\\n        price: parse6decimal('90'),\\n        timestamp: TIMESTAMP + 400,\\n        valid: true,\\n    }\\n    oracle.at.whenCalledWith(oracleVersion5.timestamp).returns(oracleVersion5)\\n    oracle.status.returns([oracleVersion5, oracleVersion5.timestamp + 100])\\n    oracle.request.returns()\\n\\n    // settle\\n    await market.connect(userB).update(userB.address, positionMaker, 0, 0, 0, false)\\n    await market.connect(user).update(user.address, 0, 0, 0, 0, false)\\n\\n    var info = await market.locals(userB.address);\\n    console.log("collateral maker: " + info.collateral);\\n    var info = await market.locals(user.address);\\n    console.log("collateral long: " + info.collateral);\\n})\\n```\\n