Simplify the way we handle validations. Include a signing time instead of a

closing time. Keep only the validation with the most recent signing time.
Sign using network time. This eliminates the ValidationPair nightmare and
makes the logic must easier to understand, increasing confidence that it
does what it's supposed to do.

src/DBInit.cpp

@@ -56,7 +56,7 @@ const char *LedgerDBInit[] = {
 		LedgerHash	CHARACTER(64),					\
 		NodePubKey	CHARACTER(56),					\
 		Flags		BIGINT UNSIGNED,				\
-		CloseTime	BIGINT UNSIGNED,				\
+		SignTime	BIGINT UNSIGNED,				\
 		Signature	BLOB							\
 	);",
 	"CREATE INDEX ValidationByHash ON				\

src/LedgerConsensus.cpp

@@ -1005,7 +1005,7 @@ void LedgerConsensus::accept(const SHAMap::pointer& set)
 	if (mValidating)
 	{
 		SerializedValidation::pointer v = boost::make_shared<SerializedValidation>
-			(newLCLHash, newLCL->getCloseTimeNC(), mValSeed, mProposing);
+			(newLCLHash, theApp->getOPs().getValidationTimeNC(), mValSeed, mProposing);
 		v->setTrusted();
 		Log(lsINFO) << "CNF Val " << newLCLHash;
 		theApp->getValidations().addValidation(v);

src/NetworkOPs.cpp

@@ -25,7 +25,7 @@
 
 NetworkOPs::NetworkOPs(boost::asio::io_service& io_service, LedgerMaster* pLedgerMaster) :
 	mMode(omDISCONNECTED),mNetTimer(io_service), mLedgerMaster(pLedgerMaster), mCloseTimeOffset(0),
-	mLastCloseProposers(0), mLastCloseConvergeTime(LEDGER_IDLE_INTERVAL)
+	mLastCloseProposers(0), mLastCloseConvergeTime(LEDGER_IDLE_INTERVAL), mLastValidationTime(0)
 {
 }
 
@@ -46,6 +46,15 @@ uint32 NetworkOPs::getCloseTimeNC()
 	return iToSeconds(getNetworkTimePT() + boost::posix_time::seconds(mCloseTimeOffset));
 }
 
