Add a data-backed `ScriptContext` #6171

ana-pantilie · 2024-06-04T10:27:42Z

Adds a first version of a data-backed ScriptContext. For now, "data-backed" means that all the maps contained by the ScriptContext are represented as BuiltinData. The next step will be to make lists data-backed as well, hoping that reduces encoding/decoding time to be negligible.

The interface provided to users is the same as the regular ScriptContext, the difference is that they have to import i.e. PlutusLedgerAPI.Data.V1 instead of PlutusLedgerAPI.V1 to use the data version. The modules which don't have anything to do with maps are shared.
Note: this might have introduced some unnecessary duplication in the code, but it doesn't seem trivial to fix it without messing up the interface. For this first version, I think this is fine but let me know if there's something we can do to improve things!

Pre-submit checklist:

Branch
- Tests are provided (if possible)
- Commit sequence broadly makes sense
- Key commits have useful messages
- Changelog fragments have been written (if appropriate)
- Relevant tickets are mentioned in commit messages
- Formatting, PNG optimization, etc. are updated
PR
- (For external contributions) Corresponding issue exists and is linked in the description
- Targeting master unless this is a cherry-pick backport
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested

ana-pantilie

Consistently better performance results for the Data-backed version on the plutus-benchmark/script-contexts tests!

ana-pantilie · 2024-07-16T09:33:25Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContext1-20.budget.golden

@@ -0,0 +1,2 @@
+({cpu: 215441185
+| mem: 798229})


Non-data is:

({cpu: 291862695 | mem: 1092729})

26% less cpu, 27% less of mem!

ana-pantilie · 2024-07-16T09:34:18Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContext1-20.eval.golden

@@ -0,0 +1 @@
+(constr 0)


This is correct.

TODO: write tests which check that the SOP and the Data versions eval to the same result

It would be ideal to simply share golden files for evaluation tests of the two flavors of ScriptContext.

ana-pantilie · 2024-07-16T09:34:40Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContext1-4.budget.golden

@@ -0,0 +1,2 @@
+({cpu: 68663201
+| mem: 260597})


Non-data version:

({cpu: 87564279 | mem: 338905})

ana-pantilie · 2024-07-16T09:34:52Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContext1-4.eval.golden

@@ -0,0 +1 @@
+(constr 0)


ana-pantilie · 2024-07-16T09:35:15Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContext1.size.golden

@@ -0,0 +1 @@
+2829


Non-data version: 3183

ana-pantilie · 2024-07-16T09:36:41Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContext2.size.golden

@@ -0,0 +1 @@
+2765


Non-data version: 3119

ana-pantilie · 2024-07-16T09:37:00Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContextEqualityData-20.budget.golden

@@ -0,0 +1,2 @@
+({cpu: 51650332
+| mem: 118202})


Non-data version:

({cpu: 67298332 | mem: 216002})

ana-pantilie · 2024-07-16T09:37:14Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContextEqualityData-20.eval.golden

@@ -0,0 +1 @@
+(constr 0)


ana-pantilie · 2024-07-16T09:37:29Z

...-benchmark/script-contexts/test/Data/9.6/checkScriptContextEqualityOverhead-20.budget.golden

@@ -0,0 +1,2 @@
+({cpu: 18656100
+| mem: 116700})


Non-data version:

({cpu: 34304100 | mem: 214500})

ana-pantilie · 2024-07-16T09:37:37Z

...us-benchmark/script-contexts/test/Data/9.6/checkScriptContextEqualityOverhead-20.eval.golden

@@ -0,0 +1 @@
+(constr 0)


Unisay · 2024-07-16T11:13:17Z

plutus-benchmark/script-contexts/src/PlutusBenchmark/Data/ScriptContexts.hs

+  -- Just using a bang pattern was not enough to stop GHC from getting
+  -- rid of the dead binding before we even hit the plugin, this works
+  -- for now!


Did you consider deriving NFData instances and using deepseq to deeply evaluate the script context?

This is basically all copied over from PlutusBenchmark.ScriptContexts, and most of the documentation in the other modules is as well copied over from the non-data versions. Which reminds me that I need properly document things, but once I refactor and remove the duplication.

zliu41 · 2024-07-26T00:18:37Z

plutus-ledger-api/src/PlutusLedgerApi/Data/V1.hs

+ready to be evaluated on-chain.
+Called inside phase-1 validation (i.e., deserialisation error is a phase-1 error).
+-}
+deserialiseScript ::


