resolve comments left by @walldiss

Author: vgonkivs

share/shwap/p2p/shrex/metrics.go

@@ -107,7 +107,7 @@ func (m *Metrics) observePayloadRequested(
 	// this attribute can be used to filter successful throughput vs total
 	incomplete := status != statusSuccess
 
-	m.payloadServed.Add(ctx, int64(payloadSize), metric.WithAttributes(
+	m.payloadRequested.Add(ctx, int64(payloadSize), metric.WithAttributes(
 		attribute.String("protocol", requestName),
 		attribute.Bool("incomplete", incomplete),
 	))

share/shwap/p2p/shrex/shrex_getter/shrex.go

@@ -99,13 +99,12 @@ func (sg *Getter) GetSamples(
 	ctx context.Context,
 	header *header.ExtendedHeader,
 	coords []shwap.SampleCoords,
-) ([]shwap.Sample, error) {
+) (_ []shwap.Sample, err error) {
 	// short circuit if the data root is empty
 	if header.DAH.Equals(share.EmptyEDSRoots()) {
 		return []shwap.Sample{}, nil
 	}
 
-	var err error
 	ctx, span := tracer.Start(ctx, "shrex/get-samples")
 	defer func() {
 		utils.SetStatusAndEnd(span, err)
@@ -435,13 +434,13 @@ func (sg *Getter) executeRequest(
 		switch {
 		case getErr == nil:
 			setStatus(peers.ResultNoop)
-			sg.metrics.recordAttempts(ctx, reqType, attempt, true)
 			verifyErr := handle()
 			if verifyErr != nil {
 				getErr = verifyErr
 				setStatus(peers.ResultBlacklistPeer)
 				break
 			}
+			sg.metrics.recordAttempts(ctx, reqType, attempt, true)
 			setStatus(peers.ResultNoop)
 			return nil
 		case errors.Is(getErr, context.DeadlineExceeded),

share/shwap/p2p/shrex/shrex_getter/shrex_test.go

@@ -2,6 +2,7 @@ package shrex_getter //nolint:stylecheck // underscore in pkg name will be fixed
 
 import (
 	"context"
+	"fmt"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -241,7 +242,7 @@ func TestShrexGetter(t *testing.T) {
 	})
 
 	t.Run("Samples_Available", func(t *testing.T) {
-		ctx, cancel := context.WithTimeout(ctx, time.Second)
+		ctx, cancel := context.WithTimeout(ctx, time.Minute)
 		t.Cleanup(cancel)
 
 		// generate test data
@@ -257,20 +258,24 @@ func TestShrexGetter(t *testing.T) {
 			Height:   height,
 		})
 
-		coords := []shwap.SampleCoords{
-			{Row: 0, Col: 0},
-			{Row: 0, Col: 1},
-			{Row: 0, Col: 2},
-			{Row: 0, Col: 3},
+		odsSize := len(eh.DAH.RowRoots) / 2
+		coords := make([]shwap.SampleCoords, odsSize*odsSize)
+		total := 0
+		for i := 0; i < odsSize; i++ {
+			for j := 0; j < odsSize; j++ {
+				coords[total] = shwap.SampleCoords{Row: i, Col: j}
+				total++
+			}
 		}
+
 		got, err := getter.GetSamples(ctx, eh, coords)
 		require.NoError(t, err)
 		assert.Len(t, got, len(coords))
 
-		rowShares := randEDS.Row(0)
+		odsShares := randEDS.FlattenedODS()
 		require.NoError(t, err)
 		for i, samples := range got {
-			assert.Equal(t, rowShares[i], samples.ToBytes())
+			assert.Equal(t, odsShares[i], samples.ToBytes(), fmt.Sprintf("sample %d", i))
 		}
 	})
 
@@ -343,13 +348,16 @@ func TestShrexGetter(t *testing.T) {
 			Height:   height,
 		})
 
-		row, err := getter.GetRow(ctx, eh, 0)
-		require.NoError(t, err)
-		shrs, err := row.Shares()
-		require.NoError(t, err)
-		edsRow := randEDS.Row(0)
-		for i, shr := range shrs {
-			require.Equal(t, edsRow[i], shr.ToBytes())
+		odsSize := len(roots.RowRoots)
+		for i := 0; i < odsSize; i++ {
+			row, err := getter.GetRow(ctx, eh, i)
+			require.NoError(t, err)
+			shrs, err := row.Shares()
+			require.NoError(t, err)
+			edsRow := randEDS.Row(uint(i))
+			for j, shr := range shrs {
+				require.Equal(t, edsRow[j], shr.ToBytes())
+			}
 		}
 	})
 

share/shwap/range_namespace_data_id.go

@@ -170,7 +170,6 @@ func (rngid RangeNamespaceDataID) ResponseReader(ctx context.Context, acc Access
 		return nil, fmt.Errorf("getting rngdata from accessor: %w", err)
 	}
 
-	// TODO(@vgonkivs): This is a temporary solution that will be reworked
 	buf := &bytes.Buffer{}
 	_, err = rngdata.WriteTo(buf)
 	if err != nil {

share/shwap/row.go

@@ -272,9 +272,8 @@ func (rid RowID) ResponseReader(ctx context.Context, acc Accessor) (io.Reader, e
 	if err != nil {
 		return nil, fmt.Errorf("getting half row from accessor: %w", err)
 	}
-	row := halfRow.ToRow()
 
-	// TODO(@vgonkivs): This is a temporary solution that will be reworked
+	row := halfRow.ToRow()
 	buf := &bytes.Buffer{}
 	_, err = row.WriteTo(buf)
 	if err != nil {

share/shwap/sample_id.go

@@ -207,7 +207,6 @@ func (sid SampleID) ResponseReader(ctx context.Context, acc Accessor) (io.Reader
 		return nil, fmt.Errorf("getting sample from accessor: %w", err)
 	}
 
-	// TODO(@vgonkivs): This is a temporary solution that will be reworked
 	buf := &bytes.Buffer{}
 	_, err = sample.WriteTo(buf)
 	if err != nil {