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
En 6847 timestamp in p2p message #2023
En 6847 timestamp in p2p message #2023
Conversation
iulianpascalau
commented
Jun 25, 2020
- added timestamp on each and every p2p message.go
- added extra protection for all peers to follow the message protocol
- added extra protection for all peers to follow the message protocol
cmd/seednode/config/config.toml
Outdated
# 0 disables the feature. | ||
[Marshalizer] | ||
Type = "gogo protobuf" | ||
SizeCheckDelta = 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cmd/seednode/main.go
Outdated
p2pConfigurationFile = "./config/p2p.toml" | ||
|
||
// ErrMarshalizerCreation signals that the marshalizer cannot be created based on provided data | ||
ErrMarshalizerCreation = errors.New("error creating marshalizer") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for this define - just create the error where it is needed. it is in main
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
p2p/data/proto/topicMessage.proto
Outdated
import "github.com/gogo/protobuf/gogoproto/gogo.proto"; | ||
|
||
message TopicMessage{ | ||
bytes Payload = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
var ErrNilCacher = errors.New("nil cacher") | ||
// ErrMessageUnmarshalError signals that an invalid message was received from a peer. There is no way to communicate | ||
// with such a peer as it does not respect the protocol | ||
var ErrMessageUnmarshalError = errors.New("message unmarshal error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty line to end of file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is an empty line there
p2p/libp2p/message.go
Outdated
"github.com/ElrondNetwork/elrond-go/p2p/message" | ||
"github.com/libp2p/go-libp2p-core/peer" | ||
"github.com/libp2p/go-libp2p-pubsub" | ||
) | ||
|
||
// NewMessage returns a new instance of a Message object | ||
func NewMessage(msg *pubsub.Message) (*message.Message, error) { | ||
func NewMessage(msg *pubsub.Message, marshalizer p2p.Marshalizer) (*message.Message, error) { | ||
if marshalizer == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chekc.ifnil with reflection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better, check.IfNil()
p2p/libp2p/message.go
Outdated
SeqNoField: msg.Seqno, | ||
TopicsField: msg.TopicIDs, | ||
SignatureField: msg.Signature, | ||
KeyField: msg.Key, | ||
} | ||
|
||
var err error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for var err error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Usage: "The `" + filePathPlaceholder + "` for the main configuration file. This TOML file contain the main " + | ||
"configurations such as the marshalizer type", | ||
Value: "./config/config.toml", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't have an option for a custom configuration file before this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System tests passed.