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.

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

Kwxm/bls12 381/minor fixes #5367

Merged
merged 3 commits into from
Jun 2, 2023
Merged

Kwxm/bls12 381/minor fixes #5367

merged 3 commits into from
Jun 2, 2023

Conversation

kwxm
Copy link
Contributor

@kwxm kwxm commented Jun 1, 2023

This is just responding to a couple of small things raised in the PR comments; the main branch got merged while I was fixing these.

@kwxm kwxm added the No Changelog Required Add this to skip the Changelog Check label Jun 1, 2023
@@ -274,7 +276,7 @@ groth16Verify
:: BuiltinByteString -- G1
-> BuiltinByteString -- G2
-> BuiltinByteString -- G2
-> BuiltinByteString -- G1
-> BuiltinByteString -- G2
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like it might be worth having some newtypes here idk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was a bit confused when I looked at that today. These types are wrapped in newtypes elsewhere in this file, but not here. I think I did that because what we're really wanting to measure here is what happens on the chain, and having newtypes here would cost a little more. I think we get delay or something in the uplc if we do that, which I was slightly surprised by when I found out. I could be wrong about that though: I'll check.

instance ExMemoryUsage BLS12_381.Pairing.MlResult where
memoryUsage _ = singletonRose . unsafeToSatInt $ BLS12_381.Pairing.mlResultMemSizeBytes `div` 8
memoryUsage _ = mlResultElementCost
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The stuff above is in response to this comment from @effectfully .

@kwxm kwxm merged commit d2390b8 into master Jun 2, 2023
@kwxm kwxm deleted the kwxm/BLS12-381/minor-fixes branch June 2, 2023 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Changelog Required Add this to skip the Changelog Check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants