Merge pull request #2839 from codablock/pr_llmq_scanquorumsfix

Fix invalid results from ScanQuorum

src/dbwrapper.h

@@ -464,7 +464,14 @@ public:
         }
 
         if (curIsParent) {
-            return parentIt->GetKey(key);
+            try {
+                // TODO try to avoid this copy (we need a stream that allows reading from external buffers)
+                CDataStream ssKey = parentKey;
+                ssKey >> key;
+            } catch (const std::exception&) {
+                return false;
+            }
+            return true;
         } else {
             try {
                 // TODO try to avoid this copy (we need a stream that allows reading from external buffers)
@@ -482,7 +489,7 @@ public:
             return CDataStream(SER_DISK, CLIENT_VERSION);
         }
         if (curIsParent) {
-            return parentIt->GetKey();
+            return parentKey;
         } else {
             return transactionIt->first;
         }
@@ -546,7 +553,9 @@ protected:
 
     struct DataStreamCmp {
         static bool less(const CDataStream& a, const CDataStream& b) {
-            return std::lexicographical_compare(a.begin(), a.end(), b.begin(), b.end());
+            return std::lexicographical_compare(
+                    (const uint8_t*)a.data(), (const uint8_t*)a.data() + a.size(),
+                    (const uint8_t*)b.data(), (const uint8_t*)b.data() + b.size());
         }
         bool operator()(const CDataStream& a, const CDataStream& b) const {
             return less(a, b);

src/llmq/quorums_blockprocessor.cpp

@@ -25,7 +25,7 @@ CQuorumBlockProcessor* quorumBlockProcessor;
 static const std::string DB_MINED_COMMITMENT = "q_mc";
 static const std::string DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT = "q_mcih";
 
-static const std::string DB_BEST_BLOCK_UPGRADE = "q_bbu";
+static const std::string DB_BEST_BLOCK_UPGRADE = "q_bbu2";
 
 void CQuorumBlockProcessor::ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, CConnman& connman)
 {
@@ -168,9 +168,10 @@ bool CQuorumBlockProcessor::ProcessBlock(const CBlock& block, const CBlockIndex*
 
 // We store a mapping from minedHeight->quorumHeight in the DB
 // minedHeight is inversed so that entries are traversable in reversed order
-static std::tuple<std::string, uint8_t, int> BuildInversedHeightKey(Consensus::LLMQType llmqType, int nMinedHeight)
+static std::tuple<std::string, uint8_t, uint32_t> BuildInversedHeightKey(Consensus::LLMQType llmqType, int nMinedHeight)
 {
-    return std::make_tuple(DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT, (uint8_t)llmqType, std::numeric_limits<int>::max() - nMinedHeight);
+    // nMinedHeight must be converted to big endian to make it comparable when serialized
+    return std::make_tuple(DB_MINED_COMMITMENT_BY_INVERSED_HEIGHT, (uint8_t)llmqType, htobe32(std::numeric_limits<uint32_t>::max() - nMinedHeight));
 }
 
 bool CQuorumBlockProcessor::ProcessCommitment(int nHeight, const uint256& blockHash, const CFinalCommitment& qc, CValidationState& state)
@@ -422,7 +423,7 @@ std::vector<const CBlockIndex*> CQuorumBlockProcessor::GetMinedCommitmentsUntilB
             break;
         }
 
-        int nMinedHeight = std::numeric_limits<int>::max() - std::get<2>(curKey);
+        uint32_t nMinedHeight = std::numeric_limits<uint32_t>::max() - be32toh(std::get<2>(curKey));
         if (nMinedHeight > pindex->nHeight) {
             break;
         }

src/llmq/quorums_instantsend.cpp

@@ -90,21 +90,26 @@ void CInstantSendDb::RemoveInstantSendLock(CDBBatch& batch, const uint256& hash,
     }
 }
 
+static std::tuple<std::string, uint32_t, uint256> BuildInversedISLockMinedKey(int nHeight, const uint256& islockHash)
+{
+    return std::make_tuple(std::string("is_m"), htobe32(std::numeric_limits<uint32_t>::max() - nHeight), islockHash);
+}
+
 void CInstantSendDb::WriteInstantSendLockMined(const uint256& hash, int nHeight)
 {
-    db.Write(std::make_tuple(std::string("is_m"), std::numeric_limits<int>::max() - nHeight, hash), true);
+    db.Write(BuildInversedISLockMinedKey(nHeight, hash), true);
 }
 
 void CInstantSendDb::RemoveInstantSendLockMined(const uint256& hash, int nHeight)
 {
-    db.Erase(std::make_tuple(std::string("is_m"), std::numeric_limits<int>::max() - nHeight, hash));
+    db.Erase(BuildInversedISLockMinedKey(nHeight, hash));
 }
 
 std::unordered_map<uint256, CInstantSendLockPtr> CInstantSendDb::RemoveConfirmedInstantSendLocks(int nUntilHeight)
 {
     auto it = std::unique_ptr<CDBIterator>(db.NewIterator());
 
-    auto firstKey = std::make_tuple(std::string("is_m"), std::numeric_limits<int>::max() - nUntilHeight, uint256());
+    auto firstKey = BuildInversedISLockMinedKey(nUntilHeight, uint256());
 
     it->Seek(firstKey);
 
@@ -115,7 +120,7 @@ std::unordered_map<uint256, CInstantSendLockPtr> CInstantSendDb::RemoveConfirmed
         if (!it->GetKey(curKey) || std::get<0>(curKey) != "is_m") {
             break;
         }
-        int nHeight = std::numeric_limits<int>::max() - std::get<1>(curKey);
+        uint32_t nHeight = std::numeric_limits<uint32_t>::max() - be32toh(std::get<1>(curKey));
         if (nHeight > nUntilHeight) {
             break;
         }