fix(x/accounts/lockup): prevent double withdraw (backport #21619)

x/accounts/defaults/lockup/lockup.go

@@ -5,6 +5,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
+	"slices"
 	"time"
 
 	"github.com/cosmos/gogoproto/proto"
@@ -254,6 +256,10 @@ func (bva *BaseLockup) SendCoins(
 
 	hs := bva.headerService.HeaderInfo(ctx)
 
+	if err := msg.Amount.Validate(); err != nil {
+		return nil, err
+	}
+
 	lockedCoins, err := getLockedCoinsFunc(ctx, hs.Time, msg.Amount.Denoms()...)
 	if err != nil {
 		return nil, err
@@ -294,15 +300,21 @@ func (bva *BaseLockup) WithdrawUnlockedCoins(
 		return nil, err
 	}
 
+	// deduplicate the denoms
+	denoms := make(map[string]struct{})
+	for _, denom := range msg.Denoms {
+		denoms[denom] = struct{}{}
+	}
+	uniqueDenoms := slices.Collect(maps.Keys(denoms))
+
 	hs := bva.headerService.HeaderInfo(ctx)
-	lockedCoins, err := getLockedCoinsFunc(ctx, hs.Time, msg.Denoms...)
+	lockedCoins, err := getLockedCoinsFunc(ctx, hs.Time, uniqueDenoms...)
 	if err != nil {
 		return nil, err
 	}
 
 	amount := sdk.Coins{}
-
-	for _, denom := range msg.Denoms {
+	for _, denom := range uniqueDenoms {
 		balance, err := bva.getBalance(ctx, fromAddress, denom)
 		if err != nil {
 			return nil, err

x/accounts/defaults/lockup/periodic_locking_account_test.go

@@ -231,12 +231,16 @@ func TestPeriodicAccountWithdrawUnlockedCoins(t *testing.T) {
 		Time: startTime.Add(time.Minute * 1),
 	})
 
-	_, err = acc.WithdrawUnlockedCoins(sdkCtx, &lockuptypes.MsgWithdraw{
+	// withdraw unlocked token
+	resp, err := acc.WithdrawUnlockedCoins(sdkCtx, &lockuptypes.MsgWithdraw{
 		Withdrawer: "owner",
 		ToAddress:  "receiver",
-		Denoms:     []string{"test"},
+		Denoms:     []string{"test", "test"}, // duplicate tokens should be ignored
 	})
 	require.NoError(t, err)
+	require.Equal(t, resp.AmountReceived.Len(), 1)
+	require.Equal(t, resp.AmountReceived, sdk.NewCoins(sdk.NewCoin("test", math.NewInt(5))))
+	require.Equal(t, resp.Receiver, "receiver")
 }
 
 func TestPeriodicAccountGetLockCoinInfo(t *testing.T) {

x/accounts/proto/cosmos/accounts/defaults/multisig/v1/multisig.proto

@@ -3,39 +3,44 @@ syntax = "proto3";
 package cosmos.accounts.defaults.multisig.v1;
 
 import "google/protobuf/any.proto";
-import "cosmos/msg/v1/msg.proto";
 import "cosmos_proto/cosmos.proto";
 
 option go_package = "cosmossdk.io/x/accounts/defaults/multisig/v1";
 
 // MsgInit is used to initialize a multisig account.
 message MsgInit {
   repeated Member members = 1;
-  Config          Config  = 2;
+  Config          config  = 2;
 }
 
 // MsgInitResponse is the response returned after account initialization.
 message MsgInitResponse {}
 
+// MsgCreateProposal creates a new proposal.
 message MsgCreateProposal {
   Proposal proposal = 1;
 }
 
+// MsgCreateProposalResponse is the response returned after creating a proposal.
 message MsgCreateProposalResponse {
   uint64 proposal_id = 1;
 }
 
+// MsgVote is used to vote on a proposal.
 message MsgVote {
   uint64     proposal_id = 1;
   VoteOption vote        = 2;
 }
 
+// MsgVoteResponse is the response returned after voting on a proposal.
 message MsgVoteResponse {}
 
+// MsgExecuteProposal is used to execute a proposal.
 message MsgExecuteProposal {
   uint64 proposal_id = 1;
 }
 
+// MsgExecuteProposalResponse is the response returned after executing a proposal.
 message MsgExecuteProposalResponse {
   repeated google.protobuf.Any responses = 1;
 }
@@ -46,11 +51,13 @@ message MsgUpdateConfig {
   repeated Member update_members = 1;
 
   // not all fields from Config can be changed
-  Config Config = 2;
+  Config config = 2;
 }
 
+// MsgUpdateConfigResponse is the response returned after updating the config.
 message MsgUpdateConfigResponse {}
 
+// Member defines the member of the multisig account.
 message Member {
   string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
   uint64 weight  = 2;
@@ -59,6 +66,7 @@ message Member {
 // when aggregating on-chain I can only use the address
 // off-chain would send a tx directly, won't create a proposal.
 
+// Config defines the configuration of the multisig account.
 message Config {
   int64 threshold = 1;
 
@@ -74,6 +82,7 @@ message Config {
   bool early_execution = 5;
 }
 
+// Proposal defines the structure of a proposal.
 message Proposal {
   string   title                        = 1;
   string   summary                      = 2;
@@ -94,19 +103,22 @@ message QuerySequenceResponse {
   uint64 sequence = 1;
 }
 
+// QueryConfig is the request for the account config.
 message QueryConfig {}
 
 // QueryConfigResponse returns the config of the account.
 message QueryConfigResponse {
   repeated Member members = 1;
 
-  Config Config = 2;
+  Config config = 2;
 }
 
+// QueryProposal is the request for a proposal.
 message QueryProposal {
   uint64 proposal_id = 1;
 }
 
+// QueryProposalResponse returns the proposal.
 message QueryProposalResponse {
   Proposal proposal = 1;
 }