From 66ed0c96302d9c015b60b1598e82251ebb5556bb Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 5 Mar 2024 17:55:47 +0300 Subject: [PATCH 1/6] dbft: remove useless setters of dBFT interfaces Ref. https://github.com/nspcc-dev/dbft/issues/84#issuecomment-1976441193. Signed-off-by: Anna Shaleva --- consensus_message.go | 8 ----- consensus_payload.go | 1 - dbft_test.go | 52 ++++++++------------------- internal/payload/consensus_message.go | 15 -------- internal/payload/constructors.go | 15 ++++++-- internal/payload/message.go | 15 -------- internal/payload/message_test.go | 38 ++++++-------------- internal/payload/recovery_message.go | 5 --- internal/simulation/main.go | 13 +++---- recovery_message.go | 2 -- 10 files changed, 41 insertions(+), 123 deletions(-) diff --git a/consensus_message.go b/consensus_message.go index 5552e469..7e72029f 100644 --- a/consensus_message.go +++ b/consensus_message.go @@ -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 diff --git a/consensus_payload.go b/consensus_payload.go index af6bf533..e537d3e0 100644 --- a/consensus_payload.go +++ b/consensus_payload.go @@ -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 diff --git a/dbft_test.go b/dbft_test.go index 4aa9511d..951e5265 100644 --- a/dbft_test.go +++ b/dbft_test.go @@ -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()) }) @@ -735,18 +734,12 @@ 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 } @@ -754,10 +747,7 @@ 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 } @@ -765,30 +755,20 @@ 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 } @@ -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 @@ -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 } diff --git a/internal/payload/consensus_message.go b/internal/payload/consensus_message.go index 88dded51..dd0e2436 100644 --- a/internal/payload/consensus_message.go +++ b/internal/payload/consensus_message.go @@ -98,27 +98,12 @@ func (m message) ViewNumber() byte { return m.viewNumber } -// SetViewNumber implements ConsensusMessage interface. -func (m *message) SetViewNumber(view byte) { - m.viewNumber = view -} - // Type implements ConsensusMessage interface. func (m message) Type() dbft.MessageType { return m.cmType } -// SetType implements ConsensusMessage interface. -func (m *message) SetType(t dbft.MessageType) { - m.cmType = t -} - // Payload implements ConsensusMessage interface. func (m message) Payload() any { return m.payload } - -// SetPayload implements ConsensusMessage interface. -func (m *message) SetPayload(p any) { - m.payload = p -} diff --git a/internal/payload/constructors.go b/internal/payload/constructors.go index ac1ecd0a..d4d29093 100644 --- a/internal/payload/constructors.go +++ b/internal/payload/constructors.go @@ -6,8 +6,16 @@ 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. @@ -36,8 +44,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), diff --git a/internal/payload/message.go b/internal/payload/message.go index e7de8274..a39c49a4 100644 --- a/internal/payload/message.go +++ b/internal/payload/message.go @@ -100,11 +100,6 @@ func (p Payload) Version() uint32 { return p.version } -// SetVersion implements ConsensusPayload interface. -func (p *Payload) SetVersion(v uint32) { - p.version = v -} - // ValidatorIndex implements ConsensusPayload interface. func (p Payload) ValidatorIndex() uint16 { return p.validatorIndex @@ -120,17 +115,7 @@ func (p Payload) PrevHash() crypto.Uint256 { return p.prevHash } -// SetPrevHash implements ConsensusPayload interface. -func (p *Payload) SetPrevHash(h crypto.Uint256) { - p.prevHash = h -} - // Height implements ConsensusPayload interface. func (p Payload) Height() uint32 { return p.height } - -// SetHeight implements ConsensusPayload interface. -func (p *Payload) SetHeight(h uint32) { - p.height = h -} diff --git a/internal/payload/message_test.go b/internal/payload/message_test.go index 8d76185e..d9de377e 100644 --- a/internal/payload/message_test.go +++ b/internal/payload/message_test.go @@ -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{ @@ -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}, }) @@ -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, }) @@ -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, @@ -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, }) @@ -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) @@ -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()) }) } diff --git a/internal/payload/recovery_message.go b/internal/payload/recovery_message.go index 870f402c..f98647f3 100644 --- a/internal/payload/recovery_message.go +++ b/internal/payload/recovery_message.go @@ -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() { diff --git a/internal/simulation/main.go b/internal/simulation/main.go index e4587935..8cc655db 100644 --- a/internal/simulation/main.go +++ b/internal/simulation/main.go @@ -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) } func initSimNode(nodes []*simNode, i int, log *zap.Logger) error { @@ -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) + }), dbft.WithNewRecoveryRequest[crypto.Uint256, crypto.Uint160](payload.NewRecoveryRequest), ) diff --git a/recovery_message.go b/recovery_message.go index be14aa2c..8e6f760d 100644 --- a/recovery_message.go +++ b/recovery_message.go @@ -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) } From 164d0d1cbeb31c04a1677d644fc4b7d1678f35cf Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 5 Mar 2024 19:41:32 +0300 Subject: [PATCH 2/6] dbft: replace setters with extended constructor for dbft.Commit A part of #84. Signed-off-by: Anna Shaleva --- commit.go | 3 --- config.go | 4 ++-- dbft_test.go | 6 ++---- internal/payload/commit.go | 5 ----- internal/payload/constructors.go | 6 ++++-- send.go | 3 +-- 6 files changed, 9 insertions(+), 18 deletions(-) diff --git a/commit.go b/commit.go index 7b5a38c6..40a44ee7 100644 --- a/commit.go +++ b/commit.go @@ -5,7 +5,4 @@ type Commit interface { // Signature returns commit's signature field // which is a block signature for the current epoch. Signature() []byte - - // SetSignature sets commit's signature. - SetSignature(signature []byte) } diff --git a/config.go b/config.go index 8bad7283..d2e4a01d 100644 --- a/config.go +++ b/config.go @@ -68,7 +68,7 @@ type Config[H Hash, A Address] struct { // NewChangeView is a constructor for payload.ChangeView. NewChangeView func() ChangeView // NewCommit is a constructor for payload.Commit. - NewCommit func() Commit + NewCommit func(signature []byte) Commit // NewRecoveryRequest is a constructor for payload.RecoveryRequest. NewRecoveryRequest func() RecoveryRequest // NewRecoveryMessage is a constructor for payload.RecoveryMessage. @@ -327,7 +327,7 @@ func WithNewChangeView[H Hash, A Address](f func() ChangeView) func(config *Conf } // WithNewCommit sets NewCommit. -func WithNewCommit[H Hash, A Address](f func() Commit) func(config *Config[H, A]) { +func WithNewCommit[H Hash, A Address](f func([]byte) Commit) func(config *Config[H, A]) { return func(cfg *Config[H, A]) { cfg.NewCommit = f } diff --git a/dbft_test.go b/dbft_test.go index 951e5265..9ef8acbb 100644 --- a/dbft_test.go +++ b/dbft_test.go @@ -482,7 +482,7 @@ func TestDBFT_Invalid(t *testing.T) { require.Nil(t, dbft.New(opts...)) }) - opts = append(opts, dbft.WithNewCommit[crypto.Uint256, crypto.Uint160](func() dbft.Commit { + opts = append(opts, dbft.WithNewCommit[crypto.Uint256, crypto.Uint160](func([]byte) dbft.Commit { return nil })) t.Run("without NewRecoveryRequest", func(t *testing.T) { @@ -744,9 +744,7 @@ func (s testState) getRecoveryRequest(from uint16) Payload { } func (s testState) getCommit(from uint16, sign []byte) Payload { - c := payload.NewCommit() - c.SetSignature(sign) - + c := payload.NewCommit(sign) p := payload.NewConsensusPayload(dbft.CommitType, s.currHeight+1, from, 0, c) return p } diff --git a/internal/payload/commit.go b/internal/payload/commit.go index 70e962bb..9c8fe0b6 100644 --- a/internal/payload/commit.go +++ b/internal/payload/commit.go @@ -41,8 +41,3 @@ func (c *commit) DecodeBinary(r *gob.Decoder) error { func (c commit) Signature() []byte { return c.signature[:] } - -// SetSignature implements Commit interface. -func (c *commit) SetSignature(sig []byte) { - copy(c.signature[:], sig) -} diff --git a/internal/payload/constructors.go b/internal/payload/constructors.go index d4d29093..5517ee77 100644 --- a/internal/payload/constructors.go +++ b/internal/payload/constructors.go @@ -34,8 +34,10 @@ func NewChangeView() dbft.ChangeView { } // NewCommit returns minimal Commit implementation. -func NewCommit() dbft.Commit { - return new(commit) +func NewCommit(signature []byte) dbft.Commit { + c := new(commit) + copy(c.signature[:], signature) + return c } // NewRecoveryRequest returns minimal RecoveryRequest implementation. diff --git a/send.go b/send.go index a48fc4bb..384968eb 100644 --- a/send.go +++ b/send.go @@ -118,8 +118,7 @@ func (c *Context[H, A]) makeCommit() ConsensusPayload[H, A] { sign = b.Signature() } - commit := c.Config.NewCommit() - commit.SetSignature(sign) + commit := c.Config.NewCommit(sign) return c.Config.NewConsensusPayload(c, CommitType, commit) } From 5086d091095408c0077d499cca6671a5980e9538 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 5 Mar 2024 20:03:37 +0300 Subject: [PATCH 3/6] dbft: replace setters with extended constructor for dbft.ChangeView A part of #84. Signed-off-by: Anna Shaleva --- change_view.go | 9 --------- config.go | 4 ++-- internal/payload/change_view.go | 14 -------------- internal/payload/constructors.go | 7 +++++-- internal/payload/message_test.go | 5 +---- send.go | 5 +---- 6 files changed, 9 insertions(+), 35 deletions(-) diff --git a/change_view.go b/change_view.go index 93067f7c..ec0ac7a9 100644 --- a/change_view.go +++ b/change_view.go @@ -5,18 +5,9 @@ type ChangeView interface { // NewViewNumber returns proposed view number. NewViewNumber() byte - // SetNewViewNumber sets the proposed view number. - SetNewViewNumber(view byte) - // Timestamp returns message's timestamp. Timestamp() uint64 - // SetTimestamp sets message's timestamp. - SetTimestamp(ts uint64) - // Reason returns change view reason. Reason() ChangeViewReason - - // SetReason sets change view reason. - SetReason(reason ChangeViewReason) } diff --git a/config.go b/config.go index d2e4a01d..6239868f 100644 --- a/config.go +++ b/config.go @@ -66,7 +66,7 @@ type Config[H Hash, A Address] struct { // NewPrepareResponse is a constructor for payload.PrepareResponse. NewPrepareResponse func() PrepareResponse[H] // NewChangeView is a constructor for payload.ChangeView. - NewChangeView func() ChangeView + NewChangeView func(newViewNumber byte, reason ChangeViewReason, timestamp uint64) ChangeView // NewCommit is a constructor for payload.Commit. NewCommit func(signature []byte) Commit // NewRecoveryRequest is a constructor for payload.RecoveryRequest. @@ -320,7 +320,7 @@ func WithNewPrepareResponse[H Hash, A Address](f func() PrepareResponse[H]) func } // WithNewChangeView sets NewChangeView. -func WithNewChangeView[H Hash, A Address](f func() ChangeView) func(config *Config[H, A]) { +func WithNewChangeView[H Hash, A Address](f func(byte, ChangeViewReason, uint64) ChangeView) func(config *Config[H, A]) { return func(cfg *Config[H, A]) { cfg.NewChangeView = f } diff --git a/internal/payload/change_view.go b/internal/payload/change_view.go index 1e5d0693..de3d03a9 100644 --- a/internal/payload/change_view.go +++ b/internal/payload/change_view.go @@ -41,26 +41,12 @@ func (c changeView) NewViewNumber() byte { return c.newViewNumber } -// SetNewViewNumber implements ChangeView interface. -func (c *changeView) SetNewViewNumber(view byte) { - c.newViewNumber = view -} - // Timestamp implements ChangeView interface. func (c changeView) Timestamp() uint64 { return secToNanoSec(c.timestamp) } -// SetTimestamp implements ChangeView interface. -func (c *changeView) SetTimestamp(ts uint64) { - c.timestamp = nanoSecToSec(ts) -} - // Reason implements ChangeView interface. func (c changeView) Reason() dbft.ChangeViewReason { return dbft.CVUnknown } - -// SetReason implements ChangeView interface. -func (c *changeView) SetReason(_ dbft.ChangeViewReason) { -} diff --git a/internal/payload/constructors.go b/internal/payload/constructors.go index 5517ee77..a6db5aee 100644 --- a/internal/payload/constructors.go +++ b/internal/payload/constructors.go @@ -29,8 +29,11 @@ func NewPrepareResponse() dbft.PrepareResponse[crypto.Uint256] { } // NewChangeView returns minimal ChangeView implementation. -func NewChangeView() dbft.ChangeView { - return new(changeView) +func NewChangeView(newViewNumber byte, _ dbft.ChangeViewReason, ts uint64) dbft.ChangeView { + return &changeView{ + newViewNumber: newViewNumber, + timestamp: nanoSecToSec(ts), + } } // NewCommit returns minimal Commit implementation. diff --git a/internal/payload/message_test.go b/internal/payload/message_test.go index d9de377e..f0a51bad 100644 --- a/internal/payload/message_test.go +++ b/internal/payload/message_test.go @@ -154,12 +154,9 @@ func TestCompact_EncodeDecode(t *testing.T) { func TestPayload_Setters(t *testing.T) { t.Run("ChangeView", func(t *testing.T) { - cv := NewChangeView() + cv := NewChangeView(4, 0, secToNanoSec(1234)) - cv.SetTimestamp(secToNanoSec(1234)) assert.EqualValues(t, secToNanoSec(1234), cv.Timestamp()) - - cv.SetNewViewNumber(4) assert.EqualValues(t, 4, cv.NewViewNumber()) }) diff --git a/send.go b/send.go index 384968eb..02431ced 100644 --- a/send.go +++ b/send.go @@ -42,10 +42,7 @@ func (d *DBFT[H, A]) sendPrepareRequest() { } func (c *Context[H, A]) makeChangeView(ts uint64, reason ChangeViewReason) ConsensusPayload[H, A] { - cv := c.Config.NewChangeView() - cv.SetNewViewNumber(c.ViewNumber + 1) - cv.SetTimestamp(ts) - cv.SetReason(reason) + cv := c.Config.NewChangeView(c.ViewNumber+1, reason, ts) msg := c.Config.NewConsensusPayload(c, ChangeViewType, cv) c.ChangeViewPayloads[c.MyIndex] = msg From 2b2e6cb97aedb5c3b80b8dd7fb154d25b3d0ea45 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 5 Mar 2024 20:47:41 +0300 Subject: [PATCH 4/6] dbft: replace setters with extended constructor for dbft.PrepareRequest A part of #84. Signed-off-by: Anna Shaleva --- config.go | 4 ++-- dbft_test.go | 13 +++++-------- internal/payload/constructors.go | 9 +++++++-- internal/payload/prepare_request.go | 20 -------------------- prepare_request.go | 11 ----------- send.go | 6 +----- 6 files changed, 15 insertions(+), 48 deletions(-) diff --git a/config.go b/config.go index 6239868f..5307e942 100644 --- a/config.go +++ b/config.go @@ -62,7 +62,7 @@ type Config[H Hash, A Address] struct { // NewConsensusPayload is a constructor for payload.ConsensusPayload. NewConsensusPayload func(*Context[H, A], MessageType, any) ConsensusPayload[H, A] // NewPrepareRequest is a constructor for payload.PrepareRequest. - NewPrepareRequest func() PrepareRequest[H, A] + NewPrepareRequest func(ts uint64, nonce uint64, nextConsensus A, transactionHashes []H) PrepareRequest[H, A] // NewPrepareResponse is a constructor for payload.PrepareResponse. NewPrepareResponse func() PrepareResponse[H] // NewChangeView is a constructor for payload.ChangeView. @@ -306,7 +306,7 @@ func WithNewConsensusPayload[H Hash, A Address](f func(*Context[H, A], MessageTy } // WithNewPrepareRequest sets NewPrepareRequest. -func WithNewPrepareRequest[H Hash, A Address](f func() PrepareRequest[H, A]) func(config *Config[H, A]) { +func WithNewPrepareRequest[H Hash, A Address](f func(ts uint64, nonce uint64, nextConsensus A, transactionsHashes []H) PrepareRequest[H, A]) func(config *Config[H, A]) { return func(cfg *Config[H, A]) { cfg.NewPrepareRequest = f } diff --git a/dbft_test.go b/dbft_test.go index 9ef8acbb..8f122f31 100644 --- a/dbft_test.go +++ b/dbft_test.go @@ -461,7 +461,7 @@ func TestDBFT_Invalid(t *testing.T) { require.Nil(t, dbft.New(opts...)) }) - opts = append(opts, dbft.WithNewPrepareRequest[crypto.Uint256, crypto.Uint160](func() dbft.PrepareRequest[crypto.Uint256, crypto.Uint160] { + opts = append(opts, dbft.WithNewPrepareRequest[crypto.Uint256, crypto.Uint160](func(uint64, uint64, crypto.Uint160, []crypto.Uint256) dbft.PrepareRequest[crypto.Uint256, crypto.Uint160] { return nil })) t.Run("without NewPrepareResponse", func(t *testing.T) { @@ -475,7 +475,7 @@ func TestDBFT_Invalid(t *testing.T) { require.Nil(t, dbft.New(opts...)) }) - opts = append(opts, dbft.WithNewChangeView[crypto.Uint256, crypto.Uint160](func() dbft.ChangeView { + opts = append(opts, dbft.WithNewChangeView[crypto.Uint256, crypto.Uint160](func(byte, dbft.ChangeViewReason, uint64) dbft.ChangeView { return nil })) t.Run("without NewCommit", func(t *testing.T) { @@ -731,10 +731,9 @@ func TestDBFT_FourGoodNodesDeadlock(t *testing.T) { } func (s testState) getChangeView(from uint16, view byte) Payload { - cv := payload.NewChangeView() - cv.SetNewViewNumber(view) + cv := payload.NewChangeView(view, 0, 0) - p := payload.NewConsensusPayload(dbft.ChangeViewType, s.currHeight+1, from, view, cv) + p := payload.NewConsensusPayload(dbft.ChangeViewType, s.currHeight+1, from, 0, cv) return p } @@ -762,9 +761,7 @@ func (s testState) getPrepareRequest(from uint16, hashes ...crypto.Uint256) Payl } func (s testState) getPrepareRequestWithHeight(from uint16, height uint32, hashes ...crypto.Uint256) Payload { - req := payload.NewPrepareRequest() - req.SetTransactionHashes(hashes) - req.SetNextConsensus(s.nextConsensus()) + req := payload.NewPrepareRequest(0, 0, s.nextConsensus(), hashes) p := payload.NewConsensusPayload(dbft.PrepareRequestType, height, from, 0, req) return p diff --git a/internal/payload/constructors.go b/internal/payload/constructors.go index a6db5aee..4b6e0603 100644 --- a/internal/payload/constructors.go +++ b/internal/payload/constructors.go @@ -19,8 +19,13 @@ func NewConsensusPayload(t dbft.MessageType, height uint32, validatorIndex uint1 } // NewPrepareRequest returns minimal prepareRequest implementation. -func NewPrepareRequest() dbft.PrepareRequest[crypto.Uint256, crypto.Uint160] { - return new(prepareRequest) +func NewPrepareRequest(ts uint64, nonce uint64, nextConsensus crypto.Uint160, transactionsHashes []crypto.Uint256) dbft.PrepareRequest[crypto.Uint256, crypto.Uint160] { + return &prepareRequest{ + transactionHashes: transactionsHashes, + nonce: nonce, + timestamp: nanoSecToSec(ts), + nextConsensus: nextConsensus, + } } // NewPrepareResponse returns minimal PrepareResponse implementation. diff --git a/internal/payload/prepare_request.go b/internal/payload/prepare_request.go index ccbab16b..1fffb097 100644 --- a/internal/payload/prepare_request.go +++ b/internal/payload/prepare_request.go @@ -54,37 +54,17 @@ func (p prepareRequest) Timestamp() uint64 { return secToNanoSec(p.timestamp) } -// SetTimestamp implements PrepareRequest interface. -func (p *prepareRequest) SetTimestamp(ts uint64) { - p.timestamp = nanoSecToSec(ts) -} - // Nonce implements PrepareRequest interface. func (p prepareRequest) Nonce() uint64 { return p.nonce } -// SetNonce implements PrepareRequest interface. -func (p *prepareRequest) SetNonce(nonce uint64) { - p.nonce = nonce -} - // TransactionHashes implements PrepareRequest interface. func (p prepareRequest) TransactionHashes() []crypto.Uint256 { return p.transactionHashes } -// SetTransactionHashes implements PrepareRequest interface. -func (p *prepareRequest) SetTransactionHashes(hs []crypto.Uint256) { - p.transactionHashes = hs -} - // NextConsensus implements PrepareRequest interface. func (p prepareRequest) NextConsensus() crypto.Uint160 { return p.nextConsensus } - -// SetNextConsensus implements PrepareRequest interface. -func (p *prepareRequest) SetNextConsensus(nc crypto.Uint160) { - p.nextConsensus = nc -} diff --git a/prepare_request.go b/prepare_request.go index 9f96be16..c27a2318 100644 --- a/prepare_request.go +++ b/prepare_request.go @@ -4,22 +4,11 @@ package dbft type PrepareRequest[H Hash, A Address] interface { // Timestamp returns this message's timestamp. Timestamp() uint64 - // SetTimestamp sets timestamp of this message. - SetTimestamp(ts uint64) - // Nonce is a random nonce. Nonce() uint64 - // SetNonce sets Nonce. - SetNonce(nonce uint64) - // TransactionHashes returns hashes of all transaction in a proposed block. TransactionHashes() []H - // SetTransactionHashes sets transaction's hashes. - SetTransactionHashes(hs []H) - // NextConsensus returns hash which is based on which validators will // try to agree on a block in the current epoch. NextConsensus() A - // SetNextConsensus sets next consensus field. - SetNextConsensus(nc A) } diff --git a/send.go b/send.go index 02431ced..f4e1f5ec 100644 --- a/send.go +++ b/send.go @@ -17,11 +17,7 @@ func (d *DBFT[H, A]) broadcast(msg ConsensusPayload[H, A]) { func (c *Context[H, A]) makePrepareRequest() ConsensusPayload[H, A] { c.Fill() - req := c.Config.NewPrepareRequest() - req.SetTimestamp(c.Timestamp) - req.SetNonce(c.Nonce) - req.SetNextConsensus(c.NextConsensus) - req.SetTransactionHashes(c.TransactionHashes) + req := c.Config.NewPrepareRequest(c.Timestamp, c.Nonce, c.NextConsensus, c.TransactionHashes) return c.Config.NewConsensusPayload(c, PrepareRequestType, req) } From fca4ff6d44a018b5d5138afe6c4bb55ce534962f Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 5 Mar 2024 20:51:49 +0300 Subject: [PATCH 5/6] dbft: replace setters with extended constructor for dbft.PrepareResponse A part of #84. Signed-off-by: Anna Shaleva --- config.go | 4 ++-- dbft_test.go | 5 ++--- internal/payload/constructors.go | 6 ++++-- internal/payload/prepare_response.go | 5 ----- prepare_response.go | 2 -- send.go | 3 +-- 6 files changed, 9 insertions(+), 16 deletions(-) diff --git a/config.go b/config.go index 5307e942..5a6610e2 100644 --- a/config.go +++ b/config.go @@ -64,7 +64,7 @@ type Config[H Hash, A Address] struct { // NewPrepareRequest is a constructor for payload.PrepareRequest. NewPrepareRequest func(ts uint64, nonce uint64, nextConsensus A, transactionHashes []H) PrepareRequest[H, A] // NewPrepareResponse is a constructor for payload.PrepareResponse. - NewPrepareResponse func() PrepareResponse[H] + NewPrepareResponse func(preparationHash H) PrepareResponse[H] // NewChangeView is a constructor for payload.ChangeView. NewChangeView func(newViewNumber byte, reason ChangeViewReason, timestamp uint64) ChangeView // NewCommit is a constructor for payload.Commit. @@ -313,7 +313,7 @@ func WithNewPrepareRequest[H Hash, A Address](f func(ts uint64, nonce uint64, ne } // WithNewPrepareResponse sets NewPrepareResponse. -func WithNewPrepareResponse[H Hash, A Address](f func() PrepareResponse[H]) func(config *Config[H, A]) { +func WithNewPrepareResponse[H Hash, A Address](f func(preparationHash H) PrepareResponse[H]) func(config *Config[H, A]) { return func(cfg *Config[H, A]) { cfg.NewPrepareResponse = f } diff --git a/dbft_test.go b/dbft_test.go index 8f122f31..a6f59720 100644 --- a/dbft_test.go +++ b/dbft_test.go @@ -468,7 +468,7 @@ func TestDBFT_Invalid(t *testing.T) { require.Nil(t, dbft.New(opts...)) }) - opts = append(opts, dbft.WithNewPrepareResponse[crypto.Uint256, crypto.Uint160](func() dbft.PrepareResponse[crypto.Uint256] { + opts = append(opts, dbft.WithNewPrepareResponse[crypto.Uint256, crypto.Uint160](func(crypto.Uint256) dbft.PrepareResponse[crypto.Uint256] { return nil })) t.Run("without NewChangeView", func(t *testing.T) { @@ -749,8 +749,7 @@ func (s testState) getCommit(from uint16, sign []byte) Payload { } func (s testState) getPrepareResponse(from uint16, phash crypto.Uint256) Payload { - resp := payload.NewPrepareResponse() - resp.SetPreparationHash(phash) + resp := payload.NewPrepareResponse(phash) p := payload.NewConsensusPayload(dbft.PrepareResponseType, s.currHeight+1, from, 0, resp) return p diff --git a/internal/payload/constructors.go b/internal/payload/constructors.go index 4b6e0603..0adb4e86 100644 --- a/internal/payload/constructors.go +++ b/internal/payload/constructors.go @@ -29,8 +29,10 @@ func NewPrepareRequest(ts uint64, nonce uint64, nextConsensus crypto.Uint160, tr } // NewPrepareResponse returns minimal PrepareResponse implementation. -func NewPrepareResponse() dbft.PrepareResponse[crypto.Uint256] { - return new(prepareResponse) +func NewPrepareResponse(preparationHash crypto.Uint256) dbft.PrepareResponse[crypto.Uint256] { + return &prepareResponse{ + preparationHash: preparationHash, + } } // NewChangeView returns minimal ChangeView implementation. diff --git a/internal/payload/prepare_response.go b/internal/payload/prepare_response.go index fb49bfe5..2d5d5e47 100644 --- a/internal/payload/prepare_response.go +++ b/internal/payload/prepare_response.go @@ -41,8 +41,3 @@ func (p *prepareResponse) DecodeBinary(r *gob.Decoder) error { func (p *prepareResponse) PreparationHash() crypto.Uint256 { return p.preparationHash } - -// SetPreparationHash implements PrepareResponse interface. -func (p *prepareResponse) SetPreparationHash(h crypto.Uint256) { - p.preparationHash = h -} diff --git a/prepare_response.go b/prepare_response.go index 917e1e2c..1675d185 100644 --- a/prepare_response.go +++ b/prepare_response.go @@ -5,6 +5,4 @@ type PrepareResponse[H Hash] interface { // PreparationHash returns the hash of PrepareRequest payload // for this epoch. PreparationHash() H - // SetPreparationHash sets preparations hash. - SetPreparationHash(h H) } diff --git a/send.go b/send.go index f4e1f5ec..675e9d37 100644 --- a/send.go +++ b/send.go @@ -84,8 +84,7 @@ func (d *DBFT[H, A]) sendChangeView(reason ChangeViewReason) { } func (c *Context[H, A]) makePrepareResponse() ConsensusPayload[H, A] { - resp := c.Config.NewPrepareResponse() - resp.SetPreparationHash(c.PreparationPayloads[c.PrimaryIndex].Hash()) + resp := c.Config.NewPrepareResponse(c.PreparationPayloads[c.PrimaryIndex].Hash()) msg := c.Config.NewConsensusPayload(c, PrepareResponseType, resp) c.PreparationPayloads[c.MyIndex] = msg From a2fdfa4a1533f270903251409e36a8aa191ffaed Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 5 Mar 2024 20:56:35 +0300 Subject: [PATCH 6/6] dbft: replace setters with extended constructor for dbft.RecoveryRequest A part of #84. Signed-off-by: Anna Shaleva --- config.go | 4 ++-- dbft_test.go | 4 ++-- internal/payload/constructors.go | 6 ++++-- internal/payload/message_test.go | 3 +-- internal/payload/recovery_request.go | 5 ----- recovery_request.go | 2 -- send.go | 3 +-- 7 files changed, 10 insertions(+), 17 deletions(-) diff --git a/config.go b/config.go index 5a6610e2..d0b3f316 100644 --- a/config.go +++ b/config.go @@ -70,7 +70,7 @@ type Config[H Hash, A Address] struct { // NewCommit is a constructor for payload.Commit. NewCommit func(signature []byte) Commit // NewRecoveryRequest is a constructor for payload.RecoveryRequest. - NewRecoveryRequest func() RecoveryRequest + NewRecoveryRequest func(ts uint64) RecoveryRequest // NewRecoveryMessage is a constructor for payload.RecoveryMessage. NewRecoveryMessage func() RecoveryMessage[H, A] // VerifyPrepareRequest can perform external payload verification and returns true iff it was successful. @@ -334,7 +334,7 @@ func WithNewCommit[H Hash, A Address](f func([]byte) Commit) func(config *Config } // WithNewRecoveryRequest sets NewRecoveryRequest. -func WithNewRecoveryRequest[H Hash, A Address](f func() RecoveryRequest) func(config *Config[H, A]) { +func WithNewRecoveryRequest[H Hash, A Address](f func(ts uint64) RecoveryRequest) func(config *Config[H, A]) { return func(cfg *Config[H, A]) { cfg.NewRecoveryRequest = f } diff --git a/dbft_test.go b/dbft_test.go index a6f59720..abd7a056 100644 --- a/dbft_test.go +++ b/dbft_test.go @@ -489,7 +489,7 @@ func TestDBFT_Invalid(t *testing.T) { require.Nil(t, dbft.New(opts...)) }) - opts = append(opts, dbft.WithNewRecoveryRequest[crypto.Uint256, crypto.Uint160](func() dbft.RecoveryRequest { + opts = append(opts, dbft.WithNewRecoveryRequest[crypto.Uint256, crypto.Uint160](func(uint64) dbft.RecoveryRequest { return nil })) t.Run("without NewRecoveryMessage", func(t *testing.T) { @@ -738,7 +738,7 @@ func (s testState) getChangeView(from uint16, view byte) Payload { } func (s testState) getRecoveryRequest(from uint16) Payload { - p := payload.NewConsensusPayload(dbft.RecoveryRequestType, s.currHeight+1, from, 0, payload.NewRecoveryRequest()) + p := payload.NewConsensusPayload(dbft.RecoveryRequestType, s.currHeight+1, from, 0, payload.NewRecoveryRequest(0)) return p } diff --git a/internal/payload/constructors.go b/internal/payload/constructors.go index 0adb4e86..68f19fe7 100644 --- a/internal/payload/constructors.go +++ b/internal/payload/constructors.go @@ -51,8 +51,10 @@ func NewCommit(signature []byte) dbft.Commit { } // NewRecoveryRequest returns minimal RecoveryRequest implementation. -func NewRecoveryRequest() dbft.RecoveryRequest { - return new(recoveryRequest) +func NewRecoveryRequest(ts uint64) dbft.RecoveryRequest { + return &recoveryRequest{ + timestamp: nanoSecToSec(ts), + } } // NewRecoveryMessage returns minimal RecoveryMessage implementation. diff --git a/internal/payload/message_test.go b/internal/payload/message_test.go index f0a51bad..59fc44a6 100644 --- a/internal/payload/message_test.go +++ b/internal/payload/message_test.go @@ -161,9 +161,8 @@ func TestPayload_Setters(t *testing.T) { }) t.Run("RecoveryRequest", func(t *testing.T) { - r := NewRecoveryRequest() + r := NewRecoveryRequest(secToNanoSec(321)) - r.SetTimestamp(secToNanoSec(321)) require.EqualValues(t, secToNanoSec(321), r.Timestamp()) }) diff --git a/internal/payload/recovery_request.go b/internal/payload/recovery_request.go index 5fe78446..43576139 100644 --- a/internal/payload/recovery_request.go +++ b/internal/payload/recovery_request.go @@ -40,8 +40,3 @@ func (m *recoveryRequest) DecodeBinary(r *gob.Decoder) error { func (m *recoveryRequest) Timestamp() uint64 { return secToNanoSec(m.timestamp) } - -// SetTimestamp implements RecoveryRequest interface. -func (m *recoveryRequest) SetTimestamp(ts uint64) { - m.timestamp = nanoSecToSec(ts) -} diff --git a/recovery_request.go b/recovery_request.go index 08d74137..232b9cf9 100644 --- a/recovery_request.go +++ b/recovery_request.go @@ -4,6 +4,4 @@ package dbft type RecoveryRequest interface { // Timestamp returns this message's timestamp. Timestamp() uint64 - // SetTimestamp sets this message's timestamp. - SetTimestamp(ts uint64) } diff --git a/send.go b/send.go index 675e9d37..4e32149a 100644 --- a/send.go +++ b/send.go @@ -131,8 +131,7 @@ func (d *DBFT[H, A]) sendRecoveryRequest() { if d.RequestSentOrReceived() && !d.hasAllTransactions() { d.processMissingTx() } - req := d.NewRecoveryRequest() - req.SetTimestamp(uint64(d.Timer.Now().UnixNano())) + req := d.NewRecoveryRequest(uint64(d.Timer.Now().UnixNano())) d.broadcast(d.Config.NewConsensusPayload(&d.Context, RecoveryRequestType, req)) }