Compare commits

...

7 Commits

Author SHA1 Message Date
William Banfield
07e226207e fix capitalization in comment 2022-01-19 12:09:51 -05:00
William Banfield
800ecdbb8c Merge branch 'master' into wb/remove-privval-reseuse 2022-01-19 11:36:30 -05:00
William Banfield
c4a569d9fe udpate comments 2022-01-19 11:34:36 -05:00
mergify[bot]
0fb85f8809 Merge branch 'master' into wb/remove-privval-reseuse 2022-01-19 01:04:10 +00:00
William Banfield
f73bfb11b2 Merge branch 'master' into wb/remove-privval-reseuse 2022-01-18 19:52:26 -05:00
William Banfield
7a0bcf6641 fix test failure 2022-01-18 15:29:37 -05:00
William Banfield
07fc54ab81 privval: do not use old proposal timestamp 2022-01-18 11:25:07 -05:00
2 changed files with 18 additions and 80 deletions

View File

@@ -99,14 +99,14 @@ type FilePVLastSignState struct {
filePath string
}
// CheckHRS checks the given height, round, step (HRS) against that of the
// checkHRS checks the given height, round, step (HRS) against that of the
// FilePVLastSignState. It returns an error if the arguments constitute a regression,
// or if they match but the SignBytes are empty.
// The returned boolean indicates whether the last Signature should be reused -
// it returns true if the HRS matches the arguments and the SignBytes are not empty (indicating
// we have already signed for this HRS, and can reuse the existing signature).
// It panics if the HRS matches the arguments, there's a SignBytes, but no Signature.
func (lss *FilePVLastSignState) CheckHRS(height int64, round int32, step int8) (bool, error) {
func (lss *FilePVLastSignState) checkHRS(height int64, round int32, step int8) (bool, error) {
if lss.Height > height {
return false, fmt.Errorf("height regression. Got %v, last height %v", height, lss.Height)
@@ -345,7 +345,7 @@ func (pv *FilePV) signVote(chainID string, vote *tmproto.Vote) error {
round := vote.Round
lss := pv.LastSignState
sameHRS, err := lss.CheckHRS(height, round, step)
sameHRS, err := lss.checkHRS(height, round, step)
if err != nil {
return err
}
@@ -388,14 +388,12 @@ func (pv *FilePV) signVote(chainID string, vote *tmproto.Vote) error {
}
// signProposal checks if the proposal is good to sign and sets the proposal signature.
// It may need to set the timestamp as well if the proposal is otherwise the same as
// a previously signed proposal ie. we crashed after signing but before the proposal hit the WAL).
func (pv *FilePV) signProposal(chainID string, proposal *tmproto.Proposal) error {
height, round, step := proposal.Height, proposal.Round, stepPropose
lss := pv.LastSignState
sameHRS, err := lss.CheckHRS(height, round, step)
sameHRS, err := lss.checkHRS(height, round, step)
if err != nil {
return err
}
@@ -405,23 +403,12 @@ func (pv *FilePV) signProposal(chainID string, proposal *tmproto.Proposal) error
// We might crash before writing to the wal,
// causing us to try to re-sign for the same HRS.
// If signbytes are the same, use the last signature.
// If they only differ by timestamp, use last timestamp and signature
// Otherwise, return error
if sameHRS {
if bytes.Equal(signBytes, lss.SignBytes) {
proposal.Signature = lss.Signature
} else {
timestamp, ok, err := checkProposalsOnlyDifferByTimestamp(lss.SignBytes, signBytes)
if err != nil {
return err
}
if !ok {
return errors.New("conflicting data")
}
proposal.Timestamp = timestamp
proposal.Signature = lss.Signature
return nil
if !bytes.Equal(signBytes, lss.SignBytes) {
return errors.New("conflicting data")
}
proposal.Signature = lss.Signature
return nil
}
// It passed the checks. Sign the proposal
@@ -467,23 +454,3 @@ func checkVotesOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) (time.T
return lastTime, proto.Equal(&newVote, &lastVote), nil
}
// returns the timestamp from the lastSignBytes.
// returns true if the only difference in the proposals is their timestamp
func checkProposalsOnlyDifferByTimestamp(lastSignBytes, newSignBytes []byte) (time.Time, bool, error) {
var lastProposal, newProposal tmproto.CanonicalProposal
if err := protoio.UnmarshalDelimited(lastSignBytes, &lastProposal); err != nil {
return time.Time{}, false, fmt.Errorf("LastSignBytes cannot be unmarshalled into proposal: %w", err)
}
if err := protoio.UnmarshalDelimited(newSignBytes, &newProposal); err != nil {
return time.Time{}, false, fmt.Errorf("signBytes cannot be unmarshalled into proposal: %w", err)
}
lastTime := lastProposal.Timestamp
// set the times to the same value and check equality
now := tmtime.Now()
lastProposal.Timestamp = now
newProposal.Timestamp = now
return lastTime, proto.Equal(&newProposal, &lastProposal), nil
}

View File

@@ -234,7 +234,8 @@ func TestSignProposal(t *testing.T) {
height, round := int64(10), int32(1)
// sign a proposal for first time
proposal := newProposal(height, round, block1)
ts := tmtime.Now()
proposal := newProposal(height, round, block1, ts)
pbp := proposal.ToProto()
err = privVal.SignProposal(ctx, "mychainid", pbp)
@@ -246,23 +247,17 @@ func TestSignProposal(t *testing.T) {
// now try some bad Proposals
cases := []*types.Proposal{
newProposal(height, round-1, block1), // round regression
newProposal(height-1, round, block1), // height regression
newProposal(height-2, round+4, block1), // height regression and different round
newProposal(height, round, block2), // different block
newProposal(height, round-1, block1, ts), // round regression
newProposal(height-1, round, block1, ts), // height regression
newProposal(height-2, round+4, block1, ts), // height regression and different round
newProposal(height, round, block2, ts), // different block
newProposal(height, round, block1, ts.Add(time.Second)), // different timestamp
}
for _, c := range cases {
assert.Error(t, privVal.SignProposal(ctx, "mychainid", c.ToProto()),
assert.Errorf(t, privVal.SignProposal(ctx, "mychainid", c.ToProto()),
"expected error on signing conflicting proposal")
}
// try signing a proposal with a different time stamp
sig := proposal.Signature
proposal.Timestamp = proposal.Timestamp.Add(time.Duration(1000))
err = privVal.SignProposal(ctx, "mychainid", pbp)
assert.NoError(t, err)
assert.Equal(t, sig, proposal.Signature)
}
func TestDifferByTimestamp(t *testing.T) {
@@ -277,33 +272,9 @@ func TestDifferByTimestamp(t *testing.T) {
privVal, err := GenFilePV(tempKeyFile.Name(), tempStateFile.Name(), "")
require.NoError(t, err)
randbytes := tmrand.Bytes(tmhash.Size)
block1 := types.BlockID{Hash: randbytes, PartSetHeader: types.PartSetHeader{Total: 5, Hash: randbytes}}
height, round := int64(10), int32(1)
chainID := "mychainid"
// test proposal
{
proposal := newProposal(height, round, block1)
pb := proposal.ToProto()
err := privVal.SignProposal(ctx, chainID, pb)
require.NoError(t, err, "expected no error signing proposal")
signBytes := types.ProposalSignBytes(chainID, pb)
sig := proposal.Signature
timeStamp := proposal.Timestamp
// manipulate the timestamp. should get changed back
pb.Timestamp = pb.Timestamp.Add(time.Millisecond)
var emptySig []byte
proposal.Signature = emptySig
err = privVal.SignProposal(ctx, "mychainid", pb)
require.NoError(t, err, "expected no error on signing same proposal")
assert.Equal(t, timeStamp, pb.Timestamp)
assert.Equal(t, signBytes, types.ProposalSignBytes(chainID, pb))
assert.Equal(t, sig, proposal.Signature)
}
// test vote
{
voteType := tmproto.PrevoteType
@@ -343,11 +314,11 @@ func newVote(addr types.Address, idx int32, height int64, round int32,
}
}
func newProposal(height int64, round int32, blockID types.BlockID) *types.Proposal {
func newProposal(height int64, round int32, blockID types.BlockID, t time.Time) *types.Proposal {
return &types.Proposal{
Height: height,
Round: round,
BlockID: blockID,
Timestamp: tmtime.Now(),
Timestamp: t,
}
}