Provide fee data for all txs in RPC getblocktemplate response #2115

pull forrestv wants to merge 3 commits into bitcoin:master from forrestv:getblocktemplate_allfees changing 4 files +70 −55
  1. forrestv commented at 8:33 am on December 19, 2012: contributor
    Having the fee data for every transaction returned by “getblocktemplate” was broken by the ultraprune commit (450cbb09). It made it so transactions that depend on other transactions in the block-to-be don’t have fee data.
  2. BitcoinPullTester commented at 9:00 am on December 19, 2012: none
    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/d42d932bed6b564cbfc00216d1c04cd5248ddeef for binaries and test log.
  3. in src/rpcmining.cpp: in d42d932bed outdated
    306@@ -307,14 +307,15 @@ Value getblocktemplate(const Array& params, bool fHelp)
    307         entry.push_back(Pair("depends", deps));
    308 
    309         int64_t nSigOps = tx.GetLegacySigOpCount();
    310-        if (tx.HaveInputs(view))
    311-        {
    312-            entry.push_back(Pair("fee", (int64_t)(tx.GetValueIn(view) - tx.GetValueOut())));
    313-            nSigOps += tx.GetP2SHSigOpCount(view);
    314-        }
    315+        assert(tx.HaveInputs(view));
    


    luke-jr commented at 9:45 am on December 19, 2012:
    I dislike the possibility of bitcoind just aborting. Is there some reason not to make this an if() block as a failsafe?
  4. sipa commented at 4:07 pm on December 19, 2012: member

    I think it would be better to have CreateNewBlock construct some meta-data object with fees in it, which can be used by getblocktemplate.

    EDIT: this definitely needs fixing of course, but it seems silly to have GBT redo the fee calculation in an incomplete way, when CreateNewBlock perfectly knows all fees.

  5. luke-jr commented at 8:16 pm on December 19, 2012: member
    No reason it can’t be optimized after merging this fix, IMO.
  6. forrestv commented at 8:17 pm on December 19, 2012: contributor
    /me is working on having CreateNewBlock return fee data in addition
  7. BitcoinPullTester commented at 9:02 pm on December 19, 2012: none

    Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/3a291f97f0dc1c98f980a3f5679000339416961f for binaries and test log.

    This could happen for one of several reasons:

    1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts
    2. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
    3. The test suite fails on either Linux i386 or Win32
    4. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)
  8. changed CreateNewBlock to return a CBlockTemplate object, which includes per-tx fee and sigop count data 03cac0bb8e
  9. use fee/sigop data in BlockTemplate struct instead of (not always correctly) calculating it ourselves 0f927ceb5b
  10. forrestv commented at 9:15 pm on December 19, 2012: contributor
    Okay, CreateNewBlock now returns a CBlockTemplate struct that contains the block and fee and sigop data.
  11. BitcoinPullTester commented at 9:30 pm on December 19, 2012: none
    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/0f927ceb5b90ec02be36ddb20b2f4cff82589265 for binaries and test log.
  12. sipa commented at 1:52 am on December 20, 2012: member
    Code looks good to me. I’ll do some testing soon.
  13. sipa commented at 3:20 am on January 4, 2013: member
    ACK
  14. gmaxwell commented at 3:24 am on January 4, 2013: contributor
    Been running this for almost two weeks on a node. Did basic sanity checks. ACK.
  15. jgarzik commented at 3:59 am on January 4, 2013: contributor
    ACK
  16. moved "index_in_template" to a separate variable to clarify what it is f3d872d1ea
  17. BitcoinPullTester commented at 5:27 am on January 4, 2013: none
    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f3d872d1eabeb5c999162f709626ee20c8789c42 for binaries and test log.
  18. sipa referenced this in commit 45a1ec51b1 on Jan 10, 2013
  19. sipa merged this on Jan 10, 2013
  20. sipa closed this on Jan 10, 2013

  21. laudney referenced this in commit d364506758 on Mar 19, 2014
  22. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-02-22 15:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me