Skip to content

Commit

Permalink
Improves operations/attestations/kv test coverage (#6181)
Browse files Browse the repository at this point in the history 
* fixes span names
* TestKV_SaveUnaggregatedAttestation tests added
* TestKV_SaveUnaggregatedAttestations test
* adds count tests
* TestKV_Unaggregated_CanDelete
* fixes naming
* Merge branch 'master' into att-pool-kv-cleanup
* better naming
* Merge branch 'att-pool-kv-cleanup' of github.com:prysmaticlabs/prysm into att-pool-kv-cleanup
* naming
* TestKV_Aggregated_SaveUnaggregatedAttestation added
* Merge branch 'master' into att-pool-kv-cleanup
* naming
* updates TestKV_Aggregated_HasAggregatedAttestation
* updates delete tests
* fix order
* Merge branch 'master' into att-pool-kv-cleanup
  • Loading branch information
@farazdagi
farazdagi committed Jun 9, 2020
1 parent 187e08d commit 50af507
Show file tree
Hide file tree
Showing 4 changed files with 317 additions and 63 deletions.
2 changes: 1 addition & 1 deletion beacon-chain/operations/attestations/kv/aggregated.go
View file
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil"
)

// AggregateUnaggregatedAttestations aggregates the unaggregated attestations and save the
// AggregateUnaggregatedAttestations aggregates the unaggregated attestations and saves the
// newly aggregated attestations in the pool.
// It tracks the unaggregated attestations that weren't able to aggregate to prevent
// the deletion of unaggregated attestations in the pool.
Expand Down
201 changes: 175 additions & 26 deletions beacon-chain/operations/attestations/kv/aggregated_test.go
View file
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/prysmaticlabs/prysm/shared/bls"
)

func TestKV_AggregateUnaggregatedAttestations(t *testing.T) {
func TestKV_Aggregated_AggregateUnaggregatedAttestations(t *testing.T) {
cache := NewAttCaches()
priv := bls.RandKey()
sig1 := priv.Sign([]byte{'a'})
Expand Down Expand Up @@ -39,7 +39,76 @@ func TestKV_AggregateUnaggregatedAttestations(t *testing.T) {
}
}

func TestKV_Aggregated_CanSaveRetrieve(t *testing.T) {
func TestKV_Aggregated_SaveUnaggregatedAttestation(t *testing.T) {
tests := []struct {
name string
att *ethpb.Attestation
count int
wantErrString string
}{
{
name: "nil attestation",
att: nil,
},
{
name: "nil attestation data",
att: &ethpb.Attestation{},
},
{
name: "not aggregated",
att: &ethpb.Attestation{
Data: &ethpb.AttestationData{}, AggregationBits: bitfield.Bitlist{0b10100}},
wantErrString: "attestation is not aggregated",
},
{
name: "invalid hash",
att: &ethpb.Attestation{
Data: &ethpb.AttestationData{
BeaconBlockRoot: []byte{0b0},
},
AggregationBits: bitfield.Bitlist{0b10111},
},
wantErrString: "could not tree hash attestation: incorrect fixed bytes marshalling",
},
{
name: "normal save",
att: &ethpb.Attestation{
Data: &ethpb.AttestationData{
Slot: 1,
},
AggregationBits: bitfield.Bitlist{0b1101},
},
count: 1,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cache := NewAttCaches()
if len(cache.unAggregatedAtt) != 0 {
t.Errorf("Invalid start pool, atts: %d", len(cache.unAggregatedAtt))
}

err := cache.SaveAggregatedAttestation(tt.att)
if tt.wantErrString != "" && (err == nil || err.Error() != tt.wantErrString) {
t.Errorf("Did not receive wanted error, want: %q, got: %v", tt.wantErrString, err)
return
}
if tt.wantErrString == "" && err != nil {
t.Error(err)
return
}
if len(cache.aggregatedAtt) != tt.count {
t.Errorf("Wrong attestation count, want: %d, got: %d", tt.count, len(cache.unAggregatedAtt))
}
if cache.AggregatedAttestationCount() != tt.count {
t.Errorf("Wrong attestation count, want: %d, got: %d", tt.count, cache.AggregatedAttestationCount())
}
})
}
}

func TestKV_Aggregated_AggregatedAttestations(t *testing.T) {
cache := NewAttCaches()

att1 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 1}, AggregationBits: bitfield.Bitlist{0b1101}}
Expand All @@ -54,7 +123,6 @@ func TestKV_Aggregated_CanSaveRetrieve(t *testing.T) {
}

returned := cache.AggregatedAttestations()

sort.Slice(returned, func(i, j int) bool {
return returned[i].Data.Slot < returned[j].Data.Slot
})
Expand All @@ -64,43 +132,124 @@ func TestKV_Aggregated_CanSaveRetrieve(t *testing.T) {
}
}

