From c76c3ed90ac97d674042026d39bd5d1abb4b682a Mon Sep 17 00:00:00 2001 From: Sasha Bogicevic Date: Thu, 26 Sep 2024 17:20:26 +0200 Subject: [PATCH] Resolve fixme in deposit observation Make sure to check if the deposit datum actually contains the deposit tx inputs. Remove the `utxo` from `DepositObservation` and propagate this change. Increment observation is broken now since it was depending on this utxo but we need to do it differently any way. --- hydra-node/json-schemas/logs.yaml | 6 ----- hydra-node/src/Hydra/Chain.hs | 1 - hydra-node/src/Hydra/Chain/Direct/Handlers.hs | 4 ++-- hydra-node/src/Hydra/Chain/Direct/State.hs | 4 ++-- hydra-node/src/Hydra/HeadLogic.hs | 23 ++++++++----------- hydra-node/src/Hydra/HeadLogic/Outcome.hs | 2 +- hydra-node/src/Hydra/HeadLogic/State.hs | 2 -- .../test/Hydra/Chain/Direct/StateSpec.hs | 2 +- .../test/Hydra/HeadLogicSnapshotSpec.hs | 2 -- hydra-node/test/Hydra/HeadLogicSpec.hs | 6 +---- hydra-tx/src/Hydra/Tx/Deposit.hs | 16 ++++++------- 11 files changed, 23 insertions(+), 45 deletions(-) diff --git a/hydra-node/json-schemas/logs.yaml b/hydra-node/json-schemas/logs.yaml index b62d5db3d7c..f4aa54d23fe 100644 --- a/hydra-node/json-schemas/logs.yaml +++ b/hydra-node/json-schemas/logs.yaml @@ -1585,7 +1585,6 @@ definitions: - allTxs - confirmedSnapshot - seenSnapshot - - depositScriptUTxO - commitUTxO - decommitTx - version @@ -1606,11 +1605,6 @@ definitions: $ref: "api.yaml#/components/schemas/ConfirmedSnapshot" seenSnapshot: $ref: "logs.yaml#/definitions/SeenSnapshot" - depositScriptUTxO: - oneOf: - - type: "null" - - type: object - $ref: "api.yaml#/components/schemas/UTxO" commitUTxO: oneOf: - type: "null" diff --git a/hydra-node/src/Hydra/Chain.hs b/hydra-node/src/Hydra/Chain.hs index ba81d7b2b88..09f6b00828c 100644 --- a/hydra-node/src/Hydra/Chain.hs +++ b/hydra-node/src/Hydra/Chain.hs @@ -121,7 +121,6 @@ data OnChainTx tx | OnCollectComTx {headId :: HeadId} | OnDepositTx { headId :: HeadId - , utxo :: UTxOType tx , deposited :: UTxOType tx } | OnRecoverTx diff --git a/hydra-node/src/Hydra/Chain/Direct/Handlers.hs b/hydra-node/src/Hydra/Chain/Direct/Handlers.hs index 0fe54b530c6..2f8c76a1a89 100644 --- a/hydra-node/src/Hydra/Chain/Direct/Handlers.hs +++ b/hydra-node/src/Hydra/Chain/Direct/Handlers.hs @@ -369,8 +369,8 @@ convertObservation = \case pure OnCommitTx{headId, party, committed} CollectCom CollectComObservation{headId} -> pure OnCollectComTx{headId} - Deposit DepositObservation{headId, deposited, utxo} -> - pure OnDepositTx{headId, deposited, utxo} + Deposit DepositObservation{headId, deposited} -> + pure OnDepositTx{headId, deposited} Recover RecoverObservation{headId, recoveredUTxO} -> pure OnRecoverTx{headId, recoveredUTxO} Increment IncrementObservation{headId, newVersion, committedUTxO, depositScriptUTxO} -> diff --git a/hydra-node/src/Hydra/Chain/Direct/State.hs b/hydra-node/src/Hydra/Chain/Direct/State.hs index 9b8a247923d..de1215805d2 100644 --- a/hydra-node/src/Hydra/Chain/Direct/State.hs +++ b/hydra-node/src/Hydra/Chain/Direct/State.hs @@ -1145,9 +1145,9 @@ genDepositTx = do genRecoverTx :: Gen (UTxO, Tx) genRecoverTx = do - (_depositedUTxO, txDeposit) <- genDepositTx + (depositedUTxO, txDeposit) <- genDepositTx let DepositObservation{deposited} = - fromJust $ observeDepositTx testNetworkId txDeposit + fromJust $ observeDepositTx testNetworkId depositedUTxO txDeposit -- TODO: generate multiple various slots after deadline let tx = recoverTx (mkTxIn txDeposit 0) deposited 100 pure (utxoFromTx txDeposit, tx) diff --git a/hydra-node/src/Hydra/HeadLogic.hs b/hydra-node/src/Hydra/HeadLogic.hs index f0818cc6efb..e99a127ae37 100644 --- a/hydra-node/src/Hydra/HeadLogic.hs +++ b/hydra-node/src/Hydra/HeadLogic.hs @@ -677,8 +677,8 @@ onOpenNetworkAckSn Environment{party} openState otherParty snapshotSignature sn else outcome maybePostIncrementTx snapshot@Snapshot{utxoToCommit} signatures outcome = - case (depositScriptUTxO, commitUTxO, utxoToCommit) of - (Just depositUTxOFromState, Just commitUTxOFromState, Just commitUTxOFromSnapshot) -> + case (commitUTxO, utxoToCommit) of + (Just commitUTxOFromState, Just commitUTxOFromSnapshot) -> if commitUTxOFromState == commitUTxOFromSnapshot then outcome @@ -694,7 +694,7 @@ onOpenNetworkAckSn Environment{party} openState otherParty snapshotSignature sn { headId , headParameters = parameters , incrementingSnapshot = ConfirmedSnapshot{snapshot, signatures} - , depositScriptUTxO = depositUTxOFromState + , depositScriptUTxO = undefined } } ] @@ -730,7 +730,7 @@ onOpenNetworkAckSn Environment{party} openState otherParty snapshotSignature sn , headId } = openState - CoordinatedHeadState{seenSnapshot, localTxs, decommitTx, depositScriptUTxO, commitUTxO, version} = coordinatedHeadState + CoordinatedHeadState{seenSnapshot, localTxs, decommitTx, commitUTxO, version} = coordinatedHeadState -- | Client request to recover deposited UTxO. -- @@ -925,15 +925,14 @@ onOpenChainDepositTx :: Environment -> OpenState tx -> UTxOType tx -> - UTxOType tx -> Outcome tx -onOpenChainDepositTx headId env st deposited utxo = +onOpenChainDepositTx headId env st deposited = waitOnUnresolvedDecommit $ waitOnUnresolvedCommit $ if not snapshotInFlight && isLeader parameters party nextSn then -- TODO: here we include the deposit UTxO to localUTxO before actually signing a snapshot. revisit this. - newState CommitRecorded{depositScriptUTxO = utxo, commitUTxO = deposited, newLocalUTxO = localUTxO <> deposited} + newState CommitRecorded{commitUTxO = deposited, newLocalUTxO = localUTxO <> deposited} <> cause (ClientEffect $ ServerOutput.CommitRecorded{headId, utxoToCommit = deposited}) <> cause (NetworkEffect $ ReqSn version nextSn (txId <$> localTxs) Nothing (Just deposited)) else noop @@ -1295,8 +1294,8 @@ update env ledger st ev = case (st, ev) of onOpenClientDecommit headId ledger currentSlot coordinatedHeadState decommitTx (Open openState, NetworkInput ttl (ReceivedMessage{msg = ReqDec{transaction}})) -> onOpenNetworkReqDec env ledger ttl openState transaction - (Open openState@OpenState{headId = ourHeadId}, ChainInput Observation{observedTx = OnDepositTx{headId, deposited, utxo}}) - | ourHeadId == headId -> onOpenChainDepositTx headId env openState deposited utxo + (Open openState@OpenState{headId = ourHeadId}, ChainInput Observation{observedTx = OnDepositTx{headId, deposited}}) + | ourHeadId == headId -> onOpenChainDepositTx headId env openState deposited | otherwise -> Error NotOurHead{ourHeadId, otherHeadId = headId} (Open openState@OpenState{headId = ourHeadId}, ChainInput Observation{observedTx = OnRecoverTx{headId, recoveredUTxO}}) @@ -1403,7 +1402,7 @@ aggregate st = \case where CoordinatedHeadState{localTxs} = coordinatedHeadState _otherState -> st - CommitRecorded{depositScriptUTxO, commitUTxO, newLocalUTxO} -> case st of + CommitRecorded{commitUTxO, newLocalUTxO} -> case st of Open os@OpenState{coordinatedHeadState} -> Open @@ -1412,7 +1411,6 @@ aggregate st = \case coordinatedHeadState { localUTxO = newLocalUTxO , commitUTxO = Just commitUTxO - , depositScriptUTxO = Just depositScriptUTxO } } _otherState -> st @@ -1425,7 +1423,6 @@ aggregate st = \case coordinatedHeadState { localUTxO = newLocalUTxO , commitUTxO = Nothing - , depositScriptUTxO = Nothing } } _otherState -> st @@ -1541,7 +1538,6 @@ aggregate st = \case , confirmedSnapshot = InitialSnapshot{headId, initialUTxO} , seenSnapshot = NoSeenSnapshot , commitUTxO = Nothing - , depositScriptUTxO = Nothing , decommitTx = Nothing , version = 0 } @@ -1598,7 +1594,6 @@ aggregate st = \case { coordinatedHeadState = coordinatedHeadState { commitUTxO = Nothing - , depositScriptUTxO = Nothing , version = newVersion } } diff --git a/hydra-node/src/Hydra/HeadLogic/Outcome.hs b/hydra-node/src/Hydra/HeadLogic/Outcome.hs index f0210605dc0..e346acfda63 100644 --- a/hydra-node/src/Hydra/HeadLogic/Outcome.hs +++ b/hydra-node/src/Hydra/HeadLogic/Outcome.hs @@ -72,7 +72,7 @@ data StateChanged tx { tx :: tx , newLocalUTxO :: UTxOType tx } - | CommitRecorded {depositScriptUTxO :: UTxOType tx, commitUTxO :: UTxOType tx, newLocalUTxO :: UTxOType tx} + | CommitRecorded {commitUTxO :: UTxOType tx, newLocalUTxO :: UTxOType tx} | CommitRecovered {recoveredUTxO :: UTxOType tx, newLocalUTxO :: UTxOType tx} | DecommitRecorded {decommitTx :: tx, newLocalUTxO :: UTxOType tx} | SnapshotRequestDecided {snapshotNumber :: SnapshotNumber} diff --git a/hydra-node/src/Hydra/HeadLogic/State.hs b/hydra-node/src/Hydra/HeadLogic/State.hs index d64ccac6cd0..5a26ab1c63d 100644 --- a/hydra-node/src/Hydra/HeadLogic/State.hs +++ b/hydra-node/src/Hydra/HeadLogic/State.hs @@ -159,8 +159,6 @@ data CoordinatedHeadState tx = CoordinatedHeadState -- ^ The latest confirmed snapshot. Spec: S̅ , seenSnapshot :: SeenSnapshot tx -- ^ Last seen snapshot and signatures accumulator. Spec: Û, ŝ and Σ̂ - , depositScriptUTxO :: Maybe (UTxOType tx) - -- ^ Deposit script UTxO to be consumed by the increment tx , commitUTxO :: Maybe (UTxOType tx) -- ^ Pending deposit UTxO. Spec: Uα , decommitTx :: Maybe tx diff --git a/hydra-node/test/Hydra/Chain/Direct/StateSpec.hs b/hydra-node/test/Hydra/Chain/Direct/StateSpec.hs index 52c907a6489..2818614f4b5 100644 --- a/hydra-node/test/Hydra/Chain/Direct/StateSpec.hs +++ b/hydra-node/test/Hydra/Chain/Direct/StateSpec.hs @@ -336,7 +336,7 @@ spec = parallel $ do prop "observes deposit" $ forAllDeposit $ \utxo tx -> - case observeDepositTx testNetworkId tx of + case observeDepositTx testNetworkId utxo tx of Just DepositObservation{} -> property True Nothing -> False & counterexample ("observeDepositTx ignored transaction: " <> renderTxWithUTxO utxo tx) diff --git a/hydra-node/test/Hydra/HeadLogicSnapshotSpec.hs b/hydra-node/test/Hydra/HeadLogicSnapshotSpec.hs index bba7c128cbf..0e81aec41ac 100644 --- a/hydra-node/test/Hydra/HeadLogicSnapshotSpec.hs +++ b/hydra-node/test/Hydra/HeadLogicSnapshotSpec.hs @@ -64,7 +64,6 @@ spec = do , localTxs = mempty , confirmedSnapshot = InitialSnapshot testHeadId u0 , seenSnapshot = NoSeenSnapshot - , depositScriptUTxO = Nothing , commitUTxO = Nothing , decommitTx = Nothing , version = 0 @@ -207,7 +206,6 @@ prop_singleMemberHeadAlwaysSnapshotOnReqTx sn = monadicST $ do , localTxs = [] , confirmedSnapshot = sn , seenSnapshot - , depositScriptUTxO = Nothing , commitUTxO = Nothing , decommitTx = Nothing , version diff --git a/hydra-node/test/Hydra/HeadLogicSpec.hs b/hydra-node/test/Hydra/HeadLogicSpec.hs index 29eb104bcf7..96d0a974b63 100644 --- a/hydra-node/test/Hydra/HeadLogicSpec.hs +++ b/hydra-node/test/Hydra/HeadLogicSpec.hs @@ -97,7 +97,6 @@ spec = , confirmedSnapshot = InitialSnapshot testHeadId mempty , seenSnapshot = NoSeenSnapshot , commitUTxO = Nothing - , depositScriptUTxO = Nothing , decommitTx = Nothing , version = 0 } @@ -652,7 +651,7 @@ spec = `shouldBe` Error (NotOurHead{ourHeadId = testHeadId, otherHeadId}) prop "ignores depositTx of another head" $ \otherHeadId -> do - let depositOtherHead = observeTx $ OnDepositTx{headId = otherHeadId, utxo = mempty, deposited = mempty} + let depositOtherHead = observeTx $ OnDepositTx{headId = otherHeadId, deposited = mempty} update bobEnv ledger (inOpenState threeParties) depositOtherHead `shouldBe` Error (NotOurHead{ourHeadId = testHeadId, otherHeadId}) @@ -705,7 +704,6 @@ spec = , seenSnapshot = NoSeenSnapshot , commitUTxO = Nothing , decommitTx = Nothing - , depositScriptUTxO = Nothing , version = 0 } , chainState = Prelude.error "should not be used" @@ -741,7 +739,6 @@ spec = , localTxs = [] , confirmedSnapshot = InitialSnapshot testHeadId mempty , seenSnapshot = NoSeenSnapshot - , depositScriptUTxO = Nothing , commitUTxO = Nothing , decommitTx = Nothing , version = 0 @@ -894,7 +891,6 @@ inOpenState parties = , localTxs = mempty , confirmedSnapshot , seenSnapshot = NoSeenSnapshot - , depositScriptUTxO = Nothing , commitUTxO = Nothing , decommitTx = Nothing , version = 0 diff --git a/hydra-tx/src/Hydra/Tx/Deposit.hs b/hydra-tx/src/Hydra/Tx/Deposit.hs index 515c7c10922..8081f30ef06 100644 --- a/hydra-tx/src/Hydra/Tx/Deposit.hs +++ b/hydra-tx/src/Hydra/Tx/Deposit.hs @@ -61,21 +61,21 @@ depositTx networkId headId depositUTxO deadline = data DepositObservation = DepositObservation { headId :: HeadId , deposited :: UTxO - , -- TODO: really needed? - utxo :: UTxO } deriving stock (Show, Eq, Generic) observeDepositTx :: NetworkId -> - -- TODO: needs to also know the UTxO to resolve tx inputs + UTxO -> Tx -> Maybe DepositObservation -observeDepositTx networkId tx = do +observeDepositTx networkId utxo tx = do -- TODO: could just use the first output and fail otherwise - (depositIn, depositOut) <- findTxOutByAddress depositAddress tx - -- FIXME: need to verify consistency of datum against tx inputs! - observeDepositTxOut (networkIdToNetwork networkId) (toUTxOContext depositOut) + (_depositIn, depositOut) <- findTxOutByAddress depositAddress tx + depositObservation@DepositObservation{deposited} <- observeDepositTxOut (networkIdToNetwork networkId) (toUTxOContext depositOut) + if resolveInputsUTxO utxo tx == deposited + then Just depositObservation + else Nothing where depositScript = fromPlutusScript Deposit.validatorScript @@ -95,6 +95,4 @@ observeDepositTxOut network depositOut = do DepositObservation { headId , deposited = deposit - , -- FIXME: drop this - utxo = undefined }