Skip to content

Commit

Permalink
Resolve fixme in deposit observation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
v0d1ch committed Sep 26, 2024
1 parent 7b4b709 commit c76c3ed
Show file tree
Hide file tree
Showing 11 changed files with 23 additions and 45 deletions.
6 changes: 0 additions & 6 deletions hydra-node/json-schemas/logs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1585,7 +1585,6 @@ definitions:
- allTxs
- confirmedSnapshot
- seenSnapshot
- depositScriptUTxO
- commitUTxO
- decommitTx
- version
Expand All @@ -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"
Expand Down
1 change: 0 additions & 1 deletion hydra-node/src/Hydra/Chain.hs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ data OnChainTx tx
| OnCollectComTx {headId :: HeadId}
| OnDepositTx
{ headId :: HeadId
, utxo :: UTxOType tx
, deposited :: UTxOType tx
}
| OnRecoverTx
Expand Down
4 changes: 2 additions & 2 deletions hydra-node/src/Hydra/Chain/Direct/Handlers.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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} ->
Expand Down
4 changes: 2 additions & 2 deletions hydra-node/src/Hydra/Chain/Direct/State.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
23 changes: 9 additions & 14 deletions hydra-node/src/Hydra/HeadLogic.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -694,7 +694,7 @@ onOpenNetworkAckSn Environment{party} openState otherParty snapshotSignature sn
{ headId
, headParameters = parameters
, incrementingSnapshot = ConfirmedSnapshot{snapshot, signatures}
, depositScriptUTxO = depositUTxOFromState
, depositScriptUTxO = undefined
}
}
]
Expand Down Expand Up @@ -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.
--
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}})
Expand Down Expand Up @@ -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
Expand All @@ -1412,7 +1411,6 @@ aggregate st = \case
coordinatedHeadState
{ localUTxO = newLocalUTxO
, commitUTxO = Just commitUTxO
, depositScriptUTxO = Just depositScriptUTxO
}
}
_otherState -> st
Expand All @@ -1425,7 +1423,6 @@ aggregate st = \case
coordinatedHeadState
{ localUTxO = newLocalUTxO
, commitUTxO = Nothing
, depositScriptUTxO = Nothing
}
}
_otherState -> st
Expand Down Expand Up @@ -1541,7 +1538,6 @@ aggregate st = \case
, confirmedSnapshot = InitialSnapshot{headId, initialUTxO}
, seenSnapshot = NoSeenSnapshot
, commitUTxO = Nothing
, depositScriptUTxO = Nothing
, decommitTx = Nothing
, version = 0
}
Expand Down Expand Up @@ -1598,7 +1594,6 @@ aggregate st = \case
{ coordinatedHeadState =
coordinatedHeadState
{ commitUTxO = Nothing
, depositScriptUTxO = Nothing
, version = newVersion
}
}
Expand Down
2 changes: 1 addition & 1 deletion hydra-node/src/Hydra/HeadLogic/Outcome.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
2 changes: 0 additions & 2 deletions hydra-node/src/Hydra/HeadLogic/State.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion hydra-node/test/Hydra/Chain/Direct/StateSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions hydra-node/test/Hydra/HeadLogicSnapshotSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ spec = do
, localTxs = mempty
, confirmedSnapshot = InitialSnapshot testHeadId u0
, seenSnapshot = NoSeenSnapshot
, depositScriptUTxO = Nothing
, commitUTxO = Nothing
, decommitTx = Nothing
, version = 0
Expand Down Expand Up @@ -207,7 +206,6 @@ prop_singleMemberHeadAlwaysSnapshotOnReqTx sn = monadicST $ do
, localTxs = []
, confirmedSnapshot = sn
, seenSnapshot
, depositScriptUTxO = Nothing
, commitUTxO = Nothing
, decommitTx = Nothing
, version
Expand Down
6 changes: 1 addition & 5 deletions hydra-node/test/Hydra/HeadLogicSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ spec =
, confirmedSnapshot = InitialSnapshot testHeadId mempty
, seenSnapshot = NoSeenSnapshot
, commitUTxO = Nothing
, depositScriptUTxO = Nothing
, decommitTx = Nothing
, version = 0
}
Expand Down Expand Up @@ -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})

Expand Down Expand Up @@ -705,7 +704,6 @@ spec =
, seenSnapshot = NoSeenSnapshot
, commitUTxO = Nothing
, decommitTx = Nothing
, depositScriptUTxO = Nothing
, version = 0
}
, chainState = Prelude.error "should not be used"
Expand Down Expand Up @@ -741,7 +739,6 @@ spec =
, localTxs = []
, confirmedSnapshot = InitialSnapshot testHeadId mempty
, seenSnapshot = NoSeenSnapshot
, depositScriptUTxO = Nothing
, commitUTxO = Nothing
, decommitTx = Nothing
, version = 0
Expand Down Expand Up @@ -894,7 +891,6 @@ inOpenState parties =
, localTxs = mempty
, confirmedSnapshot
, seenSnapshot = NoSeenSnapshot
, depositScriptUTxO = Nothing
, commitUTxO = Nothing
, decommitTx = Nothing
, version = 0
Expand Down
16 changes: 7 additions & 9 deletions hydra-tx/src/Hydra/Tx/Deposit.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -95,6 +95,4 @@ observeDepositTxOut network depositOut = do
DepositObservation
{ headId
, deposited = deposit
, -- FIXME: drop this
utxo = undefined
}

0 comments on commit c76c3ed

Please sign in to comment.