-
Notifications
You must be signed in to change notification settings - Fork 1.3k
chore(deps): upgrade boxo to v0.32.0 and go-ipld-cbor to v0.2.1 with API compatibility fixes #13202
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
Conversation
…bor v0.2.1 Co-authored-by: rvagg <[email protected]>
….32.0 compatibility Co-authored-by: rvagg <[email protected]>
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 if CI is happy!
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.
Pull Request Overview
This PR upgrades IPFS-related dependencies to their latest versions (including boxo v0.32.0 and go-ipld-cbor v0.2.1) and applies the necessary API changes for Bitswap and the metrics interface.
- Adapted Bitswap imports, options, and constructor in
node/modules/chain.go
for boxo v0.32.0 - Bumped multiple dependencies in
go.mod
to address compatibility (e.g., go-block-format, go-ipld-format, go-metrics-interface)
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
node/modules/chain.go | Updated Bitswap network import path, options, and constructor call |
go.mod | Upgraded various IPFS, libp2p, and telemetry dependencies |
Comments suppressed due to low confidence (2)
go.mod:88
- The PR description mentions upgrades to boxo and go-ipld-cbor, but additional dependencies (e.g., go-block-format, go-ipld-format, go-metrics-interface) were also bumped. Please update the description to list all upgraded modules for completeness.
github.com/ipfs/boxo v0.32.0
node/modules/chain.go:48
- [nitpick] Consider updating the preceding comment to note the addition of the
rt
routing parameter in the new Bitswap constructor signature, so readers understand why the invocation changed.
exch := bitswap.New(mctx, bitswapNetwork, rt, bitswapBs, bitswapOptions...)
Co-authored-by: rvagg <[email protected]>
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
…r boxo v0.32.0 - Enable bitswap server by changing WithServerEnabled(false) to WithServerEnabled(true) This addresses reviewer feedback that disabling the server would "kill the network" by preventing nodes from serving blocks to peers during chain sync - Fix bitswap network initialization to use correct boxo v0.32.0 API: - Remove routing parameter from bsnet.NewFromIpfsHost() call - Routing is now passed directly to bitswap.New() constructor - Ensures proper network participation and resolves TestSyncSimple failures 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Co-authored-by: BigLep <[email protected]>
Fixed! The issue was that I incorrectly enabled the bitswap server in my previous commit. Reverted |
Zen and I are discussing just pulling bitswap out of lotus entirely, it looks like we're not really using it but need to carefully confirm. So let's hold off on this one for now. |
Ok, that sounds nice, but I assume that isn't free and I know we're all busy right now. I propose we backlog the bitswap removal task (I created #13213 ) and given this PR is green, we merge. This has us holding the status quo but getting us into better operational posture of being able to cut a Lotus release (goal is this week) without backing out other work we've done. Side: I see copilot has backed out |
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.
This PR upgrades the IPFS dependencies to resolve compatibility issues with go-state-types that now requires [email protected] (introduced in go-state-types PR #394).
Dependencies Upgraded
Breaking Changes Fixed
1. Bitswap API Changes (
node/modules/chain.go
)The boxo v0.32.0 upgrade introduced breaking changes to the bitswap API:
Import changes:
Network creation:
Option changes:
Constructor signature:
2. Metrics Interface Compatibility
Upgraded go-metrics-prometheus to v0.1.0 to resolve interface compatibility issues with the new metrics interface from boxo.
Testing
API Compatibility
The upgrade maintains backward compatibility for:
The changes are isolated to the dependency injection layer and do not affect broader lotus functionality.
Fixes #13197.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
esm.ubuntu.com
https://api.github.com/repos/filecoin-project/filecoin-ffi/releases/tags/v1.33.1-dev
curl --retry 3 --location REDACTED
(http block)REDACTED, pid is -1
(http block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.