Clean up code and comments

eras/conway/impl/src/Cardano/Ledger/Conway/Governance/DRepPulser.hs

@@ -160,18 +160,31 @@ instance EraPParams era => ToCBOR (PulsingSnapshot era) where
 instance EraPParams era => FromCBOR (PulsingSnapshot era) where
   fromCBOR = fromEraCBOR @era
 
--- | We iterate over a pulse-sized chunk of the UMap. For each StakingCredential
--- in the chunk that has delegated to a DRep, add the StakeDistr and rewards
--- for that Credential to the DRep Distribution, if the DRep is a DRepCredential
--- (also, AlwaysAbstain or AlwaysNoConfidence) and a member of the registered
--- DReps. If the DRepCredential is not a member of the registered DReps, ignore
--- and skip that DRep.
+-- | We iterate over a pulse-sized chunk of the UMap.
 --
--- Give or take, this operation has roughly O(a * (log(b) + log(c))) complexity,
--- where,
---   (a) is the size of the chunk of the UMap, which is expected to be the pulse-size,
---   (b) is the size of the StakeDistr, and
---   (c) is the size of the DRepDistr, this grows as the accumulator
+-- For each staking credential in the chunk that has delegated to a DRep, add
+-- the stake distribution, rewards, and proposal deposits for that credential to
+-- the DRep distribution, if the DRep is a DRepCredential (also, AlwaysAbstain
+-- or AlwaysNoConfidence) and a member of the registered DReps. If the
+-- DRepCredential is not a member of the registered DReps, ignore and skip that
+-- DRep.
+--
+-- For each staking credential in the chunk that has delegated to an SPO,
+-- add only the proposal deposits for that credential to the stake pool
+-- distribution, since the rewards and stake are already added to it by the
+-- SNAP rule.
+--
+-- Give or take, this operation has roughly
+-- @
+--   O (a * (log(b) + log(c) + log(d) + log(e) + log(f)))
+-- @
+-- complexity, where,
+--   (a) is the size of the chunk of the UMap, which is the pulse-size, iterate over
+--   (b) is the size of the StakeDistr, lookup
+--   (c) is the size of the DRepDistr, insertWith
+--   (d) is the size of the dpProposalDeposits, lookup
+--   (e) is the size of the registered DReps, lookup
+--   (f) is the size of the PoolDistr, insert
 computeDRepDistr ::
   forall c.
   Map (Credential 'Staking c) (CompactForm Coin) ->
@@ -203,11 +216,13 @@ computeDRepDistr stakeDistr regDReps proposalDeposits poolDistr dRepDistr uMapCh
               , addToPoolDistr spo proposalDeposit poolAccum
               )
     addToPoolDistr spo ccoin distr =
-      -- Only add the proposal deposits to the CompactCoin numerator and
-      -- denominator and not the rational In order not to interfere with rewards
-      -- calculation of any kind.
+      -- Avoid adding the proposal deposits to anything other than the numerator
+      -- and denominator in order not to interfere with rewards calculation,
+      -- although this is an isolated copy inside the DRepPulser, we want to
+      -- just be sure we do not confuse ourselves over which representations are
+      -- used where.
       case Map.lookup spo $ distr ^. poolDistrDistrL of
-        Nothing -> distr -- Impossible! Because, a delegation to an SPO would also appear in the PoolDistr
+        Nothing -> distr -- Impossible! Because a delegation to an SPO would also appear in the PoolDistr.
         Just ips ->
           distr
             & poolDistrDistrL %~ Map.insert spo (ips & individualPoolStakeCoinL <>~ ccoin)

eras/conway/impl/src/Cardano/Ledger/Conway/Rules/Ratify.hs

@@ -198,20 +198,16 @@ spoAccepted re rs gas =
 -- vote are considered as `Abstain` votes.
 spoAcceptedRatio :: forall era. RatifyEnv era -> GovActionState era -> Rational
 spoAcceptedRatio
-  RatifyEnv {reStakePoolDistr}
+  RatifyEnv {reStakePoolDistr = PoolDistr individualPoolStake (CompactCoin totalActiveStake)}
   GovActionState
     { gasStakePoolVotes
-    , gasProposalProcedure =
-      ProposalProcedure {pProcGovAction}
+    , gasProposalProcedure = ProposalProcedure {pProcGovAction}
     }
     | abstainVotesRatio == 1 = 0 -- guard against the degenerate case when all abstain.
     | otherwise = yesVotesRatio / (1 - abstainVotesRatio)
     where
