Code review fixes

services/horizon/internal/db2/history/account_loader.go

@@ -39,6 +39,7 @@ type AccountLoader struct {
 	sealed bool
 	set    set.Set[string]
 	ids    map[string]int64
+	stats  LoaderStats
 }
 
 var errSealed = errors.New("cannot register more entries to loader after calling Exec()")
@@ -49,6 +50,7 @@ func NewAccountLoader() *AccountLoader {
 		sealed: false,
 		set:    set.Set[string]{},
 		ids:    map[string]int64{},
+		stats:  LoaderStats{},
 	}
 }
 
@@ -99,8 +101,8 @@ func (a *AccountLoader) lookupKeys(ctx context.Context, q *Q, addresses []string
 	return nil
 }
 
-// LoaderResult describes the result of executing a history lookup id loader
-type LoaderResult struct {
+// LoaderStats describes the result of executing a history lookup id loader
+type LoaderStats struct {
 	// Total is the number of elements registered to the loader
 	Total int
 	// Inserted is the number of elements inserted into the lookup table
@@ -110,15 +112,10 @@ type LoaderResult struct {
 // Exec will look up all the history account ids for the addresses registered in the loader.
 // If there are no history account ids for a given set of addresses, Exec will insert rows
 // into the history_accounts table to establish a mapping between address and history account id.
-// Exec returns the number of addresses registered in the loader and the number of addresses
-// inserted into the history_accounts table.
-func (a *AccountLoader) Exec(ctx context.Context, session db.SessionInterface) (LoaderResult, error) {
+func (a *AccountLoader) Exec(ctx context.Context, session db.SessionInterface) error {
 	a.sealed = true
 	if len(a.set) == 0 {
-		return LoaderResult{
-			Total:    0,
-			Inserted: 0,
-		}, nil
+		return nil
 	}
 	q := &Q{session}
 	addresses := make([]string, 0, len(a.set))
@@ -127,8 +124,9 @@ func (a *AccountLoader) Exec(ctx context.Context, session db.SessionInterface) (
 	}
 
 	if err := a.lookupKeys(ctx, q, addresses); err != nil {
-		return LoaderResult{}, err
+		return err
 	}
+	a.stats.Total += len(addresses)
 
 	insert := 0
 	for _, address := range addresses {
@@ -139,10 +137,7 @@ func (a *AccountLoader) Exec(ctx context.Context, session db.SessionInterface) (
 		insert++
 	}
 	if insert == 0 {
-		return LoaderResult{
-			Total:    len(a.set),
-			Inserted: 0,
-		}, nil
+		return nil
 	}
 	addresses = addresses[:insert]
 	// sort entries before inserting rows to prevent deadlocks on acquiring a ShareLock
@@ -163,14 +158,21 @@ func (a *AccountLoader) Exec(ctx context.Context, session db.SessionInterface) (
 		},
 	)
 	if err != nil {
-		return LoaderResult{}, err
+		return err
 	}
+	a.stats.Inserted += insert
+
+	return a.lookupKeys(ctx, q, addresses)
+}
+
+// Stats returns the number of addresses registered in the loader and the number of addresses
+// inserted into the history_accounts table.
+func (a *AccountLoader) Stats() LoaderStats {
+	return a.stats
+}
 
-	err = a.lookupKeys(ctx, q, addresses)
-	return LoaderResult{
-		Total:    len(a.set),
-		Inserted: insert,
-	}, err
+func (a *AccountLoader) Name() string {
+	return "AccountLoader"
 }
 
 type bulkInsertField struct {

services/horizon/internal/db2/history/account_loader_test.go

@@ -31,12 +31,12 @@ func TestAccountLoader(t *testing.T) {
 		assert.Equal(t, future, duplicateFuture)
 	}
 
-	result, err := loader.Exec(context.Background(), session)
+	err := loader.Exec(context.Background(), session)
 	assert.NoError(t, err)
-	assert.Equal(t, LoaderResult{
+	assert.Equal(t, LoaderStats{
 		Total:    100,
 		Inserted: 100,
-	}, result)
+	}, loader.Stats())
 	assert.Panics(t, func() {
 		loader.GetFuture(keypair.MustRandom().Address())
 	})

services/horizon/internal/db2/history/asset_loader.go

@@ -60,6 +60,7 @@ type AssetLoader struct {
 	sealed bool
 	set    set.Set[AssetKey]
 	ids    map[AssetKey]int64
+	stats  LoaderStats
 }
 
 // NewAssetLoader will construct a new AssetLoader instance.
@@ -68,6 +69,7 @@ func NewAssetLoader() *AssetLoader {
 		sealed: false,
 		set:    set.Set[AssetKey]{},
 		ids:    map[AssetKey]int64{},
+		stats:  LoaderStats{},
 	}
 }
 
@@ -131,15 +133,10 @@ func (a *AssetLoader) lookupKeys(ctx context.Context, q *Q, keys []AssetKey) err
 // Exec will look up all the history asset ids for the assets registered in the loader.
 // If there are no history asset ids for a given set of assets, Exec will insert rows
 // into the history_assets table.
-// Exec returns the number of assets registered in the loader and the number of assets
-// inserted into the history_assets table.
-func (a *AssetLoader) Exec(ctx context.Context, session db.SessionInterface) (LoaderResult, error) {
+func (a *AssetLoader) Exec(ctx context.Context, session db.SessionInterface) error {
 	a.sealed = true
 	if len(a.set) == 0 {
-		return LoaderResult{
-			Total:    0,
-			Inserted: 0,
-		}, nil
+		return nil
 	}
 	q := &Q{session}
 	keys := make([]AssetKey, 0, len(a.set))
@@ -148,8 +145,9 @@ func (a *AssetLoader) Exec(ctx context.Context, session db.SessionInterface) (Lo
 	}
 
 	if err := a.lookupKeys(ctx, q, keys); err != nil {
-		return LoaderResult{}, err
+		return err
 	}
+	a.stats.Total += len(keys)
 
 	assetTypes := make([]string, 0, len(a.set)-len(a.ids))
 	assetCodes := make([]string, 0, len(a.set)-len(a.ids))
@@ -171,10 +169,7 @@ func (a *AssetLoader) Exec(ctx context.Context, session db.SessionInterface) (Lo
 		insert++
 	}
 	if insert == 0 {
-		return LoaderResult{
-			Total:    len(a.set),
-			Inserted: 0,
-		}, nil
+		return nil
 	}
 	keys = keys[:insert]
 
@@ -202,14 +197,21 @@ func (a *AssetLoader) Exec(ctx context.Context, session db.SessionInterface) (Lo
 		},
 	)
 	if err != nil {
-		return LoaderResult{}, err
+		return err
 	}
+	a.stats.Inserted += insert
+
+	return a.lookupKeys(ctx, q, keys)
+}
+
+// Stats returns the number of assets registered in the loader and the number of assets
+// inserted into the history_assets table.
+func (a *AssetLoader) Stats() LoaderStats {
+	return a.stats
+}
 
-	err = a.lookupKeys(ctx, q, keys)
-	return LoaderResult{
-		Total:    len(a.set),
-		Inserted: insert,
-	}, err
+func (a *AssetLoader) Name() string {
+	return "AssetLoader"
 }
 
 // AssetLoaderStub is a stub wrapper around AssetLoader which allows

services/horizon/internal/db2/history/asset_loader_test.go

@@ -76,12 +76,12 @@ func TestAssetLoader(t *testing.T) {
 		assert.Equal(t, future, duplicateFuture)
 	}
 
-	result, err := loader.Exec(context.Background(), session)
+	err := loader.Exec(context.Background(), session)
 	assert.NoError(t, err)
-	assert.Equal(t, LoaderResult{
+	assert.Equal(t, LoaderStats{
 		Total:    100,
 		Inserted: 100,
-	}, result)
+	}, loader.Stats())
 	assert.Panics(t, func() {
 		loader.GetFuture(AssetKey{Type: "invalid"})
 	})

services/horizon/internal/db2/history/claimable_balance_loader.go

@@ -34,6 +34,7 @@ type ClaimableBalanceLoader struct {
 	sealed bool
 	set    set.Set[string]
 	ids    map[string]int64
+	stats  LoaderStats
 }
 
 // NewClaimableBalanceLoader will construct a new ClaimableBalanceLoader instance.
@@ -42,6 +43,7 @@ func NewClaimableBalanceLoader() *ClaimableBalanceLoader {
 		sealed: false,
 		set:    set.Set[string]{},
 		ids:    map[string]int64{},
+		stats:  LoaderStats{},
 	}
 }
 
@@ -95,15 +97,10 @@ func (a *ClaimableBalanceLoader) lookupKeys(ctx context.Context, q *Q, ids []str
 // Exec will look up all the internal history ids for the claimable balances registered in the loader.
 // If there are no internal ids for a given set of claimable balances, Exec will insert rows
 // into the history_claimable_balances table.
-// Exec returns the number of claimable balances registered in the loader and the number of claimable balances
-// inserted into the history_claimable_balances table.
-func (a *ClaimableBalanceLoader) Exec(ctx context.Context, session db.SessionInterface) (LoaderResult, error) {
+func (a *ClaimableBalanceLoader) Exec(ctx context.Context, session db.SessionInterface) error {
 	a.sealed = true
 	if len(a.set) == 0 {
-		return LoaderResult{
-			Total:    0,
-			Inserted: 0,
-		}, nil
+		return nil
 	}
 	q := &Q{session}
 	ids := make([]string, 0, len(a.set))
@@ -112,8 +109,9 @@ func (a *ClaimableBalanceLoader) Exec(ctx context.Context, session db.SessionInt
 	}
 
 	if err := a.lookupKeys(ctx, q, ids); err != nil {
-		return LoaderResult{}, err
+		return err
 	}
+	a.stats.Total += len(ids)
 
 	insert := 0
 	for _, id := range ids {
@@ -124,10 +122,7 @@ func (a *ClaimableBalanceLoader) Exec(ctx context.Context, session db.SessionInt
 		insert++
 	}
 	if insert == 0 {
-		return LoaderResult{
-			Total:    len(a.set),
-			Inserted: 0,
-		}, nil
+		return nil
 	}
 	ids = ids[:insert]
 	// sort entries before inserting rows to prevent deadlocks on acquiring a ShareLock
@@ -148,12 +143,19 @@ func (a *ClaimableBalanceLoader) Exec(ctx context.Context, session db.SessionInt
 		},
 	)
 	if err != nil {
-		return LoaderResult{}, err
+		return err
 	}
+	a.stats.Inserted += insert
+
+	return a.lookupKeys(ctx, q, ids)
+}
+
+// Stats returns the number of claimable balances registered in the loader and the number of claimable balances
+// inserted into the history_claimable_balances table.
+func (a *ClaimableBalanceLoader) Stats() LoaderStats {
+	return a.stats
+}
 
-	err = a.lookupKeys(ctx, q, ids)
-	return LoaderResult{
-		Total:    len(a.set),
-		Inserted: insert,
-	}, err
+func (a *ClaimableBalanceLoader) Name() string {
+	return "ClaimableBalanceLoader"
 }

services/horizon/internal/db2/history/claimable_balance_loader_test.go

@@ -40,12 +40,12 @@ func TestClaimableBalanceLoader(t *testing.T) {
 		assert.Equal(t, future, duplicateFuture)
 	}
 
-	result, err := loader.Exec(context.Background(), session)
+	err := loader.Exec(context.Background(), session)
 	assert.NoError(t, err)
-	assert.Equal(t, LoaderResult{
+	assert.Equal(t, LoaderStats{
 		Total:    100,
 		Inserted: 100,
-	}, result)
+	}, loader.Stats())
 	assert.Panics(t, func() {
 		loader.GetFuture("not-present")
 	})

services/horizon/internal/db2/history/effect_batch_insert_builder_test.go

@@ -38,8 +38,7 @@ func TestAddEffect(t *testing.T) {
 	)
 	tt.Assert.NoError(err)
 
-	_, err = accountLoader.Exec(tt.Ctx, q)
-	tt.Assert.NoError(err)
+	tt.Assert.NoError(accountLoader.Exec(tt.Ctx, q))
 	tt.Assert.NoError(builder.Exec(tt.Ctx, q))
 	tt.Assert.NoError(q.Commit())
 

services/horizon/internal/db2/history/effect_test.go

@@ -42,8 +42,7 @@ func TestEffectsForLiquidityPool(t *testing.T) {
 		details,
 	))
 
-	_, err = accountLoader.Exec(tt.Ctx, q)
-	tt.Assert.NoError(err)
+	tt.Assert.NoError(accountLoader.Exec(tt.Ctx, q))
 	tt.Assert.NoError(builder.Exec(tt.Ctx, q))
 
 	// Insert Liquidity Pool history
@@ -52,8 +51,7 @@ func TestEffectsForLiquidityPool(t *testing.T) {
 
 	operationBuilder := q.NewOperationLiquidityPoolBatchInsertBuilder()
 	tt.Assert.NoError(operationBuilder.Add(opID, lpLoader.GetFuture(liquidityPoolID)))
-	_, err = lpLoader.Exec(tt.Ctx, q)
-	tt.Assert.NoError(err)
+	tt.Assert.NoError(lpLoader.Exec(tt.Ctx, q))
 	tt.Assert.NoError(operationBuilder.Exec(tt.Ctx, q))
 
 	tt.Assert.NoError(q.Commit())
@@ -154,8 +152,7 @@ func TestEffectsForTrustlinesSponsorshipEmptyAssetType(t *testing.T) {
 			bytes,
 		))
 	}
-	_, err := accountLoader.Exec(tt.Ctx, q)
-	tt.Require.NoError(err)
+	tt.Require.NoError(accountLoader.Exec(tt.Ctx, q))
 	tt.Require.NoError(builder.Exec(tt.Ctx, q))
 	tt.Assert.NoError(q.Commit())
 

services/horizon/internal/db2/history/fee_bump_scenario.go

@@ -300,8 +300,7 @@ func FeeBumpScenario(tt *test.T, q *Q, successful bool) FeeBumpFixture {
 	)
 
 	tt.Assert.NoError(err)
-	_, err = accountLoader.Exec(ctx, q.SessionInterface)
-	tt.Assert.NoError(err)
+	tt.Assert.NoError(accountLoader.Exec(ctx, q.SessionInterface))
 	tt.Assert.NoError(effectBuilder.Exec(ctx, q.SessionInterface))
 
 	tt.Assert.NoError(q.Commit())