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.

Sign up for GitHub

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

Jump to bottom

Ambiguous import in a btcd dependency #1063

Closed
D4ryl00 opened this issue Aug 23, 2023 · 2 comments · Fixed by #1064
Closed

Ambiguous import in a btcd dependency #1063

D4ryl00 opened this issue Aug 23, 2023 · 2 comments · Fixed by #1064
Milestone
💡Someday/Maybe

Comments

@D4ryl00
Copy link
Contributor

D4ryl00 commented Aug 23, 2023

Description

In our gnomobile project, we import some Gno packages.
When we try to compile the project with gomobile (a tool that compiles Go code to Android/iOS devices), we have an error in the dependency graph. I think gomobile is more strict to raise errors because I can compile with go build. Anyway, gomobile point out a fatal error:

gobind/gobind imports
       github.com/gnolang/gnomobile/framework imports
       github.com/gnolang/gno/tm2/pkg/crypto/keys imports
       github.com/gnolang/gno/tm2/pkg/crypto/hd imports
       github.com/btcsuite/btcd/btcec tested by
       github.com/btcsuite/btcd/btcec.test imports
       github.com/btcsuite/btcd/chaincfg/chainhash: ambiguous import: found package github.com/btcsuite/btcd/chaincfg/chainhash in multiple modules:
       github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c (/home/rems14/go/pkg/mod/github.com/btcsuite/[email protected]/chaincfg/chainhash)
       github.com/btcsuite/btcd/chaincfg/chainhash v1.0.0 (/home/rems14/go/pkg/mod/github.com/btcsuite/btcd/chaincfg/[email protected])

This error tells us that the package chainhash is imported in two different ways:

  1. directly in a subdirectory of github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c. This is now deprecated.
  2. by its path since chainhash has its own go.mod: github.com/btcsuite/btcd/chaincfg/chainhash

I tried to use a new version of github.com/btcsuite/btcd to solve the problem, but the API changed and gno/tm2/pkg/crypto/secp256k1 uses the old API. It's not a hard work to solve, but it's more complex than the other solution and I need to fix this issue quickly to go forward.

So I tried the opposite: to remove the call of the new path github.com/btcsuite/btcd/chaincfg/chainhash. Gno doesn't call it directly, it's a dependency: github.com/btcsuite/btcd/btcutil v1.1.1. The dependency graph (go mod graph) shows me that btcutil v1.0.0 is also call by github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c, so I tried to downgrade btcutil v1.1.1 to btcutil v1.0.0 and it works! I removed the ambiguous import by keeping the import from btcd v0.22.0-beta.

I'll open a PR to propose my quick fix.

In the future, it should be great to upgrade the btcd dependencies, where some subdirectories become new packages:

  • github.com/btcsuite/btcd/btcec/v2
  • github.com/btcsuite/btcd/chaincfg/chainhash

This would be more robust but it also involves dependency changes in other repos and requires more work. So for now, we can move our project forward with the quick fix PR mentioned above.
@jefft0 jefft0 mentioned this issue Aug 23, 2023
Closed
@D4ryl00 D4ryl00 mentioned this issue Aug 23, 2023
Merged
7 tasks
@jefft0 jefft0 mentioned this issue Aug 23, 2023
Open
@D4ryl00 D4ryl00 mentioned this issue Aug 24, 2023
Merged
@thehowl
Copy link
Member

thehowl commented Aug 24, 2023

I'd avoid downgrading dependencies if possible.

I looked up information about this on the btcd repo. I found this: btcsuite/btcd#1839 (comment)

Can we try applying this diff and see if it works?

