Context & versions
While investigating this head on preview was not distributing the 8.9k ADA deposited into it found the following.
0.20.0-327-gb06f4f827c should be affected
Steps to reproduce
- Deposit some
utxo1 and create a "small" depositTxId1
- Open a head,
version = 0
- Deposit some
utxo2 with a bigger depositTxId2 (depositTxId1 < depositTxId2)
- Increment of
utxo2 happens, version = 1 now
Actual behavior
- All nodes believe
utxo2 is still pending and snapshots with utxo2 in utxoToCommit and version = 1 get produced.
- Close and fanout will only distribute the utxo WITHOUT
utxo2 .. although it was actually spent into the head from the deposit
Expected behavior
- In step 5,
depositTxId2 should not be pending anymore and utxo2 should become part of the normal L2 utxo
- Close and fanout after step 5 distributes the
utxo2 (one way or another)
Hypothesis
At step 5, we saw CommitFinalized of depositTxId1 even though the increment transaction was clearly spending from depositTxId2.
The observeIncrementTx is not correctly reporting which deposit it spent and all nodes do believe another pending (or incremented into a previous head) deposit was "finalized". The L2 head state is adopting the new version, but keeps the utxoToCommit as it was before -> this is problematic, as it creates value out of thin air.
This is situation is equivalent to all nodes actively colluding to sign snapshots that don't agree with what happens on the L1.
Solution idea
Ensure that observeIncrementTx is not affected by other deposits in the "spendable UTxO".
- Write a test that exercises this scenario (most like
StateSpec is easiest)
- This is likely the fix
diff --git a/hydra-tx/src/Hydra/Tx/Increment.hs b/hydra-tx/src/Hydra/Tx/Increment.hs
index 88f29813be..aa09cf5786 100644
--- a/hydra-tx/src/Hydra/Tx/Increment.hs
+++ b/hydra-tx/src/Hydra/Tx/Increment.hs
@@ -121,7 +121,7 @@ observeIncrementTx ::
observeIncrementTx utxo tx = do
let inputUTxO = resolveInputsUTxO utxo tx
(headInput, headOutput) <- findTxOutByScript inputUTxO Head.validatorScript
- (TxIn depositTxId _, depositOutput) <- findTxOutByScript utxo depositValidatorScript
+ (TxIn depositTxId _, depositOutput) <- findTxOutByScript inputUTxO depositValidatorScript
dat <- txOutScriptData $ toTxContext depositOutput
-- we need to be able to decode the datum, no need to use it tho
_ :: Deposit.DepositDatum <- fromScriptData dat
- Improve the signature of
observeXXX functions to not fall into this trap: We should tie the UTxO provided more to the Tx, e.g. by creating a combined type ResolvedTx, and when creating that value ensure that the UTxO only contains the inputs of the transaction = bound the context available of the observation code.
Context & versions
While investigating this head on preview was not distributing the 8.9k ADA deposited into it found the following.
0.20.0-327-gb06f4f827c should be affected
Steps to reproduce
utxo1and create a "small"depositTxId1version = 0utxo2with a biggerdepositTxId2(depositTxId1 < depositTxId2)utxo2happens,version = 1nowActual behavior
utxo2is still pending and snapshots withutxo2inutxoToCommitandversion = 1get produced.utxo2.. although it was actually spent into the head from the depositExpected behavior
depositTxId2should not be pending anymore andutxo2should become part of the normal L2utxoutxo2(one way or another)Hypothesis
At step 5, we saw
CommitFinalizedofdepositTxId1even though the increment transaction was clearly spending fromdepositTxId2.The
observeIncrementTxis not correctly reporting which deposit it spent and all nodes do believe another pending (or incremented into a previous head) deposit was "finalized". The L2 head state is adopting the new version, but keeps theutxoToCommitas it was before -> this is problematic, as it creates value out of thin air.This is situation is equivalent to all nodes actively colluding to sign snapshots that don't agree with what happens on the L1.
Solution idea
Ensure that
observeIncrementTxis not affected by other deposits in the "spendable UTxO".StateSpecis easiest)observeXXXfunctions to not fall into this trap: We should tie theUTxOprovided more to theTx, e.g. by creating a combined typeResolvedTx, and when creating that value ensure that theUTxOonly contains the inputs of the transaction = bound the context available of the observation code.