Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

btcec update leads to go mod tidy failure #252

Closed
marten-seemann opened this issue May 24, 2022 · 9 comments · Fixed by #254
Closed

btcec update leads to go mod tidy failure #252

marten-seemann opened this issue May 24, 2022 · 9 comments · Fixed by #254
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@marten-seemann
Copy link
Contributor

We just released v0.16.0, which include the btcec update (#245).

go mod tidy now produces the following error (see libp2p/go-libp2p#1521):

❯ go mod tidy
github.com/libp2p/go-libp2p imports
        github.com/libp2p/go-libp2p-core/crypto imports
        github.com/btcsuite/btcd/btcec/v2/ecdsa tested by
        github.com/btcsuite/btcd/btcec/v2/ecdsa.test imports
        github.com/btcsuite/btcd/chaincfg/chainhash loaded from github.com/btcsuite/btcd@v0.22.0-beta,
        but go 1.16 would fail to locate it:
        ambiguous import: found package github.com/btcsuite/btcd/chaincfg/chainhash in multiple modules:
        github.com/btcsuite/btcd v0.22.0-beta (/Users/marten/src/go/pkg/mod/github.com/btcsuite/btcd@v0.22.0-beta/chaincfg/chainhash)
        github.com/btcsuite/btcd/chaincfg/chainhash v1.0.0 (/Users/marten/src/go/pkg/mod/github.com/btcsuite/btcd/chaincfg/chainhash@v1.0.0)

To proceed despite packages unresolved in go 1.16:
        go mod tidy -e
If reproducibility with go 1.16 is not needed:
        go mod tidy -compat=1.17
For other options, see:
        https://golang.org/doc/modules/pruning

Updating btced to v2.2.0 (from v2.1.3) doesn't solve the problem. I also haven't succeeded in manually adding github.com/btcsuite/btcd/chaincfg/chainhash to go-libp2p's go.mod file.
People had a similar issue in btcsuite/btcd#1839, but I couldn't make it work in go-libp2p either.

This is now blocking the go-libp2p v0.20.0 release. I'm planning to revert #245, unless we can get this solved within the next 24 hours or so.

cc @brianathere

@marten-seemann marten-seemann added the kind/bug A bug in existing code (including security flaws) label May 24, 2022
@brianathere
Copy link
Contributor

Looking at this now.

@vyzo
Copy link
Contributor

vyzo commented May 24, 2022

Can we fix our CI globally to do 1.17 compat tidy?
We've had this problem elsewhere too.

@marten-seemann
Copy link
Contributor Author

Can we fix our CI globally to do 1.17 compat tidy?

We could, but the problem is that every user of go-libp2p would not be able to run a normal go mod tidy any more.

I don't really understand why the Go tool would care about 1.16 when the go.mod file clearly states go 1.17 (as all our go.mod files do), but apparently that's what the Go team decided.

@galargh
Copy link

galargh commented May 24, 2022

I found another change that makes the errors go away - see libp2p/go-libp2p#1524 - but I'm yet to understand exactly what's going on.

@marten-seemann
Copy link
Contributor Author

Adding the following file to go-libp2p works:

//go:build !go1.17
// +build !go1.17

package libp2p

import (
	_ "github.com/btcsuite/btcd/chaincfg"
)

and then making sure that go.mod has require github.com/btcsuite/btcd v0.22.1.

@marten-seemann
Copy link
Contributor Author

This could also be applied in this repo: #254

Not sure where the right place for this to live would be. Thoughts?

@galargh
Copy link

galargh commented May 24, 2022

Given what I've read so far here's my guess.

Before

go-libp2p depends on btcd@v0.20.1-beta and btcd@v0.22.1-beta.

github.com/libp2p/go-libp2p github.com/btcsuite/btcd@v0.22.0-beta
github.com/libp2p/go-libp2p-core@v0.15.1 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-quic-transport@v0.17.0 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-resource-manager@v0.2.1 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-testing@v0.9.2 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-tls@v0.4.1 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-yamux@v0.9.1 github.com/btcsuite/btcd@v0.20.1-beta

This is fine, these two are compatible (in terms of what go mod tidy does i.e. dep resolution model of go 1.16) with each other.

The only version of chainhash that is used is the one that comes as a subdir of btcd.

After

go-libp2p-core changes dependency on btcd@v0.20.1-beta to a dependency on btcd/btcec/v2 v2.1.3. Important note: btcec depends on released chainhash (v1.0.0 to be exact).

go-libp2p still depends on btcd@v0.20.1-beta and btcd@v0.22.1-beta.

github.com/libp2p/go-libp2p github.com/btcsuite/btcd@v0.22.0-beta
github.com/libp2p/go-libp2p-quic-transport@v0.17.0 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-resource-manager@v0.2.1 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-testing@v0.9.2 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-tls@v0.4.1 github.com/btcsuite/btcd@v0.20.1-beta
github.com/libp2p/go-libp2p-yamux@v0.9.1 github.com/btcsuite/btcd@v0.20.1-beta

But now it also depends on btcec@v2.1.3.

github.com/libp2p/go-libp2p-core@v0.16.0 github.com/btcsuite/btcd/btcec/v2@v2.1.3

And transitively, it also starts depending on released chainhash@v1.0.0.

github.com/btcsuite/btcd/btcec/v2@v2.1.3 github.com/btcsuite/btcd/chaincfg/chainhash@v1.0.0

Now we have 2 incompatible versions of chainhash in chain, one from btcd's subdir and one from a proper v1.0.0 release.

Resolution

We can either force replace btcd to v0.22.1 which doesn't have chainhash subdir at all anymore, or we can upgrade btcd in all of these:

  • libp2p/go-libp2p
  • libp2p/go-libp2p-quic-transport
  • libp2p/go-libp2p-resource-manager
  • libp2p/go-libp2p-testing
  • libp2p/go-libp2p-tls
  • libp2p/go-libp2p-yamux

The latter seems cleaner but it's much more work.

@galargh
Copy link

galargh commented May 24, 2022

This could also be applied in this repo: #254

Not sure where the right place for this to live would be. Thoughts?

Nice! I tested it locally and it seems to do the trick :)

Testing

I checked out your branch in go-libp2p-core and updated the reference in go-libp2p's go.mod to point at the local checkout. After this, I ran go mod tidy and it didn't complain anymore.

@galargh
Copy link

galargh commented May 24, 2022

I think maybe #252 (comment) fits better in go-libp2p because we'd be able to remove it once/if we stop transitively depending on lower versions?

Either way, I think yours is better than my replace all.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
4 participants