feat: Add testnet4 chain support #2274

bullet-tooth · 2024-11-10T16:02:12Z

This PR adds support of the Bitcoin testnet version 4 chain (testnet3 is in a way of deprecation).

Testnet4 genesis block: https://mempool.space/testnet4/block/00000000da84f2bafbbc53dee25a72ae507ff4914b867c565be350b0da8bf043

Also, I have intention to add testnet4 support to btcwallet after this PR

Roasbeef · 2024-11-11T18:38:29Z

Looks like we have another here: #2275

Neither that one, nor this one, have any of the new difficulty adjustment algorithm changes.

jcvernaleo · 2024-11-11T19:49:00Z

I know that in #2275 they were working on the difficultly bits. @marcopeereboom I thought you had started on that part, is it in your PR?

bullet-tooth · 2024-11-20T12:59:05Z

@Roasbeef @jcvernaleo thanks for your inputs.
As I can see the #2275 is still in draft so if you could provide a little context about this "difficulty adjustment algorithm changes" I could implement it as well in this PR.

Roasbeef · 2024-11-20T22:52:05Z

@bullet-tooth see the BIP: https://bips.dev/94/

coveralls · 2024-11-25T08:12:09Z

Pull Request Test Coverage Report for Build 12318119030

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

15 of 41 (36.59%) changed or added relevant lines in 8 files are covered.
14 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.04%) to 57.245%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
peer/peer.go	0	1	0.0%
rpcserver.go	0	2	0.0%
blockchain/difficulty.go	6	10	60.0%
config.go	0	4	0.0%
btcd.go	0	5	0.0%
blockchain/validate.go	5	15	33.33%

Files with Coverage Reduction	New Missed Lines	%
rpcclient/infrastructure.go	2	40.17%
txscript/taproot.go	2	95.98%
peer/peer.go	10	74.1%

Totals
Change from base Build 12253655920:	-0.04%
Covered Lines:	29910
Relevant Lines:	52249

💛 - Coveralls

bullet-tooth · 2024-12-10T12:04:14Z

Hey @Roasbeef, do you have any estimates about reviewing/merging this PR?

guggero · 2024-12-10T14:52:04Z

If you clean up the PR into a review friendly commit structure, I'll take a look.
Here are some docs to go by: https://github.com/lightningnetwork/lnd/blob/master/docs/code_contribution_guidelines.md
and https://github.com/lightningnetwork/lnd/blob/master/docs/code_formatting_rules.md.
Those aren't strictly enforced in this project but definitely nice to have if you want review from the lnd team.

Adds support of the Bitcoin testnet version 4 chain. Reference: https://bips.dev/94/

bullet-tooth · 2025-01-14T15:01:06Z

Hello, @guggero. Do you have any estimates when you will be able to review the PR.
Also, such as it already has some conflicts I will resolve them in the near future.

guggero

Thanks for the update.
Still requires quite a bit of formal cleanup before more review time should be put in.

With "review friendly commit structure" I didn't mean squashing everything into a single commit.
Suggestion for commit order:

Any comment/typo fixes.
Any changes to non-Go files (e.g. .gitignore)
Add new parameter to chaincfg including unit tests.
Add EnforceBip94 flag to parameters.
Add changes to blockchain package.
Add changes to cmd package.

guggero · 2025-01-16T12:09:57Z

blockchain/difficulty.go

@@ -191,12 +191,22 @@ func calcNextRequiredDifficulty(lastNode HeaderCtx, newBlockTime time.Time,
 		adjustedTimespan = c.MaxRetargetTimespan()
 	}

+	var oldTarget *big.Int
+	// Special difficulty rule for Testnet4


Please use full sentences, including punctuation, for comments. Also, if a comment isn't at the beginning of a block, it's nice to have an empty line before it to have some vertical breathing room.
Finally, else is kind of discouraged in Go if it can be written in a different way, as it often leads to less readable code.
So I'd write this the following way:

