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

Check that the headers are equal while ignoring the signature for proposer slashings #7252

Merged
merged 16 commits into from
Sep 18, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
fcd0dff
Check that the headers are equal while ignoring the signature for pro…
prestonvanloon Sep 16, 2020
4c430d3
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 16, 2020
3158250
fix test
prestonvanloon Sep 16, 2020
23a175c
Merge branch 'fix-proposer-slashing-issue' of github.com:prysmaticlab…
prestonvanloon Sep 16, 2020
de22345
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 16, 2020
39d6ecd
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
b50da6c
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
eb63e0a
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
cc05b93
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
c2cca03
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
54b9246
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
13e6798
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
b93e8b9
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
c0a3f85
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
7f7b1bf
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 17, 2020
a2518ba
Merge refs/heads/master into fix-proposer-slashing-issue
prylabs-bulldozer[bot] Sep 18, 2020
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/core/blocks/proposer_slashing.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func VerifyProposerSlashing(
if pIdx != slashing.Header_2.Header.ProposerIndex {
return fmt.Errorf("mismatched indices, received %d == %d", slashing.Header_1.Header.ProposerIndex, slashing.Header_2.Header.ProposerIndex)
}
if proto.Equal(slashing.Header_1, slashing.Header_2) {
if proto.Equal(slashing.Header_1.Header, slashing.Header_2.Header) {
return errors.New("expected slashing headers to differ")
}
proposer, err := beaconState.ValidatorAtIndexReadOnly(slashing.Header_1.Header.ProposerIndex)
Expand Down
73 changes: 73 additions & 0 deletions beacon-chain/core/blocks/proposer_slashing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state"
pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
"github.com/prysmaticlabs/prysm/shared/bls"
"github.com/prysmaticlabs/prysm/shared/bytesutil"
"github.com/prysmaticlabs/prysm/shared/params"
"github.com/prysmaticlabs/prysm/shared/testutil"
Expand Down Expand Up @@ -99,13 +100,15 @@ func TestProcessProposerSlashings_ValidatorNotSlashable(t *testing.T) {
Header: &ethpb.BeaconBlockHeader{
ProposerIndex: 0,
Slot: 0,
BodyRoot: []byte("foo"),
},
Signature: bytesutil.PadTo([]byte("A"), 96),
},
Header_2: &ethpb.SignedBeaconBlockHeader{
Header: &ethpb.BeaconBlockHeader{
ProposerIndex: 0,
Slot: 0,
BodyRoot: []byte("bar"),
},
Signature: bytesutil.PadTo([]byte("B"), 96),
},
Expand Down Expand Up @@ -181,3 +184,73 @@ func TestProcessProposerSlashings_AppliesCorrectStatus(t *testing.T) {
newStateVals[1].ExitEpoch, beaconState.Validators()[1].ExitEpoch)
}
}

func TestVerifyProposerSlashing(t *testing.T) {
type args struct {
beaconState *stateTrie.BeaconState
slashing *ethpb.ProposerSlashing
}

beaconState, _ := testutil.DeterministicGenesisState(t, 2)
currentSlot := uint64(0)
require.NoError(t, beaconState.SetSlot(currentSlot))

tests := []struct {
name string
args args
wantErr string
}{
{
name: "same header, no signatures",
args: args{
slashing: &ethpb.ProposerSlashing{
Header_1: &ethpb.SignedBeaconBlockHeader{
Header: &ethpb.BeaconBlockHeader{
ProposerIndex: 1,
Slot: 0,
},
},
Header_2: &ethpb.SignedBeaconBlockHeader{
Header: &ethpb.BeaconBlockHeader{
ProposerIndex: 1,
Slot: 0,
},
},
},
beaconState: beaconState,
},
wantErr: "expected slashing headers to differ",
},
{ // Regression test for https://github.com/sigp/beacon-fuzz/issues/74
name: "same header, different signatures",
args: args{
slashing: &ethpb.ProposerSlashing{
Header_1: &ethpb.SignedBeaconBlockHeader{
Header: &ethpb.BeaconBlockHeader{
ProposerIndex: 1,
Slot: 0,
},
Signature: bls.RandKey().Sign([]byte("foo")).Marshal(),
},
Header_2: &ethpb.SignedBeaconBlockHeader{
Header: &ethpb.BeaconBlockHeader{
ProposerIndex: 1,
Slot: 0,
},
Signature: bls.RandKey().Sign([]byte("bar")).Marshal(),
},
},
beaconState: beaconState,
},
wantErr: "expected slashing headers to differ",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
testutil.ResetCache()
if err := blocks.VerifyProposerSlashing(tt.args.beaconState, tt.args.slashing); (err != nil || tt.wantErr != "") && err.Error() != tt.wantErr {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use our assertion library over here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the assertion library support this behavior?

I want: "if the test expects an error, assert the error is as expected"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet, it does have assert.ErrorContains which would be the closest to it. But yeah maybe not worth it to change now.

t.Errorf("VerifyProposerSlashing() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}