From fa70084c87848c63400fc6b4b8091c167be5f091 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 3 May 2018 12:27:56 +0100
Subject: [PATCH] Expire Overwinter transactions before the Sapling activation
 height

---
 src/main.cpp              | 10 ++++++----
 src/rpcrawtransaction.cpp |  1 -
 src/wallet/rpcwallet.cpp  |  9 ---------
 src/wallet/wallet.cpp     |  5 +----
 4 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/src/main.cpp b/src/main.cpp
index fff319b66..1f80e3b29 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -31,6 +31,7 @@
 #include "wallet/asyncrpcoperation_sendmany.h"
 #include "wallet/asyncrpcoperation_shieldcoinbase.h"
 
+#include <algorithm>
 #include <sstream>
 
 #include <boost/algorithm/string/replace.hpp>
@@ -6078,17 +6079,18 @@ CMutableTransaction CreateNewContextualCMutableTransaction(const Consensus::Para
     bool isOverwintered = NetworkUpgradeActive(nHeight, consensusParams, Consensus::UPGRADE_OVERWINTER);
     if (isOverwintered) {
         mtx.fOverwintered = true;
+        mtx.nExpiryHeight = nHeight + expiryDelta;
+
         if (NetworkUpgradeActive(nHeight, consensusParams, Consensus::UPGRADE_SAPLING)) {
             mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID;
             mtx.nVersion = SAPLING_TX_VERSION;
         } else {
             mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
             mtx.nVersion = OVERWINTER_TX_VERSION;
+            mtx.nExpiryHeight = std::min(
+                mtx.nExpiryHeight,
+                static_cast<uint32_t>(consensusParams.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight - 1));
         }
-        // Expiry height is not set. Only fields required for a parser to treat as a valid Overwinter V3 tx.
-
-        // TODO: In future, when moving from Overwinter to Sapling, it will be useful
-        // to set the expiry height to: min(activation_height - 1, default_expiry_height)
     }
     return mtx;
 }
diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp
index 7e03df07a..62f3e6e0a 100644
--- a/src/rpcrawtransaction.cpp
+++ b/src/rpcrawtransaction.cpp
@@ -454,7 +454,6 @@ UniValue createrawtransaction(const UniValue& params, bool fHelp)
         Params().GetConsensus(), nextBlockHeight);
     
     if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
-        rawTx.nExpiryHeight = nextBlockHeight + expiryDelta;
         if (rawTx.nExpiryHeight >= TX_EXPIRY_HEIGHT_THRESHOLD){
             throw JSONRPCError(RPC_INVALID_PARAMETER, "nExpiryHeight must be less than TX_EXPIRY_HEIGHT_THRESHOLD.");
         }
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 8cdc3a092..be5868af4 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -3674,9 +3674,6 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
     if (contextualTx.nVersion == 1 && isShielded) {
         contextualTx.nVersion = 2; // Tx format should support vjoinsplits 
     }
-    if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
-        contextualTx.nExpiryHeight = nextBlockHeight + expiryDelta;
-    }
 
     // Create operation and add to global queue
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
@@ -3872,9 +3869,6 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp)
     if (contextualTx.nVersion == 1) {
         contextualTx.nVersion = 2; // Tx format should support vjoinsplits 
     }
-    if (overwinterActive) {
-        contextualTx.nExpiryHeight = nextBlockHeight + expiryDelta;
-    }
 
     // Create operation and add to global queue
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
@@ -4211,9 +4205,6 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp)
     if (contextualTx.nVersion == 1 && isShielded) {
         contextualTx.nVersion = 2; // Tx format should support vjoinsplit
     }
-    if (overwinterActive) {
-        contextualTx.nExpiryHeight = nextBlockHeight + expiryDelta;
-    }
 
     // Create operation and add to global queue
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 448dd50e7..cab1b55f4 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2580,13 +2580,10 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
         Params().GetConsensus(), nextBlockHeight);
 
     // Activates after Overwinter network upgrade
-    // Set nExpiryHeight to expiryDelta (default 20) blocks past current block height
     if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
-        if (nextBlockHeight + expiryDelta >= TX_EXPIRY_HEIGHT_THRESHOLD){
+        if (txNew.nExpiryHeight >= TX_EXPIRY_HEIGHT_THRESHOLD){
             strFailReason = _("nExpiryHeight must be less than TX_EXPIRY_HEIGHT_THRESHOLD.");
             return false;
-        } else {
-            txNew.nExpiryHeight = nextBlockHeight + expiryDelta;
         }
     }