Skip to content

Commit

Permalink
refactor(gov): remove global bech32 (#15682)
Browse files Browse the repository at this point in the history
  • Loading branch information
tac0turtle authored Apr 6, 2023
1 parent b6bc636 commit f5993b1
Show file tree
Hide file tree
Showing 21 changed files with 198 additions and 70 deletions.
5 changes: 3 additions & 2 deletions tests/e2e/gov/deposits.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/testutil"
clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
"github.com/cosmos/cosmos-sdk/testutil/network"
Expand Down Expand Up @@ -56,7 +57,7 @@ func (s *DepositTestSuite) submitProposal(val *network.Validator, initialDeposit
s.Require().NoError(s.network.WaitForNextBlock())

// query proposals, return the last's id
cmd := cli.GetCmdQueryProposals()
cmd := cli.GetCmdQueryProposals(address.NewBech32Codec("cosmos"))
args := []string{fmt.Sprintf("--%s=json", flags.FlagOutput)}
res, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
s.Require().NoError(err)
Expand Down Expand Up @@ -130,7 +131,7 @@ func (s *DepositTestSuite) TestQueryProposalAfterVotingPeriod() {
proposalID := strconv.FormatUint(id, 10)

args := []string{fmt.Sprintf("--%s=json", flags.FlagOutput)}
cmd := cli.GetCmdQueryProposals()
cmd := cli.GetCmdQueryProposals(address.NewBech32Codec("cosmos"))
_, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
s.Require().NoError(err)

Expand Down
5 changes: 3 additions & 2 deletions tests/e2e/gov/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"

"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/codec/address"
clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/gov/client/cli"
Expand Down Expand Up @@ -305,7 +306,7 @@ func (s *E2ETestSuite) TestCmdGetProposals() {
tc := tc

s.Run(tc.name, func() {
cmd := cli.GetCmdQueryProposals()
cmd := cli.GetCmdQueryProposals(address.NewBech32Codec("cosmos"))
clientCtx := val.ClientCtx

out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
Expand Down Expand Up @@ -532,7 +533,7 @@ func (s *E2ETestSuite) TestCmdQueryVote() {
for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
cmd := cli.GetCmdQueryVote()
cmd := cli.GetCmdQueryVote(address.NewBech32Codec("cosmos"))
clientCtx := val.ClientCtx

out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
Expand Down
18 changes: 9 additions & 9 deletions x/gov/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ import (
"strconv"
"strings"

"cosmossdk.io/core/address"
"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/version"
gcutils "github.com/cosmos/cosmos-sdk/x/gov/client/utils"
"github.com/cosmos/cosmos-sdk/x/gov/types"
v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
)

// GetQueryCmd returns the cli query commands for this module
func GetQueryCmd() *cobra.Command {
func GetQueryCmd(ac address.Codec) *cobra.Command {
// Group gov queries under a subcommand
govQueryCmd := &cobra.Command{
Use: types.ModuleName,
Expand All @@ -29,8 +29,8 @@ func GetQueryCmd() *cobra.Command {

govQueryCmd.AddCommand(
GetCmdQueryProposal(),
GetCmdQueryProposals(),
GetCmdQueryVote(),
GetCmdQueryProposals(ac),
GetCmdQueryVote(ac),
GetCmdQueryVotes(),
GetCmdQueryParams(),
GetCmdQueryParam(),
Expand Down Expand Up @@ -93,7 +93,7 @@ $ %s query gov proposal 1

// GetCmdQueryProposals implements a query proposals command. Command to Get
// Proposals Information.
func GetCmdQueryProposals() *cobra.Command {
func GetCmdQueryProposals(ac address.Codec) *cobra.Command {
cmd := &cobra.Command{
Use: "proposals",
Short: "Query proposals with optional filters",
Expand All @@ -117,14 +117,14 @@ $ %s query gov proposals --page=2 --limit=100
var proposalStatus v1.ProposalStatus

if len(bechDepositorAddr) != 0 {
_, err := sdk.AccAddressFromBech32(bechDepositorAddr)
_, err := ac.StringToBytes(bechDepositorAddr)
if err != nil {
return err
}
}

if len(bechVoterAddr) != 0 {
_, err := sdk.AccAddressFromBech32(bechVoterAddr)
_, err := ac.StringToBytes(bechVoterAddr)
if err != nil {
return err
}
Expand Down Expand Up @@ -181,7 +181,7 @@ $ %s query gov proposals --page=2 --limit=100

// GetCmdQueryVote implements the query proposal vote command. Command to Get a
// Vote Information.
func GetCmdQueryVote() *cobra.Command {
func GetCmdQueryVote(ac address.Codec) *cobra.Command {
cmd := &cobra.Command{
Use: "vote [proposal-id] [voter-addr]",
Args: cobra.ExactArgs(2),
Expand Down Expand Up @@ -218,7 +218,7 @@ $ %s query gov vote 1 cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk
return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
}

voterAddr, err := sdk.AccAddressFromBech32(args[1])
voterAddr, err := ac.StringToBytes(args[1])
if err != nil {
return err
}
Expand Down
5 changes: 3 additions & 2 deletions x/gov/client/cli/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"strings"

"github.com/cosmos/cosmos-sdk/codec/address"
clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"

"github.com/cosmos/cosmos-sdk/client/flags"
Expand Down Expand Up @@ -214,7 +215,7 @@ func (s *CLITestSuite) TestCmdGetProposals() {
tc := tc

s.Run(tc.name, func() {
cmd := cli.GetCmdQueryProposals()
cmd := cli.GetCmdQueryProposals(address.NewBech32Codec("cosmos"))
cmd.SetArgs(tc.args)
s.Require().Contains(fmt.Sprint(cmd), strings.TrimSpace(tc.expCmdOutput))
})
Expand Down Expand Up @@ -374,7 +375,7 @@ func (s *CLITestSuite) TestCmdQueryVote() {
for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
cmd := cli.GetCmdQueryVote()
cmd := cli.GetCmdQueryVote(address.NewBech32Codec("cosmos"))
cmd.SetArgs(tc.args)

if len(tc.args) != 0 {
Expand Down
4 changes: 4 additions & 0 deletions x/gov/keeper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,14 @@ func setupGovKeeper(t *testing.T) (
acctKeeper.EXPECT().GetModuleAddress(types.ModuleName).Return(govAcct).AnyTimes()
acctKeeper.EXPECT().GetModuleAddress(disttypes.ModuleName).Return(distAcct).AnyTimes()
acctKeeper.EXPECT().GetModuleAccount(gomock.Any(), types.ModuleName).Return(authtypes.NewEmptyModuleAccount(types.ModuleName)).AnyTimes()
acctKeeper.EXPECT().StringToBytes(govAcct.String()).Return(govAcct, nil).AnyTimes()
acctKeeper.EXPECT().BytesToString(govAcct).Return(govAcct.String(), nil).AnyTimes()

trackMockBalances(bankKeeper, distributionKeeper)
stakingKeeper.EXPECT().TokensFromConsensusPower(ctx, gomock.Any()).DoAndReturn(func(ctx sdk.Context, power int64) math.Int {
return sdk.TokensFromConsensusPower(power, math.NewIntFromUint64(1000000))
}).AnyTimes()

stakingKeeper.EXPECT().BondDenom(ctx).Return("stake").AnyTimes()
stakingKeeper.EXPECT().IterateBondedValidatorsByPower(gomock.Any(), gomock.Any()).AnyTimes()
stakingKeeper.EXPECT().IterateDelegations(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
Expand Down
30 changes: 23 additions & 7 deletions x/gov/keeper/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ func (keeper Keeper) GetDeposit(ctx sdk.Context, proposalID uint64, depositorAdd
func (keeper Keeper) SetDeposit(ctx sdk.Context, deposit v1.Deposit) {
store := ctx.KVStore(keeper.storeKey)
bz := keeper.cdc.MustMarshal(&deposit)
depositor := sdk.MustAccAddressFromBech32(deposit.Depositor)
depositor, err := keeper.authKeeper.StringToBytes(deposit.Depositor)
if err != nil {
panic(err)
}

store.Set(types.DepositKey(deposit.ProposalId, depositor), bz)
}
Expand Down Expand Up @@ -65,7 +68,10 @@ func (keeper Keeper) DeleteAndBurnDeposits(ctx sdk.Context, proposalID uint64) {
panic(err)
}

depositor := sdk.MustAccAddressFromBech32(deposit.Depositor)
depositor, err := keeper.authKeeper.StringToBytes(deposit.Depositor)
if err != nil {
panic(err)
}

store.Delete(types.DepositKey(proposalID, depositor))
return false
Expand Down Expand Up @@ -176,7 +182,11 @@ func (keeper Keeper) ChargeDeposit(ctx sdk.Context, proposalID uint64, destAddre
var cancellationCharges sdk.Coins

for _, deposit := range keeper.GetDeposits(ctx, proposalID) {
depositerAddress := sdk.MustAccAddressFromBech32(deposit.Depositor)
depositerAddress, err := keeper.authKeeper.StringToBytes(deposit.Depositor)
if err != nil {
return err
}

var remainingAmount sdk.Coins

for _, coins := range deposit.Amount {
Expand Down Expand Up @@ -225,8 +235,11 @@ func (keeper Keeper) ChargeDeposit(ctx sdk.Context, proposalID uint64, destAddre
return err
}
default:
destAccAddress := sdk.MustAccAddressFromBech32(destAddress)
err := keeper.bankKeeper.SendCoinsFromModuleToAccount(
destAccAddress, err := keeper.authKeeper.StringToBytes(destAddress)
if err != nil {
return err
}
err = keeper.bankKeeper.SendCoinsFromModuleToAccount(
ctx, types.ModuleName, destAccAddress, cancellationCharges,
)
if err != nil {
Expand All @@ -243,9 +256,12 @@ func (keeper Keeper) RefundAndDeleteDeposits(ctx sdk.Context, proposalID uint64)
store := ctx.KVStore(keeper.storeKey)

keeper.IterateDeposits(ctx, proposalID, func(deposit v1.Deposit) bool {
depositor := sdk.MustAccAddressFromBech32(deposit.Depositor)
depositor, err := keeper.authKeeper.StringToBytes(deposit.Depositor)
if err != nil {
panic(err)
}

err := keeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, depositor, deposit.Amount)
err = keeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, depositor, deposit.Amount)
if err != nil {
panic(err)
}
Expand Down
20 changes: 16 additions & 4 deletions x/gov/keeper/deposit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
sdkmath "cosmossdk.io/math"
"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/codec/address"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
Expand Down Expand Up @@ -35,7 +36,7 @@ func TestDeposits(t *testing.T) {

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
govKeeper, _, bankKeeper, stakingKeeper, distKeeper, _, ctx := setupGovKeeper(t)
govKeeper, authKeeper, bankKeeper, stakingKeeper, distKeeper, _, ctx := setupGovKeeper(t)
trackMockBalances(bankKeeper, distKeeper)

// With expedited proposals the minimum deposit is higher, so we must
Expand All @@ -47,6 +48,10 @@ func TestDeposits(t *testing.T) {
}

TestAddrs := simtestutil.AddTestAddrsIncremental(bankKeeper, stakingKeeper, ctx, 2, sdkmath.NewInt(10000000*depositMultiplier))
for _, addr := range TestAddrs {
authKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes()
authKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes()
}

tp := TestProposal
proposal, err := govKeeper.SubmitProposal(ctx, tp, "", "title", "summary", TestAddrs[0], tc.expedited)
Expand Down Expand Up @@ -297,10 +302,14 @@ func TestChargeDeposit(t *testing.T) {
}

t.Run(testName(i), func(t *testing.T) {
govKeeper, _, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t)
govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t)
params := v1.DefaultParams()
params.ProposalCancelRatio = tc.proposalCancelRatio
TestAddrs := simtestutil.AddTestAddrsIncremental(bankKeeper, stakingKeeper, ctx, 2, sdk.NewInt(10000000000))
for _, addr := range TestAddrs {
authKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes()
authKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes()
}

switch i {
case 0:
Expand All @@ -326,10 +335,12 @@ func TestChargeDeposit(t *testing.T) {
_, err = govKeeper.AddDeposit(ctx, proposalID, TestAddrs[0], fiveStake)
require.NoError(t, err)

codec := address.NewBech32Codec("cosmos")
// get balances of dest address
var prevBalance sdk.Coin
if len(params.ProposalCancelDest) != 0 {
accAddr := sdk.MustAccAddressFromBech32(params.ProposalCancelDest)
accAddr, err := codec.StringToBytes(params.ProposalCancelDest)
require.NoError(t, err)
prevBalance = bankKeeper.GetBalance(ctx, accAddr, sdk.DefaultBondDenom)
}

Expand All @@ -345,7 +356,8 @@ func TestChargeDeposit(t *testing.T) {
require.NoError(t, err)

if len(params.ProposalCancelDest) != 0 {
accAddr := sdk.MustAccAddressFromBech32(params.ProposalCancelDest)
accAddr, err := codec.StringToBytes(params.ProposalCancelDest)
require.NoError(t, err)
newBalanceAfterCancelProposal := bankKeeper.GetBalance(ctx, accAddr, sdk.DefaultBondDenom)
cancellationCharges := sdkmath.NewInt(0)
for _, deposits := range allDeposits {
Expand Down
8 changes: 4 additions & 4 deletions x/gov/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (q Keeper) Proposals(c context.Context, req *v1.QueryProposalsRequest) (*v1

// match voter address (if supplied)
if len(req.Voter) > 0 {
voter, err := sdk.AccAddressFromBech32(req.Voter)
voter, err := q.authKeeper.StringToBytes(req.Voter)
if err != nil {
return nil, err
}
Expand All @@ -77,7 +77,7 @@ func (q Keeper) Proposals(c context.Context, req *v1.QueryProposalsRequest) (*v1

// match depositor (if supplied)
if len(req.Depositor) > 0 {
depositor, err := sdk.AccAddressFromBech32(req.Depositor)
depositor, err := q.authKeeper.StringToBytes(req.Depositor)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -115,7 +115,7 @@ func (q Keeper) Vote(c context.Context, req *v1.QueryVoteRequest) (*v1.QueryVote

ctx := sdk.UnwrapSDKContext(c)

voter, err := sdk.AccAddressFromBech32(req.Voter)
voter, err := q.authKeeper.StringToBytes(req.Voter)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -211,7 +211,7 @@ func (q Keeper) Deposit(c context.Context, req *v1.QueryDepositRequest) (*v1.Que

ctx := sdk.UnwrapSDKContext(c)

depositor, err := sdk.AccAddressFromBech32(req.Depositor)
depositor, err := q.authKeeper.StringToBytes(req.Depositor)
if err != nil {
return nil, err
}
Expand Down
14 changes: 10 additions & 4 deletions x/gov/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (

simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/types/query"
v3 "github.com/cosmos/cosmos-sdk/x/gov/migrations/v3"
v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
Expand Down Expand Up @@ -657,8 +659,10 @@ func (suite *KeeperTestSuite) TestGRPCQueryVotes() {
{ProposalId: proposal.Id, Voter: addrs[0].String(), Options: v1.NewNonSplitVoteOption(v1.OptionAbstain)},
{ProposalId: proposal.Id, Voter: addrs[1].String(), Options: v1.NewNonSplitVoteOption(v1.OptionYes)},
}
accAddr1, err1 := sdk.AccAddressFromBech32(votes[0].Voter)
accAddr2, err2 := sdk.AccAddressFromBech32(votes[1].Voter)

codec := address.NewBech32Codec("cosmos")
accAddr1, err1 := codec.StringToBytes(votes[0].Voter)
accAddr2, err2 := codec.StringToBytes(votes[1].Voter)
suite.Require().NoError(err1)
suite.Require().NoError(err2)
suite.Require().NoError(suite.govKeeper.AddVote(ctx, proposal.Id, accAddr1, votes[0].Options, ""))
Expand Down Expand Up @@ -759,8 +763,10 @@ func (suite *KeeperTestSuite) TestLegacyGRPCQueryVotes() {
{ProposalId: proposal.Id, Voter: addrs[0].String(), Options: v1beta1.NewNonSplitVoteOption(v1beta1.OptionAbstain)},
{ProposalId: proposal.Id, Voter: addrs[1].String(), Options: v1beta1.NewNonSplitVoteOption(v1beta1.OptionYes)},
}
accAddr1, err1 := sdk.AccAddressFromBech32(votes[0].Voter)
accAddr2, err2 := sdk.AccAddressFromBech32(votes[1].Voter)
codec := address.NewBech32Codec("cosmos")

accAddr1, err1 := codec.StringToBytes(votes[0].Voter)
accAddr2, err2 := codec.StringToBytes(votes[1].Voter)
suite.Require().NoError(err1)
suite.Require().NoError(err2)
suite.Require().NoError(suite.govKeeper.AddVote(ctx, proposal.Id, accAddr1, v1.NewNonSplitVoteOption(v1.OptionAbstain), ""))
Expand Down
7 changes: 6 additions & 1 deletion x/gov/keeper/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,14 @@ func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx sdk.Context, p

func TestHooks(t *testing.T) {
minDeposit := v1.DefaultParams().MinDeposit
govKeeper, _, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t)
govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t)
addrs := simtestutil.AddTestAddrs(bankKeeper, stakingKeeper, ctx, 1, minDeposit[0].Amount)

for _, addr := range addrs {
authKeeper.EXPECT().BytesToString(addr).Return(addr.String(), nil).AnyTimes()
authKeeper.EXPECT().StringToBytes(addr.String()).Return(addr, nil).AnyTimes()
}

govHooksReceiver := MockGovHooksReceiver{}

keeper.UnsafeSetHooks(
Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func NewKeeper(
panic(fmt.Sprintf("%s module account has not been set", types.ModuleName))
}

if _, err := sdk.AccAddressFromBech32(authority); err != nil {
if _, err := authKeeper.StringToBytes(authority); err != nil {
panic(fmt.Sprintf("invalid authority address: %s", authority))
}

Expand Down
Loading

0 comments on commit f5993b1

Please sign in to comment.