fix: Ensure the block explorer paginate is the right order

CHANGELOG.md

@@ -64,6 +64,7 @@
 - [10211](https://github.com/vegaprotocol/vega/issues/10211) - Ensure infra fees don't get counted for vesting.
 - [10217](https://github.com/vegaprotocol/vega/issues/10217) - Game ID for reward entity should be optional
 - [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

@@ -71,33 +71,43 @@ 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
-	limit := first
+	limit := uint32(0)
+
 	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
+	if first > 0 {
+		// 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"
+	} else if last > 0 {
+		// 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.
+		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"
 	}
 
@@ -143,7 +153,9 @@ func (s *Store) ListTransactions(ctx context.Context,
 	}
 
 	query = fmt.Sprintf("%s ORDER BY t.block_height %s, t.index %s", query, sortOrder, sortOrder)
-	query = fmt.Sprintf("%s LIMIT %d", query, limit)
+	if limit != 0 {
+		query = fmt.Sprintf("%s LIMIT %d", query, limit)
+	}
 
 	var rows []entities.TxResultRow
 	if err := pgxscan.Select(ctx, s.pool, &rows, query, args...); err != nil {
@@ -160,8 +172,10 @@ func (s *Store) ListTransactions(ctx context.Context,
 		txs = append(txs, tx)
 	}
 
-	// make sure the results are always order in the same direction, i.e. newest first, regardless of the order of the
-	// results from the database.
+	// 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

blockexplorer/store/transactions_test.go

@@ -123,8 +123,8 @@ func addTestTxResults(ctx context.Context, t *testing.T, txResults ...txResult)
 	rows := make([]*pb.Transaction, 0, len(txResults))
 	blockIDs := make(map[int64]int64)
 
-	blockSQL := `insert into blocks (height, chain_id, created_at) values ($1, $2, $3) on conflict (height, chain_id) do update set created_at = EXCLUDED.created_at returning rowid`
-	resultSQL := `insert into tx_results (block_id, index, created_at, tx_hash, tx_result, submitter, cmd_type) values ($1, $2, $3, $4, $5, $6, $7) returning rowid`
+	blockSQL := `INSERT INTO blocks (height, chain_id, created_at) VALUES ($1, $2, $3) ON CONFLICT (height, chain_id) DO UPDATE SET created_at = EXCLUDED.created_at RETURNING rowid`
+	resultSQL := `INSERT INTO tx_results (block_id, index, created_at, tx_hash, tx_result, submitter, cmd_type) VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING rowid`
 
 	for _, txr := range txResults {
 		var blockID int64
@@ -295,24 +295,39 @@ func TestStore_ListTransactions(t *testing.T) {
 	})
 
 	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)
 	})
+
+	t.Run("should return the transactions before the cursor when last is set", func(t *testing.T) {
+		before := entities.TxCursor{
+			BlockNumber: 5,
+			TxIndex:     1,
+		}
+		after := entities.TxCursor{
+			BlockNumber: 2,
+			TxIndex:     2,
+		}
+		got, err := s.ListTransactions(ctx, nil, nil, nil, nil, 0, &after, 0, &before)
+		require.NoError(t, err)
+		want := []*pb.Transaction{inserted[7], inserted[6], inserted[5]}
+		assert.Equal(t, want, got)
+	})
 }