feat(ante): prevent oversight DAO change to be bundled in proposals

CHANGELOG.md

@@ -23,6 +23,7 @@
 - Migrate `x/gov` fork from Atom One to Atom One SDK [#248](https://github.com/atomone-hub/atomone/pull/248)
 - Add governors to `x/gov` module [#258](https://github.com/atomone-hub/atomone/pull/258)
 - Prevent Oversight DAO from vetoing proposals that include a change to the Oversight DAO address [#275](https://github.com/atomone-hub/atomone/pull/275)
+- Prevent Oversight DAO change to be bundled in proposals [#316](https://github.com/atomone-hub/atomone/pull/316)
 
 ### STATE BREAKING
 

ante/ante.go

@@ -28,6 +28,7 @@ type HandlerOptions struct {
 	PhotonKeeper     *photonkeeper.Keeper
 	TxFeeChecker     ante.TxFeeChecker
 	DynamicfeeKeeper *dynamicfeekeeper.Keeper
+	CoreDAOsKeeper   CoredaosParamsGetter
 }
 
 func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
@@ -52,6 +53,9 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
 	if opts.DynamicfeeKeeper == nil {
 		return nil, errorsmod.Wrap(atomoneerrors.ErrNotFound, "dynamicfee keeper is required for AnteHandler")
 	}
+	if opts.CoreDAOsKeeper == nil {
+		return nil, errorsmod.Wrap(atomoneerrors.ErrNotFound, "coredaos keeper is required for AnteHandler")
+	}
 
 	sigGasConsumer := opts.SigGasConsumer
 	if sigGasConsumer == nil {
@@ -83,6 +87,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
 			),
 		),
 		NewGovVoteDecorator(opts.Codec, opts.StakingKeeper),
+		NewGovSubmitProposalDecorator(opts.Codec, opts.CoreDAOsKeeper),
 		ibcante.NewRedundantRelayDecorator(opts.IBCkeeper),
 	}
 

ante/gov_ante_utils.go

@@ -0,0 +1,57 @@
+package ante
+
+import (
+	errorsmod "cosmossdk.io/errors"
+
+	"github.com/cosmos/cosmos-sdk/codec"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+
+	atomoneerrors "github.com/atomone-hub/atomone/types/errors"
+)
+
+// iterateMsg calls fn for each message in msgs. For authz.MsgExec messages,
+// fn is called for each inner message instead of the exec message itself.
+// Returns ErrUnauthorized if an inner message cannot be unpacked.
+func iterateMsg(cdc codec.BinaryCodec, msgs []sdk.Msg, fn func(sdk.Msg) error) error {
+	for _, m := range msgs {
+		if execMsg, ok := m.(*authz.MsgExec); ok {
+			for _, anyInner := range execMsg.Msgs {
+				var inner sdk.Msg
+				if err := cdc.UnpackAny(anyInner, &inner); err != nil {
+					return errorsmod.Wrap(atomoneerrors.ErrUnauthorized, "cannot unmarshal authz exec msgs")
+				}
+				if err := fn(inner); err != nil {
+					return err
+				}
+			}
+			continue
+		}
+		if err := fn(m); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// flattenAnyMsgs recursively unpacks a list of Any-encoded messages, expanding
+// authz.MsgExec wrappers so that all leaf messages are returned in a flat
+// slice. Entries that cannot be unpacked are represented as nil — they still
+// count towards the total length, which callers use to detect bundling.
+func flattenAnyMsgs(cdc codec.BinaryCodec, anyMsgs []*codectypes.Any) []sdk.Msg {
+	var result []sdk.Msg
+	for _, anyMsg := range anyMsgs {
+		var msg sdk.Msg
+		if err := cdc.UnpackAny(anyMsg, &msg); err != nil {
+			result = append(result, nil)
+			continue
+		}
+		if execMsg, ok := msg.(*authz.MsgExec); ok {
+			result = append(result, flattenAnyMsgs(cdc, execMsg.Msgs)...)
+		} else {
+			result = append(result, msg)
+		}
+	}
+	return result
+}

ante/gov_submit_proposal_ante.go

@@ -0,0 +1,129 @@
+package ante
+
+import (
+	"context"
+
+	errorsmod "cosmossdk.io/errors"
+
+	"github.com/cosmos/cosmos-sdk/codec"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkgovv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
+
+	atomoneerrors "github.com/atomone-hub/atomone/types/errors"
+	coredaostypes "github.com/atomone-hub/atomone/x/coredaos/types"
+	govv1 "github.com/atomone-hub/atomone/x/gov/types/v1"
+)
+
+// CoredaosParamsGetter is the interface required by GovSubmitProposalDecorator to
+// retrieve the current coredaos params.
+type CoredaosParamsGetter interface {
+	GetParams(ctx context.Context) coredaostypes.Params
+}
+
+// GovSubmitProposalDecorator rejects any governance proposal whose message list
+// contains a coredaos MsgUpdateParams that changes the oversight DAO address
+// when that message is bundled together with other proposal messages.
+type GovSubmitProposalDecorator struct {
+	cdc            codec.BinaryCodec
+	coredaosKeeper CoredaosParamsGetter
+}
+
+// NewGovSubmitProposalDecorator creates a new GovSubmitProposalDecorator.
+func NewGovSubmitProposalDecorator(cdc codec.BinaryCodec, coredaosKeeper CoredaosParamsGetter) GovSubmitProposalDecorator {
+	return GovSubmitProposalDecorator{
+		cdc:            cdc,
+		coredaosKeeper: coredaosKeeper,
+	}
+}
+
+func (g GovSubmitProposalDecorator) AnteHandle(
+	ctx sdk.Context, tx sdk.Tx,
+	simulate bool, next sdk.AnteHandler,
+) (newCtx sdk.Context, err error) {
+	msgs := tx.GetMsgs()
+	if err = g.ValidateSubmitProposalMsgs(ctx, msgs); err != nil {
+		return ctx, err
+	}
+	return next(ctx, tx, simulate)
+}
+
+// ValidateSubmitProposalMsgs checks that no submit-proposal message bundles a
+// coredaos MsgUpdateParams that changes the oversight DAO address together with
+// other proposal messages. It also inspects authz.MsgExec wrappers.
+func (g GovSubmitProposalDecorator) ValidateSubmitProposalMsgs(ctx sdk.Context, msgs []sdk.Msg) error {
+	validateMsg := func(m sdk.Msg) error {
+		var proposalMsgs []*codectypes.Any
+		switch msg := m.(type) {
+		case *govv1.MsgSubmitProposal:
+			proposalMsgs = msg.GetMessages()
+		case *sdkgovv1.MsgSubmitProposal:
+			proposalMsgs = msg.GetMessages()
+		default:
+			return nil
+		}
+		return g.validateProposalMessages(ctx, proposalMsgs)
+	}
+	return iterateMsg(g.cdc, msgs, validateMsg)
+}
+
+// addressChanged returns true if newAddr and currentAddr refer to
+// different accounts. Comparison is done on the decoded bytes to
+// make it case-insensitive
+func addressChanged(newAddr, currentAddr string) (bool, error) {
+	if newAddr == "" && currentAddr == "" {
+		return false, nil
+	}
+	if newAddr == "" || currentAddr == "" {
+		return true, nil
+	}
+	newAccAddr, err := sdk.AccAddressFromBech32(newAddr)
+	if err != nil {
+		return false, err
+	}
+	currentAccAddr, err := sdk.AccAddressFromBech32(currentAddr)
+	if err != nil {
+		return false, err
+	}
+	return !newAccAddr.Equals(currentAccAddr), nil
+}
+
+// validateProposalMessages inspects a list of proposal messages and returns an
+// error if a coredaos MsgUpdateParams that changes the oversight DAO address is
+// bundled with other messages. authz.MsgExec wrappers are expanded recursively
+// so that nested oversight-DAO changes are also detected.
+func (g GovSubmitProposalDecorator) validateProposalMessages(ctx sdk.Context, anyMsgs []*codectypes.Any) error {
+	effectiveMsgs := flattenAnyMsgs(g.cdc, anyMsgs)
+
+	// Bundling is only possible when there is more than one effective message.
+	if len(effectiveMsgs) <= 1 {
+		return nil
+	}
+
+	currentParams := g.coredaosKeeper.GetParams(ctx)
+
+	for _, msg := range effectiveMsgs {
+		if msg == nil {
+			// Unpackable message — cannot be MsgUpdateParams, skip.
+			continue
+		}
+		updateParams, ok := msg.(*coredaostypes.MsgUpdateParams)
+		if !ok {
+			continue
+		}
+		changed, err := addressChanged(updateParams.Params.OversightDaoAddress, currentParams.OversightDaoAddress)
+		if err != nil {
+			return errorsmod.Wrap(
+				atomoneerrors.ErrUnauthorized,
+				"failed to compare Oversight DAO addresses: "+err.Error(),
+			)
+		}
+		if changed {
+			return errorsmod.Wrap(
+				atomoneerrors.ErrUnauthorized,
+				"proposal that changes the Oversight DAO address cannot be bundled with other messages",
+			)
+		}
+	}
+	return nil
+}