CIP-0381 | Adapt to Plutus bindings #506

iquerejeta · 2023-04-17T10:45:28Z

The cardano base branch has been merged, and the PR for plutus bindings is open IntersectMBO/plutus#5231. Some of the initial notation has changed. This PR adapts the CIP to the new notation.

rphair · 2023-04-17T15:44:28Z

thanks @iquerejeta - I've removed [WIP] from title because people will recognise that from the current Draft review mode.

rphair · 2023-04-17T17:34:33Z

also @iquerejeta (cc @michaelpj @dcoutts) do you think the scope of this update PR could be expanded a bit to include the suggestion in #220 (comment)?

iquerejeta · 2023-04-18T07:13:27Z

I thought about adding Duncan's comment in the CIP, and finally decided to simply include a reference to it. Do we want to add the full paragraph, or is a pointer sufficient here?
cc: @michaelpj

michaelpj · 2023-04-19T10:22:56Z

I'll perhaps let the editors comment, but I think if it's part of the substantive rationale for why the CIP is good, it should go in the content of the CIP.

rphair · 2023-04-19T11:05:26Z

yes, I was not being specific enough in #506 (comment) so yes I think what's stated in that text should be addressed in the text of this CIP. 🤓

rphair

@iquerejeta this was (due to its age I guess) scheduled for today's CIP meeting, but so we can have @michaelpj's review I've delayed it to CIP meeting # 69 in 2 weeks' time when we can also discuss #507.

Also it looks like your last few commits have cleaned up a few things as well as including the supporting material suggested above. So I hope it's OK that I take this out of Draft mode with the intention of reviewing online in the next 2 weeks & then again at the first CIP meeting in July when we'll have Plutus representation. cc @KtorZ

rphair · 2023-07-02T10:14:56Z

@iquerejeta this, with other Plutus items, is on the agenda for review early in the CIP editors' meeting Tuesday 04 July 16:00 UTC: https://hackmd.io/@cip-editors/69

iquerejeta · 2023-07-05T07:44:45Z

In yesterdays CIP meeting, it was requested by @michaelpj to have a review on the compression rationale by @kwxm. However, I've seen that the commit of compression was coauthored by Kenneth. Nonetheless, Kenneth, could we get an explicit approval of that rationale in order to move forward with this PR?

kwxm · 2023-07-05T09:50:53Z

I see that the link to this CIP on https://cips.cardano.org/ is broken. I think that it points to https://cips.cardano.org/cips/cip381 when it should point to https://cips.cardano.org/cips/cip0381, but I have no idea how to get it fixed.

rphair · 2023-07-05T10:55:05Z

@kwxm ... @KtorZ did some work on this in #438 which fixed some link building issues (https://github.com/cardano-foundation/CIPs/tree/master/scripts), but apparently the three significant digits is a unique case that still doesn't work properly.

We have no issue (yet) like "Fix all the things on cips.cardano.org" although I think all prior PRs and issues can be found by working backwards from that PR. As it says in #436 (comment) we have a dependent issue delaying us from overhauling the code now, but maybe @KtorZ can do another band-aid fix like we had with #438 (I am lacking some prerequisites to do this myself).

kwxm

This looks fine to me. I'm afraid I got a bit carried away spotting typos and things though. I agree that the rationale for using compression is sensible (although it was me that wrote it, so maybe I shouldn't be approving it).

CIP-0381/README.md

kwxm · 2023-07-05T10:12:41Z

CIP-0381/README.md

-    * `BLS12_381_G2_neg :: BLS12_381G2Element -> BLS12_381G2Element`
-    * `BLS12_381_H2G2 :: ByteString -> BLS12_381G2Element`
-    * `BLS12_381_GT_mul :: Blst12381GTElement -> Blst12381GTElement -> Blst12381GTElement`
+    * `bls12_381_G1_add :: bls12_381_G1_element -> bls12_381_G1_element -> bls12_381_G1_element`


The function types here are all Haskell types, which are slightly different from Plutus Core types (or at least have different names). I'm not sure which ones we actually want here.

I think we should have the names that will be used by plutus script developers. I thought we did modify this, but could you provide a pointer to the plutus names please?

If you go into the plutus repo and type cabal run uplc print-builtin-signatures it'll show you the names and types of all the built-in functions. I've attached the info for the BLS functions below. Note that it has signatures like

bls12_381_G1_add : [ bls12_381_G1_element, bls12_381_G1_element ] -> bls12_381_G1_element

The notation here is non-standard but matches what we have in the Plutus Core specification. The stuff in square brackets represents the types of the arguments, not some list type. It's like that to make the notation in our spec less ambiguous (if you've got lots of arrows it's not clear where the input types stop and the output type starts). I'm not sure if you should put types like that in the CIP though.

Here's the full list:

bls12_381_G1_add : [ bls12_381_G1_element, bls12_381_G1_element ] -> bls12_381_G1_element bls12_381_G1_neg : [ bls12_381_G1_element ] -> bls12_381_G1_element bls12_381_G1_scalarMul : [ integer, bls12_381_G1_element ] -> bls12_381_G1_element bls12_381_G1_equal : [ bls12_381_G1_element, bls12_381_G1_element ] -> bool bls12_381_G1_hashToGroup : [ bytestring, bytestring ] -> bls12_381_G1_element bls12_381_G1_compress : [ bls12_381_G1_element ] -> bytestring bls12_381_G1_uncompress : [ bytestring ] -> bls12_381_G1_element bls12_381_G2_add : [ bls12_381_G2_element, bls12_381_G2_element ] -> bls12_381_G2_element bls12_381_G2_neg : [ bls12_381_G2_element ] -> bls12_381_G2_element bls12_381_G2_scalarMul : [ integer, bls12_381_G2_element ] -> bls12_381_G2_element bls12_381_G2_equal : [ bls12_381_G2_element, bls12_381_G2_element ] -> bool bls12_381_G2_hashToGroup : [ bytestring, bytestring ] -> bls12_381_G2_element bls12_381_G2_compress : [ bls12_381_G2_element ] -> bytestring bls12_381_G2_uncompress : [ bytestring ] -> bls12_381_G2_element bls12_381_millerLoop : [ bls12_381_G1_element, bls12_381_G2_element ] -> bls12_381_mlresult bls12_381_mulMlResult : [ bls12_381_mlresult, bls12_381_mlresult ] -> bls12_381_mlresult bls12_381_finalVerify : [ bls12_381_mlresult, bls12_381_mlresult ] -> bool

