Page MenuHomePhabricator
Differential D1591

Remove blockFinished from BlockAssembler
ClosedPublic
Actions

Authored by schancel on Jul 25 2018, 01:11.

Details

Reviewers
jasonbcox
deadalnix
Group Reviewers
Restricted Project
Commits
rSTAGING9c6f4b94b4a1: Remove blockFinished from BlockAssembler
rABC9c6f4b94b4a1: Remove blockFinished from BlockAssembler
Summary

As per title, stop using spooky action at a distance

Test Plan
make check && ./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
more-miner-nits
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2941
Build 3982: Bitcoin ABC Buildbot (legacy)
Build 3981: arc lint + arc unit

Event Timeline

schancel created this revision.Jul 25 2018, 01:11
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 25 2018, 01:11
teamcity added a subscriber: teamcity.Jul 25 2018, 01:13

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

schancel added a parent revision: D1590: Couple of code nits in miner.cpp.Jul 25 2018, 01:13
schancel updated this revision to Diff 4389.Jul 25 2018, 01:14

Fix accidentally removed code during rebase

teamcity added a comment.Jul 25 2018, 01:16

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Jul 25 2018, 01:17

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B2929: Diff 4388!Jul 25 2018, 01:17
teamcity added a comment.Jul 25 2018, 01:20

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B2930: Diff 4389!Jul 25 2018, 01:20
schancel updated this revision to Diff 4390.Jul 25 2018, 01:43

Remove a missed reference

teamcity added a comment.Jul 25 2018, 01:43

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

jasonbcox accepted this revision.Jul 25 2018, 02:03
This revision is now accepted and ready to land.Jul 25 2018, 02:03
schancel updated this revision to Diff 4392.Jul 25 2018, 02:08

Remove wild semicolon

teamcity added a comment.Jul 25 2018, 02:10

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

schancel updated this revision to Diff 4393.Jul 25 2018, 02:18

Reword TestForBlockResult parameter to make it clearer

teamcity added a comment.Jul 25 2018, 02:18

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Jul 25 2018, 02:20

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B2931: Diff 4390!Jul 25 2018, 02:20
teamcity added a comment.Jul 25 2018, 02:50

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B2932: Diff 4392!Jul 25 2018, 02:50
teamcity added a comment.Jul 25 2018, 02:57

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B2933: Diff 4393!Jul 25 2018, 02:57
deadalnix requested changes to this revision.Jul 25 2018, 14:30
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/miner.cpp
302 ↗(On Diff #4393)

You should move this in the header.

643 ↗(On Diff #4393)

This clearly changes the semantic. You tested for a specific package to be added, but then never adds it. Also, if blockFinished is true, the thing continue to loop until iterator runs out.

This revision now requires changes to proceed.Jul 25 2018, 14:30
schancel updated this revision to Diff 4401.Jul 25 2018, 14:58

Fix nits

teamcity added a comment.Jul 25 2018, 15:00

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

schancel updated this revision to Diff 4402.Jul 25 2018, 15:13

Address some more feedback

teamcity added a comment.Jul 25 2018, 15:15

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

deadalnix accepted this revision.Jul 25 2018, 15:21
deadalnix added inline comments.
src/miner.h
128

You can probably declare this as a private local class in BlockAssembler, as the only function using it is private anyway.

130

TXCantFit ?

This revision is now accepted and ready to land.Jul 25 2018, 15:21
schancel updated this revision to Diff 4403.Jul 25 2018, 15:24

Fix a typo

teamcity added a comment.Jul 25 2018, 15:24

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Jul 25 2018, 15:28

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B2942: Diff 4403!Jul 25 2018, 15:28
schancel updated this revision to Diff 4404.Jul 25 2018, 15:41

Fix a few nits

teamcity added a comment.Jul 25 2018, 15:41

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Jul 25 2018, 15:41

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B2940: Diff 4401.Jul 25 2018, 15:41
Closed by commit rABC9c6f4b94b4a1: Remove blockFinished from BlockAssembler (authored by schancel). · Explain WhyJul 25 2018, 15:53
This revision was automatically updated to reflect the committed changes.
Harbormaster completed remote builds in B2941: Diff 4402.Jul 25 2018, 15:54
teamcity added a comment.Jul 25 2018, 15:54

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

teamcity added a comment.Jul 25 2018, 16:23

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B2943: Diff 4404!Jul 25 2018, 16:23

Revision Contents
Changeset List

PathSize
src/
10 lines
29 lines

Diff 4402

View Options

src/miner.h

Loading...
View Options

src/miner.cpp

Loading...