Add Maybe to the end bound of proofs (Part 2) #1813
Conversation
…into MaybeBounds1
…into MaybeBounds1
x/sync/g_db/db_client.go
Outdated
| @@ -16,6 +16,13 @@ import ( | |||
| pb "github.com/ava-labs/avalanchego/proto/pb/sync" | |||
| ) | |||
|
|
|||
| func MaybeBytesToMaybe(mb *pb.MaybeBytes) merkledb.Maybe[[]byte] { | |||
There was a problem hiding this comment.
nit: should we unexport MaybeBytesToMaybe in both packages if we are replicating this function or should we use one implementation?
There was a problem hiding this comment.
I feel like this function should be defined once with the rest of the maybe code
There was a problem hiding this comment.
It is more of a proto thing though, but you can't define it with protos either
There was a problem hiding this comment.
Not sure I follow. Can't we define this in maybe.go?
There was a problem hiding this comment.
we could, but it isn't really a maybe functionality thing, it is more a codec/ marshaling thing which primarily lives in proto and codec. It feels weird to have this in the merkledb module since it isn't called anywhere in the merkledb module. There was also talk of moving maybe.go out of merkledb to someplace more common.
There was a problem hiding this comment.
This would be in line with the UnmarshalProto and ToProto definitions in merkledb, which also aren't used in merkledb. I feel like it makes sense for an operation defined on Maybe to live with the maybe code. I agree that moving maybe to its own package would make sense in a subsequent PR.
There was a problem hiding this comment.
Also related to the above, I think it might make sense to name this UnmarshalProto for parallelism with other similar functions
There was a problem hiding this comment.
It isn't a Maybe function, it is a maybebytes function. I'm just going to remove it from the gdb code and leave it unexported in sync.
…into MaybeBounds2
There have been several bugs related to the nil key being confused for no end bound. By converting the end bounds to Maybe[T], it will be more explicit in the code which bound is meant (nil key vs no bound)
See also #1793 #1657