Skip to content

Commit

Permalink
refactor(x/gov)!: remove Accounts.String() (#19850)
Browse files Browse the repository at this point in the history
Co-authored-by: son trinh <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Mar 26, 2024
1 parent a82615b commit 160c418
Show file tree
Hide file tree
Showing 37 changed files with 774 additions and 386 deletions.
29 changes: 21 additions & 8 deletions tests/integration/gov/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ func TestUnregisteredProposal_InactiveProposalFails(t *testing.T) {
suite := createTestSuite(t)
ctx := suite.app.BaseApp.NewContext(false)
addrs := simtestutil.AddTestAddrs(suite.BankKeeper, suite.StakingKeeper, ctx, 10, valTokens)
addr0Str, err := suite.AccountKeeper.AddressCodec().BytesToString(addrs[0])
require.NoError(t, err)

// manually set proposal in store
startTime, endTime := time.Now().Add(-4*time.Hour), ctx.BlockHeader().Time
proposal, err := v1.NewProposal([]sdk.Msg{
&v1.Proposal{}, // invalid proposal message
}, 1, startTime, startTime, "", "Unsupported proposal", "Unsupported proposal", addrs[0], v1.ProposalType_PROPOSAL_TYPE_STANDARD)
}, 1, startTime, startTime, "", "Unsupported proposal", "Unsupported proposal", addr0Str, v1.ProposalType_PROPOSAL_TYPE_STANDARD)
require.NoError(t, err)

err = suite.GovKeeper.Proposals.Set(ctx, proposal.Id, proposal)
Expand All @@ -51,12 +53,13 @@ func TestUnregisteredProposal_ActiveProposalFails(t *testing.T) {
suite := createTestSuite(t)
ctx := suite.app.BaseApp.NewContext(false)
addrs := simtestutil.AddTestAddrs(suite.BankKeeper, suite.StakingKeeper, ctx, 10, valTokens)

addr0Str, err := suite.AccountKeeper.AddressCodec().BytesToString(addrs[0])
require.NoError(t, err)
// manually set proposal in store
startTime, endTime := time.Now().Add(-4*time.Hour), ctx.BlockHeader().Time
proposal, err := v1.NewProposal([]sdk.Msg{
&v1.Proposal{}, // invalid proposal message
}, 1, startTime, startTime, "", "Unsupported proposal", "Unsupported proposal", addrs[0], v1.ProposalType_PROPOSAL_TYPE_STANDARD)
}, 1, startTime, startTime, "", "Unsupported proposal", "Unsupported proposal", addr0Str, v1.ProposalType_PROPOSAL_TYPE_STANDARD)
require.NoError(t, err)
proposal.Status = v1.StatusVotingPeriod
proposal.VotingEndTime = &endTime
Expand Down Expand Up @@ -194,7 +197,9 @@ func TestTickPassedDepositPeriod(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(time.Duration(1) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)

newDepositMsg := v1.NewMsgDeposit(addrs[1], proposalID, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 100000)})
addr1Str, err := suite.AccountKeeper.AddressCodec().BytesToString(addrs[1])
require.NoError(t, err)
newDepositMsg := v1.NewMsgDeposit(addr1Str, proposalID, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 100000)})

res1, err := govMsgSvr.Deposit(ctx, newDepositMsg)
require.NoError(t, err)
Expand Down Expand Up @@ -293,7 +298,9 @@ func TestTickPassedVotingPeriod(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(time.Duration(1) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)

newDepositMsg := v1.NewMsgDeposit(addrs[1], proposalID, proposalCoins)
addr1Str, err := suite.AccountKeeper.AddressCodec().BytesToString(addrs[1])
require.NoError(t, err)
newDepositMsg := v1.NewMsgDeposit(addr1Str, proposalID, proposalCoins)

res1, err := govMsgSvr.Deposit(ctx, newDepositMsg)
require.NoError(t, err)
Expand Down Expand Up @@ -373,7 +380,9 @@ func TestProposalPassedEndblocker(t *testing.T) {
require.NoError(t, err)

proposalCoins := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, suite.StakingKeeper.TokensFromConsensusPower(ctx, 10*depositMultiplier))}
newDepositMsg := v1.NewMsgDeposit(addrs[0], proposal.Id, proposalCoins)
addr0Str, err := suite.AccountKeeper.AddressCodec().BytesToString(addrs[0])
require.NoError(t, err)
newDepositMsg := v1.NewMsgDeposit(addr0Str, proposal.Id, proposalCoins)

