Skip to content

Commit c6e8a66

Browse files
committed
Fix two critical stability issues: share verification freeze and header persistence loss
1. Persist verified share status in LevelDB (p2pool known_verified pattern): - Add is_verified flag to ShareMetadata with backward-compatible serialization - Pre-populate verified tracker on startup, bypassing expensive re-verification - Budget think() Phase 2 to 100 shares per call with deferred continuation - Flush verified hashes in batches of 50 via mark_shares_verified() 2. Switch header chain to write-back persistence (Litecoin Core FlushStateToDisk pattern): - Replace per-header individual put() with dirty set + atomic WriteBatch - flush_dirty() writes all dirty headers + tip in single fsync'd batch - Prevents header loss on crash (was losing 4.6M headers, keeping only ~8000) - Add BatchWriter::commit_sync() for forced fsync 3. Fix on_tip_changed threading: post callback to ioc instead of executing on hdr_pool thread, preventing cross-thread access to bcaster/web_server/UTXO
1 parent 29be121 commit c6e8a66

9 files changed

Lines changed: 255 additions & 34 deletions

File tree

src/c2pool/c2pool_refactored.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1850,9 +1850,15 @@ int main(int argc, char* argv[]) {
18501850
chain = embedded_chain.get(),
18511851
utxo = ltc_utxo_cache.get(),
18521852
utxo_db = ltc_utxo_db.get(),
1853-
&web_server](
1853+
&web_server, &ioc](
18541854
const uint256& old_tip, uint32_t old_height,
18551855
const uint256& new_tip, uint32_t new_height) {
1856+
// This callback fires on the hdr_pool thread.
1857+
// Post all work to ioc to avoid cross-thread access to
1858+
// bcaster/web_server/UTXO which are ioc-thread objects.
1859+
boost::asio::post(ioc,
1860+
[tracker, bcaster, chain, utxo, utxo_db, &web_server,
1861+
old_tip, old_height, new_tip, new_height]() {
18561862
bool is_reorg = (new_height <= old_height);
18571863
LOG_WARNING << "[EMB-LTC] Chain tip changed: "
18581864
<< old_tip.GetHex().substr(0, 16) << " (h=" << old_height << ") → "
@@ -1919,6 +1925,7 @@ int main(int argc, char* argv[]) {
19191925
}
19201926
// Trigger work refresh so stratum miners get the new tip
19211927
web_server.trigger_work_refresh_debounced();
1928+
}); // end post(ioc)
19221929
});
19231930
LOG_INFO << "[EMB-LTC] Chain reorg handler registered";
19241931

src/c2pool/storage/sharechain_storage.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,26 @@ bool SharechainStorage::remove_share(const uint256& hash)
182182
return m_leveldb_store->remove_share(hash);
183183
}
184184

