Merge pull request #1766 from evoskuil/master

Fix block.fees() (use cb.spend, not cb.value).

Author: evoskuil

include/bitcoin/system/chain/transaction.hpp

@@ -105,7 +105,7 @@ class BC_API transaction
     /// Computed properties.
     size_t weight() const NOEXCEPT;
     uint64_t fee() const NOEXCEPT;
-    uint64_t claim() const NOEXCEPT;
+    uint64_t spend() const NOEXCEPT;
     uint64_t value() const NOEXCEPT;
     bool is_coinbase() const NOEXCEPT;
     bool is_segregated() const NOEXCEPT;

src/chain/block.cpp

@@ -669,26 +669,25 @@ static uint64_t block_subsidy(size_t height, uint64_t subsidy_interval,
 
 // Prevouts required.
 
+// The fees() is the sum of all transaction fees (coinbase is zero).
 uint64_t block::fees() const NOEXCEPT
 {
-    if (txs_->empty())
-        return {};
-
     // Overflow returns max_uint64.
     const auto value = [](uint64_t total, const auto& tx) NOEXCEPT
     {
         return ceilinged_add(total, tx->fee());
     };
 
-    return std::accumulate(std::next(txs_->begin()), txs_->end(),
-        uint64_t{}, value);
+    return std::accumulate(txs_->begin(), txs_->end(), 0_u64, value);
 }
 
+// The claim() is the spend of the coinbase transaction.
 uint64_t block::claim() const NOEXCEPT
 {
-    return txs_->empty() ? zero : txs_->front()->value();
+    return txs_->empty() ? zero : txs_->front()->spend();
 }
 
+// The reward() is the sum of all transaction.fee() and the block subsidy.
 uint64_t block::reward(size_t height, uint64_t subsidy_interval,
     uint64_t initial_block_subsidy_satoshi, bool bip42) const NOEXCEPT
 {

src/chain/transaction.cpp

@@ -347,8 +347,8 @@ const outputs_cptr& transaction::outputs_ptr() const NOEXCEPT
 uint64_t transaction::fee() const NOEXCEPT
 {
     // Underflow returns zero (and is_overspent() will be true).
-    // This is value of prevouts spent by inputs minus that claimed by outputs.
-    return floored_subtract(value(), claim());
+    // The value of prevouts referenced by inputs minus that spent by outputs.
+    return floored_subtract(value(), spend());
 }
 
 // Methods.
@@ -650,36 +650,39 @@ bool transaction::is_missing_prevouts() const NOEXCEPT
     return std::any_of(inputs_->begin(), inputs_->end(), missing);
 }
 
-uint64_t transaction::claim() const NOEXCEPT
+// The value() is the sum of own inputs.
+uint64_t transaction::value() const NOEXCEPT
 {
-    // Overflow returns max_uint64.
-    const auto sum = [](uint64_t total, const auto& output) NOEXCEPT
+    // Overflow returns max_uint64. Not populated/coinbase return zero.
+    const auto sum = [](uint64_t total, const auto& input) NOEXCEPT
     {
-        return ceilinged_add(total, output->value());
+        const auto value = input->prevout ? input->prevout->value() : zero;
+        return ceilinged_add(total, value);
     };
 
-    // The amount claimed by outputs.
-    return std::accumulate(outputs_->begin(), outputs_->end(), 0_u64, sum);
+    // The amount referenced by inputs.
+    return std::accumulate(inputs_->begin(), inputs_->end(), 0_u64, sum);
 }
 
-uint64_t transaction::value() const NOEXCEPT
+// The spend() is the sum of own outputs.
+uint64_t transaction::spend() const NOEXCEPT
 {
-    // Overflow, not populated, and coinbase (default) return max_uint64.
-    const auto sum = [](uint64_t total, const auto& input) NOEXCEPT
+    // Overflow returns max_uint64.
+    const auto sum = [](uint64_t total, const auto& output) NOEXCEPT
     {
-        const auto value = input->prevout ? input->prevout->value() : max_uint64;
-        return ceilinged_add(total, value);
+        return ceilinged_add(total, output->value());
     };
 
-    // The amount of prevouts (referenced by inputs).
-    return std::accumulate(inputs_->begin(), inputs_->end(), 0_u64, sum);
+    // The amount spent by outputs.
+    return std::accumulate(outputs_->begin(), outputs_->end(), 0_u64, sum);
 }
 
+// Overspent is invalid (spend exceeds value), while underspent is the fee().
 bool transaction::is_overspent() const NOEXCEPT
 {
     BC_ASSERT(!is_coinbase());
 
-    return claim() > value();
+    return spend() > value();
 }
 
 constexpr bool is_non_coinbase_mature(size_t tx_height, size_t height) NOEXCEPT

test/chain/transaction.cpp

@@ -425,7 +425,7 @@ BOOST_AUTO_TEST_CASE(transaction__fee__empty__zero)
     BOOST_REQUIRE_EQUAL(instance.fee(), 0u);
 }
 
-BOOST_AUTO_TEST_CASE(transaction__fee__default_input__max_uint64)
+BOOST_AUTO_TEST_CASE(transaction__fee__default_input__zero)
 {
     const transaction instance
     {
@@ -435,8 +435,7 @@ BOOST_AUTO_TEST_CASE(transaction__fee__default_input__max_uint64)
         0
     };
 
-    // floored_subtract(max_uint64, 0)
-    BOOST_REQUIRE_EQUAL(instance.fee(), max_uint64);
+    BOOST_REQUIRE_EQUAL(instance.fee(), zero);
 }
 
 BOOST_AUTO_TEST_CASE(transaction__fee__default_output__zero)
@@ -449,17 +448,16 @@ BOOST_AUTO_TEST_CASE(transaction__fee__default_output__zero)
         0
     };
 
-    // floored_subtract(0, max_uint64)
     BOOST_REQUIRE_EQUAL(instance.fee(), 0u);
 }
 
-BOOST_AUTO_TEST_CASE(transaction__fee__nonempty__outputs_minus_inputs)
+BOOST_AUTO_TEST_CASE(transaction__fee__nonempty__prevouts_minus_outputs)
 {
     constexpr uint64_t value0 = 123;
     constexpr uint64_t value1 = 321;
-    constexpr uint64_t claim0 = 11;
-    constexpr uint64_t claim1 = 11;
-    constexpr uint64_t claim2 = 22;
+    constexpr uint64_t spend0 = 11;
+    constexpr uint64_t spend1 = 11;
+    constexpr uint64_t spend2 = 22;
 
     input input0;
     input input1;
@@ -471,41 +469,41 @@ BOOST_AUTO_TEST_CASE(transaction__fee__nonempty__outputs_minus_inputs)
         0,
         { input0, input1 },
         {
-            { claim0, script{} },
-            { claim1, script{} },
-            { claim2, script{} }
+            { spend0, script{} },
+            { spend1, script{} },
+            { spend2, script{} }
         },
         0
     };
 
-    // floored_subtract(values, claims)
-    BOOST_REQUIRE_EQUAL(instance.fee(), value0 + value1 - claim0 - claim1 - claim2);
+    // floored_subtract(values, spend)
+    BOOST_REQUIRE_EQUAL(instance.fee(), value0 + value1 - spend0 - spend1 - spend2);
 }
 
-BOOST_AUTO_TEST_CASE(transaction__claim__empty_outputs__zero)
+BOOST_AUTO_TEST_CASE(transaction__spend__empty_outputs__zero)
 {
     transaction instance;
-    BOOST_REQUIRE_EQUAL(instance.claim(), 0u);
+    BOOST_REQUIRE_EQUAL(instance.spend(), 0u);
 }
 
-BOOST_AUTO_TEST_CASE(transaction__claim__two_outputs__sum)
+BOOST_AUTO_TEST_CASE(transaction__spend__two_outputs__sum)
 {
-    const uint64_t claim0 = 123;
-    const uint64_t claim1 = 321;
+    constexpr uint64_t spend0{ 123 };
+    constexpr uint64_t spend1{ 321 };
     const transaction instance
     {
         0,
         {},
         {
-            { claim0, script{} },
-            { claim1, script{} }
+            { spend0, script{} },
+            { spend1, script{} }
         },
         0
     };
 
 
-    // ceilinged_add(claim0, claim1)
-    BOOST_REQUIRE_EQUAL(instance.claim(), claim0 + claim1);
+    // ceilinged_add(spend0, spend1)
+    BOOST_REQUIRE_EQUAL(instance.spend(), spend0 + spend1);
 }
 
 BOOST_AUTO_TEST_CASE(transaction__value__no_inputs__zero)
@@ -514,7 +512,7 @@ BOOST_AUTO_TEST_CASE(transaction__value__no_inputs__zero)
     BOOST_REQUIRE_EQUAL(instance.value(), 0u);
 }
 
-BOOST_AUTO_TEST_CASE(transaction__value__default_input2__max_uint64)
+BOOST_AUTO_TEST_CASE(transaction__value__default_input2__zero)
 {
     transaction instance
     {
@@ -524,14 +522,13 @@ BOOST_AUTO_TEST_CASE(transaction__value__default_input2__max_uint64)
         0
     };
 
-    // ceilinged_add(max_uint64, max_uint64)
-    BOOST_REQUIRE_EQUAL(instance.value(), max_uint64);
+    BOOST_REQUIRE_EQUAL(instance.value(), zero);
 }
 
 BOOST_AUTO_TEST_CASE(transaction__value__two_prevouts__sum)
 {
-    constexpr uint64_t value0 = 123;
-    constexpr uint64_t value1 = 321;
+    constexpr uint64_t value0{ 123 };
+    constexpr uint64_t value1{ 321 };
 
     const input input0;
     const input input1;