res, err := govMsgSvr.Deposit(ctx, newDepositMsg)
require.NoError(t, err)
Expand Down Expand Up @@ -433,7 +442,9 @@ func TestEndBlockerProposalHandlerFailed(t *testing.T) {
require.NoError(t, err)

proposalCoins := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, suite.StakingKeeper.TokensFromConsensusPower(ctx, 10)))
newDepositMsg := v1.NewMsgDeposit(addrs[0], proposal.Id, proposalCoins)
addr0Str, err := suite.AccountKeeper.AddressCodec().BytesToString(addrs[0])
require.NoError(t, err)
newDepositMsg := v1.NewMsgDeposit(addr0Str, proposal.Id, proposalCoins)

govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper)
res, err := govMsgSvr.Deposit(ctx, newDepositMsg)
Expand Down Expand Up @@ -531,7 +542,9 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(time.Duration(1) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)

newDepositMsg := v1.NewMsgDeposit(addrs[1], proposalID, proposalCoins)
addr1Str, err := suite.AccountKeeper.AddressCodec().BytesToString(addrs[1])
require.NoError(t, err)
newDepositMsg := v1.NewMsgDeposit(addr1Str, proposalID, proposalCoins)

res1, err := govMsgSvr.Deposit(ctx, newDepositMsg)
require.NoError(t, err)
Expand Down
11 changes: 6 additions & 5 deletions tests/sims/gov/operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ func TestSimulateMsgCancelProposal(t *testing.T) {
r := rand.New(s)
accounts := getTestingAccounts(t, r, suite.AccountKeeper, suite.BankKeeper, suite.StakingKeeper, ctx, 3)
// setup a proposal
proposer := accounts[0].Address
proposer, err := suite.AccountKeeper.AddressCodec().BytesToString(accounts[0].Address)
require.NoError(t, err)
content := v1beta1.NewTextProposal("Test", "description")
contentMsg, err := v1.NewLegacyContent(content, suite.GovKeeper.GetGovernanceAccount(ctx).GetAddress().String())
require.NoError(t, err)
Expand All @@ -233,7 +234,7 @@ func TestSimulateMsgCancelProposal(t *testing.T) {
require.NoError(t, err)
require.True(t, operationMsg.OK)
require.Equal(t, uint64(1), msg.ProposalId)
require.Equal(t, proposer.String(), msg.Proposer)
require.Equal(t, proposer, msg.Proposer)
require.Equal(t, simulation.TypeMsgCancelProposal, sdk.MsgTypeURL(&msg))
}

Expand All @@ -259,7 +260,7 @@ func TestSimulateMsgDeposit(t *testing.T) {
params, _ := suite.GovKeeper.Params.Get(ctx)
depositPeriod := params.MaxDepositPeriod

proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "description", sdk.AccAddress("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r"), v1.ProposalType_PROPOSAL_TYPE_STANDARD)
proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "description", "cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r", v1.ProposalType_PROPOSAL_TYPE_STANDARD)
require.NoError(t, err)

err = suite.GovKeeper.Proposals.Set(ctx, proposal.Id, proposal)
Expand Down Expand Up @@ -303,7 +304,7 @@ func TestSimulateMsgVote(t *testing.T) {
params, _ := suite.GovKeeper.Params.Get(ctx)
depositPeriod := params.MaxDepositPeriod

proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "description", sdk.AccAddress("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r"), v1.ProposalType_PROPOSAL_TYPE_STANDARD)
proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "description", "cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r", v1.ProposalType_PROPOSAL_TYPE_STANDARD)
require.NoError(t, err)

