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

Move from gogo protobuf #838

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ The following emojis are used to highlight certain changes:
### Changed

- upgrade to `go-libp2p` [v0.39.0](https://github.com/libp2p/go-libp2p/releases/tag/v0.39.0)
- upgrade to `go-libp2p-kad-dht` [v0.29.0](github.com/libp2p/go-libp2p-kad-dht v0.29.0)

### Removed

Expand Down
72 changes: 46 additions & 26 deletions bitswap/message/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
pool "github.com/libp2p/go-buffer-pool"
"github.com/libp2p/go-libp2p/core/network"
msgio "github.com/libp2p/go-msgio"
"google.golang.org/protobuf/proto"
)

// BitSwapMessage is the basic interface for interacting building, encoding,
Expand Down Expand Up @@ -107,14 +108,13 @@

// Get the size of the entry on the wire
func (e *Entry) Size() int {
epb := e.ToPB()
return epb.Size()
return proto.Size(e.ToPB())
}

// Get the entry in protobuf form
func (e *Entry) ToPB() pb.Message_Wantlist_Entry {
return pb.Message_Wantlist_Entry{
Block: pb.Cid{Cid: e.Cid},
func (e *Entry) ToPB() *pb.Message_Wantlist_Entry {
return &pb.Message_Wantlist_Entry{
Block: e.Cid.Bytes(),
Priority: e.Priority,
Cancel: e.Cancel,
WantType: e.WantType,
Expand Down Expand Up @@ -189,13 +189,17 @@

var errCidMissing = errors.New("missing cid")

func newMessageFromProto(pbm pb.Message) (BitSwapMessage, error) {

Check failure on line 192 in bitswap/message/message.go

View workflow job for this annotation

GitHub Actions / go-check / All

newMessageFromProto passes lock by value: github.com/ipfs/boxo/bitswap/message/pb.Message contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
m := newMsg(pbm.Wantlist.Full)
for _, e := range pbm.Wantlist.Entries {
if !e.Block.Cid.Defined() {
if len(e.Block) == 0 {
return nil, errCidMissing
}
m.addEntry(e.Block.Cid, e.Priority, e.Cancel, e.WantType, e.SendDontHave)
c, err := cid.Cast(e.Block)
if err != nil {
return nil, err
}
m.addEntry(c, e.Priority, e.Cancel, e.WantType, e.SendDontHave)
}

// deprecated
Expand Down Expand Up @@ -226,10 +230,17 @@
}

for _, bi := range pbm.GetBlockPresences() {
if !bi.Cid.Cid.Defined() {
if len(bi.Cid) == 0 {
return nil, errCidMissing
}
c, err := cid.Cast(bi.Cid)
if err != nil {
return nil, err
}
if !c.Defined() {
return nil, errCidMissing
}
m.AddBlockPresence(bi.Cid.Cid, bi.Type)
m.AddBlockPresence(c, bi.Type)
}

m.pendingBytes = pbm.PendingBytes
Expand Down Expand Up @@ -398,10 +409,10 @@
}

func BlockPresenceSize(c cid.Cid) int {
return (&pb.Message_BlockPresence{
Cid: pb.Cid{Cid: c},
return proto.Size(&pb.Message_BlockPresence{
Cid: c.Bytes(),
Type: pb.Message_Have,
}).Size()
})
}

// FromNet generates a new BitswapMessage from incoming data on an io.Reader.
Expand All @@ -410,26 +421,30 @@
return FromMsgReader(reader)
}

// FromPBReader generates a new Bitswap message from a gogo-protobuf reader
// FromPBReader generates a new Bitswap message from a protobuf reader.
func FromMsgReader(r msgio.Reader) (BitSwapMessage, error) {
msg, err := r.ReadMsg()
if err != nil {
return nil, err
}

var pb pb.Message
err = pb.Unmarshal(msg)
err = proto.Unmarshal(msg, &pb)
r.ReleaseMsg(msg)
if err != nil {
return nil, err
}

return newMessageFromProto(pb)

Check failure on line 438 in bitswap/message/message.go

View workflow job for this annotation

GitHub Actions / go-check / All

call of newMessageFromProto copies lock value: github.com/ipfs/boxo/bitswap/message/pb.Message contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
}

func (m *impl) ToProtoV0() *pb.Message {
pbm := new(pb.Message)
pbm.Wantlist.Entries = make([]pb.Message_Wantlist_Entry, 0, len(m.wantlist))
pbm := &pb.Message{
Wantlist: &pb.Message_Wantlist{
Entries: make([]*pb.Message_Wantlist_Entry, 0, len(m.wantlist)),
},
}

for _, e := range m.wantlist {
pbm.Wantlist.Entries = append(pbm.Wantlist.Entries, e.ToPB())
}
Expand All @@ -444,26 +459,30 @@
}

func (m *impl) ToProtoV1() *pb.Message {
pbm := new(pb.Message)
pbm.Wantlist.Entries = make([]pb.Message_Wantlist_Entry, 0, len(m.wantlist))
pbm := &pb.Message{
Wantlist: &pb.Message_Wantlist{
Entries: make([]*pb.Message_Wantlist_Entry, 0, len(m.wantlist)),
},
}

for _, e := range m.wantlist {
pbm.Wantlist.Entries = append(pbm.Wantlist.Entries, e.ToPB())
}
pbm.Wantlist.Full = m.full

blocks := m.Blocks()
pbm.Payload = make([]pb.Message_Block, 0, len(blocks))
pbm.Payload = make([]*pb.Message_Block, 0, len(blocks))
for _, b := range blocks {
pbm.Payload = append(pbm.Payload, pb.Message_Block{
pbm.Payload = append(pbm.Payload, &pb.Message_Block{
Data: b.RawData(),
Prefix: b.Cid().Prefix().Bytes(),
})
}

pbm.BlockPresences = make([]pb.Message_BlockPresence, 0, len(m.blockPresences))
pbm.BlockPresences = make([]*pb.Message_BlockPresence, 0, len(m.blockPresences))
for c, t := range m.blockPresences {
pbm.BlockPresences = append(pbm.BlockPresences, pb.Message_BlockPresence{
Cid: pb.Cid{Cid: c},
pbm.BlockPresences = append(pbm.BlockPresences, &pb.Message_BlockPresence{
Cid: c.Bytes(),
Type: t,
})
}
Expand All @@ -482,18 +501,19 @@
}

func write(w io.Writer, m *pb.Message) error {
size := m.Size()
size := proto.Size(m)

buf := pool.Get(size + binary.MaxVarintLen64)
defer pool.Put(buf)

n := binary.PutUvarint(buf, uint64(size))

written, err := m.MarshalTo(buf[n:])
opts := proto.MarshalOptions{}
written, err := opts.MarshalAppend(buf[:n], m)
if err != nil {
return err
}
n += written
n += len(written)

_, err = w.Write(buf[:n])
return err
Expand Down
27 changes: 17 additions & 10 deletions bitswap/message/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
blocks "github.com/ipfs/go-block-format"
cid "github.com/ipfs/go-cid"
"github.com/ipfs/go-test/random"
"google.golang.org/protobuf/proto"
)

func mkFakeCid(s string) cid.Cid {
Expand All @@ -21,26 +22,31 @@
m := New(true)
m.AddEntry(str, 1, pb.Message_Wantlist_Block, true)

if !wantlistContains(&m.ToProtoV0().Wantlist, str) {
if !wantlistContains(m.ToProtoV0().Wantlist, str) {
t.Fail()
}
}

func TestNewMessageFromProto(t *testing.T) {
str := mkFakeCid("a_key")
protoMessage := new(pb.Message)
protoMessage.Wantlist.Entries = []pb.Message_Wantlist_Entry{
{Block: pb.Cid{Cid: str}},

protoMessage := &pb.Message{
Wantlist: &pb.Message_Wantlist{
Entries: []*pb.Message_Wantlist_Entry{
{Block: str.Bytes()},
},
},
}
if !wantlistContains(&protoMessage.Wantlist, str) {

if !wantlistContains(protoMessage.Wantlist, str) {
t.Fail()
}
m, err := newMessageFromProto(*protoMessage)

Check failure on line 44 in bitswap/message/message_test.go

View workflow job for this annotation

GitHub Actions / go-check / All

call of newMessageFromProto copies lock value: github.com/ipfs/boxo/bitswap/message/pb.Message contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
if err != nil {
t.Fatal(err)
}

if !wantlistContains(&m.ToProtoV0().Wantlist, str) {
if !wantlistContains(m.ToProtoV0().Wantlist, str) {
t.Fail()
}
}
Expand Down Expand Up @@ -92,7 +98,7 @@
m := New(true)
protoBeforeAppend := m.ToProtoV0()
m.AddEntry(str, 1, pb.Message_Wantlist_Block, true)
if wantlistContains(&protoBeforeAppend.Wantlist, str) {
if wantlistContains(protoBeforeAppend.Wantlist, str) {
t.Fail()
}
}
Expand Down Expand Up @@ -162,7 +168,8 @@

func wantlistContains(wantlist *pb.Message_Wantlist, c cid.Cid) bool {
for _, e := range wantlist.GetEntries() {
if e.Block.Cid.Defined() && c.Equals(e.Block.Cid) {
blkCid, err := cid.Cast(e.Block)
if err == nil && blkCid.Defined() && c.Equals(blkCid) {
return true
}
}
Expand Down Expand Up @@ -291,7 +298,7 @@
Cancel: false,
}
epb := e.ToPB()
if e.Size() != epb.Size() {
t.Fatal("entry size calculation incorrect", e.Size(), epb.Size())
if e.Size() != proto.Size(epb) {
t.Fatal("entry size calculation incorrect", e.Size(), proto.Size(epb))
}
}
11 changes: 0 additions & 11 deletions bitswap/message/pb/Makefile

This file was deleted.

44 changes: 0 additions & 44 deletions bitswap/message/pb/cid.go

This file was deleted.

8 changes: 4 additions & 4 deletions bitswap/message/pb/cid_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package bitswap_message_pb_test
package pb

import (
"bytes"
"testing"

pb "github.com/ipfs/boxo/bitswap/message/pb"
u "github.com/ipfs/boxo/util"
"github.com/ipfs/go-cid"
"google.golang.org/protobuf/proto"
)

func TestCID(t *testing.T) {
Expand All @@ -20,8 +20,8 @@ func TestCID(t *testing.T) {
}

c := cid.NewCidV0(u.Hash([]byte("foobar")))
msg := pb.Message_BlockPresence{Cid: pb.Cid{Cid: c}}
actual, err := msg.Marshal()
msg := Message_BlockPresence{Cid: c.Bytes()}
actual, err := proto.Marshal(&msg)
if err != nil {
t.Fatal(err)
}
Expand Down
20 changes: 20 additions & 0 deletions bitswap/message/pb/gen.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// These commands work around namespace conflicts that occur when multiple
// repositories depend on .proto files with generic filenames. Due to the way
// protobuf registers files (e.g., foo.proto or pb/foo.proto), naming
// collisions can occur when the same filename is used across different
// packages.
//
// The only way to generate a *.pb.go file that includes the full package path
// (e.g., github.com/org/repo/pb/foo.proto) is to place the .proto file in a
// directory structure that mirrors its package path.
//
// References:
// - https://protobuf.dev/reference/go/faq#namespace-conflict
// - https://github.com/golang/protobuf/issues/1122#issuecomment-2045945265
//
//go:generate mkdir -p github.com/ipfs/boxo/bitswap/message/pb
//go:generate ln -f message.proto github.com/ipfs/boxo/bitswap/message/pb
//go:generate protoc --go_out=. github.com/ipfs/boxo/bitswap/message//pb/message.proto
//go:generate mv -f github.com/ipfs/boxo/bitswap/message//pb/message.pb.go .
//go:generate rm -rf github.com
package pb
Loading
Loading