Issue a warning when filtering by height but also asking for mempool transactions, fix #37

docs/source/conf.py

@@ -55,9 +55,9 @@ author = 'Michal Salaban'
 # built documents.
 #
 # The short X.Y version.
-version = '0.4.3'
+version = '0.4.4'
 # The full version, including alpha/beta/rc tags.
-release = '0.4.3'
+release = '0.4.4'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

docs/source/transactions.rst

@@ -177,6 +177,10 @@ and ``unconfirmed`` query parameters that accept boolean values:
 You may as well query for both confirmed and unconfirmed transactions using
 ``wallet.incoming(unconfirmed=True)`` (the default value for ``confirmed`` is ``True``).
 
+.. note:: Mempool transactions don't belong to the blockchain (yet), therefore they have no height.
+    Setting ``min_height`` or ``max_height`` arguments will **always exclude mempool
+    transactions**. If ``unconfirmed`` is also set to ``True``, a warning will be issued.
+
 .. _sending-payments:
 
 Sending payments

monero/transaction.py

@@ -1,4 +1,5 @@
 import sys
+import warnings
 from .address import address
 from .numbers import PaymentID
 
@@ -160,7 +161,12 @@ class PaymentFilter(object):
         _payment_id = filterparams.pop('payment_id', None)
         if len(filterparams) > 0:
             raise ValueError("Excessive arguments for payment query: {}".format(filterparams))
-
+        if self.unconfirmed and (self.min_height is not None or self.max_height is not None):
+            warnings.warn("Height filtering (min_height/max_height) has been requested while "
+                    "also asking for unconfirmed transactions. These are mutually exclusive. "
+                    "As mempool transactions have no height at all, they will be excluded "
+                    "from the result.",
+                    RuntimeWarning)
         if _local_address is None:
             self.local_addresses = []
         else:

tests/test_wallet.py

@@ -1,12 +1,13 @@
 from datetime import datetime
 from decimal import Decimal
 import unittest
+import warnings
 
 from monero.wallet import Wallet
 from monero.account import Account
 from monero.address import address
 from monero.numbers import PaymentID
-from monero.transaction import IncomingPayment, OutgoingPayment, Transaction
+from monero.transaction import IncomingPayment, Transaction
 
 class FiltersTestCase(unittest.TestCase):
     def setUp(self):
@@ -146,43 +147,51 @@ class FiltersTestCase(unittest.TestCase):
         self.assertEqual(len(pmts), 3)
 
     def test_filter_mempool(self):