err = suite.GovKeeper.ActivateVotingPeriod(ctx, proposal)
Expand Down Expand Up @@ -345,7 +346,7 @@ func TestSimulateMsgVoteWeighted(t *testing.T) {
params, _ := suite.GovKeeper.Params.Get(ctx)
depositPeriod := params.MaxDepositPeriod

proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "test", sdk.AccAddress("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r"), v1.ProposalType_PROPOSAL_TYPE_STANDARD)
proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "test", "cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r", v1.ProposalType_PROPOSAL_TYPE_STANDARD)
require.NoError(t, err)

err = suite.GovKeeper.ActivateVotingPeriod(ctx, proposal)
Expand Down
4 changes: 4 additions & 0 deletions x/gov/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* [#19850](https://github.com/cosmos/cosmos-sdk/pull/19850) Removes the use of Accounts String method:
* `NewDeposit`, `NewMsgDeposit`, `NewMsgVote`, `NewMsgVoteWeighted`, `NewVote`, `NewProposal`, `NewMsgSubmitProposal` now take a string as an argument instead of an `sdk.AccAddress`.
* `Prompt` and `PromptMetadata` take an address.Codec as arguments.
* `SetProposer` takes a String as an argument instead of a `fmt.Stringer`.
* [#19481](https://github.com/cosmos/cosmos-sdk/pull/19481) Migrate module to use `appmodule.Environment`; `NewKeeper` now takes `appmodule.Environment` instead of a store service and no `baseapp.MessageRouter` anymore.
* [#19481](https://github.com/cosmos/cosmos-sdk/pull/19481) v1beta1 proposal handlers now take a `context.Context` instead of an `sdk.Context`.
* [#19592](https://github.com/cosmos/cosmos-sdk/pull/19592) `types.Config` and `types.DefaultConfig` have been moved to the keeper package in order to support the custom tallying function.
Expand Down
21 changes: 13 additions & 8 deletions x/gov/client/cli/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/manifoldco/promptui"
"github.com/spf13/cobra"

"cosmossdk.io/core/address"
authtypes "cosmossdk.io/x/auth/types"
"cosmossdk.io/x/gov/types"

Expand Down Expand Up @@ -64,7 +65,7 @@ var suggestedProposalTypes = []proposalType{
// namePrefix is the name to be displayed as "Enter <namePrefix> <field>"
// TODO: when bringing this in autocli, use proto message instead
// this will simplify the get address logic
func Prompt[T any](data T, namePrefix string) (T, error) {
func Prompt[T any](data T, namePrefix string, addressCodec address.Codec) (T, error) {
v := reflect.ValueOf(&data).Elem()
if v.Kind() == reflect.Interface {
v = reflect.ValueOf(data)
Expand Down Expand Up @@ -95,7 +96,11 @@ func Prompt[T any](data T, namePrefix string) (T, error) {

if strings.EqualFold(fieldName, "authority") {
// pre-fill with gov address
prompt.Default = authtypes.NewModuleAddress(types.ModuleName).String()
defaultAddr, err := addressCodec.BytesToString(authtypes.NewModuleAddress(types.ModuleName))
if err != nil {
return data, err
}
prompt.Default = defaultAddr
prompt.Validate = client.ValidatePromptAddress
}

Expand Down Expand Up @@ -158,8 +163,8 @@ type proposalType struct {
}

// Prompt the proposal type values and return the proposal and its metadata
func (p *proposalType) Prompt(cdc codec.Codec, skipMetadata bool) (*proposal, types.ProposalMetadata, error) {
metadata, err := PromptMetadata(skipMetadata)
func (p *proposalType) Prompt(cdc codec.Codec, skipMetadata bool, addressCodec address.Codec) (*proposal, types.ProposalMetadata, error) {
metadata, err := PromptMetadata(skipMetadata, addressCodec)
if err != nil {
return nil, metadata, fmt.Errorf("failed to set proposal metadata: %w", err)
}
Expand All @@ -185,7 +190,7 @@ func (p *proposalType) Prompt(cdc codec.Codec, skipMetadata bool) (*proposal, ty
}

// set messages field
result, err := Prompt(p.Msg, "msg")
result, err := Prompt(p.Msg, "msg", addressCodec)
if err != nil {
return nil, metadata, fmt.Errorf("failed to set proposal message: %w", err)
}
Expand All @@ -209,9 +214,9 @@ func getProposalSuggestions() []string {
}

// PromptMetadata prompts for proposal metadata or only title and summary if skip is true
func PromptMetadata(skip bool) (types.ProposalMetadata, error) {
func PromptMetadata(skip bool, addressCodec address.Codec) (types.ProposalMetadata, error) {
if !skip {
metadata, err := Prompt(types.ProposalMetadata{}, "proposal")
metadata, err := Prompt(types.ProposalMetadata{}, "proposal", addressCodec)
if err != nil {
return metadata, fmt.Errorf("failed to set proposal metadata: %w", err)
}
Expand Down Expand Up @@ -306,7 +311,7 @@ func NewCmdDraftProposal() *cobra.Command {

skipMetadataPrompt, _ := cmd.Flags().GetBool(flagSkipMetadata)

result, metadata, err := proposal.Prompt(clientCtx.Codec, skipMetadataPrompt)
result, metadata, err := proposal.Prompt(clientCtx.Codec, skipMetadataPrompt, clientCtx.AddressCodec)
if err != nil {
return err
}
Expand Down
6 changes: 4 additions & 2 deletions x/gov/client/cli/prompt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"github.com/stretchr/testify/require"

"cosmossdk.io/x/gov/client/cli"

codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"
)

type st struct {
Expand Down Expand Up @@ -50,7 +52,7 @@ func TestPromptIntegerOverflow(t *testing.T) {
_, err := fw.Write([]byte(overflowStr + "\n"))
assert.NoError(t, err)

v, err := cli.Prompt(st{}, "")
v, err := cli.Prompt(st{}, "", codectestutil.CodecOptions{}.GetAddressCodec())
assert.Equal(t, st{}, v, "expected a value of zero")
require.NotNil(t, err, "expected a report of an overflow")
require.Contains(t, err.Error(), "range")
Expand Down Expand Up @@ -81,7 +83,7 @@ func TestPromptParseInteger(t *testing.T) {
readline.Stdin = fin
_, err := fw.Write([]byte(tc.in + "\n"))
assert.NoError(t, err)
v, err := cli.Prompt(st{}, "")
v, err := cli.Prompt(st{}, "", codectestutil.CodecOptions{}.GetAddressCodec())
assert.Nil(t, err, "expected a nil error")
assert.Equal(t, tc.want, v.I, "expected %d = %d", tc.want, v.I)
})
Expand Down
19 changes: 16 additions & 3 deletions x/gov/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,12 @@ metadata example:
return err
}

msg, err := v1.NewMsgSubmitProposal(msgs, deposit, clientCtx.GetFromAddress().String(), proposal.Metadata, proposal.Title, proposal.Summary, proposal.proposalType)
addr, err := clientCtx.AddressCodec.BytesToString(clientCtx.GetFromAddress())
if err != nil {
return err
}

msg, err := v1.NewMsgSubmitProposal(msgs, deposit, addr, proposal.Metadata, proposal.Title, proposal.Summary, proposal.proposalType)
if err != nil {
return fmt.Errorf("invalid message: %w", err)
}
Expand Down Expand Up @@ -203,7 +208,12 @@ $ %s tx gov submit-legacy-proposal --title="Test Proposal" --description="My awe
return fmt.Errorf("failed to create proposal content: unknown proposal type %s", proposal.Type)
}

msg, err := v1beta1.NewMsgSubmitProposal(content, amount, clientCtx.GetFromAddress())
proposer, err := clientCtx.AddressCodec.BytesToString(clientCtx.GetFromAddress())
if err != nil {
return err
}

msg, err := v1beta1.NewMsgSubmitProposal(content, amount, proposer)
if err != nil {
return fmt.Errorf("invalid message: %w", err)
}
Expand Down Expand Up @@ -247,7 +257,10 @@ $ %s tx gov weighted-vote 1 yes=0.6,no=0.3,abstain=0.05,no-with-veto=0.05 --from
}

// Get voter address
from := clientCtx.GetFromAddress()
from, err := clientCtx.AddressCodec.BytesToString(clientCtx.GetFromAddress())
if err != nil {
return err
}

// validate that the proposal id is a uint
proposalID, err := strconv.ParseUint(args[0], 10, 64)
Expand Down
Loading

0 comments on commit 160c418

Please sign in to comment.