Fix unit fees dynamics #732
Conversation
| } | ||
| n, over := math.Add64(nextPrice, baseDelta) | ||
| rawDelta := previousUnitFee * (totalUnitsConsumed - target) / target | ||
| delta := max(rawDelta/changeDenom, 1) * changeDenom // price must change in increments on changeDenom |
There was a problem hiding this comment.
This is the main point: delta here is ensured to be a multiple of changeDenom. Moverover, from a dimensional analysis standpoint, delta is a "token amont" quantity, which can be added to unit fees.
In the previous version baseDelta is a dimensionless quantity (ration of two fees) which cannot be added to a dimensioned quantity like next price
There was a problem hiding this comment.
This is actually wrong: this code comes from coreth (see coreth code) which in turns comes from EIP1559 proposal (see for instance ethereum/EIPs#1559 for the original meaning of changeDenom and https://eips.ethereum.org/EIPS/eip-1559 for how it was evolved from there).
The goal of changeDenom is not to make sure that delta happens in multiples of the changeDenom.
abi87
force-pushed
the
fix_fee_dynamics
branch
from
96f26e1
to
bca0f15
Compare
|
I misunderstood the meaning of |
I believe there is an issue with the way we update unit fees here.
I understand unit fees should change in steps of
changeDenomfrom a given unit fee.This is not what happen in the code right now.
Probably the easier way to understand that there is an issue is to note that current code sums a dimensioned quantity (unitfee, which is Avax or denominated) with a dimensionless quantity (baseDelta).
I propose a change to fix it up.