Reducing required confirmations from 6 to 2 (#994)

* Reducing required confirmations from 6 to 2

* Fixing tests

* Fixing rpc tests

* Review comments applied and more refactoring done

qa/rpc-tests/lelantus_mintspend.py

@@ -36,30 +36,28 @@ def run_test(self):
             'Unexpected current balance: {}, should be minus two mints and two fee, ' \
             'but start was {}'.format(cur_bal, start_bal)
 
-        # confirmations should be i due to less than 4 blocks was generated after transactions send
-        for i in range(5):
-            for tr in mint_trans:
-                info = self.nodes[0].gettransaction(tr[0])
-                confrms = info['confirmations']
-                assert confrms == i, \
-                    'Confirmations should be {}, ' \
-                    'due to {} blocks was generated after transaction was created,' \
-                    'but was {}'.format(i, i, confrms)
-
-                tr_type = info['details'][0]['category']
-                assert tr_type == 'mint', 'Unexpected transaction type: {}'.format(tr_type)
-
-            res = False
-            args = {'THAYjKnnCsN5xspnEcb1Ztvw4mSPBuwxzU': 1}
-            try:
-                res = self.nodes[0].joinsplit(args)
-            except JSONRPCException as ex:
-                assert ex.error['message'] == 'Insufficient funds'
-
-            assert not res, 'Did not raise spend exception, but should be.'
-
-            self.nodes[0].generate(1)
-            self.sync_all()
+        for tr in mint_trans:
+            info = self.nodes[0].gettransaction(tr[0])
+            confrms = info['confirmations']
+            assert confrms == 0, \
+                'Confirmations should be {}, ' \
+                'due to {} blocks was generated after transaction was created,' \
+                'but was {}'.format(0, 0, confrms)
+
+            tr_type = info['details'][0]['category']
+            assert tr_type == 'mint', 'Unexpected transaction type: {}'.format(tr_type)
+
+        res = False
+        args = {'THAYjKnnCsN5xspnEcb1Ztvw4mSPBuwxzU': 1}
+        try:
+            res = self.nodes[0].joinsplit(args)
+        except JSONRPCException as ex:
+            assert ex.error['message'] == 'Insufficient funds'
+
+        assert not res, 'Did not raise spend exception, but should be.'
+
+        self.nodes[0].generate(1)
+        self.sync_all()
 
         # generate last confirmation block - now all transactions should be confimed
         self.nodes[0].generate(1)
@@ -68,9 +66,9 @@ def run_test(self):
         for tr in mint_trans:
             info = self.nodes[0].gettransaction(tr[0])
             confrms = info['confirmations']
-            assert confrms == 6, \
-                'Confirmations should be 6, ' \
-                'due to 6 blocks was generated after transaction was created,' \
+            assert confrms == 2, \
+                'Confirmations should be 2, ' \
+                'due to 2 blocks was generated after transaction was created,' \
                 'but was {}.'.format(confrms)
             tr_type = info['details'][0]['category']
             assert tr_type == 'mint', 'Unexpected transaction type'

qa/rpc-tests/sigma_listsigmapubcoins_validation.py

@@ -48,19 +48,15 @@
            ('0.1', True), ('0.1', True), ('0.5', False), ('0.5', True), ('0.5', True), ('1', True), ('1', True),
            ('10', False), ('10', True), ('100', False), ('100', False), ('25', False), ('25', False)],
 
-    '25': [('0.05', False), ('0.05', False), ('0.05', False), ('0.05', True), ('0.05', True), ('0.1', False),
-           ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False),
-           ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', True),
-           ('0.1', True), ('0.5', False), ('0.5', False), ('0.5', True), ('0.5', True), ('1', False), ('1', False),
-           ('1', False), ('1', False), ('1', True), ('1', True), ('10', False), ('10', False), ('10', False),
-           ('10', True), ('100', False), ('100', False), ('25', True), ('25', True)],
-
-    '100': [('0.05', False), ('0.05', False), ('0.05', True), ('0.05', True), ('0.05', True),
+    '25': [('0.05', False), ('0.05', True), ('0.05', True), ('0.05', True), ('0.1', False), ('0.1', False),
             ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False),
-            ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False),
-            ('0.1', True), ('0.1', True), ('0.5', False), ('0.5', False), ('0.5', True), ('0.5', True), ('1', False),
-            ('1', False), ('1', False), ('1', False), ('1', True), ('1', True), ('10', False), ('10', False),
-            ('10', False), ('10', True), ('100', False), ('100', True), ('25', True), ('25', True)]
+            ('0.1', True), ('0.1', True), ('0.5', False), ('0.5', True), ('0.5', True), ('1', True), ('1', True),
+            ('10', False), ('10', True), ('100', False), ('100', False), ('25', False), ('25', True)],
+
+    '100':  [('0.05', True), ('0.05', True), ('0.05', True), ('0.05', True), ('0.1', False), ('0.1', False),
+             ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False), ('0.1', False),
+             ('0.1', True), ('0.1', True), ('0.5', False), ('0.5', True), ('0.5', True), ('1', True), ('1', True),
+             ('10', False), ('10', True), ('100', False), ('100', True), ('25', False), ('25', True)]
 }
 
 