+uint32 NetworkOPs::getValidationTimeNC()
+{
+	uint32 vt = getNetworkTimeNC();
+	if (vt >= mLastValidationTime)
+		vt = mLastValidationTime + 1;
+	mLastValidationTime = vt;
+	return vt;
+}
+
 void NetworkOPs::closeTimeOffset(int offset)
 {
 	mCloseTimeOffset += offset / 4;

src/NetworkOPs.h

@@ -64,6 +64,7 @@ protected:
 	int									mLastCloseProposers, mLastCloseConvergeTime;
 	uint256								mLastCloseHash;
 	uint32								mLastCloseTime;
+	uint32								mLastValidationTime;
 
 	// XXX Split into more locks.
     boost::interprocess::interprocess_upgradable_mutex	mMonitorLock;
@@ -89,6 +90,7 @@ public:
 	// network information
 	uint32 getNetworkTimeNC();
 	uint32 getCloseTimeNC();
+	uint32 getValidationTimeNC();
 	void closeTimeOffset(int);
 	boost::posix_time::ptime getNetworkTimePT();
 	uint32 getCurrentLedgerID();

src/SerializedObject.h

@@ -92,6 +92,7 @@ enum SOE_Field
 	sfSequence,
 	sfSignature,
 	sfSigningKey,
+	sfSigningTime,
 	sfSourceTag,
 	sfTakerGets,
 	sfTakerPays,

src/SerializedValidation.cpp

@@ -6,7 +6,7 @@
 SOElement SerializedValidation::sValidationFormat[] = {
 	{ sfFlags,			"Flags",			STI_UINT32,		SOE_FLAGS,		0 },
 	{ sfLedgerHash,		"LedgerHash",		STI_HASH256,	SOE_REQUIRED,	0 },
-	{ sfCloseTime,		"CloseTime",		STI_UINT32,		SOE_REQUIRED,	0 },
+	{ sfSigningTime,	"SignTime",			STI_UINT32,		SOE_REQUIRED,	0 },
 	{ sfSigningKey,		"SigningKey",		STI_VL,			SOE_REQUIRED,	0 },
 	{ sfInvalid,		NULL,				STI_DONE,		SOE_NEVER,		-1 },
 };
@@ -19,12 +19,12 @@ SerializedValidation::SerializedValidation(SerializerIterator& sit, bool checkSi
 	if  (checkSignature && !isValid()) throw std::runtime_error("Invalid validation");
 }
 
-SerializedValidation::SerializedValidation(const uint256& ledgerHash, uint32 closeTime,
+SerializedValidation::SerializedValidation(const uint256& ledgerHash, uint32 signTime,
 		const NewcoinAddress& naSeed, bool isFull)
 	: STObject(sValidationFormat), mSignature("Signature"), mTrusted(false)
 {
 	setValueFieldH256(sfLedgerHash, ledgerHash);
-	setValueFieldU32(sfCloseTime, closeTime);
+	setValueFieldU32(sfSigningTime, signTime);
 	if (naSeed.isValid())
 		setValueFieldVL(sfSigningKey, NewcoinAddress::createNodePublic(naSeed).getNodePublic());
 	if (!isFull) setFlag(sFullFlag);
@@ -50,9 +50,9 @@ uint256 SerializedValidation::getLedgerHash() const
 	return getValueFieldH256(sfLedgerHash);
 }
 
-uint32 SerializedValidation::getCloseTime() const
+uint32 SerializedValidation::getSignTime() const
 {
-	return getValueFieldU32(sfCloseTime);
+	return getValueFieldU32(sfSigningTime);
 }
 
 uint32 SerializedValidation::getFlags() const

src/SerializedValidation.h

@@ -23,10 +23,10 @@ public:
 	SerializedValidation(SerializerIterator& sit, bool checkSignature = true);
 	SerializedValidation(const Serializer& s, bool checkSignature = true);
 
-	SerializedValidation(const uint256& ledgerHash, uint32 closeTime, const NewcoinAddress& naSeed, bool isFull);
+	SerializedValidation(const uint256& ledgerHash, uint32 signTime, const NewcoinAddress& naSeed, bool isFull);
 
 	uint256			getLedgerHash()		const;
-	uint32			getCloseTime()		const;
+	uint32			getSignTime()		const;
 	uint32			getFlags()			const;
 	NewcoinAddress  getSignerPublic()	const;
 	bool			isValid()			const;

src/ValidationCollection.cpp

@@ -17,7 +17,7 @@ bool ValidationCollection::addValidation(const SerializedValidation::pointer& va
 	{
 		val->setTrusted();
 		uint32 now = theApp->getOPs().getCloseTimeNC();
-		uint32 valClose = val->getCloseTime();
+		uint32 valClose = val->getSignTime();
 		if ((now > (valClose - LEDGER_EARLY_INTERVAL)) && (now < (valClose + LEDGER_VAL_INTERVAL)))
 			isCurrent = true;
 		else
@@ -34,22 +34,16 @@ bool ValidationCollection::addValidation(const SerializedValidation::pointer& va
 			return false;
 		if (isCurrent)
 		{
-			boost::unordered_map<uint160, ValidationPair>::iterator it = mCurrentValidations.find(node);
-			if ((it == mCurrentValidations.end()) || (!it->second.newest) ||
-				(val->getCloseTime() > it->second.newest->getCloseTime()))
+			boost::unordered_map<uint160, SerializedValidation::pointer>::iterator it = mCurrentValidations.find(node);
+			if (it == mCurrentValidations.end())
+				mCurrentValidations.insert(std::make_pair(node, val));
+			else if (!it->second)
+				it->second = val;
+			else if (val->getSignTime() > it->second->getSignTime())
 			{
-				if (it != mCurrentValidations.end())
-				{
-					if  (it->second.oldest)
-					{
-						mStaleValidations.push_back(it->second.oldest);
-						condWrite();
-					}
-					it->second.oldest = it->second.newest;
-					it->second.newest = val;
-				}
-				else
-					mCurrentValidations.insert(std::make_pair(node, ValidationPair(val)));
+				mStaleValidations.push_back(it->second);
+				it->second = val;
+				condWrite();
 			}
 		}
 	}
@@ -76,7 +70,7 @@ void ValidationCollection::getValidationCount(const uint256& ledger, bool curren
 	trusted = untrusted = 0;
 	boost::mutex::scoped_lock sl(mValidationLock);
 	boost::unordered_map<uint256, ValidationSet>::iterator it = mValidations.find(ledger);
-	uint32 now = theApp->getOPs().getCloseTimeNC();
+	uint32 now = theApp->getOPs().getNetworkTimeNC();
 	if (it != mValidations.end())
 	{
 		for (ValidationSet::iterator vit = it->second.begin(), end = it->second.end(); vit != end; ++vit)
@@ -84,7 +78,7 @@ void ValidationCollection::getValidationCount(const uint256& ledger, bool curren
 			bool isTrusted = vit->second->isTrusted();
 			if (isTrusted && currentOnly)
 			{
-				uint32 closeTime = vit->second->getCloseTime();
+				uint32 closeTime = vit->second->getSignTime();
 				if ((now < (closeTime - LEDGER_EARLY_INTERVAL)) || (now > (closeTime + LEDGER_VAL_INTERVAL)))
 					isTrusted = false;
 				else
@@ -125,10 +119,10 @@ int ValidationCollection::getCurrentValidationCount(uint32 afterTime)
 {
 	int count = 0;
 	boost::mutex::scoped_lock sl(mValidationLock);
-	for (boost::unordered_map<uint160, ValidationPair>::iterator it = mCurrentValidations.begin(),
+	for (boost::unordered_map<uint160, SerializedValidation::pointer>::iterator it = mCurrentValidations.begin(),
 		end = mCurrentValidations.end(); it != end; ++it)
 	{
-		if (it->second.newest->isTrusted() && (it->second.newest->getCloseTime() > afterTime))
+		if (it->second->isTrusted() && (it->second->getSignTime() > afterTime))
 			++count;
 	}
 	return count;
@@ -136,54 +130,26 @@ int ValidationCollection::getCurrentValidationCount(uint32 afterTime)
 
 boost::unordered_map<uint256, int> ValidationCollection::getCurrentValidations()
 {
-    uint32 now = theApp->getOPs().getCloseTimeNC();
+    uint32 cutoff = theApp->getOPs().getNetworkTimeNC() - LEDGER_VAL_INTERVAL;
 	boost::unordered_map<uint256, int> ret;
 
 	{
 		boost::mutex::scoped_lock sl(mValidationLock);
-		boost::unordered_map<uint160, ValidationPair>::iterator it = mCurrentValidations.begin();
+		boost::unordered_map<uint160, SerializedValidation::pointer>::iterator it = mCurrentValidations.begin();
 		while (it != mCurrentValidations.end())
 		{
-			ValidationPair& pair = it->second;
-
-			if (pair.oldest && (now > (pair.oldest->getCloseTime() + LEDGER_VAL_INTERVAL)))
-			{
-#ifdef VC_DEBUG
-				Log(lsINFO) << "VC: " << it->first << " removeOldestStale";
-#endif
-				mStaleValidations.push_back(pair.oldest);
-				pair.oldest = SerializedValidation::pointer();
-				condWrite();
-			}
-			if (pair.newest && (now > (pair.newest->getCloseTime() + LEDGER_VAL_INTERVAL)))
-			{
-#ifdef VC_DEBUG
-				Log(lsINFO) << "VC: " << it->first << " removeNewestStale";
-#endif
-				mStaleValidations.push_back(pair.newest);
-				pair.newest = SerializedValidation::pointer();
-				condWrite();
-			}
-			if (!pair.newest && !pair.oldest)
+			if (!it->second) // contains no record
 				it = mCurrentValidations.erase(it);
+			else if (it->second->getSignTime() < cutoff)
+			{ // contains a stale record
+				mStaleValidations.push_back(it->second);
+				it->second = SerializedValidation::pointer();
+				condWrite();
+				it = mCurrentValidations.erase(it);
+			}
 			else
-			{
-				if (pair.oldest)
-				{
-#ifdef VC_DEBUG
-					Log(lsTRACE) << "VC: OLD " << pair.oldest->getLedgerHash() << " " <<
-						boost::lexical_cast<std::string>(pair.oldest->getCloseTime());
-#endif
-					++ret[pair.oldest->getLedgerHash()];
-				}
-				if (pair.newest)
-				{
-#ifdef VC_DEBUG
-					Log(lsTRACE) << "VC: NEW " << pair.newest->getLedgerHash() << " " <<
-						boost::lexical_cast<std::string>(pair.newest->getCloseTime());
-#endif
-					++ret[pair.newest->getLedgerHash()];
-				}
+			{ // contains a live record
+				++ret[it->second->getLedgerHash()];
 				++it;
 			}
 		}
@@ -213,14 +179,12 @@ void ValidationCollection::addDeadLedger(const uint256& ledger)
 void ValidationCollection::flush()
 {
 		boost::mutex::scoped_lock sl(mValidationLock);
-		boost::unordered_map<uint160, ValidationPair>::iterator it = mCurrentValidations.begin();
+		boost::unordered_map<uint160, SerializedValidation::pointer>::iterator it = mCurrentValidations.begin();
 		bool anyNew = false;
 		while (it != mCurrentValidations.end())
 		{
-			if (it->second.oldest)
-				mStaleValidations.push_back(it->second.oldest);
-			if (it->second.newest)
-				mStaleValidations.push_back(it->second.newest);
+			if (it->second)
+				mStaleValidations.push_back(it->second);
 			++it;
 			anyNew = true;
 		}
@@ -247,7 +211,7 @@ void ValidationCollection::condWrite()
 void ValidationCollection::doWrite()
 {
 	static boost::format insVal("INSERT INTO LedgerValidations "
-		"(LedgerHash,NodePubKey,Flags,CloseTime,Signature) VALUES ('%s','%s','%u','%u',%s);");
+		"(LedgerHash,NodePubKey,Flags,SignTime,Signature) VALUES ('%s','%s','%u','%u',%s);");
 
 	boost::mutex::scoped_lock sl(mValidationLock);
 	assert(mWriting);
@@ -265,7 +229,7 @@ void ValidationCollection::doWrite()
 
 			BOOST_FOREACH(const SerializedValidation::pointer& it, vector)
 				db->executeSQL(boost::str(insVal % it->getLedgerHash().GetHex()
-					% it->getSignerPublic().humanNodePublic() % it->getFlags() % it->getCloseTime()
+					% it->getSignerPublic().humanNodePublic() % it->getFlags() % it->getSignTime()
 					% db->escape(strCopy(it->getSignature()))));
 			db->executeSQL("END TRANSACTION;");
 		}

src/ValidationCollection.h

@@ -12,23 +12,15 @@
 
 typedef boost::unordered_map<uint160, SerializedValidation::pointer> ValidationSet;
 
-class ValidationPair
-{
-public:
-	SerializedValidation::pointer oldest, newest;
-
-	ValidationPair(const SerializedValidation::pointer& v) : newest(v) { ; }
-};
-
 class ValidationCollection
 {
 protected:
 
 	boost::mutex mValidationLock;
-	boost::unordered_map<uint256, ValidationSet> 	mValidations;
-	boost::unordered_map<uint160, ValidationPair> 	mCurrentValidations;
-	std::vector<SerializedValidation::pointer> 		mStaleValidations;
-	std::list<uint256>								mDeadLedgers;
+	boost::unordered_map<uint256, ValidationSet> 					mValidations;
+	boost::unordered_map<uint160, SerializedValidation::pointer> 	mCurrentValidations;
+	std::vector<SerializedValidation::pointer> 						mStaleValidations;
+	std::list<uint256>												mDeadLedgers;
 
 	bool mWriting;