Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CIP-0138 Builtin Array #6727

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+364 −51
Open

CIP-0138 Builtin Array #6727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a3cca3f
chore: remove unused pragmas and make explicit import lists where app…
Unisay Nov 28, 2024
763a261
chore: a few eta reductions
Unisay Nov 29, 2024
6eb76ee
fixup! chore(deps): bump actions/github-script from 6 to 7 (#5633)
Unisay Nov 29, 2024
2ac9dba
BuiltinArray + functions in Plutus Core
Unisay Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions plutus-core/cost-model/create-cost-model/BuiltinMemoryModels.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ builtinMemoryModels = BuiltinCostModelBase
, paramHeadList = Id $ ModelOneArgumentConstantCost 32
, paramTailList = Id $ ModelOneArgumentConstantCost 32
, paramNullList = Id $ ModelOneArgumentConstantCost 32
, paramLengthArray = Id $ ModelOneArgumentConstantCost 99
, paramListToArray = Id $ ModelOneArgumentConstantCost 99
Comment on lines +113 to +114
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dummy values

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwxm is this what should be done here before costing? Particularly given that listToArray isn't gonna be constant-cost.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwxm is this what should be done here before costing? Particularly given that listToArray isn't gonna be constant-cost.

Good question. I think that this doesn't really matter as long as you remember to fix it. The only time this gets used is when you run generate-cost-model, which uses this information to fill in the memory function in the JSON version of the cost model, so it has to be correct before you generate the final form of the cost model.

, paramIndexArray = Id $ ModelTwoArgumentsConstantCost 99
, paramChooseData = Id $ ModelSixArgumentsConstantCost 32
, paramConstrData = Id $ ModelTwoArgumentsConstantCost 32
, paramMapData = Id $ ModelOneArgumentConstantCost 32
Expand Down
7 changes: 7 additions & 0 deletions plutus-core/cost-model/create-cost-model/CreateBuiltinCostModel.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ builtinCostModelNames = BuiltinCostModelBase
, paramHeadList = "headListModel"
, paramTailList = "tailListModel"
, paramNullList = "nullListModel"
, paramLengthArray = "lengthArrayModel"
, paramListToArray = "listToArrayModel"
, paramIndexArray = "indexArrayModel"
, paramChooseData = "chooseDataModel"
, paramConstrData = "constrDataModel"
, paramMapData = "mapDataModel"
Expand Down Expand Up @@ -209,6 +212,10 @@ createBuiltinCostModel bmfile rfile = do
paramHeadList <- getParams readCF1 paramHeadList
paramTailList <- getParams readCF1 paramTailList
paramNullList <- getParams readCF1 paramNullList
-- Arrays
paramLengthArray <- getParams readCF1 paramLengthArray
paramListToArray <- getParams readCF1 paramListToArray
paramIndexArray <- getParams readCF2 paramIndexArray
-- Data
paramChooseData <- getParams readCF6 paramChooseData
paramConstrData <- getParams readCF2 paramConstrData
Expand Down
30 changes: 30 additions & 0 deletions plutus-core/cost-model/data/builtinCostModelA.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,36 @@
"type": "constant_cost"
}
},
"lengthArray" : {
"cpu": {
"arguments": 99999999999999,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporary value (stub)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I was adding builtins, I was avoiding doing any of this, but maybe that was wrong, dunno. @kwxm should we add this stuff when adding a new builtin before costing is done?

Copy link
Contributor

@kwxm kwxm Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwxm should we add this stuff when adding a new builtin before costing is done?

I think that in order to get the code to compile you have to add something here once you've added paramLengthArray here, and similarly for the other new functions. However, if you use unimplementedCostingFunction in toBuiltinMeaning then these numbers won't be used: they only come into play when you tell the denotation to run the real costing function. I'd normally add a new builtin without adding any costing code at all in order to test that it behaves as it's supposed to, and then do all of the costing at once. That's just to cut down on the initial amount of work, and because of the way all the costing code is plumbed in it's probably not obvious that you don't initially need it. The important thing is that there's no link at all between the builtin and the costing code until you put runCostingFunction<N>Arguments . param<MyNewBuiltin> in toBuiltinMeaning. Maybe we should try to make that fact clearer to implementers somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, doing it first time it isn't 100% clear where to draw the line, so I followed the compiler and it wanted me to fix JSON as there is TH which reads it and fails to compile if the section is missing.

"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"listToArray": {
"cpu": {
"arguments": 99999999999999,
"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"indexArray": {
"cpu": {
"arguments": 99999999999999,
"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"quotientInteger": {
"cpu": {
"arguments": {
Expand Down
30 changes: 30 additions & 0 deletions plutus-core/cost-model/data/builtinCostModelB.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,36 @@
"type": "constant_cost"
}
},
"lengthArray" : {
"cpu": {
"arguments": 99999999999999,
"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"listToArray": {
"cpu": {
"arguments": 99999999999999,
"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"indexArray": {
"cpu": {
"arguments": 99999999999999,
"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"quotientInteger": {
"cpu": {
"arguments": {
Expand Down
30 changes: 30 additions & 0 deletions plutus-core/cost-model/data/builtinCostModelC.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,36 @@
"type": "constant_cost"
}
},
"lengthArray" : {
"cpu": {
"arguments": 99999999999999,
"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"listToArray": {
"cpu": {
"arguments": 99999999999999,
"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"indexArray": {
"cpu": {
"arguments": 99999999999999,
"type": "constant_cost"
},
"memory": {
"arguments": 99999999999999,
"type": "constant_cost"
}
},
"quotientInteger": {
"cpu": {
"arguments": {
Expand Down
7 changes: 5 additions & 2 deletions plutus-core/plutus-core.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ library
other-modules:
Data.Aeson.Flatten
Data.Functor.Foldable.Monadic
Data.Vector.Orphans
PlutusCore.Builtin.HasConstant
PlutusCore.Builtin.KnownKind
PlutusCore.Builtin.KnownType
Expand Down Expand Up @@ -341,7 +342,7 @@ library
, time
, transformers
, unordered-containers
, vector
, vector ^>=0.13.2
Unisay marked this conversation as resolved.
Show resolved Hide resolved
, witherable

if impl(ghc <9.0)
Expand Down Expand Up @@ -376,7 +377,7 @@ test-suite plutus-core-test
default-language: Haskell2010
build-depends:
, aeson
, base >=4.9 && <5
, base >=4.9 && <5
, bytestring
, containers
, data-default-class
Expand All @@ -400,6 +401,7 @@ test-suite plutus-core-test
, text
, th-lift-instances
, th-utilities
, vector ^>=0.13.2

test-suite untyped-plutus-core-test
import: lang
Expand Down Expand Up @@ -815,6 +817,7 @@ library plutus-core-testlib
, tasty-hedgehog
, tasty-hunit
, text
, vector

-- This wraps up the use of the certifier library
-- so we can present a consistent inteface whether we
Expand Down
16 changes: 16 additions & 0 deletions plutus-core/plutus-core/src/Data/Vector/Orphans.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{-# OPTIONS_GHC -Wno-orphans #-}

module Data.Vector.Orphans () where

import Data.Hashable (Hashable (hashWithSalt))
import Data.Vector.Strict qualified as Strict
import Flat (Flat (..))
import Flat.Instances.Vector ()

instance (Hashable a) => Hashable (Strict.Vector a) where
hashWithSalt = Strict.foldl' hashWithSalt

instance (Flat a) => Flat (Strict.Vector a) where
size = size . Strict.toLazy -- Strict to Lazy is O(1)
encode = encode . Strict.toLazy
decode = Strict.fromLazy <$> decode -- Strict from Lazy is O(1)
Unisay marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions plutus-core/plutus-core/src/PlutusCore/Builtin/Result.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,12 @@ _UnliftingErrorVia :: Pretty err => err -> Prism' err UnliftingError
_UnliftingErrorVia err = iso (MkUnliftingError . display) (const err)
{-# INLINE _UnliftingErrorVia #-}

-- | See Note [Structural vs operational errors within builtins]
_StructuralUnliftingError :: AsBuiltinError err => Prism' err UnliftingError
_StructuralUnliftingError = _BuiltinUnliftingEvaluationError . _StructuralEvaluationError
{-# INLINE _StructuralUnliftingError #-}

-- | See Note [Structural vs operational errors within builtins]
_OperationalUnliftingError :: AsBuiltinError err => Prism' err UnliftingError
_OperationalUnliftingError = _BuiltinUnliftingEvaluationError . _OperationalEvaluationError
{-# INLINE _OperationalUnliftingError #-}
Expand Down
2 changes: 1 addition & 1 deletion plutus-core/plutus-core/src/PlutusCore/Crypto/BLS12_381/Pairing.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ instance NFData MlResult where
rnf _ = ()

instance Hashable MlResult where
hashWithSalt salt = const salt
hashWithSalt salt _MlResult = salt
Unisay marked this conversation as resolved.
Show resolved Hide resolved

millerLoop :: G1.Element -> G2.Element -> MlResult
millerLoop = coerce BlstBindings.millerLoop
Expand Down
Loading
Loading