better errors, some other minor edits

README.md

@@ -130,15 +130,24 @@ script1dnt66jufkgx8rqxypxtz0hcrxs6hhayuj9cqh0eca82lcpwawd4
 ```
 </details>
 
+<details>
+  <summary>How to construct a multisig script hash with timelocks</summary>
+
+```console
+$  cardano-address script hash "all [$(cat script.1.xvk), $(cat script.2.xvk), active_from 100, active_until 120]"
+```
+</details>
+
+
 <details>
   <summary>How to validate a script</summary>
 
 ```console
-$  cardano-address script validate --required  "at_least 1 [$(cat script.1.xvk), $(cat script.2.xvk), $(cat script.2.xvk)]"
+$  cardano-address script validate "at_least 1 [$(cat script.1.xvk), $(cat script.2.xvk), $(cat script.2.xvk)]"
 Validated.
 
 $  cardano-address script validate --recommended  "at_least 1 [$(cat script.1.xvk), $(cat script.2.xvk), $(cat script.2.xvk)]"
-Not validated: The list inside a script has duplicate keys.
+Not validated: The list inside a script has duplicate keys (which is not recommended)..
 ```
 </details>
 

command-line/lib/Command/Script/Validation.hs

@@ -79,7 +79,7 @@ mod liftCmd = command "validate" $
             , indent 2 $ string "Validated."
             , string ""
             , indent 2 $ bold $ string $ progName<>" script validate --recommended 'all []'"
-            , indent 2 $ string "Not validated: The list inside a script is empty."
+            , indent 2 $ string "Not validated: The list inside a script is empty or only contains timelocks (which is not recommended)."
             , string ""
             , indent 2 $ bold $ string $ progName<>" script validate 'at_least 1 [active_from 11, active_until 16]'"
             , indent 2 $ string "Validated."
@@ -94,7 +94,7 @@ mod liftCmd = command "validate" $
             , indent 4 $ bold $ string "[ script_vkh18srsxr3khll7vl3w9mqfu55n6wzxxlxj7qzr2mhnyreluzt36ms"
             , indent 4 $ bold $ string ", script_vkh18srsxr3khll7vl3w9mqfu55n6wzxxlxj7qzr2mhnyreluzt36ms"
             , indent 4 $ bold $ string "]'"
-            , indent 2 $ string "Not validated: The list inside a script has duplicate keys."
+            , indent 2 $ string "Not validated: The list inside a script has duplicate keys (which is not recommended)."
             ])
   where
     parser = Cmd

command-line/lib/Options/Applicative/Script.hs

@@ -19,10 +19,11 @@ import Cardano.Address.Script
     , Script (..)
     , ScriptHash
     , ValidationLevel (..)
+    , prettyErrValidateScript
     , scriptHashFromBytes
     )
 import Cardano.Address.Script.Parser
-    ( prettyErrValidateScript, scriptFromString )
+    ( scriptFromString )
 import Control.Applicative
     ( (<|>) )
 import Control.Arrow

command-line/test/Command/Script/HashSpec.hs

@@ -8,7 +8,7 @@ module Command.Script.HashSpec
 
 import Prelude
 
-import Cardano.Address.Script.Parser
+import Cardano.Address.Script
     ( ErrValidateScript (..), prettyErrValidateScript )
 import Data.String.Interpolate
     ( iii )

command-line/test/Command/Script/ValidationSpec.hs

@@ -9,9 +9,11 @@ module Command.Script.ValidationSpec
 import Prelude
 
 import Cardano.Address.Script
-    ( ValidationLevel (..) )
-import Cardano.Address.Script.Parser
-    ( ErrValidateScript (..), prettyErrValidateScript )
+    ( ErrRecommendedValidateScript (..)
+    , ErrValidateScript (..)
+    , ValidationLevel (..)
+    , prettyErrValidateScript
+    )
 import Data.String.Interpolate
     ( iii )
 import Test.Hspec
@@ -43,7 +45,7 @@ spec = do
         specScriptNotValidated Malformed RequiredValidation
             [iii|any [ #{verKeyH1}, #{verKeyH2}, active_from a]|]
 
-        specScriptNotValidated EmptyList RecommendedValidation
+        specScriptNotValidated (NotRecommended EmptyList) RecommendedValidation
             [iii|all []|]
 
         specScriptValidated RequiredValidation
@@ -52,25 +54,25 @@ spec = do
         specScriptValidated RequiredValidation
             [iii|at_least 2 [ #{verKeyH1},  active_from 11, active_until 25]|]
 
-        specScriptNotValidated ListTooSmall RecommendedValidation
+        specScriptNotValidated (NotRecommended ListTooSmall) RecommendedValidation
             [iii|at_least 2 [ #{verKeyH1},  active_from 11, active_until 25]|]
 
         specScriptValidated RequiredValidation
             [iii|at_least 2 [ #{verKeyH1},  active_from 11, active_until 25, active_until 30]|]
 
-        specScriptNotValidated RedundantTimelocks RecommendedValidation
+        specScriptNotValidated (NotRecommended RedundantTimelocks) RecommendedValidation
             [iii|at_least 1 [ #{verKeyH1},  active_from 11, active_until 25, active_until 30]|]
 
         specScriptValidated RequiredValidation
             [iii|any [ #{verKeyH1}, #{verKeyH2}, #{verKeyH2}]|]
 
-        specScriptNotValidated DuplicateSignatures RecommendedValidation
+        specScriptNotValidated (NotRecommended DuplicateSignatures) RecommendedValidation
             [iii|any [ #{verKeyH1}, #{verKeyH2}, #{verKeyH2}]|]
 
         specScriptValidated RequiredValidation
             [iii|at_least 0 [ #{verKeyH1}, #{verKeyH2} ]|]
 
-        specScriptNotValidated MZero RecommendedValidation
+        specScriptNotValidated (NotRecommended MZero) RecommendedValidation
             [iii|at_least 0 [ #{verKeyH1}, #{verKeyH2} ]|]
 
 levelStr :: ValidationLevel -> String

core/lib/Cardano/Address/Script.hs

@@ -27,6 +27,7 @@ module Cardano.Address.Script
     -- * Validation
     , ValidationLevel (..)
     , ErrValidateScript (..)
+    , ErrRecommendedValidateScript (..)
     , ErrValidateScriptTemplate (..)
     , TxValidity (..)
     , validateScript
@@ -338,52 +339,55 @@ validateScript level interval script = do
             (BS.length bytes == credentialHashSize)
     let allSigs = foldScript (:) [] script
     unless (L.all validateKeyHash allSigs) $ Left WrongKeyHash
-    unless (requiredValidation interval script)
-        $ Left LedgerIncompatible
-    when (level == RecommendedValidation ) $
-        recommendedValidation script
+
+    requiredValidation interval script
+
+    when (level == RecommendedValidation) $
+        mapLeft NotRecommended (recommendedValidation script)
 
 requiredValidation
-    :: Eq elem
-    => Maybe TxValidity
+    :: Maybe TxValidity
     -> Script elem
-    -> Bool
-requiredValidation validity = \case
-    RequireSignatureOf _ -> True
-
-    RequireAllOf xs ->
-        L.all (requiredValidation validity) xs
-
-    RequireAnyOf xs ->
-        L.any (requiredValidation validity) xs
-
-    RequireSomeOf m xs ->
-        m <= sum (fmap (\x -> if requiredValidation validity x then 1 else 0) xs)
-
-    ActiveFromSlot lockStart -> case validity of
-        Just validity' ->
-            let (TxValidity txStart _) = validity'
-            in lockStart `lteZero` txStart
-        Nothing -> True
-
-    ActiveUntilSlot lockExpiry -> case validity of
-        Just validity' ->
-            let (TxValidity _ txExpiry) = validity'
-            in txExpiry `ltePosInfty` lockExpiry
-        Nothing -> True
+    -> Either ErrValidateScript ()
+requiredValidation validity script =
+    unless (check script) $ Left LedgerIncompatible
   where
-      lteZero :: Natural -> Maybe Natural -> Bool
-      lteZero i Nothing = i==0
-      lteZero i (Just j) = i <= j
+    check = \case
+        RequireSignatureOf _ -> True
+
+        RequireAllOf xs ->
+            L.all check xs
+
+        RequireAnyOf xs ->
+            L.any check xs
+
+        RequireSomeOf m xs ->
+            m <= sum (fmap (\x -> if check x then 1 else 0) xs)
+
+        ActiveFromSlot lockStart -> case validity of
+            Just validity' ->
+                let (TxValidity txStart _) = validity'
+                in lockStart `lteZero` txStart
+            Nothing -> True
+
+        ActiveUntilSlot lockExpiry -> case validity of
+            Just validity' ->
+                let (TxValidity _ txExpiry) = validity'
+                in txExpiry `ltePosInfty` lockExpiry
+            Nothing -> True
+
+    lteZero :: Natural -> Maybe Natural -> Bool
+    lteZero i Nothing = i==0
+    lteZero i (Just j) = i <= j
 
-      ltePosInfty :: Maybe Natural -> Natural -> Bool
-      ltePosInfty Nothing _ = False -- ∞ > j
-      ltePosInfty (Just i) j = i <= j
+    ltePosInfty :: Maybe Natural -> Natural -> Bool
+    ltePosInfty Nothing _ = False -- ∞ > j
+    ltePosInfty (Just i) j = i <= j
 
 recommendedValidation
     :: Eq elem
     => Script elem
-    -> Either ErrValidateScript ()
+    -> Either ErrRecommendedValidateScript ()
 recommendedValidation = \case
     RequireSignatureOf _ -> pure ()
 
@@ -448,26 +452,32 @@ validateScriptTemplate level interval (ScriptTemplate cosigners' script) = do
             Set.fromList (Map.keys cosigners') `difference` allCosigners
     unless (Set.null unusedCosigners) $ Left UnusedCosigner
     mapLeft WrongScript $ do
-        unless (requiredValidation interval script)
-            $ Left LedgerIncompatible
+        requiredValidation interval script
         when (level == RecommendedValidation ) $
-            recommendedValidation script
+            mapLeft NotRecommended (recommendedValidation script)
 
 -- | Possible validation errors when validating a script
 --
 -- @since 3.0.0
 data ErrValidateScript
     = LedgerIncompatible
-    | EmptyList
+    | WrongKeyHash
+    | Malformed
+    | NotRecommended ErrRecommendedValidateScript
+    deriving (Eq, Show)
+
+-- | Possible recommended validation errors when validating a script
+--
+-- @since 3.2.0
+data ErrRecommendedValidateScript
+    = EmptyList
     | ListTooSmall
     | MZero
     | DuplicateSignatures
-    | WrongKeyHash
-    | Malformed
     | RedundantTimelocks
     deriving (Eq, Show)
 
--- | Possible validation errors when validating a script
+-- | Possible validation errors when validating a script template
 --
 -- @since 3.2.0
 data ErrValidateScriptTemplate
@@ -487,22 +497,25 @@ prettyErrValidateScript
 prettyErrValidateScript = \case
     LedgerIncompatible ->
         "The script is ill-formed and is not going to be accepted by ledger."
-    EmptyList ->
-        "The list inside a script is empty."
-    MZero ->
-        "At least's number must be not smaller than 1."
-    ListTooSmall ->
-        "At least's number must not be larger than the non-timelock elements in the list."
-    DuplicateSignatures ->
-        "The list inside a script has duplicate keys."
     WrongKeyHash ->
-        "The hash of verification key is expected to have "<>show credentialHashSize<>" bytes."
-    RedundantTimelocks ->
-        "Timelocks used are either redundant or contradictory."
+        "The hash of verification key is expected to have "
+        <> show credentialHashSize <> " bytes."
     Malformed ->
         "Parsing of the script failed. The script should be composed of nested \
         \lists, the verification keys should be bech32-encoded with prefix 'script_vhk', \
         \timelocks must use non-negative numbers as slots."
+    NotRecommended EmptyList ->
+        "The list inside a script is empty or only contains timelocks \
+        \(which is not recommended)."
+    NotRecommended MZero ->
+        "At least's coefficient is 0 (which is not recommended)."
+    NotRecommended ListTooSmall ->
+        "At least's coefficient is larger than the number of non-timelock \
+        \elements in the list (which is not recommended)."
+    NotRecommended DuplicateSignatures ->
+        "The list inside a script has duplicate keys (which is not recommended)."
+    NotRecommended RedundantTimelocks ->
+        "Some timelocks used are redundant (which is not recommended)."
 
 -- | Pretty-print a script template validation error.
 --

core/lib/Cardano/Address/Script/Parser.hs

@@ -11,11 +11,6 @@ module Cardano.Address.Script.Parser
       scriptFromString
     , scriptParser
 
-    -- ** Script Validator
-    , validateScript
-    , ErrValidateScript (..)
-    , prettyErrValidateScript
-
     -- Internal
     , requireSignatureOfParser
     , requireAllOfParser
@@ -31,8 +26,6 @@ import Cardano.Address.Script
     , Script (..)
     , keyHashFromText
     , prettyErrKeyHashFromText
-    , prettyErrValidateScript
-    , validateScript
     )
 import Data.Char
     ( isDigit, isLetter )