Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add reason to BlockResponse messages #607

Merged
merged 5 commits into from
Jul 30, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion sync/bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ func TestMessageCompress(t *testing.T) {
d, _ := b.Bytes()
blocksData = append(blocksData, d)
}
msg := message.NewBlocksResponseMessage(message.ResponseCodeBusy, 1234, 888, blocksData, nil)
msg := message.NewBlocksResponseMessage(message.ResponseCodeRejected, message.ResponseCodeRejected.String(),
amirvalhalla marked this conversation as resolved.
Show resolved Hide resolved
1234, 888, blocksData, nil)
bdl := NewBundle(ts.RandomPeerID(), msg)
bs0, err := bdl.Encode()
assert.NoError(t, err)
Expand Down
11 changes: 4 additions & 7 deletions sync/bundle/message/blocks_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@ type BlocksResponseMessage struct {
From uint32 `cbor:"3,keyasint"`
BlocksData [][]byte `cbor:"4,keyasint"`
LastCertificate *block.Certificate `cbor:"6,keyasint"`
Reason string `cbor:"7,keyasint"`
}

func NewBlocksResponseMessage(code ResponseCode, sid int, from uint32,
func NewBlocksResponseMessage(code ResponseCode, reason string, sid int, from uint32,
blocksData [][]byte, lastCert *block.Certificate) *BlocksResponseMessage {
return &BlocksResponseMessage{
ResponseCode: code,
SessionID: sid,
From: from,
BlocksData: blocksData,
LastCertificate: lastCert,
Reason: reason,
}
}
func (m *BlocksResponseMessage) SanityCheck() error {
Expand Down Expand Up @@ -69,10 +71,5 @@ func (m *BlocksResponseMessage) Fingerprint() string {
}

func (m *BlocksResponseMessage) IsRequestRejected() bool {
if m.ResponseCode == ResponseCodeBusy ||
m.ResponseCode == ResponseCodeRejected {
return true
}

return false
return m.ResponseCode == ResponseCodeRejected
}
22 changes: 15 additions & 7 deletions sync/bundle/message/blocks_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,79 +22,87 @@ func TestBlocksResponseMessage(t *testing.T) {
b := ts.GenerateTestBlock(nil, nil)
c := block.NewCertificate(-1, nil, nil, nil)
d, _ := b.Bytes()
m := NewBlocksResponseMessage(ResponseCodeMoreBlocks, sid, 100, [][]byte{d}, c)
m := NewBlocksResponseMessage(ResponseCodeMoreBlocks, ResponseCodeMoreBlocks.String(), sid, 100, [][]byte{d}, c)

assert.Equal(t, errors.Code(m.SanityCheck()), errors.ErrInvalidRound)
assert.Equal(t, m.Reason, ResponseCodeMoreBlocks.String())
})

t.Run("Unexpected block for height zero", func(t *testing.T) {
b := ts.GenerateTestBlock(nil, nil)
d, _ := b.Bytes()
m := NewBlocksResponseMessage(ResponseCodeMoreBlocks, sid, 0, [][]byte{d}, nil)
m := NewBlocksResponseMessage(ResponseCodeMoreBlocks, ResponseCodeMoreBlocks.String(), sid, 0, [][]byte{d}, nil)

assert.Equal(t, errors.Code(m.SanityCheck()), errors.ErrInvalidHeight)
assert.Equal(t, m.Reason, ResponseCodeMoreBlocks.String())
})

t.Run("OK", func(t *testing.T) {
b1 := ts.GenerateTestBlock(nil, nil)
b2 := ts.GenerateTestBlock(nil, nil)
d1, _ := b1.Bytes()
d2, _ := b2.Bytes()
m := NewBlocksResponseMessage(ResponseCodeMoreBlocks, sid, 100, [][]byte{d1, d2}, nil)
m := NewBlocksResponseMessage(ResponseCodeMoreBlocks, ResponseCodeMoreBlocks.String(), sid, 100,
[][]byte{d1, d2}, nil)

assert.NoError(t, m.SanityCheck())
assert.Zero(t, m.LastCertificateHeight())
assert.Contains(t, m.Fingerprint(), "100")
assert.Equal(t, m.Reason, ResponseCodeMoreBlocks.String())
})
}