diff --git a/go.mod b/go.mod
index eb7c3b9a..4beb5c2e 100644
--- a/go.mod
+++ b/go.mod
@@ -5,7 +5,6 @@ go 1.19
 require (
 	github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c
 	github.com/btcsuite/btcd/btcutil v1.1.1
-	github.com/btcsuite/btcutil v1.0.2
 	github.com/cockroachdb/apd v1.1.0
 	github.com/davecgh/go-spew v1.1.1
 	github.com/dgraph-io/badger/v3 v3.2103.4
diff --git a/go.sum b/go.sum
index c51f8d91..0ba87bc0 100644
--- a/go.sum
+++ b/go.sum
@@ -14,8 +14,6 @@ github.com/btcsuite/btcd/btcutil v1.1.1/go.mod h1:nbKlBMNm9FGsdvKvu0essceubPiAcI
 github.com/btcsuite/btcd/chaincfg/chainhash v1.0.0/go.mod h1:7SFka0XMvUgj3hfZtydOrQY2mwhPclbT2snogU7SQQc=
 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA=
 github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg=
-github.com/btcsuite/btcutil v1.0.2 h1:9iZ1Terx9fMIOtq1VrwdqfsATL9MC2l8ZrUY6YZ2uts=
-github.com/btcsuite/btcutil v1.0.2/go.mod h1:j9HUFwoQRsZL3V4n+qG+CUnEGHOarIxfC3Le2Yhbcts=
 github.com/btcsuite/go-socks v0.0.0-20170105172521-4720035b7bfd/go.mod h1:HHNXQzUsZCxOoE+CPiyCTO6x34Zs86zZUiwtpXoGdtg=
 github.com/btcsuite/goleveldb v0.0.0-20160330041536-7834afc9e8cd/go.mod h1:F+uVaaLLH7j4eDXPRvw78tMflu7Ie2bzYOH4Y8rRKBY=
 github.com/btcsuite/goleveldb v1.0.0/go.mod h1:QiK9vBlgftBg6rWQIj6wFzbPfRjiykIEhBH4obrXJ/I=
@@ -209,7 +207,6 @@ golang.org/x/crypto v0.0.0-20170930174604-9419663f5a44/go.mod h1:6SG95UA2DQfeDnf
 golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
-golang.org/x/crypto v0.0.0-20200115085410-6d4e4cb37c7d/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.12.0 h1:tFM/ta59kqch6LlvYnPa0yx5a83cL2nHflFhYKvv9Yk=
 golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw=
diff --git a/tm2/pkg/crypto/secp256k1/secp256k1_test.go b/tm2/pkg/crypto/secp256k1/secp256k1_test.go
index 86aa058f..7e960b5c 100644
--- a/tm2/pkg/crypto/secp256k1/secp256k1_test.go
+++ b/tm2/pkg/crypto/secp256k1/secp256k1_test.go
@@ -5,7 +5,7 @@ import (
 	"math/big"
 	"testing"
 
-	"github.com/btcsuite/btcutil/base58"
+	"github.com/btcsuite/btcd/btcutil/base58"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"

@D4ryl00
Copy link
Contributor Author

D4ryl00 commented Aug 24, 2023

Same error:

gobind/gobind imports
        github.com/gnolang/gnomobile/framework imports
        github.com/gnolang/gno/tm2/pkg/crypto/keys imports
        github.com/gnolang/gno/tm2/pkg/crypto/hd imports
        github.com/btcsuite/btcd/btcec tested by
        github.com/btcsuite/btcd/btcec.test imports
        github.com/btcsuite/btcd/chaincfg/chainhash: ambiguous import: found package github.com/btcsuite/btcd/chaincfg/chainhash in multiple modules:
        github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c (/home/rems14/go/pkg/mod/github.com/btcsuite/[email protected]/chaincfg/chainhash)
        github.com/btcsuite/btcd/chaincfg/chainhash v1.0.0 (/home/rems14/go/pkg/mod/github.com/btcsuite/btcd/chaincfg/[email protected])

It's because github.com/btcsuite/btcutil doesn't import github.com/btcsuite/btcd/chaincfg/chainhash so that don't resolve the issue. This is its dependency graph in Gno:

github.com/btcsuite/[email protected] github.com/aead/[email protected]
github.com/btcsuite/[email protected] github.com/btcsuite/[email protected]
github.com/btcsuite/[email protected] github.com/davecgh/[email protected]
github.com/btcsuite/[email protected] github.com/kkdai/[email protected]
github.com/btcsuite/[email protected] golang.org/x/[email protected]

thehowl pushed a commit that referenced this issue Aug 24, 2023
@D4ryl00
fix: remove ambiguous import by downgrading btcutil v1.1.1 to v1.0.0 (#…
aa7c1df 
…1064)

This PR fixes #1063.
It downgrades `github.com/btcsuite/btcd/btcutil v1.1.1` to `v1.0.0` to
resolve conflicts in the dependency graph for the
`github.com/btcsuite/btcd` package and its dependencies.
More information can be found in the issue above.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
- [x] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

Signed-off-by: D4ryl00 <[email protected]>
Doozers pushed a commit to Doozers/gno that referenced this issue Aug 31, 2023
@D4ryl00 @Doozers
fix: remove ambiguous import by downgrading btcutil v1.1.1 to v1.0.0 (g…
bc8f280 
…nolang#1064)

This PR fixes gnolang#1063.
It downgrades `github.com/btcsuite/btcd/btcutil v1.1.1` to `v1.0.0` to
resolve conflicts in the dependency graph for the
`github.com/btcsuite/btcd` package and its dependencies.
More information can be found in the issue above.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
- [x] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

Signed-off-by: D4ryl00 <[email protected]>
@moul moul moved this to 🔵 Not Needed for Launch in 🚀 The Launch [DEPRECATED] Sep 8, 2023
@moul moul added this to the 💡Someday/Maybe milestone Sep 8, 2023
@thehowl thehowl mentioned this issue Nov 7, 2023
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
🚀 The Launch [DEPRECATED]
Archived in project
Milestone
💡Someday/Maybe
Development

Successfully merging a pull request may close this issue.

fix: remove ambiguous import by downgrading btcutil v1.1.1 to v1.0.0 D4ryl00/gno
3 participants
@moul @thehowl @D4ryl00