Skip to content

Commit

Permalink
fix TestPrivvalVectors
Browse files Browse the repository at this point in the history
  • Loading branch information
melekes committed Jan 9, 2025
1 parent 12c0ad1 commit 8f82994
Show file tree
Hide file tree
Showing 7 changed files with 3 additions and 205 deletions.
139 changes: 0 additions & 139 deletions internal/consensus/pbts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,145 +529,6 @@ func TestPBTSTooFarInTheFutureProposal(t *testing.T) {
require.Nil(t, results.height2.prevote.BlockID.Hash)
}

// NOTE: Commented as the test makes no sense when PBTS has to be enabled
// from the beginning
// // TestPBTSEnableHeight tests the transition between BFT Time and PBTS.
// // The test runs multiple heights. BFT Time is used until the configured
// // PbtsEnableHeight. During some of these heights, the timestamp of votes
// // is shifted to the future to increase block timestamps. PBTS is enabled
// // at pbtsSetHeight, via FinalizeBlock. From this point, some nodes select
// // timestamps using PBTS, which is not yet enabled. When PbtsEnableHeight
// // is reached, some nodes propose bad timestamps. At the end, only blocks
// // proposed by the tested node are accepted, as they are not tweaked.
// func TestPBTSEnableHeight(t *testing.T) {
// numValidators := 4
// election := func(h int64, r int32) int {
// return (int(h-1) + int(r)) % numValidators
// }

// c := test.ConsensusParams()
// c.Feature.PbtsEnableHeight = 0 // Start with PBTS disabled

// app := abcimocks.NewApplication(t)
// app.On("ProcessProposal", mock.Anything, mock.Anything).Return(&abci.ProcessProposalResponse{
// Status: abci.PROCESS_PROPOSAL_STATUS_ACCEPT,
// }, nil)
// app.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.PrepareProposalResponse{}, nil)
// app.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ExtendVoteResponse{}, nil)
// app.On("VerifyVoteExtension", mock.Anything, mock.Anything).Return(&abci.VerifyVoteExtensionResponse{
// Status: abci.VERIFY_VOTE_EXTENSION_STATUS_ACCEPT,
// }, nil)
// app.On("Commit", mock.Anything, mock.Anything).Return(&abci.CommitResponse{}, nil).Maybe()

// cs, vss := randStateWithAppImpl(numValidators, app, c)
// height, round, chainID := cs.Height, cs.Round, cs.state.ChainID

// proposalCh := subscribe(cs.eventBus, types.EventQueryCompleteProposal)
// newRoundCh := subscribe(cs.eventBus, types.EventQueryNewRound)
// voteCh := subscribe(cs.eventBus, types.EventQueryVote)

// lastHeight := height + 4
// pbtsSetHeight := height + 2
// pbtsEnableHeight := height + 3

// startTestRound(cs, height, round)
// for height <= lastHeight {
// var block *types.Block
// var blockID types.BlockID

// ensureNewRound(newRoundCh, height, round)
// proposer := election(height, round)
// pbtsEnabled := (height >= pbtsEnableHeight)
// rejectProposal := false

// // Propose step
// if proposer == 0 {
// // Wait until we receive our own proposal
// // This may take longer when switching to PBTS since
// // BFT Time timestamps are shifted to the future.
// ensureProposalWithTimeout(proposalCh, height, round, nil, 2*time.Second)
// rs := cs.GetRoundState()
// block, _ = rs.ProposalBlock, rs.ProposalBlockParts
// blockID = rs.Proposal.BlockID
// } else {
// var ts time.Time
// var blockParts *types.PartSet

// if height >= pbtsSetHeight && height < pbtsEnableHeight {
// // Use PBTS logic while PBTS is not yet activated
// ts = cmttime.Now()
// rejectProposal = true
// } else if height >= pbtsEnableHeight {
// // Shift timestamp to the future 2*PRECISION => not timely
// ts = cmttime.Now().Add(2 * c.Synchrony.Precision)
// rejectProposal = true
// }
// block, blockParts, blockID = createProposalBlockWithTime(t, cs, ts)
// proposal := types.NewProposal(height, round, -1, blockID, block.Header.Time)
// // BFT Time should not care about Proposal's timestamps
// if height < pbtsSetHeight {
// proposal.Timestamp = cmttime.Now()
// }
// signProposal(t, proposal, chainID, vss[proposer])
// err := cs.SetProposalAndBlock(proposal, blockParts, "p")
// require.NoError(t, err)
// ensureProposal(proposalCh, height, round, blockID)
// }

// delta := cmttime.Since(block.Time)
// t.Log("BLOCK", height, round, "PROPOSER", proposer, "PBTS", pbtsEnabled,
// "TIMESTAMP", block.Time, delta, "ACCEPTED", !rejectProposal)

