Skip to content

Commit 96554c8

Browse files
authored
refactor: removed flag checkMalleable from bls's related code (#5462)
## Issue being fixed or feature implemented Current implementation of BLS wrapper has an unclear interface related to `checkMalleable` flag. There are 2 methods Unserialize, that has both default arguments: ``` template inline void Unserialize(Stream& s, const bool specificLegacyScheme, bool checkMalleable = true); template inline void Unserialize(Stream& s, bool checkMalleable = true); ``` Let's assume that I am calling `Unserialize(s, true)` - it's very non-obvious which one will be called and not error prune at all. It should be re-implemented, and there should not be default argument. Pasta noticed that this flag can be useful from performance point of view - let's have better new method such as `UnserializeNoMalleable` or similar and use it when reindexing/etc. It should be specified explicit. Reverting this change and adding new interface in future won't be difficult task so far as changes are quite trivial. ## What was done? Removed flag checkMalleable to simplify code because it's always true. It splits from #5443 ## How Has This Been Tested? Run unit functional tests. ## Breaking Changes No breaking changes - flag is always true. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone
1 parent bf284e3 commit 96554c8

File tree

2 files changed

+10
-14
lines changed

2 files changed

+10
-14
lines changed

src/bls/bls.h

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,13 @@ class CBLSWrapper
173173
}
174174

175175
template <typename Stream>
176-
inline void Unserialize(Stream& s, const bool specificLegacyScheme, bool checkMalleable = true)
176+
inline void Unserialize(Stream& s, const bool specificLegacyScheme)
177177
{
178178
std::vector<uint8_t> vecBytes(SerSize, 0);
179179
s.read(reinterpret_cast<char*>(vecBytes.data()), SerSize);
180180
SetByteVector(vecBytes, specificLegacyScheme);
181181

182-
if (checkMalleable && !CheckMalleable(vecBytes, specificLegacyScheme)) {
182+
if (!CheckMalleable(vecBytes, specificLegacyScheme)) {
183183
// If CheckMalleable failed with specificLegacyScheme, we need to try again with the opposite scheme.
184184
// Probably we received the BLS object sent with legacy scheme, but in the meanwhile the fork activated.
185185
SetByteVector(vecBytes, !specificLegacyScheme);
@@ -195,9 +195,9 @@ class CBLSWrapper
195195
}
196196

197197
template <typename Stream>
198-
inline void Unserialize(Stream& s, bool checkMalleable = true)
198+
inline void Unserialize(Stream& s)
199199
{
200-
Unserialize(s, bls::bls_legacy_scheme.load(), checkMalleable);
200+
Unserialize(s, bls::bls_legacy_scheme.load());
201201
}
202202

203203
inline bool CheckMalleable(Span<uint8_t> vecBytes, const bool specificLegacyScheme) const
@@ -323,20 +323,18 @@ class CBLSPublicKeyVersionWrapper {
323323
private:
324324
CBLSPublicKey& obj;
325325
bool legacy;
326-
bool checkMalleable;
327326
public:
328-
CBLSPublicKeyVersionWrapper(CBLSPublicKey& obj, bool legacy, bool checkMalleable = true)
327+
CBLSPublicKeyVersionWrapper(CBLSPublicKey& obj, bool legacy)
329328
: obj(obj)
330329
, legacy(legacy)
331-
, checkMalleable(checkMalleable)
332330
{}
333331
template <typename Stream>
334332
inline void Serialize(Stream& s) const {
335333
obj.Serialize(s, legacy);
336334
}
337335
template <typename Stream>
338336
inline void Unserialize(Stream& s) {
339-
obj.Unserialize(s, legacy, checkMalleable);
337+
obj.Unserialize(s, legacy);
340338
}
341339
};
342340

@@ -371,20 +369,18 @@ class CBLSSignatureVersionWrapper {
371369
private:
372370
CBLSSignature& obj;
373371
bool legacy;
374-
bool checkMalleable;
375372
public:
376-
CBLSSignatureVersionWrapper(CBLSSignature& obj, bool legacy, bool checkMalleable = true)
373+
CBLSSignatureVersionWrapper(CBLSSignature& obj, bool legacy)
377374
: obj(obj)
378375
, legacy(legacy)
379-
, checkMalleable(checkMalleable)
380376
{}
381377
template <typename Stream>
382378
inline void Serialize(Stream& s) const {
383379
obj.Serialize(s, legacy);
384380
}
385381
template <typename Stream>
386382
inline void Unserialize(Stream& s) {
387-
obj.Unserialize(s, legacy, checkMalleable);
383+
obj.Unserialize(s, legacy);
388384
}
389385
};
390386

src/evo/providertx.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ class CProUpServTx
167167
}
168168
if (!(s.GetType() & SER_GETHASH)) {
169169
READWRITE(
170-
CBLSSignatureVersionWrapper(const_cast<CBLSSignature&>(obj.sig), (obj.nVersion == LEGACY_BLS_VERSION), true)
170+
CBLSSignatureVersionWrapper(const_cast<CBLSSignature&>(obj.sig), (obj.nVersion == LEGACY_BLS_VERSION))
171171
);
172172
}
173173
}
@@ -305,7 +305,7 @@ class CProUpRevTx
305305
);
306306
if (!(s.GetType() & SER_GETHASH)) {
307307
READWRITE(
308-
CBLSSignatureVersionWrapper(const_cast<CBLSSignature&>(obj.sig), (obj.nVersion == LEGACY_BLS_VERSION), true)
308+
CBLSSignatureVersionWrapper(const_cast<CBLSSignature&>(obj.sig), (obj.nVersion == LEGACY_BLS_VERSION))
309309
);
310310
}
311311
}

0 commit comments

Comments
 (0)