Skip to content

chaincfg/chainhash: add strict parsing #2520

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

Open
starius wants to merge 1 commit into
base: master
Choose a base branch
from
+196 −2
Open

chaincfg/chainhash: add strict parsing #2520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 44 additions & 1 deletion chaincfg/chainhash/hash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ var (
// string that has too many characters.
var ErrHashStrSize = fmt.Errorf("max hash string length is %v bytes", MaxHashStringSize)

// ErrHashStrSizeMismatch describes an error that indicates the caller
// specified a hash string that does not meet the exact length required for
// strict parsing.
var ErrHashStrSizeMismatch = fmt.Errorf("hash string must be exactly %d "+
"characters", MaxHashStringSize)

// Hash is used in several of the bitcoin messages and common structures. It
// typically represents the double sha256 of data.
type Hash [HashSize]byte
Expand Down Expand Up @@ -180,6 +186,10 @@ func TaggedHash(tag []byte, msgs ...[]byte) *Hash {
// NewHashFromStr creates a Hash from a hash string. The string should be
// the hexadecimal string of a byte-reversed hash, but any missing characters
// result in zero padding at the end of the Hash.
//
// NOTE: This function accepts short and odd-length hex strings and pads them.
// Typical parsing of full txids or block hashes should use NewHashFromStrStrict
// instead.
func NewHashFromStr(hash string) (*Hash, error) {
ret := new(Hash)
err := Decode(ret, hash)
Expand All @@ -189,8 +199,23 @@ func NewHashFromStr(hash string) (*Hash, error) {
return ret, nil
}

// NewHashFromStrStrict creates a Hash from a hash string. The string must be
// the full hexadecimal string of a byte-reversed hash.
func NewHashFromStrStrict(hash string) (*Hash, error) {
ret := new(Hash)
err := DecodeStrict(ret, hash)
if err != nil {
return nil, err
}
return ret, nil
}

// Decode decodes the byte-reversed hexadecimal string encoding of a Hash to a
// destination.
//
// NOTE: This function accepts short and odd-length hex strings and pads them.
// Typical parsing of full txids or block hashes should use DecodeStrict
// instead.
func Decode(dst *Hash, src string) error {
// Return error if hash string is too long.
if len(src) > MaxHashStringSize {
Expand All @@ -208,9 +233,27 @@ func Decode(dst *Hash, src string) error {
copy(srcBytes[1:], src)
}

return decodeHash(dst, srcBytes)
}

// DecodeStrict decodes the byte-reversed hexadecimal string encoding of a Hash
// to a destination. The source string must be exactly MaxHashStringSize
// bytes, or ErrHashStrSizeMismatch is returned.
func DecodeStrict(dst *Hash, src string) error {
if len(src) != MaxHashStringSize {
return ErrHashStrSizeMismatch
}

return decodeHash(dst, []byte(src))
}

// decodeHash decodes the provided byte-reversed hexadecimal bytes into dst.
// The caller is responsible for applying any caller-specific length validation
// before invoking this helper.
func decodeHash(dst *Hash, src []byte) error {
Copy link
Copy Markdown

@utafrali utafrali Apr 8, 2026

Choose a reason for hiding this comment

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

The decodeHash helper accepts []byte so that Decode can pass its already-allocated srcBytes slice without an extra copy. However, DecodeStrict calls it with []byte(src), which allocates on every call. Since DecodeStrict only accepts exactly 64-char strings, you could declare a [MaxHashStringSize]byte on the stack and copy into it (or keep a local var b [MaxHashStringSize]byte; copy(b[:], src); return decodeHash(dst, b[:])) to avoid the heap allocation on the hot path.

Copy link
Copy Markdown
Contributor Author

@starius starius Apr 9, 2026

Choose a reason for hiding this comment

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

Go already optimizes such cases - no need to use a stack array for this.

I added a benchmark and ensured there is already no memory allocation here:

 go test -run '^$' -bench '^BenchmarkDecodeStrict$' -benchmem -count=5
goos: linux
goarch: amd64
pkg: github.com/btcsuite/btcd/chaincfg/chainhash
BenchmarkDecodeStrict-2         15530290                78.59 ns/op            0 B/op          0 allocs/op
BenchmarkDecodeStrict-2         14514126                78.90 ns/op            0 B/op          0 allocs/op
BenchmarkDecodeStrict-2         19157900                77.40 ns/op            0 B/op          0 allocs/op
BenchmarkDecodeStrict-2         14660510                78.14 ns/op            0 B/op          0 allocs/op
BenchmarkDecodeStrict-2         18853245                78.26 ns/op            0 B/op          0 allocs/op
PASS
ok      github.com/btcsuite/btcd/chaincfg/chainhash     8.575s

// Hex decode the source bytes to a temporary destination.
var reversedHash Hash
_, err := hex.Decode(reversedHash[HashSize-hex.DecodedLen(len(srcBytes)):], srcBytes)
_, err := hex.Decode(reversedHash[HashSize-hex.DecodedLen(len(src)):], src)
if err != nil {
return err
}
Expand Down
153 changes: 152 additions & 1 deletion chaincfg/chainhash/hash_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"encoding/hex"
"encoding/json"
"strings"
"testing"
)

Expand Down Expand Up @@ -110,7 +111,8 @@ func TestHashString(t *testing.T) {
}
}

// TestNewHashFromStr executes tests against the NewHashFromStr function.
// TestNewHashFromStr executes compatibility tests against the lenient
// NewHashFromStr function.
func TestNewHashFromStr(t *testing.T) {
tests := []struct {
in string
Expand Down Expand Up @@ -196,6 +198,155 @@ func TestNewHashFromStr(t *testing.T) {
}
}

// TestNewHashFromStrStrict executes tests against the NewHashFromStrStrict
// function.
func TestNewHashFromStrStrict(t *testing.T) {
tests := []struct {
name string
in string
want Hash
err error
}{
{
name: "genesis hash",
in: "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f",
want: mainNetGenesisHash,
err: nil,
},
{
name: "stripped leading zeros",
in: "19d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f",
want: Hash{},
err: ErrHashStrSizeMismatch,
},
{
name: "odd length hash",
in: "1",
want: Hash{},
err: ErrHashStrSizeMismatch,
},
{
name: "empty string",
in: "",
want: Hash{},
err: ErrHashStrSizeMismatch,
},
{
name: "hash string that is too long",
in: "01234567890123456789012345678901234567890123456789012345678912345",
want: Hash{},
err: ErrHashStrSizeMismatch,
},
{
name: "hash string that contains non-hex chars",
in: "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26g",
want: Hash{},
err: hex.InvalidByteError('g'),
},
}

unexpectedErrStr := "NewHashFromStrStrict #%d failed to detect expected error - got: %v want: %v"
unexpectedResultStr := "NewHashFromStrStrict #%d got: %v want: %v"
t.Logf("Running %d tests", len(tests))
for i, test := range tests {
test := test

t.Run(test.name, func(t *testing.T) {
result, err := NewHashFromStrStrict(test.in)
if err != test.err {
t.Errorf(unexpectedErrStr, i, err, test.err)
return
} else if err != nil {
return
}
if !test.want.IsEqual(result) {
t.Errorf(unexpectedResultStr, i, result, &test.want)
}
})
}
}

// TestDecodeStrict executes tests against the DecodeStrict function.
func TestDecodeStrict(t *testing.T) {
Copy link
Copy Markdown

@utafrali utafrali Apr 8, 2026

Choose a reason for hiding this comment

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

TestDecodeStrict covers odd-length, non-hex, and too-long inputs, but there's no test for a valid even-length hex string that is simply too short (e.g., a 32-character hex string). That's likely the most common real-world mistake this function is meant to catch (someone passing an abbreviated hash). Adding a case like {"deadbeef" + strings.Repeat("0", 24), ErrHashStrSizeMismatch} (a 32-char string) would make the intent clearer.

Copy link
Copy Markdown
Contributor Author

@starius starius Apr 9, 2026

Choose a reason for hiding this comment

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

Added "deadbeef" + strings.Repeat("0", 24) case

tests := []struct {
name string
in string
want Hash
err error
}{
{
name: "genesis hash",
in: "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f",
want: mainNetGenesisHash,
err: nil,
},
{
name: "odd length hash",
in: "1",
want: Hash{},
err: ErrHashStrSizeMismatch,
},
{
name: "even length hash that is too short, 32 chars",
in: "deadbeef" + strings.Repeat("0", 24),
want: Hash{},
err: ErrHashStrSizeMismatch,
},
{
name: "hash string that contains non-hex chars",
in: "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26g",
want: Hash{},
err: hex.InvalidByteError('g'),
},
{
name: "hash string that is too long",
in: "01234567890123456789012345678901234567890123456789012345678912345",
want: Hash{},
err: ErrHashStrSizeMismatch,
},
}

unexpectedErrStr := "DecodeStrict #%d failed to detect expected error - got: %v want: %v"
unexpectedResultStr := "DecodeStrict #%d got: %v want: %v"
t.Logf("Running %d tests", len(tests))
for i, test := range tests {
test := test

t.Run(test.name, func(t *testing.T) {
var result Hash
err := DecodeStrict(&result, test.in)
if err != test.err {
t.Errorf(unexpectedErrStr, i, err, test.err)
return
} else if err != nil {
return
}
if !test.want.IsEqual(&result) {
t.Errorf(unexpectedResultStr, i, &result, &test.want)
}
})
}
}

// BenchmarkDecodeStrict benchmarks DecodeStrict on a valid canonical hash.
func BenchmarkDecodeStrict(b *testing.B) {
// Use a canonical 64-character hash to exercise the successful strict path.
hashStr := "000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f"
var result Hash

b.ReportAllocs()
for i := 0; i < b.N; i++ {
if err := DecodeStrict(&result, hashStr); err != nil {
b.Fatalf("unexpected decode error: %v", err)
}
}

if !mainNetGenesisHash.IsEqual(&result) {
b.Fatalf("unexpected decode result: got %v want %v",
&result, &mainNetGenesisHash)
}
}

// TestHashJsonMarshal tests json marshal and unmarshal.
func TestHashJsonMarshal(t *testing.T) {
hashStr := "000000000003ba27aa200b1cecaad478d2b00432346c3f1f3986da1afd33e506"
Expand Down
Loading