feat: add gov spam prevention antehandler #2251
Conversation
ghost
requested review from
alexanderbez,
zmanian,
yaruwangway,
crodriguezvega,
jackzampolin,
mmulji-ic,
glnro,
jtremback,
mpoke,
sainoe,
MSalopek and
shaspitz
as code owners
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2251 +/- ##
==========================================
- Coverage 84.76% 82.86% -1.90%
==========================================
Files 21 22 +1
Lines 1483 1535 +52
==========================================
+ Hits 1257 1272 +15
- Misses 181 212 +31
- Partials 45 51 +6
|
Co-authored-by: vuong <[email protected]>
|
I don't think we can fix the SDK simulations, as the deposit fees generation does not take in account our minimum deposit constraint. Fixing e2e however. |
ante/ante.go
Outdated
| paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" | ||
| ibcante "github.com/cosmos/ibc-go/v4/modules/core/ante" | ||
| ibckeeper "github.com/cosmos/ibc-go/v4/modules/core/keeper" | ||
|
|
||
| gaiafeeante "github.com/cosmos/gaia/v9/x/globalfee/ante" | ||
| gaiagovante "github.com/cosmos/gaia/v9/x/gov/ante" |
There was a problem hiding this comment.
This shouldn't be under x/ as it's not a module.
There was a problem hiding this comment.
you can move it to gaia/ante/
tests/e2e/e2e_setup_test.go
Outdated
| @@ -75,7 +75,7 @@ var ( | |||
| stakingAmountCoin = sdk.NewCoin(uatomDenom, stakingAmount) | |||
| tokenAmount = sdk.NewCoin(uatomDenom, sdk.NewInt(3300000000)) // 3,300uatom | |||
| standardFees = sdk.NewCoin(uatomDenom, sdk.NewInt(330000)) // 0.33uatom | |||
| depositAmount = sdk.NewCoin(uatomDenom, sdk.NewInt(10000000)) // 10uatom | |||
| depositAmount = sdk.NewCoin(uatomDenom, sdk.NewInt(3300000000)) // 3,300uatom | |||
There was a problem hiding this comment.
Could you give more context on this change and how it tests the new ante handler?
There was a problem hiding this comment.
unit tests and e2e tests need to be added:
- gov proposal with deposit less than
MiniumInitialDepositRate - gov proposal with deposit more than/equal to
MiniumInitialDepositRate - authz.msg contains gov proposal with deposit less than
MiniumInitialDepositRate - authz.msg contains gov proposal with deposit more than/equal to
MiniumInitialDepositRate
There was a problem hiding this comment.
the depositAmount = 3,300uatom, is this enough ? depositAmount need to be >= 2,000,000uatom due to
MiniumInitialDepositRate = sdk.NewDecWithPrec(10, 2), gas = 200,000
| // prevent spam gov msg | ||
| depositParams := gpsd.govKeeper.GetDepositParams(ctx) | ||
| miniumInitialDeposit := gpsd.calcMiniumInitialDeposit(depositParams.MinDeposit) | ||
| if msg.InitialDeposit.IsAllLT(miniumInitialDeposit) { |
There was a problem hiding this comment.
if deposit IsAllLT than the required deposit, fail.
may use
if deposit IsAnyGTE than the required deposit, pass. This can do a fast looping, when find one coin GTE, return nil ?
both are fine
|
@yaruwangway would you mind doing the changes? I will let the pro do it. |
ghost
closed this
Closes: #2246.
This should fix the spam issue from the Cosmos Hub.
This ports CosmosContracts/juno#394 from @vuong177 and @faddat.