func TestLatestBlocksResponseCode(t *testing.T) {
ts := testsuite.NewTestSuite(t)

t.Run("busy", func(t *testing.T) {
m := NewBlocksResponseMessage(ResponseCodeBusy, 1, 0, nil, nil)
m := NewBlocksResponseMessage(ResponseCodeRejected, ResponseCodeRejected.String(), 1, 0, nil, nil)

assert.NoError(t, m.SanityCheck())
assert.Zero(t, m.From)
assert.Zero(t, m.To())
assert.Zero(t, m.Count())
assert.True(t, m.IsRequestRejected())
assert.Equal(t, m.Reason, ResponseCodeRejected.String())
})

t.Run("rejected", func(t *testing.T) {
m := NewBlocksResponseMessage(ResponseCodeRejected, 1, 0, nil, nil)
m := NewBlocksResponseMessage(ResponseCodeRejected, ResponseCodeRejected.String(), 1, 0, nil, nil)

assert.NoError(t, m.SanityCheck())
assert.Zero(t, m.From)
assert.Zero(t, m.To())
assert.Zero(t, m.Count())
assert.True(t, m.IsRequestRejected())
assert.Equal(t, m.Reason, ResponseCodeRejected.String())
})

t.Run("OK - MoreBlocks", func(t *testing.T) {
b1 := ts.GenerateTestBlock(nil, nil)
b2 := ts.GenerateTestBlock(nil, nil)
d1, _ := b1.Bytes()
d2, _ := b2.Bytes()
m := NewBlocksResponseMessage(ResponseCodeMoreBlocks, 1, 100, [][]byte{d1, d2}, nil)
m := NewBlocksResponseMessage(ResponseCodeMoreBlocks, ResponseCodeMoreBlocks.String(), 1, 100, [][]byte{d1, d2}, nil)

assert.NoError(t, m.SanityCheck())
assert.Equal(t, m.From, uint32(100))
assert.Equal(t, m.To(), uint32(101))
assert.Equal(t, m.Count(), uint32(2))
assert.Zero(t, m.LastCertificateHeight())
assert.False(t, m.IsRequestRejected())
assert.Equal(t, m.Reason, ResponseCodeMoreBlocks.String())
})

t.Run("OK - Synced", func(t *testing.T) {
cert := ts.GenerateTestCertificate(ts.RandomHash())

m := NewBlocksResponseMessage(ResponseCodeSynced, 1, 100, nil, cert)
m := NewBlocksResponseMessage(ResponseCodeSynced, ResponseCodeSynced.String(), 1, 100, nil, cert)
assert.NoError(t, m.SanityCheck())
assert.Equal(t, m.From, uint32(100))
assert.Zero(t, m.To())
assert.Zero(t, m.Count())
assert.Equal(t, m.LastCertificateHeight(), uint32(100))
assert.False(t, m.IsRequestRejected())
assert.Equal(t, m.Reason, ResponseCodeSynced.String())
})
}
11 changes: 4 additions & 7 deletions sync/bundle/message/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,17 @@ const (
ResponseCodeNone = ResponseCode(-1)
ResponseCodeOK = ResponseCode(0)
ResponseCodeRejected = ResponseCode(1)
ResponseCodeBusy = ResponseCode(2)
ResponseCodeMoreBlocks = ResponseCode(3)
ResponseCodeNoMoreBlocks = ResponseCode(4)
ResponseCodeSynced = ResponseCode(5)
ResponseCodeMoreBlocks = ResponseCode(2)
ResponseCodeNoMoreBlocks = ResponseCode(3)
ResponseCodeSynced = ResponseCode(4)
)

