Merge pull request #402 from omisego/216_fix_root_chain

IFE with deposit tx has wrong priority
omgnetwork · Oct 29, 2019 · 0cbd6fd · 0cbd6fd
2 parents 2ff791e + 78ba5d1
commit 0cbd6fd
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 13 deletions.
diff --git a/contracts/RootChain.sol b/contracts/RootChain.sol
@@ -1066,19 +1066,27 @@ contract RootChain {
      * @param _utxoPos Output identifier.
      * @return uint256 Timestamp after which this output is exitable.
      */
-    function getExitableTimestamp(uint256 _utxoPos)
+    function getTxExitableTimestamp(uint256 _utxoPos)
         public
         view
         returns (uint256)
     {
         uint256 blknum = _utxoPos.getBlknum();
-        if (_isDeposit(blknum)) {
-            // High priority exit for the deposit.
-            return block.timestamp + minExitPeriod;
-        }
-        else {
-            return Math.max(blocks[blknum].timestamp + (minExitPeriod * 2), block.timestamp + minExitPeriod);
-        }
+        return Math.max(blocks[blknum].timestamp + (minExitPeriod * 2), block.timestamp + minExitPeriod);
+    }
+
+    /**
+     * @dev Given an utxo position of a deposit, determines when it's exitable, if it were to be exited now.
+     * @notice Should only be used to boost priority of the SE from a deposit.
+     * @param _utxoPos Output identifier.
+     * @return uint256 Timestamp after which this output is exitable.
+     */
+    function getDepositTxExitableTimestamp(uint256 _utxoPos)
+        public
+        view
+        returns (uint256)
+    {
+        return block.timestamp + minExitPeriod;
     }
 
     /**
@@ -1112,8 +1120,13 @@ contract RootChain {
         view
         returns (uint256)
     {
-        uint256 tx_pos = _utxoPos.getTxPos();
-        return ((getExitableTimestamp(_utxoPos) << 214) | (tx_pos << 160)) | _exitId;
+        uint256 exitableTimestamp;
+        if (_isDeposit(_utxoPos.getBlknum())) {
+            exitableTimestamp = getDepositTxExitableTimestamp(_utxoPos);
+        } else {
+            exitableTimestamp = getTxExitableTimestamp(_utxoPos);
+        }
+        return _getExitPriority(exitableTimestamp, _exitId, _utxoPos);
     }
 
     /**
@@ -1126,7 +1139,17 @@ contract RootChain {
         view
         returns (uint256)
     {
-        return getStandardExitPriority(getInFlightExitId(_tx), _txoPos);
+      uint256 exitableTimestamp = getTxExitableTimestamp(_txoPos);
+      return _getExitPriority(exitableTimestamp, getInFlightExitId(_tx), _txoPos);
+    }
+
+    function _getExitPriority(uint256 _exitableTimestamp, uint192 _exitId, uint256 _utxoPos)
+        private
+        pure
+        returns (uint256)
+    {
+        uint256 tx_pos = _utxoPos.getTxPos();
+        return ((_exitableTimestamp << 214) | (tx_pos << 160)) | _exitId;
     }
 
     /**

diff --git a/tests/contracts/root_chain/test_process_exits.py b/tests/contracts/root_chain/test_process_exits.py
@@ -84,6 +84,26 @@ def test_finalize_exits_for_erc20_should_succeed(testlang, root_chain, token):
     assert token.balanceOf(owner.address) == pre_balance + amount
 
 
+def test_finalize_in_flight_exit_with_deposit_is_mature_after_mfp_plus_rep(testlang):
+    owner, amount = testlang.accounts[0], 100
+    deposit_id = testlang.deposit(owner, amount)
+    spend_id = testlang.spend_utxo([deposit_id], [owner.key], [(owner.address, NULL_ADDRESS, 100)])
+    testlang.start_in_flight_exit(spend_id)
+    testlang.piggyback_in_flight_exit_output(spend_id, 0, owner.key)
+
+    testlang.forward_timestamp(MIN_EXIT_PERIOD + 1)
+    testlang.process_exits(NULL_ADDRESS, 0, 100)
+    in_flight_exit = testlang.get_in_flight_exit(spend_id)
+    assert in_flight_exit.exit_start_timestamp != 0
+
+    testlang.forward_timestamp(MIN_EXIT_PERIOD)
+    testlang.process_exits(NULL_ADDRESS, 0, 100)
+    in_flight_exit = testlang.get_in_flight_exit(spend_id)
+    assert in_flight_exit.exit_start_timestamp == 0
+    assert in_flight_exit.bond_owner == NULL_ADDRESS_HEX
+    assert in_flight_exit.oldest_competitor == 0
+
+
 def test_finalize_exits_old_utxo_is_mature_after_single_mfp(testlang):
     minimal_finalization_period = MIN_EXIT_PERIOD  # aka MFP - see tesuji blockchain design
     required_exit_period = MIN_EXIT_PERIOD  # aka REP - see tesuji blockchain design
@@ -94,12 +114,14 @@ def test_finalize_exits_old_utxo_is_mature_after_single_mfp(testlang):
 
     testlang.forward_timestamp(required_exit_period)
     testlang.start_standard_exit(spend_id, owner.key)
-    testlang.forward_timestamp(minimal_finalization_period)
 
+    testlang.forward_timestamp(minimal_finalization_period)
     assert testlang.get_standard_exit(spend_id).owner == owner.address
+
     testlang.process_exits(NULL_ADDRESS, 0, 100)
-    testlang.forward_timestamp(1)
     assert testlang.get_standard_exit(spend_id).owner == owner.address
+
+    testlang.forward_timestamp(1)
     testlang.process_exits(NULL_ADDRESS, 0, 100)
     assert testlang.get_standard_exit(spend_id).owner == NULL_ADDRESS_HEX