Skip to content

Commit 01c4b14

Browse files
laanwjlaanwj
committed
Merge #10248: Rewrite addrdb with less duplication using CHashVerifier
cf68a48 Deduplicate addrdb.cpp and use CHashWriter/Verifier (Pieter Wuille) Tree-SHA512: 0301332e797f64da3a1588c9ebaf533af58da41e38f8a64206bff20102c5e82c2a7c630ca3150cf451b2ccf4acb3dd45e44259b6ba15e92786e9e9a2b225bd2f
2 parents b750b33 + cf68a48 commit 01c4b14

File tree

2 files changed

+72
-144
lines changed

2 files changed

+72
-144
lines changed

‎src/addrdb.cpp

Lines changed: 71 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,31 @@
1515
#include "tinyformat.h"
1616
#include "util.h"
1717

18+
namespace {
1819

19-
CBanDB::CBanDB()
20+
template <typename Stream, typename Data>
21+
bool SerializeDB(Stream& stream, const Data& data)
2022
{
21-
pathBanlist = GetDataDir() / "banlist.dat";
23+
// Write and commit header, data
24+
try {
25+
CHashWriter hasher(SER_DISK, CLIENT_VERSION);
26+
stream << FLATDATA(Params().MessageStart()) << data;
27+
hasher << FLATDATA(Params().MessageStart()) << data;
28+
stream << hasher.GetHash();
29+
} catch (const std::exception& e) {
30+
return error("%s: Serialize or I/O error - %s", __func__, e.what());
31+
}
32+
33+
return true;
2234
}
2335

24-
bool CBanDB::Write(const banmap_t& banSet)
36+
template <typename Data>
37+
bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data& data)
2538
{
2639
// Generate random temporary filename
2740
unsigned short randv = 0;
2841
GetRandBytes((unsigned char*)&randv, sizeof(randv));
29-
std::string tmpfn = strprintf("banlist.dat.%04x", randv);
30-
31-
// serialize banlist, checksum data up to that point, then append csum
32-
CDataStream ssBanlist(SER_DISK, CLIENT_VERSION);
33-
ssBanlist << FLATDATA(Params().MessageStart());
34-
ssBanlist << banSet;
35-
uint256 hash = Hash(ssBanlist.begin(), ssBanlist.end());
36-
ssBanlist << hash;
42+
std::string tmpfn = strprintf("%s.%04x", prefix, randv);
3743

3844
// open temp output file, and associate with CAutoFile
3945
fs::path pathTmp = GetDataDir() / tmpfn;
@@ -42,69 +48,41 @@ bool CBanDB::Write(const banmap_t& banSet)
4248
if (fileout.IsNull())
4349
return error("%s: Failed to open file %s", __func__, pathTmp.string());
4450

45-
// Write and commit header, data
46-
try {
47-
fileout << ssBanlist;
48-
}
49-
catch (const std::exception& e) {
50-
return error("%s: Serialize or I/O error - %s", __func__, e.what());
51-
}
51+
// Serialize
52+
if (!SerializeDB(fileout, data)) return false;
5253
FileCommit(fileout.Get());
5354
fileout.fclose();
5455

55-
// replace existing banlist.dat, if any, with new banlist.dat.XXXX
56-
if (!RenameOver(pathTmp, pathBanlist))
56+
// replace existing file, if any, with new file
57+
if (!RenameOver(pathTmp, path))
5758
return error("%s: Rename-into-place failed", __func__);
5859

5960
return true;
6061
}
6162