func (c ResponseCode) String() string {
switch c {
case ResponseCodeOK:
return "ok"
case ResponseCodeRejected:
return "rejected"
case ResponseCodeBusy:
return "busy"
return "too many requests"
case ResponseCodeMoreBlocks:
return "more-blocks"
case ResponseCodeNoMoreBlocks:
Expand Down
23 changes: 13 additions & 10 deletions sync/handler_blocks_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

if handler.peerSet.NumberOfOpenSessions() > handler.config.MaxOpenSessions {
handler.logger.Warn("we are busy", "message", msg, "pid", initiator)
response := message.NewBlocksResponseMessage(message.ResponseCodeBusy,
response := message.NewBlocksResponseMessage(message.ResponseCodeRejected, message.ResponseCodeRejected.String(),
amirvalhalla marked this conversation as resolved.
Show resolved Hide resolved
msg.SessionID, 0, nil, nil)
handler.sendTo(response, initiator, msg.SessionID)

Expand All @@ -33,32 +33,35 @@

peer := handler.peerSet.GetPeer(initiator)
if !peer.IsKnownOrTrusty() {
response := message.NewBlocksResponseMessage(message.ResponseCodeRejected,
err := errors.Errorf(errors.ErrInvalidMessage, "peer status is %v", peer.Status)
response := message.NewBlocksResponseMessage(message.ResponseCodeRejected, err.Error(),
msg.SessionID, 0, nil, nil)
handler.sendTo(response, initiator, msg.SessionID)

return errors.Errorf(errors.ErrInvalidMessage, "peer status is %v", peer.Status)
return err
}

if !handler.config.NodeNetwork {
ourHeight := handler.state.LastBlockHeight()
if msg.From < ourHeight-LatestBlockInterval {
response := message.NewBlocksResponseMessage(message.ResponseCodeRejected,
err := errors.Errorf(errors.ErrInvalidMessage, "the request height is not acceptable: %v", msg.From)
response := message.NewBlocksResponseMessage(message.ResponseCodeRejected, err.Error(),
msg.SessionID, 0, nil, nil)
handler.sendTo(response, initiator, msg.SessionID)

return errors.Errorf(errors.ErrInvalidMessage, "the request height is not acceptable: %v", msg.From)
return err
}
}
height := msg.From
count := msg.Count

if count > LatestBlockInterval {
response := message.NewBlocksResponseMessage(message.ResponseCodeRejected,
err := errors.Errorf(errors.ErrInvalidMessage, "too many blocks requested: %v-%v", msg.From, msg.Count)
response := message.NewBlocksResponseMessage(message.ResponseCodeRejected, err.Error(),

Check warning on line 60 in sync/handler_blocks_request.go

View check run for this annotation

Codecov / codecov/patch

sync/handler_blocks_request.go#L59-L60

Added lines #L59 - L60 were not covered by tests
msg.SessionID, 0, nil, nil)
handler.sendTo(response, initiator, msg.SessionID)

return errors.Errorf(errors.ErrInvalidMessage, "too many blocks requested: %v-%v", msg.From, msg.Count)
return err

Check warning on line 64 in sync/handler_blocks_request.go

View check run for this annotation

Codecov / codecov/patch

sync/handler_blocks_request.go#L64

Added line #L64 was not covered by tests
}

// Help this peer to sync up
Expand All @@ -69,7 +72,7 @@
break
}

response := message.NewBlocksResponseMessage(message.ResponseCodeMoreBlocks,
response := message.NewBlocksResponseMessage(message.ResponseCodeMoreBlocks, message.ResponseCodeMoreBlocks.String(),
msg.SessionID, height, blocksData, nil)
handler.sendTo(response, initiator, msg.SessionID)

Expand All @@ -85,12 +88,12 @@

if msg.To() >= handler.state.LastBlockHeight() {
lastCertificate := handler.state.LastCertificate()
response := message.NewBlocksResponseMessage(message.ResponseCodeSynced,
response := message.NewBlocksResponseMessage(message.ResponseCodeSynced, message.ResponseCodeSynced.String(),
msg.SessionID, peerHeight, nil, lastCertificate)
handler.sendTo(response, initiator, msg.SessionID)
} else {
response := message.NewBlocksResponseMessage(message.ResponseCodeNoMoreBlocks,
msg.SessionID, 0, nil, nil)
message.ResponseCodeNoMoreBlocks.String(), msg.SessionID, 0, nil, nil)
handler.sendTo(response, initiator, msg.SessionID)
}

Expand Down
2 changes: 1 addition & 1 deletion sync/handler_blocks_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func TestLatestBlocksRequestMessages(t *testing.T) {
msg := message.NewBlocksRequestMessage(s.SessionID(), 100, 105)
assert.NoError(t, td.receivingNewMessage(td.sync, msg, pid))
bdl := td.shouldPublishMessageWithThisType(t, td.network, message.TypeBlocksResponse)
assert.Equal(t, bdl.Message.(*message.BlocksResponseMessage).ResponseCode, message.ResponseCodeBusy)
assert.Equal(t, bdl.Message.(*message.BlocksResponseMessage).ResponseCode, message.ResponseCodeRejected)
})
})
}
8 changes: 4 additions & 4 deletions sync/handler_blocks_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestInvalidBlockData(t *testing.T) {

pid := td.RandomPeerID()
sid := td.sync.peerSet.OpenSession(pid).SessionID()
msg := message.NewBlocksResponseMessage(message.ResponseCodeMoreBlocks, sid,
msg := message.NewBlocksResponseMessage(message.ResponseCodeMoreBlocks, message.ResponseCodeMoreBlocks.String(), sid,
0, [][]byte{{1, 2, 3}}, nil)

assert.Error(t, td.receivingNewMessage(td.sync, msg, pid))
Expand All @@ -40,7 +40,7 @@ func TestOneBlockShorter(t *testing.T) {

t.Run("Peer is busy. Session should be closed", func(t *testing.T) {
sid := td.sync.peerSet.OpenSession(pid).SessionID()
msg := message.NewBlocksResponseMessage(message.ResponseCodeBusy, sid,
msg := message.NewBlocksResponseMessage(message.ResponseCodeRejected, message.ResponseCodeRejected.String(), sid,
0, nil, nil)
assert.NoError(t, td.receivingNewMessage(td.sync, msg, pid))

Expand All @@ -49,7 +49,7 @@ func TestOneBlockShorter(t *testing.T) {

t.Run("Request is rejected. Session should be closed", func(t *testing.T) {
sid := td.sync.peerSet.OpenSession(pid).SessionID()
msg := message.NewBlocksResponseMessage(message.ResponseCodeRejected, sid,
msg := message.NewBlocksResponseMessage(message.ResponseCodeRejected, message.ResponseCodeRejected.String(), sid,
0, nil, nil)
assert.NoError(t, td.receivingNewMessage(td.sync, msg, pid))

Expand All @@ -58,7 +58,7 @@ func TestOneBlockShorter(t *testing.T) {

t.Run("Commit one block", func(t *testing.T) {
sid := td.sync.peerSet.OpenSession(pid).SessionID()
msg := message.NewBlocksResponseMessage(message.ResponseCodeSynced, sid,
msg := message.NewBlocksResponseMessage(message.ResponseCodeSynced, message.ResponseCodeRejected.String(), sid,
lastBlockHeight+1, [][]byte{d1}, c1)
assert.NoError(t, td.receivingNewMessage(td.sync, msg, pid))

Expand Down
5 changes: 0 additions & 5 deletions sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,11 +482,6 @@ func (sync *synchronizer) updateSession(sessionID int, pid peer.ID, code message
sync.peerSet.IncreaseSendFailedCounter(pid)
sync.updateBlockchain()

case message.ResponseCodeBusy:
sync.logger.Debug("peer is busy. close session", "session-id", sessionID)
sync.peerSet.CloseSession(sessionID)
sync.updateBlockchain()

case message.ResponseCodeMoreBlocks:
sync.logger.Debug("peer responding us. keep session open", "session-id", sessionID)

Expand Down
3 changes: 2 additions & 1 deletion sync/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ func TestDownload(t *testing.T) {

t.Run("download request is rejected", func(t *testing.T) {
session := td.sync.peerSet.OpenSession(pid)
msg2 := message.NewBlocksResponseMessage(message.ResponseCodeRejected, session.SessionID(), 1, nil, nil)
msg2 := message.NewBlocksResponseMessage(message.ResponseCodeRejected, message.ResponseCodeRejected.String(),
session.SessionID(), 1, nil, nil)
assert.NoError(t, td.receivingNewMessage(td.sync, msg2, pid))
bdl := td.shouldPublishMessageWithThisType(t, td.network, message.TypeBlocksRequest)

Expand Down
Loading