From dc889d7f52ca8672ecf45cb56548fc6e3ade36ff Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 3 May 2018 12:02:51 +0100 Subject: [PATCH 1/3] Update CreateNewContextualCMutableTransaction to create Sapling transactions --- src/main.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 27d045135..fff319b66 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6078,8 +6078,13 @@ 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; + 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; + } // 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 From fa70084c87848c63400fc6b4b8091c167be5f091 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 3 May 2018 12:27:56 +0100 Subject: [PATCH 2/3] 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 #include #include @@ -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(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 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 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 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& 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; } } From e1d41f21f3936a9748f31ae042a579d78b1296d7 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 3 May 2018 13:48:20 +0100 Subject: [PATCH 3/3] Update tests for CreateNewContextualCMutableTransaction changes --- src/gtest/test_checktransaction.cpp | 58 ++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index d7527dbac..4e2affcd9 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -684,7 +684,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( @@ -704,10 +706,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); }