Add functionality to retrieve all pending items from pools (#7530)

* Allow slashing pools to retrieve all items

* Add functionality to exits too to retrieve all exits

* Rename to noLimit

* ndo err

* Fix tests

* Fix test

* Fix test again

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

beacon-chain/operations/slashings/BUILD.bazel

@@ -19,7 +19,6 @@ go_library(
         "//beacon-chain/core/blocks:go_default_library",
         "//beacon-chain/core/helpers:go_default_library",
         "//beacon-chain/state:go_default_library",
-        "//shared/mathutil:go_default_library",
         "//shared/params:go_default_library",
         "//shared/sliceutil:go_default_library",
         "@com_github_pkg_errors//:go_default_library",

beacon-chain/operations/slashings/service.go

@@ -10,7 +10,6 @@ import (
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/blocks"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state"
-	"github.com/prysmaticlabs/prysm/shared/mathutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/shared/sliceutil"
 	"go.opencensus.io/trace"
@@ -26,10 +25,11 @@ func NewPool() *Pool {
 }
 
 // PendingAttesterSlashings returns attester slashings that are able to be included into a block.
-// This method will not return more than the block enforced MaxAttesterSlashings.
-func (p *Pool) PendingAttesterSlashings(ctx context.Context, state *beaconstate.BeaconState) []*ethpb.AttesterSlashing {
-	p.lock.RLock()
-	defer p.lock.RUnlock()
+// This method will return the amount of pending attester slashings for a block transition unless parameter `noLimit` is true
+// to indicate the request is for noLimit pending items.
+func (p *Pool) PendingAttesterSlashings(ctx context.Context, state *beaconstate.BeaconState, noLimit bool) []*ethpb.AttesterSlashing {
+	p.lock.Lock()
+	defer p.lock.Unlock()
 	ctx, span := trace.StartSpan(ctx, "operations.PendingAttesterSlashing")
 	defer span.End()
 
@@ -37,14 +37,18 @@ func (p *Pool) PendingAttesterSlashings(ctx context.Context, state *beaconstate.
 	numPendingAttesterSlashings.Set(float64(len(p.pendingAttesterSlashing)))
 
 	included := make(map[uint64]bool)
-	// Allocate pending slice with a capacity of min(len(p.pendingAttesterSlashing), maxAttesterSlashings)
-	// since the array cannot exceed the max and is typically less than the max value.
-	pending := make([]*ethpb.AttesterSlashing, 0, mathutil.Min(uint64(len(p.pendingAttesterSlashing)), params.BeaconConfig().MaxAttesterSlashings))
+
+	// Allocate pending slice with a capacity of maxAttesterSlashings or len(p.pendingAttesterSlashing)) depending on the request.
+	maxSlashings := params.BeaconConfig().MaxAttesterSlashings
+	if noLimit {
+		maxSlashings = uint64(len(p.pendingAttesterSlashing))
+	}
+	pending := make([]*ethpb.AttesterSlashing, 0, maxSlashings)
 	for i := 0; i < len(p.pendingAttesterSlashing); i++ {
-		slashing := p.pendingAttesterSlashing[i]
-		if uint64(len(pending)) >= params.BeaconConfig().MaxAttesterSlashings {
+		if uint64(len(pending)) >= maxSlashings {
 			break
 		}
+		slashing := p.pendingAttesterSlashing[i]
 		valid, err := p.validatorSlashingPreconditionCheck(state, slashing.validatorToSlash)
 		if err != nil {
 			log.WithError(err).Error("could not validate attester slashing")
@@ -68,24 +72,28 @@ func (p *Pool) PendingAttesterSlashings(ctx context.Context, state *beaconstate.
 }
 
 // PendingProposerSlashings returns proposer slashings that are able to be included into a block.
-// This method will not return more than the block enforced MaxProposerSlashings.
-func (p *Pool) PendingProposerSlashings(ctx context.Context, state *beaconstate.BeaconState) []*ethpb.ProposerSlashing {
-	p.lock.RLock()
-	defer p.lock.RUnlock()
+// This method will return the amount of pending proposer slashings for a block transition unless the `noLimit` parameter
+// is set to true to indicate the request is for noLimit pending items.
+func (p *Pool) PendingProposerSlashings(ctx context.Context, state *beaconstate.BeaconState, noLimit bool) []*ethpb.ProposerSlashing {
+	p.lock.Lock()
+	defer p.lock.Unlock()
 	ctx, span := trace.StartSpan(ctx, "operations.PendingProposerSlashing")
 	defer span.End()
 
 	// Update prom metric.
 	numPendingProposerSlashings.Set(float64(len(p.pendingProposerSlashing)))
 
-	// Allocate pending slice with a capacity of min(len(p.pendingProposerSlashing), maxProposerSlashings)
-	// since the array cannot exceed the max and is typically less than the max value.
-	pending := make([]*ethpb.ProposerSlashing, 0, mathutil.Min(uint64(len(p.pendingProposerSlashing)), params.BeaconConfig().MaxProposerSlashings))
+	// Allocate pending slice with a capacity of len(p.pendingProposerSlashing) or maxProposerSlashings depending on the request.
+	maxSlashings := params.BeaconConfig().MaxProposerSlashings
+	if noLimit {
+		maxSlashings = uint64(len(p.pendingProposerSlashing))
+	}
+	pending := make([]*ethpb.ProposerSlashing, 0, maxSlashings)
 	for i := 0; i < len(p.pendingProposerSlashing); i++ {
-		slashing := p.pendingProposerSlashing[i]
-		if uint64(len(pending)) >= params.BeaconConfig().MaxProposerSlashings {
+		if uint64(len(pending)) >= maxSlashings {
 			break
 		}
+		slashing := p.pendingProposerSlashing[i]
 		valid, err := p.validatorSlashingPreconditionCheck(state, slashing.Header_1.Header.ProposerIndex)
 		if err != nil {
 			log.WithError(err).Error("could not validate proposer slashing")

beacon-chain/operations/slashings/service_attester_test.go

@@ -451,6 +451,7 @@ func TestPool_MarkIncludedAttesterSlashing(t *testing.T) {
 func TestPool_PendingAttesterSlashings(t *testing.T) {
 	type fields struct {
 		pending []*PendingAttesterSlashing
+		all     bool
 	}
 	params.SetupTestConfigCleanup(t)
 	beaconState, privKeys := testutil.DeterministicGenesisState(t, 64)
@@ -477,6 +478,14 @@ func TestPool_PendingAttesterSlashings(t *testing.T) {
 			},
 			want: []*ethpb.AttesterSlashing{},
 		},
+		{
+			name: "All pending",
+			fields: fields{
+				pending: pendingSlashings,
+				all:     true,
+			},
+			want: slashings,
+		},
 		{
 			name: "All eligible",
 			fields: fields{
@@ -497,7 +506,7 @@ func TestPool_PendingAttesterSlashings(t *testing.T) {
 			p := &Pool{
 				pendingAttesterSlashing: tt.fields.pending,
 			}
-			assert.DeepEqual(t, tt.want, p.PendingAttesterSlashings(context.Background(), beaconState))
+			assert.DeepEqual(t, tt.want, p.PendingAttesterSlashings(context.Background(), beaconState, tt.fields.all))
 		})
 	}
 }
@@ -505,6 +514,7 @@ func TestPool_PendingAttesterSlashings(t *testing.T) {
 func TestPool_PendingAttesterSlashings_Slashed(t *testing.T) {
 	type fields struct {
 		pending []*PendingAttesterSlashing
+		all     bool
 	}
 	params.SetupTestConfigCleanup(t)
 	conf := params.BeaconConfig()
@@ -515,15 +525,12 @@ func TestPool_PendingAttesterSlashings_Slashed(t *testing.T) {
 	require.NoError(t, err)
 	val.Slashed = true
 	require.NoError(t, beaconState.UpdateValidatorAtIndex(0, val))
-	val, err = beaconState.ValidatorAtIndex(3)
-	require.NoError(t, err)
-	val.Slashed = true
-	require.NoError(t, beaconState.UpdateValidatorAtIndex(3, val))
 	val, err = beaconState.ValidatorAtIndex(5)
 	require.NoError(t, err)
 	val.Slashed = true
 	require.NoError(t, beaconState.UpdateValidatorAtIndex(5, val))
 	pendingSlashings := make([]*PendingAttesterSlashing, 20)
+	pendingSlashings2 := make([]*PendingAttesterSlashing, 20)
 	slashings := make([]*ethpb.AttesterSlashing, 20)
 	for i := 0; i < len(pendingSlashings); i++ {
 		sl, err := testutil.GenerateAttesterSlashingForValidator(beaconState, privKeys[i], uint64(i))
@@ -532,32 +539,45 @@ func TestPool_PendingAttesterSlashings_Slashed(t *testing.T) {
 			attesterSlashing: sl,
 			validatorToSlash: uint64(i),
 		}
+		pendingSlashings2[i] = &PendingAttesterSlashing{
+			attesterSlashing: sl,
+			validatorToSlash: uint64(i),
+		}
 		slashings[i] = sl
 	}
+	result := append(slashings[1:5], slashings[6:]...)
 	tests := []struct {
 		name   string
 		fields fields
 		want   []*ethpb.AttesterSlashing
 	}{
 		{
-			name: "Skips slashed validator",
+			name: "One item",
 			fields: fields{
-				pending: pendingSlashings,
+				pending: pendingSlashings[:2],
 			},
-			want: slashings[1:3],
+			want: slashings[1:2],
 		},
 		{
-			name: "Skips gapped slashed validators",
+			name: "Skips gapped slashed",
 			fields: fields{
-				pending: pendingSlashings[2:],
+				pending: pendingSlashings[4:7],
 			},
-			want: []*ethpb.AttesterSlashing{slashings[4], slashings[6]},
+			want: result[3:5],
+		},
+		{
+			name: "All and skips gapped slashed validators",
+			fields: fields{
+				pending: pendingSlashings2,
+				all:     true,
+			},
+			want: result,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			p := &Pool{pendingAttesterSlashing: tt.fields.pending}
-			assert.DeepEqual(t, tt.want, p.PendingAttesterSlashings(context.Background(), beaconState))
+			assert.DeepEqual(t, tt.want, p.PendingAttesterSlashings(context.Background(), beaconState, tt.fields.all /*noLimit*/))
 		})
 	}
 }
@@ -585,5 +605,5 @@ func TestPool_PendingAttesterSlashings_NoDuplicates(t *testing.T) {
 	p := &Pool{
 		pendingAttesterSlashing: pendingSlashings,
 	}
-	assert.DeepEqual(t, slashings[0:2], p.PendingAttesterSlashings(context.Background(), beaconState))
+	assert.DeepEqual(t, slashings[0:2], p.PendingAttesterSlashings(context.Background(), beaconState, false /*noLimit*/))
 }

beacon-chain/operations/slashings/service_proposer_test.go

@@ -323,6 +323,7 @@ func TestPool_MarkIncludedProposerSlashing(t *testing.T) {
 func TestPool_PendingProposerSlashings(t *testing.T) {
 	type fields struct {
 		pending []*ethpb.ProposerSlashing
+		noLimit bool
 	}
 	beaconState, privKeys := testutil.DeterministicGenesisState(t, 64)
 	slashings := make([]*ethpb.ProposerSlashing, 20)
@@ -344,7 +345,15 @@ func TestPool_PendingProposerSlashings(t *testing.T) {
 			want: []*ethpb.ProposerSlashing{},
 		},
 		{
-			name: "All eligible",
+			name: "All",
+			fields: fields{
+				pending: slashings,
+				noLimit: true,
+			},
+			want: slashings,
+		},
+		{
+			name: "All block eligible",
 			fields: fields{
 				pending: slashings[:params.BeaconConfig().MaxProposerSlashings],
 			},
@@ -363,13 +372,14 @@ func TestPool_PendingProposerSlashings(t *testing.T) {
 			p := &Pool{
 				pendingProposerSlashing: tt.fields.pending,
 			}
-			assert.DeepEqual(t, tt.want, p.PendingProposerSlashings(context.Background(), beaconState))
+			assert.DeepEqual(t, tt.want, p.PendingProposerSlashings(context.Background(), beaconState, tt.fields.noLimit))
 		})
 	}
 }
 
 func TestPool_PendingProposerSlashings_Slashed(t *testing.T) {
 	type fields struct {
+		all     bool
 		pending []*ethpb.ProposerSlashing
 	}
 	beaconState, privKeys := testutil.DeterministicGenesisState(t, 64)
@@ -382,13 +392,16 @@ func TestPool_PendingProposerSlashings_Slashed(t *testing.T) {
 	val.Slashed = true
 	require.NoError(t, beaconState.UpdateValidatorAtIndex(5, val))
 	slashings := make([]*ethpb.ProposerSlashing, 32)
+	slashings2 := make([]*ethpb.ProposerSlashing, 32)
+	result := make([]*ethpb.ProposerSlashing, 32)
 	for i := 0; i < len(slashings); i++ {
 		sl, err := testutil.GenerateProposerSlashingForValidator(beaconState, privKeys[i], uint64(i))
 		require.NoError(t, err)
 		slashings[i] = sl
+		slashings2[i] = sl
+		result[i] = sl
 	}
-	result := make([]*ethpb.ProposerSlashing, 32)
-	copy(result, slashings)
+	result = append(result[1:5], result[6:]...)
 	tests := []struct {
 		name   string
 		fields fields
@@ -399,7 +412,15 @@ func TestPool_PendingProposerSlashings_Slashed(t *testing.T) {
 			fields: fields{
 				pending: slashings,
 			},
-			want: append(result[1:5], result[6:18]...),
+			want: result[:16],
+		},
+		{
+			name: "gets noLimit and no slashed",
+			fields: fields{
+				all:     true,
+				pending: slashings2,
+			},
+			want: result,
 		},
 	}
 	for _, tt := range tests {
@@ -407,7 +428,10 @@ func TestPool_PendingProposerSlashings_Slashed(t *testing.T) {
 			p := &Pool{
 				pendingProposerSlashing: tt.fields.pending,
 			}
-			assert.DeepEqual(t, tt.want, p.PendingProposerSlashings(context.Background(), beaconState))
+			result := p.PendingProposerSlashings(context.Background(), beaconState, tt.fields.all /*noLimit*/)
+			t.Log(tt.want[0].Header_1.Header.ProposerIndex)
+			t.Log(result[0].Header_1.Header.ProposerIndex)
+			assert.DeepEqual(t, tt.want, result)
 		})
 	}
 }

beacon-chain/operations/voluntaryexits/BUILD.bazel

@@ -15,7 +15,6 @@ go_library(
     deps = [
         "//beacon-chain/core/helpers:go_default_library",
         "//beacon-chain/state:go_default_library",
-        "//shared/mathutil:go_default_library",
         "//shared/params:go_default_library",
         "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library",
         "@io_opencensus_go//trace:go_default_library",

beacon-chain/operations/voluntaryexits/service.go

@@ -8,7 +8,6 @@ import (
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	beaconstate "github.com/prysmaticlabs/prysm/beacon-chain/state"
-	"github.com/prysmaticlabs/prysm/shared/mathutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"go.opencensus.io/trace"
 )
@@ -32,13 +31,17 @@ func NewPool() *Pool {
 
 // PendingExits returns exits that are ready for inclusion at the given slot. This method will not
 // return more than the block enforced MaxVoluntaryExits.
-func (p *Pool) PendingExits(state *beaconstate.BeaconState, slot uint64) []*ethpb.SignedVoluntaryExit {
+func (p *Pool) PendingExits(state *beaconstate.BeaconState, slot uint64, noLimit bool) []*ethpb.SignedVoluntaryExit {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 
 	// Allocate pending slice with a capacity of min(len(p.pending), maxVoluntaryExits) since the
 	// array cannot exceed the max and is typically less than the max value.
-	pending := make([]*ethpb.SignedVoluntaryExit, 0, mathutil.Min(uint64(len(p.pending)), params.BeaconConfig().MaxVoluntaryExits))
+	maxExits := params.BeaconConfig().MaxVoluntaryExits
+	if noLimit {
+		maxExits = uint64(len(p.pending))
+	}
+	pending := make([]*ethpb.SignedVoluntaryExit, 0, maxExits)
 	for _, e := range p.pending {
 		if e.Exit.Epoch > helpers.SlotToEpoch(slot) {
 			continue
@@ -47,8 +50,8 @@ func (p *Pool) PendingExits(state *beaconstate.BeaconState, slot uint64) []*ethp
 			pending = append(pending, e)
 		}
 	}
-	if uint64(len(pending)) > params.BeaconConfig().MaxVoluntaryExits {
-		pending = pending[:params.BeaconConfig().MaxVoluntaryExits]
+	if uint64(len(pending)) > maxExits {
+		pending = pending[:maxExits]
 	}
 	return pending
 }

beacon-chain/operations/voluntaryexits/service_test.go

@@ -332,6 +332,7 @@ func TestPool_MarkIncluded(t *testing.T) {
 func TestPool_PendingExits(t *testing.T) {
 	type fields struct {
 		pending []*ethpb.SignedVoluntaryExit
+		noLimit bool
 	}
 	type args struct {
 		slot uint64
@@ -375,7 +376,60 @@ func TestPool_PendingExits(t *testing.T) {
 			},
 		},
 		{
-			name: "All eligible, more than max",
+			name: "All eligible, above max",
+			fields: fields{
+				noLimit: true,
+				pending: []*ethpb.SignedVoluntaryExit{
+					{Exit: &ethpb.VoluntaryExit{Epoch: 0}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 1}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 2}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 3}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 4}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 5}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 6}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 7}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 8}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 9}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 10}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 11}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 12}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 13}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 14}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 15}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 16}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 17}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 18}},
+					{Exit: &ethpb.VoluntaryExit{Epoch: 19}},
+				},
+			},
+			args: args{
+				slot: 1000000,
+			},
+			want: []*ethpb.SignedVoluntaryExit{
+				{Exit: &ethpb.VoluntaryExit{Epoch: 0}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 1}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 2}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 3}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 4}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 5}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 6}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 7}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 8}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 9}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 10}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 11}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 12}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 13}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 14}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 15}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 16}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 17}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 18}},
+				{Exit: &ethpb.VoluntaryExit{Epoch: 19}},
+			},
+		},
+		{
+			name: "All eligible, block max",
 			fields: fields{
 				pending: []*ethpb.SignedVoluntaryExit{
 					{Exit: &ethpb.VoluntaryExit{Epoch: 0}},
@@ -450,7 +504,7 @@ func TestPool_PendingExits(t *testing.T) {
 			}
 			s, err := beaconstate.InitializeFromProtoUnsafe(&p2ppb.BeaconState{Validators: []*ethpb.Validator{{ExitEpoch: params.BeaconConfig().FarFutureEpoch}}})
 			require.NoError(t, err)
-			if got := p.PendingExits(s, tt.args.slot); !reflect.DeepEqual(got, tt.want) {
+			if got := p.PendingExits(s, tt.args.slot, tt.fields.noLimit); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("PendingExits() = %v, want %v", got, tt.want)
 			}
 		})

beacon-chain/rpc/validator/proposer.go

@@ -117,9 +117,9 @@ func (vs *Server) GetBlock(ctx context.Context, req *ethpb.BlockRequest) (*ethpb
 			Deposits:          deposits,
 			Attestations:      atts,
 			RandaoReveal:      req.RandaoReveal,
-			ProposerSlashings: vs.SlashingsPool.PendingProposerSlashings(ctx, head),
-			AttesterSlashings: vs.SlashingsPool.PendingAttesterSlashings(ctx, head),
-			VoluntaryExits:    vs.ExitPool.PendingExits(head, req.Slot),
+			ProposerSlashings: vs.SlashingsPool.PendingProposerSlashings(ctx, head, false /*noLimit*/),
+			AttesterSlashings: vs.SlashingsPool.PendingAttesterSlashings(ctx, head, false /*noLimit*/),
+			VoluntaryExits:    vs.ExitPool.PendingExits(head, req.Slot, false /*noLimit*/),
 			Graffiti:          graffiti[:],
 		},
 	}

beacon-chain/sync/subscriber_test.go

@@ -110,7 +110,7 @@ func TestSubscribe_ReceivesAttesterSlashing(t *testing.T) {
 	if testutil.WaitTimeout(&wg, time.Second) {
 		t.Fatal("Did not receive PubSub in 1 second")
 	}
-	as := r.slashingPool.PendingAttesterSlashings(ctx, beaconState)
+	as := r.slashingPool.PendingAttesterSlashings(ctx, beaconState, false /*noLimit*/)
 	assert.Equal(t, 1, len(as), "Expected attester slashing")
 }
 
@@ -159,7 +159,7 @@ func TestSubscribe_ReceivesProposerSlashing(t *testing.T) {
 	if testutil.WaitTimeout(&wg, time.Second) {
 		t.Fatal("Did not receive PubSub in 1 second")
 	}
-	ps := r.slashingPool.PendingProposerSlashings(ctx, beaconState)
+	ps := r.slashingPool.PendingProposerSlashings(ctx, beaconState, false /*noLimit*/)
 	assert.Equal(t, 1, len(ps), "Expected proposer slashing")
 }