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

Implement clawback of airdrop from inactive genesis addresses #560

Merged
merged 12 commits into from
Nov 19, 2021
22 changes: 14 additions & 8 deletions x/claim/keeper/claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ func (k Keeper) EndAirdrop(ctx sdk.Context) error {
return err
}
k.clearInitialClaimables(ctx)

k.ClawbackAirdrop(ctx)
return nil
}

Expand All @@ -51,17 +53,21 @@ func (k Keeper) ClawbackAirdrop(ctx sdk.Context) error {
if err != nil {
return err
}
seq, err := k.accountKeeper.GetSequence(ctx, addr)
if err != nil {
return err
}
if seq == 0 {
osmoBal := k.bankKeeper.GetBalance(ctx, addr, "uosmo")
ionBal := k.bankKeeper.GetBalance(ctx, addr, "uion")
err = k.distrKeeper.FundCommunityPool(ctx, sdk.NewCoins(osmoBal, ionBal), addr)

acc := k.accountKeeper.GetAccount(ctx, addr)
if acc != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to make sure I understand the prior bug. Why would it clawback from all accounts? (Or were the addresses just wrong)

Also is there any expected scenario where acc would be nil, or this is just in case something unexpected occurs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it was doing the opposite. It wasn't clawing back from any accounts.
See original testcases:
https://github.com/osmosis-labs/osmosis/blob/cf7ba91f7bd74e6b9e7d3559f5e0e0ff9da87b8b/x/claim/keeper/claim_test.go

It's cause the function was exiting early, b/c as soon as it saw an account that wasn't in the account store it would error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh that makes sense

seq, err := k.accountKeeper.GetSequence(ctx, addr)
if err != nil {
return err
}
if seq == 0 {
osmoBal := k.bankKeeper.GetBalance(ctx, addr, "uosmo")
ionBal := k.bankKeeper.GetBalance(ctx, addr, "uion")
err = k.distrKeeper.FundCommunityPool(ctx, sdk.NewCoins(osmoBal, ionBal), addr)
if err != nil {
return err
}
}
}
}
return nil
Expand Down
6 changes: 4 additions & 2 deletions x/claim/keeper/claim_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package keeper_test

import (
"fmt"
"time"

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
Expand Down Expand Up @@ -356,13 +357,13 @@ func (suite *KeeperTestSuite) TestClawbackAirdrop() {
}{
{
name: "airdrop address active",
address: "osmo1000644etcp4k4awz77kvy3pryplv8ky8py962d",
address: "osmo122fypjdzwscz998aytrrnmvavtaaarjjt6223p",
sequence: 1,
expectClawback: false,
},
{
name: "airdrop address inactive",
address: "osmo10008uvk6fj3ja05u092ya5sx6fn355wayx20rq",
address: "osmo122g3jv9que3zkxy25a2wt0wlgh68mudwptyvzw",
sequence: 0,
expectClawback: true,
},
Expand All @@ -387,6 +388,7 @@ func (suite *KeeperTestSuite) TestClawbackAirdrop() {
err = acc.SetSequence(tc.sequence)
suite.Require().NoError(err, "err: %s test: %s", err, tc.name)
suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
fmt.Println(acc)
suite.app.BankKeeper.AddCoins(suite.ctx, addr, sdk.NewCoins(
sdk.NewInt64Coin("uosmo", 100), sdk.NewInt64Coin("uion", 100),
))
Expand Down
5 changes: 3 additions & 2 deletions x/claim/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package types

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

// BankKeeper defines the banking contract that must be fulfilled when
Expand All @@ -18,7 +18,8 @@ type BankKeeper interface {
// AccountKeeper defines the expected account keeper used for simulations (noalias)
type AccountKeeper interface {
GetModuleAddress(name string) sdk.AccAddress
SetModuleAccount(ctx sdk.Context, macc types.ModuleAccountI)
SetModuleAccount(ctx sdk.Context, macc authtypes.ModuleAccountI)
GetAccount(sdk.Context, sdk.AccAddress) authtypes.AccountI
// Fetch the sequence of an account at a specified address.
GetSequence(sdk.Context, sdk.AccAddress) (uint64, error)
}
Expand Down