Support gov/canonical/pparams/v0 namespace

[qnikst]

Description

Checklist

• Commits in meaningful sequence and with useful messages.
• Tests added or updated when needed.
• CHANGELOG.md files updated for packages with externally visible changes.
  NOTE: New section is never added with the code changes. (See RELEASING.md).
• Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
  versioning process.
• Version bounds in .cabal files updated when necessary.
  NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
• Code formatted (use scripts/fourmolize.sh).
• Cabal files formatted (use scripts/cabal-format.sh).
• CDDL files are up to date (use scripts/gen-cddl.sh)
• hie.yaml updated (use scripts/gen-hie.sh).
• Self-reviewed the diff.

[lehins]

Sorry, but this is not a viable approach for protocol parameters. We can talk more about it later on today in the meeting(s).

[qnikst]

Fixed all of the comments!

[lehins]

Few things needs some adjustments, but overall looks good.

[lehins]

We do actually need to save the actual protocol version in the canonical ledger state

Suggested change

	& ppProtocolVersionL .~ ProtVer (eraProtVerLow @ConwayEra) 0
	& ppProtocolVersionL .~ protVer

[lehins]

Suggested change

	tau <- decodeField @"gov/pparams/v0" 11
	tau <- decodeField @"gov/pparams/v0" 11
	protVer <- decodeField @"gov/pparams/v0" 14

[lehins]

Is there no accessor for Versioned?

Suggested change

	Versioned value <- fromCanonicalCBOR @v
	return value
	unVersioned <$> fromCanonicalCBOR @v

[lehins]

Could I please ask you to add to Data.MapExtras in cardano-data this functionality:

boundedEnumMap :: (Ord k, Bounded a, Enum a) => (a -> k) -> Map k a
boundedEnumMap toTxt = Map.fromList [(toTxt t, t) | t <- [minBound .. maxBound]]

lookupMapFail
  :: (Ord k, Show k, MonadFail m) => String -> Map k v -> k -> m v
lookupMapFail name m key =
  case Map.lookup key m of
    Just t -> pure t
    Nothing -> fail $ "Unrecognized " <> name <> ": " ++ show key

This will let us solve this kind of pattern in a much more robust way:

Suggested change

	deriving (Eq, Ord, Show)
	instance IsKey GovPParamsIn where
	keySize = namespaceKeySize @"gov/pparams/v0"
	packKeyM GovPParamsInPrev =
	packByteStringM "prev"
	packKeyM GovPParamsInCurr = packByteStringM "curr"
	packKeyM GovPParamsInPossibleFuture = packByteStringM "fut0"
	packKeyM GovPParamsInDefiniteFuture = packByteStringM "fut1"
	unpackKeyM = do
	tag :: ByteString <- unpackByteStringM 4
	case tag of
	_
	| tag == "prev" -> return GovPParamsInPrev
	| tag == "curr" -> return GovPParamsInCurr
	| tag == "fut0" -> return GovPParamsInPossibleFuture
	| tag == "fut1" -> return GovPParamsInDefiniteFuture
	| otherwise -> fail "Invalid GovPParamsIn tag"
	deriving (Eq, Ord, Show, Enum, Bounded)
	keyGovPParamsIn :: Exchange -> ByteString
	keyGovPParamsIn = \case
	GovPParamsInPrev -> "prev"
	GovPParamsInCurr -> "curr"
	GovPParamsInPossibleFuture -> "fut0"
	GovPParamsInDefiniteFuture -> "fut1"
	mapGovPParamsIn :: Map ByteString Exchange
	mapGovPParamsIn = boundedEnumMap keyGovPParamsIn
	instance IsKey GovPParamsIn where
	keySize = namespaceKeySize @"gov/pparams/v0"
	packKeyM = packByteStringM . keyGovPParamsIn
	unpackKeyM = do
	tag :: ByteString <- unpackByteStringM 4
	lookupMapFail "GovPParamsIn tag" mapGovPParamsIn tag

[lehins]

This shouldn't exist. PParams without protocol version do not make sense

Suggested change

	newtype PParamsWithProtVer era = PParamsWithProtVer {getPParams :: PParams era}
	deriving instance Eq (PParams era) => Eq (PParamsWithProtVer era)
	deriving instance Show (PParams era) => Show (PParamsWithProtVer era)
	instance
	ToCanonicalCBOR "gov/pparams/v0" (PParams era) =>
	ToCanonicalCBOR "gov/pparams/v0" (PParamsWithProtVer era)
	where
	toCanonicalCBOR v (PParamsWithProtVer pparams) = toCanonicalCBOR v pparams
	instance
	FromCanonicalCBOR "gov/pparams/v0" (PParams era) =>
	FromCanonicalCBOR "gov/pparams/v0" (PParamsWithProtVer era)
	where
	fromCanonicalCBOR = fmap PParamsWithProtVer <$> fromCanonicalCBOR
	instance (Era era, EraPParams era, Arbitrary (PParams era)) => Arbitrary (PParamsWithProtVer era) where
	arbitrary = do
	pparams <- arbitrary @(PParams era)
	return $ PParamsWithProtVer $ pparams & ppProtocolVersionL .~ ProtVer (eraProtVerLow @era) 0

[lehins]

Suggested change

	describe "gov/pparams/v0" $ do
	prop "conforms to spec" $
	propNamespaceEntryConformsToSpec @"gov/pparams/v0"
	-- Is checked by the: isCanonical @"gov/pparams/v0" @(PParamsWithProtVer ConwayEra)
	-- prop "canonical with regards to it's definition" $
	-- propNamespaceEntryRoundTrip @"gov/pparams/v0"
	testNS "gov/pparams/v0"

[qnikst]

I've addressed (applied suggestions actually) all the comments

[qnikst]

UPD, now rebased atop of MapExtras and passed the tests.

I've returned SRP because we still adjusting specs