From 7e57fb2fe439069129ba382bc6524e26040fa460 Mon Sep 17 00:00:00 2001 From: Miguel Freitas Date: Mon, 7 Oct 2013 19:33:29 -0300 Subject: [PATCH] mempool uses txhash again (instead of userhash). this is needed to fix key replacement. using userhash in mempool was a bad idea, it creates incompatibilities with merkle tree code etc. now we are back to standard bitcoin protocol regarding tx and inv commands, possibly filter as well. --- src/main.cpp | 32 ++++++++++++++++---------------- src/main.h | 10 +++++----- src/net.cpp | 10 +++++----- src/net.h | 4 ++-- src/rpcrawtransaction.cpp | 6 +++--- src/wallet.cpp | 2 +- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 69bc6ebc..965353eb 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -474,16 +474,16 @@ bool CTxMemPool::accept(CValidationState &state, CTransaction &tx, bool fLimitFr reason.c_str()); // is it already in the memory pool? - uint256 userhash = tx.GetUsernameHash(); + uint256 txhash = tx.GetHash(); { LOCK(cs); - if (mapTx.count(userhash)) + if (mapTx.count(txhash)) return false; } { // do we already have it? - if( pblocktree->HaveTxIndex(userhash) && + if( pblocktree->HaveTxIndex(tx.GetUsernameHash()) && // duplicate should be discarded but replacement is allowed. !verifyDuplicateOrReplacementTx(tx, false, true) ) { return false; @@ -518,25 +518,25 @@ bool CTxMemPool::accept(CValidationState &state, CTransaction &tx, bool fLimitFr // Store transaction in memory { LOCK(cs); - addUnchecked(userhash, tx); // adds to mapTx + addUnchecked(txhash, tx); // adds to mapTx } ///// are we sure this is ok when loading transactions or restoring block txes - SyncWithWallets(userhash, tx, NULL, true); + SyncWithWallets(txhash, tx, NULL, true); printf("CTxMemPool::accept() : accepted %s (poolsz %"PRIszu")\n", - userhash.ToString().c_str(), + txhash.ToString().c_str(), mapTx.size()); return true; } -bool CTxMemPool::addUnchecked(const uint256& userhash, CTransaction &tx) +bool CTxMemPool::addUnchecked(const uint256& txhash, CTransaction &tx) { // Add to memory pool without checking anything. Don't call this directly, // call CTxMemPool::accept to properly check the transaction first. { - mapTx[userhash] = tx; + mapTx[txhash] = tx; nTransactionsUpdated++; } return true; @@ -548,7 +548,7 @@ bool CTxMemPool::remove(const CTransaction &tx, bool fRecursive) // Remove transaction from memory pool if( !tx.IsSpamMessage() ){ LOCK(cs); - uint256 hash = tx.GetUsernameHash(); + uint256 hash = tx.GetHash(); if (mapTx.count(hash)) { mapTx.erase(hash); @@ -636,12 +636,14 @@ bool GetTransaction(const uint256 &userhash, CTransaction &txOut, uint256 &hashB { LOCK(cs_main); { + /* [MF] don't look in mempool anymore - userhash must be written to some block. LOCK(mempool.cs); - if (mempool.exists(userhash)) + if (mempool.exists(userhash)) // and btw mempool is not indexed by userhash anymore! { txOut = mempool.lookup(userhash); return true; } + */ } if (fTxIndex) { @@ -1740,12 +1742,10 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter& filter) for (unsigned int i = 0; i < block.vtx.size(); i++) { uint256 hash = block.vtx[i].GetHash(); - // [MF] unsure. force 0, use userhash for filter, hash for merkletree - // [MF] FIXME: this is most likely broken! - if (i == 0 || filter.IsRelevantAndUpdate(block.vtx[i], block.vtx[i].GetUsernameHash())) + if (i == 0 || filter.IsRelevantAndUpdate(block.vtx[i], hash)) { vMatch.push_back(true); - vMatchedTxn.push_back(make_pair(i, block.vtx[i].GetUsernameHash())); + vMatchedTxn.push_back(make_pair(i, hash)); } else vMatch.push_back(false); @@ -2388,6 +2388,7 @@ bool static AlreadyHave(const CInv& inv) * "inv" cmd of the current txs in memory. if recipient decides he * already has a given tx in txindex (by calling this function), * he would never ask/receive the key replacement. + * Besides: HaveTxIndex is userhash, mempool is txhash. */ /* if( !txInMap ) { @@ -2447,7 +2448,6 @@ void static ProcessGetData(CNode* pfrom) // Thus, the protocol spec specified allows for us to provide duplicate txn here, // however we MUST always provide at least what the remote peer needs typedef std::pair PairType; - // [MF] check if vMatchedTxn is userhash BOOST_FOREACH(PairType& pair, merkleBlock.vMatchedTxn) if (!pfrom->setInventoryKnown.count(CInv(MSG_TX, pair.second))) pfrom->PushMessage("tx", block.vtx[pair.first]); @@ -2863,7 +2863,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) CTransaction tx; vRecv >> tx; - CInv inv(MSG_TX, tx.GetUsernameHash()); + CInv inv(MSG_TX, tx.GetHash()); pfrom->AddInventoryKnown(inv); // Truncate messages to the size of the tx in them diff --git a/src/main.h b/src/main.h index 47ffb1e1..b97be4fb 100644 --- a/src/main.h +++ b/src/main.h @@ -1007,7 +1007,7 @@ class CTxMemPool { public: mutable CCriticalSection cs; - std::map mapTx; // [MF] hash is now userhash + std::map mapTx; // [MF] hash is txhash again bool accept(CValidationState &state, CTransaction &tx, bool fLimitFree, bool* pfMissingInputs); bool addUnchecked(const uint256& userhash, CTransaction &tx); @@ -1021,14 +1021,14 @@ public: return mapTx.size(); } - bool exists(uint256 userhash) + bool exists(uint256 txhash) { - return (mapTx.count(userhash) != 0); + return (mapTx.count(txhash) != 0); } - CTransaction& lookup(uint256 userhash) + CTransaction& lookup(uint256 txhash) { - return mapTx[userhash]; + return mapTx[txhash]; } }; diff --git a/src/net.cpp b/src/net.cpp index b98b8c7a..1bb14446 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1792,17 +1792,17 @@ instance_of_cnetcleanup; -void RelayTransaction(const CTransaction& tx, const uint256& userhash) +void RelayTransaction(const CTransaction& tx, const uint256& txhash) { CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss.reserve(10000); ss << tx; - RelayTransaction(tx, userhash, ss); + RelayTransaction(tx, txhash, ss); } -void RelayTransaction(const CTransaction& tx, const uint256& userhash, const CDataStream& ss) +void RelayTransaction(const CTransaction& tx, const uint256& txhash, const CDataStream& ss) { - CInv inv(MSG_TX, userhash); + CInv inv(MSG_TX, txhash); { LOCK(cs_mapRelay); // Expire old relay messages @@ -1824,7 +1824,7 @@ void RelayTransaction(const CTransaction& tx, const uint256& userhash, const CDa LOCK(pnode->cs_filter); if (pnode->pfilter) { - if (pnode->pfilter->IsRelevantAndUpdate(tx, userhash)) + if (pnode->pfilter->IsRelevantAndUpdate(tx, txhash)) pnode->PushInventory(inv); } else pnode->PushInventory(inv); diff --git a/src/net.h b/src/net.h index ecb0da06..51a02c24 100644 --- a/src/net.h +++ b/src/net.h @@ -644,7 +644,7 @@ public: class CTransaction; -void RelayTransaction(const CTransaction& tx, const uint256& userhash); -void RelayTransaction(const CTransaction& tx, const uint256& userhash, const CDataStream& ss); +void RelayTransaction(const CTransaction& tx, const uint256& txhash); +void RelayTransaction(const CTransaction& tx, const uint256& txhash, const CDataStream& ss); #endif diff --git a/src/rpcrawtransaction.cpp b/src/rpcrawtransaction.cpp index c641d108..91b7d6a2 100644 --- a/src/rpcrawtransaction.cpp +++ b/src/rpcrawtransaction.cpp @@ -220,16 +220,16 @@ Value sendrawtransaction(const Array& params, bool fHelp) catch (std::exception &e) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); } - uint256 hashTx = tx.GetUsernameHash(); + uint256 hashTx = tx.GetHash(); bool fHave = false; uint256 hashBlock; CTransaction tx2; - fHave = GetTransaction(hashTx, tx2, hashBlock); + fHave = GetTransaction(tx.GetUsernameHash(), tx2, hashBlock); // treat replacement as !fHave if( fHave && verifyDuplicateOrReplacementTx(tx, false, true) ) { - printf("verifyDuplicateOrReplacementTx true\n"); + printf("sendrawtransaction: is ReplacementTx true\n"); fHave = false; } diff --git a/src/wallet.cpp b/src/wallet.cpp index d3fd5acd..c36ce522 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -675,7 +675,7 @@ void CWalletTx::RelayWalletTransaction() if (!IsSpamMessage()) { if (GetDepthInMainChain() == 0) { - uint256 hash = GetUsernameHash(); + uint256 hash = GetHash(); printf("Relaying wtx %s\n", hash.ToString().c_str()); RelayTransaction((CTransaction)*this, hash); }