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

fix invalid MinerPayouts in testutil.MineBlock #46

Merged
merged 3 commits into from Apr 17, 2024

Conversation

peterjan
Copy link
Member

Silly little bug in our MineBlock implementation where we call PoolTransactions twice and thus introduce a race. This eventually results in an error upon validating the block that indicates the miner payout sum doesn't match block reward + fees.

@peterjan peterjan self-assigned this Apr 17, 2024
@peterjan peterjan marked this pull request as ready for review April 17, 2024 10:43
Copy link
Member

@lukechampine lukechampine left a comment

Choose a reason for hiding this comment

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

we should support V2 transactions while we're at it.

Also, I spot another race: the state when you call cm.PoolTransactions might not match the state when you called cm.TipState! That's annoying. We can squash it with a loop:

retry:
    cs := cm.TipState()
    txns := cm.PoolTransactions()
    txnsv2 := cm.PoolTransactionsV2()
    if cs.Index != cm.Tip() {
        goto retry
    }

But it might make more sense to add a helper function to chain.Manager that returns a state and the pool transactions together.

Alternatively, maybe this isn't a big deal? Miners already have to assume that a new block might arrive while they're mining. So they need to regularly check the tip and restart anyway.

@peterjan
Copy link
Member Author

Hm if we do all that we might as well drop this outdated impl. all together in favour of the one in coreutils package?

@lukechampine lukechampine merged commit a3dce82 into master Apr 17, 2024
8 checks passed
@lukechampine lukechampine deleted the pj/fix-miner-fee-race branch April 17, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants