diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index a0efee14e..467c2cf3b 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -779,7 +779,9 @@ TEST(checktransaction_tests, OverwinteredContextualCreateTx) { SelectParams(CBaseChainParams::REGTEST); const Consensus::Params& consensusParams = Params().GetConsensus(); int activationHeight = 5; + int saplingActivationHeight = 30; UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, activationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); { CMutableTransaction mtx = CreateNewContextualCMutableTransaction( @@ -799,10 +801,64 @@ TEST(checktransaction_tests, OverwinteredContextualCreateTx) { EXPECT_EQ(mtx.nVersion, 3); EXPECT_EQ(mtx.fOverwintered, true); EXPECT_EQ(mtx.nVersionGroupId, OVERWINTER_VERSION_GROUP_ID); - EXPECT_EQ(mtx.nExpiryHeight, 0); + EXPECT_EQ(mtx.nExpiryHeight, activationHeight + expiryDelta); + } + + // Close to Sapling activation + { + CMutableTransaction mtx = CreateNewContextualCMutableTransaction( + consensusParams, saplingActivationHeight - expiryDelta - 2); + + EXPECT_EQ(mtx.fOverwintered, true); + EXPECT_EQ(mtx.nVersionGroupId, OVERWINTER_VERSION_GROUP_ID); + EXPECT_EQ(mtx.nVersion, OVERWINTER_TX_VERSION); + EXPECT_EQ(mtx.nExpiryHeight, saplingActivationHeight - 2); + } + + { + CMutableTransaction mtx = CreateNewContextualCMutableTransaction( + consensusParams, saplingActivationHeight - expiryDelta - 1); + + EXPECT_EQ(mtx.fOverwintered, true); + EXPECT_EQ(mtx.nVersionGroupId, OVERWINTER_VERSION_GROUP_ID); + EXPECT_EQ(mtx.nVersion, OVERWINTER_TX_VERSION); + EXPECT_EQ(mtx.nExpiryHeight, saplingActivationHeight - 1); + } + + { + CMutableTransaction mtx = CreateNewContextualCMutableTransaction( + consensusParams, saplingActivationHeight - expiryDelta); + + EXPECT_EQ(mtx.fOverwintered, true); + EXPECT_EQ(mtx.nVersionGroupId, OVERWINTER_VERSION_GROUP_ID); + EXPECT_EQ(mtx.nVersion, OVERWINTER_TX_VERSION); + EXPECT_EQ(mtx.nExpiryHeight, saplingActivationHeight - 1); + } + + // Just before Sapling activation + { + CMutableTransaction mtx = CreateNewContextualCMutableTransaction( + consensusParams, saplingActivationHeight - 1); + + EXPECT_EQ(mtx.fOverwintered, true); + EXPECT_EQ(mtx.nVersionGroupId, OVERWINTER_VERSION_GROUP_ID); + EXPECT_EQ(mtx.nVersion, OVERWINTER_TX_VERSION); + EXPECT_EQ(mtx.nExpiryHeight, saplingActivationHeight - 1); + } + + // Sapling activates + { + CMutableTransaction mtx = CreateNewContextualCMutableTransaction( + consensusParams, saplingActivationHeight); + + EXPECT_EQ(mtx.fOverwintered, true); + EXPECT_EQ(mtx.nVersionGroupId, SAPLING_VERSION_GROUP_ID); + EXPECT_EQ(mtx.nVersion, SAPLING_TX_VERSION); + EXPECT_EQ(mtx.nExpiryHeight, saplingActivationHeight + expiryDelta); } // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } diff --git a/src/main.cpp b/src/main.cpp index bca80c31a..e4c9e12f8 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -31,6 +31,7 @@ #include "wallet/asyncrpcoperation_sendmany.h" #include "wallet/asyncrpcoperation_shieldcoinbase.h" +#include #include #include @@ -6089,12 +6090,18 @@ CMutableTransaction CreateNewContextualCMutableTransaction(const Consensus::Para bool isOverwintered = NetworkUpgradeActive(nHeight, consensusParams, Consensus::UPGRADE_OVERWINTER); if (isOverwintered) { mtx.fOverwintered = true; - mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID; - mtx.nVersion = OVERWINTER_TX_VERSION; - // Expiry height is not set. Only fields required for a parser to treat as a valid Overwinter V3 tx. + mtx.nExpiryHeight = nHeight + expiryDelta; - // 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) + 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(consensusParams.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight - 1)); + } } return mtx; } diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp index 3f6981619..e59a9b369 100644 --- a/src/rpcrawtransaction.cpp +++ b/src/rpcrawtransaction.cpp @@ -455,7 +455,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 9023b80f6..5de71c2c6 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3701,9 +3701,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 q = getAsyncRPCQueue(); @@ -3902,9 +3899,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 q = getAsyncRPCQueue(); @@ -4244,9 +4238,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 q = getAsyncRPCQueue(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5ab1103ff..96e85eb19 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2580,13 +2580,10 @@ bool CWallet::CreateTransaction(const vector& 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; } }