fix: panic due to nil limit (backport #2215) (#2217)

Part of celestiaorg/celestia-app#5279

#1949 contained a
regression because a nil limitPerPage causes UnconfirmedTxs to panic.

Before that PR, the
[validatePerPage](https://github.com/rootulp/celestia-core/blob/041b5177919562c111cad22c6aabac8a62de5a3a/rpc/core/env.go#L120-L123)
would handle a nil limitPerPage by falling back to the default so that's
what I did in this PR.

Added unit tests to cover it too.<hr>This is an automatic backport of
pull request #2215 done by [Mergify](https://mergify.com).

---------

Co-authored-by: Rootul P <[email protected]>
Co-authored-by: evan-forbes <[email protected]>

Author: 3 people

rpc/core/env.go

@@ -131,6 +131,24 @@ func (env *Environment) validatePerPage(perPagePtr *int) int {
 	return perPage
 }
 
+func (env *Environment) validateUnconfirmedTxsPerPage(perPagePtr *int) int {
+	if perPagePtr == nil { // no per_page parameter
+		return defaultPerPage
+	}
+
+	perPage := *perPagePtr
+	if perPage < -1 {
+		return defaultPerPage
+	}
+	if perPage == 0 {
+		return defaultPerPage
+	}
+	if perPage > maxPerPage {
+		return maxPerPage
+	}
+	return perPage
+}
+
 // InitGenesisChunks configures the environment and should be called on service
 // startup.
 func (env *Environment) InitGenesisChunks() error {

rpc/core/env_test.go

@@ -80,3 +80,36 @@ func TestPaginationPerPage(t *testing.T) {
 	p := env.validatePerPage(nil)
 	assert.Equal(t, defaultPerPage, p)
 }
+
+func TestValidateUnconfirmedTxsPerPage(t *testing.T) {
+	env := &Environment{}
+
+	t.Run("should return default if input is nil", func(t *testing.T) {
+		got := env.validateUnconfirmedTxsPerPage(nil)
+		assert.Equal(t, defaultPerPage, got)
+	})
+
+	type testCase struct {
+		input int
+		want  int
+	}
+
+	cases := []testCase{
+		{-2, defaultPerPage},
+		{-1, -1}, // -1 is now a valid input and means query all unconfirmed txs
+		{0, defaultPerPage},
+		{1, 1},
+		{10, 10},
+		{30, 30},
+		{defaultPerPage, defaultPerPage},
+		{maxPerPage - 1, maxPerPage - 1},
+		{maxPerPage, maxPerPage},
+		{maxPerPage + 1, maxPerPage},
+	}
+	for _, c := range cases {
+		t.Run(fmt.Sprintf("should return %d if input is %d", c.want, c.input), func(t *testing.T) {
+			got := env.validateUnconfirmedTxsPerPage(&c.input)
+			assert.Equal(t, c.want, got)
+		})
+	}
+}

rpc/core/mempool.go

@@ -148,11 +148,7 @@ func (env *Environment) BroadcastTxCommit(ctx *rpctypes.Context, tx types.Tx) (*
 // More: https://docs.cometbft.com/v0.38.x/rpc/#/Info/unconfirmed_txs
 // If limitPtr == -1, it will return all the unconfirmed transactions in the mempool.
 func (env *Environment) UnconfirmedTxs(_ *rpctypes.Context, limitPtr *int) (*ctypes.ResultUnconfirmedTxs, error) {
-	limit := *limitPtr
-	if limit != -1 {
-		// reuse per_page validator
-		limit = env.validatePerPage(limitPtr)
-	}
+	limit := env.validateUnconfirmedTxsPerPage(limitPtr)
 
 	txs := env.Mempool.ReapMaxTxs(limit)
 	return &ctypes.ResultUnconfirmedTxs{

rpc/core/mempool_test.go

@@ -0,0 +1,25 @@
+package core
+
+import (
+	"testing"
+
+	mempoolmocks "github.com/cometbft/cometbft/mempool/mocks"
+	rpctypes "github.com/cometbft/cometbft/rpc/jsonrpc/types"
+	"github.com/cometbft/cometbft/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUnconfirmedTxs(t *testing.T) {
+	mempool := &mempoolmocks.Mempool{}
+	mempool.On("ReapMaxTxs", 30).Return(types.Txs{})
+	mempool.On("Size").Return(0)
+	mempool.On("SizeBytes").Return(int64(0))
+	env := &Environment{
+		Mempool: mempool,
+	}
+
+	t.Run("should not panic with nil limit", func(t *testing.T) {
+		_, err := env.UnconfirmedTxs(&rpctypes.Context{}, nil)
+		require.NoError(t, err)
+	})
+}