Skip to content

Commit

Permalink
dbft: remove useless setters of dBFT interfaces
Browse files Browse the repository at this point in the history
Ref. #84 (comment).

Signed-off-by: Anna Shaleva <[email protected]>
  • Loading branch information
AnnaShaleva committed Mar 5, 2024
1 parent 6d71362 commit 95f22e5
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 93 deletions.
8 changes: 0 additions & 8 deletions consensus_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,10 @@ package dbft
type ConsensusMessage[H Hash, A Address] interface {
// ViewNumber returns view number when this message was originated.
ViewNumber() byte
// SetViewNumber sets view number.
SetViewNumber(view byte)

// Type returns type of this message.
Type() MessageType
// SetType sets the type of this message.
SetType(t MessageType)

// Payload returns this message's actual payload.
Payload() any
// SetPayload sets this message's payload to p.
SetPayload(p any)

// GetChangeView returns payload as if it was ChangeView.
GetChangeView() ChangeView
Expand Down
1 change: 0 additions & 1 deletion consensus_payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ type ConsensusPayload[H Hash, A Address] interface {
SetValidatorIndex(i uint16)

Height() uint32
SetHeight(h uint32)

// Hash returns 32-byte checksum of the payload.
Hash() H
Expand Down
52 changes: 14 additions & 38 deletions dbft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,7 @@ func TestDBFT_OnReceiveRequestSendResponse(t *testing.T) {
})

t.Run("old height", func(t *testing.T) {
p := s.getPrepareRequest(5, txs[0].Hash())
p.SetHeight(3)
p := s.getPrepareRequestWithHeight(5, 3, txs[0].Hash())
service.OnReceive(p)
require.Nil(t, s.tryRecv())
})
Expand Down Expand Up @@ -735,60 +734,41 @@ func (s testState) getChangeView(from uint16, view byte) Payload {
cv := payload.NewChangeView()
cv.SetNewViewNumber(view)

p := s.getPayload(from)
p.SetType(dbft.ChangeViewType)
p.SetPayload(cv)

p := payload.NewConsensusPayload(dbft.ChangeViewType, s.currHeight+1, from, view, cv)
return p
}

func (s testState) getRecoveryRequest(from uint16) Payload {
p := s.getPayload(from)
p.SetType(dbft.RecoveryRequestType)
p.SetPayload(payload.NewRecoveryRequest())

p := payload.NewConsensusPayload(dbft.RecoveryRequestType, s.currHeight+1, from, 0, payload.NewRecoveryRequest())
return p
}

func (s testState) getCommit(from uint16, sign []byte) Payload {
c := payload.NewCommit()
c.SetSignature(sign)

p := s.getPayload(from)
p.SetType(dbft.CommitType)
p.SetPayload(c)

p := payload.NewConsensusPayload(dbft.CommitType, s.currHeight+1, from, 0, c)
return p
}

func (s testState) getPrepareResponse(from uint16, phash crypto.Uint256) Payload {
resp := payload.NewPrepareResponse()
resp.SetPreparationHash(phash)

p := s.getPayload(from)
p.SetType(dbft.PrepareResponseType)
p.SetPayload(resp)

p := payload.NewConsensusPayload(dbft.PrepareResponseType, s.currHeight+1, from, 0, resp)
return p
}

func (s testState) getPrepareRequest(from uint16, hashes ...crypto.Uint256) Payload {
return s.getPrepareRequestWithHeight(from, s.currHeight+1, hashes...)
}

func (s testState) getPrepareRequestWithHeight(from uint16, height uint32, hashes ...crypto.Uint256) Payload {
req := payload.NewPrepareRequest()
req.SetTransactionHashes(hashes)
req.SetNextConsensus(s.nextConsensus())

p := s.getPayload(from)
p.SetType(dbft.PrepareRequestType)
p.SetPayload(req)

return p
}

func (s testState) getPayload(from uint16) Payload {
p := payload.NewConsensusPayload()
p.SetHeight(s.currHeight + 1)
p.SetValidatorIndex(from)

p := payload.NewConsensusPayload(dbft.PrepareRequestType, height, from, 0, req)
return p
}

Expand Down Expand Up @@ -867,7 +847,9 @@ func (s *testState) getOptions() []func(*dbft.Config[crypto.Uint256, crypto.Uint
dbft.WithNewChangeView[crypto.Uint256, crypto.Uint160](payload.NewChangeView),
dbft.WithNewCommit[crypto.Uint256, crypto.Uint160](payload.NewCommit),
dbft.WithNewRecoveryRequest[crypto.Uint256, crypto.Uint160](payload.NewRecoveryRequest),
dbft.WithNewRecoveryMessage[crypto.Uint256, crypto.Uint160](payload.NewRecoveryMessage),
dbft.WithNewRecoveryMessage[crypto.Uint256, crypto.Uint160](func() dbft.RecoveryMessage[crypto.Uint256, crypto.Uint160] {
return payload.NewRecoveryMessage(nil)
}),
}

verify := s.verify
Expand Down Expand Up @@ -898,13 +880,7 @@ func newBlockFromContext(ctx *dbft.Context[crypto.Uint256, crypto.Uint160]) dbft
// newConsensusPayload is a function for creating consensus payload of specific
// type.
func newConsensusPayload(c *dbft.Context[crypto.Uint256, crypto.Uint160], t dbft.MessageType, msg any) dbft.ConsensusPayload[crypto.Uint256, crypto.Uint160] {
cp := payload.NewConsensusPayload()
cp.SetHeight(c.BlockIndex)
cp.SetValidatorIndex(uint16(c.MyIndex))
cp.SetViewNumber(c.ViewNumber)
cp.SetType(t)
cp.SetPayload(msg)

cp := payload.NewConsensusPayload(t, c.BlockIndex, uint16(c.MyIndex), c.ViewNumber, msg)
return cp
}

Expand Down
16 changes: 13 additions & 3 deletions internal/payload/constructors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,17 @@ import (
)

// NewConsensusPayload returns minimal ConsensusPayload implementation.
func NewConsensusPayload() dbft.ConsensusPayload[crypto.Uint256, crypto.Uint160] {
return &Payload{}
func NewConsensusPayload(t dbft.MessageType, height uint32, validatorIndex uint16, viewNumber byte, consensusMessage any) dbft.ConsensusPayload[crypto.Uint256, crypto.Uint160] {
return &Payload{
message: message{
cmType: t,
viewNumber: viewNumber,
payload: consensusMessage,
},
validatorIndex: validatorIndex,
height: height,
}

}

// NewPrepareRequest returns minimal prepareRequest implementation.
Expand Down Expand Up @@ -36,8 +45,9 @@ func NewRecoveryRequest() dbft.RecoveryRequest {
}

// NewRecoveryMessage returns minimal RecoveryMessage implementation.
func NewRecoveryMessage() dbft.RecoveryMessage[crypto.Uint256, crypto.Uint160] {
func NewRecoveryMessage(preparationHash *crypto.Uint256) dbft.RecoveryMessage[crypto.Uint256, crypto.Uint160] {
return &recoveryMessage{
preparationHash: preparationHash,
preparationPayloads: make([]preparationCompact, 0),
commitPayloads: make([]commitCompact, 0),
changeViewPayloads: make([]changeViewCompact, 0),
Expand Down
38 changes: 11 additions & 27 deletions internal/payload/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,12 @@ import (
)

func TestPayload_EncodeDecode(t *testing.T) {
m := NewConsensusPayload().(*Payload)
m.SetValidatorIndex(10)
m.SetHeight(77)
m.SetPrevHash(crypto.Uint256{1})
m.SetVersion(8)
m.SetViewNumber(3)
generateMessage := func(typ dbft.MessageType, payload any) *Payload {
return NewConsensusPayload(typ, 77, 10, 3, payload).(*Payload)
}

t.Run("PrepareRequest", func(t *testing.T) {
m.SetType(dbft.PrepareRequestType)
m.SetPayload(&prepareRequest{
m := generateMessage(dbft.PrepareRequestType, &prepareRequest{
nonce: 123,
timestamp: 345,
transactionHashes: []crypto.Uint256{
Expand All @@ -36,8 +32,7 @@ func TestPayload_EncodeDecode(t *testing.T) {
})

t.Run("PrepareResponse", func(t *testing.T) {
m.SetType(dbft.PrepareResponseType)
m.SetPayload(&prepareResponse{
m := generateMessage(dbft.PrepareResponseType, &prepareResponse{
preparationHash: crypto.Uint256{3},
})

Expand All @@ -46,18 +41,16 @@ func TestPayload_EncodeDecode(t *testing.T) {
})

t.Run("Commit", func(t *testing.T) {
m.SetType(dbft.CommitType)
var cc commit
fillRandom(t, cc.signature[:])
m.SetPayload(&cc)
m := generateMessage(dbft.CommitType, &cc)

testEncodeDecode(t, m, new(Payload))
testMarshalUnmarshal(t, m, new(Payload))
})

t.Run("ChangeView", func(t *testing.T) {
m.SetType(dbft.ChangeViewType)
m.SetPayload(&changeView{
m := generateMessage(dbft.ChangeViewType, &changeView{
timestamp: 12345,
newViewNumber: 4,
})
Expand All @@ -67,8 +60,7 @@ func TestPayload_EncodeDecode(t *testing.T) {
})

t.Run("RecoveryMessage", func(t *testing.T) {
m.SetType(dbft.RecoveryMessageType)
m.SetPayload(&recoveryMessage{
m := generateMessage(dbft.RecoveryMessageType, &recoveryMessage{
changeViewPayloads: []changeViewCompact{
{
Timestamp: 123,
Expand Down Expand Up @@ -97,8 +89,7 @@ func TestPayload_EncodeDecode(t *testing.T) {
})

t.Run("RecoveryRequest", func(t *testing.T) {
m.SetType(dbft.RecoveryRequestType)
m.SetPayload(&recoveryRequest{
m := generateMessage(dbft.RecoveryRequestType, &recoveryRequest{
timestamp: 17334,
})

Expand All @@ -108,13 +99,7 @@ func TestPayload_EncodeDecode(t *testing.T) {
}

func TestRecoveryMessage_NoPayloads(t *testing.T) {
m := NewConsensusPayload().(*Payload)
m.SetValidatorIndex(0)
m.SetHeight(77)
m.SetPrevHash(crypto.Uint256{1})
m.SetVersion(8)
m.SetViewNumber(3)
m.SetPayload(&recoveryMessage{})
m := NewConsensusPayload(dbft.RecoveryRequestType, 77, 0, 3, &recoveryMessage{}).(*Payload)

validators := make([]dbft.PublicKey, 1)
_, validators[0] = crypto.Generate(rand.Reader)
Expand Down Expand Up @@ -186,9 +171,8 @@ func TestPayload_Setters(t *testing.T) {
})

t.Run("RecoveryMessage", func(t *testing.T) {
r := NewRecoveryMessage()
r := NewRecoveryMessage(&crypto.Uint256{1, 2, 3})

r.SetPreparationHash(&crypto.Uint256{1, 2, 3})
require.Equal(t, &crypto.Uint256{1, 2, 3}, r.PreparationHash())
})
}
Expand Down
5 changes: 0 additions & 5 deletions internal/payload/recovery_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ func (m *recoveryMessage) PreparationHash() *crypto.Uint256 {
return m.preparationHash
}

// SetPreparationHash implements RecoveryMessage interface.
func (m *recoveryMessage) SetPreparationHash(h *crypto.Uint256) {
m.preparationHash = h
}

// AddPayload implements RecoveryMessage interface.
func (m *recoveryMessage) AddPayload(p dbft.ConsensusPayload[crypto.Uint256, crypto.Uint160]) {
switch p.Type() {
Expand Down
13 changes: 4 additions & 9 deletions internal/simulation/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,7 @@ func newBlockFromContext(ctx *dbft.Context[crypto.Uint256, crypto.Uint160]) dbft
// defaultNewConsensusPayload is default function for creating
// consensus payload of specific type.
func defaultNewConsensusPayload(c *dbft.Context[crypto.Uint256, crypto.Uint160], t dbft.MessageType, msg any) dbft.ConsensusPayload[crypto.Uint256, crypto.Uint160] {
cp := payload.NewConsensusPayload()
cp.SetHeight(c.BlockIndex)
cp.SetValidatorIndex(uint16(c.MyIndex))
cp.SetViewNumber(c.ViewNumber)
cp.SetType(t)
cp.SetPayload(msg)

return cp
return payload.NewConsensusPayload(t, c.BlockIndex, uint16(c.MyIndex), c.ViewNumber, msg)

Check warning on line 124 in internal/simulation/main.go

View check run for this annotation

Codecov / codecov/patch

internal/simulation/main.go#L124

Added line #L124 was not covered by tests
}

func initSimNode(nodes []*simNode, i int, log *zap.Logger) error {
Expand Down Expand Up @@ -164,7 +157,9 @@ func initSimNode(nodes []*simNode, i int, log *zap.Logger) error {
dbft.WithNewPrepareResponse[crypto.Uint256, crypto.Uint160](payload.NewPrepareResponse),
dbft.WithNewChangeView[crypto.Uint256, crypto.Uint160](payload.NewChangeView),
dbft.WithNewCommit[crypto.Uint256, crypto.Uint160](payload.NewCommit),
dbft.WithNewRecoveryMessage[crypto.Uint256, crypto.Uint160](payload.NewRecoveryMessage),
dbft.WithNewRecoveryMessage[crypto.Uint256, crypto.Uint160](func() dbft.RecoveryMessage[crypto.Uint256, crypto.Uint160] {
return payload.NewRecoveryMessage(nil)
}),

Check warning on line 162 in internal/simulation/main.go

View check run for this annotation

Codecov / codecov/patch

internal/simulation/main.go#L160-L162

Added lines #L160 - L162 were not covered by tests
dbft.WithNewRecoveryRequest[crypto.Uint256, crypto.Uint160](payload.NewRecoveryRequest),
)

Expand Down
2 changes: 0 additions & 2 deletions recovery_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,4 @@ type RecoveryMessage[H Hash, A Address] interface {
// PreparationHash returns has of PrepareRequest payload for this epoch.
// It can be useful in case only PrepareResponse payloads were received.
PreparationHash() *H
// SetPreparationHash sets preparation hash.
SetPreparationHash(h *H)
}

0 comments on commit 95f22e5

Please sign in to comment.