PR review changes

Signed-off-by: Sasha Bogicevic <[email protected]>
cardano-scaling · Aug 7, 2024 · eae92d1 · eae92d1
1 parent 90e433c
commit eae92d1
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 41 deletions.
diff --git a/hydra-node/json-schemas/logs.yaml b/hydra-node/json-schemas/logs.yaml
@@ -1055,6 +1055,16 @@ definitions:
             $ref: "api.yaml#/components/schemas/SnapshotNumber"
           leader:
             $ref: "api.yaml#/components/schemas/Party"
+      - title: "ReqSnDecommitNotSettled"
+        description: >-
+          Received a ReqSn message with specified new decommit but the previous one was not settled.
+        additionalProperties: false
+        required:
+          - tag
+        properties:
+          tag:
+            type: string
+            enum: ["ReqSnDecommitNotSettled"]
       - title: "InvalidMultisignature"
         description: >-
           Multisignature computed for a snapshot from individual parties signature is invalid.
@@ -2215,8 +2225,6 @@ definitions:
             enum: ["WaitOnSnapshotNumber"]
           waitingForNumber:
             "$ref": "api.yaml#/components/schemas/SnapshotNumber"
-            description: >-
-              The expected number.
       - title: WaitOnSnapshotVersion
         description: >-
           Requested snapshot version is not up to date, waiting for the next version number.
@@ -2231,8 +2239,6 @@ definitions:
             enum: ["WaitOnSnapshotVersion"]
           waitingForVersion:
             "$ref": "api.yaml#/components/schemas/SnapshotVersion"
-            description: >-
-              The expected number.
       - title: WaitOnSeenSnapshot
         description: >-
           No current snapshot is available, waiting for some snapshot to start.

diff --git a/hydra-node/src/Hydra/HeadLogic.hs b/hydra-node/src/Hydra/HeadLogic.hs
@@ -405,15 +405,12 @@ onOpenNetworkReqSn ::
   Maybe tx ->
   Outcome tx
 onOpenNetworkReqSn env ledger st otherParty sv sn requestedTxIds mDecommitTx =
-  -- Spec: require v = v ∧ s = ŝ + 1 ∧ leader(s) = j
+  -- Spec: require s = ŝ + 1 ∧ leader(s) = j
   requireReqSn $
     -- Spec: wait ŝ = ̅S.s
     waitNoSnapshotInFlight $
       -- Spec: wait v = v̂
       waitOnSnapshotVersion $
-        -- Spec: require ̅S.𝑈 ◦ txω ≠ ⊥
-        --       ηω ← combine(outputs(txω))
-        --       𝑈_active ← ̅S.𝑈 ◦ txω \ outputs(txω)
         requireApplicableDecommitTx $ \(activeUTxO, mUtxoToDecommit) ->
           -- Resolve transactions by-id
           waitResolvableTxs $ \requestedTxs -> do
@@ -482,37 +479,28 @@ onOpenNetworkReqSn env ledger st otherParty sv sn requestedTxIds mDecommitTx =
   requireApplicableDecommitTx cont =
     case mDecommitTx of
       Nothing -> cont (confirmedUTxO, Nothing)
-      Just decommitTx
-        -- Spec: if v = S̄.v
-        | sv == confVersion ->
-            case confUTxOToDecommit of
-              Nothing ->
-                -- Spec: require ̅S.𝑈 ◦ txω /= ⊥
-                case applyTransactions ledger currentSlot confirmedUTxO [decommitTx] of
-                  Left (_, err) ->
-                    Error $ RequireFailed $ SnapshotDoesNotApply sn (txId decommitTx) err
-                  Right newConfirmedUTxO -> do
-                    -- Spec: 𝑈_active ← ̅S.𝑈 ◦ txω \ outputs(txω)
-                    let utxoToDecommit = utxoFromTx decommitTx
-                    let activeUTxO = newConfirmedUTxO `withoutUTxO` utxoToDecommit
-                    cont (activeUTxO, Just utxoToDecommit)
-              Just pendingUtxOToDecommit
-                -- Spec: S̄.txω ̸= ⊥
-                -- Spec: require S̄.txω = txω
-                | pendingUtxOToDecommit /= utxoFromTx decommitTx ->
-                    Error $ RequireFailed ReqSnDecommitNotSettled
-                | otherwise ->
-                    cont (confirmedUTxO, Just $ utxoFromTx decommitTx)
-        | otherwise ->
-            -- Spec: require ̅S.𝑈 ◦ txω /= ⊥
-            case applyTransactions ledger currentSlot confirmedUTxO [decommitTx] of
-              Left (_, err) ->
-                Error $ RequireFailed $ SnapshotDoesNotApply sn (txId decommitTx) err
-              Right newConfirmedUTxO -> do
-                -- Spec: 𝑈_active ← ̅S.𝑈 ◦ txω \ outputs(txω)
-                let utxoToDecommit = utxoFromTx decommitTx
-                let activeUTxO = newConfirmedUTxO `withoutUTxO` utxoToDecommit
-                cont (activeUTxO, Just utxoToDecommit)
+      Just decommitTx ->
+        -- Spec:
+        -- if v = S̄.v ∧ S̄.txω ̸= ⊥
+        --   require S̄.txω = txω
+        --   Uactive ← S̄.U
+        --   Uω ← S̄.Uω
+        -- else
+        --   require S̄.U ◦ txω ̸= ⊥
+        --   Uactive ← S̄.U ◦ txω \ outputs(txω )
+        --   Uω ← outputs(txω )
+        if sv == confVersion && isJust confUTxOToDecommit
+          then
+            if confUTxOToDecommit == Just (utxoFromTx decommitTx)
+              then cont (confirmedUTxO, confUTxOToDecommit)
+              else Error $ RequireFailed ReqSnDecommitNotSettled
+          else case applyTransactions ledger currentSlot confirmedUTxO [decommitTx] of
+            Left (_, err) ->
+              Error $ RequireFailed $ SnapshotDoesNotApply sn (txId decommitTx) err
+            Right newConfirmedUTxO -> do
+              let utxoToDecommit = utxoFromTx decommitTx
+              let activeUTxO = newConfirmedUTxO `withoutUTxO` utxoToDecommit
+              cont (activeUTxO, Just utxoToDecommit)
 
   -- NOTE: at this point we know those transactions apply on the localUTxO because they
   -- are part of the localTxs. The snapshot can contain less transactions than the ones

diff --git a/hydra-node/test/Hydra/BehaviorSpec.hs b/hydra-node/test/Hydra/BehaviorSpec.hs
@@ -439,7 +439,6 @@ spec = parallel $ do
 
       it "can process transactions while decommit pending" $
         shouldRunInSim $ do
-          -- NOTE: The simulated network has a block time of 20 (simulated) seconds.
           withSimulatedChainAndNetwork $ \chain ->
             withHydraNode aliceSk [bob] chain $ \n1 ->
               withHydraNode bobSk [alice] chain $ \n2 -> do
@@ -690,6 +689,7 @@ dummySimulatedChainNetwork =
 -- | With-pattern wrapper around 'simulatedChainAndNetwork' which does 'cancel'
 -- the 'tickThread'. Also, this will fix tx to 'SimpleTx' so that it can pick an
 -- initial chain state to play back to our test nodes.
+-- NOTE: The simulated network has a block time of 20 (simulated) seconds.
 withSimulatedChainAndNetwork ::
   (MonadTime m, MonadDelay m, MonadAsync m) =>
   (SimulatedChainNetwork SimpleTx m -> m ()) ->

diff --git a/hydra-node/test/Hydra/HeadLogicSpec.hs b/hydra-node/test/Hydra/HeadLogicSpec.hs
@@ -499,7 +499,7 @@ spec =
         outcome <- runHeadLogic bobEnv ledger s0 $ do
           step reqSn
         outcome `shouldSatisfy` \case
-          Error RequireFailed{} -> True
+          Error RequireFailed{requirementFailure} | requirementFailure == ReqSnDecommitNotSettled -> True
           _ -> False
 
       it "ignores in-flight ReqTx when closed" $ do