tombstone for import

src/cc/importcoin.cpp

@@ -1,4 +1,5 @@
 #include "cc/importcoin.h"
+#include "coins.h"
 #include "hash.h"
 #include "script/cc.h"
 #include "primitives/transaction.h"
@@ -16,9 +17,7 @@ CTransaction MakeImportCoinTransaction(const MomoProof proof, const CTransaction
     std::vector<uint8_t> payload =
         E_MARSHAL(ss << EVAL_IMPORTCOIN; ss << proof; ss << burnTx);
     CMutableTransaction mtx;
-    mtx.vin.resize(1);
-    mtx.vin[0].scriptSig << payload;
-    mtx.vin[0].prevout.n = 10e8;
+    mtx.vin.push_back(CTxIn(COutPoint(burnTx.GetHash(), 10e8), CScript() << payload));
     mtx.vout = payouts;
     return CTransaction(mtx);
 }
@@ -95,25 +94,33 @@ bool Eval::ImportCoin(const std::vector<uint8_t> params, const CTransaction &imp
 }
 
 
-/*
- * Required by main
- */
-CAmount GetCoinImportValue(const CTransaction &tx)
+static bool UnmarshalImportTx(const CTransaction &importTx, MomoProof &proof, CTransaction &burnTx)
 {
-    CScript scriptSig = tx.vin[0].scriptSig;
+    CScript scriptSig = importTx.vin[0].scriptSig;
     auto pc = scriptSig.begin();
     opcodetype opcode;
     std::vector<uint8_t> evalScript;
-    if (!scriptSig.GetOp(pc, opcode, evalScript))
-        return false;
-    if (pc != scriptSig.end())
-        return false;
-    EvalCode code;
+    int code;
+    bool out = false;
+    if (scriptSig.GetOp(pc, opcode, evalScript))
+        if (pc == scriptSig.end())
+            out = E_UNMARSHAL(evalScript, ss >> VARINT(code); ss >> proof; ss >> burnTx);
+    return code == EVAL_IMPORTCOIN && out;
+}
+
+
+/*
+ * Required by main
+ * TODO: test
+ */
+CAmount GetCoinImportValue(const CTransaction &tx)
+{
     MomoProof proof;
     CTransaction burnTx;
-    if (!E_UNMARSHAL(evalScript, ss >> proof; ss >> burnTx))
-        return 0;
-    return burnTx.vout.size() ? burnTx.vout[0].nValue : 0;
+    if (UnmarshalImportTx(tx, proof, burnTx)) {
+        return burnTx.vout.size() ? burnTx.vout[0].nValue : 0;
+    }
+    return 0;
 }
 
 
@@ -145,3 +152,27 @@ bool VerifyCoinImport(const CScript& scriptSig, TransactionSignatureChecker& che
 
     return f() ? true : state.Invalid(false, 0, "invalid-coin-import");
 }
+
+
+void AddImportTombstone(const CTransaction &importTx, CCoinsViewCache &inputs, int nHeight)
+{
+    uint256 burnHash = importTx.vin[0].prevout.hash;
+    CCoinsModifier modifier = inputs.ModifyCoins(burnHash);
+    modifier->nHeight = nHeight;
+    modifier->nVersion = 1;
+    modifier->vout.push_back(CTxOut(0, CScript() << OP_0));
+}
+
+
+void RemoveImportTombstone(const CTransaction &importTx, CCoinsViewCache &inputs)
+{
+    uint256 burnHash = importTx.vin[0].prevout.hash;
+    inputs.ModifyCoins(burnHash)->Clear();
+}
+
+
+int ExistsImportTombstone(const CTransaction &importTx, const CCoinsViewCache &inputs)
+{
+    uint256 burnHash = importTx.vin[0].prevout.hash;
+    return inputs.HaveCoins(burnHash);
+}

src/cc/importcoin.h

@@ -2,6 +2,7 @@
 #define CC_IMPORTCOIN_H
 
 #include "cc/eval.h"
+#include "coins.h"
 #include "primitives/transaction.h"
 #include "script/interpreter.h"
 #include <cryptoconditions.h>
@@ -30,4 +31,9 @@ CTxOut MakeBurnOutput(CAmount value, int targetChain, const std::vector<CTxOut>
 bool VerifyCoinImport(const CScript& scriptSig,
     TransactionSignatureChecker& checker, CValidationState &state);
 
+
+void AddImportTombstone(const CTransaction &importTx, CCoinsViewCache &inputs, int nHeight);
+void RemoveImportTombstone(const CTransaction &importTx, CCoinsViewCache &inputs);
+int ExistsImportTombstone(const CTransaction &importTx, const CCoinsViewCache &inputs);
+
 #endif /* CC_IMPORTCOIN_H */

src/coins.h

@@ -97,11 +97,6 @@ public:
         nVersion = tx.nVersion;
         //nLockTime = tx.nLockTime;
         ClearUnspendable();
-
-        // This must live forever
-        if (tx.IsCoinImport()) {
-            vout.insert(vout.begin(), CTxOut(0, CScript() << OP_NOP << OP_RETURN));
-        }
     }
 
     //! construct a CCoins from a CTransaction, at a given height

