Skip to content

add MSM builtin #7074

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.

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

perturbing
Copy link
Contributor

@perturbing perturbing commented May 7, 2025

Implement the bls12_381_{G1/G2}_multiScalarMul builtin for Plutus-Core and Plutus-Tx.

The current implementation depends on a SRP of a PR against cardano-base (where the C bindings are added for the underlying functionality).

@perturbing perturbing changed the title add multiScalarMul builtin to G1 and G1 in plutus-core add MSM builtin May 7, 2025
@perturbing perturbing marked this pull request as ready for review May 7, 2025 09:30
@perturbing perturbing marked this pull request as draft May 7, 2025 09:30
@perturbing perturbing marked this pull request as ready for review May 7, 2025 09:31
Copy link
Contributor

@kwxm kwxm left a comment

Choose a reason for hiding this comment

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

This looks basically OK, but I think we should change the inteface to elimitate pairs in PLC. You could also add changelog entries in plutus-core and plutus-tx saying that you've added preliminary versions of the MSM functions.

I think it's probably OK to merge this with the SRP in cabal.project, but your changes will definitely have to be merged with cardano-base before, eg, a node can be released that uses a Plutus release involving the SRP. @zliu41: is that reasonable?

@perturbing
Copy link
Contributor Author

@kwxm, the manual on adding a built-in clearly states

PLEASE ADD ALL NEW CODE AFTER THE CODE FOR EXISTING BUILTINS SO THAT CONSTRUCTORS AND FUNCTION CLAUSES APPEAR IN THE HISTORICAL ORDER IN WHICH THEY WERE IMPLEMENTED.

Should I also follow this for plutus-tx and plutus-tx-plugin etc?

@kwxm
Copy link
Contributor

kwxm commented May 14, 2025

@kwxm, the manual on adding a built-in clearly states

PLEASE ADD ALL NEW CODE AFTER THE CODE FOR EXISTING BUILTINS SO THAT CONSTRUCTORS AND FUNCTION CLAUSES APPEAR IN THE HISTORICAL ORDER IN WHICH THEY WERE IMPLEMENTED.

Should I also follow this for plutus-tx and plutus-tx-plugin etc?

Actually I don't know. The thing about adding stuff at the end was mostly for plutus-core. It looks as if we've mostly been doing the same thing in plutus-tx, but now I'm wondering if the Haddock will be confusing if we have new functions that are separate from the previous ones. I think we have some generated Haddock somewhere so I'll try to find that and see what it looks like.

@kwxm
Copy link
Contributor

kwxm commented May 14, 2025

The Haddock for PlutusTx.Builtins is here and that does have related functions grouped together, so in plutus-tx and plutus-tx-plugin you should probably put the new functions in the same place as the existing ones.

Edit: no, wait. It looks as if the Haddock generator is grouping things together itself. I'm not sure how it does that. Anyway, just stick the new BLS functions in the same place as the old ones.

@perturbing
Copy link
Contributor Author

perturbing commented May 15, 2025

@kwxm, for now, this PR only adds to plutus-tx and plutus-core, for which both test suites

  • cabal test test:plutus-core-test
  • cabal test test:plutus-tx-test

pass (I updated the interface). That said, this PR does break other packages (plutus-tx-plugin for example), what is generally the approach here? This steps suggest that this now can merge, but this will break main?

The new builtin should now automatically become available in Plutus Core. At this point it is safe to merge the code, although the new builtin will be prohibitively expensive for real-world usage.

@perturbing perturbing requested a review from kwxm May 15, 2025 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants