From a13492744dee59a831633acf0faeb7cf0575d05d Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Fri, 28 Sep 2018 10:32:30 -0600 Subject: [PATCH 1/3] Use max priority for all shielded transfers --- src/coins.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 257de700b..edc4d2e20 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -616,13 +616,13 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const if (tx.IsCoinBase()) return 0.0; - // Joinsplits do not reveal any information about the value or age of a note, so we + // Shielded transfers do not reveal any information about the value or age of a note, so we // cannot apply the priority algorithm used for transparent utxos. Instead, we just - // use the maximum priority whenever a transaction contains any JoinSplits. - // (Note that coinbase transactions cannot contain JoinSplits.) + // use the maximum priority for all (partially or fully) shielded transactions. + // (Note that coinbase transactions cannot contain JoinSplits, or Sapling shielded Spends or Outputs.) // FIXME: this logic is partially duplicated between here and CreateNewBlock in miner.cpp. - if (tx.vjoinsplit.size() > 0) { + if (tx.vjoinsplit.size() > 0 || tx.vShieldedSpend.size() > 0 || tx.vShieldedOutput.size() > 0) { return MAX_PRIORITY; } From 51e6ed6110e612d02698a855b0b1b2027e2c5538 Mon Sep 17 00:00:00 2001 From: Eirik Ogilvie-Wigley Date: Fri, 28 Sep 2018 12:16:05 -0600 Subject: [PATCH 2/3] Move FIXME comment to where the fix should happen --- src/coins.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coins.cpp b/src/coins.cpp index edc4d2e20..31f2dbab0 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -620,12 +620,12 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const // cannot apply the priority algorithm used for transparent utxos. Instead, we just // use the maximum priority for all (partially or fully) shielded transactions. // (Note that coinbase transactions cannot contain JoinSplits, or Sapling shielded Spends or Outputs.) - // FIXME: this logic is partially duplicated between here and CreateNewBlock in miner.cpp. if (tx.vjoinsplit.size() > 0 || tx.vShieldedSpend.size() > 0 || tx.vShieldedOutput.size() > 0) { return MAX_PRIORITY; } + // FIXME: this logic is partially duplicated between here and CreateNewBlock in miner.cpp. double dResult = 0.0; BOOST_FOREACH(const CTxIn& txin, tx.vin) { From b7549f2aec90b44367169b4e0b113f2f27c23bf1 Mon Sep 17 00:00:00 2001 From: Simon Date: Sun, 30 Sep 2018 17:23:53 -0700 Subject: [PATCH 3/3] Add test that Sapling shielded transactions have MAX_PRIORITY --- qa/rpc-tests/wallet_sapling.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/qa/rpc-tests/wallet_sapling.py b/qa/rpc-tests/wallet_sapling.py index 0ce1c734f..bba426868 100755 --- a/qa/rpc-tests/wallet_sapling.py +++ b/qa/rpc-tests/wallet_sapling.py @@ -53,9 +53,14 @@ class WalletSaplingTest(BitcoinTestFramework): recipients = [] recipients.append({"address": saplingAddr0, "amount": Decimal('20')}) myopid = self.nodes[0].z_sendmany(taddr0, recipients, 1, 0) - wait_and_assert_operationid_status(self.nodes[0], myopid) + mytxid = wait_and_assert_operationid_status(self.nodes[0], myopid) self.sync_all() + + # Verify priority of tx is MAX_PRIORITY, defined as 1E+16 (10000000000000000) + mempool = self.nodes[0].getrawmempool(True) + assert(Decimal(mempool[mytxid]['startingpriority']) == Decimal('1E+16')) + self.nodes[2].generate(1) self.sync_all() @@ -70,9 +75,14 @@ class WalletSaplingTest(BitcoinTestFramework): recipients = [] recipients.append({"address": saplingAddr1, "amount": Decimal('15')}) myopid = self.nodes[0].z_sendmany(saplingAddr0, recipients, 1, 0) - wait_and_assert_operationid_status(self.nodes[0], myopid) + mytxid = wait_and_assert_operationid_status(self.nodes[0], myopid) self.sync_all() + + # Verify priority of tx is MAX_PRIORITY, defined as 1E+16 (10000000000000000) + mempool = self.nodes[0].getrawmempool(True) + assert(Decimal(mempool[mytxid]['startingpriority']) == Decimal('1E+16')) + self.nodes[2].generate(1) self.sync_all() @@ -92,6 +102,11 @@ class WalletSaplingTest(BitcoinTestFramework): mytxid = wait_and_assert_operationid_status(self.nodes[1], myopid) self.sync_all() + + # Verify priority of tx is MAX_PRIORITY, defined as 1E+16 (10000000000000000) + mempool = self.nodes[1].getrawmempool(True) + assert(Decimal(mempool[mytxid]['startingpriority']) == Decimal('1E+16')) + self.nodes[2].generate(1) self.sync_all()