-      PoolDistr individualPoolStake (CompactCoin totalActiveStake) = reStakePoolDistr
-      (yesVotesRatio, abstainVotesRatio) =
-        ( toInteger yesVotes % toInteger totalActiveStake
-        , toInteger abstainVotes % toInteger totalActiveStake
-        )
+      yesVotesRatio = toInteger yesVotes % toInteger totalActiveStake
+      abstainVotesRatio = toInteger abstainVotes % toInteger totalActiveStake
       (CompactCoin yesVotes, CompactCoin abstainVotes) =
         Map.foldlWithKey' getVotesStakePerStakePoolDistr (mempty, mempty) individualPoolStake
       getVotesStakePerStakePoolDistr (!yess, !abstains) poolId distr =
@@ -221,9 +217,9 @@ spoAcceptedRatio
               Nothing
                 | HardForkInitiation {} <- pProcGovAction -> (yess, abstains)
                 | otherwise -> (yess, abstains <> d)
+              Just Abstain -> (yess, abstains <> d)
               Just VoteNo -> (yess, abstains)
               Just VoteYes -> (yess <> d, abstains)
-              Just Abstain -> (yess, abstains <> d)
 
 dRepAccepted ::
   ConwayEraPParams era => RatifyEnv era -> RatifyState era -> GovActionState era -> Bool

eras/conway/impl/testlib/Test/Cardano/Ledger/Conway/Imp/RatifySpec.hs

@@ -887,13 +887,13 @@ votingSpec =
                   , pProcGovAction = addCCAction
                   , pProcAnchor = def
                   }
-            -- Submit the vote from SPO #1
+            -- Submit a yes vote from SPO #1 and a no vote from SPO #2
             submitVote_ VoteYes (StakePoolVoter poolKH1) addCCGaid
             submitVote_ VoteNo (StakePoolVoter poolKH2) addCCGaid
             passNEpochs 2
             -- The vote should not result in a ratification
             getLastEnactedCommittee `shouldReturn` SNothing
-            -- Submit another proposal to bump up the active voting stake
+            -- Submit another proposal to bump up the active voting stake of SPO #1
             anotherCC <- KeyHashObj <$> freshKeyHash
             let anotherAddCCAction = UpdateCommittee SNothing mempty (Map.singleton anotherCC 10) (75 %! 100)
             submitProposal_ $
@@ -931,7 +931,7 @@ votingSpec =
             spoRewardAccount1 <- getRewardAccountFor stakingC1
             spoRewardAccount3 <- getRewardAccountFor stakingC3
             -- Submit committee proposals
-            -- The proposal deposits comes from the root UTxO
+            -- The proposal deposits come from the root UTxO
             -- After this both stakingC1 and stakingC3 are expected to have 1_000_000 ADA of stake, each
             cc <- KeyHashObj <$> freshKeyHash
             let addCCAction = UpdateCommittee SNothing mempty (Map.singleton cc 10) (75 %! 100)
@@ -960,12 +960,7 @@ votingSpec =
             getLastEnactedCommittee `shouldReturn` SNothing
             submitTxAnn_ "Switch the delegation from SPO #3 to SPO #1" $
               mkBasicTx mkBasicTxBody
-                & bodyTxL . certsTxBodyL
-                  .~ SSeq.fromList
-                    [ mkDelegTxCert
-                        stakingC3
-                        (DelegStake poolKH1)
-                    ]
+                & bodyTxL . certsTxBodyL .~ SSeq.fromList [mkDelegTxCert stakingC3 (DelegStake poolKH1)]
             passNEpochs 2
             -- The same vote should now successfully ratify the proposal
             getLastEnactedCommittee `shouldReturn` SJust (GovPurposeId addCCGaid)

libs/cardano-ledger-core/src/Cardano/Ledger/PoolDistr.hs

@@ -54,7 +54,7 @@ import NoThunks.Class (NoThunks (..))
 -- delegated to a registered stake pool.
 data IndividualPoolStake c = IndividualPoolStake
   { individualPoolStake :: !Rational
-  , individualPoolStakeCoin :: !(CompactForm Coin)
+  , individualPoolStakeCoin :: !(CompactForm Coin) -- TODO: Document
   , individualPoolStakeVrf :: !(Hash c (VerKeyVRF c))
   }
   deriving stock (Show, Eq, Generic)