// Special difficulty rule for Testnet4 oldTarget := CompactToBig(lastNode.Bits()) if c.ChainParams().EnforceBIP94 { // Here we use the first block of the difficulty period. This way // the real difficulty is always preserved in the first block as // it is not allowed to use the min-difficulty exception. oldTarget = CompactToBig(firstNode.Bits()) }

Of course the assumption here is that CompactToBig(lastNode.Bits()) isn't computationally expensive to wanting to avoid calling it twice in the worst case.

guggero · 2025-01-16T12:10:04Z

blockchain/error.go

@@ -220,6 +220,8 @@ const (
 	// current chain tip. This is not a block validation rule, but is required
 	// for block proposals submitted via getblocktemplate RPC.
 	ErrPrevBlockNotBest
+
+	ErrTimewarpAttack


nit: missing Godoc comment.

guggero · 2025-01-16T12:11:23Z

blockchain/validate.go

@@ -85,7 +90,7 @@ func ShouldHaveSerializedBlockHeight(header *wire.BlockHeader) bool {

 // IsCoinBaseTx determines whether or not a transaction is a coinbase.  A coinbase
 // is a special transaction created by miners that has no inputs.  This is
-// represented in the block chain by a transaction with a single input that has
+// represented in the blockchain by a transaction with a single input that has


Please revert these. This specifically refer to the chain of blocks, not the package name. IMO "Blockchain" is a marketing term used by people trying to sell you something that isn't Bitcoin.

guggero · 2025-01-16T12:15:36Z

blockchain/validate.go

+		if c.ChainParams().EnforceBIP94 {
+			// Check timestamp for the first block of each difficulty adjustment
+			// interval, except the genesis block.
+			if blockHeight%c.BlocksPerRetarget() == 0 {
+				prevBlockTimestamp := time.Unix(prevNode.Timestamp(), 0)
+				if header.Timestamp.Before(prevBlockTimestamp.Add(-maxTimeWarp)) {
+					str := "block's timestamp %v is too early on diff adjustment block %v"
+					str = fmt.Sprintf(str, header.Timestamp, prevBlockTimestamp)
+					return ruleError(ErrTimewarpAttack, str)
+				}
+			}
+		}
+	}


nit: line length. Also, sounds like this whole block could be extracted into a standalone function. Something like checkBip94().

guggero · 2025-01-16T12:15:59Z

btcd.go

@@ -64,6 +64,11 @@ func btcdMain(serverChan chan<- *server) error {
 	// Show version at startup.
 	btcdLog.Infof("Version %s", version())

+	if cfg.TestNet3 {


Not sure I agree with that whole block.

guggero · 2025-01-16T12:20:46Z

cmd/addblock/config.go

@@ -79,6 +80,7 @@ func netName(chainParams *chaincfg.Params) string {
 	switch chainParams.Net {
 	case wire.TestNet3:
 		return "testnet"
+	// TODO do we need to use "testnet" dir for testnet4 as well?


We should use a distinct directory, otherwise you couldn't run both on the same computer.

guggero · 2025-01-16T12:20:55Z

cmd/btcctl/config.go

@@ -106,7 +106,8 @@ type config struct {
 	RPCUser        string `short:"u" long:"rpcuser" description:"RPC username"`
 	SimNet         bool   `long:"simnet" description:"Connect to the simulation test network"`
 	TLSSkipVerify  bool   `long:"skipverify" description:"Do not verify tls certificates (not recommended!)"`
-	TestNet3       bool   `long:"testnet" description:"Connect to testnet"`
+	TestNet3       bool   `long:"testnet" description:"Connect to testnet (version 3). Support for testnet3 is deprecated. Consider moving to testnet4 now by using -testnet4"`


Remove deprecation comment.

guggero · 2025-01-16T12:21:09Z

cmd/findcheckpoint/config.go

@@ -41,7 +41,8 @@ type config struct {
 	NumCandidates  int    `short:"n" long:"numcandidates" description:"Max num of checkpoint candidates to show {1-20}"`
 	RegressionTest bool   `long:"regtest" description:"Use the regression test network"`
 	SimNet         bool   `long:"simnet" description:"Use the simulation test network"`
-	TestNet3       bool   `long:"testnet" description:"Use the test network"`
+	TestNet3       bool   `long:"testnet" description:"Use the test network (version 3). Support for testnet3 is deprecated. Consider moving to testnet4 now by using -testnet4"`


Same here re deprecation.

guggero · 2025-01-16T12:21:26Z

config.go

@@ -169,7 +169,8 @@ type config struct {
 	SigNet               bool          `long:"signet" description:"Use the signet test network"`
 	SigNetChallenge      string        `long:"signetchallenge" description:"Connect to a custom signet network defined by this challenge instead of using the global default signet test network -- Can be specified multiple times"`
 	SigNetSeedNode       []string      `long:"signetseednode" description:"Specify a seed node for the signet network instead of using the global default signet network seed nodes"`
-	TestNet3             bool          `long:"testnet" description:"Use the test network"`
+	TestNet3             bool          `long:"testnet" description:"Use the test network (version 3). Support for testnet3 is deprecated. Consider moving to testnet4 now by using -testnet4"`


guggero · 2025-01-16T12:21:56Z

peer/peer.go

@@ -2377,6 +2377,7 @@ func newPeerBase(origCfg *Config, inbound bool) *Peer {

 	// Set the chain parameters to testnet if the caller did not specify any.
 	if cfg.ChainParams == nil {
+		// TODO: change to testnet4 because testnet3 will be deprecated soon


Roasbeef mentioned this pull request Nov 13, 2024

[feature]: testnet4 support lightningnetwork/lnd#8966

Open

bullet-tooth marked this pull request as draft November 23, 2024 00:09

bullet-tooth marked this pull request as ready for review November 23, 2024 14:59

multi: add testnet4 support

730a170

Adds support of the Bitcoin testnet version 4 chain. Reference: https://bips.dev/94/

bullet-tooth force-pushed the feat/add-testnet4-support branch from c71544f to 730a170 Compare December 13, 2024 15:08

guggero self-requested a review January 14, 2025 15:51

guggero requested changes Jan 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add testnet4 chain support #2274

feat: Add testnet4 chain support #2274

bullet-tooth commented Nov 10, 2024

Roasbeef commented Nov 11, 2024

jcvernaleo commented Nov 11, 2024

bullet-tooth commented Nov 20, 2024

Roasbeef commented Nov 20, 2024

coveralls commented Nov 25, 2024 •

edited

Loading

bullet-tooth commented Dec 10, 2024

guggero commented Dec 10, 2024

bullet-tooth commented Jan 14, 2025

guggero left a comment

guggero Jan 16, 2025

guggero Jan 16, 2025

guggero Jan 16, 2025

guggero Jan 16, 2025

guggero Jan 16, 2025

guggero Jan 16, 2025

guggero Jan 16, 2025

guggero Jan 16, 2025

guggero Jan 16, 2025

guggero Jan 16, 2025

feat: Add testnet4 chain support #2274

Are you sure you want to change the base?

feat: Add testnet4 chain support #2274

Conversation

bullet-tooth commented Nov 10, 2024

Roasbeef commented Nov 11, 2024

jcvernaleo commented Nov 11, 2024

bullet-tooth commented Nov 20, 2024

Roasbeef commented Nov 20, 2024

coveralls commented Nov 25, 2024 • edited Loading

Pull Request Test Coverage Report for Build 12318119030

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

bullet-tooth commented Dec 10, 2024

guggero commented Dec 10, 2024

bullet-tooth commented Jan 14, 2025

guggero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 25, 2024 •

edited

Loading