refactor: remove defer in loop (#20223)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

store/snapshots/store.go

@@ -3,6 +3,7 @@ package snapshots
 import (
 	"crypto/sha256"
 	"encoding/binary"
+	"fmt"
 	"hash"
 	"io"
 	"math"
@@ -140,6 +141,7 @@ func (s *Store) Load(height uint64, format uint32) (*types.Snapshot, <-chan io.R
 	}
 
 	ch := make(chan io.ReadCloser)
+
 	go func() {
 		defer close(ch)
 		for i := uint32(0); i < snapshot.Chunks; i++ {
@@ -150,14 +152,19 @@ func (s *Store) Load(height uint64, format uint32) (*types.Snapshot, <-chan io.R
 				_ = pw.CloseWithError(err)
 				return
 			}
-			defer chunk.Close()
-			_, err = io.Copy(pw, chunk)
+			err = func() error {
+				defer chunk.Close()
+
+				if _, err := io.Copy(pw, chunk); err != nil {
+					_ = pw.CloseWithError(err)
+					return fmt.Errorf("failed to copy chunk %d: %w", i, err)
+				}
+
+				return pw.Close()
+			}()
 			if err != nil {
-				_ = pw.CloseWithError(err)
 				return
 			}
-			chunk.Close()
-			pw.Close()
 		}
 	}()
 

store/v2/commitment/store.go

@@ -411,7 +411,9 @@ loop:
 				if err := importer.Commit(); err != nil {
 					return snapshotstypes.SnapshotItem{}, fmt.Errorf("failed to commit importer: %w", err)
 				}
-				importer.Close()
+				if err := importer.Close(); err != nil {
+					return snapshotstypes.SnapshotItem{}, fmt.Errorf("failed to close importer: %w", err)
+				}
 			}
 
 			storeKey = []byte(item.Store.Name)

store/v2/migration/manager.go

@@ -181,11 +181,21 @@ func (m *Manager) writeChangeset() error {
 		}
 
 		batch := m.db.NewBatch()
-		if err := batch.Set(csKey, csBytes); err != nil {
-			return fmt.Errorf("failed to write changeset to db.Batch: %w", err)
-		}
-		if err := batch.Write(); err != nil {
-			return fmt.Errorf("failed to write changeset to db: %w", err)
+		// Invoking this code in a closure so that defer is called immediately on return
+		// yet not in the for-loop which can leave resource lingering.
+		err = func() error {
+			defer batch.Close()
+
+			if err := batch.Set(csKey, csBytes); err != nil {
+				return fmt.Errorf("failed to write changeset to db.Batch: %w", err)
+			}
+			if err := batch.Write(); err != nil {
+				return fmt.Errorf("failed to write changeset to db: %w", err)
+			}
+			return nil
+		}()
+		if err != nil {
+			return err
 		}
 		batch.Close()
 	}

store/v2/storage/storage_test_suite.go

@@ -256,8 +256,6 @@ func (s *StorageTestSuite) TestDatabase_Iterator() {
 		itr, err := db.Iterator(storeKey1Bytes, v, []byte("key000"), nil)
 		s.Require().NoError(err)
 
-		defer itr.Close()
-
 		var i, count int
 		for ; itr.Valid(); itr.Next() {
 			s.Require().Equal([]byte(fmt.Sprintf("key%03d", i)), itr.Key(), string(itr.Key()))
@@ -271,6 +269,9 @@ func (s *StorageTestSuite) TestDatabase_Iterator() {
 
 		// seek past domain, which should make the iterator invalid and produce an error
 		s.Require().False(itr.Valid())
+
+		err = itr.Close()
+		s.Require().NoError(err, "Failed to close iterator")
 	}
 
 	// iterator with a start and end domain over multiple versions
@@ -278,8 +279,6 @@ func (s *StorageTestSuite) TestDatabase_Iterator() {
 		itr2, err := db.Iterator(storeKey1Bytes, v, []byte("key010"), []byte("key019"))
 		s.Require().NoError(err)
 
-		defer itr2.Close()
-
 		i, count := 10, 0
 		for ; itr2.Valid(); itr2.Next() {
 			s.Require().Equal([]byte(fmt.Sprintf("key%03d", i)), itr2.Key())
@@ -293,6 +292,11 @@ func (s *StorageTestSuite) TestDatabase_Iterator() {
 
 		// seek past domain, which should make the iterator invalid and produce an error
 		s.Require().False(itr2.Valid())
+
+		err = itr2.Close()
+		if err != nil {
+			return
+		}
 	}
 
 	// start must be <= end

tests/integration/auth/client/cli/suite_test.go

@@ -207,7 +207,6 @@ func (s *CLITestSuite) TestCLISignBatchTotalFees() {
 					sdk.NewCoins(sendTokens), clitestutil.TestTxConfig{GenOnly: true})
 				s.Require().NoError(err)
 				txFile := testutil.WriteToNewTempFile(s.T(), tx.String()+"\n")
-				defer txFile.Close()
 				txFiles[i] = txFile.Name()
 
 				unsignedTx, err := txCfg.TxJSONDecoder()(tx.Bytes())
@@ -215,6 +214,8 @@ func (s *CLITestSuite) TestCLISignBatchTotalFees() {
 				txBuilder, err := txCfg.WrapTxBuilder(unsignedTx)
 				s.Require().NoError(err)
 				expectedBatchedTotalFee += txBuilder.GetTx().GetFee().AmountOf(tc.denom).Int64()
+				err = txFile.Close()
+				s.NoError(err)
 			}
 
 			// Test batch sign

x/group/keeper/invariants.go

@@ -55,7 +55,6 @@ func GroupTotalWeightInvariantHelper(ctx sdk.Context, storeService storetypes.KV
 			msg += fmt.Sprintf("LoadNext failure on group table iterator\n%v\n", err)
 			return msg, broken
 		}
-
 		groups[groupInfo.Id] = groupInfo
 	}
 
@@ -63,6 +62,7 @@ func GroupTotalWeightInvariantHelper(ctx sdk.Context, storeService storetypes.KV
 	sort.Slice(groupByIDs, func(i, j int) bool {
 		return groupByIDs[i] < groupByIDs[j]
 	})
+
 	for _, groupID := range groupByIDs {
 		groupInfo := groups[groupID]
 		membersWeight, err := groupmath.NewNonNegativeDecFromString("0")
@@ -71,36 +71,39 @@ func GroupTotalWeightInvariantHelper(ctx sdk.Context, storeService storetypes.KV
 			return msg, broken
 		}
 
-		memIt, err := groupMemberByGroupIndex.Get(kvStore, groupInfo.Id)
+		err = func() error {
+			memIt, err := groupMemberByGroupIndex.Get(kvStore, groupInfo.Id)
+			if err != nil {
+				return fmt.Errorf("error while returning group member iterator for group with ID %d\n%w", groupInfo.Id, err)
+			}
+			defer memIt.Close()
+
+			for {
+				var groupMember group.GroupMember
+				_, err = memIt.LoadNext(&groupMember)
+				if errors.ErrORMIteratorDone.Is(err) {
+					break
+				}
+				if err != nil {
+					return fmt.Errorf("LoadNext failure on member table iterator\n%w", err)
+				}
+
+				curMemWeight, err := groupmath.NewPositiveDecFromString(groupMember.GetMember().GetWeight())
+				if err != nil {
+					return fmt.Errorf("error while parsing non-nengative decimal for group member %s\n%w", groupMember.Member.Address, err)
+				}
+
+				membersWeight, err = groupmath.Add(membersWeight, curMemWeight)
+				if err != nil {
+					return fmt.Errorf("decimal addition error while adding group member voting weight to total voting weight\n%w", err)
+				}
+			}
+			return nil
+		}()
 		if err != nil {
-			msg += fmt.Sprintf("error while returning group member iterator for group with ID %d\n%v\n", groupInfo.Id, err)
+			msg += err.Error() + "\n"
 			return msg, broken
 		}
-		defer memIt.Close()
-
-		for {
-			var groupMember group.GroupMember
-			_, err = memIt.LoadNext(&groupMember)
-			if errors.ErrORMIteratorDone.Is(err) {
-				break
-			}
-			if err != nil {
-				msg += fmt.Sprintf("LoadNext failure on member table iterator\n%v\n", err)
-				return msg, broken
-			}
-
-			curMemWeight, err := groupmath.NewPositiveDecFromString(groupMember.GetMember().GetWeight())
-			if err != nil {
-				msg += fmt.Sprintf("error while parsing non-nengative decimal for group member %s\n%v\n", groupMember.Member.Address, err)
-				return msg, broken
-			}
-
-			membersWeight, err = groupmath.Add(membersWeight, curMemWeight)
-			if err != nil {
-				msg += fmt.Sprintf("decimal addition error while adding group member voting weight to total voting weight\n%v\n", err)
-				return msg, broken
-			}
-		}
 
 		groupWeight, err := groupmath.NewNonNegativeDecFromString(groupInfo.GetTotalWeight())
 		if err != nil {