Skip to content

[Port] Eliminate unnecessary call to CheckBlock#520

Closed
kyuupichan wants to merge 1 commit into
BitcoinUnlimited:devfrom
kyuupichan:main-7225
Closed

[Port] Eliminate unnecessary call to CheckBlock#520
kyuupichan wants to merge 1 commit into
BitcoinUnlimited:devfrom
kyuupichan:main-7225

Conversation

@kyuupichan

Copy link
Copy Markdown
Contributor

From bitcoin#7225:

ProcessNewBlock would return failure early if CheckBlock failed, before
calling AcceptBlock. AcceptBlock also calls CheckBlock, and upon failure
would update mapBlockIndex to indicate that a block was failed. By returning
early in ProcessNewBlock, we were not marking blocks that fail a check in
CheckBlock as permanently failed, and thus would continue to re-request and
reprocess them.

For the reasons stated in the comments to the core PR, I think we should review the purpose and usefulness of the CheckBlockHeader call we have at the top of ProcessNewBlock and perhaps move the SendExpeditedBlock call to a more appropriate place, I don't think the current position is optimal. But that's for a later PR once this is accepted.

@kyuupichan kyuupichan changed the title Eliminate unnecessary call to CheckBlock [Port] Eliminate unnecessary call to CheckBlock May 5, 2017
@ptschip

ptschip commented May 5, 2017

Copy link
Copy Markdown
Collaborator

@kyuupichan you need to rebase all your PR's on the new dev...there are new fixes to the tests that everyone needs.

@kyuupichan

Copy link
Copy Markdown
Contributor Author

Yeah shame there is no rerun Travis button

@sickpig

sickpig commented May 6, 2017

Copy link
Copy Markdown
Collaborator

@kyuupichan there's a "button" to restart travis build, you need write permission to use it thou. if you need to restart a build you have 3 way of doing it:

  • ping me, @gandrewstone or @solex asking for a restart

  • close the PR end reopen it again

  • git commit --amend and git push -f

@sickpig sickpig added the ported label May 6, 2017
ProcessNewBlock would return failure early if CheckBlock failed, before
calling AcceptBlock.  AcceptBlock also calls CheckBlock, and upon failure
would update mapBlockIndex to indicate that a block was failed.  By returning
early in ProcessNewBlock, we were not marking blocks that fail a check in
CheckBlock as permanently failed, and thus would continue to re-request and
reprocess them.
@kyuupichan

Copy link
Copy Markdown
Contributor Author

Closing this because of bitcoin/bitcoin#9765. This whole area needs a cleanup, which is now on my to-do.

@kyuupichan kyuupichan closed this May 17, 2017
@kyuupichan
kyuupichan deleted the main-7225 branch May 17, 2017 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants