Skip to content

Commit

Permalink
fix: proposal basic checks deferred to consensus (#1288)
Browse files Browse the repository at this point in the history
  • Loading branch information
b00f authored May 29, 2024
1 parent 6bbf55a commit 3f9286b
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 18 deletions.
6 changes: 6 additions & 0 deletions consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,12 @@ func (cs *consensus) SetProposal(p *proposal.Proposal) {
return
}

if err := p.BasicCheck(); err != nil {
cs.logger.Warn("invalid proposal", "proposal", p, "error", err)

return
}

roundProposal := cs.log.RoundProposal(p.Round())
if roundProposal != nil {
cs.logger.Trace("this round has proposal", "proposal", p)
Expand Down
12 changes: 11 additions & 1 deletion consensus/consensus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,17 @@ func TestProposalWithBigRound(t *testing.T) {

p := td.makeProposal(t, 1, util.MaxInt16)
td.consP.SetProposal(p)
assert.Equal(t, td.consP.log.RoundProposal(util.MaxInt16), p)
assert.Nil(t, td.consP.Proposal())
}

func TestInvalidProposal(t *testing.T) {
td := setup(t)

td.enterNewHeight(td.consP)

p := td.makeProposal(t, 1, 0)
p.SetSignature(nil) // Make proposal invalid
td.consP.SetProposal(p)
assert.Nil(t, td.consP.Proposal())
}

Expand Down
1 change: 0 additions & 1 deletion network/ratelimit.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ func (r *rateLimit) reset() {
}

func (r *rateLimit) increment() bool {

// Check if the window has expired and reset if necessary
if r.diff() > r.window {
r.reset()
Expand Down
4 changes: 3 additions & 1 deletion sync/bundle/message/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ func NewProposalMessage(p *proposal.Proposal) *ProposalMessage {
}

func (m *ProposalMessage) BasicCheck() error {
return m.Proposal.BasicCheck()
// Basic checks for the proposal are deferred to the consensus phase
// to avoid unnecessary validation for validators outside the committee.
return nil
}

func (m *ProposalMessage) Type() Type {
Expand Down
9 changes: 0 additions & 9 deletions sync/bundle/message/proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package message
import (
"testing"

"github.com/pactus-project/pactus/types/proposal"
"github.com/pactus-project/pactus/util/errors"
"github.com/pactus-project/pactus/util/testsuite"
"github.com/stretchr/testify/assert"
)
Expand All @@ -17,13 +15,6 @@ func TestProposalType(t *testing.T) {
func TestProposalMessage(t *testing.T) {
ts := testsuite.NewTestSuite(t)

t.Run("Invalid proposal", func(t *testing.T) {
prop := proposal.NewProposal(0, 0, nil)
m := NewProposalMessage(prop)

assert.Equal(t, errors.Code(m.BasicCheck()), errors.ErrInvalidBlock)
})

t.Run("OK", func(t *testing.T) {
prop, _ := ts.GenerateTestProposal(100, 0)
m := NewProposalMessage(prop)
Expand Down
2 changes: 0 additions & 2 deletions sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,6 @@ func (sync *synchronizer) prepareBlocks(from, count uint32) [][]byte {
return blocks
}

// weAreInTheCommittee checks if one of the validators is a member of the committee
// at the current height.
func (sync *synchronizer) shouldPropagateGeneralMessage(_ *network.GossipMessage) bool {
return true
}
Expand Down
4 changes: 0 additions & 4 deletions util/testsuite/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,10 +474,6 @@ func (ts *TestSuite) HelperSignVote(valKey *bls.ValidatorKey, v *vote.Vote) {
func (ts *TestSuite) HelperSignProposal(valKey *bls.ValidatorKey, p *proposal.Proposal) {
sig := valKey.Sign(p.SignBytes())
p.SetSignature(sig)

if err := p.BasicCheck(); err != nil {
panic(err)
}
}

func (ts *TestSuite) HelperSignTransaction(prv crypto.PrivateKey, trx *tx.Tx) {
Expand Down

0 comments on commit 3f9286b

Please sign in to comment.