From 6d89373583f05f3b5c18989d20a650eb9a1d4622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Kapka?= Date: Mon, 10 Mar 2025 14:48:43 +0100 Subject: [PATCH] Handle unaggregated attestations when decomposing (#15027) --- beacon-chain/blockchain/process_block.go | 4 ++++ beacon-chain/blockchain/process_block_test.go | 18 +++++++++++++----- changelog/radek_minor-attestation-tweaks.md | 3 +++ 3 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 changelog/radek_minor-attestation-tweaks.md diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index a799ce52ce..56feb3bd00 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -503,6 +503,10 @@ func (s *Service) pruneCoveredElectraAttsFromPool(ctx context.Context, headState if err = s.cfg.AttestationCache.DeleteCovered(a); err != nil { return errors.Wrap(err, "could not delete covered attestation") } + } else if !a.IsAggregated() { + if err = s.cfg.AttPool.DeleteUnaggregatedAttestation(a); err != nil { + return errors.Wrap(err, "could not delete unaggregated attestation") + } } else if err = s.cfg.AttPool.DeleteAggregatedAttestation(a); err != nil { return errors.Wrap(err, "could not delete aggregated attestation") } diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index d6de30ce88..5f506996d7 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -50,6 +50,7 @@ import ( func Test_pruneAttsFromPool_Electra(t *testing.T) { ctx := context.Background() + logHook := logTest.NewGlobal() params.SetupTestConfigCleanup(t) cfg := params.BeaconConfig().Copy() @@ -71,7 +72,7 @@ func Test_pruneAttsFromPool_Electra(t *testing.T) { cb := primitives.NewAttestationCommitteeBits() cb.SetBitAt(0, true) att1 := ðpb.AttestationElectra{ - AggregationBits: bitfield.Bitlist{0b11110111, 0b00000001}, + AggregationBits: bitfield.Bitlist{0b10000000, 0b00000001}, Data: data, Signature: make([]byte, 96), CommitteeBits: cb, @@ -95,16 +96,15 @@ func Test_pruneAttsFromPool_Electra(t *testing.T) { CommitteeBits: cb, } - require.NoError(t, s.cfg.AttPool.SaveAggregatedAttestation(att1)) + require.NoError(t, s.cfg.AttPool.SaveUnaggregatedAttestation(att1)) require.NoError(t, s.cfg.AttPool.SaveAggregatedAttestation(att2)) require.NoError(t, s.cfg.AttPool.SaveAggregatedAttestation(att3)) - require.Equal(t, 3, len(s.cfg.AttPool.AggregatedAttestations())) cb = primitives.NewAttestationCommitteeBits() cb.SetBitAt(0, true) cb.SetBitAt(1, true) onChainAtt := ðpb.AttestationElectra{ - AggregationBits: bitfield.Bitlist{0b11110111, 0b11110111, 0b00000001}, + AggregationBits: bitfield.Bitlist{0b10000000, 0b11110111, 0b00000001}, Data: data, Signature: make([]byte, 96), CommitteeBits: cb, @@ -127,7 +127,12 @@ func Test_pruneAttsFromPool_Electra(t *testing.T) { require.Equal(t, 4, len(committees)) require.NoError(t, s.pruneAttsFromPool(ctx, st, rob)) - attsInPool := s.cfg.AttPool.AggregatedAttestations() + require.LogsDoNotContain(t, logHook, "Could not prune attestations") + + attsInPool, err := s.cfg.AttPool.UnaggregatedAttestations() + require.NoError(t, err) + assert.Equal(t, 0, len(attsInPool)) + attsInPool = s.cfg.AttPool.AggregatedAttestations() require.Equal(t, 1, len(attsInPool)) assert.DeepEqual(t, att3, attsInPool[0]) } @@ -908,6 +913,8 @@ func TestInsertFinalizedDeposits_MultipleFinalizedRoutines(t *testing.T) { } func TestRemoveBlockAttestationsInPool(t *testing.T) { + logHook := logTest.NewGlobal() + genesis, keys := util.DeterministicGenesisState(t, 64) b, err := util.GenerateFullBlock(genesis, keys, util.DefaultBlockGenConfig(), 1) assert.NoError(t, err) @@ -928,6 +935,7 @@ func TestRemoveBlockAttestationsInPool(t *testing.T) { wsb, err := consensusblocks.NewSignedBeaconBlock(b) require.NoError(t, err) require.NoError(t, service.pruneAttsFromPool(context.Background(), nil /* state not needed pre-Electra */, wsb)) + require.LogsDoNotContain(t, logHook, "Could not prune attestations") require.Equal(t, 0, service.cfg.AttPool.AggregatedAttestationCount()) } diff --git a/changelog/radek_minor-attestation-tweaks.md b/changelog/radek_minor-attestation-tweaks.md new file mode 100644 index 0000000000..9bbd9fb5f3 --- /dev/null +++ b/changelog/radek_minor-attestation-tweaks.md @@ -0,0 +1,3 @@ +### Fixed + +- Handle unaggregated attestations when decomposing Electra block attestations. \ No newline at end of file