SHAMap sync now passes its unit test with the "full below" optimization

disabled. There's a logic error in the implementation.

HashedObject.cpp

@@ -39,6 +39,7 @@ CREATE INDEX ObjectLocate ON CommittedObjects(LedgerIndex, ObjType);
 bool HashedObject::store(HashedObjectType type, uint32 index, const std::vector<unsigned char>& 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("';");

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;
 
+		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(i++ < SHAMapNode::leafDepth);
 	} while(1);
 }

SHAMapSync.cpp

@@ -27,7 +27,6 @@ void SHAMap::getMissingNodes(std::vector<SHAMapNode>& nodeIDs, std::vector<uint2
 		std::cerr << "gMN: popped " << node->getString() << std::endl;
 #endif
 
-		bool all_leaves=true;
 		for(int i=0; i<32; i++)
 		{
 			if(!node->isEmptyBranch(i))
@@ -44,28 +43,26 @@ void SHAMap::getMissingNodes(std::vector<SHAMapNode>& nodeIDs, std::vector<uint2
 					}
 					else
 					{
-					}
 						SHAMapInnerNode::pointer desc=getInner(node->getChildNodeID(i), node->getChildHash(i), false);
 						assert(desc);
 						if(!desc->isFullBelow())
 							stack.push(desc);
 					}
+				}
 				catch(const SHAMapException &x)
 				{
 					if(x!=SHAMapException(MissingNode)) throw(x);
 					if(max-->0)
 					{
 #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<unsigned char>& 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<unsigned char>&
 	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<unsigned char>& 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::vector<unsigned cha
 	if(iNode->getDepth()!=(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::vector<unsigned cha
 		}
 		mLeafByID[node]=leaf;
 		if(mDirtyLeafNodes) (*mDirtyLeafNodes)[node]=leaf;
+
+		// WRITEME: See if our parent(s) are full below
+
 		return true;
 	}
 	
@@ -259,11 +263,16 @@ bool SHAMap::addKnownNode(const SHAMapNode& node, const std::vector<unsigned cha
 	{
 #ifdef DEBUG
 			std::cerr << "inner node fails consistency check" << std::endl;
+			std::cerr << "   Built: " << newNode->getString() << " 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();