// // Accept proposal and decide, or reject proposal and move to next round
// myVote := blockID.Hash
// lockedRound := round
// if rejectProposal {
// myVote = nil
// lockedRound = int32(-1)
// } else { // We are deciding, enable FinalizeBlock mock
// res := &abci.FinalizeBlockResponse{}
// // Enable PBTS from pbtsEnableHeight via consensus params
// if height == pbtsSetHeight {
// params := types.DefaultConsensusParams()
// params.Feature.VoteExtensionsEnableHeight = 1
// params.Feature.PbtsEnableHeight = pbtsEnableHeight
// paramsProto := params.ToProto()
// res.ConsensusParamUpdates = &paramsProto
// }
// app.On("FinalizeBlock", mock.Anything, mock.Anything).
// Return(res, nil).Once()
// }

// // Prevote step
// ensurePrevote(voteCh, height, round)
// validatePrevote(t, cs, round, vss[0], myVote)
// for _, vs := range vss[2:] {
// signAddVotes(cs, types.PrevoteType, chainID, blockID, false, vs)
// ensurePrevote(voteCh, height, round)
// }

// // Precommit step
// ensurePrecommit(voteCh, height, round)
// validatePrecommit(t, cs, round, lockedRound, vss[0], myVote, myVote)
// for _, vs := range vss[2:] {

// vote := signVoteWith(vs, types.PrecommitType, chainID, blockID, true)
// cs.peerMsgQueue <- msgInfo{Msg: &VoteMessage{vote}}
// ensurePrecommit(voteCh, height, round)
// }

// if myVote != nil {
// height, round = height+1, 0
// incrementHeight(vss[1:]...)
// } else {
// round = round + 1
// incrementRound(vss[1:]...)
// }
// }
// // Last call to FinalizeBlock
// ensureNewRound(newRoundCh, height, round)
// }

// TestPbtsAdaptiveMessageDelay tests whether proposals with timestamps in the
// past are eventually accepted by validators. The test runs multiple rounds.
// Rounds where the tested node is the proposer are skipped. Rounds with other
Expand Down
29 changes: 0 additions & 29 deletions privval/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,34 +346,6 @@ func TestDifferByTimestamp(t *testing.T) {
assert.Equal(t, signBytes, types.ProposalSignBytes(chainID, pb))
assert.Equal(t, sig, proposal.Signature)
}

// test vote
// {
// voteType := types.PrevoteType
// blockID := types.BlockID{Hash: randbytes, PartSetHeader: types.PartSetHeader{}}
// vote := newVote(privVal.Key.Address, height, round, voteType, blockID)
// v := vote.ToProto()
// err := privVal.SignVote("mychainid", v, false)
// require.NoError(t, err, "expected no error signing vote")

// signBytes := types.VoteSignBytes(chainID, v)
// sig := v.Signature
// extSig := v.ExtensionSignature
// timeStamp := vote.Timestamp

// // manipulate the timestamp. should get changed back
// v.Timestamp = v.Timestamp.Add(time.Millisecond)
// var emptySig []byte
// v.Signature = emptySig
// v.ExtensionSignature = emptySig
// err = privVal.SignVote("mychainid", v, false)
// require.NoError(t, err, "expected no error on signing same vote")

// assert.Equal(t, timeStamp, v.Timestamp)
// assert.Equal(t, signBytes, types.VoteSignBytes(chainID, v))
// assert.Equal(t, sig, v.Signature)
// assert.Equal(t, extSig, v.ExtensionSignature)
// }
}

