From 3c7604133e7c5f9153f4c990d7b5f59b993f1809 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Tue, 17 Apr 2018 08:40:02 -0500 Subject: [PATCH] tombstone for import --- src/cc/importcoin.cpp | 63 +++++++++++++++++++++-------- src/cc/importcoin.h | 6 +++ src/coins.h | 5 --- src/main.cpp | 21 ++++++++-- src/primitives/transaction.h | 4 +- src/test-komodo/test_coinimport.cpp | 39 ++++++++++++++++-- 6 files changed, 109 insertions(+), 29 deletions(-) diff --git a/src/cc/importcoin.cpp b/src/cc/importcoin.cpp index 4c9331e73..d5f626683 100644 --- a/src/cc/importcoin.cpp +++ b/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 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 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 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); +} diff --git a/src/cc/importcoin.h b/src/cc/importcoin.h index 8773b09a7..13fefd513 100644 --- a/src/cc/importcoin.h +++ b/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 @@ -30,4 +31,9 @@ CTxOut MakeBurnOutput(CAmount value, int targetChain, const std::vector 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 */ diff --git a/src/coins.h b/src/coins.h index 9cee20712..fcc32caae 100644 --- a/src/coins.h +++ b/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 diff --git a/src/main.cpp b/src/main.cpp index 104540419..67220e891 100644 --- a/src/main.cpp +++ b/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 diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index cdaba66c2..88ee9a312 100644 --- a/src/primitives/transaction.h +++ b/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) diff --git a/src/test-komodo/test_coinimport.cpp b/src/test-komodo/test_coinimport.cpp index 6379aac7f..96543d80b 100644 --- a/src/test-komodo/test_coinimport.cpp +++ 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);