Skip to content

feat: add regtest to btcwallet#975

Merged
guggero merged 5 commits intobtcsuite:masterfrom
Abdulkbk:regtest-support
Feb 21, 2025
Merged

feat: add regtest to btcwallet#975
guggero merged 5 commits intobtcsuite:masterfrom
Abdulkbk:regtest-support

Conversation

@Abdulkbk
Copy link
Contributor

@Abdulkbk Abdulkbk commented Feb 11, 2025

closes #955

Following up on the comment, In this PR we add regtest support to btcwallet.

A regtest environment can benefit users who want to test their applications or programs. Specifically, I am working on a project to add BTCD support in Polar, which depends on this, as all other node implementations operate on regtest. This becomes a blocker as highlighted in this comment:

The second blocker is that btcwallet does not support regtest currently, meanwhile all the other node implementations in Polar use regtest. I think this is the real challenge.

Steps to test

  • Start BTCD in regtest
btcd --regtest
  • Initialize btcwallet on regtest
btcwallet --regtest --create
  • Finally start btcwallet
btcwallet --regtest

You can access btcwallet using the default port, which is set as 18332.

@Abdulkbk
Copy link
Contributor Author

Could you help approve CI @yyforyongyu 🙏

@yyforyongyu
Copy link
Collaborator

Approved CI run.

@Abdulkbk
Copy link
Contributor Author

The linter is throwing an error about tagalign which I don't quite get.

Error: cmd/sweepaccount/main.go:46:44: tag is not aligned, should be: description:"Use the regression bitcoin network"          long:"regtest" (tagalign)
	RegressionNet         bool                `long:"regtest" description:"Use the regression bitcoin network"`

Interestingly, the other lines follow the same format, yet the linter only complains about this. I wonder if this has any connection with this comment?. I would look into it deeper and see what I will find.

@guggero
Copy link
Collaborator

guggero commented Feb 13, 2025

The linter is throwing an error about tagalign which I don't quite get.

Error: cmd/sweepaccount/main.go:46:44: tag is not aligned, should be: description:"Use the regression bitcoin network"          long:"regtest" (tagalign)
	RegressionNet         bool                `long:"regtest" description:"Use the regression bitcoin network"`

Interestingly, the other lines follow the same format, yet the linter only complains about this. I wonder if this has any connection with this comment?. I would look into it deeper and see what I will find.

Hmm, I've never seen that linter error before. Only seems to mean the formatting of the tags, which looks okay to me.
Feel free to add tagalign to the list of disabled linters in .golangci.yml.

@Abdulkbk Abdulkbk force-pushed the regtest-support branch 3 times, most recently from 7d97082 to be21cd7 Compare February 13, 2025 17:17
@Abdulkbk
Copy link
Contributor Author

Hmm, I've never seen that linter error before. Only seems to mean the formatting of the tags, which looks okay to me. Feel free to add tagalign to the list of disabled linters in .golangci.yml.

I disabled tagalign and resolved the issue. I then encountered some linting issues with characters exceeding the max (120):

config.go:100: the line is 205 characters long, which exceeds the maximum of 120 characters. (lll)
        LegacyRPCListeners     []string                `long:"rpclisten" description:"Listen for legacy RPC connections on this interface/port (default port: 8332, testnet: 18332, simnet: 18554, regtest: 18332)"`

Fixed that by adding //nolint:lll just above the config struct definition, just like in LND

@kelis-cpu
Copy link

thx man! you are my hero!!!

Copy link
Collaborator

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I wonder if there's a way to test it?

In this commit, we add checks to set the right settings when the
user is on regtest.
In this commit we disable tagalign and turn off lll lint
on config struct
@Abdulkbk
Copy link
Contributor Author

Looking good! I wonder if there's a way to test it?

Are you specifically referring to writing tests for regtest?

Copy link
Collaborator

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you specifically referring to writing tests for regtest?

More like testing the config validations are applied, but it's minor. As for the regtest, I think it's missing an integration test framework (sth like itest used in lnd), which I will write about it.

@Abdulkbk
Copy link
Contributor Author

More like testing the config validations are applied, but it's minor. As for the regtest, I think it's missing an integration test framework (sth like itest used in lnd), which I will write about it.

Alright, I'm looking forward to what you'll write then.

Also what next steps would you recommend I take to move this PR forward?

@yyforyongyu
Copy link
Collaborator

Also what next steps would you recommend I take to move this PR forward?

Think we just need another reviewer🤓

@guggero guggero self-requested a review February 20, 2025 16:40
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK, LGTM 🎉

@guggero guggero merged commit 311812f into btcsuite:master Feb 21, 2025
3 checks passed
@Abdulkbk
Copy link
Contributor Author

Are you specifically referring to writing tests for regtest?

More like testing the config validations are applied, but it's minor. As for the regtest, I think it's missing an integration test framework (sth like itest used in lnd), which I will write about it.

Hi @yyforyongyu, you mentioned something about the missing itest framework. Could you elaborate on that? I want to see if it's something I can work on.

@yyforyongyu
Copy link
Collaborator

@Abdulkbk since we have plans to turn the walletdb into SQL, it'd be great if we have a strong test framework in place to aid the development. On a high-level, the idea is to ensure the wallet can function against different chain backends - bitcoind, btcd, and neutrino. In specific,

  • we may need to revive the wallet rpc here in btcwallet so we have endpoints to test, still need more assessment.
  • we need to test all public exported methods - as a starting point, we need to identify a set of core functions to test, such as the chain.Interface and the wallet.
  • when building the CI, we also want to test for compatibility - that is, we need to check the above set of methods against different versions of chain backends. I think testing the last major three versions of each chain backends would be a good starting point.

Basically I want the btcwallet to export two public interfaces in the end, a chain interface to deal with onchain events, and a wallet interface to access the wallet data, which means there is some refactoring work to do, sth we aim to accomplish when we SQLing the walletdb. By the end of it, I'm hoping we have a similar itest framework here - that we test these two core interfaces against different chain backends.

This is still pretty raw, so yeah maybe come visit this topic later when we have a more detailed plan.

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

Successfully merging this pull request may close these issues.

Why regtest network is absent in btcwallet?

4 participants