-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add BLS tests #15
Add BLS tests #15
Conversation
8cc6dc8
to
8b386e2
Compare
e2e-tests/test/PlutusScripts/BLS.hs
Outdated
mkAggregateSigG1 :: BlsParamsWithPubKey -> sc -> Bool | ||
mkAggregateSigG1 BlsParamsWithPubKey{..} _sc = do | ||
let | ||
hashedMsgs = mapWithConstSecondArg BI.bls12_381_G2_hashToGroup blsSigBls12381G2XmdSha256SswuRoNul messageElements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blsSigBls12381G2XmdSha256SswuRoNul
Excellent variable name! (I understand why it's like that: it's a pity it's not a bit easier to convert from strings to bytestrings. I wonder if we can use pack
here to do the conversion off-chain and then lift it into the Plutus code?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do this off chain
blsSigBls12381G2XmdSha256SswuRoNul :: P.BuiltinByteString
blsSigBls12381G2XmdSha256SswuRoNul = BI.toBuiltin $ Char8.pack "BLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_NUL_"
But what's the benefit of lifting rather than simply referencing the variable as I am currently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But what's the benefit of lifting rather than simply referencing the variable as I am currently?
I forgot about this and now I'm not sure what I was talking about. I think maybe I was alluding to the fact that you need this BuiltinByteString "424c535f5349475f424c53313233383147325f584d443a5348412d3235365f535357555f524f5f4e554c5f" that's supposed to represent a DST containing the ascii string "BLS_blahblahblah" and it's kind of error-prone to have the big hex string rather than a readable string. I think I was trying to suggest that we could just write the readable string in the source and do the hard work of converting it to a bytestring off the chain (we might be able to convert it on-chain too, but that would surely be too expensive),
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait! I always get horribly confused about his, but isn't OverloadedStrings
going to make "424c535f..." into the bytestring whose bytes are the ascii codes of the characters '4', '2', '4', 'c', ...? Don't we in fact want to call some function like bytesFromHex
as in the Plutus examples? Maybe that's fixed now though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll comment now but come back and re-review some of it when I understand a bit better what's going on. One thing that strikes me is that it might be tricky to keep the code synchronised with the versions in plutus
, but there's probably no easy way to fix that.
mkG2Element :: [Word8] -> CompressedG2Element | ||
mkG2Element = CompressedG2Element . toBuiltin . BS.pack | ||
|
||
groth16Scalar :: Integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you should update this to match the refactored version that I included in the PR for plutus-benchmarks
.
e2e-tests/test/Spec/Builtins/BLS.hs
Outdated
|
||
simpleVerifyBls12381SigTestInfo = TestInfo { | ||
testName = "VerifyBls12381SigTest", | ||
testDescription = "Uses three scripts with BLS functions." ++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outdated?
e2e-tests/test/PlutusScripts/BLS.hs
Outdated
mkAggregateSigG1 :: BlsParamsWithPubKey -> sc -> Bool | ||
mkAggregateSigG1 BlsParamsWithPubKey{..} _sc = do | ||
let | ||
hashedMsgs = mapWithConstSecondArg BI.bls12_381_G2_hashToGroup blsSigBls12381G2XmdSha256SswuRoNul messageElements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But what's the benefit of lifting rather than simply referencing the variable as I am currently?
I forgot about this and now I'm not sure what I was talking about. I think maybe I was alluding to the fact that you need this BuiltinByteString "424c535f5349475f424c53313233383147325f584d443a5348412d3235365f535357555f524f5f4e554c5f" that's supposed to represent a DST containing the ascii string "BLS_blahblahblah" and it's kind of error-prone to have the big hex string rather than a readable string. I think I was trying to suggest that we could just write the readable string in the source and do the hard work of converting it to a bytestring off the chain (we might be able to convert it on-chain too, but that would surely be too expensive),
e2e-tests/test/PlutusScripts/BLS.hs
Outdated
mkAggregateSigG1 :: BlsParamsWithPubKey -> sc -> Bool | ||
mkAggregateSigG1 BlsParamsWithPubKey{..} _sc = do | ||
let | ||
hashedMsgs = mapWithConstSecondArg BI.bls12_381_G2_hashToGroup blsSigBls12381G2XmdSha256SswuRoNul messageElements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait! I always get horribly confused about his, but isn't OverloadedStrings
going to make "424c535f..." into the bytestring whose bytes are the ascii codes of the characters '4', '2', '4', 'c', ...? Don't we in fact want to call some function like bytesFromHex
as in the Plutus examples? Maybe that's fixed now though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I think I understand what's going on now. I look forward to seeing this working!
…being used by cardano-node. Fails script eval due to unsupported protocol version in plutus-ledger-api (futurePV instead of conwayPv)
… the blocking protocol version issue. All tests passing.
Will not compile until we depend on cardano-node that uses the BLS supporting plutus version (after IntersectMBO/plutus#5231 is merged)