From f51c4d471692b016921cfc443bb653b50a39f6a2 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 1 Nov 2018 10:12:18 +0100 Subject: [PATCH 1/2] check many MoMoMs on tx import --- src/Makefile.ktest.include | 1 - src/cc/eval.cpp | 11 -- src/cc/eval.h | 1 - src/cc/import.cpp | 9 +- src/crosschain.cpp | 35 ++++- src/crosschain.h | 2 +- src/test-komodo/test_crosschain.cpp | 213 ---------------------------- 7 files changed, 38 insertions(+), 234 deletions(-) delete mode 100644 src/test-komodo/test_crosschain.cpp diff --git a/src/Makefile.ktest.include b/src/Makefile.ktest.include index 07c64caa0..640c154e4 100644 --- a/src/Makefile.ktest.include +++ b/src/Makefile.ktest.include @@ -10,7 +10,6 @@ komodo_test_SOURCES = \ test-komodo/test_coinimport.cpp \ test-komodo/test_eval_bet.cpp \ test-komodo/test_eval_notarisation.cpp \ - test-komodo/test_crosschain.cpp \ test-komodo/test_parse_notarisation.cpp komodo_test_CPPFLAGS = $(komodod_CPPFLAGS) diff --git a/src/cc/eval.cpp b/src/cc/eval.cpp index b6fcf57dd..1aaa24d56 100644 --- a/src/cc/eval.cpp +++ b/src/cc/eval.cpp @@ -192,17 +192,6 @@ bool Eval::GetNotarisationData(const uint256 notaryHash, NotarisationData &data) return true; } -/* - * Get MoMoM corresponding to a notarisation tx hash (on assetchain) - */ -bool Eval::GetProofRoot(uint256 kmdNotarisationHash, uint256 &momom) const -{ - std::pair out; - if (!GetNextBacknotarisation(kmdNotarisationHash, out)) return false; - momom = out.second.MoMoM; - return true; -} - uint32_t Eval::GetAssetchainsCC() const { diff --git a/src/cc/eval.h b/src/cc/eval.h index 9ff0ca623..b9189bc89 100644 --- a/src/cc/eval.h +++ b/src/cc/eval.h @@ -103,7 +103,6 @@ public: virtual bool GetBlock(uint256 hash, CBlockIndex& blockIdx) const; virtual int32_t GetNotaries(uint8_t pubkeys[64][33], int32_t height, uint32_t timestamp) const; virtual bool GetNotarisationData(uint256 notarisationHash, NotarisationData &data) const; - virtual bool GetProofRoot(uint256 kmdNotarisationHash, uint256 &momom) const; virtual bool CheckNotaryInputs(const CTransaction &tx, uint32_t height, uint32_t timestamp) const; virtual uint32_t GetAssetchainsCC() const; virtual std::string GetAssetchainsSymbol() const; diff --git a/src/cc/import.cpp b/src/cc/import.cpp index ffc94ac43..b250bb7bf 100644 --- a/src/cc/import.cpp +++ b/src/cc/import.cpp @@ -16,6 +16,7 @@ #include "cc/eval.h" #include "cc/utils.h" #include "importcoin.h" +#include "crosschain.h" #include "primitives/transaction.h" @@ -75,12 +76,8 @@ bool Eval::ImportCoin(const std::vector params, const CTransaction &imp // Check proof confirms existance of burnTx { - uint256 momom, target; - if (!GetProofRoot(proof.first, momom)) - return Invalid("coudnt-load-momom"); - - target = proof.second.Exec(burnTx.GetHash()); - if (momom != proof.second.Exec(burnTx.GetHash())) + uint256 target = proof.second.Exec(burnTx.GetHash()); + if (!CheckMoMoM(proof.first, target)) return Invalid("momom-check-fail"); } diff --git a/src/crosschain.cpp b/src/crosschain.cpp index 831c7bcae..01763c975 100644 --- a/src/crosschain.cpp +++ b/src/crosschain.cpp @@ -95,8 +95,9 @@ template int ScanNotarisationsFromHeight(int nHeight, const IsTarget f, Notarisation &found) { int limit = std::min(nHeight + NOTARISATION_SCAN_LIMIT_BLOCKS, chainActive.Height()); + int start = std::max(nHeight, 1); - for (int h=nHeight; hphashBlock, notarisations)) @@ -245,6 +246,38 @@ bool GetNextBacknotarisation(uint256 kmdNotarisationTxid, Notarisation &out) } +bool CheckMoMoM(uint256 kmdNotarisationHash, uint256 momom) +{ + /* + * Given a notarisation hash and an MoMoM. Backnotarisations may arrive out of order + * or multiple in the same block. So dereference the notarisation hash to the corresponding + * backnotarisation and scan around the kmdheight to see if the MoMoM is a match. + * This is a sledgehammer approach... + */ + + Notarisation bn; + if (!GetBackNotarisation(kmdNotarisationHash, bn)) + return false; + + // Need to get block height of that backnotarisation + EvalRef eval; + CBlockIndex block; + CTransaction tx; + if (!eval->GetTxConfirmed(bn.first, tx, block)){ + fprintf(stderr, "Can't get height of backnotarisation, this should not happen\n"); + return false; + } + + Notarisation nota; + auto checkMoMoM = [&](Notarisation ¬a) { + return nota.second.MoMoM == momom; + }; + + return (bool) ScanNotarisationsFromHeight(block.nHeight-100, checkMoMoM, nota); + +} + + /* * On assetchain * in: txid diff --git a/src/crosschain.h b/src/crosschain.h index 15452ac63..1fbd7603a 100644 --- a/src/crosschain.h +++ b/src/crosschain.h @@ -15,7 +15,7 @@ TxProof GetCrossChainProof(const uint256 txid, const char* targetSymbol, uint32_ void CompleteImportTransaction(CTransaction &importTx); /* On assetchain */ -bool GetNextBacknotarisation(uint256 txid, std::pair &bn); +bool CheckMoMoM(uint256 kmdNotarisationHash, uint256 momom); #endif /* CROSSCHAIN_H */ diff --git a/src/test-komodo/test_crosschain.cpp b/src/test-komodo/test_crosschain.cpp deleted file mode 100644 index 9d24b4d1d..000000000 --- a/src/test-komodo/test_crosschain.cpp +++ /dev/null @@ -1,213 +0,0 @@ -#include -#include -#include -#include -#include - -#include -#include - -#include "cc/eval.h" -#include "importcoin.h" -#include "base58.h" -#include "core_io.h" -#include "crosschain.h" -#include "key.h" -#include "komodo_structs.h" -#include "main.h" -#include "notarisationdb.h" -#include "primitives/block.h" -#include "primitives/transaction.h" -#include "script/cc.h" -#include "script/interpreter.h" -#include "script/serverchecker.h" -#include "txmempool.h" -#include "crosschain.h" - -#include "testutils.h" - - -extern uint256 komodo_calcMoM(int32_t height,int32_t MoMdepth); -extern bool KOMODO_TEST_ASSETCHAIN_SKIP_POW; - - -/* - * Tests for the whole process of creating and validating notary proofs - * using proof roots (MoMoMs). This is to support coin imports. - */ - -namespace TestCrossChainProof { - - -class TestCrossChain : public ::testing::Test, public Eval { -public: - bool CheckNotaryInputs(const CTransaction &tx, uint32_t height, uint32_t timestamp) const - { - NotarisationData data(2); - return ParseNotarisationOpReturn(tx, data); // If it parses it's valid - } -protected: - static void SetUpTestCase() { } - virtual void SetUp() { - KOMODO_TEST_ASSETCHAIN_SKIP_POW = 1; - ASSETCHAINS_CC = 1; - EVAL_TEST = this; - } -}; - - -uint256 endianHash(uint256 h) -{ - uint256 out; - for (int i=0; i<32; i++) { - out.begin()[31-i] = h.begin()[i]; - } - return out; -} - - -TEST_F(TestCrossChain, testCreateAndValidateImportProof) -{ - /* - * This tests the full process of creation of a cross chain proof. - * For the purposes of the test we will use one assetchain and a KMD chain. - * - * In order to do this test, we need 2 blockchains, so we'll fork and make a socket - * for IPC. - */ - - int childPid = fork(); - void *ctx = zmq_ctx_new(); - void *socket = zmq_socket(ctx, ZMQ_PAIR); - if (!childPid) - strcpy(ASSETCHAINS_SYMBOL, "PIZZA"); - setupChain(); - std::vector blocks; - blocks.resize(1000); - NotarisationData a2kmd(0), kmd2a(1); - int numTestNotarisations = 10; - - - auto SendIPC = [&] (std::vector v) { - assert(v.size() == zmq_send(socket, v.data(), v.size(), 0)); - }; - - auto RecvIPC = [&] () { - std::vector out; - out.resize(100000); - int len = zmq_recv(socket, out.data(), out.size(), 0); - assert(len != -1); - out.resize(len); - return out; - }; - - auto RecordNotarisation = [&] (CTransaction inputTx, NotarisationData data) { - CMutableTransaction mtx = spendTx(inputTx); - mtx.vout.resize(2); - mtx.vout[0].scriptPubKey << VCH(notaryKey.GetPubKey().begin(), 33) << OP_CHECKSIG; - mtx.vout[1].scriptPubKey << OP_RETURN << E_MARSHAL(ss << data); - mtx.vout[1].nValue = 0; - mtx.vin[0].scriptSig << getSig(mtx, inputTx.vout[0].scriptPubKey); - - acceptTxFail(CTransaction(mtx)); - return mtx.GetHash(); - }; - - auto RunTestAssetchain = [&] () - { - NotarisationData n(0), back(1); - strcpy(n.symbol, "PIZZA"); - n.ccId = 2; - int height = 0; - - /* - * Send notarisations and write backnotarisations - */ - for (int ni=0; ni> back)); - RecordNotarisation(blocks[height].vtx[0], back); - } - - /* - * Test a proof - */ - uint256 txid = blocks[7].vtx[0].GetHash(); - TxProof proof = GetAssetchainProof(txid); - SendIPC(E_MARSHAL(ss << txid; ss << proof)); - E_UNMARSHAL(RecvIPC(), ss >> proof); - - std::pair bn; - if (!GetNextBacknotarisation(proof.first, bn)) { - printf("GetNextBackNotarisation failed\n"); - return 1; - } - if (proof.second.Exec(txid) != bn.second.MoMoM) { - printf("MoMom incorrect\n"); - return 1; - } - return 0; - }; - - auto RunTestKmd = [&] () - { - NotarisationData n(0); - int height = 0; - - /* - * Write notarisations and send backnotarisations - */ - for (int ni=0; ni> n); - // Grab a coinbase input to fund notarisation - generateBlock(&blocks[++height]); - n.txHash = RecordNotarisation(blocks[height].vtx[0], n); - { - std::vector moms; - uint256 destNotarisationTxid; - n.MoMoM = CalculateProofRoot(n.symbol, 2, height, moms, destNotarisationTxid); - } - n.IsBackNotarisation = 1; - SendIPC(E_MARSHAL(ss << n)); - } - - /* - * Extend proof - */ - TxProof proof; - uint256 txid; - // Extend proof to MoMoM - assert(E_UNMARSHAL(RecvIPC(), ss >> txid; ss >> proof)); - proof = GetCrossChainProof(txid, (char*)"PIZZA", 2, proof); - SendIPC(E_MARSHAL(ss << proof)); - }; - - const char endpoint[] = "ipc://tmpKomodoTestCrossChainSock"; - - if (!childPid) { - assert(0 == zmq_connect(socket, endpoint)); - usleep(20000); - int out = RunTestAssetchain(); - if (!out) printf("Assetchain success\n"); - exit(out); - } - else { - assert(0 == zmq_bind(socket, endpoint)); - RunTestKmd(); - int returnStatus; - waitpid(childPid, &returnStatus, 0); - unlink("tmpKomodoTestCrossChainSock"); - ASSERT_EQ(0, returnStatus); - } - -} - - -} /* namespace TestCrossChainProof */ From 05908caa14a5db6c9207fad84cb367beace1365d Mon Sep 17 00:00:00 2001 From: blackjok3r Date: Thu, 6 Dec 2018 20:26:41 +0800 Subject: [PATCH 2/2] Prevent possible skip of notarisation on KMD --- src/crosschain.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/crosschain.cpp b/src/crosschain.cpp index 250a0e6ca..56c310ce7 100644 --- a/src/crosschain.cpp +++ b/src/crosschain.cpp @@ -69,7 +69,7 @@ uint256 CalculateProofRoot(const char* symbol, uint32_t targetCCid, int kmdHeigh destNotarisationTxid = nota.first; else if (seenOwnNotarisations == 2) goto end; - break; + //break; } } @@ -97,7 +97,7 @@ int ScanNotarisationsFromHeight(int nHeight, const IsTarget f, Notarisation &fou { int limit = std::min(nHeight + NOTARISATION_SCAN_LIMIT_BLOCKS, chainActive.Height()); int start = std::max(nHeight, 1); - + for (int h=start; hGetHeight(), isTarget, nota)) throw std::runtime_error("backnotarisation not yet confirmed"); - + // index of block in MoM leaves nIndex = nota.second.height - blockIndex->GetHeight(); } @@ -325,7 +325,7 @@ TxProof GetAssetchainProof(uint256 hash) } bool fMutated; BuildMerkleTree(&fMutated, leaves, tree); - branch = GetMerkleBranch(nIndex, leaves.size(), tree); + branch = GetMerkleBranch(nIndex, leaves.size(), tree); // Check branch uint256 ourResult = SafeCheckMerkleBranch(blockIndex->hashMerkleRoot, branch, nIndex); @@ -364,7 +364,7 @@ TxProof GetAssetchainProof(uint256 hash) } // Check the proof - if (nota.second.MoM != CBlock::CheckMerkleBranch(hash, branch, nIndex)) + if (nota.second.MoM != CBlock::CheckMerkleBranch(hash, branch, nIndex)) throw std::runtime_error("Failed validating MoM"); // All done!