trees/cosigned: Add cosigned.Message#8819
Conversation
| LogOrigin string | ||
| Start uint64 | ||
| End uint64 | ||
| SubtreeHash [sha256.Size]byte |
There was a problem hiding this comment.
I believe we'll be using tlog.Hash throughout trees so we should use it here to avoid unnecessary conversions.
| SubtreeHash [sha256.Size]byte | |
| SubtreeHash tlog.Hash |
There was a problem hiding this comment.
Good point, but it feels weird to pull in all of tlog in this PR just for an alias type. How about keeping [sha256.Size]byte in this PR, pulling in tlog as a new dependency in #8808 (which uses tlog in earnest), and then bumping this to use the alias type as a followup?
aarongable
left a comment
aarongable
left a comment
There was a problem hiding this comment.
LGTM with one API question.
| } | ||
|
|
||
| // Unmarshal unmarshals the input bytes into its receiver. | ||
| func (message *Message) Unmarshal(input []byte) error { |
There was a problem hiding this comment.
I know that the UnmarshalJSON interface requires a pointer receiver so that it can modify its receiver, but that's because it's an interface and therefore has to be a method.
As far as I know, we're not trying to have this method match anyone's interface. Is there a particular reason to require that users write
cm := new(cosigned.Message)
err := cm.Unmarshal(input)instead of
cm, err := cosigned.Parse(input)?
There was a problem hiding this comment.
No particular API design reason; really only naming cuteness. I'm happy with cosigned.Parse(), though I think I'll write it as cosigned.Unmarshal() to match the existing Marshal() in this package and the Go common usage (e.g. encoding/asn1 also uses Marshal/Unmarshal).
beautifulentropy
changed the title
3 participants
Part of #8818