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

mining: Remove unneeded disapproval check #2397

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

matheusd
Copy link
Member

@matheusd matheusd commented Oct 2, 2020

This removes a now unneeded check for unmineable transactions due to
block disapproval from the template generation code.

Previously this check was meant to ensure that the list of transactions
considered for inclusion in a block template did not include
transactions with an input referencing a regular transaction from a
parent disapproved block.

However this check is redudant due to check executed earlier in the
template building loop that asserts all inputs are valid utxos
according to the PoV of the parent block and taking into account whether
the new block being created will approve or disapprove it.

The check being removed had a bug due to the dcrutil.Tx being duplicated
without also copying the internal tree, which caused transactions to
later fail to be added to the template due to an unknown tree being
used.

Removing this corrects template generation for disapproved blocks. This can be verified by running the current code with --debuglevel=MINR=trace and configuring the wallet to disapprove some block. Notice there will be errors such as:

MINR: Error adding tx fc5371054e6f211bb7105ce30f60784c82089228b302865e6fb269d96893bf71 to block; invalid tree

This removes a now unneeded check for unmineable transactions due to
block disapproval from the template generation code.

Previously this check was meant to ensure that the list of transactions
considered for inclusion in a block template did not include
transactions with an input referencing a regular transaction from a
parent disapproved block.

However this check is redudant due to check executed earlier in the
template building loop that asserts all inputs are valid utxos
according to the PoV of the parent block and taking into account whether
the new block being created will approve or disapprove it.

The check being removed had a bug due to the dcrutil.Tx being duplicated
without also copying the internal tree, which caused transactions to
later fail to be added to the template due to an unknown tree being
used.
@davecgh davecgh added this to the 1.6.0 milestone Oct 3, 2020
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Nice observation. I confirmed this is indeed the case and tested that it works as expected with disapproved blocks.

@davecgh davecgh merged commit 492a4a9 into decred:master Oct 6, 2020
@matheusd matheusd deleted the cleanup-mining branch October 6, 2020 11:42
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