[Feature Request] Remove the `MUL` multiplier from `aptos-framework.rs`

[alinush]

🚀 Feature Request

Motivation

It is easy to forget to add the * MUL at the end of every gas cost in aptos-move/aptos-gas/src/aptos-framework.rs. This could lead to bugs.

Pitch

Describe the solution you'd like

Gas costs should be declared without the MUL multiplier. The MUL multiplier can be applied at a later stage, when the gas costs are passed down, ideally.

Describe alternatives you've considered

None.

Are you willing to open a pull request?

Maybe in 2 weeks.

Additional context

See internal Slack discussion in Move channel on March 16th by started @zjma.

[davidiw]

Good call. We planned to remove the MUL, I think it was overlooked.

@vgao1996, shall we push a PR that does just that?

[vgao1996]

Yes

[alinush]

So... this might mean we need to re-gas Ristretto255. All the costs there might be wrong because they were computed without accounting for the missing MUL multiplier.

I will take care of this at some point.

Actually, it's fine. When I computed Ristretto255 costs, I used the base=3000 and per_byte=50 costs of sha2_256 which had the multiplier MUL later applied to them.

[alinush]

@vgao1996 should we wait for you to fix all the MUL stuff? Or should we patch the gas costs that don't have the MUL multiplier now?

[vgao1996]

I'm fine with either. If you choose to patch part of it now, make sure to leave some comments so someone doesn't accidentally add back the MUL.

[alinush]

I mean we'd only be fixing the missing *MUL's by adding *MUL. I am not saying we'll remove MUL from everywhere & replace all gas constants c by another constant c' equal to c * MUL.

We'd leave that fix up to you, since you know best how to take the gas costs from aptos_framework.rs and apply the *MUL multiplier to them later on.

[vgao1996]

Wait I'm confused. If you have noticed any cost that should be equal to c * MUL but currently doesn't have * MUL attached, then you have to fix it by adding the * MUL, right?

The general refactoring I've been mentioning won't change any of the actual costs.

[alinush]

Yes, right!

The general refactoring I had in mind would involve replacing every cost formatted as c * MUL in aptos_framework.rs by c and then accounting for the MUL multiplication somewhere else in the code.

Does that make sense / is it possible?

[vgao1996]

We may be able to do it by adjusting the gas scaling factor.

I was actually thinking of an even (conceptually) simpler change: replace all c * MUL by the resulting product, for example, 100 * MUL by 2000.

[alinush]

I was actually thinking of an even (conceptually) simpler change: replace all c * MUL by the resulting product, for example, 100 * MUL by 2000.

Sure, but that would only make sense if MUL was never needed in the first place, no?

I assumed it was there cause we might wanna play with it/change it?

[vgao1996]

I assumed it was there cause we might wanna play with it/change it?

It was added right before the October launch as a way to quickly experiment with different values. Since our gas prices have stabilized to some extent I'd say there's not much value in keeping them.

[github-actions[bot]]

This issue is stale because it has been open 45 days with no activity. Remove the stale label or comment - otherwise this will be closed in 15 days.

[github-actions[bot]]

This issue is stale because it has been open 45 days with no activity. Remove the stale label or comment - otherwise this will be closed in 15 days.