func TestVoteExtensionsAreSignedIfSignExtensionIsTrue(t *testing.T) {
Expand Down Expand Up @@ -422,7 +394,6 @@ func TestVoteExtensionsAreSignedIfSignExtensionIsTrue(t *testing.T) {

err = privVal.SignVote("mychainid", vpb2, true)
require.NoError(t, err, "expected no error signing same vote with manipulated timestamp and vote extension")
// assert.Equal(t, expectedTimestamp, vpb2.Timestamp)

vesb3 := types.VoteExtensionSignBytes("mychainid", vpb2)
assert.True(t, pubKey.VerifySignature(vesb3, vpb2.ExtensionSignature))
Expand Down
6 changes: 3 additions & 3 deletions privval/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ func TestPrivvalVectors(t *testing.T) {
{"pubKey request", &privproto.PubKeyRequest{}, "0a00"},
{"pubKey response", &privproto.PubKeyResponse{PubKeyType: pk.Type(), PubKeyBytes: pk.Bytes(), Error: nil}, "122b1a20556a436f1218d30942efe798420f51dc9b6a311b929c578257457d05c5fcf230220765643235353139"},
{"pubKey response with error", &privproto.PubKeyResponse{PubKeyType: "", PubKeyBytes: []byte{}, Error: remoteError}, "121212100801120c697427732061206572726f72"},
{"Vote Request", &privproto.SignVoteRequest{Vote: votepb}, "1a81010a7f080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0608f49a8ded0532146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb034a09657874656e73696f6e"},
{"Vote Response", &privproto.SignedVoteResponse{Vote: *votepb, Error: nil}, "2281010a7f080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a2a0608f49a8ded0532146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb034a09657874656e73696f6e"},
{"Vote Response with error", &privproto.SignedVoteResponse{Vote: cmtproto.Vote{}, Error: remoteError}, "22250a11220212002a0b088092b8c398feffffff0112100801120c697427732061206572726f72"},
{"Vote Request", &privproto.SignVoteRequest{Vote: votepb}, "1a790a77080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a32146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb034a09657874656e73696f6e"},
{"Vote Response", &privproto.SignedVoteResponse{Vote: *votepb, Error: nil}, "22790a77080210031802224a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a32146af1f4111082efb388211bc72c55bcd61e9ac3d538d5bb034a09657874656e73696f6e"},
{"Vote Response with error", &privproto.SignedVoteResponse{Vote: cmtproto.Vote{}, Error: remoteError}, "22180a042202120012100801120c697427732061206572726f72"},
{"Proposal Request", &privproto.SignProposalRequest{Proposal: proposalpb}, "2a700a6e08011003180220022a4a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a320608f49a8ded053a10697427732061207369676e6174757265"},
{"Proposal Response", &privproto.SignedProposalResponse{Proposal: *proposalpb, Error: nil}, "32700a6e08011003180220022a4a0a208b01023386c371778ecb6368573e539afc3cc860ec3a2f614e54fe5652f4fc80122608c0843d122072db3d959635dff1bb567bedaa70573392c5159666a3f8caf11e413aac52207a320608f49a8ded053a10697427732061207369676e6174757265"},
{"Proposal Response with error", &privproto.SignedProposalResponse{Proposal: cmtproto.Proposal{}, Error: remoteError}, "32250a112a021200320b088092b8c398feffffff0112100801120c697427732061206572726f72"},
Expand Down
3 changes: 0 additions & 3 deletions state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,8 @@ func (state State) MakeBlock(
switch {
case state.ConsensusParams.Feature.PbtsEnabled(height):
timestamp = cmttime.Now()
// case height == state.InitialHeight:
// timestamp = state.LastBlockTime // genesis time
default:
panic("PBTS has to be enabled")
// timestamp = lastCommit.MedianTime(state.LastValidators)
}

// Fill rest of header with state data.
Expand Down
7 changes: 0 additions & 7 deletions state/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,6 @@ func validateBlock(state State, block *types.Block) error {
}
if !state.ConsensusParams.Feature.PbtsEnabled(block.Height) {
panic("PBTS has to be enabled")
// medianTime := block.LastCommit.MedianTime(state.LastValidators)
// if !block.Time.Equal(medianTime) {
// return fmt.Errorf("invalid block time. Expected %v, got %v",
// medianTime.Format(time.RFC3339Nano),
// block.Time.Format(time.RFC3339Nano),
// )
// }
}

case block.Height == state.InitialHeight:
Expand Down
21 changes: 0 additions & 21 deletions test/e2e/tests/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,24 +142,3 @@ func TestBlock_Range(t *testing.T) {
}
})
}

// Tests that time is monotonically increasing,
// and that blocks produced according to BFT Time follow MedianTime calculation.
func TestBlock_Time(t *testing.T) {
t.Helper()
blocks := fetchBlockChain(t)
testnet := loadTestnet(t)

lastBlock := blocks[0]
valSchedule := newValidatorSchedule(testnet)
for _, block := range blocks[1:] {
require.Less(t, lastBlock.Time, block.Time)
lastBlock = block

valSchedule.Increment(1)
if testnet.PbtsEnableHeight == 0 || block.Height < testnet.PbtsEnableHeight {
expTime := block.LastCommit.MedianTime(valSchedule.Set)
require.Equal(t, expTime, block.Time, "height=%d", block.Height)
}
}
}
3 changes: 0 additions & 3 deletions types/canonical.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ func CanonicalizeVote(chainID string, vote *cmtproto.Vote) cmtproto.CanonicalVot
Height: vote.Height, // encoded as sfixed64
Round: int64(vote.Round), // encoded as sfixed64
BlockID: CanonicalizeBlockID(vote.BlockID),
// Timestamp is not included in the canonical vote
// because we won't be able to aggregate votes with different timestamps.
// Timestamp: vote.Timestamp,
ChainID: chainID,
}
}
Expand Down

0 comments on commit 8f82994

Please sign in to comment.