Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

moving cloners to attestation.go and adding fuzzing #14254

Merged
merged 3 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Loading