-
Notifications
You must be signed in to change notification settings - Fork 22
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
api/v0: reject Metadata.ProtocolID == 0 #95
Conversation
Why is Circle running and just erroring immediately? |
Because nobody has pushed #87 over the finish line |
Integers in Go default to the zero value, meaning that if the user forgets to set the protocol ID, it could transparently default to a confusing value. For instance: metadata := v0.Metadata{Data: someData} This could lead to confusing decode errors later on, or to metadata being mistaken for the wrong kind. Instead, reject this zero value. The protocol ID is a multihash anyway, and we want users to either select registered indexer metadata multihashes, or a multihash in the reserved range for testing. The check happens at both encode and decode, for extra safety. This required adding an error return on the encode side. While we're there, make Metadata implement encoding's Binary interfaces. Updates #94.
dfd2784
to
5df6652
Compare
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.
LGTM.
// EncodeMetadata serializes Metadata to []byte. | ||
func (m Metadata) Encode() []byte { | ||
// MarshalBinary implements encoding.BinaryMarshaler. | ||
func (m Metadata) MarshalBinary() ([]byte, 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.
🎉
(see commit message)