fix: fix broken canonicity in IFE processing (#591)

plasma_framework/contracts/mocks/exits/SpyPlasmaFrameworkForExitGame.sol

@@ -58,8 +58,8 @@ contract SpyPlasmaFrameworkForExitGame is PlasmaFramework {
         blocks[_blockNum] = BlockModel.Block(_root, _timestamp);
     }
 
-    function setOutputFinalized(bytes32 _outputId) external {
-        isOutputFinalized[_outputId] = true;
+    function setOutputFinalized(bytes32 _outputId, uint160 _exitId) external {
+        outputsFinalizations[_outputId] = _exitId;
     }
 
     /**

plasma_framework/contracts/mocks/exits/payment/routers/PaymentInFlightExitRouterMock.sol

@@ -70,8 +70,8 @@ contract PaymentInFlightExitRouterMock is FailFastReentrancyGuard, PaymentInFlig
     }
 
     /** calls the flagOutputFinalized function on behalf of the exit game */
-    function proxyFlagOutputFinalized(bytes32 _outputId) public {
-        framework.flagOutputFinalized(_outputId);
+    function proxyFlagOutputFinalized(bytes32 outputId, uint160 exitId) public {
+        framework.flagOutputFinalized(outputId, exitId);
     }
 
     /**

plasma_framework/contracts/mocks/exits/payment/routers/PaymentStandardExitRouterMock.sol

@@ -25,12 +25,12 @@ contract PaymentStandardExitRouterMock is PaymentStandardExitRouter {
     }
 
     /** helper functions for testing */
-    function setExit(uint160 _exitId, PaymentExitDataModel.StandardExit memory _exitData) public {
-        PaymentStandardExitRouter.standardExitMap.exits[_exitId] = _exitData;
+    function setExit(uint160 exitId, PaymentExitDataModel.StandardExit memory exitData) public {
+        PaymentStandardExitRouter.standardExitMap.exits[exitId] = exitData;
     }
 
-    function proxyFlagOutputFinalized(bytes32 _outputId) public {
-        framework.flagOutputFinalized(_outputId);
+    function proxyFlagOutputFinalized(bytes32 outputId, uint160 exitId) public {
+        framework.flagOutputFinalized(outputId, exitId);
     }
 
     function depositFundForTest() public payable {}

plasma_framework/contracts/mocks/framework/DummyExitGame.sol

@@ -41,35 +41,35 @@ contract DummyExitGame is IExitProcessor {
         exitGameController = ExitGameController(_contract);
     }
 
-    function enqueue(uint256 _vaultId, address _token, uint64 _exitableAt, uint256 _txPos, uint160 _exitId, IExitProcessor _exitProcessor)
+    function enqueue(uint256 vaultId, address token, uint64 exitableAt, uint256 txPos, uint160 exitId, IExitProcessor exitProcessor)
         public
     {
-        priorityFromEnqueue = exitGameController.enqueue(_vaultId, _token, _exitableAt, PosLib.decode(_txPos), _exitId, _exitProcessor);
+        priorityFromEnqueue = exitGameController.enqueue(vaultId, token, exitableAt, PosLib.decode(txPos), exitId, exitProcessor);
     }
 
-    function proxyBatchFlagOutputsFinalized(bytes32[] memory _outputIds) public {
-        exitGameController.batchFlagOutputsFinalized(_outputIds);
+    function proxyBatchFlagOutputsFinalized(bytes32[] memory outputIds, uint160 exitId) public {
+        exitGameController.batchFlagOutputsFinalized(outputIds, exitId);
     }
 
-    function proxyFlagOutputFinalized(bytes32 _outputId) public {
-        exitGameController.flagOutputFinalized(_outputId);
+    function proxyFlagOutputFinalized(bytes32 outputId, uint160 exitId) public {
+        exitGameController.flagOutputFinalized(outputId, exitId);
     }
 
     // setter function only for test, not a real Exit Game function
-    function setEthVault(EthVault _vault) public {
-        ethVault = _vault;
+    function setEthVault(EthVault vault) public {
+        ethVault = vault;
     }
 
-    function proxyEthWithdraw(address payable _target, uint256 _amount) public {
-        ethVault.withdraw(_target, _amount);
+    function proxyEthWithdraw(address payable target, uint256 amount) public {
+        ethVault.withdraw(target, amount);
     }
 
     // setter function only for test, not a real Exit Game function
-    function setErc20Vault(Erc20Vault _vault) public {
-        erc20Vault = _vault;
+    function setErc20Vault(Erc20Vault vault) public {
+        erc20Vault = vault;
     }
 
-    function proxyErc20Withdraw(address payable _target, address _token, uint256 _amount) public {
-        erc20Vault.withdraw(_target, _token, _amount);
+    function proxyErc20Withdraw(address payable target, address token, uint256 amount) public {
+        erc20Vault.withdraw(target, token, amount);
     }
 }

plasma_framework/contracts/src/exits/payment/controllers/PaymentProcessInFlightExit.sol

@@ -65,12 +65,26 @@ library PaymentProcessInFlightExit {
             return;
         }
 
-        // Check whether any input is already spent. Required to prevent operator stealing funds.
-        // See: https://github.com/omisego/plasma-contracts/issues/102#issuecomment-495809967
-        // Also, slightly different from the solution above, we treat input spent as non-canonical.
-        // So an IFE is only canonical if all inputs of the in-flight tx are not double spent by competing tx or exit.
-        // see: https://github.com/omisego/plasma-contracts/issues/470
-        if (!exit.isCanonical || isAnyInputSpent(self.framework, exit, token)) {
+        /* To prevent a double spend, it is needed to know if an output can be exited.
+         * An output can not be exited if:
+         * - it is finalized by a standard exit
+         * - it is finalized by an in-flight exit as input of a non-canonical transaction
+         * - it is blocked from exiting, because it is an input of a canonical transaction
+         *   that exited from one of it's outputs
+         * - it is finalized by an in-flight exit as an output of a canonical transaction
+         * - it is an output of a transaction for which at least one of its inputs is already finalized
+         *
+         * Hence, Plasma Framework stores each output with an exit id that finalized it.
+         * When transaction is marked as canonical but any of it's input was finalized by
+         * other exit, it is not allowed to exit from the transaction's outputs.
+         * In that case exit from an unspent input is possible.
+         * When all inputs of a transaction that is marked as canonical are either not finalized or finalized
+         * by the same exit (which means they were marked as finalized when processing the same exit for a different token),
+         * only exit from outputs is possible.
+         *
+         * See: https://github.com/omisego/plasma-contracts/issues/102#issuecomment-495809967 for more details
+         */
+        if (!exit.isCanonical || isAnyInputFinalizedByOtherExit(self.framework, exit, exitId)) {
             for (uint16 i = 0; i < exit.inputs.length; i++) {
                 PaymentExitDataModel.WithdrawData memory withdrawal = exit.inputs[i];
 
@@ -80,7 +94,7 @@ library PaymentProcessInFlightExit {
                 }
             }
 
-            flagOutputsWhenNonCanonical(self.framework, exit, token);
+            flagOutputsWhenNonCanonical(self.framework, exit, token, exitId);
         } else {
             for (uint16 i = 0; i < exit.outputs.length; i++) {
                 PaymentExitDataModel.WithdrawData memory withdrawal = exit.outputs[i];
@@ -91,7 +105,7 @@ library PaymentProcessInFlightExit {
                 }
             }
 
-            flagOutputsWhenCanonical(self.framework, exit, token);
+            flagOutputsWhenCanonical(self.framework, exit, token, exitId);
         }
 
         returnInputPiggybackBonds(self, exit, token);
@@ -113,30 +127,30 @@ library PaymentProcessInFlightExit {
         }
     }
 
-    function isAnyInputSpent(
+    function isAnyInputFinalizedByOtherExit(
         PlasmaFramework framework,
         PaymentExitDataModel.InFlightExit memory exit,
-        address token
+        uint160 exitId
     )
         private
         view
         returns (bool)
     {
-        uint256 inputNumOfTheToken;
+        uint256 nonEmptyInputIndex;
         for (uint16 i = 0; i < exit.inputs.length; i++) {
-            if (exit.inputs[i].token == token && !exit.isInputEmpty(i)) {
-                inputNumOfTheToken++;
+            if (!exit.isInputEmpty(i)) {
+                nonEmptyInputIndex++;
             }
         }
-        bytes32[] memory outputIdsOfInputs = new bytes32[](inputNumOfTheToken);
-        uint sameTokenIndex = 0;
+        bytes32[] memory outputIdsOfInputs = new bytes32[](nonEmptyInputIndex);
+        nonEmptyInputIndex = 0;
         for (uint16 i = 0; i < exit.inputs.length; i++) {
-            if (exit.inputs[i].token == token && !exit.isInputEmpty(i)) {
-                outputIdsOfInputs[sameTokenIndex] = exit.inputs[i].outputId;
-                sameTokenIndex++;
+            if (!exit.isInputEmpty(i)) {
+                outputIdsOfInputs[nonEmptyInputIndex] = exit.inputs[i].outputId;
+                nonEmptyInputIndex++;
             }
         }
-        return framework.isAnyOutputFinalized(outputIdsOfInputs);
+        return framework.isAnyInputFinalizedByOtherExit(outputIdsOfInputs, exitId);
     }
 
     function shouldWithdrawInput(
@@ -187,7 +201,8 @@ library PaymentProcessInFlightExit {
     function flagOutputsWhenNonCanonical(
         PlasmaFramework framework,
         PaymentExitDataModel.InFlightExit memory exit,
-        address token
+        address token,
+        uint160 exitId
     )
         private
     {
@@ -206,19 +221,20 @@ library PaymentProcessInFlightExit {
                 indexForOutputIds++;
             }
         }
-        framework.batchFlagOutputsFinalized(outputIdsToFlag);
+        framework.batchFlagOutputsFinalized(outputIdsToFlag, exitId);
     }
 
     function flagOutputsWhenCanonical(
         PlasmaFramework framework,
         PaymentExitDataModel.InFlightExit memory exit,
-        address token
+        address token,
+        uint160 exitId
     )
         private
     {
         uint256 inputNumOfTheToken;
         for (uint16 i = 0; i < exit.inputs.length; i++) {
-            if (exit.inputs[i].token == token && !exit.isInputEmpty(i)) {
+            if (!exit.isInputEmpty(i)) {
                 inputNumOfTheToken++;
             }
         }
@@ -233,7 +249,7 @@ library PaymentProcessInFlightExit {
         bytes32[] memory outputIdsToFlag = new bytes32[](inputNumOfTheToken + piggybackedOutputNumOfTheToken);
         uint indexForOutputIds = 0;
         for (uint16 i = 0; i < exit.inputs.length; i++) {
-            if (exit.inputs[i].token == token && !exit.isInputEmpty(i)) {
+            if (!exit.isInputEmpty(i)) {
                 outputIdsToFlag[indexForOutputIds] = exit.inputs[i].outputId;
                 indexForOutputIds++;
             }
@@ -244,7 +260,7 @@ library PaymentProcessInFlightExit {
                 indexForOutputIds++;
             }
         }
-        framework.batchFlagOutputsFinalized(outputIdsToFlag);
+        framework.batchFlagOutputsFinalized(outputIdsToFlag, exitId);
     }
 
     function returnInputPiggybackBonds(

plasma_framework/contracts/src/exits/payment/controllers/PaymentProcessStandardExit.sol

@@ -54,7 +54,7 @@ library PaymentProcessStandardExit {
             return;
         }
 
-        self.framework.flagOutputFinalized(exit.outputId);
+        self.framework.flagOutputFinalized(exit.outputId, exitId);
 
         // we do not want to block a queue if bond return is unsuccessful
         bool success = SafeEthTransfer.transferReturnResult(exit.exitTarget, exit.bondSize, self.safeGasStipend);

plasma_framework/contracts/src/framework/ExitGameController.sol

@@ -18,8 +18,8 @@ contract ExitGameController is ExitGameRegistry {
     mapping (bytes32 => IExitProcessor) public delegations;
     // hashed (vault id, token) => PriorityQueue
     mapping (bytes32 => PriorityQueue) public exitsQueues;
-    // outputId => bool
-    mapping (bytes32 => bool) public isOutputFinalized;
+    // outputId => exitId
+    mapping (bytes32 => uint160) public outputsFinalizations;
     bool private mutex = false;
 
     event ExitQueueAdded(
@@ -189,33 +189,46 @@ contract ExitGameController is ExitGameRegistry {
      * @notice Checks whether any of the output with the given outputIds is already spent
      * @param _outputIds Output IDs to check
      */
-    function isAnyOutputFinalized(bytes32[] calldata _outputIds) external view returns (bool) {
+    function isAnyInputFinalizedByOtherExit(bytes32[] calldata _outputIds, uint160 exitId) external view returns (bool) {
         for (uint i = 0; i < _outputIds.length; i++) {
-            if (isOutputFinalized[_outputIds[i]] == true) {
+            uint160 finalizedExitId = outputsFinalizations[_outputIds[i]];
+            if (finalizedExitId != 0 && finalizedExitId != exitId) {
                 return true;
             }
         }
         return false;
     }
 
     /**
-     * @notice Batch flags already spent outputs
-     * @param _outputIds Output IDs to flag
+     * @notice Batch flags already spent outputs (only not already spent)
+     * @param outputIds Output IDs to flag
      */
-    function batchFlagOutputsFinalized(bytes32[] calldata _outputIds) external onlyFromNonQuarantinedExitGame {
-        for (uint i = 0; i < _outputIds.length; i++) {
-            require(_outputIds[i] != bytes32(""), "Should not flag with empty outputId");
-            isOutputFinalized[_outputIds[i]] = true;
+    function batchFlagOutputsFinalized(bytes32[] calldata outputIds, uint160 exitId) external onlyFromNonQuarantinedExitGame {
+        for (uint i = 0; i < outputIds.length; i++) {
+            require(outputIds[i] != bytes32(""), "Should not flag with empty outputId");
+            if (outputsFinalizations[outputIds[i]] == 0) {
+                outputsFinalizations[outputIds[i]] = exitId;
+            }
         }
     }
 
     /**
-     * @notice Flags a single output as spent
-     * @param _outputId The output ID to flag as spent
+     * @notice Flags a single output as spent if it is not flagged already
+     * @param outputId The output ID to flag as spent
+     */
+    function flagOutputFinalized(bytes32 outputId, uint160 exitId) external onlyFromNonQuarantinedExitGame {
+        require(outputId != bytes32(""), "Should not flag with empty outputId");
+        if (outputsFinalizations[outputId] == 0) {
+            outputsFinalizations[outputId] = exitId;
+        }
+    }
+
+     /**
+     * @notice Checks whether output with a given outputId is finalized
+     * @param outputId Output ID to check
      */
-    function flagOutputFinalized(bytes32 _outputId) external onlyFromNonQuarantinedExitGame {
-        require(_outputId != bytes32(""), "Should not flag with empty outputId");
-        isOutputFinalized[_outputId] = true;
+    function isOutputFinalized(bytes32 outputId) external view returns (bool) {
+        return outputsFinalizations[outputId] != 0;
     }
 
     function getNextExit(uint256 vaultId, address token) external view returns (uint256) {