Skip to content

Commit 90ea1da

Browse files
PastaPastaPastaPastaPastaPasta
committed
Merge #6636: refactor: replace CDeterministicMNStateDiff macros with boost::hana
0073e9a refactor: replace `CDeterministicMNStateDiff` macros with `boost::hana` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependency for #6627 * Currently, we rely on a set of macros to use a different set of instructions for (de)serializing `pubKeyOperator` in `CDeterministicMNStateDiff`, one of those macros is the following https://github.com/dashpay/dash/blob/bcd14b05cec7d94986f114ca17bbdadbee701d9b/src/evo/dmnstate.h#L219-L226 If we pretend `DMN_STATE_DIFF_ALL_FIELDS` only pertains to `pubKeyOperator`, the macro would expand as ```c++ if (strcmp("pubKeyOperator", "pubKeyOperator") == 0 && (obj.fields & Field_pubKeyOperator)) { SER_READ(obj, read_pubkey = true); READWRITE(CBLSLazyPublicKeyVersionWrapper(const_cast<CBLSLazyPublicKey&>(obj.state.pubKeyOperator), obj.state.nVersion == CProRegTx::LEGACY_BLS_VERSION)); } else if (obj.fields & Field_pubKeyOperator) READWRITE(obj.state.pubKeyOperator); ``` Even though `READWRITE(obj.state.pubKeyOperator)` is _logically_ unreachable, it is something the compiler still has to evaluate and it can because `READWRITE(obj.state.pubKeyOperator)` is still a valid expression. But if we need to carve out a similar different rule in a later PR for `newThing` where `newThing` is a `std::shared_ptr<Interface>` that is implemented by the serializable type `Implementation`, the unreachable but still evaluable expression `READWRITE(obj.state.newThing)` cannot be evaluated as you _cannot_ do anything with a pure virtual class, which `Interface` is even though the code right before it uses a wrapper to handle `newThing` correctly. To sidestep this issue, we need to be able to `constexpr` evaluate what field is being (de)serialized and decide the (de)serialization logic for it accordingly, which will _exclude_ all other logic that doesn't apply _at compile time_. The current macro-based solution doesn't allow for that. While `std::tuple` allows for storing a heterogenous collection of elements, iterating through it proves to be difficult. `std::apply` proves to be too restrictive for what we need to do ([source](https://stackoverflow.com/a/54053084)) and the capability needed to do this properly, "expansion statements" could be available in C++26 ([source](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2996r0.html)), which is a long time from now. So, our best option is to use a library that specializes in working with heterogenous collections and thankfully, such a library is already available in Boost called Hana ([source](https://www.boost.org/doc/libs/1_81_0/libs/hana/doc/html/index.html)) and it is headers-only ([source](https://www.boost.org/doc/libs/1_81_0/more/getting_started/unix-variants.html#header-only-libraries), list of all libraries that need building, Hana is not on the list) and is therefore, already available to us. ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 0073e9a Tree-SHA512: c0b7e4214e6180f755e6d030bc24f7cb0d5ab0d5fb262a6394bc17d60bf4de0be9148cd0b07cd00f5e78c1b07c156d65c54c25c2ac6fa49bf7d2ae9a406650a8
2 parents 26f438f + 0073e9a commit 90ea1da

File tree

2 files changed

+69
-37
lines changed

2 files changed

+69
-37
lines changed

‎src/evo/dmnstate.h‎

Lines changed: 67 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
#include <memory>
1616
#include <utility>
1717

18+
#include <boost/hana/for_each.hpp>
19+
#include <boost/hana/tuple.hpp>
20+
1821
class CProRegTx;
1922
class UniValue;
2023

@@ -173,26 +176,38 @@ class CDeterministicMNStateDiff
173176
Field_nVersion = 0x40000,
174177
};
175178

176-
#define DMN_STATE_DIFF_ALL_FIELDS \
177-
DMN_STATE_DIFF_LINE(nRegisteredHeight) \
178-
DMN_STATE_DIFF_LINE(nLastPaidHeight) \
179-
DMN_STATE_DIFF_LINE(nPoSePenalty) \
180-
DMN_STATE_DIFF_LINE(nPoSeRevivedHeight) \
181-
DMN_STATE_DIFF_LINE(nPoSeBanHeight) \
182-
DMN_STATE_DIFF_LINE(nRevocationReason) \
183-
DMN_STATE_DIFF_LINE(confirmedHash) \
184-
DMN_STATE_DIFF_LINE(confirmedHashWithProRegTxHash) \
185-
DMN_STATE_DIFF_LINE(keyIDOwner) \
186-
DMN_STATE_DIFF_LINE(pubKeyOperator) \
187-
DMN_STATE_DIFF_LINE(keyIDVoting) \
188-
DMN_STATE_DIFF_LINE(addr) \
189-
DMN_STATE_DIFF_LINE(scriptPayout) \
190-
DMN_STATE_DIFF_LINE(scriptOperatorPayout) \
191-
DMN_STATE_DIFF_LINE(nConsecutivePayments) \
192-
DMN_STATE_DIFF_LINE(platformNodeID) \
193-
DMN_STATE_DIFF_LINE(platformP2PPort) \
194-
DMN_STATE_DIFF_LINE(platformHTTPPort) \
195-
DMN_STATE_DIFF_LINE(nVersion)
179+
private:
180+
template <auto CDeterministicMNState::*Field, uint32_t Mask>
181+
struct Member
182+
{
183+
static constexpr uint32_t mask = Mask;
184+
static auto& get(CDeterministicMNState& state) { return state.*Field; }
185+
static const auto& get(const CDeterministicMNState& state) { return state.*Field; }
186+
};
187+
188+
#define DMN_STATE_MEMBER(name) Member<&CDeterministicMNState::name, Field_##name>{}
189+
static constexpr auto members = boost::hana::make_tuple(
190+
DMN_STATE_MEMBER(nRegisteredHeight),
191+
DMN_STATE_MEMBER(nLastPaidHeight),
192+
DMN_STATE_MEMBER(nPoSePenalty),
193+
DMN_STATE_MEMBER(nPoSeRevivedHeight),
194+
DMN_STATE_MEMBER(nPoSeBanHeight),
195+
DMN_STATE_MEMBER(nRevocationReason),
196+
DMN_STATE_MEMBER(confirmedHash),
197+
DMN_STATE_MEMBER(confirmedHashWithProRegTxHash),
198+
DMN_STATE_MEMBER(keyIDOwner),
199+
DMN_STATE_MEMBER(pubKeyOperator),
200+
DMN_STATE_MEMBER(keyIDVoting),
201+
DMN_STATE_MEMBER(addr),
202+
DMN_STATE_MEMBER(scriptPayout),
203+
DMN_STATE_MEMBER(scriptOperatorPayout),
204+
DMN_STATE_MEMBER(nConsecutivePayments),
205+
DMN_STATE_MEMBER(platformNodeID),
206+
DMN_STATE_MEMBER(platformP2PPort),
207+
DMN_STATE_MEMBER(platformHTTPPort),
208+
DMN_STATE_MEMBER(nVersion)
209+
);
210+
#undef DMN_STATE_MEMBER
196211

