fix: Ensure the block explorer paginate is the right order

CHANGELOG.md

@@ -63,6 +63,7 @@
 - [10205](https://github.com/vegaprotocol/vega/issues/10205) - Fix for transfer discount fees.
 - [10211](https://github.com/vegaprotocol/vega/issues/10211) - Ensure infra fees don't get counted for vesting.
 - [10193](https://github.com/vegaprotocol/vega/issues/10193) - Denormalize tx_results to avoid joins with blocks when queried.
+- [10215](https://github.com/vegaprotocol/vega/issues/10215) - Rework pagination to align with the natural reverse-chronological order of the block explorer.
 
 ## 0.73.0
 

blockexplorer/api/grpc/implementation.go

@@ -81,28 +81,12 @@ func (b *blockExplorerAPI) ListTransactions(ctx context.Context, req *pb.ListTra
 	var before, after *entities.TxCursor
 	var first, last uint32
 
-	if req.First > 0 && req.Last > 0 {
-		return nil, apiError(codes.InvalidArgument, errors.New("cannot specify both first and last"))
-	}
-
-	first = b.MaxPageSizeDefault
-	if req.First > 0 {
-		first = req.First
-		if req.After == nil && req.Before != nil {
-			return nil, apiError(codes.InvalidArgument, errors.New("cannot specify before when using first"))
-		}
+	if req.Before != nil && req.After != nil && (req.First > 0 || req.Last > 0) {
+		return nil, apiError(codes.InvalidArgument, errors.New("cannot use neither limits `first`, nor `last` when both cursors `before` and `after` are set"))
 	}
 
-	if req.Last > 0 {
-		last = req.Last
-		if req.Before == nil && req.After != nil {
-			return nil, apiError(codes.InvalidArgument, errors.New("cannot specify after when using last"))
-		}
-	}
-
-	// Temporary for now, until we have fully deprecated the limit field in the request.
-	if req.Limit > 0 && req.First == 0 && req.Last == 0 {
-		first = req.Limit
+	if req.First > 0 && req.Last > 0 {
+		return nil, apiError(codes.InvalidArgument, errors.New("cannot use both limits `first` and `last` within the same query"))
 	}
 
 	if req.Before != nil {
@@ -111,6 +95,7 @@ func (b *blockExplorerAPI) ListTransactions(ctx context.Context, req *pb.ListTra
 			return nil, apiError(codes.InvalidArgument, err)
 		}
 		before = &cursor
+		last = b.MaxPageSizeDefault
 	}
 
 	if req.After != nil {
@@ -119,6 +104,37 @@ func (b *blockExplorerAPI) ListTransactions(ctx context.Context, req *pb.ListTra
 			return nil, apiError(codes.InvalidArgument, err)
 		}
 		after = &cursor
+		first = b.MaxPageSizeDefault
+	}
+
+	if before != nil && after != nil {
+		// The order of the parameters may seem odd, but this is expected as we have
+		// to keep in mind the natural order of the block-explorer is reverse-chronological.
+		// so, given transactions 4.2, 4.1, 3.2, 3.1, 2.2, when applying the window between
+		// 3.1 and 4.2, then we have to set after to 3.1 and before to 4.2.
+		// So effectively, after is the start and before is the end of the set.
+		if entities.AreValidCursorBoundaries(after, before) {
+			return nil, apiError(codes.InvalidArgument, errors.New("cursors `before` and `after` do not create a valid window"))
+		}
+	}
+
+	if req.First > 0 {
+		if req.Before != nil {
+			return nil, apiError(codes.InvalidArgument, errors.New("cannot use cursor `before` when using limit `first`"))
+		}
+		first = req.First
+	} else if req.Last > 0 {
+		if req.After != nil {
+			return nil, apiError(codes.InvalidArgument, errors.New("cannot use cursor `after` when using limit `last`"))
+		}
+		last = req.Last
+	}
+
+	// Entering this condition means there is no pagination set, so it defaults
+	// to listing the MaxPageSizeDefault newest transactions.
+	// Note, setting limits on a cursor window is not supported.
+	if !(before != nil && after != nil) && first == 0 && last == 0 {
+		first = b.MaxPageSizeDefault
 	}
 
 	transactions, err := b.store.ListTransactions(ctx,

blockexplorer/entities/transaction.go

@@ -131,3 +131,13 @@ func TxCursorFromString(s string) (TxCursor, error) {
 func (c *TxCursor) String() string {
 	return fmt.Sprintf("%d.%d", c.BlockNumber, c.TxIndex)
 }
+
+// AreValidCursorBoundaries checks if the start and end cursors creates valid
+// set boundaries for cursors, as: [start, end]
+func AreValidCursorBoundaries(start, end *TxCursor) bool {
+	if start.BlockNumber == end.BlockNumber {
+		return start.TxIndex < end.TxIndex
+	}
+
+	return start.BlockNumber < end.BlockNumber
+}

blockexplorer/store/transactions.go

@@ -19,6 +19,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"sort"
 	"strings"
 
 	"code.vegaprotocol.io/vega/blockexplorer/entities"
@@ -70,33 +71,39 @@ func (s *Store) ListTransactions(ctx context.Context,
 	args := []interface{}{}
 	predicates := []string{}
 
-	// by default we want the most recent transactions so we'll set the limit to first
-	// and sort order to desc
+	// We want the N most recent transactions, descending on block height and block
+	// index: 4.1, 3.2, 3.1, 2.2...
+	// The resulting query should already sort the rows in the right order.
 	limit := first
 	sortOrder := "desc"
 
-	// if we have a before cursor we want the results ordered earliest to latest
-	// so the limit will be set to last and sort order to asc
+	// We want the N oldest transactions, ascending on block height and block
+	// index: 1.1, 1.2, 2.1, 2.2...
+	// The resulting query should sort the rows in the chronological order. But
+	// that's necessary to apply the LIMIT clause. It will be sorted in the
+	// reverse chronological order later on.
+	if last > 0 {
+		limit = last
+		sortOrder = "asc"
+	}
+
 	if before != nil {
 		block := nextBindVar(&args, before.BlockNumber)
 		index := nextBindVar(&args, before.TxIndex)
-		predicate := fmt.Sprintf("(t.block_height, t.index) > (%s, %s)", block, index)
+		predicate := fmt.Sprintf("(t.block_height, t.index) < (%s, %s)", block, index)
 		predicates = append(predicates, predicate)
-		limit = last
-		sortOrder = "asc"
+		// We change the sorting order because we want the transactions right before
+		// the cursor, meaning older transactions.
+		sortOrder = "desc"
 	}
-
 	if after != nil {
 		block := nextBindVar(&args, after.BlockNumber)
 		index := nextBindVar(&args, after.TxIndex)
-		predicate := fmt.Sprintf("(t.block_height, t.index) < (%s, %s)", block, index)
+		predicate := fmt.Sprintf("(t.block_height, t.index) > (%s, %s)", block, index)
 		predicates = append(predicates, predicate)
-	}
-
-	// just in case we have no before cursor, but we want to have the last N transactions in the data set
-	// i.e. the earliest transactions, sorting ascending
-	if last > 0 && first == 0 && after == nil && before == nil {
-		limit = last
+		// We change the sorting order because we want the transactions right after
+		// the cursor, meaning newer transaction. That's necessary to apply the
+		// LIMIT clause.
 		sortOrder = "asc"
 	}
 
@@ -159,5 +166,16 @@ func (s *Store) ListTransactions(ctx context.Context,
 		txs = append(txs, tx)
 	}
 
+	// Make sure the results are always order in the reverse chronological order,
+	// as required.
+	// This cannot be replaced by the `order by` in the request as it's used by the
+	// pagination system.
+	sort.Slice(txs, func(i, j int) bool {
+		if txs[i].Block == txs[j].Block {
+			return txs[i].Index > txs[j].Index
+		}
+		return txs[i].Block > txs[j].Block
+	})
+
 	return txs, nil
 }

blockexplorer/store/transactions_test.go

@@ -293,27 +293,27 @@ func TestStore_ListTransactions(t *testing.T) {
 	t.Run("should return the oldest transactions when last is set without cursor", func(t *testing.T) {
 		got, err := s.ListTransactions(ctx, nil, nil, nil, nil, 0, nil, 2, nil)
 		require.NoError(t, err)
-		want := []*pb.Transaction{inserted[0], inserted[1]}
+		want := []*pb.Transaction{inserted[1], inserted[0]}
 		assert.Equal(t, want, got)
 	})
 
 	t.Run("should return the transactions after the cursor when first is set", func(t *testing.T) {
-		first := entities.TxCursor{
-			BlockNumber: 5,
+		after := entities.TxCursor{
+			BlockNumber: 2,
 			TxIndex:     1,
 		}
-		got, err := s.ListTransactions(ctx, nil, nil, nil, nil, 2, &first, 0, nil)
+		got, err := s.ListTransactions(ctx, nil, nil, nil, nil, 2, &after, 0, nil)
 		require.NoError(t, err)
-		want := []*pb.Transaction{inserted[7], inserted[6]}
+		want := []*pb.Transaction{inserted[5], inserted[4]}
 		assert.Equal(t, want, got)
 	})
 
 	t.Run("should return the transactions before the cursor when last is set", func(t *testing.T) {
-		first := entities.TxCursor{
+		before := entities.TxCursor{
 			BlockNumber: 2,
 			TxIndex:     1,
 		}
-		got, err := s.ListTransactions(ctx, nil, nil, nil, nil, 2, &first, 0, nil)
+		got, err := s.ListTransactions(ctx, nil, nil, nil, nil, 0, nil, 2, &before)
 		require.NoError(t, err)
 		want := []*pb.Transaction{inserted[2], inserted[1]}
 		assert.Equal(t, want, got)