From 5888d19f2be83c42304c446d67791936e5f86587 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 1 Feb 2012 19:10:07 -0800 Subject: [PATCH] SHAMap sync now passes its unit test with the "full below" optimization disabled. There's a logic error in the implementation. --- HashedObject.cpp | 2 ++ SHAMap.cpp | 37 +++++++++++++++++++++++-------------- SHAMapSync.cpp | 24 +++++++++++++++++------- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/HashedObject.cpp b/HashedObject.cpp index f530e2950b..8303a9b49f 100644 --- a/HashedObject.cpp +++ b/HashedObject.cpp @@ -39,6 +39,7 @@ CREATE INDEX ObjectLocate ON CommittedObjects(LedgerIndex, ObjType); bool HashedObject::store(HashedObjectType type, uint32 index, const std::vector& data, const uint256& hash) { + if(!theApp->getHashNodeDB()) return true; #ifdef DEBUG Serializer s(data); assert(hash==s.getSHA512Half()); @@ -76,6 +77,7 @@ bool HashedObject::store() const HashedObject::pointer HashedObject::retrieve(const uint256& hash) { + if(!theApp->getHashNodeDB()) return HashedObject::pointer(); std::string sql="SELECT * from CommitedObjects WHERE Hash='"; sql.append(hash.GetHex()); sql.append("';"); diff --git a/SHAMap.cpp b/SHAMap.cpp index aa0e97d61a..8c2d8a132a 100644 --- a/SHAMap.cpp +++ b/SHAMap.cpp @@ -16,9 +16,6 @@ void SHAMap::dirtyUp(const uint256& id) { // walk the tree up from through the inner nodes to the root // update linking hashes and add nodes to dirty list -#ifdef DEBUG - std::cerr << "dirtyUp(" << id.GetHex() << ")" << std::endl; -#endif assert(!mImmutable && !mSynching); SHAMapLeafNode::pointer leaf=checkCacheLeaf(SHAMapNode(SHAMapNode::leafDepth, id)); if(!leaf) throw SHAMapException(MissingNode); @@ -79,12 +76,6 @@ SHAMapInnerNode::pointer SHAMap::checkCacheNode(const SHAMapNode& iNode) SHAMapLeafNode::pointer SHAMap::walkToLeaf(const uint256& id, bool create, bool modify) { // walk down to the leaf that would contain this ID // is leaf node in cache -#ifdef DEBUG - std::cerr << "walkToLeaf(" << id.GetHex() << ")"; - if(create) std::cerr << " create"; - if(modify) std::cerr << " modify"; - std::cerr << std::endl; -#endif SHAMapLeafNode::pointer ln=checkCacheLeaf(SHAMapNode(SHAMapNode::leafDepth, id)); if(ln) return returnLeaf(ln, modify); @@ -135,15 +126,33 @@ SHAMapInnerNode::pointer SHAMap::walkTo(const SHAMapNode& id) if(branch<0) // somehow we got on the wrong branch throw SHAMapException(InvalidNode); if (inNode->isEmptyBranch(branch)) // we know no branches below this one + { +#ifdef DEBUG + std::cerr << "Walk down empty branch" << std::endl; +#endif return inNode; - if(!inNode->isChildLeaf()) // this is the last inner node + } + if(inNode->isChildLeaf()) // this is the last inner node return inNode; - SHAMapInnerNode::pointer next=getInner(inNode->getChildNodeID(branch), inNode->getChildHash(branch), false); - if(!next) // we don't have the next node + try + { + SHAMapInnerNode::pointer next=getInner(inNode->getChildNodeID(branch), inNode->getChildHash(branch), false); + if(!next) // we don't have the next node + { +#ifdef DEBUG + std::cerr << "Unable to find node " << inNode->getChildNodeID(branch).getString() << std::endl; +#endif + return inNode; + } + assert(next->getDepth() == (inNode->getDepth()+1)); + inNode=next; + } + catch (const SHAMapException& x) + { + if(x!=SHAMapException(MissingNode)) throw(x); return inNode; - assert(next->getDepth() == (inNode->getDepth()+1)); - inNode=next; + } assert(i++ < SHAMapNode::leafDepth); } while(1); } diff --git a/SHAMapSync.cpp b/SHAMapSync.cpp index 97f12da3d2..eeda44ae20 100644 --- a/SHAMapSync.cpp +++ b/SHAMapSync.cpp @@ -27,7 +27,6 @@ void SHAMap::getMissingNodes(std::vector& nodeIDs, std::vectorgetString() << std::endl; #endif - bool all_leaves=true; for(int i=0; i<32; i++) { if(!node->isEmptyBranch(i)) @@ -44,11 +43,11 @@ void SHAMap::getMissingNodes(std::vector& nodeIDs, std::vectorgetChildNodeID(i), node->getChildHash(i), false); assert(desc); if(!desc->isFullBelow()) stack.push(desc); + } } catch(const SHAMapException &x) { @@ -56,16 +55,14 @@ void SHAMap::getMissingNodes(std::vector& nodeIDs, std::vector0) { #ifdef DEBUG - std::cerr << "gMN: need leaf " << i << std::endl; + std::cerr << "gMN: need leaf " << node->getChildNodeID(i).getString() << std::endl; #endif nodeIDs.push_back(node->getChildNodeID(i)); hashes.push_back(node->getChildHash(i)); } - all_leaves=false; } } } - if(all_leaves) node->setFullBelow(); } } @@ -164,6 +161,7 @@ bool SHAMap::addRootNode(const std::vector& rootNode) root=node; mInnerNodeByID[*node]=node; if(mDirtyInnerNodes) (*mDirtyInnerNodes)[*node]=node; + if(!root->getNodeHash()) root->setFullBelow(); return true; } @@ -188,12 +186,14 @@ bool SHAMap::addRootNode(const uint256& hash, const std::vector& root=node; mInnerNodeByID[*node]=node; if(mDirtyInnerNodes) (*mDirtyInnerNodes)[*node]=node; + if(!root->getNodeHash()) root->setFullBelow(); return true; } bool SHAMap::addKnownNode(const SHAMapNode& node, const std::vector& rawNode) { // return value: true=okay, false=error assert(!node.isRoot()); + assert(mSynching); boost::recursive_mutex::scoped_lock sl(mLock); @@ -224,7 +224,8 @@ bool SHAMap::addKnownNode(const SHAMapNode& node, const std::vectorgetDepth()!=(node.getDepth()-1)) { // Either this node is broken or we didn't request it #ifdef DEBUG - std::cerr << "got inner node, unable to hook it" << std::endl; + std::cerr << "unable to hook node " << node.getString() << std::endl; + std::cerr << " stuck at " << iNode->getString() << std::endl; std::cerr << "got depth=" << node.getDepth() << ", walked to= " << iNode->getDepth() << std::endl; #endif return false; @@ -251,6 +252,9 @@ bool SHAMap::addKnownNode(const SHAMapNode& node, const std::vectorgetString() << " h=" << newNode->getNodeHash().GetHex() << std::endl; + std::cerr << "Expected: " << node.getString() << " h=" << hash.GetHex() << std::endl; #endif return false; } mInnerNodeByID[node]=newNode; if(mDirtyInnerNodes) (*mDirtyInnerNodes)[node]=newNode; +#ifdef DEBUG + std::cerr << "Hooked: " << node.getString() << std::endl; +#endif return true; } @@ -424,6 +433,7 @@ bool SHAMap::syncTest() assert(false); return false; } + assert(gotNodeIDs.size() == gotNodes.size()); nodeIDs.clear(); hashes.clear(); @@ -445,7 +455,7 @@ bool SHAMap::syncTest() return false; } } - nodeIDs.clear(); + gotNodeIDs.clear(); gotNodes.clear();