197212
public:
198213
uint32_t fields{0};
@@ -203,27 +218,41 @@ class CDeterministicMNStateDiff
203218
CDeterministicMNStateDiff() = default;
204219
CDeterministicMNStateDiff(const CDeterministicMNState& a, const CDeterministicMNState& b)
205220
{
206-
#define DMN_STATE_DIFF_LINE(f) if (a.f != b.f) { state.f = b.f; fields |= Field_##f; }
207-
DMN_STATE_DIFF_ALL_FIELDS
208-
#undef DMN_STATE_DIFF_LINE
209-
if (fields & Field_pubKeyOperator) { state.nVersion = b.nVersion; fields |= Field_nVersion; }
221+
boost::hana::for_each(members, [&](auto&& member) {
222+
if (member.get(a) != member.get(b)) {
223+
member.get(state) = member.get(b);
224+
fields |= member.mask;
225+
}
226+
});
227+
if (fields & Field_pubKeyOperator) {
228+
// pubKeyOperator needs nVersion
229+
state.nVersion = b.nVersion;
230+
fields |= Field_nVersion;
231+
}
210232
}
211233

212234
[[nodiscard]] UniValue ToJson(MnType nType) const;
213235

214236
SERIALIZE_METHODS(CDeterministicMNStateDiff, obj)
215237
{
238+
READWRITE(VARINT(obj.fields));
239+
216240
// NOTE: reading pubKeyOperator requires nVersion
217241
bool read_pubkey{false};
218-
READWRITE(VARINT(obj.fields));
219-
#define DMN_STATE_DIFF_LINE(f) \
220-
if (strcmp(#f, "pubKeyOperator") == 0 && (obj.fields & Field_pubKeyOperator)) {\
221-
SER_READ(obj, read_pubkey = true); \
222-
READWRITE(CBLSLazyPublicKeyVersionWrapper(const_cast<CBLSLazyPublicKey&>(obj.state.pubKeyOperator), obj.state.nVersion == ProTxVersion::LegacyBLS)); \
223-
} else if (obj.fields & Field_##f) READWRITE(obj.state.f);
224-
225-
DMN_STATE_DIFF_ALL_FIELDS
226-
#undef DMN_STATE_DIFF_LINE
242+
boost::hana::for_each(members, [&](auto&& member) {
243+
using BaseType = std::decay_t<decltype(member)>;
244+
if constexpr (BaseType::mask == Field_pubKeyOperator) {
245+
if (obj.fields & member.mask) {
246+
SER_READ(obj, read_pubkey = true);
247+
READWRITE(CBLSLazyPublicKeyVersionWrapper(const_cast<CBLSLazyPublicKey&>(obj.state.pubKeyOperator), obj.state.nVersion == ProTxVersion::LegacyBLS));
248+
}
249+
} else {
250+
if (obj.fields & member.mask) {
251+
READWRITE(member.get(obj.state));
252+
}
253+
}
254+
});
255+
227256
if (read_pubkey) {
228257
SER_READ(obj, obj.fields |= Field_nVersion);
229258
SER_READ(obj, obj.state.pubKeyOperator.SetLegacy(obj.state.nVersion == ProTxVersion::LegacyBLS));
@@ -232,11 +261,12 @@ class CDeterministicMNStateDiff
232261

233262
void ApplyToState(CDeterministicMNState& target) const
234263
{
235-
#define DMN_STATE_DIFF_LINE(f) if (fields & Field_##f) target.f = state.f;
236-
DMN_STATE_DIFF_ALL_FIELDS
237-
#undef DMN_STATE_DIFF_LINE
264+
boost::hana::for_each(members, [&](auto&& member) {
265+
if (fields & member.mask) {
266+
member.get(target) = member.get(state);
267+
}
268+
});
238269
}
239270
};
240271

241-
242272
#endif // BITCOIN_EVO_DMNSTATE_H

‎test/lint/lint-includes.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
"src/crypto/x11/"]
2626

2727
EXPECTED_BOOST_INCLUDES = ["boost/date_time/posix_time/posix_time.hpp",
28+
"boost/hana/for_each.hpp",
29+
"boost/hana/tuple.hpp",
2830
"boost/multi_index/hashed_index.hpp",
2931
"boost/multi_index/identity.hpp",
3032
"boost/multi_index/indexed_by.hpp",

0 commit comments

Comments
 (0)