netsync : compare header hash at assume utreexo height instead of chain tip by 4xvgal · Pull Request #418 · utreexo/utreexod

4xvgal · 2026-06-20T13:00:08Z

BestHeader() returns the tip hash of header chain, which is can be at a height pass the assume utreexo point when headers are downloaded in a single batch.
this causes a false mismatch and os.Exit(1).

BestHeader() = chain tip = 954,553
00000000000000000001c86c3e8ddbffa6a572724f66fe9df46ba84f51fe70ef

hard coded = 943,013
00000000000000000001c595730bd4a5fb0e2b35af70882962ce7ae602f48aff

4xvgal · 2026-06-20T16:23:46Z

026-06-20 20:48:45.715 [INF] SYNC: Downloading headers from 1 to 954553 from peer 1.228.21.110:8333
2026-06-20 20:48:45.754 [WRN] SYNC: The node had hash 00000000000000000001c595730bd4a5fb0e2b35af70882962ce7ae602f48aff
 hardcoded in but the valid proof-of-work chain has the hash
 00000000000000000001c86c3e8ddbffa6a572724f66fe9df46ba84f51fe70ef at height 954553.
 The user should not trust this software as genuine and there may be attempts to steal funds.
 The user should delete the datadir

4xvgal · 2026-06-20T16:42:50Z

stopHas is fine. This just makes the check more forgiving.
it compares the hash at the exact assume height, so it's ok if peer sends extra headers.

kcalvinalvin · 2026-06-25T08:04:48Z

 		assumeUtreexoHash := sm.chain.AssumeUtreexoHash()
-		if !bestHash.IsEqual(&assumeUtreexoHash) {
+		headerHash, err := sm.chain.HeaderHashByHeight(assumeHeight)
+		if err != nil || !headerHash.IsEqual(&assumeUtreexoHash) {


Instead of handling this err here and making editing hashStr, can we just fail if we're unable to fetch the hash for the height?

Done. split into two separate failure cases.

kcalvinalvin · 2026-06-25T08:16:50Z

Good catch! I do wanna add a unit test so that we never have the same bug again. If you'd like to work on that that'd be cool. Otherwise I'll just do it in a follow up!

Ah also, per Go standards, the git commit should be:

netsync: compare header hash at assume utreexo height instead of chain tip

4xvgal · 2026-06-26T01:35:16Z

Good catch! I do wanna add a unit test so that we never have the same bug again. If you'd like to work on that that'd be cool. Otherwise I'll just do it in a follow up!

Ah also, per Go standards, the git commit should be:
netsync: compare header hash at assume utreexo height instead of chain tip

Sounds gooood. I'm planning to edit commit msg and add three cases:

bestHeight > assumeHeight - peer sends headers past the assume height in one batch the hash at exaclty assumeHeight matches -> should pass
hash mismatch at assumeHeight - hardcoded hash doesn't match the actual chain -> should return an error
fetch error -> HeaderHashByHeight returns an error (e.g. assumeHeight not yet in the heawder index)

Does that cover what you had in mind, or is there specific scenario you'd want to test? happy to add more if need.

kcalvinalvin · 2026-06-26T06:54:46Z

Good catch! I do wanna add a unit test so that we never have the same bug again. If you'd like to work on that that'd be cool. Otherwise I'll just do it in a follow up!
Ah also, per Go standards, the git commit should be:
netsync: compare header hash at assume utreexo height instead of chain tip
Sounds gooood. I'm planning to edit commit msg and add three cases:

bestHeight > assumeHeight - peer sends headers past the assume height in one batch the hash at exaclty assumeHeight matches -> should pass

hash mismatch at assumeHeight - hardcoded hash doesn't match the actual chain -> should return an error

fetch error -> HeaderHashByHeight returns an error (e.g. assumeHeight not yet in the heawder index)

Does that cover what you had in mind, or is there specific scenario you'd want to test? happy to add more if need.

Honestly I'd need to see the code heh. Ping me when you're ready!

4xvgal · 2026-06-27T17:00:16Z

Done. Split the check into a checkAssumeUtreexoHash() error helper so the failure paths are unit testable. commit updated, ready for review!

kcalvinalvin · 2026-06-28T05:03:47Z

 	}
 }

