Fixing on_get_block_header_by_height incorrect height output

forknote · Aug 21, 2017 · 1da81db · 1da81db
1 parent b2479cd
commit 1da81db
Show file tree

Hide file tree

Showing 11 changed files with 55 additions and 23 deletions.
diff --git a/ReleaseNotes.txt b/ReleaseNotes.txt
@@ -1,3 +1,8 @@
+Release notes 2.1.0
+
+- Transaction pool issue fixes
+- Daemon synchronization improvements
+
 Release notes 2.0.7
 
 - Wallet synchronization enhancements

diff --git a/src/CryptoNoteConfig.h b/src/CryptoNoteConfig.h
@@ -196,7 +196,8 @@ const std::initializer_list<CheckpointData> CHECKPOINTS = {
   {1273000, "496a9238c654d79c48d269224aa75d61f51831bae6dc744f5e709bec11c7c9f2"},
   {1278000, "de0225cd279ca27cc8d4f8da1b5b92ba0112e48b3777b8c50301846ccfc9146b"},
   {1283000, "826043db95e9801f038f254d223ce0d0912da269dcce1461b5f0f05ddfae9e1c"},
-  {1324000, "981e6f6871a7c295b56c5ce544adb5a7d52540ee23e15474b4357c7728952fef"}
+  {1324000, "981e6f6871a7c295b56c5ce544adb5a7d52540ee23e15474b4357c7728952fef"},
+  {1329000, "b88ed8dfe95a19bd6377f77c01d87df9cf7bd14cd6de7ec616beca95deb1fc85"}
 };
 } // CryptoNote
 

diff --git a/src/CryptoNoteCore/Core.cpp b/src/CryptoNoteCore/Core.cpp
@@ -655,7 +655,8 @@ std::error_code Core::addBlock(const CachedBlock& cachedBlock, RawBlock&& rawBlo
 
         cache->pushBlock(cachedBlock, transactions, validatorState, cumulativeBlockSize, emissionChange, currentDifficulty, std::move(rawBlock));
 
-        actualizePoolTransactions();
+        updateBlockMedianSize();
+        actualizePoolTransactionsLite(validatorState);
 
         ret = error::AddBlockErrorCode::ADDED_TO_MAIN;
         logger(Logging::DEBUGGING) << "Block " << cachedBlock.getBlockHash() << " added to main chain. Index: " << (previousBlockIndex + 1);
@@ -676,6 +677,8 @@ std::error_code Core::addBlock(const CachedBlock& cachedBlock, RawBlock&& rawBlo
           assert(endpointIndex != 0);
           std::swap(chainsLeaves[0], chainsLeaves[endpointIndex]);
           updateMainChainSet();
+
+          updateBlockMedianSize();
           actualizePoolTransactions();
           copyTransactionsToPool(chainsLeaves[endpointIndex]);
 
@@ -702,9 +705,8 @@ std::error_code Core::addBlock(const CachedBlock& cachedBlock, RawBlock&& rawBlo
                                      currentDifficulty, std::move(rawBlock));
 
       updateMainChainSet();
+      updateBlockMedianSize();
     }
-
-    updateBlockMedianSize();
   } else {
     logger(Logging::DEBUGGING) << "Adding alternative block: " << cachedBlock.getBlockHash();
 
@@ -758,6 +760,22 @@ void Core::actualizePoolTransactions() {
   }
 }
 