CIP-0381/README.md

Co-authored-by: Kenneth MacKenzie <kenneth.mackenzie@iohk.io>

rphair · 2023-07-11T12:32:37Z

@iquerejeta this is on our next CIP meeting # 70 agenda as Last Check, probably because @kwxm's review looks well accommodated by the last round of commits... can you confirm currently unresolved #506 (comment) has been addressed? 🙏 https://hackmd.io/@cip-editors/70

rphair

@iquerejeta as far as I can tell this had been revised according to @kwxm's audit, including the costing details that were requested in earlier meetings, so I'm going to try to get one more editor endorsement at today's CIP meeting so we can merge this on this spot if possible & soon afterward if not. 🚀 cc @KtorZ @SebastienGllmt @Ryun1

Ryun1

LGTM - final last check from Plutus team was positive, good to merge 🤓.

* Adapt CIP to Plutus bindings * Add dcoutts' analysis on blst library * Include motivation for using compressed over decompressed form Co-authored-by: Kenneth MacKenzie <kenneth.mackenzie@iohk.io> * Modify hash-to-curve to include DST * Include instructions for DST > 255 bytes * Address review comments --------- Co-authored-by: Kenneth MacKenzie <kenneth.mackenzie@iohk.io>

rphair changed the title ~~[WIP] Adapt CIP to Plutus bindings~~ CIP-0381 | Adapt to Plutus bindings Apr 17, 2023

rphair added the Update Adds content or significantly reworks an existing proposal label Apr 17, 2023

iquerejeta force-pushed the master branch from 66e338f to 2f69ee9 Compare May 5, 2023 12:48

rphair reviewed Jun 20, 2023

View reviewed changes

rphair marked this pull request as ready for review June 20, 2023 08:32

rphair added the Category: Plutus Proposals belonging to the 'Plutus' category. label Jun 20, 2023

kwxm approved these changes Jul 5, 2023

View reviewed changes

iquerejeta and others added 6 commits July 5, 2023 13:37

Adapt CIP to Plutus bindings

19b661c

Add dcoutts' analysis on blst library

0f6fbd7

Include motivation for using compressed over decompressed form

38f0e96

Co-authored-by: Kenneth MacKenzie <kenneth.mackenzie@iohk.io>

Modify hash-to-curve to include DST

ddd4514

Include instructions for DST > 255 bytes

84223cf

Address review comments

97d5a1f

iquerejeta force-pushed the master branch from 21259be to 97d5a1f Compare July 5, 2023 12:55

rphair added the State: Last Check Review favourable with disputes resolved; staged for merging. label Jul 11, 2023

rphair approved these changes Jul 18, 2023

View reviewed changes

Ryun1 approved these changes Jul 18, 2023

View reviewed changes

rphair merged commit 797cdc6 into cardano-foundation:master Jul 18, 2023

rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label Sep 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CIP-0381 | Adapt to Plutus bindings #506

CIP-0381 | Adapt to Plutus bindings #506

iquerejeta commented Apr 17, 2023

rphair commented Apr 17, 2023

rphair commented Apr 17, 2023

iquerejeta commented Apr 18, 2023

michaelpj commented Apr 19, 2023

rphair commented Apr 19, 2023

rphair left a comment

rphair commented Jul 2, 2023

iquerejeta commented Jul 5, 2023

kwxm commented Jul 5, 2023

rphair commented Jul 5, 2023 •

edited

Loading

kwxm left a comment •

edited

Loading

kwxm Jul 5, 2023

iquerejeta Jul 5, 2023

kwxm Jul 5, 2023 •

edited

Loading

rphair commented Jul 11, 2023

rphair left a comment •

edited

Loading

Ryun1 left a comment

CIP-0381 | Adapt to Plutus bindings #506

CIP-0381 | Adapt to Plutus bindings #506

Conversation

iquerejeta commented Apr 17, 2023

rphair commented Apr 17, 2023

rphair commented Apr 17, 2023

iquerejeta commented Apr 18, 2023

michaelpj commented Apr 19, 2023

rphair commented Apr 19, 2023

rphair left a comment

Choose a reason for hiding this comment

rphair commented Jul 2, 2023

iquerejeta commented Jul 5, 2023

kwxm commented Jul 5, 2023

rphair commented Jul 5, 2023 • edited Loading

kwxm left a comment • edited Loading

Choose a reason for hiding this comment

kwxm Jul 5, 2023

Choose a reason for hiding this comment

iquerejeta Jul 5, 2023

Choose a reason for hiding this comment

kwxm Jul 5, 2023 • edited Loading

Choose a reason for hiding this comment

rphair commented Jul 11, 2023

rphair left a comment • edited Loading

Choose a reason for hiding this comment

Ryun1 left a comment

Choose a reason for hiding this comment

rphair commented Jul 5, 2023 •

edited

Loading

kwxm left a comment •

edited

Loading

kwxm Jul 5, 2023 •

edited

Loading

rphair left a comment •

edited

Loading