I don't see why we need to redefine this. Can we not re-export the existing one?

I think I'd prefer for this module not to depend on the other one as that'll save us pretty little, but will make things quite more confusing.

Code duplication is quite unfortunate though, we'll now have to maintain twice more API modules and those are maintenance-hungry. Maybe we should eventually make the whole API type-class-based? Ledger-style.

Duplicating a function that uses map would be one thing, but duplicating something that doesn't use map at all makes little sense. Consider making PlutusLedgerApi.V1 agnostic of the choice of map/list, and making PlutusLedgerApi.V1.Term and PlutusLedgerApi.V1.Data for anything specific to a particular map/list.

zliu41 · 2024-07-26T00:23:20Z

plutus-ledger-api/src/PlutusLedgerApi/V2/Data/Contexts.hs

+
+import GHC.Generics (Generic)
+import PlutusTx
+import PlutusTx.AssocMap hiding (filter, mapMaybe)


Why is it still using AssocMap?

zliu41 · 2024-07-26T00:24:11Z

plutus-ledger-api/src/PlutusLedgerApi/V1/Data/Contexts.hs

+    _ == _                              = False
+
+-- | A pending transaction. This is the view as seen by validator scripts, so some details are stripped out.
+data TxInfo = TxInfo


This looks the same as the existing TxInfo, so why redefine it?

Not sure if it's the same, but it's definitely not gonna be, right? Because the lists will become Data-based as well. @ana-pantilie is that right?

It uses different Map and Value types, so it's not the same TxInfo.

effectfully

I really do hope we'll eventually get to cleaning it all up instead of leaving this much code duplication lying around.

effectfully · 2024-07-29T08:04:58Z

plutus-benchmark/script-contexts/test/Data/9.6/checkScriptContext1-20.eval.golden

@@ -0,0 +1 @@
+(constr 0)


It would be ideal to simply share golden files for evaluation tests of the two flavors of ScriptContext.

effectfully · 2024-07-29T08:09:57Z

plutus-ledger-api/changelog.d/20240716_124825_ana.pantilie95_data_value_ledger_api.md

@@ -0,0 +1,3 @@
+### Added
+
+- A first version of a data-backed `ScriptContext`. The ledger API is similar to the regular `ScriptContext`; one should import the `Data` versions of modules instead to use this version.


Please provide one example here.

effectfully · 2024-07-29T08:12:21Z

plutus-ledger-api/src/PlutusLedgerApi/Data/V1.hs

+ready to be evaluated on-chain.
+Called inside phase-1 validation (i.e., deserialisation error is a phase-1 error).
+-}
+deserialiseScript ::


I think I'd prefer for this module not to depend on the other one as that'll save us pretty little, but will make things quite more confusing.

Code duplication is quite unfortunate though, we'll now have to maintain twice more API modules and those are maintenance-hungry. Maybe we should eventually make the whole API type-class-based? Ledger-style.

effectfully · 2024-07-29T08:17:42Z

plutus-ledger-api/src/PlutusLedgerApi/V1/Data/Contexts.hs

+    _ == _                              = False
+
+-- | A pending transaction. This is the view as seen by validator scripts, so some details are stripped out.
+data TxInfo = TxInfo


Not sure if it's the same, but it's definitely not gonna be, right? Because the lists will become Data-based as well. @ana-pantilie is that right?

effectfully · 2024-07-29T10:10:56Z

plutus-ledger-api/src/PlutusLedgerApi/V3/Data/Contexts.hs

+{-# INLINEABLE findContinuingOutputs #-}
+
+{- | Find the indices of all the outputs that pay to the same script address we are
+currently spending from, if any.
+-}


ormolu idiocy at your service.

effectfully · 2024-07-29T10:17:10Z

plutus-tx/src/PlutusTx/Data/AssocMap.hs

+  , P.UnsafeFromData k
+  , P.UnsafeFromData a
+  ) => Pretty (Map k a) where
+  pretty = pretty . toList


Why is this needed now?

I believe it's for the tests.

ana-pantilie · 2024-08-08T19:55:02Z

Opened #6396 for addressing the code duplication concerns.

zliu41

This looks mergeable. I think we'd need a data-encoded list as well, since using regular lists in a data-encoded ScriptContext could be very bad for efficiency.

zliu41 · 2024-08-08T23:15:38Z

plutus-ledger-api/src/PlutusLedgerApi/Data/V1.hs

