namecoin · domob1812 · Jul 9, 2019 · Jun 28, 2019 · Jun 28, 2019 · Jul 9, 2019
diff --git a/doc/release-notes-namecoin.md b/doc/release-notes-namecoin.md
@@ -2,6 +2,13 @@
 
 ## Version 0.19
 
+- The mempool now allows multiple updates of a single name (in a chain of
+  transactions).  This is something that is allowed in blocks already,
+  i.e. it is just a policy change.  The `name_update` RPC command still
+  prevents such transactions from being created for now, until miners and
+  relaying nodes have updated sufficiently.  More details can be found in
+  [#322](https://github.com/namecoin/namecoin-core/pull/322).
+
 ## Version 0.18
 
 - BIP16, CSV and Segwit will be activated at block height 475,000 on mainnet

diff --git a/src/names/main.cpp b/src/names/main.cpp
@@ -221,10 +221,15 @@ CheckNameTransaction (const CTransaction& tx, unsigned nHeight,
                               REJECT_INVALID, "tx-nameupdate-name-mismatch",
                               "NAME_UPDATE name mismatch to name input");
 
-      /* This is actually redundant, since expired names are removed
-         from the UTXO set and thus not available to be spent anyway.
-         But it does not hurt to enforce this here, too.  It is also
-         exercised by the unit tests.  */
+      /* If the name input is pending, then no further checks with respect
+         to the name input in the name database are done.  Otherwise, we verify
+         that the name input matches the name database; this is redundant
+         as UTXO handling takes care of it anyway, but we do it for
+         an extra safety layer.  */
+      const unsigned inHeight = coinIn.nHeight;
+      if (inHeight == MEMPOOL_HEIGHT)
+        return true;
+
       CNameData oldName;
       if (!view.GetName (name, oldName))
         return state.Invalid (ValidationInvalidReason::CONSENSUS, false,
@@ -234,11 +239,7 @@ CheckNameTransaction (const CTransaction& tx, unsigned nHeight,
         return state.Invalid (ValidationInvalidReason::CONSENSUS, false,
                               REJECT_INVALID, "tx-nameupdate-expired",
                               "NAME_UPDATE on an expired name");
-
-      /* This is an internal consistency check.  If everything is fine,
-         the input coins from the UTXO database should match the
-         name database.  */
-      assert (static_cast<unsigned> (coinIn.nHeight) == oldName.getHeight ());
+      assert (inHeight == oldName.getHeight ());
       assert (tx.vin[nameIn].prevout == oldName.getUpdateOutpoint ());
 
       return true;

diff --git a/src/names/mempool.cpp b/src/names/mempool.cpp
@@ -14,26 +14,77 @@
 
 /* ************************************************************************** */
 
-uint256
-CNameMemPool::getTxForName (const valtype& name) const
+namespace
+{
+
+/**
+ * Returns the outpoint matching the name operation in a given mempool tx, if
+ * there is any.  The txid must be for an entry in the mempool.
+ */
+COutPoint
+getNameOutput (const CTxMemPool& pool, const uint256& txid)
 {
-  NameTxMap::const_iterator mi;
+  AssertLockHeld (pool.cs);
+
+  const auto mit = pool.mapTx.find (txid);
+  assert (mit != pool.mapTx.end ());
+  const auto& vout = mit->GetTx ().vout;
 
-  mi = mapNameRegs.find (name);
-  if (mi != mapNameRegs.end ())
+  for (unsigned i = 0; i != vout.size (); ++i)
     {
-      assert (mapNameUpdates.count (name) == 0);
-      return mi->second;
+      const CNameScript nameOp(vout[i].scriptPubKey);
+      if (nameOp.isNameOp ())
+        return COutPoint (txid, i);
     }
 
-  mi = mapNameUpdates.find (name);
-  if (mi != mapNameUpdates.end ())
+  return COutPoint ();
+}
+
+} // anonymous namespace
+
+COutPoint
+CNameMemPool::lastNameOutput (const valtype& name) const
+{
+  const auto itUpd = updates.find (name);
+  if (itUpd != updates.end ())
     {
-      assert (mapNameRegs.count (name) == 0);
-      return mi->second;
+      /* From all the pending updates, we have to find the last one.  This is
+         the unique outpoint that is not also spent by some other transaction.
+         Thus, we keep track of all the transactions spent as well, and then
+         remove those from the sets of candidates.  Doing so by txid (rather
+         than outpoint) is enough, as those transactions must be in a "chain"
+         anyway.  */
+
+      const std::set<uint256>& candidateTxids = itUpd->second;
+      std::set<uint256> spentTxids;
+
+      for (const auto& txid : candidateTxids)
+        {
+          const auto mit = pool.mapTx.find (txid);
+          assert (mit != pool.mapTx.end ());
+          for (const auto& in : mit->GetTx ().vin)
+            spentTxids.insert (in.prevout.hash);
+        }
+
+      COutPoint res;
+      for (const auto& txid : candidateTxids)
+        {
+          if (spentTxids.count (txid) > 0)
+            continue;
+
+          assert (res.IsNull ());
+          res = getNameOutput (pool, txid);
+        }
+
+      assert (!res.IsNull ());
+      return res;
     }
 
-  return uint256 ();
+  const auto itReg = mapNameRegs.find (name);
+  if (itReg != mapNameRegs.end ())
+    return getNameOutput (pool, itReg->second);
+
+  return COutPoint ();
 }
 
 void
@@ -45,7 +96,7 @@ CNameMemPool::addUnchecked (const CTxMemPoolEntry& entry)
   if (entry.isNameNew ())
     {
       const valtype& newHash = entry.getNameNewHash ();
-      const NameTxMap::const_iterator mit = mapNameNews.find (newHash);
+      const auto mit = mapNameNews.find (newHash);
       if (mit != mapNameNews.end ())
         assert (mit->second == txHash);
       else
@@ -62,8 +113,12 @@ CNameMemPool::addUnchecked (const CTxMemPoolEntry& entry)
   if (entry.isNameUpdate ())
     {
       const valtype& name = entry.getName ();
-      assert (mapNameUpdates.count (name) == 0);
-      mapNameUpdates.insert (std::make_pair (name, txHash));
+      const auto mit = updates.find (name);
+
+      if (mit == updates.end ())
+        updates.emplace (name, std::set<uint256> ({txHash}));
+      else
+        mit->second.insert (txHash);
     }
 }
 
@@ -74,15 +129,21 @@ CNameMemPool::remove (const CTxMemPoolEntry& entry)
 
   if (entry.isNameRegistration ())
     {
-      const NameTxMap::iterator mit = mapNameRegs.find (entry.getName ());
+      const auto mit = mapNameRegs.find (entry.getName ());
       assert (mit != mapNameRegs.end ());
       mapNameRegs.erase (mit);
     }
+
   if (entry.isNameUpdate ())
     {
-      const NameTxMap::iterator mit = mapNameUpdates.find (entry.getName ());
-      assert (mit != mapNameUpdates.end ());
-      mapNameUpdates.erase (mit);
+      const auto itName = updates.find (entry.getName ());
+      assert (itName != updates.end ());
+      auto& txids = itName->second;
+      const auto itTxid = txids.find (entry.GetTx ().GetHash ());
+      assert (itTxid != txids.end ());
+      txids.erase (itTxid);
+      if (txids.empty ())
+        updates.erase (itName);
     }
 }
 
@@ -100,10 +161,10 @@ CNameMemPool::removeConflicts (const CTransaction& tx)
       if (nameOp.isNameOp () && nameOp.getNameOp () == OP_NAME_FIRSTUPDATE)
         {
           const valtype& name = nameOp.getOpName ();
-          const NameTxMap::const_iterator mit = mapNameRegs.find (name);
+          const auto mit = mapNameRegs.find (name);
           if (mit != mapNameRegs.end ())
             {
-              const CTxMemPool::txiter mit2 = pool.mapTx.find (mit->second);
+              const auto mit2 = pool.mapTx.find (mit->second);
               assert (mit2 != pool.mapTx.end ());
               pool.removeRecursive (mit2->GetTx (),
                                     MemPoolRemovalReason::NAME_CONFLICT);
@@ -122,7 +183,7 @@ CNameMemPool::removeUnexpireConflicts (const std::set<valtype>& unexpired)
       LogPrint (BCLog::NAMES, "unexpired: %s, mempool: %u\n",
                 EncodeNameForMessage (name), mapNameRegs.count (name));
 
-      const NameTxMap::const_iterator mit = mapNameRegs.find (name);
+      const auto mit = mapNameRegs.find (name);
       if (mit != mapNameRegs.end ())
         {
           const CTxMemPool::txiter mit2 = pool.mapTx.find (mit->second);
@@ -140,17 +201,25 @@ CNameMemPool::removeExpireConflicts (const std::set<valtype>& expired)
 
   for (const auto& name : expired)
     {
-      LogPrint (BCLog::NAMES, "expired: %s, mempool: %u\n",
-                EncodeNameForMessage (name), mapNameUpdates.count (name));
+      LogPrint (BCLog::NAMES, "expired: %s\n", EncodeNameForMessage (name));
 
-      const NameTxMap::const_iterator mit = mapNameUpdates.find (name);
-      if (mit != mapNameUpdates.end ())
+      const auto mit = updates.find (name);
+      if (mit == updates.end ())
+        continue;
+
+      /* We need to make sure that we keep our copy of txids even when the
+         transactions are removed one by one.  */
+      const std::set<uint256> txidsCopy = mit->second;
+
+      for (const auto& txid : txidsCopy)
         {
-          const CTxMemPool::txiter mit2 = pool.mapTx.find (mit->second);
+          const CTxMemPool::txiter mit2 = pool.mapTx.find (txid);
           assert (mit2 != pool.mapTx.end ());
           pool.removeRecursive (mit2->GetTx (),
                                 MemPoolRemovalReason::NAME_CONFLICT);
         }
+
+      assert (updates.count (name) == 0);
     }
 }
 
@@ -167,14 +236,14 @@ CNameMemPool::check (const CCoinsView& coins) const
     nHeight = mapBlockIndex.find (blockHash)->second->nHeight;
 
   std::set<valtype> nameRegs;
-  std::set<valtype> nameUpdates;
+  std::map<valtype, unsigned> nameUpdates;
   for (const auto& entry : pool.mapTx)
     {
       const uint256 txHash = entry.GetTx ().GetHash ();
       if (entry.isNameNew ())
         {
           const valtype& newHash = entry.getNameNewHash ();
-          const NameTxMap::const_iterator mit = mapNameNews.find (newHash);
+          const auto mit = mapNameNews.find (newHash);
 
           assert (mit != mapNameNews.end ());
           assert (mit->second == txHash);
@@ -184,7 +253,7 @@ CNameMemPool::check (const CCoinsView& coins) const
         {
           const valtype& name = entry.getName ();
 
-          const NameTxMap::const_iterator mit = mapNameRegs.find (name);
+          const auto mit = mapNameRegs.find (name);
           assert (mit != mapNameRegs.end ());
           assert (mit->second == txHash);
 
@@ -203,31 +272,25 @@ CNameMemPool::check (const CCoinsView& coins) const
         {
           const valtype& name = entry.getName ();
 
-          const NameTxMap::const_iterator mit = mapNameUpdates.find (name);
-          assert (mit != mapNameUpdates.end ());
-          assert (mit->second == txHash);
+          const auto mit = updates.find (name);
+          assert (mit != updates.end ());
+          assert (mit->second.count (txHash) > 0);
 
-          assert (nameUpdates.count (name) == 0);
-          nameUpdates.insert (name);
+          ++nameUpdates[name];
 
           /* As above, use nHeight+1 for the expiration check.  */
           CNameData data;
-          if (!coins.GetName (name, data))
-            assert (false);
-          assert (!data.isExpired (nHeight + 1));
+          if (coins.GetName (name, data))
+            assert (!data.isExpired (nHeight + 1));
+          else
+            assert (registersName (name));
         }
     }
 
   assert (nameRegs.size () == mapNameRegs.size ());
-  assert (nameUpdates.size () == mapNameUpdates.size ());
-
-  /* Check that nameRegs and nameUpdates are disjoint.  They must be since
-     a name can only be in either category, depending on whether it exists
-     at the moment or not.  */
-  for (const auto& name : nameRegs)
-    assert (nameUpdates.count (name) == 0);
-  for (const auto& name : nameUpdates)
-    assert (nameRegs.count (name) == 0);
+  assert (nameUpdates.size () == updates.size ());
+  for (const auto& upd : nameUpdates)
+    assert (updates.at (upd.first).size () == upd.second);
 }
 
 bool
@@ -238,11 +301,6 @@ CNameMemPool::checkTx (const CTransaction& tx) const
   if (!tx.IsNamecoin ())
     return true;
 
-  /* In principle, multiple name_updates could be performed within the
-     mempool at once (building upon each other).  This is disallowed, though,
-     since the current mempool implementation does not like it.  (We keep
-     track of only a single update tx for each name.)  */
-
   for (const auto& txout : tx.vout)
     {
       const CNameScript nameOp(txout.scriptPubKey);
@@ -270,12 +328,11 @@ CNameMemPool::checkTx (const CTransaction& tx) const
           }
 
         case OP_NAME_UPDATE:
-          {
-            const valtype& name = nameOp.getOpName ();
-            if (updatesName (name))
-              return false;
-            break;
-          }
+          /* Multiple updates of the same name in a chain are perfectly fine.
+             The main mempool logic takes care that updates are ordered
+             properly and really a chain, as this is automatic due to the
+             coloured-coin nature of names.  */
+          break;
 
         default:
           assert (false);