Skip to content

Commit

Permalink
Unnecessary Slice-to-Slice Conversion analyzer (#7321)
Browse files Browse the repository at this point in the history
* analyzer with tests
* fix bazel file
* modify analyzer to fix build issues
* add analyzer to tool chain
* remove arrays from inspections
* fix redundant [:] operator
* Merge branch 'master' into use-slice-directly
* Merge branch 'master' into use-slice-directly
* fix another inspection
* add package-level comment
  • Loading branch information
rkapka authored Sep 23, 2020
1 parent 347aa14 commit dca93ce
Show file tree
Hide file tree
Showing 48 changed files with 251 additions and 87 deletions.
1 change: 1 addition & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ nogo(
"//tools/analyzers/comparesame:go_tool_library",
"//tools/analyzers/shadowpredecl:go_tool_library",
"//tools/analyzers/nop:go_tool_library",
"//tools/analyzers/slicedirect:go_tool_library",
] + select({
# nogo checks that fail with coverage enabled.
":coverage_enabled": [],
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/blockchain/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ func logBlockSyncStatus(block *ethpb.BeaconBlock, blockRoot [32]byte, finalized
"block": fmt.Sprintf("0x%s...", hex.EncodeToString(blockRoot[:])[:8]),
"epoch": helpers.SlotToEpoch(block.Slot),
"finalizedEpoch": finalized.Epoch,
"finalizedRoot": fmt.Sprintf("0x%s...", hex.EncodeToString(finalized.Root[:])[:8]),
"finalizedRoot": fmt.Sprintf("0x%s...", hex.EncodeToString(finalized.Root)[:8]),
}).Info("Synced new block")
}
2 changes: 1 addition & 1 deletion beacon-chain/blockchain/process_block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestStore_OnBlock(t *testing.T) {
require.NoError(t, service.beaconDB.SaveStateSummary(ctx, &pb.StateSummary{Slot: st.Slot(), Root: randomParentRoot[:]}))
require.NoError(t, service.beaconDB.SaveState(ctx, st.Copy(), randomParentRoot))
randomParentRoot2 := roots[1]
require.NoError(t, service.beaconDB.SaveStateSummary(ctx, &pb.StateSummary{Slot: st.Slot(), Root: randomParentRoot2[:]}))
require.NoError(t, service.beaconDB.SaveStateSummary(ctx, &pb.StateSummary{Slot: st.Slot(), Root: randomParentRoot2}))
require.NoError(t, service.beaconDB.SaveState(ctx, st.Copy(), bytesutil.ToBytes32(randomParentRoot2)))

tests := []struct {
Expand Down
10 changes: 5 additions & 5 deletions beacon-chain/core/blocks/attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func TestProcessAttestations_OK(t *testing.T) {
require.NoError(t, err)
sigs[i] = sig
}
att.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
att.Signature = bls.AggregateSignatures(sigs).Marshal()

block := testutil.NewBeaconBlock()
block.Block.Body.Attestations = []*ethpb.Attestation{att}
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestProcessAggregatedAttestation_OverlappingBits(t *testing.T) {
require.NoError(t, err)
sigs[i] = sig
}
att1.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
att1.Signature = bls.AggregateSignatures(sigs).Marshal()

aggBits2 := bitfield.NewBitlist(4)
aggBits2.SetBitAt(1, true)
Expand All @@ -305,7 +305,7 @@ func TestProcessAggregatedAttestation_OverlappingBits(t *testing.T) {
require.NoError(t, err)
sigs[i] = sig
}
att2.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
att2.Signature = bls.AggregateSignatures(sigs).Marshal()

_, err = attaggregation.AggregatePair(att1, att2)
assert.ErrorContains(t, aggregation.ErrBitsOverlap.Error(), err)
Expand Down Expand Up @@ -347,7 +347,7 @@ func TestProcessAggregatedAttestation_NoOverlappingBits(t *testing.T) {
require.NoError(t, err)
sigs[i] = sig
}
att1.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
att1.Signature = bls.AggregateSignatures(sigs).Marshal()

aggBits2 := bitfield.NewBitlist(9)
aggBits2.SetBitAt(2, true)
Expand All @@ -370,7 +370,7 @@ func TestProcessAggregatedAttestation_NoOverlappingBits(t *testing.T) {
require.NoError(t, err)
sigs[i] = sig
}
att2.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
att2.Signature = bls.AggregateSignatures(sigs).Marshal()

aggregatedAtt, err := attaggregation.AggregatePair(att1, att2)
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/core/blocks/attester_slashing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestProcessAttesterSlashings_AppliesCorrectStatus(t *testing.T) {
sig0 := privKeys[0].Sign(signingRoot[:])
sig1 := privKeys[1].Sign(signingRoot[:])
aggregateSig := bls.AggregateSignatures([]bls.Signature{sig0, sig1})
att1.Signature = aggregateSig.Marshal()[:]
att1.Signature = aggregateSig.Marshal()

att2 := &ethpb.IndexedAttestation{
Data: &ethpb.AttestationData{
Expand All @@ -157,7 +157,7 @@ func TestProcessAttesterSlashings_AppliesCorrectStatus(t *testing.T) {
sig0 = privKeys[0].Sign(signingRoot[:])
sig1 = privKeys[1].Sign(signingRoot[:])
aggregateSig = bls.AggregateSignatures([]bls.Signature{sig0, sig1})
att2.Signature = aggregateSig.Marshal()[:]
att2.Signature = aggregateSig.Marshal()

slashings := []*ethpb.AttesterSlashing{
{
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/core/blocks/block_regression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestProcessAttesterSlashings_RegressionSlashableIndices(t *testing.T) {
aggSigs = append(aggSigs, sig)
}
aggregateSig := bls.AggregateSignatures(aggSigs)
att1.Signature = aggregateSig.Marshal()[:]
att1.Signature = aggregateSig.Marshal()

root2 := [32]byte{'d', 'o', 'u', 'b', 'l', 'e', '2'}
att2 := &ethpb.IndexedAttestation{
Expand All @@ -76,7 +76,7 @@ func TestProcessAttesterSlashings_RegressionSlashableIndices(t *testing.T) {
aggSigs = append(aggSigs, sig)
}
aggregateSig = bls.AggregateSignatures(aggSigs)
att2.Signature = aggregateSig.Marshal()[:]
att2.Signature = aggregateSig.Marshal()

slashings := []*ethpb.AttesterSlashing{
{
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/blocks/exit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func TestProcessVoluntaryExits_AppliesCorrectStatus(t *testing.T) {
priv := bls.RandKey()
val, err := state.ValidatorAtIndex(0)
require.NoError(t, err)
val.PublicKey = priv.PublicKey().Marshal()[:]
val.PublicKey = priv.PublicKey().Marshal()
require.NoError(t, state.UpdateValidatorAtIndex(0, val))
exits[0].Signature, err = helpers.ComputeDomainAndSign(state, helpers.CurrentEpoch(state), exits[0].Exit, params.BeaconConfig().DomainVoluntaryExit, priv)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/blocks/randao.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func ProcessRandao(
if err != nil {
return nil, err
}
if err := verifySignature(buf, proposerPub[:], body.RandaoReveal, domain); err != nil {
if err := verifySignature(buf, proposerPub, body.RandaoReveal, domain); err != nil {
return nil, errors.Wrap(err, "could not verify block randao")
}

Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/core/blocks/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func VerifyBlockSignature(beaconState *stateTrie.BeaconState, block *ethpb.Signe
return err
}
proposerPubKey := proposer.PublicKey
return helpers.VerifyBlockSigningRoot(block.Block, proposerPubKey[:], block.Signature, domain)
return helpers.VerifyBlockSigningRoot(block.Block, proposerPubKey, block.Signature, domain)
}

// BlockSignatureSet retrieves the block signature set from the provided block and its corresponding state.
Expand All @@ -97,7 +97,7 @@ func RandaoSignatureSet(beaconState *stateTrie.BeaconState,
if err != nil {
return nil, nil, err
}
set, err := retrieveSignatureSet(buf, proposerPub[:], body.RandaoReveal, domain)
set, err := retrieveSignatureSet(buf, proposerPub, body.RandaoReveal, domain)
if err != nil {
return nil, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/helpers/attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestAttestation_AggregateSignature(t *testing.T) {
for i := 0; i < 100; i++ {
priv := bls.RandKey()
pub := priv.PublicKey()
sig := priv.Sign(msg[:])
sig := priv.Sign(msg)
pubkeys = append(pubkeys, pub)
att := &ethpb.Attestation{Signature: sig.Marshal()}
atts = append(atts, att)
Expand Down
10 changes: 5 additions & 5 deletions beacon-chain/core/state/transition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func createFullBlockWithOperations(t *testing.T) (*beaconstate.BeaconState,
},
}
validators := beaconState.Validators()
validators[proposerSlashIdx].PublicKey = privKeys[proposerSlashIdx].PublicKey().Marshal()[:]
validators[proposerSlashIdx].PublicKey = privKeys[proposerSlashIdx].PublicKey().Marshal()
require.NoError(t, beaconState.SetValidators(validators))

mockRoot2 := [32]byte{'A'}
Expand All @@ -407,7 +407,7 @@ func createFullBlockWithOperations(t *testing.T) (*beaconstate.BeaconState,
sig0 := privKeys[0].Sign(hashTreeRoot[:])
sig1 := privKeys[1].Sign(hashTreeRoot[:])
aggregateSig := bls.AggregateSignatures([]bls.Signature{sig0, sig1})
att1.Signature = aggregateSig.Marshal()[:]
att1.Signature = aggregateSig.Marshal()

mockRoot3 := [32]byte{'B'}
att2 := &ethpb.IndexedAttestation{
Expand All @@ -425,7 +425,7 @@ func createFullBlockWithOperations(t *testing.T) (*beaconstate.BeaconState,
sig0 = privKeys[0].Sign(hashTreeRoot[:])
sig1 = privKeys[1].Sign(hashTreeRoot[:])
aggregateSig = bls.AggregateSignatures([]bls.Signature{sig0, sig1})
att2.Signature = aggregateSig.Marshal()[:]
att2.Signature = aggregateSig.Marshal()

attesterSlashings := []*ethpb.AttesterSlashing{
{
Expand Down Expand Up @@ -466,7 +466,7 @@ func createFullBlockWithOperations(t *testing.T) (*beaconstate.BeaconState,
sig := privKeys[indice].Sign(hashTreeRoot[:])
sigs[i] = sig
}
blockAtt.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
blockAtt.Signature = bls.AggregateSignatures(sigs).Marshal()

exit := &ethpb.SignedVoluntaryExit{
Exit: &ethpb.VoluntaryExit{
Expand Down Expand Up @@ -784,7 +784,7 @@ func TestProcessBlk_AttsBasedOnValidatorCount(t *testing.T) {
sig := privKeys[indice].Sign(hashTreeRoot[:])
sigs[i] = sig
}
att.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
att.Signature = bls.AggregateSignatures(sigs).Marshal()
atts[i] = att
}

Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/db/kv/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (kv *Store) DeleteStates(ctx context.Context, blockRoots [][32]byte) error
continue
}
// Safe guard against deleting genesis, finalized, head state.
if bytes.Equal(blockRoot[:], checkpoint.Root) || bytes.Equal(blockRoot[:], genesisBlockRoot) || bytes.Equal(blockRoot[:], headBlkRoot) {
if bytes.Equal(blockRoot, checkpoint.Root) || bytes.Equal(blockRoot, genesisBlockRoot) || bytes.Equal(blockRoot, headBlkRoot) {
return errors.New("cannot delete genesis, finalized, or head state")
}

Expand All @@ -203,7 +203,7 @@ func (kv *Store) DeleteStates(ctx context.Context, blockRoots [][32]byte) error
return err
}
indicesByBucket := createStateIndicesFromStateSlot(ctx, slot)
if err := deleteValueForIndices(ctx, indicesByBucket, blockRoot[:], tx); err != nil {
if err := deleteValueForIndices(ctx, indicesByBucket, blockRoot, tx); err != nil {
return errors.Wrap(err, "could not delete root for DB indices")
}

Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/p2p/fork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func TestDiscv5_AddRetrieveForkEntryENR(t *testing.T) {
if !bytes.Equal(resp.CurrentForkDigest, want[:]) {
t.Errorf("Wanted fork digest: %v, received %v", want, resp.CurrentForkDigest)
}
if !bytes.Equal(resp.NextForkVersion[:], nextForkVersion) {
if !bytes.Equal(resp.NextForkVersion, nextForkVersion) {
t.Errorf("Wanted next fork version: %v, received %v", nextForkVersion, resp.NextForkVersion)
}
assert.Equal(t, nextForkEpoch, resp.NextForkEpoch, "Unexpected next fork epoch")
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/powchain/deposit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func TestProcessDeposit_UnableToVerify(t *testing.T) {
deposits, keys, err := testutil.DeterministicDepositsAndKeys(1)
require.NoError(t, err)
sig := keys[0].Sign([]byte{'F', 'A', 'K', 'E'})
deposits[0].Data.Signature = sig.Marshal()[:]
deposits[0].Data.Signature = sig.Marshal()

trie, _, err := testutil.DepositTrieFromDeposits(deposits)
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/rpc/validator/aggregator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func generateAtt(state *beaconstate.BeaconState, index uint64, privKeys []bls.Se
sigs[i] = sig
}

att.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
att.Signature = bls.AggregateSignatures(sigs).Marshal()

return att, nil
}
Expand Down Expand Up @@ -290,7 +290,7 @@ func generateUnaggregatedAtt(state *beaconstate.BeaconState, index uint64, privK
sigs[i] = sig
}

att.Signature = bls.AggregateSignatures(sigs).Marshal()[:]
att.Signature = bls.AggregateSignatures(sigs).Marshal()

return att, nil
}
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/rpc/validator/attester.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation
}
targetRoot := make([]byte, 32)
if epochStartSlot == headState.Slot() {
targetRoot = headRoot[:]
targetRoot = headRoot
} else {
targetRoot, err = helpers.BlockRootAtSlot(headState, epochStartSlot)
if err != nil {
Expand All @@ -124,7 +124,7 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation
res = &ethpb.AttestationData{
Slot: req.Slot,
CommitteeIndex: req.CommitteeIndex,
BeaconBlockRoot: headRoot[:],
BeaconBlockRoot: headRoot,
Source: headState.CurrentJustifiedCheckpoint(),
Target: &ethpb.Checkpoint{
Epoch: targetEpoch,
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/rpc/validator/proposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (vs *Server) GetBlock(ctx context.Context, req *ethpb.BlockRequest) (*ethpb

blk := &ethpb.BeaconBlock{
Slot: req.Slot,
ParentRoot: parentRoot[:],
ParentRoot: parentRoot,
StateRoot: stateRoot,
ProposerIndex: idx,
Body: &ethpb.BeaconBlockBody{
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/rpc/validator/proposer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func TestProposer_ComputeStateRoot_OK(t *testing.T) {
proposerIdx, err := helpers.BeaconProposerIndex(beaconState)
require.NoError(t, err)
require.NoError(t, beaconState.SetSlot(beaconState.Slot()-1))
req.Block.Body.RandaoReveal = randaoReveal[:]
req.Block.Body.RandaoReveal = randaoReveal
currentEpoch := helpers.CurrentEpoch(beaconState)
req.Signature, err = helpers.ComputeDomainAndSign(beaconState, currentEpoch, req.Block, params.BeaconConfig().DomainBeaconProposer, privKeys[proposerIdx])
require.NoError(t, err)
Expand Down Expand Up @@ -1892,7 +1892,7 @@ func TestProposer_FilterAttestation(t *testing.T) {
sig := privKeys[indice].Sign(hashTreeRoot[:])
sigs[i] = sig
}
atts[i].Signature = bls.AggregateSignatures(sigs).Marshal()[:]
atts[i].Signature = bls.AggregateSignatures(sigs).Marshal()
}
return atts
},
Expand Down
12 changes: 6 additions & 6 deletions beacon-chain/rpc/validator/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ func TestWaitForActivation_ValidatorOriginallyExists(t *testing.T) {
priv1 := bls.RandKey()
priv2 := bls.RandKey()

pubKey1 := priv1.PublicKey().Marshal()[:]
pubKey2 := priv2.PublicKey().Marshal()[:]
pubKey1 := priv1.PublicKey().Marshal()
pubKey2 := priv2.PublicKey().Marshal()

beaconState := &pbp2p.BeaconState{
Slot: 4000,
Expand All @@ -138,7 +138,7 @@ func TestWaitForActivation_ValidatorOriginallyExists(t *testing.T) {
require.NoError(t, err)
signingRoot, err := helpers.ComputeSigningRoot(depData, domain)
require.NoError(t, err)
depData.Signature = priv1.Sign(signingRoot[:]).Marshal()[:]
depData.Signature = priv1.Sign(signingRoot[:]).Marshal()

deposit := &ethpb.Deposit{
Data: depData,
Expand Down Expand Up @@ -200,9 +200,9 @@ func TestWaitForActivation_MultipleStatuses(t *testing.T) {
priv2 := bls.RandKey()
priv3 := bls.RandKey()

pubKey1 := priv1.PublicKey().Marshal()[:]
pubKey2 := priv2.PublicKey().Marshal()[:]
pubKey3 := priv3.PublicKey().Marshal()[:]
pubKey1 := priv1.PublicKey().Marshal()
pubKey2 := priv2.PublicKey().Marshal()
pubKey3 := priv3.PublicKey().Marshal()

beaconState := &pbp2p.BeaconState{
Slot: 4000,
Expand Down
20 changes: 10 additions & 10 deletions beacon-chain/state/cloners.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func CopyProposerSlashings(slashings []*ethpb.ProposerSlashing) []*ethpb.Propose
for i, att := range slashings {
newSlashings[i] = CopyProposerSlashing(att)
}
return newSlashings[:]
return newSlashings
}

// CopyProposerSlashing copies the provided ProposerSlashing.
Expand Down Expand Up @@ -156,9 +156,9 @@ func CopyBeaconBlockHeader(header *ethpb.BeaconBlockHeader) *ethpb.BeaconBlockHe
return &ethpb.BeaconBlockHeader{
Slot: header.Slot,
ProposerIndex: header.ProposerIndex,
ParentRoot: parentRoot[:],
StateRoot: stateRoot[:],
BodyRoot: bodyRoot[:],
ParentRoot: parentRoot,
StateRoot: stateRoot,
BodyRoot: bodyRoot,
}
}

Expand All @@ -184,10 +184,10 @@ func CopyIndexedAttestation(indexedAtt *ethpb.IndexedAttestation) *ethpb.Indexed
return nil
} else if indexedAtt.AttestingIndices != nil {
indices = make([]uint64, len(indexedAtt.AttestingIndices))
copy(indices[:], indexedAtt.AttestingIndices)
copy(indices, indexedAtt.AttestingIndices)
}
return &ethpb.IndexedAttestation{
AttestingIndices: indices[:],
AttestingIndices: indices,
Data: CopyAttestationData(indexedAtt.Data),
Signature: bytesutil.SafeCopyBytes(indexedAtt.Signature),
}
Expand All @@ -202,7 +202,7 @@ func CopyAttestations(attestations []*ethpb.Attestation) []*ethpb.Attestation {
for i, att := range attestations {
newAttestations[i] = CopyAttestation(att)
}
return newAttestations[:]
return newAttestations
}

// CopyDeposits copies the provided deposit array.
Expand All @@ -214,7 +214,7 @@ func CopyDeposits(deposits []*ethpb.Deposit) []*ethpb.Deposit {
for i, dep := range deposits {
newDeposits[i] = CopyDeposit(dep)
}
return newDeposits[:]
return newDeposits
}

// CopyDeposit copies the provided deposit.
Expand Down Expand Up @@ -250,7 +250,7 @@ func CopySignedVoluntaryExits(exits []*ethpb.SignedVoluntaryExit) []*ethpb.Signe
for i, exit := range exits {
newExits[i] = CopySignedVoluntaryExit(exit)
}
return newExits[:]
return newExits
}

// CopySignedVoluntaryExit copies the provided SignedVoluntaryExit.
Expand All @@ -274,7 +274,7 @@ func CopyValidator(val *ethpb.Validator) *ethpb.Validator {
withdrawalCreds := make([]byte, len(val.WithdrawalCredentials))
copy(withdrawalCreds, val.WithdrawalCredentials)
return &ethpb.Validator{
PublicKey: pubKey[:],
PublicKey: pubKey,
WithdrawalCredentials: withdrawalCreds,
EffectiveBalance: val.EffectiveBalance,
Slashed: val.Slashed,
Expand Down
Loading

0 comments on commit dca93ce

Please sign in to comment.