src/main.cpp

@@ -1332,15 +1332,19 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
             view.SetBackend(viewMemPool);
             
             // do we already have it?
-            // This is what stops coin import transactions from being double spent; they
-            // grow the UTXO set size slightly
             if (view.HaveCoins(hash))
             {
                 fprintf(stderr,"view.HaveCoins(hash) error\n");
                 return state.Invalid(false, REJECT_DUPLICATE, "already have coins");
             }
             
-            if (!tx.IsCoinImport())
+            if (tx.IsCoinImport())
+            {
+                // Inverse of normal case; if input exists, it's been spent
+                if (ExistsImportTombstone(tx, view))
+                    return state.Invalid(false, REJECT_DUPLICATE, "import tombstone exists");
+            }
+            else
             {
                 // do all inputs exist?
                 // Note that this does not check for the presence of actual outputs (see the next check for that),
@@ -2007,6 +2011,13 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund
         }
     }
     inputs.ModifyCoins(tx.GetHash())->FromTx(tx, nHeight); // add outputs
+    
+    // Unorthodox state
+    if (tx.IsCoinImport()) {
+        // add a tombstone for the burnTx
+        AddImportTombstone(tx, inputs, nHeight);
+    }
+
 }
 
 void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight)
@@ -2465,6 +2476,10 @@ bool DisconnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex
                 }
             }
         }
+        else if (tx.IsCoinImport())
+        {
+            RemoveImportTombstone(tx, view);
+        }
     }
     
     // set the old best anchor back

src/primitives/transaction.h

@@ -454,7 +454,7 @@ public:
 
     bool IsMint() const
     {
-        return (vin.size() == 1 && vin[0].prevout.hash.IsNull());
+        return IsCoinImport() || IsCoinBase();
     }
 
     bool IsCoinBase() const
@@ -464,7 +464,7 @@ public:
 
     bool IsCoinImport() const
     {
-        return IsMint() && vin[0].prevout.n == 10e8;
+        return (vin.size() == 1 && vin[0].prevout.n == 10e8);
     }
 
     friend bool operator==(const CTransaction& a, const CTransaction& b)

src/test-komodo/test_coinimport.cpp

@@ -35,8 +35,8 @@ public:
     CAmount amount = 100;
 
     void SetImportTx() {
-        CTxOut burnOutput = MakeBurnOutput(amount, chainId, payouts);
-        burnTx.vout.push_back(burnOutput);
+        burnTx.vout.resize(0);
+        burnTx.vout.push_back(MakeBurnOutput(amount, chainId, payouts));
         MoMoM = burnTx.GetHash();  // TODO: an actual branch
         importTx = CMutableTransaction(MakeImportCoinTransaction(proof, CTransaction(burnTx), payouts));
     }
@@ -88,7 +88,7 @@ TEST_F(TestCoinImport, testProcessImportThroughPipeline)
     ASSERT_FALSE(acceptTx(tx, mainstate));
     EXPECT_EQ("already in mempool", mainstate.GetRejectReason());
 
-    // should fail in persisted UTXO set
+    // should be in persisted UTXO set
     generateBlock();
     ASSERT_FALSE(acceptTx(tx, mainstate));
     EXPECT_EQ("already have coins", mainstate.GetRejectReason());
@@ -107,6 +107,39 @@ TEST_F(TestCoinImport, testProcessImportThroughPipeline)
 }
 
 
+TEST_F(TestCoinImport, testImportTombstone)
+{
+    CValidationState mainstate;
+    // By setting an unspendable output, there will be no addition to UTXO
+    // Nonetheless, we dont want to be able to import twice
+    payouts[0].scriptPubKey = CScript() << OP_RETURN;
+    SetImportTx();
+    MoMoM = burnTx.GetHash();  // TODO: an actual branch
+    CTransaction tx(importTx);
+
+    // first should work
+    acceptTxFail(tx);
+
+    // should be in persisted UTXO set
+    generateBlock();
+    ASSERT_FALSE(acceptTx(tx, mainstate));
+    EXPECT_EQ("import tombstone exists", mainstate.GetRejectReason());
+    ASSERT_TRUE(pcoinsTip->HaveCoins(burnTx.GetHash()));
+
+    // Now disconnect the block
+    CValidationState invalstate;
+    if (!InvalidateBlock(invalstate, chainActive.Tip())) {
+        FAIL() << invalstate.GetRejectReason();
+    }
+    // Tombstone should be gone from utxo set
+    ASSERT_FALSE(pcoinsTip->HaveCoins(burnTx.GetHash()));
+
+    // should be back in mempool
+    ASSERT_FALSE(acceptTx(tx, mainstate));
+    EXPECT_EQ("already in mempool", mainstate.GetRejectReason());
+}
+
+
 TEST_F(TestCoinImport, testNoVouts)
 {
     importTx.vout.resize(0);