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

feat: Create datagen testutil for common random generation functions #55

Merged
merged 2 commits into from
Jul 9, 2022

Conversation

vitsalis
Copy link
Member

@vitsalis vitsalis commented Jul 8, 2022

Might be useful in #47 since it uses similar functions with the ones used on the btclightclient module.

Comment on lines +23 to 24
func GenRandomBtcdHeader(version int32, bits uint32, nonce uint32,
timeInt int64, prevBlockStr string, merkleRootStr string) *wire.BlockHeader {
Copy link
Contributor

Choose a reason for hiding this comment

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

One useful function would be to create a header that extends a parent.
In that case you already have a parent hash, but you can also reuse the same version, and make sure the time is incremented by a random amount, but isn't less.

That can be used to generate a full block tree, recursively.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep that would be super useful and I intend to implement it when doing keeper tests. Let's implement it when it is needed.

// If the header hex is the same as the default one, then this is the seed input
headerBytes := bbl.NewBTCHeaderBytesFromBlockHeader(btcdHeader)
headerHex := headerBytes.MarshalHex()
seedInput := types.DefaultBaseHeaderHex == headerHex

// Make the address have a proper size
if len(addressBytes) == 0 || len(addressBytes) >= 256 {
addressBytes = genRandomByteArray(1 + uint64(rand.Intn(257)))
addressBytes = datagen.GenRandomByteArray(1 + uint64(rand.Intn(257)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the 1 +? It looks like this will go up to 257.

Copy link
Member Author

Choose a reason for hiding this comment

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

nice catch!

Copy link
Contributor

@aakoshh aakoshh left a comment

Choose a reason for hiding this comment

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

Looks good an useful, even though I don't fully understand the sizes in the tests.

@vitsalis vitsalis merged commit aa44136 into main Jul 9, 2022
@vitsalis vitsalis deleted the testutils-datagen branch July 9, 2022 09:27
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.

2 participants