-
Notifications
You must be signed in to change notification settings - Fork 482
Costing for expModInteger
#7071
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
Conversation
| , paramFindFirstSetBit = Id $ ModelOneArgumentConstantCost 1 | ||
| , paramRipemd_160 = Id $ hashMemModel Hash.ripemd_160 | ||
| , paramExpModInteger = Id $ ModelThreeArgumentsConstantCost 100000000000 -- FIXME: stub | ||
| , paramExpModInteger = Id $ ModelThreeArgumentsLinearInZ identityFunction |
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! This is the size in bytes, not words, so it'll be 8 times bigger than it should be. The size of the result will be the same as the size of the modulus, but we're using a non-standard measure here.
| data ExpModCostingFunction = ExpModCostingFunction | ||
| { coefficient00 :: Coefficient00 | ||
| , coefficient11 :: Coefficient11 | ||
| , coefficient12 :: Coefficient12 |
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.
This seemed so specific that it'd be pointless to give it a more general name. We could have made it a general cubic function in two variables, but that would have ten coefficients and we'd only be using three.
| {-# INLINE evaluateExpModCostingFunction #-} | ||
|
|
||
| -- | s * (x - y) + I | ||
| {- In principle we could use ModelConstantOrOneArgument here, but that would |
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.
Fixing this because of https://github.com/IntersectMBO/plutus-private/issues/1554
|
|
||
| {- | Calculate a 'CostingInteger' for the size of the given 'Integer', measured in | ||
| 8-bit bytes. -} | ||
| memoryUsageBytes :: Integer -> CostingInteger |
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 added this because using the standard measure would give very coarse-grained results and this is much more accurate. Now I'm worried that it's jsut adding more complexity. Maybe we should go for inaccurate but simple.
| b = sizeOf y | ||
| c = sizeOf z | ||
| in c00 + c11 * b * c + c12 * b * c * c | ||
| -- ^ THIS IS INCOMPLETE: the real costing function branches if a > 5*c; however we measure |
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 fudged this a bit to get the PR open. I'll come back and improve it a bit later.
| (Coefficient00 c00) (Coefficient11 c11) (Coefficient12 c12)) | ||
| aa ee mm = if aa <= 5*mm | ||
| then cost0 | ||
| else cost0 + (cost0 `dividedBy` 2) |
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.
ie, 1.5 times the cost. It seems a bit risky to multiply saturated integers by fractions though.
I'm a bit worried about this being hard-coded since it's not easy to adjust and it'll have to be reproduced in eg the metatheory. Maybe the penalty factor should go in the cost model parameters along with the cofficients.
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.
Looks sensible!
| evaluateExpModCostingFunction | ||
| (ExpModCostingFunction | ||
| (Coefficient00 c00) (Coefficient11 c11) (Coefficient12 c12)) | ||
| aa ee mm = if aa <= 5*mm |
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 thought it was aa <= mm - how did you arrive at 5*mm?
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 thought it was
aa <= mm- how did you arrive at5*mm?
I thought I'd be generous and allow callers to go a little bit over the limit: if you're calculating (ab)^e mod m for example, ab might be close to twice the size of m. I'm thinking that I should probably just make it aa <= mm though.
| -> BuiltinResult Natural | ||
| expModIntegerDenotation | ||
| (IntegerCostedByNumBytes a) | ||
| (IntegerCostedByNumBytes b) |
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.
b -> e for consistency?
|
I'm closing this in favour of #7080. |
Fixes https://github.com/IntersectMBO/plutus-private/issues/1448
Fixes https://github.com/IntersectMBO/plutus-private/issues/1554
This adds costing for
expModInteger. Here are the costs of the builtin with inputs of various sizes, as a percentage of the maximum execution budget. For cryptographic purposes you might go up to about 512 bits, so it doesn't look as if it's prohibitively expensive.