Add a data-backed list to ScriptContext #6597

ana-pantilie · 2024-10-25T08:49:26Z

Fixes https://github.com/IntersectMBO/plutus-private/issues/60
Fixes https://github.com/IntersectMBO/plutus-private/issues/61

I'm going to migrate both this and AssocMap to Quickcheck in a separate issue.

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

effectfully · 2024-10-31T20:57:56Z

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

+        B.caseList'
+            ys
+            (\h t -> BI.mkCons h (go t ys))
+            xs


You don't need to pass ys around just to return it untouched in the end, right?

effectfully · 2024-10-31T21:00:05Z

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

+
+{-# INLINEABLE fromSOP #-}
+fromSOP :: (ToData a) => [a] -> List a
+fromSOP = List . BI.unsafeDataAsList . B.mkList . fmap toBuiltinData


Why not explicit recursion to make it a single-pass traversal?

effectfully · 2024-10-31T21:01:21Z

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

+-- TODO: return [] or List?
+{-# INLINEABLE findIndices #-}
+findIndices :: (UnsafeFromData a) => (a -> Bool) -> List a -> [Integer]
+findIndices pred (List l) = go 0 l


Nit: not a fan of stealing a Haskell Prelude name (pred).

effectfully · 2024-10-31T21:06:04Z

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

+            []
+            (\h t ->
+                let h' = unsafeFromBuiltinData h
+                    indices = go (B.addInteger i 1) t


Please use addInteger 1 t. Search for "Yoda conditions" on Slack for why that makes any sense at all.

effectfully · 2024-10-31T21:09:43Z

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

+
+-- TODO: return [] or List?
+{-# INLINEABLE findIndices #-}
+findIndices :: (UnsafeFromData a) => (a -> Bool) -> List a -> [Integer]


Good question... Probably List. What's even the point of returning the SOP version if it has the same performance?

effectfully · 2024-10-31T21:38:10Z

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

+
+-- TODO: return [] or List?
+{-# INLINEABLE mapMaybe #-}
+mapMaybe :: (UnsafeFromData a) => (a -> Maybe b) -> List a -> [b]


The

mapMaybe :: (Data -> Maybe Data) -> List a -> List b

version might be useful.

effectfully · 2024-10-31T21:41:00Z

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

+    go =
+        B.caseList'
+            id
+            (\_ t -> B.addInteger 1 . go t)


I wouldn't be surprised if that actually performs worse than the eta-expanded version.

effectfully · 2024-10-31T21:47:29Z

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

+({cpu: 4224100
+| mem: 26500})


Impressive.

effectfully · 2024-10-31T21:47:35Z

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

Impressive too.

effectfully · 2024-10-31T21:53:39Z

plutus-tx-plugin/test/List/Spec.hs

+  num <- forAll $ Gen.integral rangeElem
+  let list = semanticsToList listS
+      dataList = semanticsToDataList listS
+      expected = mapS (+ num) listS


With QuickCheck you can generate actual functions. hedgehog is probably also capable of that.

zliu41

I think Data-backed map's toList and fromList should now use Data-backed list.

zliu41 · 2024-10-31T23:14:19Z

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

+
+-- TODO: return [] or List?
+{-# INLINEABLE filter #-}
+filter :: (UnsafeFromData a) => (a -> Bool) -> List a -> [a]


Converting elements to/from Data is supposed to be extremely cheap, since the a in List a is supposed to be Data-encoded, otherwise it makes little sense to use List a.

I think filter should return List a. Not against adding a filter' that returns SOP. But then, the [a] -> List a version can also be useful. Adding all of these seems excessive.

zliu41 · 2024-10-31T23:17:18Z

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

+append (List l) (List l') = List (go l l')
+  where
+    go xs ys =
+        B.caseList'


This will all need to be updated since caseList' isn't usable yet. @effectfully a reminder to back out the change.

@effectfully a reminder to back out the change.

Yes, sorry, I remember about it, I just can't stop discussing and reviewing things.

This code won't need to be updated though, I'll just make caseList' and others chooseList-based.

zliu41 · 2024-10-31T23:21:07Z

plutus-tx-plugin/test/List/Spec.hs

+fromSOPProgram = $$(compile [|| Data.List.fromSOP ||])
+
+toSOPSpec :: Property
+toSOPSpec = property $ do


Seems a lot of repetition in those xyzSpecs that can be factored out.

ana-pantilie temporarily deployed to github-pages October 25, 2024 08:49 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 25, 2024 09:59 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 25, 2024 10:14 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 25, 2024 13:36 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 25, 2024 14:00 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 31, 2024 08:39 — with GitHub Actions Inactive

ana-pantilie force-pushed the ana/data-backed-list branch from 9d3d29d to a55e99d Compare October 31, 2024 11:51

ana-pantilie temporarily deployed to github-pages October 31, 2024 11:51 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 31, 2024 19:28 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 31, 2024 19:30 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 31, 2024 19:39 — with GitHub Actions Inactive

ana-pantilie temporarily deployed to github-pages October 31, 2024 19:42 — with GitHub Actions Inactive

ana-pantilie marked this pull request as ready for review October 31, 2024 19:42

ana-pantilie requested a review from a team October 31, 2024 19:42

effectfully approved these changes Oct 31, 2024

View reviewed changes

zliu41 approved these changes Oct 31, 2024

View reviewed changes

ana-pantilie added 9 commits November 11, 2024 14:16

Introduce Data List API

a96f7f8

Implement first version of API

d87dbfe

Rearrange things to make TH happy

9b067c2

Compile

f831f7a

Temp

e749d3b

Add tests

b4ed825

Add doc

953943f

Docs and cleanup

9b0fff1

Add changelog

6b9a0d5

ana-pantilie force-pushed the ana/data-backed-list branch from 71f6852 to 6b9a0d5 Compare November 11, 2024 12:18

ana-pantilie temporarily deployed to github-pages November 11, 2024 12:18 — with GitHub Actions Inactive

Regenerate golden files

f669ccc

ana-pantilie temporarily deployed to github-pages November 11, 2024 15:09 — with GitHub Actions Inactive

Fix build

7ffba7d

ana-pantilie temporarily deployed to github-pages November 11, 2024 15:10 — with GitHub Actions Inactive

ana-pantilie added 3 commits November 11, 2024 17:29

Address some review comments

fb096ec

Return List instead of []

c06705d

Fix tests

e497865

ana-pantilie temporarily deployed to github-pages November 11, 2024 15:48 — with GitHub Actions Inactive

ana-pantilie enabled auto-merge (squash) November 11, 2024 15:50

ana-pantilie merged commit b526c79 into master Nov 11, 2024
8 checks passed

ana-pantilie deleted the ana/data-backed-list branch November 11, 2024 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a data-backed list to ScriptContext #6597

Add a data-backed list to ScriptContext #6597

ana-pantilie commented Oct 25, 2024 •

edited

Loading

effectfully Oct 31, 2024

effectfully Oct 31, 2024

effectfully Oct 31, 2024

effectfully Oct 31, 2024

effectfully Oct 31, 2024

effectfully Oct 31, 2024 •

edited

Loading

effectfully Oct 31, 2024

effectfully Oct 31, 2024

effectfully Oct 31, 2024

effectfully Oct 31, 2024

zliu41 left a comment

zliu41 Oct 31, 2024

zliu41 Oct 31, 2024

effectfully Nov 1, 2024

effectfully Nov 1, 2024

zliu41 Oct 31, 2024

Add a data-backed list to ScriptContext #6597

Add a data-backed list to ScriptContext #6597

Conversation

ana-pantilie commented Oct 25, 2024 • edited Loading

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 Oct 31, 2024 • edited Loading

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

zliu41 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

ana-pantilie commented Oct 25, 2024 •

edited

Loading

effectfully Oct 31, 2024 •

edited

Loading