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

Remove x/y scaling in EIP1559 calculation #13556

Closed
dantaik opened this issue Apr 6, 2023 · 4 comments
Closed

Remove x/y scaling in EIP1559 calculation #13556

dantaik opened this issue Apr 6, 2023 · 4 comments

Comments

@dantaik
Copy link
Contributor

dantaik commented Apr 6, 2023

Describe the feature request

From Brecht:

The exp implementation has as input and output values in 10**18 fixed format notation, so exp(0) == 10**18. Let's say that's 1 wei (so plenty of precision). The exp function also works with inputs up till ~135, which will return an enormous number: log10(exp(135)) == 58. The total ETH supply is ~100,000,000 ETH, and 10*18 wei/ETH, so log10(100,000,000*(10**18)) == 26, much smaller than the max value returned by exp.

@dantaik dantaik self-assigned this Apr 6, 2023
@dantaik
Copy link
Contributor Author

dantaik commented Apr 6, 2023

If we want to ensure:

  • the price is initialPrice at a given gasExcess value
  • the price difference for a block of gasTarget*2 and a block of gasTarget is p

We end up with both x and y scaling factors, inevitably :)

@Brechtpd
Copy link
Contributor

Brechtpd commented Apr 6, 2023

I think the y scaling can be avoided (I haven't done the math, so take with a grain of salt). To target p you would pick some ADJUSTMENT_QUOTIENT (total x scaling is done with TARGET * ADJUSTMENT_QUOTIENT). Then to get gasExcess to target some initial fee, you would just do the inverse basefee calculation:

initial_basefee = 10**8
gasExcess = (TARGET * ADJUSTMENT_QUOTIENT) * math.log(initial_basefee * (TARGET * ADJUSTMENT_QUOTIENT))

@dantaik
Copy link
Contributor Author

dantaik commented May 8, 2023

I suggest that we keep it as-is to reduce risk. Removing y-scale still ends up with the same number of storage slots that we have to write (1 for now), the only small downside is that we have an extra DIV opcode to be optimized away, it's too small of a gain I think.

@Brechtpd
Copy link
Contributor

Brechtpd commented May 8, 2023

Compared to the prover fee calculation (which largely does the same thing) it's quite a bit more complex I feel which makes it trickier to understand I think. Code that could be removed for example:

uint128 _xscale;
(_xscale, yscale) = Lib1559Math.calculateScales({
xExcessMax: _param1559.gasExcessMax,
price: _param1559.basefee,
target: _param1559.gasTarget,
ratio2x1x: _param1559.ratio2x1x
});
if (_xscale == 0 || _xscale >= type(uint64).max || yscale == 0)
revert L2_INVALID_1559_PARAMS();
xscale = uint64(_xscale);

function calculateScales(
uint64 xExcessMax,
uint64 price,
uint64 target,
uint64 ratio2x1x
) internal pure returns (uint128 xscale, uint128 yscale) {
assert(xExcessMax != 0);
uint64 x = xExcessMax / 2;
// calculate xscale
xscale = LibFixedPointMath.MAX_EXP_INPUT / xExcessMax;
// calculate yscale
yscale = calculatePrice(xscale, price, x, target).toUint128();
// Verify the gas price ratio between two blocks, one has
// 2*target gas and the other one has target gas.
uint256 price1x = calculatePrice(xscale, yscale, x, target);
uint256 price2x = calculatePrice(xscale, yscale, x, target * 2);
uint64 ratio = uint64((price2x * 10000) / price1x);
if (ratio2x1x != ratio)
revert M1559_UNEXPECTED_CHANGE(ratio2x1x, ratio);
}
.

I don't think there's a significant performance gain to be made, just keeping things as simple as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants