chore: Add balance check before inserting MsgWrappedCreateValidator into the epoch message queue #318
Conversation
| @@ -5,6 +5,8 @@ import ( | |||
| sdk "github.com/cosmos/cosmos-sdk/types" | |||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | |||
| stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" | |||
|
|
|||
| "github.com/babylonchain/babylon/x/epoching/types" | |||
| ) | |||
|
|
|||
| // CheckMsgCreateValidator performs stateless checks on a given `MsgCreateValidator` message | |||
There was a problem hiding this comment.
| // CheckMsgCreateValidator performs stateless checks on a given `MsgCreateValidator` message | |
| // CheckMsgCreateValidator performs checks on a given `MsgCreateValidator` message |
Now this function does not only perform stateless checks anymore
| return err | ||
| } | ||
|
|
||
| balance := k.bk.GetBalance(ctx, delegatorAddr, msg.Value.GetDenom()) | ||
| if msg.Value.IsGTE(balance) { | ||
| return types.ErrInsufficientBalance |
There was a problem hiding this comment.
Is there any error that we can reuse from x/bank module?
There was a problem hiding this comment.
No. I suppose it's up to the user of the bank module to determine whether the balance is sufficient or not
| @@ -346,7 +350,7 @@ func NewBabylonApp( | |||
|
|
|||
| // NOTE: the epoching module has to be set before the chekpointing module, as the checkpointing module will have access to the epoching module | |||
| epochingKeeper := epochingkeeper.NewKeeper( | |||
| appCodec, keys[epochingtypes.StoreKey], keys[epochingtypes.StoreKey], app.GetSubspace(epochingtypes.ModuleName), &app.StakingKeeper, | |||
| appCodec, keys[epochingtypes.StoreKey], keys[epochingtypes.StoreKey], app.GetSubspace(epochingtypes.ModuleName), app.BankKeeper, &app.StakingKeeper, | |||
There was a problem hiding this comment.
Should this be
| appCodec, keys[epochingtypes.StoreKey], keys[epochingtypes.StoreKey], app.GetSubspace(epochingtypes.ModuleName), app.BankKeeper, &app.StakingKeeper, | |
| appCodec, keys[epochingtypes.StoreKey], keys[epochingtypes.StoreKey], app.GetSubspace(epochingtypes.ModuleName), &app.BankKeeper, &app.StakingKeeper, |
?
There was a problem hiding this comment.
I was wondering the same thing that why the staking keeper is passed as a pointer but not the bank keeper, but it turned out it should be as it is in the PR...
| @@ -97,6 +98,43 @@ func FuzzWrappedCreateValidator_InsufficientTokens(f *testing.F) { | |||
| }) | |||
| } | |||
|
|
|||
| // FuzzWrappedCreateValidator_InsufficientBalance tests adding a new validator | |||
| // but the delegator has insufficient balance to perform delegating | |||
| func FuzzWrappedCreateValidator_InsufficientBalance(f *testing.F) { | |||
There was a problem hiding this comment.
Another thing we can do is to move tests on MsgCreateValidator from Cosmos SDK to here. Wdyt?
There was a problem hiding this comment.
I definitely agree but I couldn't find any tests in cosmos-sdk that do stateful tests like us here
This PR fixed #317 and added relevant fuzz tests. To support the check of delegator balance, this PR added the
BankKeeperto the epoching module