+func (sm *SyncManager) checkUtreexoHash() error {


Should say checkAssumeUtreexoHash(). Also we should add a comment on what the function is doing.

kcalvinalvin · 2026-06-28T05:06:05Z

 		"mempool should have removed the double-spending tx on block connect")
 }
+
+func TestCheckUtreexoHash(t *testing.T) {


The test should be table-driven.

kcalvinalvin · 2026-06-28T05:07:35Z

-				"software as genuine and there may be attempts to steal funds. The user "+
-				"should delete the datadir", sm.chain.AssumeUtreexoHash().String(),
-				bestHash.String(), bestHeight)
+		if err := sm.checkUtreexoHash(); err != nil {


We're not logging the error here so or in the checkUtreexoHash() so the user wouldn't know why the binary exited.

kcalvinalvin · 2026-06-28T05:09:24Z

+	headerHash, err := sm.chain.HeaderHashByHeight(assumeHeight)
+
+	if err != nil {
+		return fmt.Errorf("Failed to get header hash at assume utreexo height %d: %v", assumeHeight, err)


I don't love the messaging here. If the HeaderHashByHeight() couldn't fetch the hash at the assumeUtreexoHeight, it means something is really wrong here.

This information would be useful for the end user

4xvgal · 2026-06-28T09:48:09Z

All addressed. function renamed + documented, test is table-driven, logging moved to caller. PR updated. Let me know if there's anything else.

kcalvinalvin · 2026-06-29T04:35:37Z

-				"software as genuine and there may be attempts to steal funds. The user "+
-				"should delete the datadir", sm.chain.AssumeUtreexoHash().String(),
-				bestHash.String(), bestHeight)
+		if err := sm.checkAssumeUtreexoHash(); err != nil {


We left out the "why" here. Yes the user shouldn't trust the software but why? It's because the hash was hardcoded but the hardcoded hash didn't match the downloaded best PoW chain.

From the user's perspective, they'll either get:

The user should not trust this software as genuine and there may be attempts to steal funds. The user should delete the datadir: hash mismatch at height ..."

or

The user should not trust this software as genuine and there may be attempts to steal funds. The user should delete the datadir: hash not found at height ..."

The lack of information here would leave the user confused. In the current code, we explicitly state the reason. We should be doing the same here

kcalvinalvin · 2026-06-29T04:42:52Z

+	assumeHeight := sm.chain.AssumeUtreexoHeight()
+	assumeUtreexoHash := sm.chain.AssumeUtreexoHash()
+	headerHash, err := sm.chain.HeaderHashByHeight(assumeHeight)
+


Should not have this empty line here. The error assertion branch should be right after the err definition

kcalvinalvin · 2026-06-29T04:49:50Z

+			wantErr:   true,
+		},
+		{
+			name:      "fetch error: assumeHeight not in bestHeader",


The returned error here is the same as "mismatch: hash at assumepointhHeight differs".

[I] calvin@nixos ~/b/u/u/netsync ((30b29907))> git diff diff --git a/netsync/manager_test.go b/netsync/manager_test.go index 5365939a..c2fd8f2d 100644 --- a/netsync/manager_test.go +++ b/netsync/manager_test.go @@ -1349,6 +1349,7 @@ func TestCheckUtreexoHash(t *testing.T) { } //assert err := sm.checkAssumeUtreexoHash() + fmt.Println(err) if tc.wantErr { require.Error(t, err) } else {

[I] calvin@nixos ~/b/u/u/netsync ((30b29907))> go test -run 'TestCheckUtreexoHash/fetch_error:_assumeHeight_not_in_bestHeader' hash mismatch at height 2: expected 0000000000000000000000000000000000000000000000000000000000000000, got 04abc77b0fceb55927320b84ae78b3de4f402f85d4a326ec236f5c17f19fe27e PASS ok github.com/utreexo/utreexod/netsync 0.048s [I] calvin@nixos ~/b/u/u/netsync ((30b29907))>

Thanks for catch. when i split the check into checkAssumeUtreexoHash(), original warn lost explicit "hardcoded vs PoW chain" framing.

I'll fix hat so the user gets the full context again.

update the helper error message to explicitly mention the hardcoded assumeUtreexoHash, actual chain hash, and the height.

update the caller warning

remove empty line

fix the "fetch error" test case

4xvgal · 2026-07-07T13:32:27Z

The fetch error case's original purposewas to verify the error path when HeaderHashByHeight can't find the header at the assumepoint Height, but it was actually testing a hash mismatch. I fixed it by using numBlocks:1 and assumepointHeight: 10.

Thanks for catching those my mistakes.

4xvgal · 2026-09-16T08:31:56Z

+			numBlocks:         1,
+			assumepointHeight: 10,
+			hashFunc:          func(b []*btcutil.Block) *chainhash.Hash { return &chainhash.Hash{} },
+			wantErr:           true,


@kcalvinalvin
Would be better to identify each error path withe custom type (like RuleError )or use sentinel + error.Is?

I wanted to ask before edit

or it could be only in Test code, match err string using ErrContains

oh Custom type and Sentinel both are over engineering and until Err is useful for other boundary or interface, i'll choose ErrContains

…n tip

4xvgal marked this pull request as ready for review June 20, 2026 16:32

kcalvinalvin reviewed Jun 25, 2026

View reviewed changes

4xvgal force-pushed the fix/assumeutreexo-header-hash-check branch from 384e7d6 to b2c34a1 Compare June 27, 2026 16:57

4xvgal force-pushed the fix/assumeutreexo-header-hash-check branch from b2c34a1 to c4eff4b Compare June 27, 2026 17:12

kcalvinalvin reviewed Jun 28, 2026

View reviewed changes

4xvgal force-pushed the fix/assumeutreexo-header-hash-check branch from c4eff4b to 30b2990 Compare June 28, 2026 09:46

4xvgal changed the title ~~fix: compare header hash at assume utreexo height instead of chain tip~~ netsync : compare header hash at assume utreexo height instead of chain tip Jun 29, 2026

kcalvinalvin reviewed Jun 29, 2026

View reviewed changes

4xvgal force-pushed the fix/assumeutreexo-header-hash-check branch from 30b2990 to 01a1a32 Compare July 7, 2026 13:30

4xvgal force-pushed the fix/assumeutreexo-header-hash-check branch from 01a1a32 to 3f33b09 Compare September 16, 2026 08:36

4xvgal commented Sep 16, 2026

View reviewed changes

netsync: compare header hash at assume utreexo height instead of chai…

e2e33c3

…n tip

4xvgal force-pushed the fix/assumeutreexo-header-hash-check branch from 3f33b09 to e2e33c3 Compare September 17, 2026 13:07

Conversation

4xvgal commented Jun 20, 2026

Uh oh!

4xvgal commented Jun 20, 2026

Uh oh!

4xvgal commented Jun 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kcalvinalvin commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

4xvgal commented Jun 26, 2026

Uh oh!

kcalvinalvin commented Jun 26, 2026

Uh oh!

4xvgal commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

4xvgal commented Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

4xvgal commented Jul 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

4xvgal Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kcalvinalvin commented Jun 25, 2026 •

edited

Loading

4xvgal commented Jun 27, 2026 •

edited

Loading

4xvgal commented Jun 28, 2026 •

edited

Loading

4xvgal Sep 16, 2026 •

edited

Loading