+ready to be evaluated on-chain.
+Called inside phase-1 validation (i.e., deserialisation error is a phase-1 error).
+-}
+deserialiseScript ::


Duplicating a function that uses map would be one thing, but duplicating something that doesn't use map at all makes little sense. Consider making PlutusLedgerApi.V1 agnostic of the choice of map/list, and making PlutusLedgerApi.V1.Term and PlutusLedgerApi.V1.Data for anything specific to a particular map/list.

* Add an initial data-backed ScriptContext and ledger API

ana-pantilie mentioned this pull request Jun 18, 2024

[EXPERIMENT] Marlowe with Data-backed Value and AssocMap #6221

Closed

11 tasks

zeme-wana force-pushed the master branch 2 times, most recently from a161078 to db5cabb Compare July 9, 2024 09:24

ana-pantilie commented Jul 16, 2024

View reviewed changes

ana-pantilie marked this pull request as ready for review July 16, 2024 09:52

Unisay reviewed Jul 16, 2024

View reviewed changes

ana-pantilie requested a review from a team July 24, 2024 13:12

zliu41 reviewed Jul 26, 2024

View reviewed changes

effectfully approved these changes Jul 29, 2024

View reviewed changes

zliu41 approved these changes Aug 8, 2024

View reviewed changes

ana-pantilie force-pushed the ana/data-value-ledger-api branch from 8913c56 to b98efe8 Compare August 9, 2024 13:17

Add a data-backed ScriptContext and ledger API

4e46945

ana-pantilie force-pushed the ana/data-value-ledger-api branch from b98efe8 to 4e46945 Compare August 9, 2024 13:30

Fix

7d8a3c9

ana-pantilie merged commit c32b03a into master Aug 9, 2024
6 checks passed

ana-pantilie deleted the ana/data-value-ledger-api branch August 9, 2024 14:37

kwxm mentioned this pull request Aug 12, 2024

ripemd_160builtin #6378

Merged

v0d1ch pushed a commit to v0d1ch/plutus that referenced this pull request Dec 6, 2024

Add an initial data-backed ScriptContext (IntersectMBO#6171)

3336cf9

* Add an initial data-backed ScriptContext and ledger API

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a data-backed `ScriptContext` #6171

Add a data-backed `ScriptContext` #6171

ana-pantilie commented Jun 4, 2024 •

edited

Loading

ana-pantilie left a comment

ana-pantilie Jul 16, 2024

Unisay Jul 16, 2024

ana-pantilie Jul 16, 2024

effectfully Jul 29, 2024

ana-pantilie Jul 16, 2024

ana-pantilie Jul 16, 2024

ana-pantilie Jul 16, 2024

ana-pantilie Jul 16, 2024

ana-pantilie Jul 16, 2024

ana-pantilie Jul 16, 2024

ana-pantilie Jul 16, 2024

ana-pantilie Jul 16, 2024

Unisay Jul 16, 2024

ana-pantilie Jul 16, 2024

zliu41 Jul 26, 2024

effectfully Jul 29, 2024

zliu41 Aug 8, 2024

zliu41 Jul 26, 2024

zliu41 Jul 26, 2024

effectfully Jul 29, 2024

ana-pantilie Aug 8, 2024

effectfully left a comment

effectfully Jul 29, 2024

effectfully Jul 29, 2024

effectfully Jul 29, 2024

effectfully Jul 29, 2024

effectfully Jul 29, 2024

effectfully Jul 29, 2024

ana-pantilie Aug 8, 2024

ana-pantilie commented Aug 8, 2024

zliu41 left a comment

zliu41 Aug 8, 2024

		@@ -0,0 +1,3 @@
		### Added

		- A first version of a data-backed `ScriptContext`. The ledger API is similar to the regular `ScriptContext`; one should import the `Data` versions of modules instead to use this version.

		@@ -0,0 +1 @@
		(constr 0)

		@@ -0,0 +1 @@
		(constr 0)

		@@ -0,0 +1 @@
		2829

		@@ -0,0 +1 @@
		2765

Add a data-backed ScriptContext #6171

Add a data-backed ScriptContext #6171

Conversation

ana-pantilie commented Jun 4, 2024 • edited Loading

ana-pantilie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

effectfully left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ana-pantilie commented Aug 8, 2024

zliu41 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add a data-backed `ScriptContext` #6171

Add a data-backed `ScriptContext` #6171

ana-pantilie commented Jun 4, 2024 •

edited

Loading