In order to override these later, the specific details of how the Read, Write, Erase, and Exists functions interact with the actual database file need to go into functions that are not templated.
The functions ReadKey, WriteKey, EraseKey, and HasKey are introduced to handle the actual interaction with the database.
This is mostly a moveonly.
Based on #19290
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
In order to override these later, the specific details of how the Read,
Write, Erase, and Exists functions interact with the actual database
file need to go into functions that are not templated.
206@@ -207,6 +207,12 @@ class BerkeleyBatch
207 operator Dbt*();
208 };
209
210+private:
211+ bool ReadKey(CDataStream& key, CDataStream& value);
pdb is absent is not super intuitive, e.g. it returns false for ReadKey and true for WriteKey.
812+ // Key
813+ SafeDbt datKey(key.data(), key.size());
814+
815+ // Read
816+ SafeDbt datValue;
817+ int ret = pdb->get(activeTxn, datKey, datValue, 0);
false here if ret != 0 || datValue.get_data() == nullptr?
814+
815+ // Read
816+ SafeDbt datValue;
817+ int ret = pdb->get(activeTxn, datKey, datValue, 0);
818+ if (ret == 0 && datValue.get_data() != nullptr) {
819+ value.write((char*)datValue.get_data(), datValue.get_size());
>> operator), not this write.
822+ return false;
823+}
824+
825+bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrite)
826+{
827+ if (!pdb)
206@@ -207,6 +207,12 @@ class BerkeleyBatch
207 operator Dbt*();
208 };
209
210+private:
211+ bool ReadKey(CDataStream& key, CDataStream& value);
212+ bool WriteKey(CDataStream& key, CDataStream& value, bool overwrite=true);
213+ bool EraseKey(CDataStream& key);
214+ bool HasKey(CDataStream& key);
const& are replaced by &. Is there any reason for that? If not, it could make sense to fix this up in a follow-up commit.
const is good documentation about what can potentially be modified and what not.
re: #19292 (review)
I found it really confusing that all the
const&are replaced by&. Is there any reason for that? If not, it could make sense to fix this up in a follow-up commit.
I assumed it was necessary to remove const from all the arguments because reading from CDataStream objects changes their internal real read position. This is a reason I suggested replacing all the CDataStream arguments with Spans #18971 (review). This could be done as a later followup.
Both data and size are marked const in the CDataStream class. How would this affect the read position anyway?
I think the reason is that the in-memory key data needs to be purged (in case it is a private key)?
Both
dataandsizeare markedconstin theCDataStreamclass. How would this affect the read position anyway?
Guess it wouldn’t. I didn’t check if these were const because I just remembered CDataStream being goofy about reading. Maybe it’s less goofy than I thought but I still think Spans are better.
I agree that spans are better when all that is needed is to pass down data and size. I’ll leave this diff here up for grabs:
0diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
1index 125bf004e4..46d12546c7 100644
2--- a/src/wallet/bdb.cpp
3+++ b/src/wallet/bdb.cpp
4@@ -804,7 +804,7 @@ std::string BerkeleyDatabaseVersion()
5 return DbEnv::version(nullptr, nullptr, nullptr);
6 }
7
8-bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value)
9+bool BerkeleyBatch::ReadKey(Span<char> key, CDataStream& value)
10 {
11 if (!pdb)
12 return false;
13@@ -822,7 +822,7 @@ bool BerkeleyBatch::ReadKey(CDataStream& key, CDataStream& value)
14 return false;
15 }
16
17-bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrite)
18+bool BerkeleyBatch::WriteKey(Span<char> key, Span<char> value, bool overwrite)
19 {
20 if (!pdb)
21 return true;
22@@ -840,7 +840,7 @@ bool BerkeleyBatch::WriteKey(CDataStream& key, CDataStream& value, bool overwrit
23 return (ret == 0);
24 }
25
26-bool BerkeleyBatch::EraseKey(CDataStream& key)
27+bool BerkeleyBatch::EraseKey(Span<char> key)
28 {
29 if (!pdb)
30 return false;
31@@ -855,7 +855,7 @@ bool BerkeleyBatch::EraseKey(CDataStream& key)
32 return (ret == 0 || ret == DB_NOTFOUND);
33 }
34
35-bool BerkeleyBatch::HasKey(CDataStream& key)
36+bool BerkeleyBatch::HasKey(Span<char> key)
37 {
38 if (!pdb)
39 return false;
40diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
41index 1e717b95be..f5fa2da76a 100644
42--- a/src/wallet/bdb.h
43+++ b/src/wallet/bdb.h
44@@ -208,10 +208,10 @@ class BerkeleyBatch
45 };
46
47 private:
48- bool ReadKey(CDataStream& key, CDataStream& value);
49- bool WriteKey(CDataStream& key, CDataStream& value, bool overwrite=true);
50- bool EraseKey(CDataStream& key);
51- bool HasKey(CDataStream& key);
52+ bool ReadKey(Span<char> key, CDataStream& value);
53+ bool WriteKey(Span<char> key, Span<char> value, bool overwrite = true);
54+ bool EraseKey(Span<char> key);
55+ bool HasKey(Span<char> key);
56
57 protected:
58 Db* pdb;
BerkeleyBatch::ReadKey return an Option<CDataStream> instead of passing it in, but dunno.
ACK a389ed52e8f4939ab5b4adcf93dcb7783d9006f1 🔳
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3ACK a389ed52e8f4939ab5b4adcf93dcb7783d9006f1 🔳
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUi6RwwAo8bI3tkD2UACsJtRgQS6CUj/gTqTq0lCZ9LijSV9USsdnwawka4GUQt+
8Gd9h7dvn7c3I8F9fo1SnLQS+hB0VfYSB+Vo68V2dkxMiNixTxGt+CNoLsNfD3AQb
9LLSvBZxA+ul1rAzopNEIGRLFBeV5ddFr3RlspZPUKTo4b7iryovqAzDmeM5YnjaW
104J95TT48By2IxwV+Q7gnw0u0OaW2I6lFcklrS8QGU17n/7phTwYu1O9ML1NmuPp4
11WSvkMwx2nkJ3FXCXg572P9eCAcaj7UPwSQcA6QFeNwapUoOknIS2Qq1hhAjhtBXH
123+QxywB6Jb24ik3h57O+wWk3NQeygt7z2qXSJOr2YjfouY43M/BwyoRdE6rB9wzq
13byXTD0WLrMLgk9qMzxquS8d+Pm0FCzI0R54SnesvllhT1DgvrAny6KU934YkuZVW
142mZoPwtMTzRFuDOlvoDZVHwkFlOueuLHwpO1IFdtqERAd3V6KGHV9GCdc5F1jVBq
15k130gEHT
16=3CaZ
17-----END PGP SIGNATURE-----
Timestamp of file with hash 6bf76cee68718cc9e56aef4002215a6e74aed5587fdc5d0275fb0e5769554ec1 -