@@ -90,8 +86,8 @@ def run_test(self):
         for denom in denoms.values():
             self.nodes[0].mint(denom)
             self.nodes[0].mint(denom)
-            self.nodes[0].generate(6)
-            self.sync_all()
+        self.nodes[0].generate(2)
+        self.sync_all()
 
         pubcoins = [(pubcoin['denomination'], pubcoin['IsUsed'])
                     for pubcoin in self.nodes[0].listsigmapubcoins()]
@@ -106,7 +102,7 @@ def run_test(self):
             print("denom: " + denom_name)
             args = {'THAYjKnnCsN5xspnEcb1Ztvw4mSPBuwxzU': denom}
             self.nodes[0].spendmany("", args)
-            self.nodes[0].generate(2)
+            self.nodes[0].generate(1)
             self.sync_all()
 
             pubcoins = [(pubcoin['denomination'], pubcoin['IsUsed'])
@@ -116,6 +112,8 @@ def run_test(self):
                 'Unexpected pubcoins list returned after spend: {}. Should be: {}, but was: {}.' \
                     .format(denom, sorted(expected_pubcoins_after_denom_spend[denom_name]), sorted(pubcoins))
 
+
+
         unused_pubcoins_sum = sum([Decimal(pubcoin['denomination'])
                          for pubcoin in self.nodes[0].listsigmapubcoins() if pubcoin['IsUsed'] == False])
         expected_unused_pubcoins_sum = sum(denoms.values()) - len(denoms) * 0.05

qa/rpc-tests/sigma_listunspentmints_sigma_validation.py

@@ -48,7 +48,7 @@ def run_test(self):
             denoms_total += 2
             mint1 = self.nodes[0].mint(denom)
             mint2 = self.nodes[0].mint(denom)
-            self.nodes[0].generate(6)
+            self.nodes[0].generate(2)
             self.sync_all()
 
             unspent_sigma_mints = self.nodes[0].listunspentsigmamints(1)
@@ -67,8 +67,8 @@ def run_test(self):
             'This txid with denom {} should be in list of unspent mints: {}, but was not'.format((mint2, denom), mints)
 
 
-        # check that all sigma mints has at least 6 confirmations
-        assert len(self.nodes[0].listunspentsigmamints(6)) == denoms_total
+        # check that all sigma mints has at least 2 confirmations
+        assert len(self.nodes[0].listunspentsigmamints(2)) == denoms_total
 
         # generate mints for the fee
         self.nodes[0].mint(0.05)
@@ -78,7 +78,7 @@ def run_test(self):
         self.nodes[0].mint(0.05)
         self.nodes[0].mint(0.1)
 
-        self.nodes[0].generate(6)
+        self.nodes[0].generate(2)
         self.sync_all()
 
         for case_name, denom in denoms:
@@ -99,7 +99,7 @@ def run_test(self):
         assert sorted(expected_unspent_denoms) == sorted(actual_unspent_denoms), \
             'Unexpected denominations are Un-Used.'
 
-        unspent_mints = len(self.nodes[0].listunspentsigmamints(6))
+        unspent_mints = len(self.nodes[0].listunspentsigmamints(2))
         assert unspent_mints == denoms_total // 2, \
             'Amount of unspent mints was not decreased as expected: {}.'.format(unspent_mints)
 

qa/rpc-tests/sigma_mintspend.py

@@ -45,31 +45,29 @@ def run_test(self):
                 'Unexpected current balance: {}, should be minus two mints and two fee, ' \
                 'but start was {}'.format(cur_bal, start_bal)
 
-        # confirmations should be i due to less than 4 blocks was generated after transactions send
-        for i in range(5):
-            for tr in mint_trans:
-                info = self.nodes[0].gettransaction(tr)
-                confrms = info['confirmations']
-                assert confrms == i, \
-                    'Confirmations should be {}, ' \
-                    'due to {} blocks was generated after transaction was created,' \
-                    'but was {}'.format(i, i, confrms)
-
-                tr_type = info['details'][0]['category']
-                assert tr_type == 'mint', 'Unexpected transaction type: {}'.format(tr_type)
-
-            for denom in denoms:
-                res = False
-                args = {'THAYjKnnCsN5xspnEcb1Ztvw4mSPBuwxzU': denom}
-                try:
-                    res = self.nodes[0].spendmany("", args)
-                except JSONRPCException as ex:
-                    assert ex.error['message'] == 'Insufficient funds'
-
-                assert not res, 'Did not raise spend exception, but should be.'
-
-            self.nodes[0].generate(1)
-            self.sync_all()
+        for tr in mint_trans:
+            info = self.nodes[0].gettransaction(tr)
+            confrms = info['confirmations']
+            assert confrms == 0, \
+                'Confirmations should be {}, ' \
+                'due to {} blocks was generated after transaction was created,' \
+                'but was {}'.format(0, 0, confrms)
+
+            tr_type = info['details'][0]['category']
+            assert tr_type == 'mint', 'Unexpected transaction type: {}'.format(tr_type)
+
+        for denom in denoms:
+            res = False
+            args = {'THAYjKnnCsN5xspnEcb1Ztvw4mSPBuwxzU': denom}
+            try:
+                res = self.nodes[0].spendmany("", args)
+            except JSONRPCException as ex:
+                assert ex.error['message'] == 'Insufficient funds'
+
+            assert not res, 'Did not raise spend exception, but should be.'
+
+        self.nodes[0].generate(1)
+        self.sync_all()
 
         # generate last confirmation block - now all transactions should be confimed
         self.nodes[0].generate(1)
@@ -78,9 +76,9 @@ def run_test(self):
         for tr in mint_trans:
             info = self.nodes[0].gettransaction(tr)
             confrms = info['confirmations']
-            assert confrms == 6, \
-                'Confirmations should be 6, ' \
-                'due to 6 blocks was generated after transaction was created,' \
+            assert confrms == 2, \
+                'Confirmations should be 2, ' \
+                'due to 2 blocks was generated after transaction was created,' \
                 'but was {}.'.format(confrms)
             tr_type = info['details'][0]['category']
             assert tr_type == 'mint', 'Unexpected transaction type'

src/batchproof_container.cpp

@@ -161,7 +161,7 @@ void BatchProofContainer::batch_lelantus() {
             uint256 blockHash;
             state->GetCoinSetForSpend(
                     &chainActive,
-                    chainActive.Height() - (ZC_MINT_CONFIRMATIONS - 1), // required 6 confirmation for mint to spend
+                    chainActive.Height() - (ZC_MINT_CONFIRMATIONS - 1), // required 2 confirmation for mint to spend
                     itr.first.first,
                     blockHash,
                     coins);

src/qt/bitcoinstrings.cpp

@@ -117,7 +117,7 @@ QT_TRANSLATE_NOOP("firo-core", ""
 "Found unconfirmed denominated outputs, will wait till they confirm to "
 "continue."),
 QT_TRANSLATE_NOOP("firo-core", ""
-"Has to have at least two mint coins with at least 6 confirmation in order to "
+"Has to have at least two mint coins with at least 2 confirmation in order to "
 "spend a coin"),
 QT_TRANSLATE_NOOP("firo-core", ""
 "How thorough the block verification of -checkblocks is (0-4, default: %u)"),
@@ -321,7 +321,7 @@ QT_TRANSLATE_NOOP("firo-core", ""
 "Exodus will now shutdown - please restart the client for your new "
 "configuration to take effect."),
 QT_TRANSLATE_NOOP("firo-core", ""
-"it has to have at least two mint coins with at least 6 confirmation in order "
+"it has to have at least two mint coins with at least 2 confirmation in order "
 "to spend a coin"),
 QT_TRANSLATE_NOOP("firo-core", ""
 "znodeaddr option is deprecated. Please use znode.conf to manage your remote "

src/qt/transactionrecord.h

@@ -84,7 +84,7 @@ class TransactionRecord
     };
 
     /** Number of confirmation recommended for accepting a transaction */
-    static const int RecommendedNumConfirmations = 6;
+    static const int RecommendedNumConfirmations = 2;
 
     TransactionRecord():
             hash(), time(0), type(Other), address(""), debit(0), credit(0), idx(0)

src/test/lelantus_mintspend_test.cpp

@@ -56,18 +56,15 @@ BOOST_AUTO_TEST_CASE(lelantus_mintspend_test)
         {GetScriptForDestination(randomAddr.Get()), 30 * COIN, true},
     };
 
-    // Add 5 more blocks and verify that Mint can not be spent until 6 blocks verification
-    for (int i = 0; i < 5; i++)
+    // Add 1 more blocks and verify that Mint can not be spent until 2blocks verification
     {
-        {
-            CWalletTx wtx;
-            BOOST_CHECK_THROW(pwalletMain->JoinSplitLelantus(recipients, {}, wtx), WalletError); //this must throw as it has to have at least two mint coins with at least 6 confirmation
-        }
-
-        GenerateBlock({});
+        CWalletTx wtx;
+        BOOST_CHECK_THROW(pwalletMain->JoinSplitLelantus(recipients, {}, wtx), WalletError); //this must throw as it has to have at least two mint coins with at least 2 confirmation
     }
 
-    BOOST_CHECK_MESSAGE(previousHeight + 5 == chainActive.Height(), "Block not added to chain");
+    GenerateBlock({});
+
+    BOOST_CHECK_MESSAGE(previousHeight + 1 == chainActive.Height(), "Block not added to chain");
     BOOST_CHECK_MESSAGE(mempool.size() == 0, "Mempool must be empty");
 
     CWalletTx wtx;
@@ -106,7 +103,7 @@ BOOST_AUTO_TEST_CASE(lelantus_mintspend_test)
     GenerateBlock({CMutableTransaction(*result.tx)});
     BOOST_CHECK_MESSAGE(previousHeight + 1 == chainActive.Height(), "Block not added to chain");
     BOOST_CHECK_MESSAGE(mempool.size() == 0, "Mempool not cleared");
-    GenerateBlocks(6);
+    GenerateBlocks(2);
     for(auto mint : spendCoins)
         pwalletMain->zwallet->GetTracker().SetLelantusPubcoinUsed(primitives::GetPubCoinValueHash(mint.value), uint256());
 
@@ -126,7 +123,7 @@ BOOST_AUTO_TEST_CASE(lelantus_mintspend_test)
     GenerateBlock({CMutableTransaction(*result.tx)});
     BOOST_CHECK_MESSAGE(previousHeight + 1 == chainActive.Height(), "Block not added to chain");
     BOOST_CHECK_MESSAGE(mempool.size() == 0, "Mempool not cleared");
-    GenerateBlocks(6);
+    GenerateBlocks(2);
 
     //Set mints unused, and try to spend again
     for(auto mint : spendCoins)

src/test/sigma_manymintspend_test.cpp

@@ -100,19 +100,16 @@ BOOST_AUTO_TEST_CASE(sigma_mintspend_many)
         std::vector<CRecipient> recipients = {
                 {GetScriptForDestination(randomAddr.Get()), nAmount, true},
         };
-        //Add 5 more blocks and verify that Mint can not be spent until 6 blocks verification
-        for (int i = 0; i < 5; i++)
-        {
-            BOOST_CHECK_THROW(pwalletMain->SpendSigma(recipients, wtx), WalletError); //this must throw as 6 blocks have not passed yet,
+        //Add 1 more blocks and verify that Mint can not be spent until 2 blocks verification
+        BOOST_CHECK_THROW(pwalletMain->SpendSigma(recipients, wtx), WalletError); //this must throw as 2 blocks have not passed yet,
 
-            b = CreateAndProcessBlock(scriptPubKey);
-            wtx.Init(NULL);
-        }
-        BOOST_CHECK_MESSAGE(previousHeight + 5 == chainActive.Height(), "Block not added to chain");
+        b = CreateAndProcessBlock(scriptPubKey);
+        wtx.Init(NULL);
+        BOOST_CHECK_MESSAGE(previousHeight + 1 == chainActive.Height(), "Block not added to chain");
 
         wtx.Init(NULL);
 
-        BOOST_CHECK_THROW(pwalletMain->SpendSigma(recipients, wtx), WalletError); //this must throw as it has to have at least two mint coins with at least 6 confirmation
+        BOOST_CHECK_THROW(pwalletMain->SpendSigma(recipients, wtx), WalletError); //this must throw as it has to have at least two mint coins with at least 2 confirmation
 
         vDMints.clear();
         vecSend = CWallet::CreateSigmaMintRecipients(privCoins, vDMints);
@@ -129,17 +126,14 @@ BOOST_AUTO_TEST_CASE(sigma_mintspend_many)
 
 
         previousHeight = chainActive.Height();
-        //Add 5 more blocks and verify that Mint can not be spent until 6 blocks verification
+        //Add 1 more blocks and verify that Mint can not be spent until 2 blocks verification
         wtx.Init(NULL);
-        for (int i = 0; i < 5; i++)
-        {
-            BOOST_CHECK_THROW(pwalletMain->SpendSigma(recipients, wtx), WalletError); //this must throw as 6 blocks have not passed yet,
+        BOOST_CHECK_THROW(pwalletMain->SpendSigma(recipients, wtx), WalletError); //this must throw as 2 blocks have not passed yet,
 
-            b = CreateAndProcessBlock(scriptPubKey);
-            wtx.Init(NULL);
-        }
+        b = CreateAndProcessBlock(scriptPubKey);
+        wtx.Init(NULL);
 
-        BOOST_CHECK_MESSAGE(previousHeight + 5 == chainActive.Height(), "Block not added to chain");
+        BOOST_CHECK_MESSAGE(previousHeight + 1 == chainActive.Height(), "Block not added to chain");
 
         // Create two spend transactions using the same mints
         std::vector<CSigmaEntry> coins;