func TestKV_Aggregated_CanDelete(t *testing.T) {
cache := NewAttCaches()
func TestKV_Aggregated_DeleteAggregatedAttestation(t *testing.T) {
t.Run("nil attestation", func(t *testing.T) {
cache := NewAttCaches()
if err := cache.DeleteAggregatedAttestation(nil); err != nil {
t.Error(err)
}
att := &ethpb.Attestation{AggregationBits: bitfield.Bitlist{0b10101}}
if err := cache.DeleteAggregatedAttestation(att); err != nil {
t.Error(err)
}
})

att1 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 1}, AggregationBits: bitfield.Bitlist{0b1101}}
att2 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 2}, AggregationBits: bitfield.Bitlist{0b1101}}
att3 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 3}, AggregationBits: bitfield.Bitlist{0b1101}}
att4 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 3}, AggregationBits: bitfield.Bitlist{0b10101}}
atts := []*ethpb.Attestation{att1, att2, att3, att4}
t.Run("non aggregated attestation", func(t *testing.T) {
cache := NewAttCaches()
att := &ethpb.Attestation{AggregationBits: bitfield.Bitlist{0b1001}, Data: &ethpb.AttestationData{Slot: 2}}
err := cache.DeleteAggregatedAttestation(att)
wantErr := "attestation is not aggregated"
if err == nil || err.Error() != wantErr {
t.Errorf("Did not receive wanted error, want: %q, got: %v", wantErr, err)
}
})

for _, att := range atts {
if err := cache.SaveAggregatedAttestation(att); err != nil {
t.Run("invalid hash", func(t *testing.T) {
cache := NewAttCaches()
att := &ethpb.Attestation{
AggregationBits: bitfield.Bitlist{0b1111},
Data: &ethpb.AttestationData{
Slot: 2,
BeaconBlockRoot: []byte{0b0},
},
}
err := cache.DeleteAggregatedAttestation(att)
wantErr := "could not tree hash attestation data: incorrect fixed bytes marshalling"
if err == nil || err.Error() != wantErr {
t.Errorf("Did not receive wanted error, want: %q, got: %v", wantErr, err)
}
})

t.Run("nonexistent attestation", func(t *testing.T) {
cache := NewAttCaches()
att := &ethpb.Attestation{AggregationBits: bitfield.Bitlist{0b1111}, Data: &ethpb.AttestationData{Slot: 2}}
if err := cache.DeleteAggregatedAttestation(att); err != nil {
t.Error(err)
}
})

t.Run("non-filtered deletion", func(t *testing.T) {
cache := NewAttCaches()
att1 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 1}, AggregationBits: bitfield.Bitlist{0b1101}}
att2 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 2}, AggregationBits: bitfield.Bitlist{0b1101}}
att3 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 3}, AggregationBits: bitfield.Bitlist{0b1101}}
att4 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 3}, AggregationBits: bitfield.Bitlist{0b10101}}
atts := []*ethpb.Attestation{att1, att2, att3, att4}
if err := cache.SaveAggregatedAttestations(atts); err != nil {
t.Fatal(err)
}
}

if err := cache.DeleteAggregatedAttestation(att1); err != nil {
t.Fatal(err)
}
if err := cache.DeleteAggregatedAttestation(att3); err != nil {
t.Fatal(err)
}
if err := cache.DeleteAggregatedAttestation(att1); err != nil {
t.Fatal(err)
}
if err := cache.DeleteAggregatedAttestation(att3); err != nil {
t.Fatal(err)
}

returned := cache.AggregatedAttestations()
wanted := []*ethpb.Attestation{att2}
returned := cache.AggregatedAttestations()
wanted := []*ethpb.Attestation{att2}
if !reflect.DeepEqual(wanted, returned) {
t.Error("Did not receive correct aggregated atts")
}
})

if !reflect.DeepEqual(wanted, returned) {
t.Error("Did not receive correct aggregated atts")
}
t.Run("filtered deletion", func(t *testing.T) {
cache := NewAttCaches()
att1 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 1}, AggregationBits: bitfield.Bitlist{0b110101}}
att2 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 2}, AggregationBits: bitfield.Bitlist{0b110111}}
att3 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 2}, AggregationBits: bitfield.Bitlist{0b110100}}
att4 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 2}, AggregationBits: bitfield.Bitlist{0b110101}}
atts := []*ethpb.Attestation{att1, att2, att3, att4}
if err := cache.SaveAggregatedAttestations(atts); err != nil {
t.Fatal(err)
}

if cache.AggregatedAttestationCount() != 2 {
t.Error("Unexpected number of atts")
}
if err := cache.DeleteAggregatedAttestation(att4); err != nil {
t.Fatal(err)
}

returned := cache.AggregatedAttestations()
wanted := []*ethpb.Attestation{att1, att2}
sort.Slice(returned, func(i, j int) bool {
return string(returned[i].AggregationBits) < string(returned[j].AggregationBits)
})
if !reflect.DeepEqual(wanted, returned) {
t.Errorf("Did not receive correct aggregated atts, want: %v, got: %v", wanted, returned)
}
})
}

func TestKV_HasAggregatedAttestation(t *testing.T) {
func TestKV_Aggregated_HasAggregatedAttestation(t *testing.T) {
tests := []struct {
name string
existing []*ethpb.Attestation
input *ethpb.Attestation
want bool
}{
{
name: "nil attestation",
input: nil,
want: false,
},
{
name: "nil attestation data",
input: &ethpb.Attestation{
AggregationBits: bitfield.Bitlist{0b1111}},
want: false,
},
{
name: "empty cache aggregated",
input: &ethpb.Attestation{
Expand Down Expand Up @@ -267,7 +416,7 @@ func TestKV_HasAggregatedAttestation(t *testing.T) {
}
}

func TestKV_Aggregated_AggregatesAttestations(t *testing.T) {
func TestKV_Aggregated_DuplicateAggregatedAttestations(t *testing.T) {
cache := NewAttCaches()

att1 := &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 1}, AggregationBits: bitfield.Bitlist{0b1101}}
Expand Down

0 comments on commit 50af507

Please sign in to comment.