Address guardrail script audit comments

cardano-constitution/src/Cardano/Constitution/Validator/Common.hs

@@ -9,7 +9,6 @@ module Cardano.Constitution.Validator.Common
     ( withChangedParams
     , ChangedParams
     , ConstitutionValidator
-    , lookupUnsafe
     , validateParamValue
     ) where
 
@@ -37,7 +36,7 @@ withChangedParams fun (scriptContextToValidGovAction -> validGovAction) =
     case validGovAction of
         Just cparams ->  if fun cparams
             then BI.unitval
-            else BI.trace "ChangedParams failed to validate" (B.error ())
+            else traceError "ChangedParams failed to validate"
         Nothing -> BI.unitval -- this is a treasury withdrawal, we just accept it
 
 {-# INLINABLE validateParamValue #-}
@@ -89,7 +88,7 @@ scriptContextToValidGovAction = scriptContextToScriptInfo
     scriptInfoToProposalProcedure (BI.unsafeDataAsConstr -> si) =
         if BI.fst si `B.equalsInteger` 5 -- Constructor Index of `ProposingScript`
         then BI.head (BI.tail (BI.snd si))
-        else B.trace "Not a ProposalProcedure. This should not ever happen, because ledger should guard before, against it." (B.error ())
+        else traceError "Not a ProposalProcedure. This should not ever happen, because ledger should guard before, against it."
 
     proposalProcedureToGovernanceAction :: BuiltinData -> BuiltinData
     proposalProcedureToGovernanceAction = BI.unsafeDataAsConstr
@@ -104,14 +103,4 @@ scriptContextToValidGovAction = scriptContextToScriptInfo
         | govActionConstr `B.equalsInteger` 0 = Just (B.unsafeDataAsMap (BI.head (BI.tail (BI.snd govAction))))
         -- Constructor Index of `TreasuryWithdrawals` is 2
         | govActionConstr `B.equalsInteger` 2 = Nothing -- means treasurywithdrawal
-        | otherwise = B.trace "Not a ChangedParams or TreasuryWithdrawals. This should not ever happen, because ledger should guard before, against it." (B.error ())
-
-{-# INLINEABLE lookupUnsafe #-}
--- | An unsafe version of PlutusTx.AssocMap.lookup, specialised to Integer keys
-lookupUnsafe :: Integer -> [(Integer, v)] -> v
-lookupUnsafe k = go
- where
-   go [] = B.trace "Unsorted lookup failed" (B.error ())
-   go ((k', i) : xs') = if k `B.equalsInteger` k'
-                        then i
-                        else go xs'
+        | otherwise = traceError "Not a ChangedParams or TreasuryWithdrawals. This should not ever happen, because ledger should guard before, against it."

cardano-constitution/src/Cardano/Constitution/Validator/Unsorted.hs

@@ -1,4 +1,5 @@
 {-# LANGUAGE NoImplicitPrelude #-}
+{-# LANGUAGE OverloadedStrings #-}
 {-# LANGUAGE Strict            #-}
 {-# LANGUAGE TemplateHaskell   #-}
 {-# LANGUAGE ViewPatterns      #-}
@@ -32,9 +33,19 @@ validateParam :: ConstitutionConfig -> (BuiltinData, BuiltinData) -> Bool
 validateParam (ConstitutionConfig cfg) (B.unsafeDataAsI -> actualPid, actualValueData) =
     Common.validateParamValue
       -- If param not found, it will error
-      (Common.lookupUnsafe actualPid cfg)
+      (lookupUnsafe actualPid cfg)
       actualValueData
 
+{-# INLINEABLE lookupUnsafe #-}
+-- | An unsafe version of PlutusTx.AssocMap.lookup, specialised to Integer keys
+lookupUnsafe :: Integer -> [(Integer, v)] -> v
+lookupUnsafe k = go
+ where
+   go [] = traceError "Unsorted lookup failed"
+   go ((k', i) : xs') = if k `B.equalsInteger` k'
+                        then i
+                        else go xs'
+
 -- | Statically configure the validator with the `defaultConstitutionConfig`.
 defaultConstitutionValidator :: ConstitutionValidator
 defaultConstitutionValidator = constitutionValidator defaultConstitutionConfig

cardano-constitution/src/PlutusTx/NonCanonicalRational.hs

@@ -11,6 +11,7 @@ import PlutusTx as Tx
 import PlutusTx.Builtins as B
 import PlutusTx.Builtins.Internal as BI
 import PlutusTx.Ratio as Tx
+import PlutusTx.Trace (traceError)
 
 -- We agreed to have a different BuiltinData encoding for Rationals for the ConstitutionScript,
 -- other than the canonical encoding for datatypes.
@@ -31,5 +32,5 @@ instance UnsafeFromData NonCanonicalRational where
         let bl' = BI.tail bl
         in BI.ifThenElse (BI.null (BI.tail bl'))
             (\() -> NonCanonicalRational (Tx.unsafeRatio (B.unsafeDataAsI (BI.head bl)) (B.unsafeDataAsI (BI.head bl'))))
-            (\() -> BI.trace "A Rational had too many list components" (B.error ()))
+            (\() -> traceError "A Rational had too many list components")
            ()

cardano-constitution/test/Cardano/Constitution/Validator/GoldenTests/sorted.cbor.size.golden

@@ -1 +1 @@
-2135
+2132

cardano-constitution/test/Cardano/Constitution/Validator/GoldenTests/sorted.large.budget.golden

@@ -1 +1 @@
-ExBudget {exBudgetCPU = ExCPU 601572171, exBudgetMemory = ExMemory 2972418}
+ExBudget {exBudgetCPU = ExCPU 601524171, exBudgetMemory = ExMemory 2972118}

cardano-constitution/test/Cardano/Constitution/Validator/GoldenTests/sorted.pir.golden

@@ -202,10 +202,6 @@
                   {all dead. dead})
              {all dead. dead}
    in
-   let
-     data Unit | Unit_match where
-       Unit : Unit
-   in
    letrec
      data ParamValue | ParamValue_match where
        ParamAny : ParamValue
@@ -215,6 +211,10 @@
        ParamRational :
          (\v -> List (Tuple2 PredKey (List v))) Rational -> ParamValue
    in
+   let
+     data Unit | Unit_match where
+       Unit : Unit
+   in
    letrec
      !validateParamValue : ParamValue -> data -> Bool
        = \(eta : ParamValue) (eta : data) ->

cardano-constitution/test/Cardano/Constitution/Validator/GoldenTests/sorted.small.budget.golden

@@ -1 +1 @@
-ExBudget {exBudgetCPU = ExCPU 91621157, exBudgetMemory = ExMemory 414205}
+ExBudget {exBudgetCPU = ExCPU 91573157, exBudgetMemory = ExMemory 413905}