Merge PR #5942: Tx Client Migration: x/gov

codec/std/codec.go

@@ -9,7 +9,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/auth/vesting"
 	"github.com/cosmos/cosmos-sdk/x/evidence"
 	eviexported "github.com/cosmos/cosmos-sdk/x/evidence/exported"
-	"github.com/cosmos/cosmos-sdk/x/gov"
+	gov "github.com/cosmos/cosmos-sdk/x/gov/types"
 	"github.com/cosmos/cosmos-sdk/x/supply"
 	supplyexported "github.com/cosmos/cosmos-sdk/x/supply/exported"
 )

codec/std/codec.proto

@@ -88,7 +88,7 @@ message Proposal {
 // governance proposal.
 message Content {
   option (gogoproto.equal)             = true;
-  option (cosmos_proto.interface_type) = "github.com/cosmos/cosmos-sdk/x/gov/types.Content";
+  option (cosmos_proto.interface_type) = "*github.com/cosmos/cosmos-sdk/x/gov/types.Content";
 
   // sum defines a set of all acceptable concrete governance proposal Content types.
   oneof sum {

codec/std/msgs.go

@@ -5,12 +5,12 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/evidence"
 	eviexported "github.com/cosmos/cosmos-sdk/x/evidence/exported"
-	"github.com/cosmos/cosmos-sdk/x/gov"
+	gov "github.com/cosmos/cosmos-sdk/x/gov/types"
 )
 
 var (
 	_ eviexported.MsgSubmitEvidence = MsgSubmitEvidence{}
-	_ gov.MsgSubmitProposalI        = MsgSubmitProposal{}
+	_ gov.MsgSubmitProposalI        = &MsgSubmitProposal{}
 )
 
 // NewMsgSubmitEvidence returns a new MsgSubmitEvidence.
@@ -47,13 +47,13 @@ func (msg MsgSubmitEvidence) GetEvidence() eviexported.Evidence { return msg.Evi
 func (msg MsgSubmitEvidence) GetSubmitter() sdk.AccAddress      { return msg.Submitter }
 
 // NewMsgSubmitProposal returns a new MsgSubmitProposal.
-func NewMsgSubmitProposal(c gov.Content, d sdk.Coins, p sdk.AccAddress) (MsgSubmitProposal, error) {
+func NewMsgSubmitProposal(c gov.Content, d sdk.Coins, p sdk.AccAddress) (gov.MsgSubmitProposalI, error) {
 	content := &Content{}
 	if err := content.SetContent(c); err != nil {
-		return MsgSubmitProposal{}, err
+		return nil, err
 	}
 
-	return MsgSubmitProposal{
+	return &MsgSubmitProposal{
 		Content:               content,
 		MsgSubmitProposalBase: gov.NewMsgSubmitProposalBase(d, p),
 	}, nil
@@ -79,6 +79,13 @@ func (msg MsgSubmitProposal) ValidateBasic() error {
 }
 
 // nolint
-func (msg MsgSubmitProposal) GetContent() gov.Content      { return msg.Content.GetContent() }
-func (msg MsgSubmitProposal) GetInitialDeposit() sdk.Coins { return msg.InitialDeposit }
-func (msg MsgSubmitProposal) GetProposer() sdk.AccAddress  { return msg.Proposer }
+func (msg *MsgSubmitProposal) GetContent() gov.Content { return msg.Content.GetContent() }
+func (msg *MsgSubmitProposal) SetContent(content gov.Content) error {
+	stdContent := &Content{}
+	err := stdContent.SetContent(content)
+	if err != nil {
+		return err
+	}
+	msg.Content = stdContent
+	return nil
+}

codec/std/msgs_test.go

@@ -4,12 +4,13 @@ import (
 	"testing"
 	"time"
 
+	gov "github.com/cosmos/cosmos-sdk/x/gov/types"
+
 	"github.com/stretchr/testify/require"
 
 	"github.com/cosmos/cosmos-sdk/codec/std"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/evidence"
-	"github.com/cosmos/cosmos-sdk/x/gov"
 )
 
 func TestNewMsgSubmitEvidence(t *testing.T) {
@@ -28,15 +29,45 @@ func TestNewMsgSubmitEvidence(t *testing.T) {
 	require.NoError(t, msg.ValidateBasic())
 }
 
-func TestNewNewMsgSubmitProposal(t *testing.T) {
+type invalidProposal struct {
+	*gov.TextProposal
+}
+
+func TestMsgSubmitProposal(t *testing.T) {
 	p := sdk.AccAddress("foo")
 	d := sdk.NewCoins(sdk.NewInt64Coin("stake", 1000))
-	c := gov.TextProposal{Title: "title", Description: "description"}
+	c := gov.NewTextProposal("title", "description")
+
+	//
+	// test constructor
+	//
 
 	msg, err := std.NewMsgSubmitProposal(c, d, p)
 	require.NoError(t, err)
-	require.Equal(t, msg.GetContent(), &c)
+	require.Equal(t, msg.GetContent(), c)
 	require.Equal(t, msg.GetProposer(), p)
 	require.Equal(t, msg.GetInitialDeposit(), d)
 	require.NoError(t, msg.ValidateBasic())
+
+	_, err = std.NewMsgSubmitProposal(invalidProposal{}, d, p)
+	require.Error(t, err)
+
+	//
+	// test setter methods
+	//
+
+	msg = &std.MsgSubmitProposal{}
+	msg.SetProposer(p)
+	msg.SetInitialDeposit(d)
+	err = msg.SetContent(c)
+	require.NoError(t, err)
+	require.Equal(t, msg.GetContent(), c)
+	require.Equal(t, msg.GetProposer(), p)
+	require.Equal(t, msg.GetInitialDeposit(), d)
+	require.NoError(t, msg.ValidateBasic())
+
+	msg = &std.MsgSubmitProposal{}
+	err = msg.SetContent(invalidProposal{})
+	require.Error(t, err)
+
 }

docs/architecture/adr-020-protobuf-transaction-encoding.md

@@ -4,6 +4,7 @@
 
 - 2020 March 06: Initial Draft
 - 2020 March 12: API Updates
+- 2020 April 13: Added details on interface `oneof` handling
 
 ## Status
 
@@ -132,9 +133,42 @@ We then update `CLIContext` to have a new field: `Marshaler`.
 
 Then, each module's client handler will at the minimum accept a `Marshaler` instead
 of a concrete Amino codec and a `Generator` along with an `AccountRetriever` so
-that account fields can be retrieved for signing. If the module needs to work with
-any interface types, it will use the `Codec` interface defined by the module which
-also extends `Marshaler`.
+that account fields can be retrieved for signing.
+
+#### Interface `oneof` Handling
+
+If the module needs to work with any `sdk.Msg`s that use interface types, that
+`sdk.Msg` should be implemented as an interface with getters and setters on the
+module level and a no-arg constructor function should be passed around to
+required CLI and REST client commands.
+
+For example, in `x/gov`, `Content` is an interface type, so `MsgSubmitProposalI`
+should also be an interface and implement setter methods:
+
+```go
+// x/gov/types/msgs.go
+type MsgSubmitProposalI interface {
+	sdk.Msg
+
+	GetContent() Content
+    // SetContent returns an error if the underlying oneof does not support
+    // the concrete Content passed in
+	SetContent(Content) error
+
+	GetInitialDeposit() sdk.Coins
+	SetInitialDeposit(sdk.Coins)
+
+	GetProposer() sdk.AccAddress
+	SetProposer(sdk.AccAddress)
+}
+```
+
+Note that the implementation of `MsgSubmitProposalI` can be simplified by
+using an embedded base struct which implements most of that interface - in this
+case `MsgSubmitProposalBase`.
+
+A parameter `ctr func() MsgSubmitProposalI` would then be passed to CLI client
+methods in order to construct a concrete instance.
 
 ## Future Improvements
 

x/distribution/handler.go

@@ -103,7 +103,7 @@ func handleMsgFundCommunityPool(ctx sdk.Context, msg types.MsgFundCommunityPool,
 func NewCommunityPoolSpendProposalHandler(k Keeper) govtypes.Handler {
 	return func(ctx sdk.Context, content govtypes.Content) error {
 		switch c := content.(type) {
-		case types.CommunityPoolSpendProposal:
+		case *types.CommunityPoolSpendProposal:
 			return keeper.HandleCommunityPoolSpendProposal(ctx, k, c)
 
 		default:

x/distribution/keeper/proposal_handler.go

@@ -9,7 +9,7 @@ import (
 )
 
 // HandleCommunityPoolSpendProposal is a handler for executing a passed community spend proposal
-func HandleCommunityPoolSpendProposal(ctx sdk.Context, k Keeper, p types.CommunityPoolSpendProposal) error {
+func HandleCommunityPoolSpendProposal(ctx sdk.Context, k Keeper, p *types.CommunityPoolSpendProposal) error {
 	if k.blacklistedAddrs[p.Recipient.String()] {
 		return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is blacklisted from receiving external funds", p.Recipient)
 	}

x/distribution/proposal_handler_test.go

@@ -24,7 +24,7 @@ var (
 	amount = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1)))
 )
 
-func testProposal(recipient sdk.AccAddress, amount sdk.Coins) types.CommunityPoolSpendProposal {
+func testProposal(recipient sdk.AccAddress, amount sdk.Coins) *types.CommunityPoolSpendProposal {
 	return types.NewCommunityPoolSpendProposal("Test", "description", recipient, amount)
 }
 

x/distribution/types/codec.go

@@ -10,7 +10,7 @@ func RegisterCodec(cdc *codec.Codec) {
 	cdc.RegisterConcrete(MsgWithdrawDelegatorReward{}, "cosmos-sdk/MsgWithdrawDelegationReward", nil)
 	cdc.RegisterConcrete(MsgWithdrawValidatorCommission{}, "cosmos-sdk/MsgWithdrawValidatorCommission", nil)
 	cdc.RegisterConcrete(MsgSetWithdrawAddress{}, "cosmos-sdk/MsgModifyWithdrawAddress", nil)
-	cdc.RegisterConcrete(CommunityPoolSpendProposal{}, "cosmos-sdk/CommunityPoolSpendProposal", nil)
+	cdc.RegisterConcrete(&CommunityPoolSpendProposal{}, "cosmos-sdk/CommunityPoolSpendProposal", nil)
 }
 
 var (

x/distribution/types/proposal.go

@@ -14,32 +14,32 @@ const (
 )
 
 // Assert CommunityPoolSpendProposal implements govtypes.Content at compile-time
-var _ govtypes.Content = CommunityPoolSpendProposal{}
+var _ govtypes.Content = &CommunityPoolSpendProposal{}
 
 func init() {
 	govtypes.RegisterProposalType(ProposalTypeCommunityPoolSpend)
-	govtypes.RegisterProposalTypeCodec(CommunityPoolSpendProposal{}, "cosmos-sdk/CommunityPoolSpendProposal")
+	govtypes.RegisterProposalTypeCodec(&CommunityPoolSpendProposal{}, "cosmos-sdk/CommunityPoolSpendProposal")
 }
 
 // NewCommunityPoolSpendProposal creates a new community pool spned proposal.
-func NewCommunityPoolSpendProposal(title, description string, recipient sdk.AccAddress, amount sdk.Coins) CommunityPoolSpendProposal {
-	return CommunityPoolSpendProposal{title, description, recipient, amount}
+func NewCommunityPoolSpendProposal(title, description string, recipient sdk.AccAddress, amount sdk.Coins) *CommunityPoolSpendProposal {
+	return &CommunityPoolSpendProposal{title, description, recipient, amount}
 }
 
 // GetTitle returns the title of a community pool spend proposal.
-func (csp CommunityPoolSpendProposal) GetTitle() string { return csp.Title }
+func (csp *CommunityPoolSpendProposal) GetTitle() string { return csp.Title }
 
 // GetDescription returns the description of a community pool spend proposal.
-func (csp CommunityPoolSpendProposal) GetDescription() string { return csp.Description }
+func (csp *CommunityPoolSpendProposal) GetDescription() string { return csp.Description }
 
 // GetDescription returns the routing key of a community pool spend proposal.
-func (csp CommunityPoolSpendProposal) ProposalRoute() string { return RouterKey }
+func (csp *CommunityPoolSpendProposal) ProposalRoute() string { return RouterKey }
 
 // ProposalType returns the type of a community pool spend proposal.
-func (csp CommunityPoolSpendProposal) ProposalType() string { return ProposalTypeCommunityPoolSpend }
+func (csp *CommunityPoolSpendProposal) ProposalType() string { return ProposalTypeCommunityPoolSpend }
 
 // ValidateBasic runs basic stateless validity checks
-func (csp CommunityPoolSpendProposal) ValidateBasic() error {
+func (csp *CommunityPoolSpendProposal) ValidateBasic() error {
 	err := govtypes.ValidateAbstract(csp)
 	if err != nil {
 		return err

x/gov/abci_test.go

@@ -4,10 +4,11 @@ import (
 	"testing"
 	"time"
 
+	codecstd "github.com/cosmos/cosmos-sdk/codec/std"
+
 	"github.com/stretchr/testify/require"
 	abci "github.com/tendermint/tendermint/abci/types"
 
-	codecstd "github.com/cosmos/cosmos-sdk/codec/std"
 	"github.com/cosmos/cosmos-sdk/simapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/gov"

x/gov/alias.go

@@ -82,8 +82,8 @@ var (
 	SplitInactiveProposalQueueKey = types.SplitInactiveProposalQueueKey
 	SplitKeyDeposit               = types.SplitKeyDeposit
 	SplitKeyVote                  = types.SplitKeyVote
-	NewMsgSubmitProposalBase      = types.NewMsgSubmitProposalBase
 	NewMsgSubmitProposal          = types.NewMsgSubmitProposal
+	NewMsgSubmitProposalBase      = types.NewMsgSubmitProposalBase
 	NewMsgDeposit                 = types.NewMsgDeposit
 	NewMsgVote                    = types.NewMsgVote
 	ParamKeyTable                 = types.ParamKeyTable