Skip to content

Commit

Permalink
moving cloners to attestation.go and adding fuzzing (#14254)
Browse files Browse the repository at this point in the history
* moving cloners to attestation.go and adding fuzzing

* fixing bazel

* fixing build
  • Loading branch information
james-prysm authored Jul 22, 2024
1 parent aa868e5 commit 4d823ac
Show file tree
Hide file tree
Showing 19 changed files with 239 additions and 316 deletions.
2 changes: 1 addition & 1 deletion beacon-chain/blockchain/process_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (s *Service) OnAttestation(ctx context.Context, a ethpb.Att, disparity time
if err := helpers.ValidateSlotTargetEpoch(a.GetData()); err != nil {
return err
}
tgt := ethpb.CopyCheckpoint(a.GetData().Target)
tgt := a.GetData().Target.Copy()

// Note that target root check is ignored here because it was performed in sync's validation pipeline:
// validate_aggregate_proof.go and validate_beacon_attestation.go
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/operations/attestations/kv/aggregated.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func (c *AttCaches) SaveAggregatedAttestation(att ethpb.Att) error {
if err != nil {
return errors.Wrap(err, "could not create attestation ID")
}
copiedAtt := att.Copy()
copiedAtt := att.Clone()

c.aggregatedAttLock.Lock()
defer c.aggregatedAttLock.Unlock()
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/operations/attestations/kv/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (c *AttCaches) SaveBlockAttestation(att ethpb.Att) error {
}
}

c.blockAtt[id] = append(atts, att.Copy())
c.blockAtt[id] = append(atts, att.Clone())

return nil
}
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/operations/attestations/kv/forkchoice.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (c *AttCaches) ForkchoiceAttestations() []ethpb.Att {

atts := make([]ethpb.Att, 0, len(c.forkchoiceAtt))
for _, att := range c.forkchoiceAtt {
atts = append(atts, att.Copy())
atts = append(atts, att.Clone())
}

return atts
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/operations/attestations/kv/unaggregated.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (c *AttCaches) UnaggregatedAttestations() ([]ethpb.Att, error) {
return nil, err
}
if !seen {
atts = append(atts, att.Copy())
atts = append(atts, att.Clone())
}
}
return atts, nil
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/operations/attestations/prepare_forkchoice.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (s *Service) batchForkChoiceAtts(ctx context.Context) error {
func (s *Service) aggregateAndSaveForkChoiceAtts(atts []ethpb.Att) error {
clonedAtts := make([]ethpb.Att, len(atts))
for i, a := range atts {
clonedAtts[i] = a.Copy()
clonedAtts[i] = a.Clone()
}
aggregatedAtts, err := attaggregation.Aggregate(clonedAtts)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/rpc/prysm/v1alpha1/validator/attester.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (vs *Server) ProposeAttestation(ctx context.Context, att *ethpb.Attestation
}

go func() {
attCopy := ethpb.CopyAttestation(att)
attCopy := att.Copy()
if err := vs.AttPool.SaveUnaggregatedAttestation(attCopy); err != nil {
log.WithError(err).Error("Could not save unaggregated attestation")
return
Expand Down Expand Up @@ -84,7 +84,7 @@ func (vs *Server) ProposeAttestationElectra(ctx context.Context, att *ethpb.Atte

go func() {
ctx = trace.NewContext(context.Background(), trace.FromContext(ctx))
attCopy := ethpb.CopyAttestationElectra(att)
attCopy := att.Copy()
if err := vs.AttPool.SaveUnaggregatedAttestation(attCopy); err != nil {
log.WithError(err).Error("Could not save unaggregated attestation")
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func BenchmarkProposerAtts_sortByProfitability(b *testing.B) {
runner := func(atts []ethpb.Att) {
attsCopy := make(proposerAtts, len(atts))
for i, att := range atts {
attsCopy[i] = ethpb.CopyAttestation(att.(*ethpb.Attestation))
attsCopy[i] = att.(*ethpb.Attestation).Copy()
}
_, err := attsCopy.sortByProfitability()
require.NoError(b, err, "Could not sort attestations by profitability")
Expand Down
20 changes: 18 additions & 2 deletions beacon-chain/state/state-native/getters_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ func (b *BeaconState) PreviousEpochAttestations() ([]*ethpb.PendingAttestation,
// previousEpochAttestationsVal corresponding to blocks on the beacon chain.
// This assumes that a lock is already held on BeaconState.
func (b *BeaconState) previousEpochAttestationsVal() []*ethpb.PendingAttestation {
return ethpb.CopyPendingAttestationSlice(b.previousEpochAttestations)
if b.previousEpochAttestations == nil {
return nil
}

res := make([]*ethpb.PendingAttestation, len(b.previousEpochAttestations))
for i := 0; i < len(res); i++ {
res[i] = b.previousEpochAttestations[i].Copy()
}
return res
}

// CurrentEpochAttestations corresponding to blocks on the beacon chain.
Expand All @@ -46,5 +54,13 @@ func (b *BeaconState) CurrentEpochAttestations() ([]*ethpb.PendingAttestation, e
// currentEpochAttestations corresponding to blocks on the beacon chain.
// This assumes that a lock is already held on BeaconState.
func (b *BeaconState) currentEpochAttestationsVal() []*ethpb.PendingAttestation {
return ethpb.CopyPendingAttestationSlice(b.currentEpochAttestations)
if b.currentEpochAttestations == nil {
return nil
}

res := make([]*ethpb.PendingAttestation, len(b.currentEpochAttestations))
for i := 0; i < len(res); i++ {
res[i] = b.currentEpochAttestations[i].Copy()
}
return res
}
6 changes: 3 additions & 3 deletions beacon-chain/state/state-native/getters_checkpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (b *BeaconState) PreviousJustifiedCheckpoint() *ethpb.Checkpoint {
// previousJustifiedCheckpointVal denoting an epoch and block root.
// This assumes that a lock is already held on BeaconState.
func (b *BeaconState) previousJustifiedCheckpointVal() *ethpb.Checkpoint {
return ethpb.CopyCheckpoint(b.previousJustifiedCheckpoint)
return b.previousJustifiedCheckpoint.Copy()
}

// CurrentJustifiedCheckpoint denoting an epoch and block root.
Expand All @@ -65,7 +65,7 @@ func (b *BeaconState) CurrentJustifiedCheckpoint() *ethpb.Checkpoint {
// currentJustifiedCheckpointVal denoting an epoch and block root.
// This assumes that a lock is already held on BeaconState.
func (b *BeaconState) currentJustifiedCheckpointVal() *ethpb.Checkpoint {
return ethpb.CopyCheckpoint(b.currentJustifiedCheckpoint)
return b.currentJustifiedCheckpoint.Copy()
}

// MatchCurrentJustifiedCheckpoint returns true if input justified checkpoint matches
Expand Down Expand Up @@ -109,7 +109,7 @@ func (b *BeaconState) FinalizedCheckpoint() *ethpb.Checkpoint {
// finalizedCheckpointVal denoting an epoch and block root.
// This assumes that a lock is already held on BeaconState.
func (b *BeaconState) finalizedCheckpointVal() *ethpb.Checkpoint {
return ethpb.CopyCheckpoint(b.finalizedCheckpoint)
return b.finalizedCheckpoint.Copy()
}

// FinalizedCheckpointEpoch returns the epoch value of the finalized checkpoint.
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/state/state-native/references_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ func TestStateReferenceCopy_NoUnexpectedAttestationsMutation(t *testing.T) {
currEpochAtts, err = state.CurrentEpochAttestations()
require.NoError(t, err)
for i := range currEpochAtts {
att := ethpb.CopyPendingAttestation(currEpochAtts[i])
att := currEpochAtts[i].Copy()
att.AggregationBits = bitfield.NewBitlist(3)
currEpochAtts[i] = att
}
Expand All @@ -803,7 +803,7 @@ func TestStateReferenceCopy_NoUnexpectedAttestationsMutation(t *testing.T) {
prevEpochAtts, err = state.PreviousEpochAttestations()
require.NoError(t, err)
for i := range prevEpochAtts {
att := ethpb.CopyPendingAttestation(prevEpochAtts[i])
att := prevEpochAtts[i].Copy()
att.AggregationBits = bitfield.NewBitlist(3)
prevEpochAtts[i] = att
}
Expand Down
4 changes: 4 additions & 0 deletions proto/prysm/v1alpha1/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,14 @@ ssz_proto_files(
go_test(
name = "go_default_test",
srcs = [
"attestation_fuzz_test.go",
"cloners_test.go",
"export_test.go",
],
embed = [":go_default_library"],
deps = [
"//testing/assert:go_default_library",
"//testing/require:go_default_library",
"@com_github_google_gofuzz//:go_default_library",
],
)
131 changes: 124 additions & 7 deletions proto/prysm/v1alpha1/attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
ssz "github.com/prysmaticlabs/fastssz"
"github.com/prysmaticlabs/go-bitfield"
"github.com/prysmaticlabs/prysm/v5/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/v5/encoding/bytesutil"
"github.com/prysmaticlabs/prysm/v5/runtime/version"
"google.golang.org/protobuf/proto"
)
Expand All @@ -15,7 +16,7 @@ type Att interface {
ssz.Unmarshaler
ssz.HashRoot
Version() int
Copy() Att
Clone() Att
GetAggregationBits() bitfield.Bitlist
GetData() *AttestationData
CommitteeBitsVal() bitfield.Bitfield
Expand Down Expand Up @@ -68,14 +69,51 @@ type AttSlashing interface {
SecondAttestation() IndexedAtt
}

// Copy --
func (cp *Checkpoint) Copy() *Checkpoint {
if cp == nil {
return nil
}
return &Checkpoint{
Epoch: cp.Epoch,
Root: bytesutil.SafeCopyBytes(cp.Root),
}
}

// Copy --
func (attData *AttestationData) Copy() *AttestationData {
if attData == nil {
return nil
}
return &AttestationData{
Slot: attData.Slot,
CommitteeIndex: attData.CommitteeIndex,
BeaconBlockRoot: bytesutil.SafeCopyBytes(attData.BeaconBlockRoot),
Source: attData.Source.Copy(),
Target: attData.Target.Copy(),
}
}

// Version --
func (a *Attestation) Version() int {
return version.Phase0
}

// Clone --
func (a *Attestation) Clone() Att {
return a.Copy()
}

// Copy --
func (a *Attestation) Copy() Att {
return CopyAttestation(a)
func (att *Attestation) Copy() *Attestation {
if att == nil {
return nil
}
return &Attestation{
AggregationBits: bytesutil.SafeCopyBytes(att.AggregationBits),
Data: att.Data.Copy(),
Signature: bytesutil.SafeCopyBytes(att.Signature),
}
}

// CommitteeBitsVal --
Expand All @@ -90,9 +128,22 @@ func (a *PendingAttestation) Version() int {
return version.Phase0
}

// Clone --
func (a *PendingAttestation) Clone() Att {
return a.Copy()
}

// Copy --
func (a *PendingAttestation) Copy() Att {
return CopyPendingAttestation(a)
func (a *PendingAttestation) Copy() *PendingAttestation {
if a == nil {
return nil
}
return &PendingAttestation{
AggregationBits: bytesutil.SafeCopyBytes(a.AggregationBits),
Data: a.Data.Copy(),
InclusionDelay: a.InclusionDelay,
ProposerIndex: a.ProposerIndex,
}
}

// CommitteeBitsVal --
Expand All @@ -110,9 +161,22 @@ func (a *AttestationElectra) Version() int {
return version.Electra
}

// Clone --
func (a *AttestationElectra) Clone() Att {
return a.Copy()
}

// Copy --
func (a *AttestationElectra) Copy() Att {
return CopyAttestationElectra(a)
func (att *AttestationElectra) Copy() *AttestationElectra {
if att == nil {
return nil
}
return &AttestationElectra{
AggregationBits: bytesutil.SafeCopyBytes(att.AggregationBits),
CommitteeBits: bytesutil.SafeCopyBytes(att.CommitteeBits),
Data: att.Data.Copy(),
Signature: bytesutil.SafeCopyBytes(att.Signature),
}
}

// CommitteeBitsVal --
Expand All @@ -130,6 +194,38 @@ func (a *IndexedAttestationElectra) Version() int {
return version.Electra
}

// Copy --
func (indexedAtt *IndexedAttestation) Copy() *IndexedAttestation {
var indices []uint64
if indexedAtt == nil {
return nil
} else if indexedAtt.AttestingIndices != nil {
indices = make([]uint64, len(indexedAtt.AttestingIndices))
copy(indices, indexedAtt.AttestingIndices)
}
return &IndexedAttestation{
AttestingIndices: indices,
Data: indexedAtt.Data.Copy(),
Signature: bytesutil.SafeCopyBytes(indexedAtt.Signature),
}
}

// Copy --
func (indexedAtt *IndexedAttestationElectra) Copy() *IndexedAttestationElectra {
var indices []uint64
if indexedAtt == nil {
return nil
} else if indexedAtt.AttestingIndices != nil {
indices = make([]uint64, len(indexedAtt.AttestingIndices))
copy(indices, indexedAtt.AttestingIndices)
}
return &IndexedAttestationElectra{
AttestingIndices: indices,
Data: indexedAtt.Data.Copy(),
Signature: bytesutil.SafeCopyBytes(indexedAtt.Signature),
}
}

// Version --
func (a *AttesterSlashing) Version() int {
return version.Phase0
Expand Down Expand Up @@ -160,6 +256,27 @@ func (a *AttesterSlashingElectra) SecondAttestation() IndexedAtt {
return a.Attestation_2
}

func (a *AttesterSlashing) Copy() *AttesterSlashing {
if a == nil {
return nil
}
return &AttesterSlashing{
Attestation_1: a.Attestation_1.Copy(),
Attestation_2: a.Attestation_2.Copy(),
}
}

// Copy --
func (a *AttesterSlashingElectra) Copy() *AttesterSlashingElectra {
if a == nil {
return nil
}
return &AttesterSlashingElectra{
Attestation_1: a.Attestation_1.Copy(),
Attestation_2: a.Attestation_2.Copy(),
}
}

// Version --
func (a *AggregateAttestationAndProof) Version() int {
return version.Phase0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ func AggregatePair(a1, a2 *ethpb.Attestation) (*ethpb.Attestation, error) {
return nil, aggregation.ErrBitsOverlap
}

baseAtt := ethpb.CopyAttestation(a1)
newAtt := ethpb.CopyAttestation(a2)
baseAtt := a1.Copy()
newAtt := a2.Copy()
if newAtt.AggregationBits.Count() > baseAtt.AggregationBits.Count() {
baseAtt, newAtt = newAtt, baseAtt
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (al attList) aggregate(coverage bitfield.Bitlist) (*ethpb.Attestation, erro
}
return &ethpb.Attestation{
AggregationBits: coverage,
Data: ethpb.CopyAttestationData(al[0].GetData()),
Data: al[0].GetData().Copy(),
Signature: aggregateSignatures(signs).Marshal(),
}, nil
}
Expand Down Expand Up @@ -167,7 +167,7 @@ func aggregateAttestations(atts []ethpb.Att, keys []int, coverage *bitfield.Bitl
}
signs = append(signs, sig)
if i == 0 {
data = ethpb.CopyAttestationData(atts[idx].GetData())
data = atts[idx].GetData().Copy()
targetIdx = idx
}
}
Expand Down
Loading

0 comments on commit 4d823ac

Please sign in to comment.