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

[BUG] Bitcoin addresses generated are not valid (incorrect checksum) #130

Closed
Emyrk opened this issue Jan 26, 2023 · 3 comments
Closed

[BUG] Bitcoin addresses generated are not valid (incorrect checksum) #130

Emyrk opened this issue Jan 26, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@Emyrk
Copy link

Emyrk commented Jan 26, 2023

Describe the bug

Bitcoin keys use base58 check (see https://en.bitcoin.it/wiki/Base58Check_encoding, for example: https://gobittest.appspot.com/Address). Addresses generated by this package are not valid Bitcoin addresses since their checksums are invalid.

The generator just generates random base58 text. I implemented a function that can fix checksums of a randomly generated address, it just requires base58 encode/decode. You can copy this small implementation into the Faker lib to not need to go get it: https://github.com/akamensky/base58/blob/master/base58.go. Or implement it yourself.

If base58 is in this package, I can make a PR that runs all bitcoin generated addressed through a checksum fix function and make sure they are valid.

To Reproduce

https://go.dev/play/p/No5BYCcsUk4

package main

import (
	"crypto/sha256"
	"fmt"

	"github.com/btcsuite/btcutil/base58"
	"github.com/jaswdr/faker"
)

func main() {
	fake := faker.New()
	address := fake.Crypto().BitcoinAddress()
	fmt.Println("Before:", address)
	_, _, err := base58.CheckDecode(address)
	if err != nil {
		fmt.Println("Failed:", err)
	} else {
		fmt.Println("Success!")
	}

	// Fix the address
	address = FixChecksum(address)
	fmt.Println("After:", address)
	_, _, err = base58.CheckDecode(address)
	if err != nil {
		fmt.Println("Failed:", err)
	} else {
		fmt.Println("Success!")
	}
}

func FixChecksum(addr string) string {
	decoded := base58.Decode(addr)

	// Remove the last 4 bytes
	decoded = decoded[:len(decoded)-4]

	// Find the checksum
	checksum := sha256.Sum256(decoded)
	checksum = sha256.Sum256(checksum[:])

	// Add back the 4 byte checksum
	decoded = append(decoded, checksum[:4]...)
	return base58.Encode(decoded)
}

Expected behavior

The address should pass the CheckDecode

@Emyrk Emyrk added the bug Something isn't working label Jan 26, 2023
@jaswdr
Copy link
Owner

jaswdr commented Jan 27, 2023

thank you for the report @Emyrk, I'll apply this fix as soon as possible and release a new version.

@jaswdr jaswdr self-assigned this Jan 27, 2023
@Emyrk
Copy link
Author

Emyrk commented Jan 27, 2023

Thanks! I was going to open a PR, but the requirement of base58 is annoying since golang does not support it natively.

With your no deps in go.mod, it has to be implemented here 😢

@jaswdr jaswdr removed their assignment Oct 6, 2023
@jaswdr
Copy link
Owner

jaswdr commented Oct 9, 2023

Fixed in #153 by simplifying the way bitcoin and etherium addresses are generated.

@jaswdr jaswdr closed this as completed Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants