Update EIP-2537: Clarify the gast cost calculation formula for MSM

[rodiazet]

This PR fixes value of multiplication_cost for G2 which according to comment in the G1/G2 MSM section saying that we want to avoid non-integer arithmetic should be divisible by 1000 the multiplier.
Keeping this as it is now makes the result of the formula depends on the order of multiplications.

This PR clarifies gas cost formula calculation for MSM precompile.

[eth-bot]

✅ All reviewers have approved.

[jochem-brouwer]

I might be wrong here, but doesn't the formula: (k * multiplication_cost * discount) / multiplier already explicitly state that the numerator should be calculated first and then thus dividing by the multiplier? The discount numbers are not divisibile by 1000 also (except 1000). I think what is meant with "To avoid non-integer arithmetic" that the formula is not: k * multiplication_cost * discount where discount for example could be (2 multiplications): 0.949 and not 949.

[rodiazet]

yes but even if we calculate (k * multiplication_cost * discount) we cannot be sure that the result will be divisible by 1000. So we can change to make it divisible or add comment that this division must be integer division.

[chfast]

I might be wrong here, but doesn't the formula: (k * multiplication_cost * discount) / multiplier already explicitly state that the numerator should be calculated first and then thus dividing by the multiplier? The discount numbers are not divisibile by 1000 also (except 1000). I think what is meant with "To avoid non-integer arithmetic" that the formula is not: k * multiplication_cost * discount where discount for example could be (2 multiplications): 0.949 and not 949.

Not exactly. If you follow the formula strictly e.g. for G2MSM k=3 you get 62302.5. So you would need to specify that the division in the formula is integer division.

But the spec does the opposite. It says that the constants have been chosen carefully so that the division happens without fractions. However, we haven't read the spec and updated the constants so that this is now in conflict.

If we want to pick the "let's do integer division" path, I'd argue the formula should group constants like this

(multiplication_cost * discount // multiplier) * k

[jochem-brouwer]

Ah ok good point, I see now. I am rather sure the original intent was for integer division, so the formula should likely have read: (maybe an older version of the BLS gas schedule could work with or without integer division yielding the same results)

(k * multiplication_cost * discount) // multiplier

EDIT: never mind, if multiplication_cost is an integer multiple of multiplier then non-integer arithmetic is indeed avoided no matter the k or discount pairs.

Out of interest, why do you choose to group the formula like this?

(multiplication_cost * discount // multiplier) * k

(and not (k * multiplication_cost * discount) // multiplier?)

I have not tested, but does this yield different results from the first formula when applied to the relevant discount and k values?

[garyschulte]

I have not tested, but does this yield different results from the first formula when applied to the relevant discount and k values?

Seems it would, integer division would drop the last significant digit for discounts that are not multiples of 10.

[chfast]

Ah ok good point, I see now. I am rather sure the original intent was for integer division, so the formula should likely have read: (maybe an older version of the BLS gas schedule could work with or without integer division yielding the same results)

It seems to be it was the opposite, the constants in the spec were selected in a way the kind of the division don't matter.
But the original intent is not very important.

Out of interest, why do you choose to group the formula like this?

(multiplication_cost * discount // multiplier) * k

(and not (k * multiplication_cost * discount) // multiplier?)

Because multiplication_cost * discount // multiplier is a constant per k. So instead of a table of discounts you can have a table of these constants.

[chfast]

Using formula (k * multiplication_cost * discount) // multiplier introduces some "problematic" values for analysis. E.g. the cost per point of G2MSM, k=3 is 20767.333333333.

[rodiazet]

Why cannot we just have a table with exact costs for each input length instead of discount table and the formula?

[Marchhill]

I don't see that that this is a problem? As long as we all use the same formula (k * multiplication_cost * discount) // multiplier

[jochem-brouwer]

Using formula (k * multiplication_cost * discount) // multiplier introduces some "problematic" values for analysis. E.g. the cost per point of G2MSM, k=3 is 20767.333333333.

@chfast Ah great, thanks for the insight, this makes a lot of sense. For completeness / "correctness" my preference would go to (multiplication_cost * discount // multiplier) * k as well

[rodiazet]

ACD decided to stay with old formula with clarification that we are using an integer division.
https://ethereum-magicians.org/t/all-core-devs-execution-acde-203-january-16-2025/22346
This approach avoids a need to apply additional changes in clients and tests.

[jochem-brouwer]

LGTM! Love to clarification for // being integer division 👍 😄

[MarekM25]

This PR should be closed without merging

[rodiazet]

I think we agreed during ACDE to change this as in this PR.
#9245 (comment)

[Marchhill]

This PR should be closed without merging

@MarekM25 it now just clarifies to use integer division rather than changing gas costs, so I think it's good to merge

[MarekM25]

yes, my bad, good to merge indeed

[ralexstokes]

agreed to merge following ethereum/pm#1227

[eth-bot]

All Reviewers Have Approved; Performing Automatic Merge...