Fix off-by-one errors in use of IsFinalTx()

Previously CreateNewBlock() didn't take into account the fact that
IsFinalTx() without any arguments tests if the transaction is considered
final in the *current* block, when both those functions really needed to
know if the transaction would be final in the *next* block.

Additionally the UI had a similar misunderstanding.

Also adds some basic tests to check that CreateNewBlock() is in fact
mining nLockTime-using transactions correctly.

Thanks to Wladimir J. van der Laan for rebase.

2 files changed, 4 insertions, 4 deletions

diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp
index 2c3a9e3a5c..c76b29861d 100644
--- a/src/qt/transactiondesc.cpp
+++ b/src/qt/transactiondesc.cpp
@@ -20,10 +20,10 @@
 
 QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx)
 {
-    if (!IsFinalTx(wtx))
+    if (!IsFinalTx(wtx, chainActive.Height() + 1))
     {
         if (wtx.nLockTime < LOCKTIME_THRESHOLD)
-            return tr("Open for %n more block(s)", "", wtx.nLockTime - chainActive.Height() + 1);
+            return tr("Open for %n more block(s)", "", wtx.nLockTime - chainActive.Height());
         else
             return tr("Open until %1").arg(GUIUtil::dateTimeStr(wtx.nLockTime));
     }
diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp
index 6823557ebc..257151b926 100644
--- a/src/qt/transactionrecord.cpp
+++ b/src/qt/transactionrecord.cpp
@@ -168,12 +168,12 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx)
     status.depth = wtx.GetDepthInMainChain();
     status.cur_num_blocks = chainActive.Height();
 
-    if (!IsFinalTx(wtx))
+    if (!IsFinalTx(wtx, chainActive.Height() + 1))
     {
         if (wtx.nLockTime < LOCKTIME_THRESHOLD)
         {
             status.status = TransactionStatus::OpenUntilBlock;
-            status.open_for = wtx.nLockTime - chainActive.Height() + 1;
+            status.open_for = wtx.nLockTime - chainActive.Height();
         }
         else
         {