From 08d69f1e58ddf1e3b74dafd713885f76d40b21ef Mon Sep 17 00:00:00 2001 From: terence tsao Date: Fri, 9 Aug 2024 14:06:02 -0400 Subject: [PATCH 1/3] Add getter for payload attestation cache --- beacon-chain/cache/payload_attestation.go | 16 +++++++ .../cache/payload_attestation_test.go | 48 +++++++++++++++++++ proto/prysm/v1alpha1/cloners.go | 18 +++++-- 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/beacon-chain/cache/payload_attestation.go b/beacon-chain/cache/payload_attestation.go index 2a921559b2d7..78c7059df722 100644 --- a/beacon-chain/cache/payload_attestation.go +++ b/beacon-chain/cache/payload_attestation.go @@ -107,6 +107,22 @@ func (p *PayloadAttestationCache) Add(att *eth.PayloadAttestationMessage, idx ui return nil } +// Get returns the aggregated PayloadAttestation for the given root and status +// if the root doesn't exist or status is invalid, the function returns nil. +func (p *PayloadAttestationCache) Get(root [32]byte, status primitives.PTCStatus) *eth.PayloadAttestation { + p.Lock() + defer p.Unlock() + + if p.root != root { + return nil + } + if uint64(status) >= uint64(len(p.attestations)) { + return nil + } + + return eth.CopyPayloadAttestation(p.attestations[status]) +} + // Clear clears the internal map func (p *PayloadAttestationCache) Clear() { p.Lock() diff --git a/beacon-chain/cache/payload_attestation_test.go b/beacon-chain/cache/payload_attestation_test.go index 10c274222edf..1bb7067c9adf 100644 --- a/beacon-chain/cache/payload_attestation_test.go +++ b/beacon-chain/cache/payload_attestation_test.go @@ -93,3 +93,51 @@ func TestPayloadAttestationCache(t *testing.T) { indices = att.AggregationBits.BitIndices() require.DeepEqual(t, []int{int(idx)}, indices) } + +func TestPayloadAttestationCache_Get(t *testing.T) { + root := [32]byte{1, 2, 3} + wrongRoot := [32]byte{4, 5, 6} + status := primitives.PAYLOAD_PRESENT + invalidStatus := primitives.PAYLOAD_INVALID_STATUS + + cache := &PayloadAttestationCache{ + root: root, + attestations: [primitives.PAYLOAD_INVALID_STATUS]*eth.PayloadAttestation{ + { + Signature: []byte{1}, + }, + { + Signature: []byte{2}, + }, + { + Signature: []byte{3}, + }, + }, + } + + t.Run("valid root and status", func(t *testing.T) { + result := cache.Get(root, status) + require.NotNil(t, result, "Expected a non-nil result") + require.DeepEqual(t, cache.attestations[status], result) + }) + + t.Run("invalid root", func(t *testing.T) { + result := cache.Get(wrongRoot, status) + require.IsNil(t, result) + }) + + t.Run("status out of bound", func(t *testing.T) { + result := cache.Get(root, invalidStatus) + require.IsNil(t, result) + }) + + t.Run("no attestation", func(t *testing.T) { + emptyCache := &PayloadAttestationCache{ + root: root, + attestations: [primitives.PAYLOAD_INVALID_STATUS]*eth.PayloadAttestation{}, + } + + result := emptyCache.Get(root, status) + require.IsNil(t, result) + }) +} diff --git a/proto/prysm/v1alpha1/cloners.go b/proto/prysm/v1alpha1/cloners.go index 9cfeb6396616..b44764e81db3 100644 --- a/proto/prysm/v1alpha1/cloners.go +++ b/proto/prysm/v1alpha1/cloners.go @@ -105,7 +105,7 @@ func CopyBeaconBlockBodyEPBS(body *BeaconBlockBodyEpbs) *BeaconBlockBodyEpbs { SyncAggregate: body.SyncAggregate.Copy(), BlsToExecutionChanges: CopySlice(body.BlsToExecutionChanges), SignedExecutionPayloadHeader: CopySignedExecutionPayloadHeader(body.SignedExecutionPayloadHeader), - PayloadAttestations: CopyPayloadAttestation(body.PayloadAttestations), + PayloadAttestations: CopyPayloadAttestations(body.PayloadAttestations), } } @@ -137,8 +137,8 @@ func CopyExecutionPayloadHeaderEPBS(payload *enginev1.ExecutionPayloadHeaderEPBS } } -// CopyPayloadAttestation copies the provided PayloadAttestation array. -func CopyPayloadAttestation(attestations []*PayloadAttestation) []*PayloadAttestation { +// CopyPayloadAttestations copies the provided PayloadAttestation array. +func CopyPayloadAttestations(attestations []*PayloadAttestation) []*PayloadAttestation { if attestations == nil { return nil } @@ -164,3 +164,15 @@ func CopyPayloadAttestationData(data *PayloadAttestationData) *PayloadAttestatio PayloadStatus: data.PayloadStatus, } } + +// CopyPayloadAttestation copies the provided PayloadAttestation. +func CopyPayloadAttestation(a *PayloadAttestation) *PayloadAttestation { + if a == nil { + return nil + } + return &PayloadAttestation{ + AggregationBits: bytesutil.SafeCopyBytes(a.AggregationBits), + Data: CopyPayloadAttestationData(a.Data), + Signature: bytesutil.SafeCopyBytes(a.Signature), + } +} From dd4127bcf243a9d8110ec8a28c0faa78f703f81b Mon Sep 17 00:00:00 2001 From: terence tsao Date: Fri, 9 Aug 2024 19:52:03 -0400 Subject: [PATCH 2/3] Check against status --- beacon-chain/cache/payload_attestation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/cache/payload_attestation.go b/beacon-chain/cache/payload_attestation.go index 78c7059df722..359b6b03871e 100644 --- a/beacon-chain/cache/payload_attestation.go +++ b/beacon-chain/cache/payload_attestation.go @@ -116,7 +116,7 @@ func (p *PayloadAttestationCache) Get(root [32]byte, status primitives.PTCStatus if p.root != root { return nil } - if uint64(status) >= uint64(len(p.attestations)) { + if status >= primitives.PAYLOAD_INVALID_STATUS { return nil } From cda6fa35392418d866a7001f18b521fcb6692d3a Mon Sep 17 00:00:00 2001 From: terence tsao Date: Mon, 12 Aug 2024 16:38:59 +0000 Subject: [PATCH 3/3] Feedback #1 --- proto/prysm/v1alpha1/cloners.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/proto/prysm/v1alpha1/cloners.go b/proto/prysm/v1alpha1/cloners.go index b44764e81db3..98599e5b8df1 100644 --- a/proto/prysm/v1alpha1/cloners.go +++ b/proto/prysm/v1alpha1/cloners.go @@ -74,7 +74,7 @@ func CopySignedBeaconBlockEPBS(sigBlock *SignedBeaconBlockEpbs) *SignedBeaconBlo } } -// CopyBeaconBlockEPBS copies the provided CopyBeaconBlockEPBS. +// CopyBeaconBlockEPBS copies the provided BeaconBlockEPBS. func CopyBeaconBlockEPBS(block *BeaconBlockEpbs) *BeaconBlockEpbs { if block == nil { return nil @@ -88,7 +88,7 @@ func CopyBeaconBlockEPBS(block *BeaconBlockEpbs) *BeaconBlockEpbs { } } -// CopyBeaconBlockBodyEPBS copies the provided CopyBeaconBlockBodyEPBS. +// CopyBeaconBlockBodyEPBS copies the provided BeaconBlockBodyEPBS. func CopyBeaconBlockBodyEPBS(body *BeaconBlockBodyEpbs) *BeaconBlockBodyEpbs { if body == nil { return nil @@ -144,11 +144,7 @@ func CopyPayloadAttestations(attestations []*PayloadAttestation) []*PayloadAttes } newAttestations := make([]*PayloadAttestation, len(attestations)) for i, att := range attestations { - newAttestations[i] = &PayloadAttestation{ - AggregationBits: bytesutil.SafeCopyBytes(att.AggregationBits), - Data: CopyPayloadAttestationData(att.Data), - Signature: bytesutil.SafeCopyBytes(att.Signature), - } + newAttestations[i] = CopyPayloadAttestation(att) } return newAttestations }