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-related fixups for 0.13.0 #8295

Merged
merged 5 commits into from Jul 18, 2016

Conversation

Projects
None yet
5 participants
@sdaftuar
Member

sdaftuar commented Jun 30, 2016

My suggestion for how to address the first three items mentioned in #8294:

  • Add size-accounting to addPackageTxs(), and use that instead of addScoreTxs() even when -blockmaxsize is in use.
  • Bugfix: prevent addPackageTxs() from selecting witness transactions before segwit activation, even if mempool policy allows witness transactions in.
  • Remove addScoreTxs(), which is no longer used anywhere.
  • Remove -blockminsize command line option, which has no effect on the mining behavior anymore.

I can drop the last two commits if we decide to go a different direction.

sdaftuar added some commits Jun 27, 2016

@sdaftuar sdaftuar referenced this pull request Jun 30, 2016

Closed

Mining updates for 0.13.0 #8294

5 of 5 tasks complete

@MarcoFalke MarcoFalke added the Mining label Jun 30, 2016

@sdaftuar

This comment has been minimized.

Show comment
Hide comment
@sdaftuar

sdaftuar Jul 5, 2016

Member

I added a warning if -blockminsize is given on the command line, as we discussed on IRC last week, so this PR is now ready for review.

@laanwj I left the string for the warning untranslated because we're past the string freeze for translations for 0.13.0, but I wasn't sure if that was the right thing to do? Not sure what your preference is for how we display the warning.

Member

sdaftuar commented Jul 5, 2016

I added a warning if -blockminsize is given on the command line, as we discussed on IRC last week, so this PR is now ready for review.

@laanwj I left the string for the warning untranslated because we're past the string freeze for translations for 0.13.0, but I wasn't sure if that was the right thing to do? Not sure what your preference is for how we display the warning.

@jonasschnelli jonasschnelli added this to the 0.13.0 milestone Jul 7, 2016

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jul 11, 2016

Member

ACK c1d61fb, that was easier than I had anticipated (I thought we'd need to add size-based accounting to the mempool, but only a per-package check is needed).

Also nice to see AddScoreTx disappear. This also means that the existing mining tests now all test the only remaining algorithm.

Member

sipa commented Jul 11, 2016

ACK c1d61fb, that was easier than I had anticipated (I thought we'd need to add size-based accounting to the mempool, but only a per-package check is needed).

Also nice to see AddScoreTx disappear. This also means that the existing mining tests now all test the only remaining algorithm.

@@ -453,7 +453,6 @@ std::string HelpMessage(HelpMessageMode mode)
strUsage += HelpMessageGroup(_("Block creation options:"));
strUsage += HelpMessageOpt("-blockmaxcost=<n>", strprintf(_("Set maximum block cost (default: %d)"), DEFAULT_BLOCK_MAX_COST));

This comment has been minimized.

@MarcoFalke

MarcoFalke Jul 16, 2016

Member

Is this done in this pull?

ACTION: rename blockmaxcost to blockmaxweight (wumpus, 19:25:14)
http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-07-14-19.00.html

@MarcoFalke

MarcoFalke Jul 16, 2016

Member

Is this done in this pull?

ACTION: rename blockmaxcost to blockmaxweight (wumpus, 19:25:14)
http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-07-14-19.00.html

This comment has been minimized.

@laanwj
@laanwj

@laanwj laanwj merged commit c1d61fb into bitcoin:master Jul 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jul 18, 2016

Merge #8295: Mining-related fixups for 0.13.0
c1d61fb Add warning if -blockminsize is used. (Suhas Daftuar)
27362dd Remove -blockminsize option (Suhas Daftuar)
d2e46e1 Remove addScoreTxs() (Suhas Daftuar)
6dd4bc2 Exclude witness transactions in addPackageTxs() pre-segwit activation (Suhas Daftuar)
f15c2cd CreateNewBlock: add support for size-accounting to addPackageTxs (Suhas Daftuar)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge bitcoin#8295: Mining-related fixups for 0.13.0
c1d61fb Add warning if -blockminsize is used. (Suhas Daftuar)
27362dd Remove -blockminsize option (Suhas Daftuar)
d2e46e1 Remove addScoreTxs() (Suhas Daftuar)
6dd4bc2 Exclude witness transactions in addPackageTxs() pre-segwit activation (Suhas Daftuar)
f15c2cd CreateNewBlock: add support for size-accounting to addPackageTxs (Suhas Daftuar)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Remove SegWit related checks added by backporting of Bitcoin bitcoin#…
…8295

Also remove nBlockMinSize check from addPackageTxs. Removed by SegWit
related commits in Bitcoin.

codablock added a commit to codablock/dash that referenced this pull request Dec 27, 2017

Merge bitcoin#8295: Mining-related fixups for 0.13.0
c1d61fb Add warning if -blockminsize is used. (Suhas Daftuar)
27362dd Remove -blockminsize option (Suhas Daftuar)
d2e46e1 Remove addScoreTxs() (Suhas Daftuar)
6dd4bc2 Exclude witness transactions in addPackageTxs() pre-segwit activation (Suhas Daftuar)
f15c2cd CreateNewBlock: add support for size-accounting to addPackageTxs (Suhas Daftuar)

codablock added a commit to codablock/dash that referenced this pull request Dec 27, 2017

Remove SegWit related checks added by backporting of Bitcoin bitcoin#…
…8295

Also remove nBlockMinSize check from addPackageTxs. Removed by SegWit
related commits in Bitcoin.

codablock added a commit to codablock/dash that referenced this pull request Dec 28, 2017

Merge bitcoin#8295: Mining-related fixups for 0.13.0
c1d61fb Add warning if -blockminsize is used. (Suhas Daftuar)
27362dd Remove -blockminsize option (Suhas Daftuar)
d2e46e1 Remove addScoreTxs() (Suhas Daftuar)
6dd4bc2 Exclude witness transactions in addPackageTxs() pre-segwit activation (Suhas Daftuar)
f15c2cd CreateNewBlock: add support for size-accounting to addPackageTxs (Suhas Daftuar)

codablock added a commit to codablock/dash that referenced this pull request Dec 28, 2017

Remove SegWit related checks added by backporting of Bitcoin bitcoin#…
…8295

Also remove nBlockMinSize check from addPackageTxs. Removed by SegWit
related commits in Bitcoin.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment