feat(incentives): added fees for adding to gauge and gauge creation #1188
Conversation
| if !incentivestypes.CreateGaugeFee.Equal(expectCreateGaugeFee) || !incentivestypes.AddToGaugeFee.Equal(expectAddToGaugeFee) { | ||
| return fmt.Errorf("incentives parameters not set correctly") | ||
| } | ||
| //if !incentivestypes.CreateGaugeFee.Equal(expectCreateGaugeFee) || !incentivestypes.AddToGaugeFee.Equal(expectAddToGaugeFee) { |
There was a problem hiding this comment.
not a fan of commented code! :D
There was a problem hiding this comment.
it's an old state migration (v3 while current is v4), and i didn't want to modify old migration scripts, so i intentionally left this code commented here to have a historical reference. added an explanation comment for that.
app/upgrades/v4/upgrade.go
Outdated
| params := ik.GetParams(ctx) | ||
| defaultParams := incentivestypes.DefaultParams() | ||
| params.CreateGaugeFee = defaultParams.CreateGaugeFee | ||
| params.AddToGaugeFee = defaultParams.AddToGaugeFee |
There was a problem hiding this comment.
Dont we include the defaults for base_gas_fee_for_create_gauge and base_gas_fee_for_add_reward_to_gauge ?
There was a problem hiding this comment.
fixed it, thanks for the catch!
|
I only saw that issue with the DefaultParams |
|
@keruch can u elaborte why we need both explicit creation/adding isn't the goal is to charge the user for action? why we need both types of fee charge? |
re gas first. as you know, there are rewards in each gauge. when we send tokens to lockup holders, we iterate through all denoms for every gauge, so adding more than one denom while creating/adding to gauge is a big overhead. eg if the gauge has Osmo + Dym + Atom rewards, then the num of iterations is 3x implying O(denoms * lockups). that's why we decided to consume gas depending on the number of denoms added. this is actually a solution from osmosis: osmosis-labs/osmosis#4830. fee is a one-time charge independent of the number of denom. just a maker fee. it was added before, we just decided to make it a module parameter and set the default value for adding rewards to 1 DYM (prev was 0). |
well, it doesn't have to be. not sure why osmosis did it this way, it also harder to manage (e.g if I want the fee to be burned rather than distributed to validators) |
| } | ||
|
|
||
| // Charge fess based on the number of coins to add | ||
| // Fee = AddToGaugeBaseFee + AddDenomFee * (NumAddedDenoms + NumGaugeDenoms) |
There was a problem hiding this comment.
Why do we need to pay for existing denoms as well? Isn't it already paid for when added initially?
There was a problem hiding this comment.
to prevent spam where somebody keeps excessively adding funds to some already overloaded gauge
There was a problem hiding this comment.
so this fee makes it pricey to add more and more new denoms to the gauge
Description
This PR adds extra
gasfees for gauge depositing and gauge creation.Closes #1171
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
PR review checkboxes:
I have...
Unreleasedsection inCHANGELOG.mdgodoccommentsSDK Checklist
maptime.Now()sendCoinand notSendCoinsFull security checklist here
For Reviewer:
After reviewer approval: