Skip to content

Commit

Permalink
Start moving from gogo protobuf (#839)
Browse files Browse the repository at this point in the history
* Move from gogo protobuf

Upgrade to go-libp2p-kad-dht v0.29.0

* write to existing buffer
* need to pass pointer to Message
* Add note in README to check deterministic CIDs
  • Loading branch information
gammazero authored Feb 17, 2025
1 parent 2b56109 commit 281f30d
Show file tree
Hide file tree
Showing 20 changed files with 698 additions and 1,957 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ The following emojis are used to highlight certain changes:

- `provider`: Prevent multiple instances of reprovider.Reprovide() from running at the same time. [#834](https://github.com/ipfs/boxo/pull/834)
- upgrade to `go-libp2p` [v0.39.1](https://github.com/libp2p/go-libp2p/releases/tag/v0.39.1)
- upgrade to `go-libp2p-kad-dht` [v0.29.0](github.com/libp2p/go-libp2p-kad-dht v0.29.0)
- move bitswap and filestore away from gogo protobuf [#839](https://github.com/ipfs/boxo/pull/839)
- updated Go in `go.mod` to 1.23 [#848](https://github.com/ipfs/boxo/pull/848)

**Note: This release contains changes to protocol buffer library code. If you depend on deterministic CIDs then please double-check,, before upgrading, that this release does not generate different CIDs.**

### Removed

### Fixed
Expand Down
77 changes: 48 additions & 29 deletions bitswap/message/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
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 @@ type Entry struct {

// 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 @@ func (m *impl) Reset(full bool) {

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

func newMessageFromProto(pbm pb.Message) (BitSwapMessage, error) {
func newMessageFromProto(pbm *pb.Message) (BitSwapMessage, error) {
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 @@ func newMessageFromProto(pbm pb.Message) (BitSwapMessage, error) {
}

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 (m *impl) Size() int {
}

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,15 +421,15 @@ func FromNet(r io.Reader) (BitSwapMessage, error) {
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)
pb := new(pb.Message)
err = proto.Unmarshal(msg, pb)
r.ReleaseMsg(msg)
if err != nil {
return nil, err
Expand All @@ -428,8 +439,12 @@ func FromMsgReader(r msgio.Reader) (BitSwapMessage, error) {
}

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) ToProtoV0() *pb.Message {
}

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,20 +501,20 @@ func (m *impl) ToNetV1(w io.Writer) error {
}

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{}
wrBuf, err := opts.MarshalAppend(buf[:n], m)
if err != nil {
return err
}
n += written

_, err = w.Write(buf[:n])
_, err = w.Write(wrBuf)
return err
}

Expand Down
29 changes: 18 additions & 11 deletions bitswap/message/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
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 @@ func TestAppendWanted(t *testing.T) {
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)
m, err := newMessageFromProto(protoMessage)
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 @@ func TestCopyProtoByValue(t *testing.T) {
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 TestToAndFromNetMessage(t *testing.T) {

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 @@ func TestEntrySize(t *testing.T) {
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

0 comments on commit 281f30d

Please sign in to comment.