From 2586be29ac6f0873deb9c396bc47b91564b02123 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Thu, 21 Jan 2021 10:38:51 -0800 Subject: [PATCH] Refactor `on_attestation` return signature (#8310) * Update func on_attestation's return signature * Add return --- beacon-chain/blockchain/process_attestation.go | 18 +++++++++--------- .../blockchain/process_attestation_test.go | 8 ++------ beacon-chain/blockchain/receive_attestation.go | 3 +-- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/beacon-chain/blockchain/process_attestation.go b/beacon-chain/blockchain/process_attestation.go index 8fb8708b31c7..47cee31ca0f0 100644 --- a/beacon-chain/blockchain/process_attestation.go +++ b/beacon-chain/blockchain/process_attestation.go @@ -39,15 +39,15 @@ var ErrTargetRootNotInDB = errors.New("target root does not exist in db") // // # Update latest messages for attesting indices // update_latest_messages(store, indexed_attestation.attesting_indices, attestation) -func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) ([]uint64, error) { +func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) error { ctx, span := trace.StartSpan(ctx, "blockChain.onAttestation") defer span.End() if err := helpers.ValidateNilAttestation(a); err != nil { - return nil, err + return err } if err := helpers.ValidateSlotTargetEpoch(a.Data); err != nil { - return nil, err + return err } tgt := stateTrie.CopyCheckpoint(a.Data.Target) @@ -59,19 +59,19 @@ func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) ([]ui // save it to the cache. baseState, err := s.getAttPreState(ctx, tgt) if err != nil { - return nil, err + return err } genesisTime := baseState.GenesisTime() // Verify attestation target is from current epoch or previous epoch. if err := s.verifyAttTargetEpoch(ctx, genesisTime, uint64(timeutils.Now().Unix()), tgt); err != nil { - return nil, err + return err } // Verify attestation beacon block is known and not from the future. if err := s.verifyBeaconBlock(ctx, a.Data); err != nil { - return nil, errors.Wrap(err, "could not verify attestation beacon block") + return errors.Wrap(err, "could not verify attestation beacon block") } // Note that LMG GHOST and FFG consistency check is ignored because it was performed in sync's validation pipeline: @@ -79,13 +79,13 @@ func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) ([]ui // Verify attestations can only affect the fork choice of subsequent slots. if err := helpers.VerifySlotTime(genesisTime, a.Data.Slot+1, params.BeaconNetworkConfig().MaximumGossipClockDisparity); err != nil { - return nil, err + return err } // Use the target state to validate attestation and calculate the committees. indexedAtt, err := s.verifyAttestationIndices(ctx, baseState, a) if err != nil { - return nil, err + return err } // Note that signature verification is ignored here because it was performed in sync's validation pipeline: @@ -95,5 +95,5 @@ func (s *Service) onAttestation(ctx context.Context, a *ethpb.Attestation) ([]ui // Update forkchoice store with the new attestation for updating weight. s.forkChoiceStore.ProcessAttestation(ctx, indexedAtt.AttestingIndices, bytesutil.ToBytes32(a.Data.BeaconBlockRoot), a.Data.Target.Epoch) - return indexedAtt.AttestingIndices, nil + return nil } diff --git a/beacon-chain/blockchain/process_attestation_test.go b/beacon-chain/blockchain/process_attestation_test.go index 5ae42b390cfa..a07d8fd0091f 100644 --- a/beacon-chain/blockchain/process_attestation_test.go +++ b/beacon-chain/blockchain/process_attestation_test.go @@ -114,7 +114,7 @@ func TestStore_OnAttestation_ErrorConditions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := service.onAttestation(ctx, tt.a) + err := service.onAttestation(ctx, tt.a) if tt.wantedErr != "" { assert.ErrorContains(t, tt.wantedErr, err) } else { @@ -146,11 +146,7 @@ func TestStore_OnAttestation_Ok(t *testing.T) { require.NoError(t, err) require.NoError(t, service.beaconDB.SaveState(ctx, copied, tRoot)) require.NoError(t, service.forkChoiceStore.ProcessBlock(ctx, 0, tRoot, tRoot, tRoot, 1, 1)) - indices, err := service.onAttestation(ctx, att[0]) - require.NoError(t, err) - wanted, err := helpers.BeaconCommitteeFromState(copied, 0, 0) - require.NoError(t, err) - require.DeepEqual(t, wanted, indices) + require.NoError(t, service.onAttestation(ctx, att[0])) } func TestStore_SaveCheckpointState(t *testing.T) { diff --git a/beacon-chain/blockchain/receive_attestation.go b/beacon-chain/blockchain/receive_attestation.go index 863fc9bfe3b1..e8a297b2ae1d 100644 --- a/beacon-chain/blockchain/receive_attestation.go +++ b/beacon-chain/blockchain/receive_attestation.go @@ -35,8 +35,7 @@ func (s *Service) ReceiveAttestationNoPubsub(ctx context.Context, att *ethpb.Att ctx, span := trace.StartSpan(ctx, "beacon-chain.blockchain.ReceiveAttestationNoPubsub") defer span.End() - _, err := s.onAttestation(ctx, att) - if err != nil { + if err := s.onAttestation(ctx, att); err != nil { return errors.Wrap(err, "could not process attestation") }