omgnetwork / plasma-mvp

OmiseGO's research implementation of Minimal Viable Plasma
MIT License
561 stars 158 forks source link

Successful exit doesn't mark UTXO as spent #23

Closed jdkanani closed 6 years ago

jdkanani commented 6 years ago

finalizeExits doesn't mark UTXO as spent which can cause multiple exits on single UTXO with multiple startExit.

smartcontracts commented 6 years ago

This is a known issue and something I've discussed with David. There's also an additional issue that UTXOs that have been successfully challenged can be resubmitted. There are a few possible ways to handle this, all of them take some minor changes to the smart contract code. The simplest way (IMO) is to add a status field to the UTXO list, but that fails to take advantage of the deletes used to save space on chain. No matter what, successful or successfully challenged exits need to be stored on chain in some way in order to ensure users can't exit twice. Suggestions are welcome!

jdkanani commented 6 years ago

The first thing that comes to my mind is to create the list of exit transactions (output as [address(0), 0]) and create a block.

smartcontracts commented 6 years ago

If blocks are being withheld from the plasma chain, then creating additional blocks may not work. There's a potential case where an output is spent in the latest (withheld) block but not in the latest available one. Creating a block with a single exit transaction in a similar fashion to deposit() could result in a series of blocks with invalid transactions (output spent in two different blocks) being committed to the root chain. I haven't thought through that completely, I could be missing something.

jdkanani commented 6 years ago

Creating a block with a single exit transaction in a similar fashion to deposit() could result in a series of blocks with invalid transactions (output spent in two different blocks) being committed to the root chain.

I am not following this.

Here is the approach: Create a block while finalizing exits, on root chain, not on plasma chain as you said.

function finalizeExits()
    public
    incrementOldBlocks
    returns (uint256)
  {
     // ....    
     bytes32[] txs = new bytes32[]();
     uint256 txLength = 0;

    while (...) {
         // ...

         // add tx
        txs[txLength] = _encodeSpentUTXO(currentExit.utxoPos, currentExit.amount);
        txLength++;

        // ....
    }

    if (txLength > 0) {
      childChain[currentChildBlock] = ChildBlock({
        root: Merkle.root(txs),
        exited: true,
        createdAt: block.timestamp
      });
      currentChildBlock = currentChildBlock.add(1);
    }
}

Encode spent transaction output

function _encodeSpentTXO(uint256[3] utxoPos, uint256 value)
    internal
    returns (bytes32)
  {
    var txBytesList = new bytes[](11);
    for (uint8 i = 0; i < 3; i++) {
      txBytesList[i] = utxoPos[i].toBytes();
    }

    // set address bytes
    var n = address(0).toBytes();
    txBytes[6] = n;
    txBytes[7] = value.toBytes();
    txBytes[8] = n;

    // tx hash
    return keccak256(RLP.encode(txBytes), new bytes(130));
  }

In case of already exited TXO, challengeExit just checks if tx included in the block with exited block. No need to check sig or confirmSig in that case.

jdkanani commented 6 years ago

@kfichter any thoughts?

DavidKnott commented 6 years ago

@jdkanani I believe I've solved this, will push up later today.

jdkanani commented 6 years ago

@DavidKnott Great.