+void Core::actualizePoolTransactionsLite(const TransactionValidatorState& validatorState) {
+  auto& pool = *transactionPool;
+  auto hashes = pool.getTransactionHashes();
+
+  for (auto& hash : hashes) {
+    auto tx = pool.getTransaction(hash);
+
+    auto txState = extractSpentOutputs(tx);
+
+    if (hasIntersections(validatorState, txState) || tx.getTransactionBinaryArray().size() > getMaximumTransactionAllowedSize(blockMedianSize, currency)) {
+      pool.removeTransaction(hash);
+      notifyObservers(makeDelTransactionMessage({ hash }, Messages::DeleteTransaction::Reason::NotActual));
+    }
+  }
+}
+
 void Core::switchMainChainStorage(uint32_t splitBlockIndex, IBlockchainCache& newChain) {
   assert(mainChainStorage->getBlockCount() > splitBlockIndex);
 

diff --git a/src/CryptoNoteCore/Core.h b/src/CryptoNoteCore/Core.h
@@ -188,7 +188,9 @@ class Core : public ICore, public ICoreInformation {
   void notifyOnSuccess(error::AddBlockErrorCode opResult, uint32_t previousBlockIndex, const CachedBlock& cachedBlock,
                        const IBlockchainCache& cache);
   void copyTransactionsToPool(IBlockchainCache* alt);
+
   void actualizePoolTransactions();
+  void actualizePoolTransactionsLite(const TransactionValidatorState& validatorState); //Checks pool txs only for double spend.
 
   void transactionPoolCleaningProcedure();
   void updateBlockMedianSize();

diff --git a/src/CryptoNoteCore/Currency.cpp b/src/CryptoNoteCore/Currency.cpp
@@ -830,7 +830,7 @@ fusionTxMaxSize(parameters::MAX_TRANSACTION_SIZE_LIMIT * 30 / 100);
   fusionTxMinInputCount(parameters::FUSION_TX_MIN_INPUT_COUNT);
   fusionTxMinInOutCountRatio(parameters::FUSION_TX_MIN_IN_OUT_COUNT_RATIO);
 
-  upgradeHeightV2(parameters::UPGRADE_HEIGHT_V2);
+    keyImageCheckingBlockIndex(parameters::KEY_IMAGE_CHECKING_BLOCK_INDEX);
   upgradeHeightV3(parameters::UPGRADE_HEIGHT_V3);
   upgradeVotingThreshold(parameters::UPGRADE_VOTING_THRESHOLD);
   upgradeVotingWindow(parameters::UPGRADE_VOTING_WINDOW);

diff --git a/src/CryptoNoteCore/Currency.h b/src/CryptoNoteCore/Currency.h
@@ -61,7 +61,7 @@ class Currency {
   uint64_t expectedNumberOfBlocksPerDay() const { return m_expectedNumberOfBlocksPerDay; }
   size_t blockGrantedFullRewardZoneV1() const { return m_blockGrantedFullRewardZoneV1; }
   size_t blockGrantedFullRewardZoneV2() const { return m_blockGrantedFullRewardZoneV2; }
-  size_t keyImageCheckingBlockIndex() const { return m_keyImageCheckingBlockIndex; }
+  uint32_t keyImageCheckingBlockIndex() const { return m_keyImageCheckingBlockIndex; }
   size_t blockGrantedFullRewardZoneByBlockVersion(uint8_t blockMajorVersion) const;
   size_t minerTxBlobReservedSize() const { return m_minerTxBlobReservedSize; }
   uint64_t maxTransactionSizeLimit() const { return m_maxTransactionSizeLimit; }
@@ -188,7 +188,7 @@ Difficulty nextDifficulty(uint8_t version, uint32_t blockIndex, std::vector<uint
   uint64_t m_expectedNumberOfBlocksPerDay;
   size_t m_blockGrantedFullRewardZoneV1;
   size_t m_blockGrantedFullRewardZoneV2;
-  size_t m_keyImageCheckingBlockIndex;
+  uint32_t m_keyImageCheckingBlockIndex;
   size_t m_minerTxBlobReservedSize;
   uint64_t m_maxTransactionSizeLimit;
 
@@ -288,7 +288,7 @@ class CurrencyBuilder : boost::noncopyable {
   CurrencyBuilder& expectedNumberOfBlocksPerDay(uint64_t val) { m_currency.m_expectedNumberOfBlocksPerDay = val; return *this; }
   CurrencyBuilder& blockGrantedFullRewardZoneV1(size_t val) { m_currency.m_blockGrantedFullRewardZoneV1 = val; return *this; }
   CurrencyBuilder& blockGrantedFullRewardZoneV2(size_t val) { m_currency.m_blockGrantedFullRewardZoneV2 = val; return *this; }
-  CurrencyBuilder& keyImageCheckingBlockIndex(size_t val) { m_currency.m_keyImageCheckingBlockIndex = val; return *this; }
+  CurrencyBuilder& keyImageCheckingBlockIndex(uint32_t val) { m_currency.m_keyImageCheckingBlockIndex = val; return *this; }
   CurrencyBuilder& minerTxBlobReservedSize(size_t val) { m_currency.m_minerTxBlobReservedSize = val; return *this; }
   CurrencyBuilder& maxTransactionSizeLimit(uint64_t val) { m_currency.m_maxTransactionSizeLimit = val; return *this; }
 

diff --git a/src/CryptoNoteCore/TransactionPool.cpp b/src/CryptoNoteCore/TransactionPool.cpp
@@ -77,11 +77,13 @@ bool TransactionPool::pushTransaction(CachedTransaction&& transaction, Transacti
     return false;
   }
 
-  if (!mergeStates(poolState, transactionState)) {
+  if (hasIntersections(poolState, transactionState)) {
     logger(Logging::DEBUGGING) << "pushTransaction: failed to merge states, some keys already used";
     return false;
   }
 
+  mergeStates(poolState, transactionState);
+
   logger(Logging::DEBUGGING) << "pushed transaction " << pendingTx.getTransactionHash() << " to pool";
   return transactionHashIndex.emplace(std::move(pendingTx)).second;
 }

diff --git a/src/CryptoNoteCore/TransactionValidatiorState.cpp b/src/CryptoNoteCore/TransactionValidatiorState.cpp
@@ -19,14 +19,17 @@
 
 namespace CryptoNote {
 
-bool mergeStates(TransactionValidatorState& source, TransactionValidatorState& destination) {  
-  /* source.spentKeyImages.insert(destination.spentKeyImages.begin(), destination.spentKeyImages.end()); */
-  /* source.spentMultisignatureGlobalIndexes.insert(destination.spentMultisignatureGlobalIndexes.begin(), destination.spentMultisignatureGlobalIndexes.end()); */
-  return std::all_of(destination.spentKeyImages.begin(), destination.spentKeyImages.end(),
-                     [&](const Crypto::KeyImage& ki) { return source.spentKeyImages.insert(ki).second; }) &&
-         std::all_of(destination.spentMultisignatureGlobalIndexes.begin(), destination.spentMultisignatureGlobalIndexes.end(),
+void mergeStates(TransactionValidatorState& destionation, const TransactionValidatorState& source) {
+  destionation.spentKeyImages.insert(source.spentKeyImages.begin(), source.spentKeyImages.end());
+  destionation.spentMultisignatureGlobalIndexes.insert(source.spentMultisignatureGlobalIndexes.begin(), source.spentMultisignatureGlobalIndexes.end());
+}
+
+bool hasIntersections(const TransactionValidatorState& destination, const TransactionValidatorState& source) {
+  return std::any_of(source.spentKeyImages.begin(), source.spentKeyImages.end(),
+                     [&](const Crypto::KeyImage& ki) { return destination.spentKeyImages.count(ki) != 0; }) ||
+         std::any_of(source.spentMultisignatureGlobalIndexes.begin(), source.spentMultisignatureGlobalIndexes.end(),
                      [&](const std::pair<uint64_t, uint32_t>& pr) {
-                       return source.spentMultisignatureGlobalIndexes.insert(pr).second;
+                       return destination.spentMultisignatureGlobalIndexes.count(pr) != 0;
                      });
 }
 

diff --git a/src/CryptoNoteCore/TransactionValidatiorState.h b/src/CryptoNoteCore/TransactionValidatiorState.h
@@ -29,7 +29,8 @@ struct TransactionValidatorState {
   std::set<std::pair<uint64_t, uint32_t>> spentMultisignatureGlobalIndexes;
 };
 
-bool mergeStates(TransactionValidatorState& destionation, TransactionValidatorState& source);
+void mergeStates(TransactionValidatorState& destionation, const TransactionValidatorState& source);
+bool hasIntersections(const TransactionValidatorState& destionation, const TransactionValidatorState& source);
 void excludeFromState(TransactionValidatorState& state, const CachedTransaction& transaction);
 
 }
diff --git a/src/Rpc/RpcServer.cpp b/src/Rpc/RpcServer.cpp
@@ -1105,15 +1105,15 @@ bool RpcServer::on_get_block_header_by_hash(const COMMAND_RPC_GET_BLOCK_HEADER_B
 }
 
 bool RpcServer::on_get_block_header_by_height(const COMMAND_RPC_GET_BLOCK_HEADER_BY_HEIGHT::request& req, COMMAND_RPC_GET_BLOCK_HEADER_BY_HEIGHT::response& res) {
-  if (m_core.getTopBlockIndex() + 1 < req.height) {
+  if (m_core.getTopBlockIndex() < req.height) {
     throw JsonRpc::JsonRpcError{ CORE_RPC_ERROR_CODE_TOO_BIG_HEIGHT,
-      std::string("To big height: ") + std::to_string(req.height) + ", current blockchain height = " + std::to_string(m_core.getTopBlockIndex() + 1) };
+      std::string("To big height: ") + std::to_string(req.height) + ", current blockchain height = " + std::to_string(m_core.getTopBlockIndex()) };
   }
 
-  uint32_t index = static_cast<uint32_t>(req.height) - 1;
+uint32_t index = static_cast<uint32_t>(req.height);
   auto block = m_core.getBlockByIndex(index);
   CachedBlock cachedBlock(block);
-  assert(cachedBlock.getBlockIndex() == req.height - 1);
+assert(cachedBlock.getBlockIndex() == req.height);
   fill_block_header_response(block, false, index, cachedBlock.getBlockHash(), res.block_header);
   res.status = CORE_RPC_STATUS_OK;
   return true;

diff --git a/src/version.h.in b/src/version.h.in
@@ -1,4 +1,4 @@
 #define BUILD_COMMIT_ID "@VERSION@"
-#define PROJECT_VERSION "2.0.7"
-#define PROJECT_VERSION_BUILD_NO "1242"
+#define PROJECT_VERSION "2.1.0"
+#define PROJECT_VERSION_BUILD_NO "1243"
 #define PROJECT_VERSION_LONG PROJECT_VERSION "." PROJECT_VERSION_BUILD_NO " (" BUILD_COMMIT_ID ")"