Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve full & compressed inner node deserialization #4004

Closed
wants to merge 2 commits into from
Closed

Improve full & compressed inner node deserialization #4004

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c85fa85
Improve full & compressed inner node deserialization
nbougalis Nov 27, 2021
1c10c9e
[FOLD] signer numbers are from Mars, unsigned numbers are from Venus
nbougalis Dec 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 23 additions & 17 deletions src/ripple/shamap/impl/SHAMapInnerNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,21 @@ SHAMapInnerNode::makeFullInner(
SHAMapHash const& hash,
bool hashValid)
{
if (data.size() != 512)
// A full inner node is serialized as 16 256-bit hashes, back to back:
if (data.size() != branchFactor * uint256::bytes)
Throw<std::runtime_error>("Invalid FI node");

auto ret = std::make_shared<SHAMapInnerNode>(0, branchFactor);

Serializer s(data.data(), data.size());
SerialIter si(data);

auto hashes = ret->hashesAndChildren_.getHashes();

auto retHashes = ret->hashesAndChildren_.getHashes();
for (int i = 0; i < branchFactor; ++i)
{
s.getBitString(retHashes[i].as_uint256(), i * 32);
hashes[i].as_uint256() = si.getBitString<256>();

if (retHashes[i].isNonZero())
if (hashes[i].isNonZero())
ret->isBranch_ |= (1 << i);
}

Expand All @@ -154,39 +156,43 @@ SHAMapInnerNode::makeFullInner(
ret->hash_ = hash;
else
ret->updateHash();

return ret;
}

std::shared_ptr<SHAMapTreeNode>
SHAMapInnerNode::makeCompressedInner(Slice data)
{
Serializer s(data.data(), data.size());
// A compressed inner node is serialized as a series of 33 byte chunks,
// representing a one byte "position" and a 256-bit hash:
constexpr std::size_t chunkSize = uint256::bytes + 1;

int len = s.getLength();
if (auto const s = data.size();
(s % chunkSize != 0) || (s > chunkSize * branchFactor))
Throw<std::runtime_error>("Invalid CI node");

SerialIter si(data);

auto ret = std::make_shared<SHAMapInnerNode>(0, branchFactor);

auto retHashes = ret->hashesAndChildren_.getHashes();
for (int i = 0; i < (len / 33); ++i)
{
int pos;
auto hashes = ret->hashesAndChildren_.getHashes();

if (!s.get8(pos, 32 + (i * 33)))
Throw<std::runtime_error>("short CI node");
while (!si.empty())
{
auto const hash = si.getBitString<256>();
auto const pos = si.get8();

if ((pos < 0) || (pos >= branchFactor))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gcc warns here:

warning: comparison is always false due to limited range of data type [-Wtype-limits]

Yes, I know this isn't new code; I'm not sure why it didn't use to warn, but we probably want to fix the warning anyway. (pos is unsigned, so it can't be less than 0).

Throw<std::runtime_error>("invalid CI node");

s.getBitString(retHashes[pos].as_uint256(), i * 33);
hashes[pos].as_uint256() = hash;

if (retHashes[pos].isNonZero())
if (hashes[pos].isNonZero())
ret->isBranch_ |= (1 << pos);
}

ret->resizeChildArrays(ret->getBranchCount());

ret->updateHash();

return ret;
}

Expand Down