SHAMap fixups:

* Change state_ to Invalid if inner node types mismatch
* flushDirty before unsharing to ensure modified nodes get written
* Remove unnecessary code
* Fix updateHash for V2 inner nodes
* In descend, call isInconsistentNode only if node is found
* getMissingNodes could request duplicates in some cases
* An invalid node with the right hash is useful, it proves the map invalid
This commit is contained in:
JoelKatz
2016-07-06 10:59:51 -07:00
committed by Nik Bougalis
parent a5589dcec6
commit 4758050444
7 changed files with 117 additions and 114 deletions

View File

@@ -84,7 +84,7 @@ private:
std::uint32_t seq_;
std::uint32_t ledgerSeq_ = 0; // sequence number of ledger this is part of
std::shared_ptr<SHAMapAbstractNode> root_;
SHAMapState state_;
mutable SHAMapState state_;
SHAMapType type_;
bool backed_ = true; // Map is backed by the database
@@ -372,6 +372,7 @@ inline
bool
SHAMap::is_v2() const
{
assert (root_);
return std::dynamic_pointer_cast<SHAMapInnerNodeV2>(root_) != nullptr;
}

View File

@@ -208,6 +208,7 @@ public:
bool has_common_prefix(uint256 const& key) const;
int get_common_prefix(uint256 const& key) const;
void set_common(int depth, uint256 const& key);
bool updateHash () override;
void addRaw(Serializer& s, SHANodeFormat format) const override;
uint256 const& key() const override;
void setChildren(std::shared_ptr<SHAMapTreeNode> const& child1,

View File

@@ -111,8 +111,8 @@ SHAMap::make_v2() const
t = hotUNKNOWN;
break;
}
ret->unshare();
ret->flushDirty(t, ret->seq_);
ret->unshare();
return ret;
}
@@ -144,8 +144,8 @@ SHAMap::make_v1() const
t = hotUNKNOWN;
break;
}
ret->unshare();
ret->flushDirty(t, ret->seq_);
ret->unshare();
return ret;
}
@@ -184,12 +184,13 @@ SHAMap::walkTowardsKey(uint256 const& id, SharedPtrNodeStack* stack) const
assert(stack == nullptr || stack->empty());
auto inNode = root_;
SHAMapNodeID nodeID;
if (stack != nullptr)
stack->push({inNode, nodeID});
auto const isv2 = is_v2();
while (inNode->isInner())
{
if (stack != nullptr)
stack->push({inNode, nodeID});
auto const inner = std::static_pointer_cast<SHAMapInnerNode>(std::move(inNode));
if (isv2)
{
@@ -208,7 +209,10 @@ SHAMap::walkTowardsKey(uint256 const& id, SharedPtrNodeStack* stack) const
{
auto n = std::dynamic_pointer_cast<SHAMapInnerNodeV2>(inNode);
if (n == nullptr)
{
assert (false);
return nullptr;
}
nodeID = SHAMapNodeID{n->depth(), n->common()};
}
else
@@ -220,9 +224,10 @@ SHAMap::walkTowardsKey(uint256 const& id, SharedPtrNodeStack* stack) const
{
nodeID = nodeID.getChildNodeID (branch);
}
if (stack != nullptr)
stack->push({inNode, nodeID});
}
if (stack != nullptr)
stack->push({inNode, nodeID});
return static_cast<SHAMapTreeNode*>(inNode.get());
}
@@ -305,26 +310,6 @@ SHAMap::checkFilter(SHAMapHash const& hash,
{
node = SHAMapAbstractNode::make(
nodeData, 0, snfPREFIX, hash, true, f_.journal ());
if (node && node->isInner())
{
bool isv2 = std::dynamic_pointer_cast<SHAMapInnerNodeV2>(node) != nullptr;
if (isv2 != is_v2())
{
auto root = std::dynamic_pointer_cast<SHAMapInnerNode>(root_);
assert(root);
assert(root->isEmpty());
if (isv2)
{
auto temp = make_v2();
swap(temp->root_, const_cast<std::shared_ptr<SHAMapAbstractNode>&>(root_));
}
else
{
auto temp = make_v1();
swap(temp->root_, const_cast<std::shared_ptr<SHAMapAbstractNode>&>(root_));
}
}
}
if (node)
{
filter->gotNode (true, hash,
@@ -458,14 +443,15 @@ SHAMap::descend (SHAMapInnerNode * parent, SHAMapNodeID const& parentID,
if (!child)
{
std::shared_ptr<SHAMapAbstractNode> childNode = fetchNodeNT (childHash, filter);
if (isInconsistentNode(childNode))
childNode = nullptr;
if (childNode)
{
childNode = parent->canonicalizeChild (branch, std::move(childNode));
child = childNode.get ();
}
if (child && isInconsistentNode(childNode))
child = nullptr;
}
if (child && is_v2())
@@ -512,21 +498,12 @@ SHAMap::descendAsync (SHAMapInnerNode* parent, int branch,
ptr = SHAMapAbstractNode::make(obj->getData(), 0, snfPREFIX,
hash, true, f_.journal());
if (ptr && ptr->isInner())
{
bool isv2 = std::dynamic_pointer_cast<SHAMapInnerNodeV2>(ptr) != nullptr;
if (isv2 != is_v2())
{
assert(false);
}
}
if (ptr && backed_)
canonicalize (hash, ptr);
}
}
if (isInconsistentNode(ptr))
if (ptr && isInconsistentNode(ptr))
ptr = nullptr;
if (ptr)
ptr = parent->canonicalizeChild (branch, std::move(ptr));
@@ -955,7 +932,7 @@ SHAMap::addGiveItem (std::shared_ptr<SHAMapItem const> const& item,
auto parent_depth = parent->depth();
auto depth = inner->get_common_prefix(tag);
auto new_inner = std::make_shared<SHAMapInnerNodeV2>(seq_);
auto nodeID = SHAMapNodeID{depth, prefix(depth, inner->common())};
nodeID = SHAMapNodeID{depth, prefix(depth, inner->common())};
new_inner->setChild(nodeID.selectBranch(inner->common()), inner);
nodeID = SHAMapNodeID{depth, prefix(depth, tag)};
new_inner->setChild(nodeID.selectBranch(tag),
@@ -1394,10 +1371,18 @@ SHAMap::invariants() const
bool
SHAMap::isInconsistentNode(std::shared_ptr<SHAMapAbstractNode> const& node) const
{
assert(root_);
assert(node);
if (std::dynamic_pointer_cast<SHAMapTreeNode>(node) != nullptr)
return false;
bool is_node_v2 = std::dynamic_pointer_cast<SHAMapInnerNodeV2>(node) != nullptr;
return is_v2() != is_node_v2;
assert (! is_node_v2 || (std::dynamic_pointer_cast<SHAMapInnerNodeV2>(node)->depth() != 0));
if (is_v2() == is_node_v2)
return false;
state_ = SHAMapState::Invalid;
return true;
}
} // ripple

View File

@@ -183,6 +183,7 @@ SHAMap::getMissingNodes(std::size_t max, SHAMapSyncFilter* filter)
{
if (!pending)
{ // node is not in the database
missingHashes.insert (childHash);
ret.emplace_back (
childID,
childHash.as_uint256());
@@ -254,15 +255,15 @@ SHAMap::getMissingNodes(std::size_t max, SHAMapSyncFilter* filter)
// Process all deferred reads
int hits = 0;
for (auto const& node : deferredReads)
for (auto const& deferredNode : deferredReads)
{
auto parent = std::get<0>(node);
auto branch = std::get<1>(node);
auto const& nodeID = std::get<2>(node);
auto parent = std::get<0>(deferredNode);
auto branch = std::get<1>(deferredNode);
auto const& deferredNodeID = std::get<2>(deferredNode);
auto const& nodeHash = parent->getChildHash (branch);
auto nodePtr = fetchNodeNT(nodeHash, filter);
if (nodePtr && !isInconsistentNode(nodePtr))
if (nodePtr)
{
++hits;
if (backed_)
@@ -273,7 +274,7 @@ SHAMap::getMissingNodes(std::size_t max, SHAMapSyncFilter* filter)
{
ret.push_back (
std::make_pair (
nodeID,
deferredNodeID,
nodeHash.as_uint256()));
--max;
@@ -401,10 +402,10 @@ bool SHAMap::getNodeFat (SHAMapNodeID wanted,
else if (childNode->isInner() || fatLeaves)
{
// Just include this node
Serializer s;
childNode->addRaw (s, snfWIRE);
Serializer ns;
childNode->addRaw (ns, snfWIRE);
nodeIDs.push_back (childID);
rawNodes.push_back (std::move (s.peekData ()));
rawNodes.push_back (std::move (ns.peekData ()));
}
}
}
@@ -498,18 +499,6 @@ SHAMap::addKnownNode (const SHAMapNodeID& node, Blob const& rawNode,
if (iNode == nullptr)
{
if ((std::dynamic_pointer_cast<SHAMapInnerNodeV2>(newNode) && !iNodeID.has_common_prefix(node)) ||
(!std::dynamic_pointer_cast<SHAMapInnerNodeV2>(newNode) && iNodeID != node))
{
// Either this node is broken or we didn't request it (yet)
JLOG(journal_.warn()) << "unable to hook node " << node;
JLOG(journal_.info()) << " stuck at " << iNodeID;
JLOG(journal_.info()) <<
"got depth=" << node.getDepth () <<
", walked to= " << iNodeID.getDepth ();
return SHAMapAddNode::invalid ();
}
if (!newNode || !newNode->isValid() || childHash != newNode->getNodeHash ())
{
JLOG(journal_.warn()) << "Corrupt node received";
@@ -525,16 +514,25 @@ SHAMap::addKnownNode (const SHAMapNodeID& node, Blob const& rawNode,
if (newNode && isInconsistentNode(newNode))
{
return SHAMapAddNode::invalid();
state_ = SHAMapState::Invalid;
return SHAMapAddNode::useful();
}
if ((std::dynamic_pointer_cast<SHAMapInnerNodeV2>(newNode) && !iNodeID.has_common_prefix(node)) ||
(!std::dynamic_pointer_cast<SHAMapInnerNodeV2>(newNode) && iNodeID != node))
{
// Either this node is broken or we didn't request it (yet)
JLOG(journal_.warn()) << "unable to hook node " << node;
JLOG(journal_.info()) << " stuck at " << iNodeID;
JLOG(journal_.info()) <<
"got depth=" << node.getDepth () <<
", walked to= " << iNodeID.getDepth ();
return SHAMapAddNode::useful ();
}
if (backed_)
{
auto temp = newNode;
canonicalize (childHash, newNode);
if (isInconsistentNode(newNode))
newNode = temp;
}
newNode = prevNode->canonicalizeChild (branch, std::move(newNode));
if (filter)

View File

@@ -202,7 +202,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f
}
else if (type == 5)
{
// full inner
// full v2 inner
if (len != 512)
Throw<std::runtime_error> ("invalid FI node");
@@ -224,7 +224,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f
else if (type == 6)
{
auto ret = std::make_shared<SHAMapInnerNodeV2>(seq);
// compressed inner
// compressed v2 inner
for (int i = 0; i < (len / 33); ++i)
{
int pos;
@@ -296,7 +296,7 @@ SHAMapAbstractNode::make(Blob const& rawNode, std::uint32_t seq, SHANodeFormat f
auto len = s.getLength();
bool isV2 = (prefix == HashPrefix::innerNodeV2);
if ((len < 512) || (!isV2 && (len != 512)))
if ((len < 512) || (!isV2 && (len != 512)) || (isV2 && (len == 512)))
Throw<std::runtime_error> ("invalid PIN node");
std::shared_ptr<SHAMapInnerNode> ret;
@@ -498,6 +498,24 @@ SHAMapInnerNodeV2::addRaw(Serializer& s, SHANodeFormat format) const
}
}
bool
SHAMapInnerNodeV2::updateHash()
{
uint256 nh;
if (mIsBranch != 0)
{
Serializer s(580);
addRaw (s, snfPREFIX);
nh = s.getSHA512Half();
}
if (nh == mHash.as_uint256())
return false;
mHash = SHAMapHash{nh};
return true;
}
void
SHAMapTreeNode::addRaw(Serializer& s, SHANodeFormat format) const
{

View File

@@ -224,14 +224,14 @@ public:
std::vector<uint256> hashes(8);
if (is_v2)
{
hashes[0].SetHex ("B7387CFEA0465759ADC718E8C42B52D2309D179B326E239EB5075C64B6281F7F");
hashes[1].SetHex ("6A70885D21024F9F4F50E688B365D0E017266F53AE3E77B52AAEF84E167FE942");
hashes[2].SetHex ("BA635322BDF510CCFC0578C194C1F87DA2D97C1A55A469F6E7A463BE963663D7");
hashes[3].SetHex ("5F751361AC7A161DED4D6EAAC4B587C7C01E50E1F9EC3DD61207BD6B196E7DB1");
hashes[4].SetHex ("FC1C57DD4BF15E37961E0B3064C43E60A9DC26EA332D7A6178FE2284901DB49F");
hashes[5].SetHex ("4FCDFE944E8E19E35FF60E7BDA7DB21B1CD99670BCF158FEF8F0F8B49BF5C9AD");
hashes[6].SetHex ("31F6DE8152FBE77EAD59805631FCDDB71F1BC6A5E9BD8FA3948D82D1CE1F93D6");
hashes[7].SetHex ("00895AD3B161D483C4EF7B5469B0D305685222B5102C2C3F614FD84AD50C4B14");
hashes[0].SetHex ("90F77DA53895E34042DC8048518CC98AD24276D0A96CCA2C515A83FDAF9F9FC9");
hashes[1].SetHex ("425A3B6A68FAD9CB43B9981C7D0D39B942FE62110B437201057EE703F5E76390");
hashes[2].SetHex ("1B4BE72DD18F90F367D64C0147D2414329149724339F79958D6470E7C99E3F4A");
hashes[3].SetHex ("CCC18ED9B0C353278F02465E2E2F3A8A07427B458CF74C51D87ABE9C1B2ECAD8");
hashes[4].SetHex ("24AF98675227F387CE0E4932B71B099FE8BC66E5F07BE2DA70D7E7D98E16C8BC");
hashes[5].SetHex ("EAA373271474A9BF18F1CC240B40C7B5C83C7017977F1388771E56D5943F2B9B");
hashes[6].SetHex ("C7968A323A06BD46769B402B2A85A7FE7F37FCE99C0004A6197AD8E5D76F200D");
hashes[7].SetHex ("0A2412DBB16308706211E5FA5B0160817D54757B4DDC0CB105391A79D06B47BA");
}
else
{
@@ -250,11 +250,11 @@ public:
map.setUnbacked ();
BEAST_EXPECT(map.getHash() == zero);
for (int i = 0; i < keys.size(); ++i)
for (int k = 0; k < keys.size(); ++k)
{
SHAMapItem item (keys[i], IntToVUC (i));
SHAMapItem item (keys[k], IntToVUC (k));
BEAST_EXPECT(map.addItem (std::move(item), true, false));
BEAST_EXPECT(map.getHash().as_uint256() == hashes[i]);
BEAST_EXPECT(map.getHash().as_uint256() == hashes[k]);
map.invariants();
}
if (v == SHAMap::version{1})
@@ -264,21 +264,21 @@ public:
BEAST_EXPECT(map_v2 != nullptr);
BEAST_EXPECT(map_v2->is_v2());
map_v2->invariants();
auto i1 = map.begin();
auto m1 = map.begin();
auto e1 = map.end();
auto i2 = map_v2->begin();
auto m2 = map_v2->begin();
auto e2 = map_v2->end();
for (; i1 != e1; ++i1, ++i2)
for (; m1 != e1; ++m1, ++m2)
{
BEAST_EXPECT(i2 != e2);
BEAST_EXPECT(*i1 == *i2);
BEAST_EXPECT(m2 != e2);
BEAST_EXPECT(*m1 == *m2);
}
BEAST_EXPECT(i2 == e2);
BEAST_EXPECT(m2 == e2);
}
for (int i = keys.size() - 1; i >= 0; --i)
for (int k = keys.size() - 1; k >= 0; --k)
{
BEAST_EXPECT(map.getHash().as_uint256() == hashes[i]);
BEAST_EXPECT(map.delItem (keys[i]));
BEAST_EXPECT(map.getHash().as_uint256() == hashes[k]);
BEAST_EXPECT(map.delItem (keys[k]));
map.invariants();
}
BEAST_EXPECT(map.getHash() == zero);
@@ -300,8 +300,8 @@ public:
keys[6].SetHex ("b91891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8");
keys[7].SetHex ("292891fe4ef6cee585fdc6fda1e09eb4d386363158ec3321b8123e5a772c6ca8");
tests::TestFamily f{beast::Journal{}};
SHAMap map{SHAMapType::FREE, f, v};
tests::TestFamily tf{beast::Journal{}};
SHAMap map{SHAMapType::FREE, tf, v};
if (! backed)
map.setUnbacked ();
for (auto const& k : keys)
@@ -310,11 +310,11 @@ public:
map.invariants();
}
int i = 7;
int h = 7;
for (auto const& k : map)
{
BEAST_EXPECT(k.key() == keys[i]);
--i;
BEAST_EXPECT(k.key() == keys[h]);
--h;
}
}
}

View File

@@ -94,9 +94,9 @@ public:
void run(SHAMap::version v)
{
beast::Journal const j; // debug journal
TestFamily f(j);
TestFamily f(j), f2(j);
SHAMap source (SHAMapType::FREE, f, v);
SHAMap destination (SHAMapType::FREE, f, v);
SHAMap destination (SHAMapType::FREE, f2, v);
int items = 10000;
for (int i = 0; i < items; ++i)
@@ -133,21 +133,21 @@ public:
destination.setSynching ();
{
std::vector<SHAMapNodeID> gotNodeIDs;
std::vector<Blob> gotNodes;
std::vector<SHAMapNodeID> gotNodeIDs_a;
std::vector<Blob> gotNodes_a;
BEAST_EXPECT(source.getNodeFat (
SHAMapNodeID (),
gotNodeIDs,
gotNodes,
gotNodeIDs_a,
gotNodes_a,
rand_bool(),
rand_int(2)));
unexpected (gotNodes.size () < 1, "NodeSize");
unexpected (gotNodes_a.size () < 1, "NodeSize");
BEAST_EXPECT(destination.addRootNode (
source.getHash(),
*gotNodes.begin (),
*gotNodes_a.begin (),
snfWIRE,
nullptr).isGood());
}
@@ -163,29 +163,29 @@ public:
break;
// get as many nodes as possible based on this information
std::vector<SHAMapNodeID> gotNodeIDs;
std::vector<Blob> gotNodes;
std::vector<SHAMapNodeID> gotNodeIDs_b;
std::vector<Blob> gotNodes_b;
for (auto& it : nodesMissing)
{
BEAST_EXPECT(source.getNodeFat (
it.first,
gotNodeIDs,
gotNodes,
gotNodeIDs_b,
gotNodes_b,
rand_bool(),
rand_int(2)));
}
BEAST_EXPECT(gotNodeIDs.size () == gotNodes.size ());
BEAST_EXPECT(!gotNodeIDs.empty ());
BEAST_EXPECT(gotNodeIDs_b.size () == gotNodes_b.size ());
BEAST_EXPECT(!gotNodeIDs_b.empty ());
for (std::size_t i = 0; i < gotNodeIDs.size(); ++i)
for (std::size_t i = 0; i < gotNodeIDs_b.size(); ++i)
{
BEAST_EXPECT(
destination.addKnownNode (
gotNodeIDs[i],
gotNodes[i],
nullptr).isGood ());
gotNodeIDs_b[i],
gotNodes_b[i],
nullptr).isUseful ());
}
}
while (true);