From d7f9dd38fd4400e48deb6728bb8e82f9807a6e59 Mon Sep 17 00:00:00 2001 From: Michael Peyton Jones Date: Mon, 4 Dec 2023 15:28:12 +0000 Subject: [PATCH 1/3] Don't evaluate away builtins where the result might be unserializable See the note. The test case does evaluate the uncompress application until you add the guard, so this is working as desired. We can see if it fixes Kenneth's problem. --- ...eyton-jones_non_representable_constants.md | 3 + .../src/PlutusIR/Analysis/Builtins.hs | 61 ++++++++++++++++++- .../plutus-ir/src/PlutusIR/Core/Plated.hs | 5 ++ .../PlutusIR/Transform/EvaluateBuiltins.hs | 7 ++- .../Transform/EvaluateBuiltins/Tests.hs | 13 +++- .../{trace => traceConservative} | 0 ...{trace.golden => traceConservative.golden} | 0 .../EvaluateBuiltins/traceNonConservative | 2 + .../traceNonConservative.golden | 1 + .../uncompressBlsConservative | 8 +++ .../uncompressBlsConservative.golden | 7 +++ .../uncompressBlsNonConservative | 8 +++ .../uncompressBlsNonConservative.golden | 7 +++ 13 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 plutus-core/changelog.d/20231204_155446_michael.peyton-jones_non_representable_constants.md rename plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/{trace => traceConservative} (100%) rename plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/{trace.golden => traceConservative.golden} (100%) create mode 100644 plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceNonConservative create mode 100644 plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceNonConservative.golden create mode 100644 plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsConservative create mode 100644 plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsConservative.golden create mode 100644 plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsNonConservative create mode 100644 plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsNonConservative.golden diff --git a/plutus-core/changelog.d/20231204_155446_michael.peyton-jones_non_representable_constants.md b/plutus-core/changelog.d/20231204_155446_michael.peyton-jones_non_representable_constants.md new file mode 100644 index 00000000000..eb14a0f96ce --- /dev/null +++ b/plutus-core/changelog.d/20231204_155446_michael.peyton-jones_non_representable_constants.md @@ -0,0 +1,3 @@ +### Fixed + +- The `EvaluateBuiltins` pass will no longer produce constants that can't be serialized. diff --git a/plutus-core/plutus-ir/src/PlutusIR/Analysis/Builtins.hs b/plutus-core/plutus-ir/src/PlutusIR/Analysis/Builtins.hs index 537e230a9bc..2103eea01ec 100644 --- a/plutus-core/plutus-ir/src/PlutusIR/Analysis/Builtins.hs +++ b/plutus-core/plutus-ir/src/PlutusIR/Analysis/Builtins.hs @@ -1,6 +1,7 @@ {-# LANGUAGE FlexibleInstances #-} {-# LANGUAGE GADTs #-} {-# LANGUAGE KindSignatures #-} +{-# LANGUAGE LambdaCase #-} {-# LANGUAGE PatternSynonyms #-} {-# LANGUAGE RankNTypes #-} {-# LANGUAGE TemplateHaskell #-} @@ -13,9 +14,13 @@ module PlutusIR.Analysis.Builtins , BuiltinsInfo (..) , biSemanticsVariant , biMatcherLike + , biUnserializableConstants , builtinArityInfo + , constantIsSerializable + , termIsSerializable , asBuiltinDatatypeMatch , builtinDatatypeMatchBranchArities + , defaultUniUnserializableConstants ) where import Control.Lens hiding (parts) @@ -29,6 +34,8 @@ import PlutusCore.Builtin qualified as PLC import PlutusCore.Default import PlutusCore.MkPlc (mkIterTyAppNoAnn) import PlutusIR.Contexts +import PlutusIR.Core (Term) +import PlutusIR.Core.Plated (_Constant, termSubtermsDeep) import PlutusPrelude (Default (..)) -- | The information we need to work with a builtin that is like a datatype matcher. @@ -41,8 +48,10 @@ makeLenses ''BuiltinMatcherLike -- | All non-static information about builtins that the compiler might want. data BuiltinsInfo (uni :: Type -> Type) fun = BuiltinsInfo - { _biSemanticsVariant :: PLC.BuiltinSemanticsVariant fun - , _biMatcherLike :: Map.Map fun (BuiltinMatcherLike uni fun) + { _biSemanticsVariant :: PLC.BuiltinSemanticsVariant fun + , _biMatcherLike :: Map.Map fun (BuiltinMatcherLike uni fun) + -- See Note [Unserializable constants] + , _biUnserializableConstants :: Some (ValueOf uni) -> Bool } makeLenses ''BuiltinsInfo @@ -50,6 +59,7 @@ instance Default (BuiltinsInfo DefaultUni DefaultFun) where def = BuiltinsInfo { _biSemanticsVariant = def , _biMatcherLike = defaultUniMatcherLike + , _biUnserializableConstants = defaultUniUnserializableConstants } -- | Get the arity of a builtin function from the 'PLC.BuiltinInfo'. @@ -61,6 +71,19 @@ builtinArityInfo -> Arity builtinArityInfo binfo = builtinArity (Proxy @uni) (binfo ^. biSemanticsVariant) +constantIsSerializable + :: forall uni fun + . BuiltinsInfo uni fun + -> Some (ValueOf uni) + -> Bool +constantIsSerializable bi v = not $ _biUnserializableConstants bi v + +termIsSerializable :: BuiltinsInfo uni fun -> Term tyname name uni fun a -> Bool +termIsSerializable binfo = + allOf + (termSubtermsDeep . _Constant) + (constantIsSerializable binfo . snd) + -- | Split a builtin 'match'. asBuiltinDatatypeMatch :: Ord fun @@ -162,3 +185,37 @@ defaultUniMatcherLike = Map.fromList vars <> TypeAppContext resTy resTyAnn (TermAppContext scrut scrutAnn branches) -- both branches have no args chooseListBranchArities = [[], []] + +-- See Note [Unserializable constants] +defaultUniUnserializableConstants :: Some (ValueOf DefaultUni) -> Bool +defaultUniUnserializableConstants = \case + Some (ValueOf DefaultUniBLS12_381_G1_Element _) -> True + Some (ValueOf DefaultUniBLS12_381_G2_Element _) -> True + Some (ValueOf DefaultUniBLS12_381_MlResult _) -> True + _ -> False + +{- Note [Unserializable constants] +Generally we require that programs are (de-)serializable, which requires that all constants +in the program are (de-)serializable. This is enforced by the type system, we have to +have instances for all builtin types in the universe. + +However, in practice we have to limit this somewhat. In particular, some builtin types +have no _cheap_ (de-)serialization method. This notably applies to the BLS constants, where +both BLS "deserialization" and "uncompression" do some sanity checks that take a non-trivial +amount of time. + +This is a problem: in our time budgeting for validating transactions we generally assume +that deserialization is proportional to input size with low constant factors. But for a +malicious program made up of only BLS points, this would not be true! + +So pragmatically we disallow (de-)serialization of constants of such types (the instances +still exist, but they will fail at runtime). The problem then is that we need to make +sure that we don't accidentally create any such constants. It's one thing if the _user_ +does it - then we can just tell them not to (there are usually workarounds). But the +compiler should also not do it! Notably, the EvaluateBuiltins pass can produce _new_ +constants. + +To deal with this problem we pass around a predicate that tells us which constants are +bad, so we can just refuse to perform a transformation if it would produce unrepresentable +constants. +-} diff --git a/plutus-core/plutus-ir/src/PlutusIR/Core/Plated.hs b/plutus-core/plutus-ir/src/PlutusIR/Core/Plated.hs index 7743d17cdd0..2da3f71aebd 100644 --- a/plutus-core/plutus-ir/src/PlutusIR/Core/Plated.hs +++ b/plutus-core/plutus-ir/src/PlutusIR/Core/Plated.hs @@ -28,6 +28,7 @@ module PlutusIR.Core.Plated , termUniquesDeep , varDeclSubtypes , underBinders + , _Constant ) where import PlutusCore qualified as PLC @@ -49,6 +50,10 @@ infixr 6 <^> (<^>) :: Fold s a -> Fold s a -> Fold s a (f1 <^> f2) g s = f1 g s *> f2 g s +-- | View a term as a constant. +_Constant :: Prism' (Term tyname name uni fun a) (a, PLC.Some (PLC.ValueOf uni)) +_Constant = prism' (uncurry Constant) (\case { Constant a v -> Just (a, v); _ -> Nothing }) + {-# INLINE bindingSubterms #-} -- | Get all the direct child 'Term's of the given 'Binding'. bindingSubterms :: Traversal' (Binding tyname name uni fun a) (Term tyname name uni fun a) diff --git a/plutus-core/plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs b/plutus-core/plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs index 35c98902bbc..7be25428e08 100644 --- a/plutus-core/plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs +++ b/plutus-core/plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs @@ -13,7 +13,7 @@ import PlutusCore.Builtin import PlutusIR.Contexts import PlutusIR.Core -import Control.Lens (transformOf, (^.)) +import Control.Lens (allOf, transformOf, (^.)) import Data.Functor (void) import PlutusIR.Analysis.Builtins @@ -68,6 +68,7 @@ evaluateBuiltins conservative binfo costModel = transformOf termSubterms process -- suboptimal, e.g. in `ifThenElse True x y`, we will get back `x`, but -- with the annotation that was on the `ifThenElse` node. But we can't -- easily do better. - Just t' -> x <$ t' - Nothing -> t + -- See Note [Unserializable constants] + Just t' | termIsSerializable binfo t' -> x <$ t' + _ -> t processTerm t = t diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs index 4d5c3a6b028..dea56074222 100644 --- a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs @@ -15,15 +15,26 @@ import Test.QuickCheck.Property (Property, withMaxSuccess) test_evaluateBuiltins :: TestTree test_evaluateBuiltins = runTestNestedIn ["plutus-ir", "test", "PlutusIR", "Transform"] $ testNested "EvaluateBuiltins" $ + conservative ++ nonConservative + where + conservative = map (goldenPir (evaluateBuiltins True def def) pTerm) [ "addInteger" , "ifThenElse" - , "trace" + , "traceConservative" , "failingBuiltin" , "nonConstantArg" , "overApplication" , "underApplication" + , "uncompressBlsConservative" + ] + nonConservative = + map + (goldenPir (evaluateBuiltins False def def) pTerm) + -- We want to test the case where this would reduce, i.e. + [ "traceNonConservative" + , "uncompressBlsNonConservative" ] -- | Check that a term typechecks after a `PlutusIR.Transform.EvaluateBuiltins` diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/trace b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceConservative similarity index 100% rename from plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/trace rename to plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceConservative diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/trace.golden b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceConservative.golden similarity index 100% rename from plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/trace.golden rename to plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceConservative.golden diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceNonConservative b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceNonConservative new file mode 100644 index 00000000000..4f77aa46869 --- /dev/null +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceNonConservative @@ -0,0 +1,2 @@ +-- In non-conservative mode should be removed +[{(builtin trace) (con integer) } (con string "hello") (con integer 1)] diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceNonConservative.golden b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceNonConservative.golden new file mode 100644 index 00000000000..132831f390c --- /dev/null +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/traceNonConservative.golden @@ -0,0 +1 @@ +(con integer 1) \ No newline at end of file diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsConservative b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsConservative new file mode 100644 index 00000000000..b0b86dd7e6c --- /dev/null +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsConservative @@ -0,0 +1,8 @@ +-- In conservative mode should be left +[ + (builtin bls12_381_G2_uncompress) + (con + bytestring + #c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + ) +] diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsConservative.golden b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsConservative.golden new file mode 100644 index 00000000000..1238bc334e3 --- /dev/null +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsConservative.golden @@ -0,0 +1,7 @@ +[ + (builtin bls12_381_G2_uncompress) + (con + bytestring + #c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + ) +] \ No newline at end of file diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsNonConservative b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsNonConservative new file mode 100644 index 00000000000..04486a211d4 --- /dev/null +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsNonConservative @@ -0,0 +1,8 @@ +-- Should also be left in non-conservative mode because the result is unrepresentable +[ + (builtin bls12_381_G2_uncompress) + (con + bytestring + #c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + ) +] diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsNonConservative.golden b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsNonConservative.golden new file mode 100644 index 00000000000..1238bc334e3 --- /dev/null +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressBlsNonConservative.golden @@ -0,0 +1,7 @@ +[ + (builtin bls12_381_G2_uncompress) + (con + bytestring + #c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + ) +] \ No newline at end of file From 56d73f4bcedc99b5dc47e138a9798e75fc28d5a0 Mon Sep 17 00:00:00 2001 From: Michael Peyton Jones Date: Mon, 4 Dec 2023 16:04:08 +0000 Subject: [PATCH 2/3] Fix warning --- .../plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plutus-core/plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs b/plutus-core/plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs index 7be25428e08..ea5455d2784 100644 --- a/plutus-core/plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs +++ b/plutus-core/plutus-ir/src/PlutusIR/Transform/EvaluateBuiltins.hs @@ -13,7 +13,7 @@ import PlutusCore.Builtin import PlutusIR.Contexts import PlutusIR.Core -import Control.Lens (allOf, transformOf, (^.)) +import Control.Lens (transformOf, (^.)) import Data.Functor (void) import PlutusIR.Analysis.Builtins From 413b3a9f73a21af521b54fb7ad7efd9b434381f1 Mon Sep 17 00:00:00 2001 From: Michael Peyton Jones Date: Thu, 7 Dec 2023 11:38:22 +0000 Subject: [PATCH 3/3] Comments --- .../src/PlutusIR/Analysis/Builtins.hs | 8 ++++++++ .../Transform/EvaluateBuiltins/Tests.hs | 1 + .../uncompressAndEqualBlsNonConservative | 7 +++++++ ...ncompressAndEqualBlsNonConservative.golden | 19 +++++++++++++++++++ 4 files changed, 35 insertions(+) create mode 100644 plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressAndEqualBlsNonConservative create mode 100644 plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressAndEqualBlsNonConservative.golden diff --git a/plutus-core/plutus-ir/src/PlutusIR/Analysis/Builtins.hs b/plutus-core/plutus-ir/src/PlutusIR/Analysis/Builtins.hs index 2103eea01ec..6baa82fc0cf 100644 --- a/plutus-core/plutus-ir/src/PlutusIR/Analysis/Builtins.hs +++ b/plutus-core/plutus-ir/src/PlutusIR/Analysis/Builtins.hs @@ -218,4 +218,12 @@ constants. To deal with this problem we pass around a predicate that tells us which constants are bad, so we can just refuse to perform a transformation if it would produce unrepresentable constants. + +An alternative approach would be to instead add a pass to rewrite the problematic +constants into a non-problematic form (e.g. conversion from a bytestring instead of a constant). +This would be better for optimization, since we wouldn't be blocking EvaluateBuiltins +from working, even if it was good, but it has two problems: +1. It would fight with EvaluateBuiltins, which each undoing the other. +2. It can't work generically, since we don't always have a way to do the transformation. In +particular, there isn't a way to do this for the ML-result BLS type. -} diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs index dea56074222..f044776bf35 100644 --- a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs @@ -35,6 +35,7 @@ test_evaluateBuiltins = runTestNestedIn ["plutus-ir", "test", "PlutusIR", "Trans -- We want to test the case where this would reduce, i.e. [ "traceNonConservative" , "uncompressBlsNonConservative" + , "uncompressAndEqualBlsNonConservative" ] -- | Check that a term typechecks after a `PlutusIR.Transform.EvaluateBuiltins` diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressAndEqualBlsNonConservative b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressAndEqualBlsNonConservative new file mode 100644 index 00000000000..5faf93649b5 --- /dev/null +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressAndEqualBlsNonConservative @@ -0,0 +1,7 @@ +-- This would evaluate all the way to a boolean constant (which is serializable!) but we block the intermediate states +-- (which have unserializable constants), so we can't get there. +[ + (builtin bls12_381_G1_equal) + [(builtin bls12_381_G1_uncompress) (con bytestring #97f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb)] + [(builtin bls12_381_G1_uncompress) (con bytestring #97f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb)] +] diff --git a/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressAndEqualBlsNonConservative.golden b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressAndEqualBlsNonConservative.golden new file mode 100644 index 00000000000..ba9e9f50707 --- /dev/null +++ b/plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/uncompressAndEqualBlsNonConservative.golden @@ -0,0 +1,19 @@ +[ + [ + (builtin bls12_381_G1_equal) + [ + (builtin bls12_381_G1_uncompress) + (con + bytestring + #97f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb + ) + ] + ] + [ + (builtin bls12_381_G1_uncompress) + (con + bytestring + #97f1d3a73197d7942695638c4fa9ac0fc3688c4f9774b905a14e3a3f171bac586c55e83ff97a1aeffb3af00adb22c6bb + ) + ] +] \ No newline at end of file