62-
bool CBanDB::Read(banmap_t& banSet)
63+
template <typename Stream, typename Data>
64+
bool DeserializeDB(Stream& stream, Data& data, bool fCheckSum = true)
6365
{
64-
// open input file, and associate with CAutoFile
65-
FILE *file = fsbridge::fopen(pathBanlist, "rb");
66-
CAutoFile filein(file, SER_DISK, CLIENT_VERSION);
67-
if (filein.IsNull())
68-
return error("%s: Failed to open file %s", __func__, pathBanlist.string());
69-
70-
// use file size to size memory buffer
71-
uint64_t fileSize = fs::file_size(pathBanlist);
72-
uint64_t dataSize = 0;
73-
// Don't try to resize to a negative number if file is small
74-
if (fileSize >= sizeof(uint256))
75-
dataSize = fileSize - sizeof(uint256);
76-
std::vector<unsigned char> vchData;
77-
vchData.resize(dataSize);
78-
uint256 hashIn;
79-
80-
// read data and checksum from file
81-
try {
82-
filein.read((char *)&vchData[0], dataSize);
83-
filein >> hashIn;
84-
}
85-
catch (const std::exception& e) {
86-
return error("%s: Deserialize or I/O error - %s", __func__, e.what());
87-
}
88-
filein.fclose();
89-
90-
CDataStream ssBanlist(vchData, SER_DISK, CLIENT_VERSION);
91-
92-
// verify stored checksum matches input data
93-
uint256 hashTmp = Hash(ssBanlist.begin(), ssBanlist.end());
94-
if (hashIn != hashTmp)
95-
return error("%s: Checksum mismatch, data corrupted", __func__);
96-
97-
unsigned char pchMsgTmp[4];
9866
try {
67+
CHashVerifier<Stream> verifier(&stream);
9968
// de-serialize file header (network specific magic number) and ..
100-
ssBanlist >> FLATDATA(pchMsgTmp);
101-
69+
unsigned char pchMsgTmp[4];
70+
verifier >> FLATDATA(pchMsgTmp);
10271
// ... verify the network matches ours
10372
if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp)))
10473
return error("%s: Invalid network magic number", __func__);
10574

106-
// de-serialize ban data
107-
ssBanlist >> banSet;
75+
// de-serialize data
76+
verifier >> data;
77+
78+
// verify checksum
79+
if (fCheckSum) {
80+
uint256 hashTmp;
81+
stream >> hashTmp;
82+
if (hashTmp != verifier.GetHash()) {
83+
return error("%s: Checksum mismatch, data corrupted", __func__);
84+
}
85+
}
10886
}
10987
catch (const std::exception& e) {
11088
return error("%s: Deserialize or I/O error - %s", __func__, e.what());
@@ -113,106 +91,56 @@ bool CBanDB::Read(banmap_t& banSet)
11391
return true;
11492
}
11593

116-
CAddrDB::CAddrDB()
94+
template <typename Data>
95+
bool DeserializeFileDB(const fs::path& path, Data& data)
11796
{
118-
pathAddr = GetDataDir() / "peers.dat";
97+
// open input file, and associate with CAutoFile
98+
FILE *file = fsbridge::fopen(path, "rb");
99+
CAutoFile filein(file, SER_DISK, CLIENT_VERSION);
100+
if (filein.IsNull())
101+
return error("%s: Failed to open file %s", __func__, path.string());
102+
103+
return DeserializeDB(filein, data);
119104
}
120105

121-
bool CAddrDB::Write(const CAddrMan& addr)
122-
{
123-
// Generate random temporary filename
124-
unsigned short randv = 0;
125-
GetRandBytes((unsigned char*)&randv, sizeof(randv));
126-
std::string tmpfn = strprintf("peers.dat.%04x", randv);
106+
}
127107

128-
// serialize addresses, checksum data up to that point, then append csum
129-
CDataStream ssPeers(SER_DISK, CLIENT_VERSION);
130-
ssPeers << FLATDATA(Params().MessageStart());
131-
ssPeers << addr;
132-
uint256 hash = Hash(ssPeers.begin(), ssPeers.end());
133-
ssPeers << hash;
108+
CBanDB::CBanDB()
109+
{
110+
pathBanlist = GetDataDir() / "banlist.dat";
111+
}
134112

135-
// open temp output file, and associate with CAutoFile
136-
fs::path pathTmp = GetDataDir() / tmpfn;
137-
FILE *file = fsbridge::fopen(pathTmp, "wb");
138-
CAutoFile fileout(file, SER_DISK, CLIENT_VERSION);
139-
if (fileout.IsNull())
140-
return error("%s: Failed to open file %s", __func__, pathTmp.string());
113+
bool CBanDB::Write(const banmap_t& banSet)
114+
{
115+
return SerializeFileDB("banlist", pathBanlist, banSet);
116+
}
141117

