From b558f5c3f100cf8b795f98d0227530ca8a0159e9 Mon Sep 17 00:00:00 2001 From: Aniket Deshpande Date: Thu, 2 Nov 2023 00:59:02 +0530 Subject: [PATCH] Use a Data.OMap.Strict to replace ProposalsSnapshot (#3791) * Add Data.OMap.Strict to cardano-data * Make ProposalsSnapshot an OMap --- eras/conway/impl/CHANGELOG.md | 1 + eras/conway/impl/cardano-ledger-conway.cabal | 2 +- .../Ledger/Conway/Governance/Procedures.hs | 11 + .../Ledger/Conway/Governance/Snapshots.hs | 86 ++-- libs/cardano-data/CHANGELOG.md | 4 + libs/cardano-data/cardano-data.cabal | 18 +- libs/cardano-data/src/Data/OMap/Strict.hs | 426 ++++++++++++++++++ libs/cardano-data/test/Main.hs | 8 +- .../test/Test/Cardano/Data/OMap/StrictSpec.hs | 167 +++++++ .../testlib/Test/Cardano/Data/Arbitrary.hs | 16 +- libs/cardano-ledger-binary/CHANGELOG.md | 1 + .../Cardano/Ledger/Binary/Decoding/Decoder.hs | 29 +- 12 files changed, 699 insertions(+), 70 deletions(-) create mode 100644 libs/cardano-data/src/Data/OMap/Strict.hs create mode 100644 libs/cardano-data/test/Test/Cardano/Data/OMap/StrictSpec.hs diff --git a/eras/conway/impl/CHANGELOG.md b/eras/conway/impl/CHANGELOG.md index f28ce86d3db..9e6b8ccbb61 100644 --- a/eras/conway/impl/CHANGELOG.md +++ b/eras/conway/impl/CHANGELOG.md @@ -2,6 +2,7 @@ ## 1.10.1.0 +* Switch to using `OMap` for `ProposalsSnapshot` #3791 * Add `VotingOnExpiredGovAction` predicate failure in `GOV` #3825 ### `testlib` diff --git a/eras/conway/impl/cardano-ledger-conway.cabal b/eras/conway/impl/cardano-ledger-conway.cabal index 10cb0aaa18f..6d2189d3754 100644 --- a/eras/conway/impl/cardano-ledger-conway.cabal +++ b/eras/conway/impl/cardano-ledger-conway.cabal @@ -75,7 +75,7 @@ library aeson >=2.2, data-default-class, cardano-crypto-class, - cardano-data >=1.1.1.0, + cardano-data >=1.1.2.0, cardano-ledger-binary >=1.2, cardano-ledger-allegra >=1.1, cardano-ledger-alonzo ^>=1.5, diff --git a/eras/conway/impl/src/Cardano/Ledger/Conway/Governance/Procedures.hs b/eras/conway/impl/src/Cardano/Ledger/Conway/Governance/Procedures.hs index d23eb0c34f5..f9b1f9e30ee 100644 --- a/eras/conway/impl/src/Cardano/Ledger/Conway/Governance/Procedures.hs +++ b/eras/conway/impl/src/Cardano/Ledger/Conway/Governance/Procedures.hs @@ -3,9 +3,11 @@ {-# LANGUAGE DeriveGeneric #-} {-# LANGUAGE DerivingStrategies #-} {-# LANGUAGE FlexibleContexts #-} +{-# LANGUAGE GADTs #-} {-# LANGUAGE GeneralizedNewtypeDeriving #-} {-# LANGUAGE KindSignatures #-} {-# LANGUAGE LambdaCase #-} +{-# LANGUAGE MultiParamTypeClasses #-} {-# LANGUAGE NamedFieldPuns #-} {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE RecordWildCards #-} @@ -14,6 +16,7 @@ {-# LANGUAGE StandaloneDeriving #-} {-# LANGUAGE TupleSections #-} {-# LANGUAGE TypeApplications #-} +{-# LANGUAGE TypeOperators #-} {-# LANGUAGE UndecidableInstances #-} module Cardano.Ledger.Conway.Governance.Procedures ( @@ -105,6 +108,7 @@ import Data.Aeson.Types (toJSONKeyText) import Data.Default.Class import Data.Map.Strict (Map) import Data.Maybe.Strict (StrictMaybe (..)) +import qualified Data.OMap.Strict as OMap import qualified Data.OSet.Strict as OSet import qualified Data.Sequence as Seq import Data.Set (Set) @@ -264,6 +268,13 @@ instance EraPParams era => EncCBOR (GovActionState era) where !> To gasProposedIn !> To gasExpiresAfter +-- Ref: https://gitlab.haskell.org/ghc/ghc/-/issues/14046 +instance + c ~ EraCrypto era => + OMap.HasOKey (GovActionId c) (GovActionState era) + where + okeyL = lens gasId $ \gas gi -> gas {gasId = gi} + data Voter c = CommitteeVoter !(Credential 'HotCommitteeRole c) | DRepVoter !(Credential 'DRepRole c) diff --git a/eras/conway/impl/src/Cardano/Ledger/Conway/Governance/Snapshots.hs b/eras/conway/impl/src/Cardano/Ledger/Conway/Governance/Snapshots.hs index 0309fa8c4fb..3d4e047fe2b 100644 --- a/eras/conway/impl/src/Cardano/Ledger/Conway/Governance/Snapshots.hs +++ b/eras/conway/impl/src/Cardano/Ledger/Conway/Governance/Snapshots.hs @@ -1,7 +1,10 @@ {-# LANGUAGE DeriveGeneric #-} -{-# LANGUAGE NamedFieldPuns #-} +{-# LANGUAGE DerivingStrategies #-} +{-# LANGUAGE GADTs #-} +{-# LANGUAGE GeneralizedNewtypeDeriving #-} +{-# LANGUAGE MultiParamTypeClasses #-} {-# LANGUAGE RankNTypes #-} -{-# LANGUAGE RecordWildCards #-} +{-# LANGUAGE UndecidableInstances #-} module Cardano.Ledger.Conway.Governance.Snapshots ( ProposalsSnapshot, @@ -32,25 +35,20 @@ import Cardano.Ledger.TreeDiff (ToExpr) import Control.DeepSeq (NFData) import Data.Aeson (ToJSON) import Data.Default.Class (Default (..)) -import Data.Foldable (Foldable (..)) -import Data.List (sort) import qualified Data.Map as Map import Data.Map.Strict (Map) -import Data.MapExtras (extractKeys) -import Data.Maybe (fromMaybe) +import qualified Data.OMap.Strict as OMap import Data.Sequence.Strict (StrictSeq (..)) import Data.Set (Set) -import qualified Data.Set as Set import GHC.Generics (Generic) import Lens.Micro (Lens', (%~)) import NoThunks.Class (NoThunks) -data ProposalsSnapshot era = ProposalsSnapshot - { psGovActionStates :: !(Map (GovActionId (EraCrypto era)) (GovActionState era)) - , psProposalOrder :: !(StrictSeq (GovActionId (EraCrypto era))) - -- ^ Newer actions are near the end - } - deriving (Generic, Eq, Show) +newtype ProposalsSnapshot era + = ProposalsSnapshot + (OMap.OMap (GovActionId (EraCrypto era)) (GovActionState era)) + deriving newtype (Show, Eq) + deriving stock (Generic) instance EraPParams era => ToExpr (ProposalsSnapshot era) @@ -61,7 +59,7 @@ instance EraPParams era => NFData (ProposalsSnapshot era) instance EraPParams era => NoThunks (ProposalsSnapshot era) instance Default (ProposalsSnapshot era) where - def = ProposalsSnapshot def def + def = ProposalsSnapshot def instance EraPParams era => EncCBOR (ProposalsSnapshot era) where encCBOR = encCBOR . snapshotActions @@ -73,49 +71,43 @@ instance EraPParams era => DecCBOR (ProposalsSnapshot era) where instance EraPParams era => DecShareCBOR (ProposalsSnapshot era) where decShareCBOR _ = fromGovActionStateSeq <$> decCBOR +-- | Insert a `GovActionState`, overwriting an entry of it if the +-- corresponding `GovActionId` already exists. snapshotInsertGovAction :: GovActionState era -> ProposalsSnapshot era -> ProposalsSnapshot era -snapshotInsertGovAction gas@GovActionState {gasId} ps@ProposalsSnapshot {..} - | Map.member gasId psGovActionStates = - ps {psGovActionStates = Map.insert gasId gas psGovActionStates} - | otherwise = - ProposalsSnapshot - { psGovActionStates = Map.insert gasId gas psGovActionStates - , psProposalOrder = psProposalOrder :|> gasId - } +snapshotInsertGovAction gas (ProposalsSnapshot omap) = + ProposalsSnapshot (omap OMap.||> gas) +-- | Get the sequence of `GovActionState`s snapshotActions :: ProposalsSnapshot era -> StrictSeq (GovActionState era) -snapshotActions ProposalsSnapshot {..} = toGovAction <$> psProposalOrder - where - toGovAction gaId = - fromMaybe - (error $ "Impossible: ProposalsSnapshot invariant is not maintained: " <> show gaId) - (Map.lookup gaId psGovActionStates) +snapshotActions (ProposalsSnapshot omap) = OMap.toStrictSeq omap +-- | Get the sequence of `GovActionId`s snapshotIds :: ProposalsSnapshot era -> StrictSeq (GovActionId (EraCrypto era)) -snapshotIds = psProposalOrder +snapshotIds (ProposalsSnapshot omap) = OMap.toStrictSeqOKeys omap +-- | Get the unordered map of `GovActionId`s and `GovActionState`s snapshotGovActionStates :: ProposalsSnapshot era -> Map (GovActionId (EraCrypto era)) (GovActionState era) -snapshotGovActionStates = psGovActionStates +snapshotGovActionStates (ProposalsSnapshot omap) = OMap.toMap omap +-- | Add a vote to an existing `GovActionState` This is a no-op if the . +-- provided `GovActionId` does not already exist . snapshotAddVote :: Voter (EraCrypto era) -> Vote -> GovActionId (EraCrypto era) -> ProposalsSnapshot era -> ProposalsSnapshot era -snapshotAddVote voter vote gId ps@ProposalsSnapshot {..} = - ps - { psGovActionStates = Map.update (Just . updateVote) gId psGovActionStates - } +snapshotAddVote voter vote gai (ProposalsSnapshot omap) = + ProposalsSnapshot $ OMap.adjust updateVote gai omap where insertVote :: Ord k => @@ -129,39 +121,29 @@ snapshotAddVote voter vote gId ps@ProposalsSnapshot {..} = StakePoolVoter kh -> insertVote gasStakePoolVotesL kh CommitteeVoter c -> insertVote gasCommitteeVotesL c +-- | Extract `GovActionState`s for the given set of `GovActionId`s from the `Proposals` snapshotRemoveIds :: Set (GovActionId (EraCrypto era)) -> ProposalsSnapshot era -> (ProposalsSnapshot era, Map.Map (GovActionId (EraCrypto era)) (GovActionState era)) -snapshotRemoveIds gIds (ProposalsSnapshot {..}) = (retainedProposals, removedGovActionStates) - where - (retainedGovActionStates, removedGovActionStates) = psGovActionStates `extractKeys` gIds - retainedProposals = - ProposalsSnapshot - { psGovActionStates = retainedGovActionStates - , psProposalOrder = - foldl' (\s x -> if x `Set.member` gIds then s else x :<| s) mempty psProposalOrder - } +snapshotRemoveIds gais (ProposalsSnapshot omap) = + let (retained, removed) = OMap.extractKeys gais omap + in (ProposalsSnapshot retained, removed) snapshotLookupId :: GovActionId (EraCrypto era) -> ProposalsSnapshot era -> Maybe (GovActionState era) -snapshotLookupId gId ProposalsSnapshot {psGovActionStates} = - Map.lookup gId psGovActionStates +snapshotLookupId gai (ProposalsSnapshot omap) = OMap.lookup gai omap -- | Converts a sequence of `GovActionState`s to a `ProposalsSnapshot`. -- -- /Warning/ - This function expects `GovActionState`'s to have unique -- `GovActionId`s, because duplicate Ids will result in `GovActionStates` -- to be dropped. -fromGovActionStateSeq :: - StrictSeq (GovActionState era) -> - ProposalsSnapshot era -fromGovActionStateSeq = foldl' (flip snapshotInsertGovAction) def +fromGovActionStateSeq :: StrictSeq (GovActionState era) -> ProposalsSnapshot era +fromGovActionStateSeq = ProposalsSnapshot . OMap.fromFoldable -- | Internal function for checking if the invariants are maintained isConsistent_ :: ProposalsSnapshot era -> Bool -isConsistent_ (ProposalsSnapshot {psGovActionStates, psProposalOrder}) = - Map.keys psGovActionStates == sort (toList psProposalOrder) - && all (\(k, GovActionState {gasId}) -> k == gasId) (Map.toList psGovActionStates) +isConsistent_ (ProposalsSnapshot omap) = OMap.invariantHolds' omap diff --git a/libs/cardano-data/CHANGELOG.md b/libs/cardano-data/CHANGELOG.md index acf57976ddb..eba6df967db 100644 --- a/libs/cardano-data/CHANGELOG.md +++ b/libs/cardano-data/CHANGELOG.md @@ -1,5 +1,9 @@ # Version history for `cardano-data` +## 1.1.2.0 + +- Add Data.OMap.Strict #3791 + ## 1.1.1.0 - Add Data.OSet.Strict #3779 diff --git a/libs/cardano-data/cardano-data.cabal b/libs/cardano-data/cardano-data.cabal index 822237e9062..47ae2878f43 100644 --- a/libs/cardano-data/cardano-data.cabal +++ b/libs/cardano-data/cardano-data.cabal @@ -1,6 +1,6 @@ cabal-version: 3.0 name: cardano-data -version: 1.1.1.0 +version: 1.1.2.0 license: Apache-2.0 maintainer: operations@iohk.io author: IOHK @@ -23,6 +23,7 @@ library Data.ListMap Data.Universe Data.OSet.Strict + Data.OMap.Strict hs-source-dirs: src default-language: Haskell2010 @@ -36,10 +37,12 @@ library cardano-ledger-binary >=1.2, cardano-strict-containers >=0.1.2.1, containers, + data-default-class, deepseq, mtl, nothunks, - vector + vector, + microlens library testlib exposed-modules: @@ -55,11 +58,14 @@ library testlib build-depends: base, + cardano-data, + cardano-ledger-binary:testlib, containers, hspec, QuickCheck, cardano-ledger-binary:testlib, - cardano-data + cardano-data, + microlens test-suite cardano-data-tests type: exitcode-stdio-1.0 @@ -68,6 +74,7 @@ test-suite cardano-data-tests other-modules: Test.Cardano.Data.MapExtrasSpec Test.Cardano.Data.OSet.StrictSpec + Test.Cardano.Data.OMap.StrictSpec default-language: Haskell2010 ghc-options: @@ -77,11 +84,12 @@ test-suite cardano-data-tests build-depends: base, + cardano-strict-containers, containers, hspec, cardano-data, cardano-ledger-binary:testlib, - cardano-strict-containers, testlib, QuickCheck, - quickcheck-classes-base + quickcheck-classes-base, + microlens diff --git a/libs/cardano-data/src/Data/OMap/Strict.hs b/libs/cardano-data/src/Data/OMap/Strict.hs new file mode 100644 index 00000000000..2d87cddfc5c --- /dev/null +++ b/libs/cardano-data/src/Data/OMap/Strict.hs @@ -0,0 +1,426 @@ +{-# LANGUAGE BangPatterns #-} +{-# LANGUAGE DeriveAnyClass #-} +{-# LANGUAGE DeriveGeneric #-} +{-# LANGUAGE DerivingStrategies #-} +{-# LANGUAGE FlexibleContexts #-} +{-# LANGUAGE FunctionalDependencies #-} +{-# LANGUAGE ImportQualifiedPost #-} +{-# LANGUAGE PatternSynonyms #-} +{-# LANGUAGE RankNTypes #-} +{-# LANGUAGE ScopedTypeVariables #-} +{-# LANGUAGE StandaloneDeriving #-} +{-# LANGUAGE TypeFamilies #-} +{-# LANGUAGE ViewPatterns #-} + +module Data.OMap.Strict ( + HasOKey (okeyL), + OMap (Empty, (:<|:), (:|>:)), + null, + size, + empty, + singleton, + lookup, + member, + (!?), + fromSet, + fromFoldable, + fromFoldableDuplicates, + toMap, + toStrictSeq, + toStrictSeqOKeys, + toStrictSeqOfPairs, + invariantHolds, + invariantHolds', + (|>), + (<|), + (<||), + (||>), + (|><), + (><|), + elem, + extractKeys, + adjust, + filter, +) +where + +import Cardano.Ledger.Binary ( + DecCBOR, + EncCBOR (encCBOR), + decodeListLikeEnforceNoDuplicates, + encodeStrictSeq, + ) +import Cardano.Ledger.Binary.Decoding (DecCBOR (decCBOR)) +import Cardano.Ledger.TreeDiff (ToExpr (..)) +import Control.DeepSeq (NFData (..)) +import Data.Aeson (ToJSON (..)) +import Data.Default.Class (Default (..)) +import Data.Foldable qualified as F +import Data.Map.Strict qualified as Map +import Data.MapExtras qualified as MapE +import Data.Maybe (isJust) +import Data.Sequence.Strict qualified as SSeq +import Data.Set qualified as Set +import Data.Typeable (Typeable) +import GHC.Exts (IsList (..)) +import GHC.Generics (Generic) +import Lens.Micro +import NoThunks.Class (NoThunks (..)) +import Prelude hiding (elem, filter, lookup, null, seq) + +-- | Class of types that can be mapped by a lens or a projection to an +-- Ord type. +-- +-- For a type @V@, defines a lens from @V@ to and Ord type @K@. +class Ord k => HasOKey k v | v -> k where + okeyL :: Lens' v k + +-- | A general-purpose finite, insert-ordered, map that is strict in its +-- keys and values. +-- +-- The strictness is enforced by the underlying strict `Map` that can +-- be looked-up by a projection or lens. and the ordering is maintained +-- by the constructing functions, leveraging `StrictSeq` to hold the +-- insert-order of the keys. +-- +-- TODO: DecShareCBOR instance +data OMap k v = OMap + { omSSeq :: !(SSeq.StrictSeq k) + , omMap :: !(Map.Map k v) + } + deriving (Generic, Eq) + +instance (Show v, Ord k, Show k) => Show (OMap k v) where + show = show . toStrictSeqOfPairs + +deriving instance (NoThunks k, NoThunks v) => NoThunks (OMap k v) + +deriving instance (NFData k, NFData v) => NFData (OMap k v) + +-- | \(O(1)\). +empty :: OMap k v +empty = OMap SSeq.Empty Map.empty + +instance Default (OMap k v) where + def = empty + +-- | \(O(1)\). Shallow invariant using just `length` and `size`. +invariantHolds :: OMap k v -> Bool +invariantHolds (OMap sseq kv) = SSeq.length sseq == Map.size kv + +-- | \(O(n \log n)\). Deep, costly invariant using membership check for each +-- value. By the pigeon-hole principle, this check is exhaustive. +invariantHolds' :: Ord k => OMap k v -> Bool +invariantHolds' omap@(OMap sseq kv) = + invariantHolds omap && all (\k -> isJust $ Map.lookup k kv) sseq + +-- | \(O(1)\). +null :: OMap k v -> Bool +null (OMap sseq _) = SSeq.null sseq + +-- | \(O(1)\). +size :: OMap k v -> Int +size (OMap sseq _) = SSeq.length sseq + +-- | \(O(1)\). Strict in its arguments. +singleton :: HasOKey k v => v -> OMap k v +singleton !v = + let k = v ^. okeyL + in OMap (SSeq.singleton k) (Map.singleton k v) + +-- | \(O(\log n)\). If the key is not present 'lookup' returns +-- 'Nothing'. +lookup :: Ord k => k -> OMap k v -> Maybe v +lookup k (OMap _seq kv) = Map.lookup k kv + +-- | `flip`ed version of `lookup` +(!?) :: Ord k => OMap k v -> k -> Maybe v +(!?) = flip lookup + +-- | \(O(\log n)\). Checks membership before cons'ing. +cons :: HasOKey k v => v -> OMap k v -> OMap k v +cons v omap@(OMap sseq kv) + | Map.member k kv = omap + | otherwise = OMap (k SSeq.<| sseq) (Map.insert k v kv) + where + k = v ^. okeyL + +-- | \(O(\log n)\). Checks membership before cons'ing. +(<|) :: HasOKey k v => v -> OMap k v -> OMap k v +(<|) = cons + +infixr 5 <| + +-- | \(O(\log n)\). Checks membership before cons'ing. Overwrites a +-- duplicate. +cons' :: HasOKey k v => v -> OMap k v -> OMap k v +cons' v (OMap sseq kv) + | Map.member k kv = OMap sseq kv' + | otherwise = OMap (k SSeq.<| sseq) kv' + where + k = v ^. okeyL + kv' = Map.insert k v kv + +-- | \(O(\log n)\). Checks membership before cons'ing. Overwrites a +-- duplicate. +(<||) :: HasOKey k v => v -> OMap k v -> OMap k v +(<||) = cons' + +infixr 5 <|| + +-- | \(O(\log n)\). Checks membership before snoc'ing. +snoc :: HasOKey k v => OMap k v -> v -> OMap k v +snoc omap@(OMap sseq kv) v + | Map.member k kv = omap + | otherwise = OMap (sseq SSeq.|> k) (Map.insert k v kv) + where + k = v ^. okeyL + +-- | \(O(\log n)\). Checks membership before snoc'ing. +(|>) :: HasOKey k v => OMap k v -> v -> OMap k v +(|>) = snoc + +infixl 5 |> + +-- | \(O(\log n)\). Checks membership before snoc'ing. Overwrites a +-- duplicate. +snoc' :: HasOKey k v => OMap k v -> v -> OMap k v +snoc' (OMap sseq kv) v + | Map.member k kv = OMap sseq kv' + | otherwise = OMap (sseq SSeq.|> k) kv' + where + k = v ^. okeyL + kv' = Map.insert k v kv + +-- | \(O(\log n)\). Checks membership before snoc'ing. Overwrites a +-- duplicate. +(||>) :: HasOKey k v => OMap k v -> v -> OMap k v +(||>) = snoc' + +infixl 5 ||> + +-- | \(O(\log n)\). +uncons :: Ord k => OMap k v -> Maybe (v, OMap k v) +uncons (OMap sseq kv) = case sseq of + SSeq.Empty -> Nothing + k SSeq.:<| ks -> + case Map.lookup k kv of + Just v -> Just (v, OMap ks (Map.delete k kv)) + Nothing -> error "Invariant falsified! In OMap, key from sequence not found in corresponding map" + +-- | \(O(\log n)\). +unsnoc :: Ord k => OMap k v -> Maybe (OMap k v, v) +unsnoc (OMap sseq kv) = case sseq of + SSeq.Empty -> Nothing + ks SSeq.:|> k -> + case Map.lookup k kv of + Just v -> Just (OMap ks (Map.delete k kv), v) + Nothing -> error "Invariant falsified! In OMap, key from sequence not found in corresponding map" + +-- | \(O(n \log n)\). Checks membership before snoc'ing. +-- De-duplicates the StrictSeq without overwriting. +-- Starts from the left or head, using `foldl'` +fromFoldable :: (Foldable f, HasOKey k v) => f v -> OMap k v +fromFoldable = F.foldl' snoc empty + +-- | \(O(n \log n)\). Checks membership before snoc'ing. +-- De-duplicates the StrictSeq and collects and returns the duplicates found. +-- Starts from the left or head, using `foldl'` +fromFoldableDuplicates :: (Foldable f, HasOKey k v, Ord v) => f v -> (Set.Set v, OMap k v) +fromFoldableDuplicates = F.foldl' snoc_ (Set.empty, empty) + where + snoc_ :: (HasOKey k v, Ord v) => (Set.Set v, OMap k v) -> v -> (Set.Set v, OMap k v) + snoc_ (duplicates, omap@(OMap sseq kv)) v = + let k = v ^. okeyL + in if Map.member k kv + then (Set.insert v duplicates, omap) + else (duplicates, OMap (sseq SSeq.|> k) (Map.insert k v kv)) + +-- | \(O(n \log n)\). +fromSet :: HasOKey k v => Set.Set v -> OMap k v +fromSet = fromFoldable + +-- | \(O(1)\). +toMap :: OMap k v -> Map.Map k v +toMap = omMap + +-- | \(O(n \log n)\). +toStrictSeq :: Ord k => OMap k v -> SSeq.StrictSeq v +toStrictSeq (OMap sseq kv) = sseq <&> \k -> let !v = kv Map.! k in v + +-- | \(O(1)\). +toStrictSeqOKeys :: OMap k v -> SSeq.StrictSeq k +toStrictSeqOKeys = omSSeq + +-- | \(O(n \log n)\). +toStrictSeqOfPairs :: Ord k => OMap k v -> SSeq.StrictSeq (k, v) +toStrictSeqOfPairs (OMap sseq kv) = sseq <&> \k -> let !v = kv Map.! k in (k, v) + +-- | \(O(\log n)\). Key membership check. +member :: Ord k => k -> OMap k v -> Bool +member k (OMap _sseq kv) = Map.member k kv + +-- | \(O(\log n)\). Value membership check. +elem :: (HasOKey k v, Eq v) => v -> OMap k v -> Bool +elem v = (Just v ==) . lookup (v ^. okeyL) + +-- | \(O(n)\). Given a `Set` of @k@s, and an `OMap` @k@ @v@ return +-- a pair of `Map` and `OMap` where the @k@s in the `Set` have been +-- removed from the `OMap` and presented as a separate `Map`. +extractKeys :: Ord k => Set.Set k -> OMap k v -> (OMap k v, Map.Map k v) +extractKeys ks (OMap sseq kv) = + let (kv', extractedKv) = MapE.extractKeys kv ks + sseq' = + F.foldl' + (\accum k -> if Set.member k ks then accum else accum SSeq.|> k) + SSeq.empty + sseq + in (OMap sseq' kv', extractedKv) + +-- | \(O(n)\). Like `Map.adjust`. +-- +-- Returns the original `OMap` unaltered when the key does not exist. +-- +-- If the key exists, then the function is applied to the value, but we need to consider +-- three possible cases: +-- +-- 1. The modified value's `okeyL` is unaltered +-- - we return omap with the adjusted value, +-- 2. The modified value's `okeyL` is altered, but not a duplicate +-- - we return the omap with adjusted key (in place) and value +-- 3. The modified value's `okeyL` is altered and is a duplicate +-- - we return the omap with the old key deleted from the sequence but +-- without inserting the new key since it is a duplicate, and +-- deleting old value and inserting the new value in place of its duplicate. +-- +-- Examples: +-- +-- >>> import Data.OMap.Strict +-- >>> import Lens.Micro +-- >>> instance HasOKey Int (Int, Char) where okeyL = _1 +-- >>> let m = fromFoldable $ zip [1,2] ['a','b'] :: OMap Int (Int, Char) +-- >>> m +-- StrictSeq {fromStrict = fromList [(1,(1,'a')),(2,(2,'b'))]} +-- >>> let adjustingFn (k, v) = (k, succ v) -- Changes the value +-- >>> let overwritingAdjustingFn (k,v) = (succ k, v) -- Changes the `okeyL`. +-- >>> adjust adjustingFn 1 m +-- StrictSeq {fromStrict = fromList [(1,(1,'b')),(2,(2,'b'))]} +-- >>> adjust overwritingAdjustingFn 1 m +-- StrictSeq {fromStrict = fromList [(2,(2,'a'))]} +adjust :: HasOKey k v => (v -> v) -> k -> OMap k v -> OMap k v +adjust f k omap@(OMap sseq kv) = + case Map.lookup k kv of + Nothing -> omap + Just v -> + let v' = f v + k' = v' ^. okeyL + in if k' == k + then OMap sseq (Map.insert k v' kv) + else + let kv' = Map.insert k' v' $ Map.delete k kv + (lseq, rseq) = case SSeq.spanl (/= k) sseq of + (l, _ SSeq.:<| r) -> (l, r) + _ -> error "Impossible: supplied key expected to be in the sequence" + in case Map.lookup k' kv of + Nothing -> OMap (lseq <> (k' SSeq.:<| rseq)) kv' + Just _ -> OMap (lseq <> rseq) kv' + +-- | \(O(1)\) +pattern Empty :: OMap k v +pattern Empty <- (null -> True) + where + Empty = empty + +-- | \(O(\log n)\). +pattern (:<|:) :: (HasOKey k v, Ord k) => v -> OMap k v -> OMap k v +pattern x :<|: xs <- (uncons -> Just (x, xs)) + where + x :<|: xs = x <| xs + +infixr 5 :<|: + +-- | \(O(\log n)\). +pattern (:|>:) :: (HasOKey k v, Ord k) => OMap k v -> v -> OMap k v +pattern xs :|>: x <- (unsnoc -> Just (xs, x)) + where + xs :|>: x = xs |> x + +infixl 5 :|>: + +{-# COMPLETE Empty, (:|>:) #-} +{-# COMPLETE Empty, (:<|:) #-} + +-- | \( O(n \log m) \). For every uncons-ed element from the sequence on the right, +-- check its membership in the sequence on the left, before snoc'ing it. +-- Preserve order. Remove duplicates from sequence on the right. +(|><) :: HasOKey k v => OMap k v -> OMap k v -> OMap k v +omapl |>< omapr = case omapr of + Empty -> omapl + r :<|: rs -> (omapl |> r) |>< rs + +infixl 5 |>< + +-- | \( O(m \log n) \). For every unsnoc-ed element from the sequence on the left, +-- check its membership in the sequence on the right, before cons'ing it. +-- Preserve order. Remove duplicates from sequence on the left. +(><|) :: HasOKey k v => OMap k v -> OMap k v -> OMap k v +omapl ><| omapr = case omapl of + Empty -> omapr + ls :|>: l -> ls ><| (l <| omapr) + +infixr 5 ><| + +instance HasOKey k v => IsList (OMap k v) where + type Item (OMap k v) = v + fromList = fromFoldable + toList = F.toList + +instance (HasOKey k v, ToExpr v) => ToExpr (OMap k v) where + listToExpr = listToExpr . F.toList + toExpr = toExpr . F.toList + +instance (HasOKey k v, ToJSON v) => ToJSON (OMap k v) where + toJSON = toJSON . toStrictSeq + toEncoding = toEncoding . toStrictSeq + +instance HasOKey k v => Semigroup (OMap k v) where + (<>) = (|><) + +instance HasOKey k v => Monoid (OMap k v) where + mempty = empty + +instance Ord k => Foldable (OMap k) where + foldMap f (OMap sseq kv) = F.foldMap (\k -> f (kv Map.! k)) sseq + {-# INLINEABLE foldMap #-} + foldr f z (OMap sseq kv) = F.foldr (\k -> f (kv Map.! k)) z sseq + {-# INLINEABLE foldr #-} + foldl f z (OMap sseq kv) = F.foldl (\acc k -> f acc (kv Map.! k)) z sseq + {-# INLINEABLE foldl #-} + foldr' f z (OMap sseq kv) = F.foldr' (\k -> f (kv Map.! k)) z sseq + {-# INLINEABLE foldr' #-} + foldl' f z (OMap sseq kv) = F.foldl' (\acc k -> f acc (kv Map.! k)) z sseq + {-# INLINEABLE foldl' #-} + length = Map.size . omMap + {-# INLINE length #-} + null = Map.null . omMap + {-# INLINE null #-} + +instance (Typeable k, EncCBOR v, Ord k) => EncCBOR (OMap k v) where + encCBOR omap = encodeStrictSeq encCBOR (toStrictSeq omap) + +instance (Typeable k, HasOKey k v, DecCBOR v, Eq v) => DecCBOR (OMap k v) where + decCBOR = decodeListLikeEnforceNoDuplicates isMember insert decCBOR + where + -- we can't use `elem` here because it returns `False` when the + -- element is not fully equal, but the criterion for an element + -- to be included in the `OMap` only depends on its `okeyL` not + -- clashing. + isMember e = member (e ^. okeyL) + insert v omap = omap |> v + +-- | \( O(n \log n) \) +filter :: Ord k => (v -> Bool) -> OMap k v -> OMap k v +filter f (OMap sseq kv) = + let kv' = Map.filter f kv + sseq' = F.foldl' (\accum k -> if Map.member k kv' then accum SSeq.:|> k else accum) SSeq.empty sseq + in OMap sseq' kv' diff --git a/libs/cardano-data/test/Main.hs b/libs/cardano-data/test/Main.hs index 6269aab4c11..fba967d054f 100644 --- a/libs/cardano-data/test/Main.hs +++ b/libs/cardano-data/test/Main.hs @@ -1,3 +1,5 @@ +{-# LANGUAGE ImportQualifiedPost #-} + module Main where import System.IO ( @@ -8,7 +10,8 @@ import System.IO ( utf8, ) import Test.Cardano.Data.MapExtrasSpec (mapExtrasSpec) -import qualified Test.Cardano.Data.OSet.StrictSpec as SOSet +import Test.Cardano.Data.OMap.StrictSpec qualified as OMap +import Test.Cardano.Data.OSet.StrictSpec qualified as OSet import Test.Hspec import Test.Hspec.Runner @@ -23,7 +26,8 @@ spec :: Spec spec = describe "cardano-data" $ do describe "MapExtras" mapExtrasSpec - describe "OSet.Strict" SOSet.spec + describe "OSet.Strict" OSet.spec + describe "OMap.Strict" OMap.spec main :: IO () main = do diff --git a/libs/cardano-data/test/Test/Cardano/Data/OMap/StrictSpec.hs b/libs/cardano-data/test/Test/Cardano/Data/OMap/StrictSpec.hs new file mode 100644 index 00000000000..fca3dc01529 --- /dev/null +++ b/libs/cardano-data/test/Test/Cardano/Data/OMap/StrictSpec.hs @@ -0,0 +1,167 @@ +{-# LANGUAGE FlexibleInstances #-} +{-# LANGUAGE ImportQualifiedPost #-} +{-# LANGUAGE MultiParamTypeClasses #-} +{-# LANGUAGE NamedFieldPuns #-} +{-# LANGUAGE ScopedTypeVariables #-} +{-# LANGUAGE TypeApplications #-} +{-# OPTIONS_GHC -Wno-orphans #-} + +module Test.Cardano.Data.OMap.StrictSpec where + +import Data.OMap.Strict +import Data.Proxy (Proxy (Proxy)) +import Data.Sequence.Strict qualified as SSeq +import Data.Set qualified as Set +import Lens.Micro hiding (set) +import Test.Cardano.Data.Arbitrary () +import Test.Cardano.Ledger.Binary.RoundTrip (roundTripCborSpec) +import Test.Hspec +import Test.Hspec.QuickCheck +import Test.QuickCheck (Arbitrary) +import Test.QuickCheck.Arbitrary (Arbitrary (arbitrary)) +import Test.QuickCheck.Classes.Base +import Prelude hiding (elem, filter, lookup, null) + +spec :: Spec +spec = + describe "OMap.Strict" $ do + context "membership checks work" $ do + prop "unconsed" $ + \(m :: OMap Int Int) -> case m of + Empty -> pure () + v :<|: _kv -> v ^. okeyL `shouldSatisfy` (`member` m) + prop "unsnoced" $ + \(m :: OMap Int Int) -> case m of + Empty -> pure () + _kv :|>: v -> v ^. okeyL `shouldSatisfy` (`member` m) + context "when cons-ing" $ do + prop "adding a duplicate results in a no-op" $ + \(m :: OMap Int Int) -> do + case m of + Empty -> pure () + v :<|: _kv -> m `shouldBe` v <| m + case m of + Empty -> pure () + _kv :|>: v -> m `shouldBe` v <| m + prop "new values get added" $ + \((m, v) :: (OMap Int Int, Int)) -> + if v `elem` m + then v <| m `shouldBe` m + else v <| m `shouldBe` v :<|: m + context "when snoc-ing" $ do + prop "adding a duplicate results in a no-op" $ + \(m :: OMap Int Int) -> do + case m of + Empty -> pure () + v :<|: _kv -> m `shouldBe` m |> v + case m of + Empty -> pure () + _kv :|>: v -> m `shouldBe` m |> v + prop "new values get added" $ + \((m, v) :: (OMap Int Int, Int)) -> + if v `elem` m + then m |> v `shouldBe` m + else m |> v `shouldBe` m :|>: v + context "mappend preserves uniqueness" $ do + prop "mappending with itself should be a no-op" $ + \(i :: OMap Int Int) -> do + let il = i |>< i + ir = i ><| i + il `shouldBe` i + ir `shouldBe` i + il `shouldSatisfy` invariantHolds' + ir `shouldSatisfy` invariantHolds' + prop "mappending with duplicates: left-preserving" $ + \((i, j) :: (OMap Int Int, OMap Int Int)) -> + case j of + Empty -> i `shouldBe` i |>< j + j' :<|: _js -> do + let result = i |>< j + result `shouldBe` (i |> j') |>< j + result `shouldSatisfy` invariantHolds' + prop "mappending with duplicates: right-preserving" $ + \((i, j) :: (OMap Int Int, OMap Int Int)) -> + case i of + Empty -> i ><| j `shouldBe` j + _is :|>: i' -> do + let result = i ><| j + result `shouldBe` i ><| (i' <| j) + result `shouldSatisfy` invariantHolds' + prop "extractKeys should satisfy membership" $ + \((omap, set) :: (OMap Int Int, Set.Set Int)) -> do + let result = extractKeys set omap + result `shouldSatisfy` (all (`Set.notMember` set) . fst) + result `shouldSatisfy` (all (`Set.member` set) . snd) + result `shouldSatisfy` invariantHolds' . fst + prop "filter" $ + \((omap, i) :: (OMap Int Int, Int)) -> do + let result = filter (< i) omap + result `shouldSatisfy` all (< i) + result `shouldSatisfy` invariantHolds' + prop "adjust" $ + \((omap, i) :: (OMap Int OMapTest, Int)) -> do + let adjustingFn omt@OMapTest {omSnd} = omt {omSnd = omSnd + 1} + overwritingAdjustingFn omt@OMapTest {omFst} = omt {omFst = omFst + 1} -- Changes the `okeyL`. + adjust adjustingFn i omap `shouldSatisfy` invariantHolds' + adjust overwritingAdjustingFn i omap `shouldSatisfy` invariantHolds' + context "overwriting" $ do + prop "cons' - (<||)" $ + \((omap, i) :: (OMap Int OMapTest, OMapTest)) -> do + let consed = i <|| omap + k = i ^. okeyL + if k `member` omap + then consed `shouldBe` adjust (const i) (i ^. okeyL) omap + else consed `shouldBe` i <| omap + prop "snoc' - (||>)" $ + \((omap, i) :: (OMap Int OMapTest, OMapTest)) -> do + let snoced = omap ||> i + k = i ^. okeyL + if k `member` omap + then snoced `shouldBe` adjust (const i) (i ^. okeyL) omap + else snoced `shouldBe` omap |> i + prop "fromFoldable preserves order" $ + \(set :: Set.Set Int) -> do + let sseq = SSeq.fromList $ Set.elems set + omap = fromFoldable sseq + toStrictSeq omap `shouldBe` sseq + omap `shouldSatisfy` invariantHolds' + context "fromFoldableDuplicates preserves order" $ do + prop "with duplicates" $ + \(set :: Set.Set Int) -> do + let sseq = SSeq.fromList $ Set.elems set + omap = fromFoldable sseq + result = fromFoldableDuplicates (sseq SSeq.>< sseq) + toStrictSeq (snd result) `shouldBe` sseq + result `shouldBe` (set, omap) + snd result `shouldSatisfy` invariantHolds' + prop "without duplicates" $ + \(set :: Set.Set Int) -> do + let sseq = SSeq.fromList $ Set.elems set + omap = fromFoldable sseq + result = fromFoldableDuplicates sseq + toStrictSeq (snd result) `shouldBe` sseq + result `shouldBe` (Set.empty, omap) + snd result `shouldSatisfy` invariantHolds' + context "CBOR round-trip" $ do + roundTripCborSpec @(OMap Int Int) + context "Typeclass laws" $ do + it "Type" $ + lawsCheckOne + (Proxy :: Proxy (OMap Int Int)) + [ isListLaws + , semigroupLaws + , monoidLaws + , semigroupMonoidLaws + ] + +instance HasOKey Int Int where + okeyL = lens id const + +data OMapTest = OMapTest {omFst :: Int, omSnd :: Int} + deriving (Eq, Show, Ord) + +instance HasOKey Int OMapTest where + okeyL = lens omFst $ \om u -> om {omFst = u} + +instance Arbitrary OMapTest where + arbitrary = OMapTest <$> arbitrary <*> arbitrary diff --git a/libs/cardano-data/testlib/Test/Cardano/Data/Arbitrary.hs b/libs/cardano-data/testlib/Test/Cardano/Data/Arbitrary.hs index 1b4fb875574..c5b6872ff87 100644 --- a/libs/cardano-data/testlib/Test/Cardano/Data/Arbitrary.hs +++ b/libs/cardano-data/testlib/Test/Cardano/Data/Arbitrary.hs @@ -1,10 +1,20 @@ +{-# LANGUAGE FlexibleInstances #-} +{-# LANGUAGE ImportQualifiedPost #-} {-# OPTIONS_GHC -Wno-orphans #-} module Test.Cardano.Data.Arbitrary where -import Data.OSet.Strict +import Data.Map.Strict qualified as Map +import Data.OMap.Strict qualified as OMap +import Data.OSet.Strict qualified as OSet +import Data.Set qualified as Set +import Lens.Micro (set) import Test.Cardano.Ledger.Binary.Arbitrary () import Test.QuickCheck -instance (Arbitrary a, Ord a) => Arbitrary (OSet a) where - arbitrary = fromSet <$> arbitrary +instance (Arbitrary a, Ord a) => Arbitrary (OSet.OSet a) where + arbitrary = fmap (OSet.fromSet . Set.fromList) . shuffle . Set.toList =<< arbitrary + +instance (Ord v, Arbitrary v, OMap.HasOKey k v, Arbitrary k) => Arbitrary (OMap.OMap k v) where + arbitrary = + fmap OMap.fromFoldable . shuffle . Map.elems . Map.mapWithKey (flip (set OMap.okeyL)) =<< arbitrary diff --git a/libs/cardano-ledger-binary/CHANGELOG.md b/libs/cardano-ledger-binary/CHANGELOG.md index 912923150c9..7b7806048f3 100644 --- a/libs/cardano-ledger-binary/CHANGELOG.md +++ b/libs/cardano-ledger-binary/CHANGELOG.md @@ -2,6 +2,7 @@ ## 1.2.1.0 +* Export `decodeListLikeEnforceNoDuplicates` #3791 * Add `Show` and `Eq` for `CBORGroup` ### `testlib` diff --git a/libs/cardano-ledger-binary/src/Cardano/Ledger/Binary/Decoding/Decoder.hs b/libs/cardano-ledger-binary/src/Cardano/Ledger/Binary/Decoding/Decoder.hs index f6a8b9ff938..fa89a4e5ae9 100644 --- a/libs/cardano-ledger-binary/src/Cardano/Ledger/Binary/Decoding/Decoder.hs +++ b/libs/cardano-ledger-binary/src/Cardano/Ledger/Binary/Decoding/Decoder.hs @@ -151,6 +151,7 @@ module Cardano.Ledger.Binary.Decoding.Decoder ( peekByteOffset, peekTokenType, decodeSetLikeEnforceNoDuplicates, + decodeListLikeEnforceNoDuplicates, ) where @@ -837,19 +838,18 @@ decodeSetEnforceNoDuplicates :: decodeSetEnforceNoDuplicates = decodeSetLikeEnforceNoDuplicates Set.member Set.insert {-# INLINE decodeSetEnforceNoDuplicates #-} --- | Decode a Set as a either a definite or indefinite list. Duplicates are not --- allowed. Set tag 258 is permitted, but not enforced. -decodeSetLikeEnforceNoDuplicates :: +-- | Decode a collection of values either as a definite or indefinite list. Duplicates are not +-- allowed. +decodeListLikeEnforceNoDuplicates :: forall s a f. Monoid (f a) => -- | Check for membership. Decoder will fail if this predicate returns True (a -> f a -> Bool) -> - -- | Add an aelement into the decoded Set like data structure + -- | Add an element into the decoded List like data structure (a -> f a -> f a) -> Decoder s a -> Decoder s (f a) -decodeSetLikeEnforceNoDuplicates isMember insert decodeElement = do - allowTag setTag +decodeListLikeEnforceNoDuplicates isMember insert decodeElement = do decodeListLenOrIndef >>= \case Just len -> loop (\x -> pure (x - 1, x <= 0)) len mempty Nothing -> loop (\x -> (,) x <$> decodeBreakOr) () mempty @@ -861,8 +861,23 @@ decodeSetLikeEnforceNoDuplicates isMember insert decodeElement = do then pure acc else do a <- decodeElement - when (a `isMember` acc) $ fail "Duplicate key detected in the Set" + when (a `isMember` acc) $ fail "Duplicate key detected in the List" loop condition nextStep (insert a acc) +{-# INLINE decodeListLikeEnforceNoDuplicates #-} + +-- | Decode a Set as a either a definite or indefinite list. Duplicates are not +-- allowed. Set tag 258 is permitted, but not enforced. +decodeSetLikeEnforceNoDuplicates :: + forall s a f. + Monoid (f a) => + -- | Check for membership. Decoder will fail if this predicate returns True + (a -> f a -> Bool) -> + -- | Add an aelement into the decoded Set like data structure + (a -> f a -> f a) -> + Decoder s a -> + Decoder s (f a) +decodeSetLikeEnforceNoDuplicates isMember insert decodeElement = + allowTag setTag >> decodeListLikeEnforceNoDuplicates isMember insert decodeElement {-# INLINE decodeSetLikeEnforceNoDuplicates #-} decodeContainerSkelWithReplicate ::