185+
bool SharechainStorage::load_share(const uint256& hash, std::vector<uint8_t>& serialized_data,
186+
core::ShareMetadata& metadata)
187+
{
188+
if (!m_leveldb_store)
189+
return false;
190+
try {
191+
return m_leveldb_store->load_share(hash, serialized_data, metadata);
192+
} catch (const std::exception& e) {
193+
LOG_ERROR << "Error loading share from LevelDB: " << e.what();
194+
return false;
195+
}
196+
}
197+
198+
bool SharechainStorage::mark_shares_verified(const std::vector<uint256>& hashes)
199+
{
200+
if (!m_leveldb_store || hashes.empty())
201+
return false;
202+
return m_leveldb_store->mark_shares_verified(hashes);
203+
}
204+
185205
void SharechainStorage::log_storage_stats()
186206
{
187207
if (!m_leveldb_store) {

src/c2pool/storage/sharechain_storage.hpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,10 @@ class SharechainStorage {
106106
bool load_share(const uint256& hash, std::vector<uint8_t>& serialized_data,
107107
uint256& prev_hash, uint64_t& height, uint64_t& timestamp,
108108
uint256& work, uint256& target, bool& is_orphan);
109+
110+
/// Load a share with full metadata (includes is_verified)
111+
bool load_share(const uint256& hash, std::vector<uint8_t>& serialized_data,
112+
core::ShareMetadata& metadata);
109113

110114
/**
111115
* @brief Check if a share exists in storage
@@ -120,7 +124,10 @@ class SharechainStorage {
120124
* @return True if successfully removed
121125
*/
122126
bool remove_share(const uint256& hash);
123-
127+
128+
/// Batch-mark shares as verified in LevelDB metadata.
129+
bool mark_shares_verified(const std::vector<uint256>& hashes);
130+
124131
/**
125132
* @brief Log storage statistics
126133
*/

src/core/leveldb_store.cpp

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,25 @@ bool LevelDBStore::BatchWriter::commit()
302302
return true;
303303
}
304304

305+
bool LevelDBStore::BatchWriter::commit_sync()
306+
{
307+
if (!m_store || !m_store->m_db) {
308+
LOG_ERROR << "LevelDB store not available for batch commit_sync";
309+
return false;
310+
}
311+
312+
leveldb::WriteOptions write_options;
313+
write_options.sync = true; // force fsync
314+
315+
leveldb::Status status = m_store->m_db->Write(write_options, m_batch.get());
316+
if (!status.ok()) {
317+
LOG_ERROR << "Failed to commit_sync batch: " << status.ToString();
318+
return false;
319+
}
320+
321+
return true;
322+
}
323+
305324
// SharechainLevelDBStore implementation
306325
SharechainLevelDBStore::SharechainLevelDBStore(const std::string& base_path, const std::string& network_name)
307326
: m_base_path(base_path), m_network_name(network_name)
@@ -459,15 +478,16 @@ bool SharechainLevelDBStore::store_share(const uint256& hash, const std::vector<
459478
metadata_stream << metadata.work;
460479
metadata_stream << metadata.target;
461480
metadata_stream << static_cast<uint8_t>(metadata.is_orphan ? 1 : 0);
462-
481+
metadata_stream << static_cast<uint8_t>(metadata.is_verified ? 1 : 0);
482+
463483
// Convert from std::vector<std::byte> to std::vector<uint8_t>
464484
auto span = metadata_stream.get_span();
465485
std::vector<uint8_t> metadata_data(
466486
reinterpret_cast<const uint8_t*>(span.data()),
467487
reinterpret_cast<const uint8_t*>(span.data()) + span.size()
468488
);
469489
batch.put(index_key, metadata_data);
470-
490+
471491
// Store height -> hash mapping for chain traversal
472492
if (metadata.height > 0) {
473493
std::string height_key = make_height_key(metadata.height);
@@ -515,6 +535,7 @@ bool SharechainLevelDBStore::store_shares_batch(const std::vector<BatchShareEntr
515535
metadata_stream << e.metadata.work;
516536
metadata_stream << e.metadata.target;
517537
metadata_stream << static_cast<uint8_t>(e.metadata.is_orphan ? 1 : 0);
538+
metadata_stream << static_cast<uint8_t>(e.metadata.is_verified ? 1 : 0);
518539
auto span = metadata_stream.get_span();
519540
std::vector<uint8_t> metadata_data(
520541
reinterpret_cast<const uint8_t*>(span.data()),
@@ -586,7 +607,17 @@ bool SharechainLevelDBStore::load_share(const uint256& hash, std::vector<uint8_t
586607
uint8_t orphan_flag;
587608
metadata_stream >> orphan_flag;
588609
metadata.is_orphan = (orphan_flag != 0);
589-
610+
611+
// is_verified: backward-compatible — old DBs won't have this byte
612+
// PackStream clears m_vch when cursor reaches end, so non-empty means more data
613+
try {
614+
uint8_t verified_flag;
615+
metadata_stream >> verified_flag;
616+
metadata.is_verified = (verified_flag != 0);
617+
} catch (...) {
618+
metadata.is_verified = false;
619+
}
620+
590621
return true;
591622

592623
} catch (const std::exception& e) {
@@ -647,6 +678,52 @@ bool SharechainLevelDBStore::remove_share(const uint256& hash)
647678
}
648679
}
649680

681+
bool SharechainLevelDBStore::mark_shares_verified(const std::vector<uint256>& hashes)
682+
{
683+
if (!m_store || hashes.empty())
684+
return false;
685+
686+
try {
687+
auto batch = m_store->create_batch();
688+
int updated = 0;
689+
690+
for (const auto& hash : hashes) {
691+
ShareMetadata metadata;
692+
std::vector<uint8_t> dummy;
693+
if (!load_share(hash, dummy, metadata))
694+
continue;
695+
if (metadata.is_verified)
696+
continue; // already marked
697+
698+
metadata.is_verified = true;
699+
700+
PackStream ms;
701+
ms << metadata.prev_hash;
702+
ms << metadata.height;
703+
ms << metadata.timestamp;
704+
ms << metadata.work;
705+
ms << metadata.target;
706+
ms << static_cast<uint8_t>(metadata.is_orphan ? 1 : 0);
707+
ms << static_cast<uint8_t>(1); // is_verified = true
708+
auto span = ms.get_span();
709+
std::vector<uint8_t> md(
710+
reinterpret_cast<const uint8_t*>(span.data()),
711+
reinterpret_cast<const uint8_t*>(span.data()) + span.size());
712+
batch.put(make_index_key(hash), md);
713+
++updated;
714+
}
715+
716+
if (updated > 0 && !batch.commit()) {
717+
LOG_ERROR << "mark_shares_verified: batch commit failed";
718+
return false;
719+
}
720+
return true;
721+
} catch (const std::exception& e) {
722+
LOG_ERROR << "mark_shares_verified failed: " << e.what();
723+
return false;
724+
}
725+
}
726+
650727
std::vector<uint256> SharechainLevelDBStore::get_chain_hashes(const uint256& start_hash, uint64_t max_count, bool forward)
651728
{
652729
std::vector<uint256> hashes;

src/core/leveldb_store.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class LevelDBStore
4949
void put(const std::string& key, const std::vector<uint8_t>& value);
5050
void remove(const std::string& key);
5151
bool commit();
52+
bool commit_sync(); // force fsync regardless of store options
5253
};
5354

5455
LevelDBStore(const std::string& db_path, const LevelDBOptions& options);
@@ -84,6 +85,7 @@ struct ShareMetadata {
8485
uint256 work = uint256::ZERO;
8586
uint256 target = uint256::ZERO;
8687
bool is_orphan = false;
88+
bool is_verified = false;
8789
};
8890

8991
/**
@@ -164,6 +166,9 @@ class SharechainLevelDBStore
164166
bool has_share(const uint256& hash);
165167
bool remove_share(const uint256& hash);
166168

169+
/// Batch-update the is_verified flag for multiple shares without rewriting share data.
170+
bool mark_shares_verified(const std::vector<uint256>& hashes);
171+
167172
// Chain traversal
168173
std::vector<uint256> get_chain_hashes(const uint256& start_hash, uint64_t max_count, bool forward = true);
169174
std::vector<uint256> get_shares_by_height_range(uint64_t start_height, uint64_t end_height);

src/impl/ltc/coin/header_chain.hpp

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <map>
2323
#include <mutex>
2424
#include <optional>
25+
#include <set>
2526
#include <thread>
2627
#include <functional>
2728
#include <iomanip>
@@ -803,36 +804,60 @@ class HeaderChain {
803804
// "height" → uint32_t
804805

805806
void persist_header(const IndexEntry& entry) {
806-
if (!m_db || !m_db->is_open()) {
807-
LOG_DEBUG_COIND << "[EMB-LTC] persist_header: no DB (in-memory mode)";
808-
return;
807+
// Write-back model (matches Litecoin Core's setDirtyBlockIndex):
808+
// Mark header as dirty — actual DB write happens in flush_dirty().
809+
m_dirty_headers.insert(entry.block_hash);
810+
}
811+
812+
/// Flush all dirty headers + tip to LevelDB in a single atomic WriteBatch
813+
/// with sync=true (fsync). Matches Litecoin Core's FlushStateToDisk() pattern.
814+
/// Caller must hold m_mutex.
815+
void flush_dirty() {
816+
if (!m_db || !m_db->is_open() || m_dirty_headers.empty()) return;
817+
818+
auto batch = m_db->create_batch();
819+
int count = 0;
820+
821+
for (const auto& hash : m_dirty_headers) {
822+
auto it = m_headers.find(hash);
823+
if (it == m_headers.end()) continue;
824+
825+
auto packed = pack(it->second);
826+
std::vector<uint8_t> data(
827+
reinterpret_cast<const uint8_t*>(packed.data()),
828+
reinterpret_cast<const uint8_t*>(packed.data()) + packed.size());
829+
830+
std::string key = "h";
831+
key.append(reinterpret_cast<const char*>(hash.data()), 32);
832+
batch.put(key, data);
833+
++count;
809834
}
810835

811-
auto packed = pack(entry);
812-
std::vector<uint8_t> data(
813-
reinterpret_cast<const uint8_t*>(packed.data()),
814-
reinterpret_cast<const uint8_t*>(packed.data()) + packed.size());
836+
// Include tip in the same atomic batch
837+
{
838+
std::vector<uint8_t> tip_data(m_tip.data(), m_tip.data() + 32);
839+
batch.put("tip", tip_data);
840+
841+
uint32_t h = m_tip_height;
842+
std::vector<uint8_t> height_data(4);
843+
height_data[0] = (h >> 24) & 0xFF;
844+
height_data[1] = (h >> 16) & 0xFF;
845+
height_data[2] = (h >> 8) & 0xFF;
846+
height_data[3] = h & 0xFF;
847+
batch.put("height", height_data);
848+
}
815849

816-
std::string key = "h";
817-
key.append(reinterpret_cast<const char*>(entry.block_hash.data()), 32);
818-
m_db->put(key, data);
850+
if (batch.commit_sync()) {
851+
m_dirty_headers.clear();
852+
LOG_DEBUG_COIND << "[EMB-LTC] flush_dirty: wrote " << count << " headers to LevelDB (synced)";
853+
} else {
854+
LOG_ERROR << "[EMB-LTC] flush_dirty: WriteBatch FAILED for " << count << " headers";
855+
}
819856
}
820857

821858
void persist_tip() {
822-
if (!m_db || !m_db->is_open()) return;
823-
824-
// Store tip hash
825-
std::vector<uint8_t> tip_data(m_tip.data(), m_tip.data() + 32);
826-
m_db->put("tip", tip_data);
827-
828-
// Store height
829-
uint32_t h = m_tip_height;
830-
std::vector<uint8_t> height_data(4);
831-
height_data[0] = (h >> 24) & 0xFF;
832-
height_data[1] = (h >> 16) & 0xFF;
833-
height_data[2] = (h >> 8) & 0xFF;
834-
height_data[3] = h & 0xFF;
835-
m_db->put("height", height_data);
859+
// Write-back: flush all dirty headers + tip atomically.
860+
flush_dirty();
836861
}
837862

838863
void load_from_db() {
@@ -910,6 +935,9 @@ class HeaderChain {
910935

911936
std::unique_ptr<core::LevelDBStore> m_db;
912937

938+
/// Dirty set: headers modified since last flush (write-back model).
939+
std::set<uint256> m_dirty_headers;
940+
913941
/// Callback fired on tip change (reorg / equal-work switch).
914942
TipChangedCallback m_on_tip_changed;
915943

0 commit comments

Comments
 (0)