142-
// Write and commit header, data
143-
try {
144-
fileout << ssPeers;
145-
}
146-
catch (const std::exception& e) {
147-
return error("%s: Serialize or I/O error - %s", __func__, e.what());
148-
}
149-
FileCommit(fileout.Get());
150-
fileout.fclose();
118+
bool CBanDB::Read(banmap_t& banSet)
119+
{
120+
return DeserializeFileDB(pathBanlist, banSet);
121+
}
151122

152-
// replace existing peers.dat, if any, with new peers.dat.XXXX
153-
if (!RenameOver(pathTmp, pathAddr))
154-
return error("%s: Rename-into-place failed", __func__);
123+
CAddrDB::CAddrDB()
124+
{
125+
pathAddr = GetDataDir() / "peers.dat";
126+
}
155127

156-
return true;
128+
bool CAddrDB::Write(const CAddrMan& addr)
129+
{
130+
return SerializeFileDB("peers", pathAddr, addr);
157131
}
158132

159133
bool CAddrDB::Read(CAddrMan& addr)
160134
{
161-
// open input file, and associate with CAutoFile
162-
FILE *file = fsbridge::fopen(pathAddr, "rb");
163-
CAutoFile filein(file, SER_DISK, CLIENT_VERSION);
164-
if (filein.IsNull())
165-
return error("%s: Failed to open file %s", __func__, pathAddr.string());
166-
167-
// use file size to size memory buffer
168-
uint64_t fileSize = fs::file_size(pathAddr);
169-
uint64_t dataSize = 0;
170-
// Don't try to resize to a negative number if file is small
171-
if (fileSize >= sizeof(uint256))
172-
dataSize = fileSize - sizeof(uint256);
173-
std::vector<unsigned char> vchData;
174-
vchData.resize(dataSize);
175-
uint256 hashIn;
176-
177-
// read data and checksum from file
178-
try {
179-
filein.read((char *)&vchData[0], dataSize);
180-
filein >> hashIn;
181-
}
182-
catch (const std::exception& e) {
183-
return error("%s: Deserialize or I/O error - %s", __func__, e.what());
184-
}
185-
filein.fclose();
186-
187-
CDataStream ssPeers(vchData, SER_DISK, CLIENT_VERSION);
188-
189-
// verify stored checksum matches input data
190-
uint256 hashTmp = Hash(ssPeers.begin(), ssPeers.end());
191-
if (hashIn != hashTmp)
192-
return error("%s: Checksum mismatch, data corrupted", __func__);
193-
194-
return Read(addr, ssPeers);
135+
return DeserializeFileDB(pathAddr, addr);
195136
}
196137

197138
bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers)
198139
{
199-
unsigned char pchMsgTmp[4];
200-
try {
201-
// de-serialize file header (network specific magic number) and ..
202-
ssPeers >> FLATDATA(pchMsgTmp);
203-
204-
// ... verify the network matches ours
205-
if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp)))
206-
return error("%s: Invalid network magic number", __func__);
207-
208-
// de-serialize address data into one CAddrMan object
209-
ssPeers >> addr;
210-
}
211-
catch (const std::exception& e) {
212-
// de-serialization has failed, ensure addrman is left in a clean state
140+
bool ret = DeserializeDB(ssPeers, addr, false);
141+
if (!ret) {
142+
// Ensure addrman is left in a clean state
213143
addr.Clear();
214-
return error("%s: Deserialize or I/O error - %s", __func__, e.what());
215144
}
216-
217-
return true;
145+
return ret;
218146
}

‎src/addrdb.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class CAddrDB
8585
CAddrDB();
8686
bool Write(const CAddrMan& addr);
8787
bool Read(CAddrMan& addr);
88-
bool Read(CAddrMan& addr, CDataStream& ssPeers);
88+
static bool Read(CAddrMan& addr, CDataStream& ssPeers);
8989
};
9090

9191
/** Access to the banlist database (banlist.dat) */

0 commit comments

Comments
 (0)