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

Migrate to btcd/btcutils submodule #787

Closed
3nprob opened this issue Jan 25, 2022 · 7 comments
Closed

Migrate to btcd/btcutils submodule #787

3nprob opened this issue Jan 25, 2022 · 7 comments

Comments

@3nprob
Copy link

3nprob commented Jan 25, 2022

As per btcsuite/btcd#1785, btcutil needs to migrate as well in order to afford upgrading the btcd dependency. Attempting to use current btcd@master:

$ GO111MODULE=on go install -v . ./cmd/...
github.com/btcsuite/btcwallet/internal/legacy/keystore
github.com/btcsuite/btcwallet/wallet/txrules
github.com/lightninglabs/neutrino
# github.com/btcsuite/btcwallet/wallet/txrules
wallet/txrules/rules.go:30:23: cannot use relayFeePerKb (type "github.com/btcsuite/btcutil".Amount) as type "github.com/btcsuite/btcd/btcutil".Amount in argument to mempool.IsDust
# github.com/btcsuite/btcwallet/internal/legacy/keystore
internal/legacy/keystore/keystore.go:2795:3: cannot use addresses (type []"github.com/btcsuite/btcd/btcutil".Address) as type []"github.com/btcsuite/btcutil".Address in field value
internal/legacy/keystore/keystore.go:2877:15: cannot use addresses (type []"github.com/btcsuite/btcd/btcutil".Address) as type []"github.com/btcsuite/btcutil".Address in assignment
# github.com/lightninglabs/neutrino
../../../../pkg/mod/github.com/lightninglabs/neutrino@v0.12.1/blockmanager.go:2718:35: cannot use stubBlock (type *"github.com/btcsuite/btcutil".Block) as type *"github.com/btcsuite/btcd/btcutil".Block in argument to blockchain.CheckProofOfWork
../../../../pkg/mod/github.com/lightninglabs/neutrino@v0.12.1/query.go:988:42: cannot use block (type *"github.com/btcsuite/btcutil".Block) as type *"github.com/btcsuite/btcd/btcutil".Block in argument to blockchain.CheckBlockSanity
../../../../pkg/mod/github.com/lightninglabs/neutrino@v0.12.1/rescan.go:931:53: cannot use relevantTxs (type []*"github.com/btcsuite/btcutil".Tx) as type []*"github.com/btcsuite/btcd/btcutil".Tx in argument to ro.ntfn.OnFilteredBlockConnected
../../../../pkg/mod/github.com/lightninglabs/neutrino@v0.12.1/rescan.go:988:26: cannot use tx (type *"github.com/btcsuite/btcutil".Tx) as type *"github.com/btcsuite/btcd/btcutil".Tx in argument to ro.ntfn.OnRedeemingTx
../../../../pkg/mod/github.com/lightninglabs/neutrino@v0.12.1/rescan.go:1004:21: cannot use tx (type *"github.com/btcsuite/btcutil".Tx) as type *"github.com/btcsuite/btcd/btcutil".Tx in argument to ro.ntfn.OnRecvTx
../../../../pkg/mod/github.com/lightninglabs/neutrino@v0.12.1/rescan.go:1048:44: cannot use relevantTxs (type []*"github.com/btcsuite/btcutil".Tx) as type []*"github.com/btcsuite/btcd/btcutil".Tx in argument to ro.ntfn.OnFilteredBlockConnected
@chappjc
Copy link
Contributor

chappjc commented Jan 25, 2022

Related: #782
With the last tagged release, you cannot just go get or even add a require for btcwallet because it currently relies on replaces.

@3nprob
Copy link
Author

3nprob commented Jan 25, 2022

As seen above, there are also several references in Neutrino, which is addressed here: lightninglabs/neutrino#242

@chappjc
Copy link
Contributor

chappjc commented Jan 25, 2022

Ahhh, dang I did not even see that btcutil was moved into the btcd repo as a submodule in btcsuite/btcd#1785. About a month ago now. I'm just going to close the PR I linked, but note that you can get wallet to build if you set your requires to specific revisions in that way, but I get your issue now that code changes are needed to use the new btcutil import path. 👍

@3nprob
Copy link
Author

3nprob commented Jan 25, 2022

@chappjc Ah , I looked at your PR now - I was actually just about to do a similar one here, to address this issue. Would you say that what you addressed in your PR and 825e09b still is relevant to put on top of a btcd dependency upgrade here?

@chappjc
Copy link
Contributor

chappjc commented Jan 25, 2022

The wtxmgr thing is still an issue, especially for a tagged release, and all the replaces were just hiding the issues from devs who either work directly out of a git workspace or had their own requires patching it up. Tagged releases should really have replaces removed. It would still be nice if 0.13 were properly re-released with a 0.13.1 patch, but with these btcutil changes at hand now, it's probably a moot point now.

I say you should go ahead and fix things up with btcutil. If the maintainers are interested in a usable 0.13.x with the previous btcutil module, I'm happy to update my PR.

With these big btcutil changes, I'm curious if the intent is to bump to 0.14 or just patch 0.13.

@3nprob 3nprob mentioned this issue Jan 25, 2022
@Roasbeef
Copy link
Member

Roasbeef commented Feb 2, 2022

With these big btcutil changes, I'm curious if the intent is to bump to 0.14

We'll likely just do a big ump everywhere.

@Roasbeef
Copy link
Member

Has been done in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants