From c2f17a6d4d0f30f766b7ac10c817e4a5ad7712b5 Mon Sep 17 00:00:00 2001 From: Giuseppe Natale <12249307+giunatale@users.noreply.github.com> Date: Mon, 30 Mar 2026 17:36:41 +0200 Subject: [PATCH 1/4] prevent oversight dao change to be bundled --- ante/ante.go | 5 + ante/gov_submit_proposal_ante.go | 130 +++++++++++++++ ante/gov_submit_proposal_ante_test.go | 226 ++++++++++++++++++++++++++ app/app.go | 1 + 4 files changed, 362 insertions(+) create mode 100644 ante/gov_submit_proposal_ante.go create mode 100644 ante/gov_submit_proposal_ante_test.go diff --git a/ante/ante.go b/ante/ante.go index 8e095dfd..fa3a6c6d 100644 --- a/ante/ante.go +++ b/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), } diff --git a/ante/gov_submit_proposal_ante.go b/ante/gov_submit_proposal_ante.go new file mode 100644 index 00000000..1740699a --- /dev/null +++ b/ante/gov_submit_proposal_ante.go @@ -0,0 +1,130 @@ +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. +func (g GovSubmitProposalDecorator) ValidateSubmitProposalMsgs(ctx sdk.Context, msgs []sdk.Msg) error { + for _, m := range msgs { + var proposalMsgs []*codectypes.Any + switch msg := m.(type) { + case *govv1.MsgSubmitProposal: + proposalMsgs = msg.GetMessages() + case *sdkgovv1.MsgSubmitProposal: + proposalMsgs = msg.GetMessages() + default: + continue + } + + if err := g.validateProposalMessages(ctx, proposalMsgs); err != nil { + return err + } + } + return nil +} + +// 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. +func (g GovSubmitProposalDecorator) validateProposalMessages(ctx sdk.Context, anyMsgs []*codectypes.Any) error { + // Bundling is only possible when there is more than one message. + if len(anyMsgs) <= 1 { + return nil + } + + currentParams := g.coredaosKeeper.GetParams(ctx) + + for _, anyMsg := range anyMsgs { + var innerMsg sdk.Msg + if err := g.cdc.UnpackAny(anyMsg, &innerMsg); err != nil { + // If we cannot unpack, let other ante handlers or the msg server handle it. + continue + } + updateParams, ok := innerMsg.(*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 +} diff --git a/ante/gov_submit_proposal_ante_test.go b/ante/gov_submit_proposal_ante_test.go new file mode 100644 index 00000000..6f5fa1c4 --- /dev/null +++ b/ante/gov_submit_proposal_ante_test.go @@ -0,0 +1,226 @@ +package ante_test + +import ( + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" + + simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkgovv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" + + "github.com/atomone-hub/atomone/ante" + "github.com/atomone-hub/atomone/app/helpers" + coredaostypes "github.com/atomone-hub/atomone/x/coredaos/types" + govv1 "github.com/atomone-hub/atomone/x/gov/types/v1" +) + +// TestGovSubmitProposalDecoratorAtomOneV1 checks that the decorator rejects atomone +// gov v1 proposals that bundle a coredaos MsgUpdateParams changing the oversight +// DAO address with other messages. +func TestGovSubmitProposalDecoratorAtomOneV1(t *testing.T) { + atomoneApp := helpers.Setup(t) + ctx := atomoneApp.NewUncachedContext(true, tmproto.Header{}) + decorator := ante.NewGovSubmitProposalDecorator(atomoneApp.AppCodec(), atomoneApp.CoreDaosKeeper) + + addrs := simtestutil.CreateRandomAccounts(3) + currentOversightAddr := addrs[0].String() + newOversightAddr := addrs[1].String() + proposer := addrs[2].String() + extDuration := 7 * 24 * time.Hour + + // Set initial coredaos params with a known oversight DAO address. + err := atomoneApp.CoreDaosKeeper.Params.Set(ctx, coredaostypes.Params{ + OversightDaoAddress: currentOversightAddr, + VotingPeriodExtensionDuration: &extDuration, + }) + require.NoError(t, err) + + updateParamsChanging := &coredaostypes.MsgUpdateParams{ + Authority: proposer, + Params: coredaostypes.Params{ + OversightDaoAddress: newOversightAddr, // different from current + VotingPeriodExtensionDuration: &extDuration, + }, + } + updateParamsSame := &coredaostypes.MsgUpdateParams{ + Authority: proposer, + Params: coredaostypes.Params{ + OversightDaoAddress: currentOversightAddr, // same as current + VotingPeriodExtensionDuration: &extDuration, + }, + } + updateParamsSameUppercase := &coredaostypes.MsgUpdateParams{ + Authority: proposer, + Params: coredaostypes.Params{ + OversightDaoAddress: strings.ToUpper(currentOversightAddr), // same address, uppercased + VotingPeriodExtensionDuration: &extDuration, + }, + } + otherMsg := govv1.NewMsgVote(addrs[2], 1, govv1.VoteOption_VOTE_OPTION_YES, "") + + tests := []struct { + name string + msgs []sdk.Msg + expectPass bool + }{ + { + name: "single MsgUpdateParams changing oversight DAO — allowed", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{updateParamsChanging}, proposer)}, + expectPass: true, + }, + { + name: "single MsgUpdateParams not changing oversight DAO — allowed", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{updateParamsSame}, proposer)}, + expectPass: true, + }, + { + name: "MsgUpdateParams (oversight change) bundled with other msg — rejected", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{updateParamsChanging, otherMsg}, proposer)}, + expectPass: false, + }, + { + name: "MsgUpdateParams (no oversight change) bundled with other msg — allowed", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{updateParamsSame, otherMsg}, proposer)}, + expectPass: true, + }, + { + name: "MsgUpdateParams (same address uppercased) bundled with other msg — allowed", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{updateParamsSameUppercase, otherMsg}, proposer)}, + expectPass: true, + }, + { + name: "multiple non-MsgUpdateParams msgs — allowed", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{otherMsg, otherMsg}, proposer)}, + expectPass: true, + }, + { + name: "non-submit-proposal message — allowed", + msgs: []sdk.Msg{otherMsg}, + expectPass: true, + }, + } + + for _, tc := range tests { + err := decorator.ValidateSubmitProposalMsgs(ctx, tc.msgs) + if tc.expectPass { + require.NoError(t, err, "expected %v to pass", tc.name) + } else { + require.Error(t, err, "expected %v to fail", tc.name) + } + } +} + +// TestGovSubmitProposalDecoratorSDKV1 checks that the decorator rejects cosmos SDK +// gov v1 proposals that bundle a coredaos MsgUpdateParams changing the oversight +// DAO address with other messages. +func TestGovSubmitProposalDecoratorSDKV1(t *testing.T) { + atomoneApp := helpers.Setup(t) + ctx := atomoneApp.NewUncachedContext(true, tmproto.Header{}) + decorator := ante.NewGovSubmitProposalDecorator(atomoneApp.AppCodec(), atomoneApp.CoreDaosKeeper) + + addrs := simtestutil.CreateRandomAccounts(3) + currentOversightAddr := addrs[0].String() + newOversightAddr := addrs[1].String() + proposer := addrs[2].String() + extDuration := 7 * 24 * time.Hour + + // Set initial coredaos params with a known oversight DAO address. + err := atomoneApp.CoreDaosKeeper.Params.Set(ctx, coredaostypes.Params{ + OversightDaoAddress: currentOversightAddr, + VotingPeriodExtensionDuration: &extDuration, + }) + require.NoError(t, err) + + updateParamsChanging := &coredaostypes.MsgUpdateParams{ + Authority: proposer, + Params: coredaostypes.Params{ + OversightDaoAddress: newOversightAddr, // different from current + VotingPeriodExtensionDuration: &extDuration, + }, + } + updateParamsSame := &coredaostypes.MsgUpdateParams{ + Authority: proposer, + Params: coredaostypes.Params{ + OversightDaoAddress: currentOversightAddr, // same as current + VotingPeriodExtensionDuration: &extDuration, + }, + } + updateParamsSameUppercase := &coredaostypes.MsgUpdateParams{ + Authority: proposer, + Params: coredaostypes.Params{ + OversightDaoAddress: strings.ToUpper(currentOversightAddr), // same address, uppercased + VotingPeriodExtensionDuration: &extDuration, + }, + } + otherMsg := govv1.NewMsgVote(addrs[2], 1, govv1.VoteOption_VOTE_OPTION_YES, "") + + tests := []struct { + name string + msgs []sdk.Msg + expectPass bool + }{ + { + name: "single MsgUpdateParams changing oversight DAO — allowed", + msgs: []sdk.Msg{mustNewSDKSubmitProposal(t, []sdk.Msg{updateParamsChanging}, proposer)}, + expectPass: true, + }, + { + name: "single MsgUpdateParams not changing oversight DAO — allowed", + msgs: []sdk.Msg{mustNewSDKSubmitProposal(t, []sdk.Msg{updateParamsSame}, proposer)}, + expectPass: true, + }, + { + name: "MsgUpdateParams (oversight change) bundled with other msg — rejected", + msgs: []sdk.Msg{mustNewSDKSubmitProposal(t, []sdk.Msg{updateParamsChanging, otherMsg}, proposer)}, + expectPass: false, + }, + { + name: "MsgUpdateParams (no oversight change) bundled with other msg — allowed", + msgs: []sdk.Msg{mustNewSDKSubmitProposal(t, []sdk.Msg{updateParamsSame, otherMsg}, proposer)}, + expectPass: true, + }, + { + name: "MsgUpdateParams (same address uppercased) bundled with other msg — allowed", + msgs: []sdk.Msg{mustNewSDKSubmitProposal(t, []sdk.Msg{updateParamsSameUppercase, otherMsg}, proposer)}, + expectPass: true, + }, + { + name: "multiple non-MsgUpdateParams msgs — allowed", + msgs: []sdk.Msg{mustNewSDKSubmitProposal(t, []sdk.Msg{otherMsg, otherMsg}, proposer)}, + expectPass: true, + }, + { + name: "non-submit-proposal message — allowed", + msgs: []sdk.Msg{otherMsg}, + expectPass: true, + }, + } + + for _, tc := range tests { + err := decorator.ValidateSubmitProposalMsgs(ctx, tc.msgs) + if tc.expectPass { + require.NoError(t, err, "expected %v to pass", tc.name) + } else { + require.Error(t, err, "expected %v to fail", tc.name) + } + } +} + +func mustNewAtomOneSubmitProposal(t *testing.T, msgs []sdk.Msg, proposer string) *govv1.MsgSubmitProposal { + t.Helper() + msg, err := govv1.NewMsgSubmitProposal(msgs, sdk.NewCoins(), proposer, "", "title", "summary") + require.NoError(t, err) + return msg +} + +func mustNewSDKSubmitProposal(t *testing.T, msgs []sdk.Msg, proposer string) *sdkgovv1.MsgSubmitProposal { + t.Helper() + msg, err := sdkgovv1.NewMsgSubmitProposal(msgs, sdk.NewCoins(), proposer, "", "title", "summary") + require.NoError(t, err) + return msg +} diff --git a/app/app.go b/app/app.go index 7e6d9ff5..c0ac68fa 100644 --- a/app/app.go +++ b/app/app.go @@ -276,6 +276,7 @@ func NewAtomOneApp( // If TxFeeChecker is nil the default ante TxFeeChecker is used TxFeeChecker: nil, DynamicfeeKeeper: app.DynamicfeeKeeper, + CoreDAOsKeeper: app.CoreDaosKeeper, }, ) if err != nil { From fc9b798b4d63477cfaa82c482bf61f54dd763fd3 Mon Sep 17 00:00:00 2001 From: Giuseppe Natale <12249307+giunatale@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:02:11 +0200 Subject: [PATCH 2/4] add CL entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a75f6041..02caf760 100644 --- a/CHANGELOG.md +++ b/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 From 3067db72d5ec5013a2ecd0ee4e53c20022c63613 Mon Sep 17 00:00:00 2001 From: Giuseppe Natale <12249307+giunatale@users.noreply.github.com> Date: Thu, 2 Apr 2026 09:27:51 +0200 Subject: [PATCH 3/4] handle authz --- ante/gov_submit_proposal_ante.go | 64 +++++++++++++++---- ante/gov_submit_proposal_ante_test.go | 89 +++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 12 deletions(-) diff --git a/ante/gov_submit_proposal_ante.go b/ante/gov_submit_proposal_ante.go index 1740699a..44521116 100644 --- a/ante/gov_submit_proposal_ante.go +++ b/ante/gov_submit_proposal_ante.go @@ -8,6 +8,7 @@ import ( "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" sdkgovv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" atomoneerrors "github.com/atomone-hub/atomone/types/errors" @@ -50,9 +51,9 @@ func (g GovSubmitProposalDecorator) AnteHandle( // ValidateSubmitProposalMsgs checks that no submit-proposal message bundles a // coredaos MsgUpdateParams that changes the oversight DAO address together with -// other proposal messages. +// other proposal messages. It also inspects authz.MsgExec wrappers func (g GovSubmitProposalDecorator) ValidateSubmitProposalMsgs(ctx sdk.Context, msgs []sdk.Msg) error { - for _, m := range msgs { + validateMsg := func(m sdk.Msg) error { var proposalMsgs []*codectypes.Any switch msg := m.(type) { case *govv1.MsgSubmitProposal: @@ -60,10 +61,25 @@ func (g GovSubmitProposalDecorator) ValidateSubmitProposalMsgs(ctx sdk.Context, case *sdkgovv1.MsgSubmitProposal: proposalMsgs = msg.GetMessages() default: - continue + return nil } + return g.validateProposalMessages(ctx, proposalMsgs) + } - if err := g.validateProposalMessages(ctx, proposalMsgs); err != nil { + for _, m := range msgs { + if execMsg, ok := m.(*authz.MsgExec); ok { + for _, anyInner := range execMsg.Msgs { + var inner sdk.Msg + if err := g.cdc.UnpackAny(anyInner, &inner); err != nil { + continue + } + if err := validateMsg(inner); err != nil { + return err + } + } + continue + } + if err := validateMsg(m); err != nil { return err } } @@ -91,24 +107,48 @@ func addressChanged(newAddr, currentAddr string) (bool, error) { return !newAccAddr.Equals(currentAccAddr), nil } +// flattenProposalMsgs recursively unpacks a list of Any-encoded messages, +// expanding authz.MsgExec wrappers so that all leaf messages are returned. +// Entries that cannot be unpacked are represented as nil (they still count +// towards the total message count for bundling detection). +func (g GovSubmitProposalDecorator) flattenProposalMsgs(anyMsgs []*codectypes.Any) []sdk.Msg { + var result []sdk.Msg + for _, anyMsg := range anyMsgs { + var msg sdk.Msg + if err := g.cdc.UnpackAny(anyMsg, &msg); err != nil { + // Cannot unpack — count as an opaque message. + result = append(result, nil) + continue + } + if execMsg, ok := msg.(*authz.MsgExec); ok { + result = append(result, g.flattenProposalMsgs(execMsg.Msgs)...) + } else { + result = append(result, msg) + } + } + return result +} + // 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. +// 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 { - // Bundling is only possible when there is more than one message. - if len(anyMsgs) <= 1 { + effectiveMsgs := g.flattenProposalMsgs(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 _, anyMsg := range anyMsgs { - var innerMsg sdk.Msg - if err := g.cdc.UnpackAny(anyMsg, &innerMsg); err != nil { - // If we cannot unpack, let other ante handlers or the msg server handle it. + for _, msg := range effectiveMsgs { + if msg == nil { + // Unpackable message — cannot be MsgUpdateParams, skip. continue } - updateParams, ok := innerMsg.(*coredaostypes.MsgUpdateParams) + updateParams, ok := msg.(*coredaostypes.MsgUpdateParams) if !ok { continue } diff --git a/ante/gov_submit_proposal_ante_test.go b/ante/gov_submit_proposal_ante_test.go index 6f5fa1c4..1ea834ba 100644 --- a/ante/gov_submit_proposal_ante_test.go +++ b/ante/gov_submit_proposal_ante_test.go @@ -11,6 +11,7 @@ import ( simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/authz" sdkgovv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" "github.com/atomone-hub/atomone/ante" @@ -211,6 +212,94 @@ func TestGovSubmitProposalDecoratorSDKV1(t *testing.T) { } } +// TestGovSubmitProposalDecoratorAuthz checks that authz.MsgExec wrappers are +// expanded when inspecting proposal messages. +func TestGovSubmitProposalDecoratorAuthz(t *testing.T) { + atomoneApp := helpers.Setup(t) + ctx := atomoneApp.NewUncachedContext(true, tmproto.Header{}) + cdc := atomoneApp.AppCodec() + decorator := ante.NewGovSubmitProposalDecorator(cdc, atomoneApp.CoreDaosKeeper) + + addrs := simtestutil.CreateRandomAccounts(3) + currentOversightAddr := addrs[0].String() + newOversightAddr := addrs[1].String() + proposer := addrs[2].String() + extDuration := 7 * 24 * time.Hour + + err := atomoneApp.CoreDaosKeeper.Params.Set(ctx, coredaostypes.Params{ + OversightDaoAddress: currentOversightAddr, + VotingPeriodExtensionDuration: &extDuration, + }) + require.NoError(t, err) + + updateParamsChanging := &coredaostypes.MsgUpdateParams{ + Authority: proposer, + Params: coredaostypes.Params{ + OversightDaoAddress: newOversightAddr, + VotingPeriodExtensionDuration: &extDuration, + }, + } + otherMsg := govv1.NewMsgVote(addrs[2], 1, govv1.VoteOption_VOTE_OPTION_YES, "") + + // mustMsgExec wraps msgs in an authz.MsgExec. + mustMsgExec := func(grantee sdk.AccAddress, msgs ...sdk.Msg) *authz.MsgExec { + execMsg := authz.NewMsgExec(grantee, msgs) + return &execMsg + } + + tests := []struct { + name string + msgs []sdk.Msg + expectPass bool + }{ + { + name: "authz MsgExec inside proposal bundles oversight change — rejected", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{ + mustMsgExec(addrs[2], updateParamsChanging, otherMsg), + }, proposer)}, + expectPass: false, + }, + { + name: "authz MsgExec for oversight change alongside other proposal msg — rejected", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{ + mustMsgExec(addrs[2], updateParamsChanging), + otherMsg, + }, proposer)}, + expectPass: false, + }, + { + name: "authz MsgExec with single oversight change — allowed", + msgs: []sdk.Msg{mustNewAtomOneSubmitProposal(t, []sdk.Msg{ + mustMsgExec(addrs[2], updateParamsChanging), + }, proposer)}, + expectPass: true, + }, + { + name: "authz MsgExec wrapping submit-proposal that bundles — rejected", + msgs: []sdk.Msg{mustMsgExec(addrs[2], + mustNewAtomOneSubmitProposal(t, []sdk.Msg{updateParamsChanging, otherMsg}, proposer), + )}, + expectPass: false, + }, + { + name: "authz MsgExec wrapping valid single-message submit-proposal — allowed", + msgs: []sdk.Msg{mustMsgExec(addrs[2], + mustNewAtomOneSubmitProposal(t, []sdk.Msg{updateParamsChanging}, proposer), + )}, + expectPass: true, + }, + } + + for _, tc := range tests { + err := decorator.ValidateSubmitProposalMsgs(ctx, tc.msgs) + if tc.expectPass { + require.NoError(t, err, "expected %v to pass", tc.name) + } else { + require.Error(t, err, "expected %v to fail", tc.name) + } + } +} + func mustNewAtomOneSubmitProposal(t *testing.T, msgs []sdk.Msg, proposer string) *govv1.MsgSubmitProposal { t.Helper() msg, err := govv1.NewMsgSubmitProposal(msgs, sdk.NewCoins(), proposer, "", "title", "summary") From 042bb716d182300c98ad63092450810bc89e33e7 Mon Sep 17 00:00:00 2001 From: Giuseppe Natale <12249307+giunatale@users.noreply.github.com> Date: Thu, 2 Apr 2026 13:50:44 +0200 Subject: [PATCH 4/4] abstract common code between vote and proposal ante --- ante/gov_ante_utils.go | 57 ++++++++++++++++++++++++++++++++ ante/gov_submit_proposal_ante.go | 47 ++------------------------ ante/gov_vote_ante.go | 30 +---------------- 3 files changed, 61 insertions(+), 73 deletions(-) create mode 100644 ante/gov_ante_utils.go diff --git a/ante/gov_ante_utils.go b/ante/gov_ante_utils.go new file mode 100644 index 00000000..02ee2e90 --- /dev/null +++ b/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 +} diff --git a/ante/gov_submit_proposal_ante.go b/ante/gov_submit_proposal_ante.go index 44521116..977a3d46 100644 --- a/ante/gov_submit_proposal_ante.go +++ b/ante/gov_submit_proposal_ante.go @@ -8,7 +8,6 @@ import ( "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" sdkgovv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" atomoneerrors "github.com/atomone-hub/atomone/types/errors" @@ -51,7 +50,7 @@ func (g GovSubmitProposalDecorator) AnteHandle( // 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 +// 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 @@ -65,25 +64,7 @@ func (g GovSubmitProposalDecorator) ValidateSubmitProposalMsgs(ctx sdk.Context, } return g.validateProposalMessages(ctx, proposalMsgs) } - - for _, m := range msgs { - if execMsg, ok := m.(*authz.MsgExec); ok { - for _, anyInner := range execMsg.Msgs { - var inner sdk.Msg - if err := g.cdc.UnpackAny(anyInner, &inner); err != nil { - continue - } - if err := validateMsg(inner); err != nil { - return err - } - } - continue - } - if err := validateMsg(m); err != nil { - return err - } - } - return nil + return iterateMsg(g.cdc, msgs, validateMsg) } // addressChanged returns true if newAddr and currentAddr refer to @@ -107,34 +88,12 @@ func addressChanged(newAddr, currentAddr string) (bool, error) { return !newAccAddr.Equals(currentAccAddr), nil } -// flattenProposalMsgs recursively unpacks a list of Any-encoded messages, -// expanding authz.MsgExec wrappers so that all leaf messages are returned. -// Entries that cannot be unpacked are represented as nil (they still count -// towards the total message count for bundling detection). -func (g GovSubmitProposalDecorator) flattenProposalMsgs(anyMsgs []*codectypes.Any) []sdk.Msg { - var result []sdk.Msg - for _, anyMsg := range anyMsgs { - var msg sdk.Msg - if err := g.cdc.UnpackAny(anyMsg, &msg); err != nil { - // Cannot unpack — count as an opaque message. - result = append(result, nil) - continue - } - if execMsg, ok := msg.(*authz.MsgExec); ok { - result = append(result, g.flattenProposalMsgs(execMsg.Msgs)...) - } else { - result = append(result, msg) - } - } - return result -} - // 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 := g.flattenProposalMsgs(anyMsgs) + effectiveMsgs := flattenAnyMsgs(g.cdc, anyMsgs) // Bundling is only possible when there is more than one effective message. if len(effectiveMsgs) <= 1 { diff --git a/ante/gov_vote_ante.go b/ante/gov_vote_ante.go index e38c4bd9..e5f9d178 100644 --- a/ante/gov_vote_ante.go +++ b/ante/gov_vote_ante.go @@ -8,7 +8,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/authz" sdkgovv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" sdkgovv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" @@ -152,32 +151,5 @@ func (g GovVoteDecorator) ValidateVoteMsgs(ctx sdk.Context, msgs []sdk.Msg) erro return nil } - validAuthz := func(execMsg *authz.MsgExec) error { - for _, v := range execMsg.Msgs { - var innerMsg sdk.Msg - if err := g.cdc.UnpackAny(v, &innerMsg); err != nil { - return errorsmod.Wrap(atomoneerrors.ErrUnauthorized, "cannot unmarshal authz exec msgs") - } - if err := validMsg(innerMsg); err != nil { - return err - } - } - - return nil - } - - for _, m := range msgs { - if msg, ok := m.(*authz.MsgExec); ok { - if err := validAuthz(msg); err != nil { - return err - } - continue - } - - // validate normal msgs - if err := validMsg(m); err != nil { - return err - } - } - return nil + return iterateMsg(g.cdc, msgs, validMsg) }