-        pmts = self.wallet.incoming()
-        self.assertEqual(len(pmts), 7)
-        for p in pmts:
-            self.assertGreater(self.wallet.confirmations(p.transaction), 0)
-        pmts = self.wallet.incoming(unconfirmed=True)
-        self.assertEqual(len(pmts), 8)
-        pmts = self.wallet.incoming(unconfirmed=True, confirmed=False)
-        self.assertEqual(len(pmts), 1)
-        self.assertEqual(
-            pmts[0].transaction.hash,
-            'd29264ad317e8fdb55ea04484c00420430c35be7b3fe6dd663f99aebf41a786c')
-        self.assertEqual(self.wallet.confirmations(pmts[0]), 0)
-        self.assertEqual(self.wallet.confirmations(pmts[0].transaction), 0)
-        pmts = self.wallet.incoming(unconfirmed=True, confirmed=False, min_height=1)
-        self.assertEqual(len(pmts), 0)
-        pmts = self.wallet.incoming(unconfirmed=True, confirmed=False, max_height=99999999999999)
-        self.assertEqual(len(pmts), 0)
-        pmts = self.wallet.incoming(payment_id='03f6649304ea4cb2')
-        self.assertEqual(len(pmts), 0)
-        pmts = self.wallet.incoming(unconfirmed=True, payment_id='03f6649304ea4cb2')
-        self.assertEqual(len(pmts), 1)
-        pmts = self.wallet.incoming(
-            local_address='9tQoHWyZ4yXUgbz9nvMcFZUfDy5hxcdZabQCxmNCUukKYicXegsDL7nQpcUa3A1pF6K3fhq3scsyY88tdB1MqucULcKzWZC')
-        self.assertEqual(len(pmts), 4)
-        pmts = self.wallet.incoming(
-            unconfirmed=True,
-            local_address='9tQoHWyZ4yXUgbz9nvMcFZUfDy5hxcdZabQCxmNCUukKYicXegsDL7nQpcUa3A1pF6K3fhq3scsyY88tdB1MqucULcKzWZC')
-        self.assertEqual(len(pmts), 5)
-        pmts = self.wallet.incoming(
-            local_address='9tQoHWyZ4yXUgbz9nvMcFZUfDy5hxcdZabQCxmNCUukKYicXegsDL7nQpcUa3A1pF6K3fhq3scsyY88tdB1MqucULcKzWZC',
-            payment_id='03f6649304ea4cb2')
-        self.assertEqual(len(pmts), 0)
-        pmts = self.wallet.incoming(
-            unconfirmed=True,
-            local_address='9tQoHWyZ4yXUgbz9nvMcFZUfDy5hxcdZabQCxmNCUukKYicXegsDL7nQpcUa3A1pF6K3fhq3scsyY88tdB1MqucULcKzWZC',
-            payment_id='03f6649304ea4cb2')
-        self.assertEqual(len(pmts), 1)
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            pmts = self.wallet.incoming()
+            self.assertEqual(len(pmts), 7)
+            for p in pmts:
+                self.assertGreater(self.wallet.confirmations(p.transaction), 0)
+            pmts = self.wallet.incoming(unconfirmed=True)
+            self.assertEqual(len(pmts), 8)
+            pmts = self.wallet.incoming(unconfirmed=True, confirmed=False)
+            self.assertEqual(len(pmts), 1)
+            self.assertEqual(
+                pmts[0].transaction.hash,
+                'd29264ad317e8fdb55ea04484c00420430c35be7b3fe6dd663f99aebf41a786c')
+            self.assertEqual(self.wallet.confirmations(pmts[0]), 0)
+            self.assertEqual(self.wallet.confirmations(pmts[0].transaction), 0)
+            self.assertEqual(len(w), 0)
+            pmts = self.wallet.incoming(unconfirmed=True, confirmed=False, min_height=1)
+            self.assertEqual(len(pmts), 0)
+            self.assertEqual(len(w), 1)
+            self.assertIs(w[0].category, RuntimeWarning)
+            pmts = self.wallet.incoming(unconfirmed=True, confirmed=False, max_height=99999999999999)
+            self.assertEqual(len(pmts), 0)
+            self.assertEqual(len(w), 2)
+            self.assertIs(w[1].category, RuntimeWarning)
+            pmts = self.wallet.incoming(payment_id='03f6649304ea4cb2')
+            self.assertEqual(len(pmts), 0)
+            pmts = self.wallet.incoming(unconfirmed=True, payment_id='03f6649304ea4cb2')
+            self.assertEqual(len(pmts), 1)
+            pmts = self.wallet.incoming(
+                local_address='9tQoHWyZ4yXUgbz9nvMcFZUfDy5hxcdZabQCxmNCUukKYicXegsDL7nQpcUa3A1pF6K3fhq3scsyY88tdB1MqucULcKzWZC')
+            self.assertEqual(len(pmts), 4)
+            pmts = self.wallet.incoming(
+                unconfirmed=True,
+                local_address='9tQoHWyZ4yXUgbz9nvMcFZUfDy5hxcdZabQCxmNCUukKYicXegsDL7nQpcUa3A1pF6K3fhq3scsyY88tdB1MqucULcKzWZC')
+            self.assertEqual(len(pmts), 5)
+            pmts = self.wallet.incoming(
+                local_address='9tQoHWyZ4yXUgbz9nvMcFZUfDy5hxcdZabQCxmNCUukKYicXegsDL7nQpcUa3A1pF6K3fhq3scsyY88tdB1MqucULcKzWZC',
+                payment_id='03f6649304ea4cb2')
+            self.assertEqual(len(pmts), 0)
+            pmts = self.wallet.incoming(
+                unconfirmed=True,
+                local_address='9tQoHWyZ4yXUgbz9nvMcFZUfDy5hxcdZabQCxmNCUukKYicXegsDL7nQpcUa3A1pF6K3fhq3scsyY88tdB1MqucULcKzWZC',
+                payment_id='03f6649304ea4cb2')
+            self.assertEqual(len(pmts), 1)
+            self.assertEqual(len(w), 2)
 
     def test_filter_excessive(self):
         self.assertRaises(ValueError, self.wallet.incoming, excessive_argument='foo')