feat: Virtual leaves should be hashed as valid block items #20401
Conversation
Signed-off-by: Nikita Lebedev <nikita.lebedev@limechain.tech>
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #20401 +/- ##
============================================
- Coverage 71.22% 71.19% -0.04%
+ Complexity 24137 24136 -1
============================================
Files 2652 2652
Lines 102279 102287 +8
Branches 10629 10633 +4
============================================
- Hits 72853 72827 -26
- Misses 25452 25476 +24
- Partials 3974 3984 +10
... and 12 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
...sdk/swirlds-virtualmap/src/main/java/com/swirlds/virtualmap/datasource/VirtualLeafBytes.java
Show resolved
Hide resolved
| final Bytes vb = valueBytes(); | ||
| if (vb != null) { | ||
| len += ProtoWriterTools.sizeOfDelimited(FIELD_LEAFRECORD_VALUE, Math.toIntExact(vb.length())); | ||
| final int valueLen = (vb != null) ? Math.toIntExact(vb.length()) : -1; |
There was a problem hiding this comment.
If valueBytes is not null, but empty, it won't be serialized. What about
final Bytes vb = valueBytes();
if (vb != null) {
final int valueLen = Math.toIntExact(vb.length());
if (valueLen > 0) {
innerLen += ProtoWriterTools.sizeOfDelimited(FIELD_LEAFRECORD_VALUE, valueLen);
}
}?
| final Bytes vb = valueBytes(); | ||
| if (vb != null) { | ||
| ProtoWriterTools.writeDelimited(out, FIELD_LEAFRECORD_VALUE, Math.toIntExact(vb.length()), vb::writeTo); | ||
| final int valueLen = (vb != null) ? Math.toIntExact(vb.length()) : -1; |
There was a problem hiding this comment.
Same as my comment above in getSizeInBytesForHashing()
Signed-off-by: Nikita Lebedev <nikita.lebedev@limechain.tech>
# Conflicts: # hedera-state-validator/src/main/java/com/hedera/statevalidation/validators/merkledb/ValidateLeafIndexHalfDiskHashMap.java
| final Bytes vb = valueBytes(); | ||
| if (vb != null) { | ||
| ProtoWriterTools.writeDelimited(out, FIELD_LEAFRECORD_VALUE, Math.toIntExact(vb.length()), vb::writeTo); | ||
| final int valueLen = Math.toIntExact(vb.length()); |
There was a problem hiding this comment.
is this the right handling of null? we do we support null as a valid return from valueBytes() should it just return empty array? how does VirtualMap handle the difference between null values and default values as a zero length byte[] in protobuf is a valid object with all default values.
There was a problem hiding this comment.
Are we testing all the edge cases like null, default etc? Also do we test that written objects are readable with PBJ generated code from schema?
Description:
This PR:
VirtualLeaftoStateItemin thevirtual_map_state.proto;VirtualMapKeyandVirtualMapValuetoStateKeyandStateValue, respectively;VirtualLeafByteshashing bytes to wrap key+value into aOneOf.Related issue(s):
Fixes: #20371
Checklist