Include shielded transaction data when calculating RecursiveDynamicUsage of transactions

src/Makefile.gtest.include

@@ -20,6 +20,7 @@ endif
 zcash_gtest_SOURCES += \
 	gtest/test_tautology.cpp \
 	gtest/test_deprecation.cpp \
+	gtest/test_dynamicusage.cpp \
 	gtest/test_equihash.cpp \
 	gtest/test_httprpc.cpp \
 	gtest/test_joinsplit.cpp \

src/core_memusage.h

@@ -25,6 +25,14 @@ static inline size_t RecursiveDynamicUsage(const CTxOut& out) {
     return RecursiveDynamicUsage(out.scriptPubKey);
 }
 
+// These constants are defined in the protocol § 7.1:
+// https://zips.z.cash/protocol/protocol.pdf#txnencoding
+#define OUTPUTDESCRIPTION_SIZE 948
+#define SPENDDESCRIPTION_SIZE 384
+static inline size_t JOINSPLIT_SIZE(int transactionVersion) {
+    return transactionVersion >= SAPLING_TX_VERSION ? 1698 : 1802;
+}
+
 static inline size_t RecursiveDynamicUsage(const CTransaction& tx) {
     size_t mem = memusage::DynamicUsage(tx.vin) + memusage::DynamicUsage(tx.vout);
     for (std::vector<CTxIn>::const_iterator it = tx.vin.begin(); it != tx.vin.end(); it++) {
@@ -33,6 +41,9 @@ static inline size_t RecursiveDynamicUsage(const CTransaction& tx) {
     for (std::vector<CTxOut>::const_iterator it = tx.vout.begin(); it != tx.vout.end(); it++) {
         mem += RecursiveDynamicUsage(*it);
     }
+    mem += tx.vJoinSplit.size() * JOINSPLIT_SIZE(tx.nVersion);
+    mem += tx.vShieldedOutput.size() * OUTPUTDESCRIPTION_SIZE;
+    mem += tx.vShieldedSpend.size() * SPENDDESCRIPTION_SIZE;
     return mem;
 }
 
@@ -44,6 +55,9 @@ static inline size_t RecursiveDynamicUsage(const CMutableTransaction& tx) {
     for (std::vector<CTxOut>::const_iterator it = tx.vout.begin(); it != tx.vout.end(); it++) {
         mem += RecursiveDynamicUsage(*it);
     }
+    mem += tx.vJoinSplit.size() * JOINSPLIT_SIZE(tx.nVersion);
+    mem += tx.vShieldedOutput.size() * OUTPUTDESCRIPTION_SIZE;
+    mem += tx.vShieldedSpend.size() * SPENDDESCRIPTION_SIZE;
     return mem;
 }
 

src/gtest/test_dynamicusage.cpp

@@ -0,0 +1,101 @@
+// Copyright (c) 2019 The Zcash developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or https://www.opensource.org/licenses/mit-license.php 
+
+#include <gtest/gtest.h>
+#include <iostream>
+
+#include "coins.h"
+#include "init.h"
+#include "key.h"
+#include "transaction_builder.h"
+#include "utiltest.h"
+
+extern ZCJoinSplit* params;
+
+TEST(RecursiveDynamicUsageTests, TestTransactionTransparent)
+{
+    auto consensusParams = RegtestActivateSapling();
+
+    CBasicKeyStore keystore;
+    CKey tsk = AddTestCKeyToKeyStore(keystore);
+    auto scriptPubKey = GetScriptForDestination(tsk.GetPubKey().GetID());
+    CTxDestination taddr = tsk.GetPubKey().GetID();
+    
+    auto builder = TransactionBuilder(consensusParams, 1, &keystore);
+    builder.AddTransparentInput(COutPoint(), scriptPubKey, 50000);
+    builder.AddTransparentOutput(taddr, 40000);
+
+    auto tx = builder.Build().GetTxOrThrow();
+    EXPECT_EQ(288, RecursiveDynamicUsage(tx));
+
+    RegtestDeactivateSapling();
+}
+
+TEST(RecursiveDynamicUsageTests, TestTransactionJoinSplit)
+{
+    auto consensusParams = RegtestActivateSapling();
+
+    auto sproutSk = libzcash::SproutSpendingKey::random();
+
+    auto wtx = GetValidSproutReceive(*params, sproutSk, 25000, true);
+    EXPECT_EQ(2806, RecursiveDynamicUsage(wtx));
+
+    RegtestDeactivateSapling();
+}
+
+TEST(RecursiveDynamicUsageTests, TestTransactionSaplingToSapling)
+{
+    auto consensusParams = RegtestActivateSapling();
+    
+    auto sk = libzcash::SaplingSpendingKey::random();
+    auto testNote = GetTestSaplingNote(sk.default_address(), 50000);
+
+    auto builder = TransactionBuilder(consensusParams, 1);
+    builder.AddSaplingSpend(sk.expanded_spending_key(), testNote.note, testNote.tree.root(), testNote.tree.witness());
+    builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 5000, {});
+    
+    auto tx = builder.Build().GetTxOrThrow();
+    EXPECT_EQ(2280, RecursiveDynamicUsage(tx));
+
+    RegtestDeactivateSapling();
+}
+
+TEST(RecursiveDynamicUsageTests, TestTransactionTransparentToSapling)
+{
+    auto consensusParams = RegtestActivateSapling();
+    
+    CBasicKeyStore keystore;
+    CKey tsk = AddTestCKeyToKeyStore(keystore);
+    auto scriptPubKey = GetScriptForDestination(tsk.GetPubKey().GetID());
+    auto sk = libzcash::SaplingSpendingKey::random();
+
+    auto builder = TransactionBuilder(consensusParams, 1, &keystore);
+    builder.AddTransparentInput(COutPoint(), scriptPubKey, 50000);
+    builder.AddSaplingOutput(sk.full_viewing_key().ovk, sk.default_address(), 40000, {});
+    
+    auto tx = builder.Build().GetTxOrThrow();
+    EXPECT_EQ(1172, RecursiveDynamicUsage(tx));
+
+    RegtestDeactivateSapling();
+}
+
+TEST(RecursiveDynamicUsageTests, TestTransactionSaplingToTransparent)
+{
+    auto consensusParams = RegtestActivateSapling();
+    
+    CBasicKeyStore keystore;
+    CKey tsk = AddTestCKeyToKeyStore(keystore);
+    CTxDestination taddr = tsk.GetPubKey().GetID();
+    auto sk = libzcash::SaplingSpendingKey::random();
+    auto testNote = GetTestSaplingNote(sk.default_address(), 50000);
+
+    auto builder = TransactionBuilder(consensusParams, 1, &keystore);
+    builder.AddSaplingSpend(sk.expanded_spending_key(), testNote.note, testNote.tree.root(), testNote.tree.witness());
+    builder.AddTransparentOutput(taddr, 40000);
+
+    auto tx = builder.Build().GetTxOrThrow();
+    EXPECT_EQ(448, RecursiveDynamicUsage(tx));
+
+    RegtestDeactivateSapling();
+}

src/mempool_limit.cpp

@@ -148,9 +148,6 @@ TxWeight TxWeight::negate() const
 WeightedTxInfo WeightedTxInfo::from(const CTransaction& tx, const CAmount& fee)
 {
     size_t memUsage = RecursiveDynamicUsage(tx);
-    memUsage += tx.vJoinSplit.size() * JOINSPLIT_SIZE;
-    memUsage += tx.vShieldedOutput.size() * OUTPUTDESCRIPTION_SIZE;
-    memUsage += tx.vShieldedSpend.size() * SPENDDESCRIPTION_SIZE;
     int64_t cost = std::max((int64_t) memUsage, (int64_t) MIN_TX_COST);
     int64_t evictionWeight = cost;
     if (fee < DEFAULT_FEE) {

src/primitives/transaction.h

@@ -23,10 +23,6 @@
 #include "zcash/JoinSplit.hpp"
 #include "zcash/Proof.hpp"
 
-#define JOINSPLIT_SIZE GetSerializeSize(JSDescription(), SER_NETWORK, PROTOCOL_VERSION)
-#define OUTPUTDESCRIPTION_SIZE GetSerializeSize(OutputDescription(), SER_NETWORK, PROTOCOL_VERSION)
-#define SPENDDESCRIPTION_SIZE GetSerializeSize(SpendDescription(), SER_NETWORK, PROTOCOL_VERSION)
-
 // Overwinter transaction version
 static const int32_t OVERWINTER_TX_VERSION = 3;
 static_assert(OVERWINTER_TX_VERSION >= OVERWINTER_MIN_TX_VERSION,

src/wallet/rpcwallet.cpp

@@ -4468,7 +4468,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
     unsigned int max_tx_size = saplingActive ? MAX_TX_SIZE_AFTER_SAPLING : MAX_TX_SIZE_BEFORE_SAPLING;
     size_t estimatedTxSize = 200;  // tx overhead + wiggle room
     if (isToSproutZaddr) {
-        estimatedTxSize += JOINSPLIT_SIZE;
+        estimatedTxSize += JOINSPLIT_SIZE(SAPLING_TX_VERSION); // We assume that sapling has activated
     } else if (isToSaplingZaddr) {
         estimatedTxSize += OUTPUTDESCRIPTION_SIZE;
     }
@@ -4556,7 +4556,8 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
             if (!maxedOutNotesFlag) {
                 // If we haven't added any notes yet and the merge is to a
                 // z-address, we have already accounted for the first JoinSplit.
-                size_t increase = (sproutNoteInputs.empty() && !isToSproutZaddr) || (sproutNoteInputs.size() % 2 == 0) ? JOINSPLIT_SIZE : 0;
+                size_t increase = (sproutNoteInputs.empty() && !isToSproutZaddr) || (sproutNoteInputs.size() % 2 == 0) ?
+                    JOINSPLIT_SIZE(SAPLING_TX_VERSION) : 0;
                 if (estimatedTxSize + increase >= max_tx_size ||
                     (sproutNoteLimit > 0 && noteCounter > sproutNoteLimit))
                 {

src/zcash/JoinSplit.hpp

@@ -21,6 +21,9 @@ static constexpr size_t GROTH_PROOF_SIZE = (
     48); // π_C
 
 typedef std::array<unsigned char, GROTH_PROOF_SIZE> GrothProof;
+// TODO: Because PHGRProof is listed first, using the default
+// constructor for JSDescription() will create a JSDescription
+// with a PHGRProof. The default however should be GrothProof.
 typedef boost::variant<PHGRProof, GrothProof> SproutProof;
 
 class JSInput {