From 0dd3a318edf4d83b5350ba65782072bcc497550b Mon Sep 17 00:00:00 2001 From: csoler Date: Sun, 8 Jan 2017 11:10:33 +0100 Subject: [PATCH 01/21] added last used TS in reputation items --- .../src/serialiser/rsgxsreputationitems.cc | 57 ++++++++++++++++--- .../src/serialiser/rsgxsreputationitems.h | 57 +++++++++++-------- 2 files changed, 82 insertions(+), 32 deletions(-) diff --git a/libretroshare/src/serialiser/rsgxsreputationitems.cc b/libretroshare/src/serialiser/rsgxsreputationitems.cc index 7a7c9e7f8..5701cc756 100644 --- a/libretroshare/src/serialiser/rsgxsreputationitems.cc +++ b/libretroshare/src/serialiser/rsgxsreputationitems.cc @@ -165,6 +165,7 @@ uint32_t RsGxsReputationSetItem::serial_size() const s += 4 ; // mOwnOpinion s += 4 ; // mOwnOpinionTS s += 4 ; // mIdentityFlags + s += 4 ; // mLastUsedTS s += mOwnerNodeId.serial_size() ; s += 4 ; // mOpinions.size() @@ -245,6 +246,7 @@ bool RsGxsReputationSetItem::serialise(void *data, uint32_t& pktsize) const ok &= setRawUInt32(data, tlvsize, &offset, mOwnOpinion); ok &= setRawUInt32(data, tlvsize, &offset, mOwnOpinionTS); ok &= setRawUInt32(data, tlvsize, &offset, mIdentityFlags) ; + ok &= setRawUInt32(data, tlvsize, &offset, mLastUsedTS) ; ok &= mOwnerNodeId.serialise(data,tlvsize,offset) ; ok &= setRawUInt32(data, tlvsize, &offset, mOpinions.size()); @@ -421,13 +423,13 @@ RsGxsReputationSetItem *RsGxsReputationSerialiser::deserialiseReputationSetItem_ return item; } -RsGxsReputationSetItem *RsGxsReputationSerialiser::deserialiseReputationSetItem(void *data,uint32_t tlvsize) +RsGxsReputationSetItem_deprecated3 *RsGxsReputationSerialiser::deserialiseReputationSetItem_deprecated3(void *data,uint32_t tlvsize) { uint32_t offset = 8; // skip the header uint32_t rssize = getRsItemSize(data); bool ok = true ; - RsGxsReputationSetItem *item = new RsGxsReputationSetItem() ; + RsGxsReputationSetItem_deprecated3 *item = new RsGxsReputationSetItem_deprecated3() ; /* add mandatory parts first */ ok &= item->mGxsId.deserialise(data, tlvsize, offset) ; @@ -460,7 +462,46 @@ RsGxsReputationSetItem *RsGxsReputationSerialiser::deserialiseReputationSetItem( return item; } +RsGxsReputationSetItem *RsGxsReputationSerialiser::deserialiseReputationSetItem(void *data,uint32_t tlvsize) +{ + uint32_t offset = 8; // skip the header + uint32_t rssize = getRsItemSize(data); + bool ok = true ; + RsGxsReputationSetItem *item = new RsGxsReputationSetItem() ; + + /* add mandatory parts first */ + ok &= item->mGxsId.deserialise(data, tlvsize, offset) ; + ok &= getRawUInt32(data, tlvsize, &offset, &item->mOwnOpinion); + ok &= getRawUInt32(data, tlvsize, &offset, &item->mOwnOpinionTS); + ok &= getRawUInt32(data, tlvsize, &offset, &item->mIdentityFlags); + ok &= getRawUInt32(data, tlvsize, &offset, &item->mLastUsedTS); + ok &= item->mOwnerNodeId.deserialise(data, tlvsize, offset) ; + + uint32_t S ; + ok &= getRawUInt32(data, tlvsize, &offset, &S); + + for(uint32_t i = 0; ok && (i < S); ++i) + { + RsPeerId pid ; + uint32_t op ; + + ok &= pid.deserialise(data, tlvsize, offset) ; + ok &= getRawUInt32(data, tlvsize, &offset, &op); + + if(ok) + item->mOpinions[pid] = op ; + } + + if (offset != rssize || !ok) + { + std::cerr << __PRETTY_FUNCTION__ << ": error while deserialising! Item will be dropped." << std::endl; + delete item; + return NULL ; + } + + return item; +} RsGxsReputationUpdateItem *RsGxsReputationSerialiser::deserialiseReputationUpdateItem(void *data,uint32_t tlvsize) { uint32_t offset = 8; // skip the header @@ -533,12 +574,12 @@ RsItem *RsGxsReputationSerialiser::deserialise(void *data, uint32_t *pktsize) switch(getRsItemSubType(rstype)) { - case RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM : return deserialiseReputationSetItem (data, *pktsize); - case RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM_deprecated : return deserialiseReputationSetItem_deprecated(data, *pktsize); - case RS_PKT_SUBTYPE_GXS_REPUTATION_BANNED_NODE_SET_ITEM: return deserialiseReputationBannedNodeSetItem(data, *pktsize); - case RS_PKT_SUBTYPE_GXS_REPUTATION_UPDATE_ITEM : return deserialiseReputationUpdateItem (data, *pktsize); - case RS_PKT_SUBTYPE_GXS_REPUTATION_REQUEST_ITEM : return deserialiseReputationRequestItem (data, *pktsize); - case RS_PKT_SUBTYPE_GXS_REPUTATION_CONFIG_ITEM : return deserialiseReputationConfigItem (data, *pktsize); + case RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM : return deserialiseReputationSetItem (data, *pktsize); + case RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM_deprecated3: return deserialiseReputationSetItem_deprecated3(data, *pktsize); + case RS_PKT_SUBTYPE_GXS_REPUTATION_BANNED_NODE_SET_ITEM: return deserialiseReputationBannedNodeSetItem (data, *pktsize); + case RS_PKT_SUBTYPE_GXS_REPUTATION_UPDATE_ITEM : return deserialiseReputationUpdateItem (data, *pktsize); + case RS_PKT_SUBTYPE_GXS_REPUTATION_REQUEST_ITEM : return deserialiseReputationRequestItem (data, *pktsize); + case RS_PKT_SUBTYPE_GXS_REPUTATION_CONFIG_ITEM : return deserialiseReputationConfigItem (data, *pktsize); default: std::cerr << "RsGxsReputationSerialiser::deserialise(): unknown item subtype " << std::hex<< rstype << std::dec << std::endl; diff --git a/libretroshare/src/serialiser/rsgxsreputationitems.h b/libretroshare/src/serialiser/rsgxsreputationitems.h index 4bf55e0d8..c7321f74b 100644 --- a/libretroshare/src/serialiser/rsgxsreputationitems.h +++ b/libretroshare/src/serialiser/rsgxsreputationitems.h @@ -32,14 +32,16 @@ #include "serialiser/rsserial.h" #include "serialiser/rstlvidset.h" #include "retroshare/rsgxsifacetypes.h" +#include "retroshare/rsreputations.h" #define RS_PKT_SUBTYPE_GXS_REPUTATION_CONFIG_ITEM 0x01 #define RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM_deprecated2 0x02 #define RS_PKT_SUBTYPE_GXS_REPUTATION_UPDATE_ITEM 0x03 #define RS_PKT_SUBTYPE_GXS_REPUTATION_REQUEST_ITEM 0x04 -#define RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM_deprecated 0x05 -#define RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM 0x06 +#define RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM_deprecated1 0x05 +#define RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM_deprecated3 0x06 #define RS_PKT_SUBTYPE_GXS_REPUTATION_BANNED_NODE_SET_ITEM 0x07 +#define RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM 0x08 /**************************************************************************/ class RsReputationItem: public RsItem @@ -79,32 +81,38 @@ public: uint32_t mLastQuery; // when we sent out. }; -// This class should disappear. Deprecated since Aug 1, 2016. The class definition is actually not needed, +// This class should disappear. Deprecated since Jan 7, 2017. The class definition is actually not needed, // that is why it's commented out. Kept here in order to explains how the deserialisation works. // -// class RsGxsReputationSetItem_deprecated: public RsReputationItem -// { -// public: -// RsGxsReputationSetItem_deprecated() :RsReputationItem(RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM_deprecated) {} -// -// virtual ~RsGxsReputationSetItem_deprecated() {} -// virtual void clear() {} -// std::ostream &print(std::ostream &out, uint16_t indent = 0) { return out;} -// -// virtual bool serialise(void *data,uint32_t& size) const { std::cerr << "(EE) serialise attempt for a deprecated reputation item. This should not happen" << std::endl; return false ; } -// virtual uint32_t serial_size() const ; -// -// RsGxsId mGxsId; -// uint32_t mOwnOpinion; -// uint32_t mOwnOpinionTS; -// uint32_t mIdentityFlags; -// std::map mOpinions; // RsPeerId -> Opinion. -// }; +class RsGxsReputationSetItem_deprecated3: public RsReputationItem +{ +public: + RsGxsReputationSetItem_deprecated3() :RsReputationItem(RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM_deprecated3) {} + virtual ~RsGxsReputationSetItem_deprecated3() {} + virtual void clear() {} + std::ostream &print(std::ostream &out, uint16_t indent = 0) { return out;} + + virtual bool serialise(void *data,uint32_t& size) const { return false ;} + virtual uint32_t serial_size() const { return 0;} + + RsGxsId mGxsId; + uint32_t mOwnOpinion; + uint32_t mOwnOpinionTS; + uint32_t mIdentityFlags ; + RsPgpId mOwnerNodeId; + std::map mOpinions; // RsPeerId -> Opinion. +}; class RsGxsReputationSetItem: public RsReputationItem { public: - RsGxsReputationSetItem() :RsReputationItem(RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM) {} + RsGxsReputationSetItem() :RsReputationItem(RS_PKT_SUBTYPE_GXS_REPUTATION_SET_ITEM) + { + mOwnOpinion = RsReputations::OPINION_NEUTRAL ; + mOwnOpinionTS = 0; + mIdentityFlags = 0; + mLastUsedTS = 0; + } virtual ~RsGxsReputationSetItem() {} virtual void clear(); @@ -116,11 +124,11 @@ public: RsGxsId mGxsId; uint32_t mOwnOpinion; uint32_t mOwnOpinionTS; - uint32_t mIdentityFlags ; + uint32_t mIdentityFlags; + uint32_t mLastUsedTS; RsPgpId mOwnerNodeId; std::map mOpinions; // RsPeerId -> Opinion. }; - class RsGxsReputationBannedNodeSetItem: public RsReputationItem { public: @@ -191,6 +199,7 @@ private: static RsGxsReputationConfigItem *deserialiseReputationConfigItem (void *data, uint32_t size); static RsGxsReputationSetItem *deserialiseReputationSetItem (void *data, uint32_t size); static RsGxsReputationSetItem *deserialiseReputationSetItem_deprecated (void *data, uint32_t size); + static RsGxsReputationSetItem_deprecated3 *deserialiseReputationSetItem_deprecated3 (void *data, uint32_t size); static RsGxsReputationUpdateItem *deserialiseReputationUpdateItem (void *data, uint32_t size); static RsGxsReputationRequestItem *deserialiseReputationRequestItem (void *data, uint32_t size); static RsGxsReputationBannedNodeSetItem *deserialiseReputationBannedNodeSetItem (void *data, uint32_t size); From b6a3f53826be87c40273699b7a5149efc27a74c7 Mon Sep 17 00:00:00 2001 From: csoler Date: Sun, 8 Jan 2017 11:14:18 +0100 Subject: [PATCH 02/21] changed the needsUpdate flag so that unset/default needs update, and added code to read old reputation set item class --- libretroshare/src/services/p3gxsreputation.cc | 93 +++++++++++++------ libretroshare/src/services/p3gxsreputation.h | 13 ++- 2 files changed, 70 insertions(+), 36 deletions(-) diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index 4fb33396f..6121b896f 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -39,6 +39,7 @@ /**** * #define DEBUG_REPUTATION 1 ****/ +#define DEBUG_REPUTATION 1 /************ IMPLEMENTATION NOTES ********************************* * @@ -228,22 +229,6 @@ int p3GxsReputation::tick() return 0; } -// void p3GxsReputation::setNodeAutoBanThreshold(uint32_t n) -// { -// RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ -// -// if(n != mPgpAutoBanThreshold) -// { -// mLastBannedNodesUpdate = 0 ; -// mPgpAutoBanThreshold = n ; -// IndicateConfigChanged() ; -// } -// } -// uint32_t p3GxsReputation::nodeAutoBanThreshold() -// { -// return mPgpAutoBanThreshold ; -// } - void p3GxsReputation::setNodeAutoPositiveOpinionForContacts(bool b) { RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ @@ -277,9 +262,6 @@ class ZeroInitCnt void p3GxsReputation::updateBannedNodesProxy() { -//#ifdef DEBUG_REPUTATION -// std::cerr << "Updating PGP ban list based on signed GxsIds to ban. Ban threshold = " << mPgpAutoBanThreshold << std::endl; -//#endif // This function keeps the Banned GXS id proxy up to date. // @@ -305,7 +287,7 @@ void p3GxsReputation::updateIdentityFlags() #endif for( std::map::iterator rit = mReputations.begin();rit!=mReputations.end();++rit) - if( (rit->second.mIdentityFlags & REPUTATION_IDENTITY_FLAG_NEEDS_UPDATE) && (mPerNodeBannedIdsProxy.find(rit->first) == mPerNodeBannedIdsProxy.end())) + if( (!(rit->second.mIdentityFlags & REPUTATION_IDENTITY_FLAG_UP_TO_DATE)) && (mPerNodeBannedIdsProxy.find(rit->first) == mPerNodeBannedIdsProxy.end())) to_update.push_back(rit->first) ; } @@ -333,7 +315,7 @@ void p3GxsReputation::updateIdentityFlags() std::cerr << " Weird situation: item " << *rit << " has been deleted from the list??" << std::endl; continue ; } - it->second.mIdentityFlags = 0 ; + it->second.mIdentityFlags = 0 ; // resets the NEEDS_UPDATE flag. All other flags set later on. if(details.mFlags & RS_IDENTITY_FLAGS_PGP_LINKED) { @@ -934,7 +916,7 @@ bool p3GxsReputation::isIdentityBanned(const RsGxsId &id) return false ; #ifdef DEBUG_REPUTATION - std::cerr << "isIdentityBanned(): returning " << (info.mAssessment == RsReputations::ASSESSMENT_BAD) << " for GXS id " << id << std::endl; + std::cerr << "isIdentityBanned(): returning " << (info.mOverallReputationLevel == RsReputations::REPUTATION_LOCALLY_NEGATIVE) << " for GXS id " << id << std::endl; #endif return info.mOverallReputationLevel == RsReputations::REPUTATION_LOCALLY_NEGATIVE ; } @@ -993,7 +975,6 @@ bool p3GxsReputation::setOwnOpinion(const RsGxsId& gxsid, const RsReputations::O reputation.updateReputation(); mUpdated.insert(std::make_pair(now, gxsid)); - mUpdatedReputations.insert(gxsid); mReputationsUpdated = true; mLastBannedNodesUpdate = 0 ; // for update of banned nodes @@ -1051,6 +1032,7 @@ bool p3GxsReputation::saveList(bool& cleanup, std::list &savelist) item->mOwnOpinionTS = rit->second.mOwnOpinionTs; item->mIdentityFlags = rit->second.mIdentityFlags; item->mOwnerNodeId = rit->second.mOwnerNode; + item->mLastUsedTS = rit->second.mLastUsedTS; std::map::iterator oit; for(oit = rit->second.mOpinions.begin(); oit != rit->second.mOpinions.end(); ++oit) @@ -1129,6 +1111,10 @@ bool p3GxsReputation::loadList(std::list& loadList) if (set) loadReputationSet(set, peerSet); + RsGxsReputationSetItem_deprecated3 *set2 = dynamic_cast(*it); + + if(set2) + loadReputationSet_deprecated3(set2, peerSet); RsGxsReputationBannedNodeSetItem *itm2 = dynamic_cast(*it) ; @@ -1177,6 +1163,57 @@ bool p3GxsReputation::loadList(std::list& loadList) loadList.clear() ; return true; } +bool p3GxsReputation::loadReputationSet_deprecated3(RsGxsReputationSetItem_deprecated3 *item, const std::set &peerSet) +{ + { + RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ + + std::map::iterator rit; + + if(item->mGxsId.isNull()) // just a protection against potential errors having put 00000 into ids. + return false ; + + /* find matching Reputation */ + RsGxsId gxsId(item->mGxsId); + rit = mReputations.find(gxsId); + if (rit != mReputations.end()) + { + std::cerr << "ERROR"; + std::cerr << std::endl; + } + + Reputation &reputation = mReputations[gxsId]; + + // install opinions. + std::map::const_iterator oit; + for(oit = item->mOpinions.begin(); oit != item->mOpinions.end(); ++oit) + { + // expensive ... but necessary. + RsPeerId peerId(oit->first); + if (peerSet.end() != peerSet.find(peerId)) + reputation.mOpinions[peerId] = safe_convert_uint32t_to_opinion(oit->second); + } + + reputation.mOwnOpinion = item->mOwnOpinion; + reputation.mOwnOpinionTs = item->mOwnOpinionTS; + reputation.mOwnerNode = item->mOwnerNodeId; + reputation.mIdentityFlags = item->mIdentityFlags & (~REPUTATION_IDENTITY_FLAG_UP_TO_DATE); + reputation.mLastUsedTS = time(NULL); + + // if dropping entries has changed the score -> must update. + + reputation.updateReputation() ; + + mUpdated.insert(std::make_pair(reputation.mOwnOpinionTs, gxsId)); + } +#ifdef DEBUG_REPUTATION + RsReputations::ReputationInfo info ; + getReputationInfo(item->mGxsId,item->mOwnerNodeId,info) ; + std::cerr << item->mGxsId << " : own: " << info.mOwnOpinion << ", owner node: " << item->mOwnerNodeId << ", overall level: " << info.mOverallReputationLevel << std::endl; +#endif + return true; +} + bool p3GxsReputation::loadReputationSet(RsGxsReputationSetItem *item, const std::set &peerSet) { @@ -1212,13 +1249,11 @@ bool p3GxsReputation::loadReputationSet(RsGxsReputationSetItem *item, const std: reputation.mOwnOpinion = item->mOwnOpinion; reputation.mOwnOpinionTs = item->mOwnOpinionTS; reputation.mOwnerNode = item->mOwnerNodeId; - reputation.mIdentityFlags = item->mIdentityFlags | REPUTATION_IDENTITY_FLAG_NEEDS_UPDATE; + reputation.mIdentityFlags = item->mIdentityFlags; + reputation.mLastUsedTS = item->mLastUsedTS; // if dropping entries has changed the score -> must update. - //float old_reputation = reputation.mReputation ; - //mUpdatedReputations.insert(gxsId) ; - reputation.updateReputation() ; mUpdated.insert(std::make_pair(reputation.mOwnOpinionTs, gxsId)); @@ -1226,7 +1261,7 @@ bool p3GxsReputation::loadReputationSet(RsGxsReputationSetItem *item, const std: #ifdef DEBUG_REPUTATION RsReputations::ReputationInfo info ; getReputationInfo(item->mGxsId,item->mOwnerNodeId,info) ; - std::cerr << item->mGxsId << " : own: " << info.mOwnOpinion << ", owner node: " << item->mOwnerNodeId << ", assessment: " << ((info.mAssessment==ASSESSMENT_BAD)?"BAD":"OK") << std::endl; + std::cerr << item->mGxsId << " : own: " << info.mOwnOpinion << ", owner node: " << item->mOwnerNodeId << ", level: " << info.mOverallReputationLevel << std::endl; #endif return true; } @@ -1426,7 +1461,7 @@ void p3GxsReputation::debug_print() for(std::map::const_iterator it(mReputations.begin());it!=mReputations.end();++it) { std::cerr << " " << it->first << ": own: " << it->second.mOwnOpinion << ", Friend average: " << it->second.mFriendAverage << ", global_score: " << it->second.mReputationScore - << ", last own update: " << now - it->second.mOwnOpinionTs << " secs ago." << std::endl; + << ", last own update: " << now - it->second.mOwnOpinionTs << " secs ago, last needed: " << now - it->second.mLastUsedTS << " secs ago." << std::endl; #ifdef DEBUG_REPUTATION2 for(std::map::const_iterator it2(it->second.mOpinions.begin());it2!=it->second.mOpinions.end();++it2) std::cerr << " " << it2->first << ": " << it2->second << std::endl; diff --git a/libretroshare/src/services/p3gxsreputation.h b/libretroshare/src/services/p3gxsreputation.h index da7831987..74d0ad803 100644 --- a/libretroshare/src/services/p3gxsreputation.h +++ b/libretroshare/src/services/p3gxsreputation.h @@ -32,7 +32,7 @@ #include #include -#define REPUTATION_IDENTITY_FLAG_NEEDS_UPDATE 0x0100 +#define REPUTATION_IDENTITY_FLAG_UP_TO_DATE 0x0100 #define REPUTATION_IDENTITY_FLAG_PGP_LINKED 0x0001 #define REPUTATION_IDENTITY_FLAG_PGP_KNOWN 0x0002 @@ -69,10 +69,10 @@ class Reputation { public: Reputation() - :mOwnOpinion(RsReputations::OPINION_NEUTRAL), mOwnOpinionTs(0),mFriendAverage(1.0f), mReputationScore(RsReputations::OPINION_NEUTRAL),mIdentityFlags(REPUTATION_IDENTITY_FLAG_NEEDS_UPDATE){ } + :mOwnOpinion(RsReputations::OPINION_NEUTRAL), mOwnOpinionTs(0),mFriendAverage(1.0f), mReputationScore(RsReputations::OPINION_NEUTRAL),mIdentityFlags(0){ } Reputation(const RsGxsId& /*about*/) - :mOwnOpinion(RsReputations::OPINION_NEUTRAL), mOwnOpinionTs(0),mFriendAverage(1.0f), mReputationScore(RsReputations::OPINION_NEUTRAL),mIdentityFlags(REPUTATION_IDENTITY_FLAG_NEEDS_UPDATE){ } + :mOwnOpinion(RsReputations::OPINION_NEUTRAL), mOwnOpinionTs(0),mFriendAverage(1.0f), mReputationScore(RsReputations::OPINION_NEUTRAL),mIdentityFlags(0){ } void updateReputation(); @@ -89,6 +89,8 @@ public: RsPgpId mOwnerNode; uint32_t mIdentityFlags; + + time_t mLastUsedTS ; // last time the reputation was asked. Used to keep track of activity and clean up some reputation data. }; @@ -148,7 +150,7 @@ private: // internal update of data. Takes care of cleaning empty boxes. void locked_updateOpinion(const RsPeerId &from, const RsGxsId &about, RsReputations::Opinion op); bool loadReputationSet(RsGxsReputationSetItem *item, const std::set &peerSet); - + bool loadReputationSet_deprecated3(RsGxsReputationSetItem_deprecated3 *item, const std::set &peerSet); int sendPackets(); void cleanup(); void sendReputationRequests(); @@ -177,9 +179,6 @@ private: std::map mReputations; std::multimap mUpdated; - // set of Reputations to send to p3IdService. - std::set mUpdatedReputations; - // PGP Ids auto-banned. This is updated regularly. std::map mBannedPgpIds ; std::set mPerNodeBannedIdsProxy ; From 2fc8d22cf91121e6438f01dab79d901dbdf688b8 Mon Sep 17 00:00:00 2001 From: csoler Date: Sun, 8 Jan 2017 22:00:54 +0100 Subject: [PATCH 03/21] changed cleaning strategy of p3gxsreputations to be based on last activity of the reputation system rather than the identity system --- libretroshare/src/services/p3gxsreputation.cc | 138 ++++++++---------- libretroshare/src/services/p3gxsreputation.h | 4 +- 2 files changed, 60 insertions(+), 82 deletions(-) diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index 6121b896f..1117b99f3 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -131,7 +131,7 @@ static const uint32_t LOWER_LIMIT = 0; // used to static const uint32_t UPPER_LIMIT = 2; // used to filter valid Opinion values from serialized data static const int kMaximumPeerAge = 180; // half a year. static const int kMaximumSetSize = 100; // max set of updates to send at once. -static const int ACTIVE_FRIENDS_UPDATE_PERIOD = 600 ; // 10 minutes +static const int CLEANUP_PERIOD = 600 ; // 10 minutes static const int ACTIVE_FRIENDS_ONLINE_DELAY = 86400*7 ; // 1 week. static const int kReputationRequestPeriod = 600; // 10 mins static const int kReputationStoreWait = 180; // 3 minutes. @@ -155,9 +155,7 @@ p3GxsReputation::p3GxsReputation(p3LinkMgr *lm) mRequestTime = 0; mStoreTime = 0; mReputationsUpdated = false; - mLastActiveFriendsUpdate = time(NULL) - 0.5*ACTIVE_FRIENDS_UPDATE_PERIOD; // avoids doing it too soon since the TS from rsIdentity needs to be loaded already mLastIdentityFlagsUpdate = time(NULL) - 3; - mAverageActiveFriends = 0 ; mLastBannedNodesUpdate = 0 ; mBannedNodesProxyNeedsUpdate = false; @@ -189,34 +187,33 @@ int p3GxsReputation::tick() time_t now = time(NULL); - if(mLastActiveFriendsUpdate + ACTIVE_FRIENDS_UPDATE_PERIOD < now) + if(mLastCleanUp + CLEANUP_PERIOD < now) { - updateActiveFriends() ; - cleanup() ; - - mLastActiveFriendsUpdate = now ; + cleanup() ; + + mLastCleanUp = now ; } - // no more than once per 5 second chunk. - - if(now > IDENTITY_FLAGS_UPDATE_DELAY+mLastIdentityFlagsUpdate) - { - updateIdentityFlags() ; - mLastIdentityFlagsUpdate = now ; - } - if(now > BANNED_NODES_UPDATE_DELAY+mLastBannedNodesUpdate) // 613 is not a multiple of 100, to avoid piling up work - { - updateIdentityFlags() ; // needed before updateBannedNodesList! - updateBannedNodesProxy(); - mLastBannedNodesUpdate = now ; - } + // no more than once per 5 second chunk. + + if(now > IDENTITY_FLAGS_UPDATE_DELAY+mLastIdentityFlagsUpdate) + { + updateIdentityFlags() ; + mLastIdentityFlagsUpdate = now ; + } + if(now > BANNED_NODES_UPDATE_DELAY+mLastBannedNodesUpdate) // 613 is not a multiple of 100, to avoid piling up work + { + updateIdentityFlags() ; // needed before updateBannedNodesList! + updateBannedNodesProxy(); + mLastBannedNodesUpdate = now ; + } + + if(mBannedNodesProxyNeedsUpdate) + { + updateBannedNodesProxy(); + mBannedNodesProxyNeedsUpdate = false ; + } - if(mBannedNodesProxyNeedsUpdate) - { - updateBannedNodesProxy(); - mBannedNodesProxyNeedsUpdate = false ; - } - #ifdef DEBUG_REPUTATION static time_t last_debug_print = time(NULL) ; @@ -276,6 +273,11 @@ void p3GxsReputation::updateBannedNodesProxy() void p3GxsReputation::updateIdentityFlags() { + // This function is the *only* place where rsIdentity is called. Normally the cross calls between p3IdService and p3GxsReputations should only + // happen one way: from rsIdentity to rsReputations. Still, reputations need to keep track of some identity flags. It's very important to make sure that: + // - rsIdentity is not called inside a mutex-protected zone, because normally calls happen in the other way. + // - + std::list to_update ; // we need to gather the list to be used in a non locked frame @@ -347,25 +349,43 @@ void p3GxsReputation::cleanup() #ifdef DEBUG_REPUTATION #endif std::cerr << "p3GxsReputation::cleanup() " << std::endl; + time_t now = time(NULL) ; - // Remove opinions about identities that do not exist anymore. That will in particular avoid asking p3idservice about deleted + // We should keep opinions about identities that do not exist anymore, but only rely on the usage TS. That will in particular avoid asking p3idservice about deleted // identities, which would cause an excess of hits to the database. We do it in two steps to avoid a deadlock when calling rsIdentity from here. // Also, neutral opinions for banned PGP linked nodes are kept, so as to be able to not request them again. bool updated = false ; - time_t now = time(NULL) ; - - std::list ids_to_check_for_last_usage_ts; { RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ for(std::map::iterator it(mReputations.begin());it!=mReputations.end();) + { + bool should_delete = false ; + + // Delete slots with basically no information + if(it->second.mOpinions.empty() && it->second.mOwnOpinion == RsReputations::OPINION_NEUTRAL && (it->second.mOwnerNode.isNull())) - { + { #ifdef DEBUG_REPUTATION std::cerr << " ID " << it->first << ": own is neutral and no opinions from friends => remove entry" << std::endl; #endif + should_delete = true ; + } + + // Delete slots that havn't been used for a while + + if(it->second.mLastUsedTS + REPUTATION_INFO_KEEP_DELAY < now) + { +#ifdef DEBUG_REPUTATION + std::cerr << " ID " << it->first << ": no request for reputation for more than " << REPUTATION_INFO_KEEP_DELAY/86400 << " days => deleting." << std::endl; +#endif + should_delete = true ; + } + + if(should_delete) + { std::map::iterator tmp(it) ; ++tmp ; mReputations.erase(it) ; @@ -373,22 +393,11 @@ void p3GxsReputation::cleanup() updated = true ; } else - { - ids_to_check_for_last_usage_ts.push_back(it->first) ; ++it; - } + } } - for(std::list::const_iterator it(ids_to_check_for_last_usage_ts.begin());it!=ids_to_check_for_last_usage_ts.end();++it) - if(rsIdentity->getLastUsageTS(*it) + REPUTATION_INFO_KEEP_DELAY < now) - { -#ifdef DEBUG_REPUTATION - std::cerr << " Identity " << *it << " has a last usage TS of " << now - rsIdentity->getLastUsageTS(*it) << " secs ago: deleting it." << std::endl; -#endif - RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ - mReputations.erase(*it) ; - updated = true ; - } + // Clean up of the banned PGP ids. { RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ @@ -414,39 +423,6 @@ void p3GxsReputation::cleanup() IndicateConfigChanged() ; } -void p3GxsReputation::updateActiveFriends() -{ - RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ - - // keep track of who is recently connected. That will give a value to average friend - // for this, we count all friends that have been online in the last week. - - time_t now = time(NULL) ; - - std::list idList ; - mLinkMgr->getFriendList(idList) ; - - mAverageActiveFriends = 0 ; -#ifdef DEBUG_REPUTATION - std::cerr << " counting recently online peers." << std::endl; -#endif - - for(std::list::const_iterator it(idList.begin());it!=idList.end();++it) - { - RsPeerDetails details ; -#ifdef DEBUG_REPUTATION - std::cerr << " " << *it << ": last seen " << now - details.lastConnect << " secs ago" << std::endl; -#endif - - if(rsPeers->getPeerDetails(*it, details) && now < details.lastConnect + ACTIVE_FRIENDS_ONLINE_DELAY) - ++mAverageActiveFriends ; - } -#ifdef DEBUG_REPUTATION - std::cerr << " new count: " << mAverageActiveFriends << std::endl; -#endif - -} - const float RsReputations::REPUTATION_THRESHOLD_ANTI_SPAM = 1.4f ; const float RsReputations::REPUTATION_THRESHOLD_DEFAULT = 1.0f ; @@ -779,6 +755,8 @@ bool p3GxsReputation::getReputationInfo(const RsGxsId& gxsid, const RsPgpId& own rep.mOwnerNode = ownerNode ; owner_id = rep.mOwnerNode ; + + rep.mLastUsedTS = now ; } // now compute overall score and reputation @@ -1114,7 +1092,10 @@ bool p3GxsReputation::loadList(std::list& loadList) RsGxsReputationSetItem_deprecated3 *set2 = dynamic_cast(*it); if(set2) + { + std::cerr << "(II) reading and converting old format ReputationSetItem." << std::endl; loadReputationSet_deprecated3(set2, peerSet); + } RsGxsReputationBannedNodeSetItem *itm2 = dynamic_cast(*it) ; @@ -1453,7 +1434,6 @@ void Reputation::updateReputation() void p3GxsReputation::debug_print() { std::cerr << "Reputations database: " << std::endl; - std::cerr << " Average number of peers: " << mAverageActiveFriends << std::endl; std::cerr << " GXS ID data: " << std::endl; time_t now = time(NULL) ; diff --git a/libretroshare/src/services/p3gxsreputation.h b/libretroshare/src/services/p3gxsreputation.h index 74d0ad803..30bf8e4f8 100644 --- a/libretroshare/src/services/p3gxsreputation.h +++ b/libretroshare/src/services/p3gxsreputation.h @@ -143,7 +143,6 @@ private: bool SendReputations(RsGxsReputationRequestItem *request); bool RecvReputations(RsGxsReputationUpdateItem *item); bool updateLatestUpdate(RsPeerId peerid, time_t latest_update); - void updateActiveFriends() ; void updateBannedNodesProxy(); @@ -161,13 +160,12 @@ private: private: RsMutex mReputationMtx; - time_t mLastActiveFriendsUpdate; + time_t mLastCleanUp; time_t mRequestTime; time_t mStoreTime; time_t mLastBannedNodesUpdate ; time_t mLastIdentityFlagsUpdate ; bool mReputationsUpdated; - uint32_t mAverageActiveFriends ; float mAutoBanIdentitiesLimit ; bool mAutoSetPositiveOptionToContacts; From 8d8453f9c08b126dc3b6b0e3eb7943c85b4a1f44 Mon Sep 17 00:00:00 2001 From: csoler Date: Mon, 9 Jan 2017 23:47:51 +0100 Subject: [PATCH 04/21] removed the reputation calls through rsIdentity. Improved the logic of updating static identity flags in p3gxsreputations --- libretroshare/src/chat/distributedchat.cc | 4 +- libretroshare/src/grouter/p3grouter.cc | 2 +- libretroshare/src/gxs/rsgixs.h | 1 + libretroshare/src/gxs/rsgxsnetservice.cc | 6 +- libretroshare/src/retroshare/rsidentity.h | 1 - libretroshare/src/retroshare/rsreputations.h | 1 + libretroshare/src/services/p3gxsreputation.cc | 89 +++++++++++++------ libretroshare/src/services/p3gxsreputation.h | 12 ++- libretroshare/src/services/p3idservice.cc | 9 -- libretroshare/src/services/p3idservice.h | 1 - 10 files changed, 78 insertions(+), 48 deletions(-) diff --git a/libretroshare/src/chat/distributedchat.cc b/libretroshare/src/chat/distributedchat.cc index 1b618593e..05312e1a9 100644 --- a/libretroshare/src/chat/distributedchat.cc +++ b/libretroshare/src/chat/distributedchat.cc @@ -138,7 +138,7 @@ bool DistributedChatService::handleRecvChatLobbyMsgItem(RsChatMsgItem *ci) return false ; } - if(rsIdentity->overallReputationLevel(cli->signature.keyId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) + if(rsReputations->overallReputationLevel(cli->signature.keyId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) { std::cerr << "(WW) Received lobby msg/item from banned identity " << cli->signature.keyId << ". Dropping it." << std::endl; return false ; @@ -648,7 +648,7 @@ void DistributedChatService::handleRecvChatLobbyEventItem(RsChatLobbyEventItem * #endif time_t now = time(NULL) ; - if(rsIdentity->overallReputationLevel(item->signature.keyId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) + if(rsReputations->overallReputationLevel(item->signature.keyId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) { std::cerr << "(WW) Received lobby msg/item from banned identity " << item->signature.keyId << ". Dropping it." << std::endl; return ; diff --git a/libretroshare/src/grouter/p3grouter.cc b/libretroshare/src/grouter/p3grouter.cc index b04a97cb4..5f2b6e7d0 100644 --- a/libretroshare/src/grouter/p3grouter.cc +++ b/libretroshare/src/grouter/p3grouter.cc @@ -1984,7 +1984,7 @@ bool p3GRouter::verifySignedDataItem(RsGRouterAbstractMsgItem *item,const RsIden { try { - if(rsIdentity->overallReputationLevel(item->signature.keyId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) + if(rsReputations->overallReputationLevel(item->signature.keyId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) { std::cerr << "(WW) received global router message from banned identity " << item->signature.keyId << ". Rejecting the message." << std::endl; return false ; diff --git a/libretroshare/src/gxs/rsgixs.h b/libretroshare/src/gxs/rsgixs.h index f27b3bf1b..4b4539273 100644 --- a/libretroshare/src/gxs/rsgixs.h +++ b/libretroshare/src/gxs/rsgixs.h @@ -179,6 +179,7 @@ public: virtual bool haveReputation(const RsGxsId &id) = 0; virtual bool loadReputation(const RsGxsId &id, const std::list& peers) = 0; virtual bool getReputation(const RsGxsId &id, GixsReputation &rep) = 0; + virtual RsReputations::ReputationLevel overallReputationLevel(const RsGxsId& id) = 0; }; /*** This Class pulls all the GXS Interfaces together ****/ diff --git a/libretroshare/src/gxs/rsgxsnetservice.cc b/libretroshare/src/gxs/rsgxsnetservice.cc index 589cea919..1be1ff58c 100644 --- a/libretroshare/src/gxs/rsgxsnetservice.cc +++ b/libretroshare/src/gxs/rsgxsnetservice.cc @@ -2738,9 +2738,11 @@ void RsGxsNetService::locked_genReqMsgTransaction(NxsTransaction* tr) return ; } +#ifdef TO_REMOVE int cutoff = 0; if(grpMeta != NULL) cutoff = grpMeta->mReputationCutOff; +#endif GxsMsgReq reqIds; reqIds[grpId] = std::vector(); @@ -2827,7 +2829,7 @@ void RsGxsNetService::locked_genReqMsgTransaction(NxsTransaction* tr) // - if author is locally banned, do not download. // - if author is not locally banned, download, whatever friends' opinion might be. - if(rsIdentity && rsIdentity->overallReputationLevel(syncItem->authorId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) + if(mReputations->overallReputationLevel(syncItem->authorId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) { #ifdef NXS_NET_DEBUG_1 GXSNETDEBUG_PG(item->PeerId(),grpId) << ", Identity " << syncItem->authorId << " is banned. Not requesting message!" << std::endl; @@ -3017,7 +3019,7 @@ void RsGxsNetService::locked_genReqGrpTransaction(NxsTransaction* tr) // FIXTESTS global variable rsReputations not available in unittests! #warning Update the code below to correctly send/recv dependign on reputation - if(!grpSyncItem->authorId.isNull() && rsIdentity && rsIdentity->overallReputationLevel(grpSyncItem->authorId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) + if(!grpSyncItem->authorId.isNull() && mReputations->overallReputationLevel(grpSyncItem->authorId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) { #ifdef NXS_NET_DEBUG_0 GXSNETDEBUG_PG(tr->mTransaction->PeerId(),grpId) << " Identity " << grpSyncItem->authorId << " is banned. Not syncing group." << std::endl; diff --git a/libretroshare/src/retroshare/rsidentity.h b/libretroshare/src/retroshare/rsidentity.h index ae36e326d..d6f0670f7 100644 --- a/libretroshare/src/retroshare/rsidentity.h +++ b/libretroshare/src/retroshare/rsidentity.h @@ -303,7 +303,6 @@ public: * \param id * \return */ - virtual RsReputations::ReputationLevel overallReputationLevel(const RsGxsId& id)=0; virtual time_t getLastUsageTS(const RsGxsId &id) =0; // Specific RsIdentity Functions.... diff --git a/libretroshare/src/retroshare/rsreputations.h b/libretroshare/src/retroshare/rsreputations.h index 1102a45be..ac518ba6c 100644 --- a/libretroshare/src/retroshare/rsreputations.h +++ b/libretroshare/src/retroshare/rsreputations.h @@ -62,6 +62,7 @@ public: virtual bool setOwnOpinion(const RsGxsId& key_id, const Opinion& op) =0; virtual bool getReputationInfo(const RsGxsId& id, const RsPgpId &ownerNode, ReputationInfo& info) =0; + virtual ReputationLevel overallReputationLevel(const RsGxsId& id)=0; // parameters diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index 1117b99f3..69a37d58b 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -144,6 +144,7 @@ static const uint32_t BANNED_NODES_INACTIVITY_KEEP = 86400*60; // remove static const uint32_t REPUTATION_DEFAULT_MIN_VOTES_FOR_REMOTELY_POSITIVE = 1; // min difference in votes that makes friends opinion globally positive static const uint32_t REPUTATION_DEFAULT_MIN_VOTES_FOR_REMOTELY_NEGATIVE = 1; // min difference in votes that makes friends opinion globally negative +static const uint32_t MIN_DELAY_BETWEEN_REPUTATION_CONFIG_SAVE = 61 ; // never save more often than once a minute. p3GxsReputation::p3GxsReputation(p3LinkMgr *lm) :p3Service(), p3Config(), @@ -162,6 +163,9 @@ p3GxsReputation::p3GxsReputation(p3LinkMgr *lm) mAutoSetPositiveOptionToContacts = true; // default mMinVotesForRemotelyPositive = REPUTATION_DEFAULT_MIN_VOTES_FOR_REMOTELY_POSITIVE; mMinVotesForRemotelyNegative = REPUTATION_DEFAULT_MIN_VOTES_FOR_REMOTELY_NEGATIVE; + + mLastReputationConfigSaved = 0; + mChanged = false ; } const std::string GXS_REPUTATION_APP_NAME = "gxsreputation"; @@ -198,12 +202,12 @@ int p3GxsReputation::tick() if(now > IDENTITY_FLAGS_UPDATE_DELAY+mLastIdentityFlagsUpdate) { - updateIdentityFlags() ; + updateStaticIdentityFlags() ; mLastIdentityFlagsUpdate = now ; } if(now > BANNED_NODES_UPDATE_DELAY+mLastBannedNodesUpdate) // 613 is not a multiple of 100, to avoid piling up work { - updateIdentityFlags() ; // needed before updateBannedNodesList! + updateStaticIdentityFlags() ; // needed before updateBannedNodesList! updateBannedNodesProxy(); mLastBannedNodesUpdate = now ; } @@ -223,6 +227,14 @@ int p3GxsReputation::tick() debug_print() ; } #endif + + if(mChanged && now > mLastReputationConfigSaved + MIN_DELAY_BETWEEN_REPUTATION_CONFIG_SAVE) + { + IndicateConfigChanged() ; + mLastReputationConfigSaved = now ; + mChanged = false ; + } + return 0; } @@ -233,7 +245,9 @@ void p3GxsReputation::setNodeAutoPositiveOpinionForContacts(bool b) if(b != mAutoSetPositiveOptionToContacts) { mLastIdentityFlagsUpdate = 0 ; + mLastCleanUp = 0 ; mAutoSetPositiveOptionToContacts = b ; + IndicateConfigChanged() ; } } @@ -271,12 +285,11 @@ void p3GxsReputation::updateBannedNodesProxy() mPerNodeBannedIdsProxy.insert(*it) ; } -void p3GxsReputation::updateIdentityFlags() +void p3GxsReputation::updateStaticIdentityFlags() { // This function is the *only* place where rsIdentity is called. Normally the cross calls between p3IdService and p3GxsReputations should only - // happen one way: from rsIdentity to rsReputations. Still, reputations need to keep track of some identity flags. It's very important to make sure that: - // - rsIdentity is not called inside a mutex-protected zone, because normally calls happen in the other way. - // - + // happen one way: from rsIdentity to rsReputations. Still, reputations need to keep track of some identity flags. It's very important to make sure that + // rsIdentity is not called inside a mutex-protected zone, because normally calls happen in the other way. std::list to_update ; @@ -289,12 +302,12 @@ void p3GxsReputation::updateIdentityFlags() #endif for( std::map::iterator rit = mReputations.begin();rit!=mReputations.end();++rit) + { if( (!(rit->second.mIdentityFlags & REPUTATION_IDENTITY_FLAG_UP_TO_DATE)) && (mPerNodeBannedIdsProxy.find(rit->first) == mPerNodeBannedIdsProxy.end())) to_update.push_back(rit->first) ; + } } - std::list should_set_to_positive ; - for(std::list::const_iterator rit(to_update.begin());rit!=to_update.end();++rit) { RsIdentityDetails details; @@ -306,8 +319,6 @@ void p3GxsReputation::updateIdentityFlags() #endif continue ; } - bool is_a_contact = rsIdentity->isARegularContact(*rit) ; - { RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ std::map::iterator it = mReputations.find(*rit) ; @@ -317,7 +328,7 @@ void p3GxsReputation::updateIdentityFlags() std::cerr << " Weird situation: item " << *rit << " has been deleted from the list??" << std::endl; continue ; } - it->second.mIdentityFlags = 0 ; // resets the NEEDS_UPDATE flag. All other flags set later on. + it->second.mIdentityFlags = REPUTATION_IDENTITY_FLAG_UP_TO_DATE ; // resets the NEEDS_UPDATE flag. All other flags set later on. if(details.mFlags & RS_IDENTITY_FLAGS_PGP_LINKED) { @@ -326,20 +337,14 @@ void p3GxsReputation::updateIdentityFlags() } if(details.mFlags & RS_IDENTITY_FLAGS_PGP_KNOWN ) it->second.mIdentityFlags |= REPUTATION_IDENTITY_FLAG_PGP_KNOWN ; - if(mAutoSetPositiveOptionToContacts && is_a_contact && it->second.mOwnOpinion == RsReputations::OPINION_NEUTRAL) - should_set_to_positive.push_back(*rit) ; - #ifdef DEBUG_REPUTATION std::cerr << " updated flags for " << *rit << " to " << std::hex << it->second.mIdentityFlags << std::dec << std::endl; #endif it->second.updateReputation() ; - IndicateConfigChanged(); + mChanged = true ; } } - - for(std::list::const_iterator it(should_set_to_positive.begin());it!=should_set_to_positive.end();++it) - setOwnOpinion(*it,RsReputations::OPINION_POSITIVE) ; } void p3GxsReputation::cleanup() @@ -355,8 +360,6 @@ void p3GxsReputation::cleanup() // identities, which would cause an excess of hits to the database. We do it in two steps to avoid a deadlock when calling rsIdentity from here. // Also, neutral opinions for banned PGP linked nodes are kept, so as to be able to not request them again. - bool updated = false ; - { RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ @@ -374,15 +377,19 @@ void p3GxsReputation::cleanup() should_delete = true ; } - // Delete slots that havn't been used for a while + // Delete slots that havn't been used for a while. The else below is here for debug display purposes, and not harmful since both conditions lead the same effect. - if(it->second.mLastUsedTS + REPUTATION_INFO_KEEP_DELAY < now) + else if(it->second.mLastUsedTS + REPUTATION_INFO_KEEP_DELAY < now) { #ifdef DEBUG_REPUTATION std::cerr << " ID " << it->first << ": no request for reputation for more than " << REPUTATION_INFO_KEEP_DELAY/86400 << " days => deleting." << std::endl; #endif should_delete = true ; } +#ifdef DEBUG_REPUTATION + else + std::cerr << " ID " << it->first << ": flags=" << std::hex << it->second.mIdentityFlags << std::dec << ". Last used: " << (now - it->second.mLastUsedTS)/86400 << " days ago: kept." << std::endl; +#endif if(should_delete) { @@ -390,7 +397,7 @@ void p3GxsReputation::cleanup() ++tmp ; mReputations.erase(it) ; it = tmp ; - updated = true ; + mChanged = true ; } else ++it; @@ -413,14 +420,31 @@ void p3GxsReputation::cleanup() mBannedPgpIds.erase(it) ; it = tmp ; - updated = true ; + mChanged = true ; } else ++it ; } - if(updated) - IndicateConfigChanged() ; + // update opinions based on flags and contact information + { + std::list should_set_to_positive ; + + { + RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ + + for(std::map::iterator it(mReputations.begin());it!=mReputations.end();++it) + { + bool is_a_contact = rsIdentity->isARegularContact(*it) ; + + if(mAutoSetPositiveOptionToContacts && is_a_contact && it->second.mOwnOpinion == RsReputations::OPINION_NEUTRAL) + should_set_to_positive.push_back(*rit) ; + } + } + + for(std::list::const_iterator it(should_set_to_positive.begin());it!=should_set_to_positive.end();++it) + setOwnOpinion(*it,RsReputations::OPINION_POSITIVE) ; + } } const float RsReputations::REPUTATION_THRESHOLD_ANTI_SPAM = 1.4f ; @@ -718,6 +742,14 @@ bool p3GxsReputation::updateLatestUpdate(RsPeerId peerid,time_t latest_update) * Opinion ****/ +RsReputations::ReputationLevel p3GxsReputation::overallReputationLevel(const RsGxsId& id) +{ + ReputationInfo info ; + getReputationInfo(id,RsPgpId(),info) ; + + return info.mOverallReputationLevel ; +} + bool p3GxsReputation::getReputationInfo(const RsGxsId& gxsid, const RsPgpId& ownerNode, RsReputations::ReputationInfo& info) { if(gxsid.isNull()) @@ -751,12 +783,13 @@ bool p3GxsReputation::getReputationInfo(const RsGxsId& gxsid, const RsPgpId& own info.mFriendsNegativeVotes = rep.mFriendsNegative ; info.mFriendsPositiveVotes = rep.mFriendsPositive ; - if(rep.mOwnerNode.isNull()) + if(rep.mOwnerNode.isNull() && !ownerNode.isNull()) rep.mOwnerNode = ownerNode ; owner_id = rep.mOwnerNode ; - rep.mLastUsedTS = now ; + + mChanged = true ; } // now compute overall score and reputation diff --git a/libretroshare/src/services/p3gxsreputation.h b/libretroshare/src/services/p3gxsreputation.h index 30bf8e4f8..ceaab3b85 100644 --- a/libretroshare/src/services/p3gxsreputation.h +++ b/libretroshare/src/services/p3gxsreputation.h @@ -32,9 +32,9 @@ #include #include -#define REPUTATION_IDENTITY_FLAG_UP_TO_DATE 0x0100 -#define REPUTATION_IDENTITY_FLAG_PGP_LINKED 0x0001 -#define REPUTATION_IDENTITY_FLAG_PGP_KNOWN 0x0002 +static const uint32_t REPUTATION_IDENTITY_FLAG_UP_TO_DATE = 0x0100; // This flag means that the static info has been initialised from p3IdService. Normally such a call should happen once. +static const uint32_t REPUTATION_IDENTITY_FLAG_PGP_LINKED = 0x0001; +static const uint32_t REPUTATION_IDENTITY_FLAG_PGP_KNOWN = 0x0002; #include "serialiser/rsgxsreputationitems.h" @@ -113,6 +113,7 @@ public: virtual bool isNodeBanned(const RsPgpId& id); virtual void banNode(const RsPgpId& id,bool b) ; + virtual ReputationLevel overallReputationLevel(const RsGxsId& id); virtual void setNodeAutoPositiveOpinionForContacts(bool b) ; virtual bool nodeAutoPositiveOpinionForContacts() ; @@ -155,7 +156,7 @@ private: void sendReputationRequests(); int sendReputationRequest(RsPeerId peerid); void debug_print() ; - void updateIdentityFlags(); + void updateStaticIdentityFlags(); private: RsMutex mReputationMtx; @@ -184,6 +185,9 @@ private: uint32_t mMinVotesForRemotelyPositive ; uint32_t mMinVotesForRemotelyNegative ; + + bool mChanged ; // slow version of IndicateConfigChanged(); + time_t mLastReputationConfigSaved ; }; #endif //SERVICE_RSGXSREPUTATION_HEADER diff --git a/libretroshare/src/services/p3idservice.cc b/libretroshare/src/services/p3idservice.cc index 321328030..88ea21072 100644 --- a/libretroshare/src/services/p3idservice.cc +++ b/libretroshare/src/services/p3idservice.cc @@ -611,15 +611,6 @@ bool p3IdService::getIdDetails(const RsGxsId &id, RsIdentityDetails &details) return false; } -RsReputations::ReputationLevel p3IdService::overallReputationLevel(const RsGxsId &id) -{ -#warning some IDs might be deleted but the reputation should still say they are banned. - RsIdentityDetails det ; - getIdDetails(id,det) ; - - return det.mReputation.mOverallReputationLevel ; -} - bool p3IdService::isOwnId(const RsGxsId& id) { RsStackMutex stack(mIdMtx); /********** STACK LOCKED MTX ******/ diff --git a/libretroshare/src/services/p3idservice.h b/libretroshare/src/services/p3idservice.h index 00faa0e15..1261195c1 100644 --- a/libretroshare/src/services/p3idservice.h +++ b/libretroshare/src/services/p3idservice.h @@ -273,7 +273,6 @@ public: virtual bool setAsRegularContact(const RsGxsId& id,bool is_a_contact) ; virtual bool isARegularContact(const RsGxsId& id) ; - virtual RsReputations::ReputationLevel overallReputationLevel(const RsGxsId& id); virtual time_t getLastUsageTS(const RsGxsId &id) ; /**************** RsGixs Implementation ***************/ From a810ae9a7454bf9a517413e91b2a38d8632b2a89 Mon Sep 17 00:00:00 2001 From: csoler Date: Tue, 10 Jan 2017 21:44:37 +0100 Subject: [PATCH 05/21] use rsReputations instead of rsIdentity for reputation calls. Suspended reputation vetting code which is not needed anymore --- libretroshare/src/gxs/rsgixs.h | 13 +++---- libretroshare/src/gxs/rsgxsnetservice.cc | 4 +++ libretroshare/src/gxs/rsgxsnetutils.cc | 34 +++++++++---------- libretroshare/src/gxs/rsgxsutil.cc | 4 +-- libretroshare/src/rsserver/rsinit.cc | 12 +++---- libretroshare/src/services/p3gxsreputation.cc | 4 +-- libretroshare/src/services/p3gxsreputation.h | 3 +- libretroshare/src/services/p3idservice.cc | 2 ++ libretroshare/src/services/p3idservice.h | 2 ++ .../src/gui/gxs/GxsIdTreeWidgetItem.cpp | 6 ++-- .../gui/gxsforums/GxsForumThreadWidget.cpp | 2 +- 11 files changed, 45 insertions(+), 41 deletions(-) diff --git a/libretroshare/src/gxs/rsgixs.h b/libretroshare/src/gxs/rsgixs.h index 4b4539273..22993b65f 100644 --- a/libretroshare/src/gxs/rsgixs.h +++ b/libretroshare/src/gxs/rsgixs.h @@ -166,19 +166,17 @@ public: class GixsReputation { - public: - GixsReputation() : score(0) {} - RsGxsId id; - int score; +public: + GixsReputation() {} + + RsGxsId id; + uint32_t reputation_level ; }; class RsGixsReputation { public: // get Reputation. - virtual bool haveReputation(const RsGxsId &id) = 0; - virtual bool loadReputation(const RsGxsId &id, const std::list& peers) = 0; - virtual bool getReputation(const RsGxsId &id, GixsReputation &rep) = 0; virtual RsReputations::ReputationLevel overallReputationLevel(const RsGxsId& id) = 0; }; @@ -186,7 +184,6 @@ public: class RsGxsIdExchange: public RsGenExchange, - public RsGixsReputation, public RsGixs { public: diff --git a/libretroshare/src/gxs/rsgxsnetservice.cc b/libretroshare/src/gxs/rsgxsnetservice.cc index 1be1ff58c..4b1677096 100644 --- a/libretroshare/src/gxs/rsgxsnetservice.cc +++ b/libretroshare/src/gxs/rsgxsnetservice.cc @@ -3139,6 +3139,9 @@ void RsGxsNetService::runVetting() RS_STACK_MUTEX(mNxsMutex) ; +#ifdef TO_BE_REMOVED + // Author response vetting is disabled since not used, as the reputations are currently not async-ed anymore. + std::vector::iterator vit = mPendingResp.begin(); for(; vit != mPendingResp.end(); ) @@ -3169,6 +3172,7 @@ void RsGxsNetService::runVetting() } } +#endif // now lets do circle vetting diff --git a/libretroshare/src/gxs/rsgxsnetutils.cc b/libretroshare/src/gxs/rsgxsnetutils.cc index 18b048cff..1956f8964 100644 --- a/libretroshare/src/gxs/rsgxsnetutils.cc +++ b/libretroshare/src/gxs/rsgxsnetutils.cc @@ -27,20 +27,13 @@ #include "pqi/p3servicecontrol.h" #include "pgp/pgpauxutils.h" - const time_t AuthorPending::EXPIRY_PERIOD_OFFSET = 30; // 30 seconds const int AuthorPending::MSG_PEND = 1; const int AuthorPending::GRP_PEND = 2; +AuthorPending::AuthorPending(RsGixsReputation* rep, time_t timeStamp) : mRep(rep), mTimeStamp(timeStamp) {} -AuthorPending::AuthorPending(RsGixsReputation* rep, time_t timeStamp) - : mRep(rep), mTimeStamp(timeStamp) { -} - -AuthorPending::~AuthorPending() -{ - -} +AuthorPending::~AuthorPending() {} bool AuthorPending::expired() const { @@ -49,7 +42,12 @@ bool AuthorPending::expired() const bool AuthorPending::getAuthorRep(GixsReputation& rep, const RsGxsId& authorId, const RsPeerId& peerId) { - if(mRep->haveReputation(authorId)) + rep.id = authorId ; + rep.reputation_level = mRep->overallReputationLevel(authorId); + +#warning can it happen that reputations do not have the info yet? + return true ; +#ifdef TO_BE_REMOVED { return mRep->getReputation(authorId, rep); } @@ -58,7 +56,7 @@ bool AuthorPending::getAuthorRep(GixsReputation& rep, const RsGxsId& authorId, c peers.push_back(peerId); mRep->loadReputation(authorId, peers); return false; - +#endif } MsgAuthEntry::MsgAuthEntry() @@ -98,7 +96,7 @@ bool MsgRespPending::accepted() GixsReputation rep; if(getAuthorRep(rep, entry.mAuthorId, mPeerId)) { - if(rep.score >= mCutOff) + if(rep.reputation_level >= mCutOff) { entry.mPassedVetting = true; count++; @@ -133,7 +131,7 @@ bool GrpRespPending::accepted() if(getAuthorRep(rep, entry.mAuthorId, mPeerId)) { - if(rep.score >= mCutOff) + if(rep.reputation_level >= mCutOff) { entry.mPassedVetting = true; count++; @@ -153,12 +151,12 @@ bool GrpRespPending::accepted() /** NxsTransaction definition **/ -const uint8_t NxsTransaction::FLAG_STATE_STARTING = 0x0001; // when -const uint8_t NxsTransaction::FLAG_STATE_RECEIVING = 0x0002; // begin receiving items for incoming trans -const uint8_t NxsTransaction::FLAG_STATE_SENDING = 0x0004; // begin sending items for outgoing trans -const uint8_t NxsTransaction::FLAG_STATE_COMPLETED = 0x008; -const uint8_t NxsTransaction::FLAG_STATE_FAILED = 0x0010; +const uint8_t NxsTransaction::FLAG_STATE_STARTING = 0x0001; // when +const uint8_t NxsTransaction::FLAG_STATE_RECEIVING = 0x0002; // begin receiving items for incoming trans +const uint8_t NxsTransaction::FLAG_STATE_SENDING = 0x0004; // begin sending items for outgoing trans +const uint8_t NxsTransaction::FLAG_STATE_FAILED = 0x0010; const uint8_t NxsTransaction::FLAG_STATE_WAITING_CONFIRM = 0x0020; +const uint8_t NxsTransaction::FLAG_STATE_COMPLETED = 0x0080; // originaly 0x008, but probably a typo, but we cannot change it since it would break backward compatibility. NxsTransaction::NxsTransaction() diff --git a/libretroshare/src/gxs/rsgxsutil.cc b/libretroshare/src/gxs/rsgxsutil.cc index f0736917c..aa5c5558a 100644 --- a/libretroshare/src/gxs/rsgxsutil.cc +++ b/libretroshare/src/gxs/rsgxsutil.cc @@ -171,7 +171,7 @@ bool RsGxsIntegrityCheck::check() GXSUTIL_DEBUG() << "TimeStamping group authors' key ID " << grp->metaData->mAuthorId << " in group ID " << grp->grpId << std::endl; #endif - if(rsIdentity!=NULL && rsIdentity->overallReputationLevel(grp->metaData->mAuthorId) > RsReputations::REPUTATION_LOCALLY_NEGATIVE) + if(rsReputations!=NULL && rsReputations->overallReputationLevel(grp->metaData->mAuthorId) > RsReputations::REPUTATION_LOCALLY_NEGATIVE) used_gxs_ids.insert(std::make_pair(grp->metaData->mAuthorId,RsIdentityUsage(mGenExchangeClient->serviceType(),RsIdentityUsage::GROUP_AUTHOR_KEEP_ALIVE,grp->grpId))) ; } } @@ -269,7 +269,7 @@ bool RsGxsIntegrityCheck::check() #ifdef DEBUG_GXSUTIL GXSUTIL_DEBUG() << "TimeStamping message authors' key ID " << msg->metaData->mAuthorId << " in message " << msg->msgId << ", group ID " << msg->grpId<< std::endl; #endif - if(rsIdentity!=NULL && rsIdentity->overallReputationLevel(msg->metaData->mAuthorId) > RsReputations::REPUTATION_LOCALLY_NEGATIVE) + if(rsReputations!=NULL && rsReputations->overallReputationLevel(msg->metaData->mAuthorId) > RsReputations::REPUTATION_LOCALLY_NEGATIVE) used_gxs_ids.insert(std::make_pair(msg->metaData->mAuthorId,RsIdentityUsage(mGenExchangeClient->serviceType(),RsIdentityUsage::MESSAGE_AUTHOR_KEEP_ALIVE,msg->metaData->mGroupId,msg->metaData->mMsgId))) ; } diff --git a/libretroshare/src/rsserver/rsinit.cc b/libretroshare/src/rsserver/rsinit.cc index 34b359fd5..47036b691 100644 --- a/libretroshare/src/rsserver/rsinit.cc +++ b/libretroshare/src/rsserver/rsinit.cc @@ -1345,8 +1345,8 @@ int RsServer::StartupRetroShare() // create GXS ID service RsGxsNetService* gxsid_ns = new RsGxsNetService( RS_SERVICE_GXS_TYPE_GXSID, gxsid_ds, nxsMgr, - mGxsIdService, mGxsIdService->getServiceInfo(), - mGxsIdService, mGxsCircles,mGxsIdService, + mGxsIdService, mGxsIdService->getServiceInfo(), + mReputations, mGxsCircles,mGxsIdService, pgpAuxUtils, false,false); // don't synchronise group automatic (need explicit group request) // don't sync messages at all. @@ -1365,7 +1365,7 @@ int RsServer::StartupRetroShare() RsGxsNetService* gxscircles_ns = new RsGxsNetService( RS_SERVICE_GXS_TYPE_GXSCIRCLE, gxscircles_ds, nxsMgr, mGxsCircles, mGxsCircles->getServiceInfo(), - mGxsIdService, mGxsCircles,mGxsIdService, + mReputations, mGxsCircles,mGxsIdService, pgpAuxUtils, true, // synchronise group automatic true); // sync messages automatic, since they contain subscription requests. @@ -1384,7 +1384,7 @@ int RsServer::StartupRetroShare() RsGxsNetService* posted_ns = new RsGxsNetService( RS_SERVICE_GXS_TYPE_POSTED, posted_ds, nxsMgr, mPosted, mPosted->getServiceInfo(), - mGxsIdService, mGxsCircles,mGxsIdService, + mReputations, mGxsCircles,mGxsIdService, pgpAuxUtils); mPosted->setNetworkExchangeService(posted_ns) ; @@ -1420,7 +1420,7 @@ int RsServer::StartupRetroShare() RsGxsNetService* gxsforums_ns = new RsGxsNetService( RS_SERVICE_GXS_TYPE_FORUMS, gxsforums_ds, nxsMgr, mGxsForums, mGxsForums->getServiceInfo(), - mGxsIdService, mGxsCircles,mGxsIdService, + mReputations, mGxsCircles,mGxsIdService, pgpAuxUtils); mGxsForums->setNetworkExchangeService(gxsforums_ns) ; @@ -1436,7 +1436,7 @@ int RsServer::StartupRetroShare() RsGxsNetService* gxschannels_ns = new RsGxsNetService( RS_SERVICE_GXS_TYPE_CHANNELS, gxschannels_ds, nxsMgr, mGxsChannels, mGxsChannels->getServiceInfo(), - mGxsIdService, mGxsCircles,mGxsIdService, + mReputations, mGxsCircles,mGxsIdService, pgpAuxUtils); mGxsChannels->setNetworkExchangeService(gxschannels_ns) ; diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index 69a37d58b..d5d0e2e7b 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -435,10 +435,10 @@ void p3GxsReputation::cleanup() for(std::map::iterator it(mReputations.begin());it!=mReputations.end();++it) { - bool is_a_contact = rsIdentity->isARegularContact(*it) ; + bool is_a_contact = rsIdentity->isARegularContact(it->first) ; if(mAutoSetPositiveOptionToContacts && is_a_contact && it->second.mOwnOpinion == RsReputations::OPINION_NEUTRAL) - should_set_to_positive.push_back(*rit) ; + should_set_to_positive.push_back(it->first) ; } } diff --git a/libretroshare/src/services/p3gxsreputation.h b/libretroshare/src/services/p3gxsreputation.h index ceaab3b85..981e8b672 100644 --- a/libretroshare/src/services/p3gxsreputation.h +++ b/libretroshare/src/services/p3gxsreputation.h @@ -40,6 +40,7 @@ static const uint32_t REPUTATION_IDENTITY_FLAG_PGP_KNOWN = 0x0002; #include "retroshare/rsidentity.h" #include "retroshare/rsreputations.h" +#include "gxs/rsgixs.h" #include "services/p3service.h" @@ -100,7 +101,7 @@ public: * */ -class p3GxsReputation: public p3Service, public p3Config, public RsReputations /* , public pqiMonitor */ +class p3GxsReputation: public p3Service, public p3Config, public RsGixsReputation, public RsReputations /* , public pqiMonitor */ { public: p3GxsReputation(p3LinkMgr *lm); diff --git a/libretroshare/src/services/p3idservice.cc b/libretroshare/src/services/p3idservice.cc index 88ea21072..8ee7f51fb 100644 --- a/libretroshare/src/services/p3idservice.cc +++ b/libretroshare/src/services/p3idservice.cc @@ -1030,6 +1030,7 @@ bool p3IdService::decryptData(const uint8_t *encrypted_data,uint32_t encrypted_d } +#ifdef TO_BE_REMOVED /********************************************************************************/ /******************* RsGixsReputation ***************************************/ /********************************************************************************/ @@ -1083,6 +1084,7 @@ bool p3IdService::getReputation(const RsGxsId &id, GixsReputation &rep) } return false; } +#endif #if 0 class RegistrationRequest diff --git a/libretroshare/src/services/p3idservice.h b/libretroshare/src/services/p3idservice.h index 1261195c1..e9e6ed9d5 100644 --- a/libretroshare/src/services/p3idservice.h +++ b/libretroshare/src/services/p3idservice.h @@ -303,9 +303,11 @@ public: /**************** RsGixsReputation Implementation ****************/ // get Reputation. +#ifdef TO_BE_REMOVED virtual bool haveReputation(const RsGxsId &id); virtual bool loadReputation(const RsGxsId &id, const std::list& peers); virtual bool getReputation(const RsGxsId &id, GixsReputation &rep); +#endif protected: diff --git a/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp b/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp index a2535c38e..79ec53e64 100644 --- a/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp +++ b/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp @@ -110,14 +110,14 @@ void GxsIdRSTreeWidgetItem::setId(const RsGxsId &id, int column, bool retryWhenF void GxsIdRSTreeWidgetItem::updateBannedState() { - if(mBannedState != (rsIdentity->overallReputationLevel(mId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE)) + if(mBannedState != (rsReputations->overallReputationLevel(mId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE)) forceUpdate() ; } void GxsIdRSTreeWidgetItem::forceUpdate() { mIdFound = false; - mBannedState = (rsIdentity->overallReputationLevel(mId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE); + mBannedState = (rsReputations->overallReputationLevel(mId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE); startProcess(); } @@ -163,7 +163,7 @@ QVariant GxsIdRSTreeWidgetItem::data(int column, int role) const if(mId.isNull()) return RSTreeWidgetItem::data(column, role); - else if(rsIdentity->overallReputationLevel(mId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) + else if(rsReputations->overallReputationLevel(mId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE) pix = QImage(BANNED_IMAGE) ; else if (mAvatar.mSize == 0 || !pix.loadFromData(mAvatar.mData, mAvatar.mSize, "PNG")) pix = GxsIdDetails::makeDefaultIcon(mId); diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index cc0890077..581fe43ed 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -1546,7 +1546,7 @@ void GxsForumThreadWidget::insertMessageData(const RsGxsForumMsg &msg) return; } - uint32_t overall_reputation = rsIdentity->overallReputationLevel(msg.mMeta.mAuthorId) ; + uint32_t overall_reputation = rsReputations->overallReputationLevel(msg.mMeta.mAuthorId) ; bool redacted = (overall_reputation == RsReputations::REPUTATION_LOCALLY_NEGATIVE) ; mStateHelper->setActive(mTokenTypeMessageData, true); From a7f0fff0f158782d9bdb2a96b0e8b0c0e09f66d9 Mon Sep 17 00:00:00 2001 From: csoler Date: Tue, 10 Jan 2017 23:05:00 +0100 Subject: [PATCH 06/21] made sure reputations are not stamped when requested for debugging/printing --- libretroshare/src/retroshare/rsreputations.h | 2 +- libretroshare/src/services/p3gxsreputation.cc | 35 +++++++++++++++---- libretroshare/src/services/p3gxsreputation.h | 2 +- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/libretroshare/src/retroshare/rsreputations.h b/libretroshare/src/retroshare/rsreputations.h index ac518ba6c..811806f4f 100644 --- a/libretroshare/src/retroshare/rsreputations.h +++ b/libretroshare/src/retroshare/rsreputations.h @@ -61,7 +61,7 @@ public: }; virtual bool setOwnOpinion(const RsGxsId& key_id, const Opinion& op) =0; - virtual bool getReputationInfo(const RsGxsId& id, const RsPgpId &ownerNode, ReputationInfo& info) =0; + virtual bool getReputationInfo(const RsGxsId& id, const RsPgpId &ownerNode, ReputationInfo& info,bool stamp=true) =0; virtual ReputationLevel overallReputationLevel(const RsGxsId& id)=0; // parameters diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index d5d0e2e7b..86d00d1f5 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -750,7 +750,7 @@ RsReputations::ReputationLevel p3GxsReputation::overallReputationLevel(const RsG return info.mOverallReputationLevel ; } -bool p3GxsReputation::getReputationInfo(const RsGxsId& gxsid, const RsPgpId& ownerNode, RsReputations::ReputationInfo& info) +bool p3GxsReputation::getReputationInfo(const RsGxsId& gxsid, const RsPgpId& ownerNode, RsReputations::ReputationInfo& info, bool stamp) { if(gxsid.isNull()) return false ; @@ -760,7 +760,7 @@ bool p3GxsReputation::getReputationInfo(const RsGxsId& gxsid, const RsPgpId& own RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ #ifdef DEBUG_REPUTATION2 - std::cerr << "getReputationInfo() for " << gxsid << std::endl; + std::cerr << "getReputationInfo() for " << gxsid << ", stamp = " << stamp << std::endl; #endif std::map::iterator it = mReputations.find(gxsid) ; RsPgpId owner_id ; @@ -787,7 +787,9 @@ bool p3GxsReputation::getReputationInfo(const RsGxsId& gxsid, const RsPgpId& own rep.mOwnerNode = ownerNode ; owner_id = rep.mOwnerNode ; - rep.mLastUsedTS = now ; + + if(stamp) + rep.mLastUsedTS = now ; mChanged = true ; } @@ -1274,7 +1276,7 @@ bool p3GxsReputation::loadReputationSet(RsGxsReputationSetItem *item, const std: } #ifdef DEBUG_REPUTATION RsReputations::ReputationInfo info ; - getReputationInfo(item->mGxsId,item->mOwnerNodeId,info) ; + getReputationInfo(item->mGxsId,item->mOwnerNodeId,info,false) ; std::cerr << item->mGxsId << " : own: " << info.mOwnOpinion << ", owner node: " << item->mOwnerNodeId << ", level: " << info.mOverallReputationLevel << std::endl; #endif return true; @@ -1468,19 +1470,38 @@ void p3GxsReputation::debug_print() { std::cerr << "Reputations database: " << std::endl; std::cerr << " GXS ID data: " << std::endl; + std::cerr << std::dec ; +std::map rep_copy; + +{ + RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ + rep_copy = mReputations ; +} time_t now = time(NULL) ; - for(std::map::const_iterator it(mReputations.begin());it!=mReputations.end();++it) + for(std::map::const_iterator it(rep_copy.begin());it!=rep_copy.end();++it) { - std::cerr << " " << it->first << ": own: " << it->second.mOwnOpinion << ", Friend average: " << it->second.mFriendAverage << ", global_score: " << it->second.mReputationScore - << ", last own update: " << now - it->second.mOwnOpinionTs << " secs ago, last needed: " << now - it->second.mLastUsedTS << " secs ago." << std::endl; + RsReputations::ReputationInfo info ; + getReputationInfo(it->first,RsPgpId(),info,false) ; + uint32_t lev = info.mOverallReputationLevel; + + std::cerr << " " << it->first << ": own: " << it->second.mOwnOpinion + << ", PGP id=" << it->second.mOwnerNode + << ", flags=" << std::setfill('0') << std::setw(4) << std::hex << it->second.mIdentityFlags << std::dec + << ", Friend pos/neg: " << it->second.mFriendsPositive << "/" << it->second.mFriendsNegative + << ", reputation lev: [" << lev + << "], last own update: " << std::setfill(' ') << std::setw(10) << now - it->second.mOwnOpinionTs << " secs ago" + << ", last needed: " << std::setfill(' ') << std::setw(10) << now - it->second.mLastUsedTS << " secs ago, " + << std::endl; + #ifdef DEBUG_REPUTATION2 for(std::map::const_iterator it2(it->second.mOpinions.begin());it2!=it->second.mOpinions.end();++it2) std::cerr << " " << it2->first << ": " << it2->second << std::endl; #endif } + RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ std::cerr << " Banned RS nodes by ID: " << std::endl; for(std::map::const_iterator it(mBannedPgpIds.begin());it!=mBannedPgpIds.end();++it) diff --git a/libretroshare/src/services/p3gxsreputation.h b/libretroshare/src/services/p3gxsreputation.h index 981e8b672..e6d4ac11d 100644 --- a/libretroshare/src/services/p3gxsreputation.h +++ b/libretroshare/src/services/p3gxsreputation.h @@ -109,7 +109,7 @@ public: /***** Interface for RsReputations *****/ virtual bool setOwnOpinion(const RsGxsId& key_id, const Opinion& op) ; - virtual bool getReputationInfo(const RsGxsId& id, const RsPgpId &ownerNode, ReputationInfo& info) ; + virtual bool getReputationInfo(const RsGxsId& id, const RsPgpId &ownerNode, ReputationInfo& info,bool stamp=true) ; virtual bool isIdentityBanned(const RsGxsId& id) ; virtual bool isNodeBanned(const RsPgpId& id); From bd7f6aca9997c144490b711b549ea391a02ebb71 Mon Sep 17 00:00:00 2001 From: csoler Date: Thu, 12 Jan 2017 20:39:49 +0100 Subject: [PATCH 07/21] added to parameters in options to fix the time banned ids are kept in list and prevented to re-download --- libretroshare/src/retroshare/rsidentity.h | 3 + libretroshare/src/retroshare/rsreputations.h | 3 + libretroshare/src/services/p3gxsreputation.cc | 40 ++++++++++-- libretroshare/src/services/p3gxsreputation.h | 4 ++ libretroshare/src/services/p3idservice.cc | 54 ++++++++++++++-- libretroshare/src/services/p3idservice.h | 4 ++ .../src/gui/settings/PeoplePage.cpp | 5 ++ retroshare-gui/src/gui/settings/PeoplePage.ui | 64 +++++++++++++++++++ 8 files changed, 168 insertions(+), 9 deletions(-) diff --git a/libretroshare/src/retroshare/rsidentity.h b/libretroshare/src/retroshare/rsidentity.h index d6f0670f7..ac0bcdf53 100644 --- a/libretroshare/src/retroshare/rsidentity.h +++ b/libretroshare/src/retroshare/rsidentity.h @@ -289,6 +289,9 @@ public: virtual bool updateIdentity(uint32_t& token, RsGxsIdGroup &group) = 0; virtual bool deleteIdentity(uint32_t& token, RsGxsIdGroup &group) = 0; + virtual void setDeleteBannedNodesThreshold(uint32_t days) =0; + virtual uint32_t deleteBannedNodesThreshold() =0; + virtual bool parseRecognTag(const RsGxsId &id, const std::string &nickname, const std::string &tag, RsRecognTagDetails &details) = 0; virtual bool getRecognTagRequest(const RsGxsId &id, const std::string &comment, diff --git a/libretroshare/src/retroshare/rsreputations.h b/libretroshare/src/retroshare/rsreputations.h index 811806f4f..c8ff572db 100644 --- a/libretroshare/src/retroshare/rsreputations.h +++ b/libretroshare/src/retroshare/rsreputations.h @@ -74,6 +74,9 @@ public: virtual void setThresholdForRemotelyNegativeReputation(uint32_t thresh)=0; virtual void setThresholdForRemotelyPositiveReputation(uint32_t thresh)=0; + virtual void setRememberDeletedNodesThreshold(uint32_t days) =0; + virtual uint32_t rememberDeletedNodesThreshold() =0; + // This one is a proxy designed to allow fast checking of a GXS id. // it basically returns true if assessment is not ASSESSMENT_OK diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index 86d00d1f5..a026e8d0d 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -139,8 +139,8 @@ static const float REPUTATION_ASSESSMENT_THRESHOLD_X1 = 0.5f ; // reputat static const uint32_t PGP_AUTO_BAN_THRESHOLD_DEFAULT = 2 ; // above this, auto ban any GXS id signed by this node static const uint32_t IDENTITY_FLAGS_UPDATE_DELAY = 100 ; // static const uint32_t BANNED_NODES_UPDATE_DELAY = 313 ; // update approx every 5 mins. Chosen to not be a multiple of IDENTITY_FLAGS_UPDATE_DELAY -static const uint32_t REPUTATION_INFO_KEEP_DELAY = 86400*35; // remove old reputation info 5 days after last usage limit, in case the ID would come back.. -static const uint32_t BANNED_NODES_INACTIVITY_KEEP = 86400*60; // remove all info about banned nodes after 2 months of inactivity +static const uint32_t REPUTATION_INFO_KEEP_DELAY_DEFAULT = 86400*35; // remove old reputation info 5 days after last usage limit, in case the ID would come back.. +static const uint32_t BANNED_NODES_INACTIVITY_KEEP_DEFAULT = 86400*60; // remove all info about banned nodes after 2 months of inactivity static const uint32_t REPUTATION_DEFAULT_MIN_VOTES_FOR_REMOTELY_POSITIVE = 1; // min difference in votes that makes friends opinion globally positive static const uint32_t REPUTATION_DEFAULT_MIN_VOTES_FOR_REMOTELY_NEGATIVE = 1; // min difference in votes that makes friends opinion globally negative @@ -257,6 +257,23 @@ bool p3GxsReputation::nodeAutoPositiveOpinionForContacts() return mAutoSetPositiveOptionToContacts ; } +void p3GxsReputation::setRememberDeletedNodesThreshold(uint32_t days) +{ + RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ + + if(mMaxPreventReloadBannedIds != days/86400) + { + mMaxPreventReloadBannedIds = days/86400 ; + IndicateConfigChanged(); + } +} +uint32_t p3GxsReputation::rememberDeletedNodesThreshold() +{ + RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ + + return mMaxPreventReloadBannedIds/86400; +} + int p3GxsReputation::status() { return 1; @@ -379,10 +396,10 @@ void p3GxsReputation::cleanup() // Delete slots that havn't been used for a while. The else below is here for debug display purposes, and not harmful since both conditions lead the same effect. - else if(it->second.mLastUsedTS + REPUTATION_INFO_KEEP_DELAY < now) + else if(it->second.mLastUsedTS + REPUTATION_INFO_KEEP_DELAY_DEFAULT < now) { #ifdef DEBUG_REPUTATION - std::cerr << " ID " << it->first << ": no request for reputation for more than " << REPUTATION_INFO_KEEP_DELAY/86400 << " days => deleting." << std::endl; + std::cerr << " ID " << it->first << ": no request for reputation for more than " << REPUTATION_INFO_KEEP_DELAY_DEFAULT/86400 << " days => deleting." << std::endl; #endif should_delete = true ; } @@ -410,7 +427,7 @@ void p3GxsReputation::cleanup() RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ for(std::map::iterator it(mBannedPgpIds.begin());it!=mBannedPgpIds.end();) - if(it->second.last_activity_TS + BANNED_NODES_INACTIVITY_KEEP < now) + if(it->second.last_activity_TS + BANNED_NODES_INACTIVITY_KEEP_DEFAULT < now) { #ifdef DEBUG_REPUTATION std::cerr << " Removing all info about banned node " << it->first << " by lack of activity." << std::endl; @@ -1084,6 +1101,10 @@ bool p3GxsReputation::saveList(bool& cleanup, std::list &savelist) kv.value = mAutoSetPositiveOptionToContacts?"YES":"NO"; vitem->tlvkvs.pairs.push_back(kv) ; + kv.key = "MAX_PREVENT_RELOAD_BANNED_IDS" ; + rs_sprintf(kv.value, "%d", mMaxPreventReloadBannedIds) ; + vitem->tlvkvs.pairs.push_back(kv) ; + savelist.push_back(vitem) ; return true; @@ -1170,6 +1191,15 @@ bool p3GxsReputation::loadList(std::list& loadList) std::cerr << "Setting AutoPositiveContacts to " << kit->value << std::endl ; mLastBannedNodesUpdate = 0 ; // force update } + if(kit->key == "MAX_PREVENT_RELOAD_BANNED_IDS" ) + { + int val ; + if (sscanf(kit->value.c_str(), "%d", &val) == 1) + { + mMaxPreventReloadBannedIds = val ; + std::cerr << "Setting mMaxPreventReloadBannedIds threshold to " << val << std::endl ; + } + } } delete (*it); diff --git a/libretroshare/src/services/p3gxsreputation.h b/libretroshare/src/services/p3gxsreputation.h index e6d4ac11d..e6b6c7f66 100644 --- a/libretroshare/src/services/p3gxsreputation.h +++ b/libretroshare/src/services/p3gxsreputation.h @@ -119,6 +119,9 @@ public: virtual void setNodeAutoPositiveOpinionForContacts(bool b) ; virtual bool nodeAutoPositiveOpinionForContacts() ; + virtual void setRememberDeletedNodesThreshold(uint32_t days) ; + virtual uint32_t rememberDeletedNodesThreshold() ; + uint32_t thresholdForRemotelyNegativeReputation(); uint32_t thresholdForRemotelyPositiveReputation(); void setThresholdForRemotelyNegativeReputation(uint32_t thresh); @@ -186,6 +189,7 @@ private: uint32_t mMinVotesForRemotelyPositive ; uint32_t mMinVotesForRemotelyNegative ; + uint32_t mMaxPreventReloadBannedIds ; bool mChanged ; // slow version of IndicateConfigChanged(); time_t mLastReputationConfigSaved ; diff --git a/libretroshare/src/services/p3idservice.cc b/libretroshare/src/services/p3idservice.cc index 8ee7f51fb..323a49ae2 100644 --- a/libretroshare/src/services/p3idservice.cc +++ b/libretroshare/src/services/p3idservice.cc @@ -62,7 +62,8 @@ // unused keys are deleted according to some heuristic that should favor known keys, signed keys etc. -static const time_t MAX_KEEP_KEYS_BANNED = 2 * 86400 ; // get rid of banned ids after 1 days. That gives a chance to un-ban someone before he gets definitely kicked out +static const time_t MAX_KEEP_KEYS_BANNED_DEFAULT = 2 * 86400 ; // get rid of banned ids after 1 days. That gives a chance to un-ban someone before he gets definitely kicked out + static const time_t MAX_KEEP_KEYS_DEFAULT = 5 * 86400 ; // default for unsigned identities: 5 days static const time_t MAX_KEEP_KEYS_SIGNED = 8 * 86400 ; // signed identities by unknown key static const time_t MAX_KEEP_KEYS_SIGNED_KNOWN = 30 * 86400 ; // signed identities by known node keys @@ -164,6 +165,7 @@ p3IdService::p3IdService(RsGeneralDataService *gds, RsNetworkExchangeService *ne mLastKeyCleaningTime = time(NULL) - int(MAX_DELAY_BEFORE_CLEANING * 0.9) ; mLastConfigUpdate = 0 ; mOwnIdsLoaded = false ; + mMaxKeepKeysBanned = MAX_KEEP_KEYS_BANNED_DEFAULT; // Kick off Cache Testing, + Others. RsTickEvent::schedule_in(GXSID_EVENT_PGPHASH, PGPHASH_PERIOD); @@ -320,6 +322,22 @@ bool p3IdService::loadList(std::list& items) mContacts = lii->mContacts ; } + RsConfigKeyValueSet *vitem = dynamic_cast(*it); + + if(vitem) + for(std::list::const_iterator kit = vitem->tlvkvs.pairs.begin(); kit != vitem->tlvkvs.pairs.end(); ++kit) + { + if(kit->key == "REMOVE_BANNED_IDENTITIES_DELAY") + { + int val ; + if (sscanf(kit->value.c_str(), "%d", &val) == 1) + { + mMaxKeepKeysBanned = val ; + std::cerr << "Setting mMaxKeepKeysBanned threshold to " << val << std::endl ; + } + }; + } + delete *it ; } @@ -327,6 +345,23 @@ bool p3IdService::loadList(std::list& items) return true ; } +void p3IdService::setDeleteBannedNodesThreshold(uint32_t days) +{ + RsStackMutex stack(mIdMtx); /****** LOCKED MUTEX *******/ + if(mMaxKeepKeysBanned != days*86400) + { + mMaxKeepKeysBanned = days*86400 ; + IndicateConfigChanged(); + } +} +uint32_t p3IdService::deleteBannedNodesThreshold() +{ + RsStackMutex stack(mIdMtx); /****** LOCKED MUTEX *******/ + + return mMaxKeepKeysBanned/86400; +} + + bool p3IdService::saveList(bool& cleanup,std::list& items) { #ifdef DEBUG_IDS @@ -343,13 +378,23 @@ bool p3IdService::saveList(bool& cleanup,std::list& items) item->mContacts = mContacts ; items.push_back(item) ; + + RsConfigKeyValueSet *vitem = new RsConfigKeyValueSet ; + RsTlvKeyValue kv; + + kv.key = "REMOVE_BANNED_IDENTITIES_DELAY" ; + rs_sprintf(kv.value, "%d", mMaxKeepKeysBanned); + vitem->tlvkvs.pairs.push_back(kv) ; + + items.push_back(vitem) ; + return true ; } class IdCacheEntryCleaner { public: - IdCacheEntryCleaner(const std::map& last_usage_TSs) : mLastUsageTS(last_usage_TSs) {} + IdCacheEntryCleaner(const std::map& last_usage_TSs,uint32_t m) : mLastUsageTS(last_usage_TSs),mMaxKeepKeysBanned(m) {} bool processEntry(RsGxsIdCache& entry) { @@ -380,7 +425,7 @@ public: if(no_ts) max_keep_time = 0 ; else if(is_id_banned) - max_keep_time = MAX_KEEP_KEYS_BANNED ; + max_keep_time = mMaxKeepKeysBanned ; else if(is_known_id) max_keep_time = MAX_KEEP_KEYS_SIGNED_KNOWN ; else if(is_signed_id) @@ -403,6 +448,7 @@ public: std::list ids_to_delete ; const std::map& mLastUsageTS; + uint32_t mMaxKeepKeysBanned ; }; void p3IdService::cleanUnusedKeys() @@ -422,7 +468,7 @@ void p3IdService::cleanUnusedKeys() } // grab at most 10 identities to delete. No need to send too many requests to the token queue at once. - IdCacheEntryCleaner idcec(mKeysTS) ; + IdCacheEntryCleaner idcec(mKeysTS,mMaxKeepKeysBanned) ; mKeyCache.applyToAllCachedEntries(idcec,&IdCacheEntryCleaner::processEntry); diff --git a/libretroshare/src/services/p3idservice.h b/libretroshare/src/services/p3idservice.h index e9e6ed9d5..addff0d1b 100644 --- a/libretroshare/src/services/p3idservice.h +++ b/libretroshare/src/services/p3idservice.h @@ -266,6 +266,9 @@ public: virtual bool updateIdentity(uint32_t& token, RsGxsIdGroup &group); virtual bool deleteIdentity(uint32_t& token, RsGxsIdGroup &group); + virtual void setDeleteBannedNodesThreshold(uint32_t days) ; + virtual uint32_t deleteBannedNodesThreshold() ; + virtual bool parseRecognTag(const RsGxsId &id, const std::string &nickname, const std::string &tag, RsRecognTagDetails &details); virtual bool getRecognTagRequest(const RsGxsId &id, const std::string &comment, @@ -536,6 +539,7 @@ private: time_t mLastConfigUpdate ; bool mOwnIdsLoaded ; + uint32_t mMaxKeepKeysBanned ; }; #endif // P3_IDENTITY_SERVICE_HEADER diff --git a/retroshare-gui/src/gui/settings/PeoplePage.cpp b/retroshare-gui/src/gui/settings/PeoplePage.cpp index 9a52cdb35..66265f4de 100644 --- a/retroshare-gui/src/gui/settings/PeoplePage.cpp +++ b/retroshare-gui/src/gui/settings/PeoplePage.cpp @@ -22,6 +22,7 @@ #include "PeoplePage.h" #include "rsharesettings.h" #include "retroshare/rsreputations.h" +#include "retroshare/rsidentity.h" PeoplePage::PeoplePage(QWidget * parent, Qt::WindowFlags flags) : ConfigPage(parent, flags) @@ -44,6 +45,8 @@ bool PeoplePage::save(QString &/*errmsg*/) rsReputations->setThresholdForRemotelyPositiveReputation(ui.thresholdForPositive_SB->value()); rsReputations->setThresholdForRemotelyNegativeReputation(ui.thresholdForNegative_SB->value()); + rsReputations->setRememberDeletedNodesThreshold(ui.preventReloadingBannedIdentitiesFor_SB->value()); + rsIdentity->setDeleteBannedNodesThreshold(ui.deleteBannedIdentitiesAfter_SB->value()); return true; } @@ -58,4 +61,6 @@ void PeoplePage::load() ui.autoPositiveOpinion_CB->setChecked(auto_positive_contacts); ui.thresholdForPositive_SB->setValue(threshold_for_positive); ui.thresholdForNegative_SB->setValue(threshold_for_negative); + ui.deleteBannedIdentitiesAfter_SB->setValue(rsIdentity->deleteBannedNodesThreshold()); + ui.preventReloadingBannedIdentitiesFor_SB->setValue(rsReputations->rememberDeletedNodesThreshold()); } diff --git a/retroshare-gui/src/gui/settings/PeoplePage.ui b/retroshare-gui/src/gui/settings/PeoplePage.ui index 133634c0c..301b3a151 100644 --- a/retroshare-gui/src/gui/settings/PeoplePage.ui +++ b/retroshare-gui/src/gui/settings/PeoplePage.ui @@ -34,9 +34,15 @@ + + Qt::RightToLeft + Difference in votes to make friends globally negative: + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + @@ -67,9 +73,67 @@ + + Qt::RightToLeft + Difference in votes to make friends globally positive: + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Qt::RightToLeft + + + Delete banned identities after: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + Qt::RightToLeft + + + Prevent re-loading previously banned identities for: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + + + + + days + + + 1 + + + 5 + + + + + + + days + + + 1 + + + 60 + From 70a92a1c3212bc462540ec273663c5fe50663fec Mon Sep 17 00:00:00 2001 From: csoler Date: Thu, 12 Jan 2017 21:14:36 +0100 Subject: [PATCH 08/21] various small fixes in reputation/identity cleaning --- libretroshare/src/services/p3gxsreputation.cc | 13 ++++++++++-- libretroshare/src/services/p3idservice.cc | 14 +++++++++---- retroshare-gui/src/gui/settings/PeoplePage.ui | 20 ++++++++++++------- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index a026e8d0d..d8a05818d 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -166,6 +166,7 @@ p3GxsReputation::p3GxsReputation(p3LinkMgr *lm) mLastReputationConfigSaved = 0; mChanged = false ; + mMaxPreventReloadBannedIds = BANNED_NODES_INACTIVITY_KEEP_DEFAULT; } const std::string GXS_REPUTATION_APP_NAME = "gxsreputation"; @@ -261,9 +262,9 @@ void p3GxsReputation::setRememberDeletedNodesThreshold(uint32_t days) { RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ - if(mMaxPreventReloadBannedIds != days/86400) + if(mMaxPreventReloadBannedIds != days*86400) { - mMaxPreventReloadBannedIds = days/86400 ; + mMaxPreventReloadBannedIds = days*86400 ; IndicateConfigChanged(); } } @@ -384,6 +385,14 @@ void p3GxsReputation::cleanup() { bool should_delete = false ; + if(it->second.mOwnOpinion == RsReputations::OPINION_NEGATIVE && mMaxPreventReloadBannedIds != 0 && it->second.mOwnOpinionTs + mMaxPreventReloadBannedIds < now) + { +#ifdef DEBUG_REPUTATION + std::cerr << " ID " << it->first << ": own is negative for more than " << mMaxPreventReloadBannedIds/86400 << " days. Reseting it!" << std::endl; +#endif + mChanged = true ; + } + // Delete slots with basically no information if(it->second.mOpinions.empty() && it->second.mOwnOpinion == RsReputations::OPINION_NEUTRAL && (it->second.mOwnerNode.isNull())) diff --git a/libretroshare/src/services/p3idservice.cc b/libretroshare/src/services/p3idservice.cc index 323a49ae2..7c5127913 100644 --- a/libretroshare/src/services/p3idservice.cc +++ b/libretroshare/src/services/p3idservice.cc @@ -421,12 +421,18 @@ public: time_t last_usage_ts = no_ts?0:(it->second.TS); time_t max_keep_time ; + bool should_check = true ; if(no_ts) max_keep_time = 0 ; - else if(is_id_banned) - max_keep_time = mMaxKeepKeysBanned ; - else if(is_known_id) + else if(is_id_banned) + { + if(mMaxKeepKeysBanned == 0) + should_check = false ; + else + max_keep_time = mMaxKeepKeysBanned ; + } + else if(is_known_id) max_keep_time = MAX_KEEP_KEYS_SIGNED_KNOWN ; else if(is_signed_id) max_keep_time = MAX_KEEP_KEYS_SIGNED ; @@ -435,7 +441,7 @@ public: std::cerr << ". Max keep = " << max_keep_time/86400 << " days. Unused for " << (now - last_usage_ts + 86399)/86400 << " days " ; - if(now > last_usage_ts + max_keep_time) + if(should_check && now > last_usage_ts + max_keep_time) { std::cerr << " => delete " << std::endl; ids_to_delete.push_back(gxs_id) ; diff --git a/retroshare-gui/src/gui/settings/PeoplePage.ui b/retroshare-gui/src/gui/settings/PeoplePage.ui index 301b3a151..c40bbec95 100644 --- a/retroshare-gui/src/gui/settings/PeoplePage.ui +++ b/retroshare-gui/src/gui/settings/PeoplePage.ui @@ -14,7 +14,7 @@ - Identities handling + Reputation @@ -41,7 +41,7 @@ Difference in votes to make friends globally negative: - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter @@ -80,7 +80,7 @@ Difference in votes to make friends globally positive: - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter @@ -90,10 +90,10 @@ Qt::RightToLeft - Delete banned identities after: + Delete banned identities after (0 means indefinitely): - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter @@ -103,15 +103,18 @@ Qt::RightToLeft - Prevent re-loading previously banned identities for: + Reset reputation of banned identities after (0 means never): - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter + + <html><head/><body><p>Banned identities are not stamped and therefore lose activity. They get deleted automatically after a finit period of time.</p></body></html> + days @@ -125,6 +128,9 @@ + + <html><head/><body><p>In order to prevent deleted banned IDs to come back because they are used in e.g. forums or channels, banned identities are kept in a list for some time. After that, they are &quot;cleared&quot; from the banning list, and will be downloaded again as unbanned if used in forus, chat rooms, etc.</p></body></html> + days From 3c07d50dacd48889ee6255ddbaad0c11ffb2f7db Mon Sep 17 00:00:00 2001 From: csoler Date: Thu, 12 Jan 2017 21:27:02 +0100 Subject: [PATCH 09/21] set auto-reset of banned nodes to "never" by default --- libretroshare/src/services/p3gxsreputation.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index d8a05818d..31ad5610b 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -166,7 +166,7 @@ p3GxsReputation::p3GxsReputation(p3LinkMgr *lm) mLastReputationConfigSaved = 0; mChanged = false ; - mMaxPreventReloadBannedIds = BANNED_NODES_INACTIVITY_KEEP_DEFAULT; + mMaxPreventReloadBannedIds = 0 ; // default is "never" } const std::string GXS_REPUTATION_APP_NAME = "gxsreputation"; From d7e068220de49e57a5fe222a87babe442280e84e Mon Sep 17 00:00:00 2001 From: csoler Date: Thu, 12 Jan 2017 21:59:44 +0100 Subject: [PATCH 10/21] fixed sorting by reputation in IdDialog --- retroshare-gui/src/gui/Identity/IdDialog.cpp | 31 ++++++++++++-------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/retroshare-gui/src/gui/Identity/IdDialog.cpp b/retroshare-gui/src/gui/Identity/IdDialog.cpp index e8d16957e..811742b62 100644 --- a/retroshare-gui/src/gui/Identity/IdDialog.cpp +++ b/retroshare-gui/src/gui/Identity/IdDialog.cpp @@ -110,24 +110,29 @@ // comment this out in order to remove the sorting of circles into "belong to" and "other visible circles" #define CIRCLE_MEMBERSHIP_CATEGORIES 1 +static const uint32_t SortRole = Qt::UserRole+1 ; + // quick solution for RSID_COL_VOTES sorting -class TreeWidgetItem : public QTreeWidgetItem { +class TreeWidgetItem : public QTreeWidgetItem +{ public: TreeWidgetItem(int type=Type): QTreeWidgetItem(type) {} TreeWidgetItem(QTreeWidget *tree): QTreeWidgetItem(tree) {} TreeWidgetItem(const QStringList& strings): QTreeWidgetItem (strings) {} - bool operator< (const QTreeWidgetItem& other ) const { + + bool operator< (const QTreeWidgetItem& other ) const + { int column = treeWidget()->sortColumn(); - const QVariant v1 = data(column, Qt::DisplayRole); - const QVariant v2 = other.data(column, Qt::DisplayRole); - double value1 = v1.toDouble(); - double value2 = v2.toDouble(); - if (value1 != value2) { - return value1 < value2; - } - else { - return (v1.toString().compare (v2.toString(), Qt::CaseInsensitive) < 0); - } + + if(column == RSID_COL_VOTES) + { + const unsigned int v1 = data(column, SortRole).toUInt(); + const unsigned int v2 = other.data(column, SortRole).toUInt(); + + return v1 < v2; + } + else + return data(column,Qt::DisplayRole).toString() < other.data(column,Qt::DisplayRole).toString(); } }; @@ -1475,6 +1480,7 @@ bool IdDialog::fillIdListItem(const RsGxsIdGroup& data, QTreeWidgetItem *&item, item->setData(RSID_COL_KEYID, Qt::UserRole,QVariant(item_flags)) ; item->setTextAlignment(RSID_COL_VOTES, Qt::AlignRight | Qt::AlignVCenter); item->setData(RSID_COL_VOTES,Qt::DecorationRole, idd.mReputation.mOverallReputationLevel); + item->setData(RSID_COL_VOTES,SortRole, idd.mReputation.mOverallReputationLevel); if(isOwnId) { @@ -1984,6 +1990,7 @@ QString IdDialog::createUsageString(const RsIdentityUsage& u) const default: return QString("Undone yet"); } + return QString("Unknown"); } void IdDialog::modifyReputation() From 19819b9b7754d4d8238a1139b7ac4a2d6e985d93 Mon Sep 17 00:00:00 2001 From: csoler Date: Thu, 12 Jan 2017 22:14:41 +0100 Subject: [PATCH 11/21] removed debug info --- libretroshare/src/services/p3gxsreputation.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index 31ad5610b..6812139b3 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -39,7 +39,6 @@ /**** * #define DEBUG_REPUTATION 1 ****/ -#define DEBUG_REPUTATION 1 /************ IMPLEMENTATION NOTES ********************************* * @@ -370,8 +369,8 @@ void p3GxsReputation::cleanup() // remove opinions from friends that havn't been seen online for more than the specified delay #ifdef DEBUG_REPUTATION -#endif std::cerr << "p3GxsReputation::cleanup() " << std::endl; +#endif time_t now = time(NULL) ; // We should keep opinions about identities that do not exist anymore, but only rely on the usage TS. That will in particular avoid asking p3idservice about deleted From 3c800c45c6dc915e16eaf6021dbdac592cde9544 Mon Sep 17 00:00:00 2001 From: csoler Date: Thu, 12 Jan 2017 22:54:38 +0100 Subject: [PATCH 12/21] fixed enabled 0 in settings->People fields --- retroshare-gui/src/gui/settings/PeoplePage.ui | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/retroshare-gui/src/gui/settings/PeoplePage.ui b/retroshare-gui/src/gui/settings/PeoplePage.ui index c40bbec95..2c0b3b45e 100644 --- a/retroshare-gui/src/gui/settings/PeoplePage.ui +++ b/retroshare-gui/src/gui/settings/PeoplePage.ui @@ -119,7 +119,7 @@ days - 1 + 0 5 @@ -135,7 +135,7 @@ days - 1 + 0 60 From 17cf6607f8392fdf9e1d81848affe64a4535c361 Mon Sep 17 00:00:00 2001 From: csoler Date: Fri, 13 Jan 2017 16:35:42 +0100 Subject: [PATCH 13/21] improved settings->People text fields and tooltip --- retroshare-gui/src/gui/settings/PeoplePage.ui | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/retroshare-gui/src/gui/settings/PeoplePage.ui b/retroshare-gui/src/gui/settings/PeoplePage.ui index 2c0b3b45e..692d1a617 100644 --- a/retroshare-gui/src/gui/settings/PeoplePage.ui +++ b/retroshare-gui/src/gui/settings/PeoplePage.ui @@ -38,7 +38,7 @@ Qt::RightToLeft - Difference in votes to make friends globally negative: + Difference in votes (+/-) to rate an ID negatively: Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter @@ -48,7 +48,7 @@ - <html><head/><body><p>The default value of -0.6 needs 3 to 4 friends to set a negative opinion in order for that identity to be banned at your own node. This is a pretty conservative value. If you want to more easily get rid of banned identities, set the value to e.g. -0.2</p></body></html> + <html><head/><body><p>When an identity receives more negative votes than positive votes, it switches from &quot;Neutral&quot; to &quot;Negative (according to your friends)&quot;. By default, a one-vote difference is enough, but you can make this harder to happen by selecting a higher number here.</p></body></html> 1 @@ -61,7 +61,7 @@ - <html><head/><body><p>The default value of -0.6 needs 3 to 4 friends to set a negative opinion in order for that identity to be banned at your own node. This is a pretty conservative value. If you want to more easily get rid of banned identities, set the value to e.g. -0.2</p></body></html> + <html><head/><body><p>When an identity receives more positive votes than negative votes, it switches from &quot;Neutral&quot; to &quot;Positive (according to your friends)&quot;. By default, a one-vote difference is enough, but you can make this harder to happen by selecting a higher number here.</p></body></html> 1 @@ -69,6 +69,9 @@ 100 + + 1 + @@ -77,7 +80,7 @@ Qt::RightToLeft - Difference in votes to make friends globally positive: + Difference in votes (+/-) to rate an ID positively: Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter From df94de9142983adcde31ae40f030ca47d48088e6 Mon Sep 17 00:00:00 2001 From: csoler Date: Fri, 13 Jan 2017 18:31:50 +0100 Subject: [PATCH 14/21] disallow to ban your own identity in forums, and make opinions show up as a function ofwhat the ID opinion already is --- libretroshare/src/retroshare/rsreputations.h | 1 + libretroshare/src/services/p3gxsreputation.cc | 23 +++++++++++ libretroshare/src/services/p3gxsreputation.h | 1 + .../src/gui/gxs/GxsIdTreeWidgetItem.cpp | 10 ++--- .../src/gui/gxs/GxsIdTreeWidgetItem.h | 1 + .../gui/gxsforums/GxsForumThreadWidget.cpp | 39 +++++++++++++++---- 6 files changed, 63 insertions(+), 12 deletions(-) diff --git a/libretroshare/src/retroshare/rsreputations.h b/libretroshare/src/retroshare/rsreputations.h index c8ff572db..b5e9bd8d5 100644 --- a/libretroshare/src/retroshare/rsreputations.h +++ b/libretroshare/src/retroshare/rsreputations.h @@ -61,6 +61,7 @@ public: }; virtual bool setOwnOpinion(const RsGxsId& key_id, const Opinion& op) =0; + virtual bool getOwnOpinion(const RsGxsId& key_id, Opinion& op) =0; virtual bool getReputationInfo(const RsGxsId& id, const RsPgpId &ownerNode, ReputationInfo& info,bool stamp=true) =0; virtual ReputationLevel overallReputationLevel(const RsGxsId& id)=0; diff --git a/libretroshare/src/services/p3gxsreputation.cc b/libretroshare/src/services/p3gxsreputation.cc index 6812139b3..aa64bec14 100644 --- a/libretroshare/src/services/p3gxsreputation.cc +++ b/libretroshare/src/services/p3gxsreputation.cc @@ -959,6 +959,29 @@ bool p3GxsReputation::isIdentityBanned(const RsGxsId &id) return info.mOverallReputationLevel == RsReputations::REPUTATION_LOCALLY_NEGATIVE ; } +bool p3GxsReputation::getOwnOpinion(const RsGxsId& gxsid, RsReputations::Opinion& opinion) +{ +#ifdef DEBUG_REPUTATION + std::cerr << "setOwnOpinion(): for GXS id " << gxsid << " to " << opinion << std::endl; +#endif + if(gxsid.isNull()) + { + std::cerr << " ID " << gxsid << " is rejected. Look for a bug in calling method." << std::endl; + return false ; + } + + RsStackMutex stack(mReputationMtx); /****** LOCKED MUTEX *******/ + + std::map::iterator rit = mReputations.find(gxsid); + + if(rit != mReputations.end()) + opinion = RsReputations::Opinion(rit->second.mOwnOpinion) ; + else + opinion = RsReputations::OPINION_NEUTRAL ; + + return true; +} + bool p3GxsReputation::setOwnOpinion(const RsGxsId& gxsid, const RsReputations::Opinion& opinion) { #ifdef DEBUG_REPUTATION diff --git a/libretroshare/src/services/p3gxsreputation.h b/libretroshare/src/services/p3gxsreputation.h index e6b6c7f66..d8ca3a39c 100644 --- a/libretroshare/src/services/p3gxsreputation.h +++ b/libretroshare/src/services/p3gxsreputation.h @@ -109,6 +109,7 @@ public: /***** Interface for RsReputations *****/ virtual bool setOwnOpinion(const RsGxsId& key_id, const Opinion& op) ; + virtual bool getOwnOpinion(const RsGxsId& key_id, Opinion& op) ; virtual bool getReputationInfo(const RsGxsId& id, const RsPgpId &ownerNode, ReputationInfo& info,bool stamp=true) ; virtual bool isIdentityBanned(const RsGxsId& id) ; diff --git a/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp b/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp index 79ec53e64..fd83547ce 100644 --- a/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp +++ b/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp @@ -93,10 +93,10 @@ void GxsIdRSTreeWidgetItem::setId(const RsGxsId &id, int column, bool retryWhenF //std::cerr << " GxsIdRSTreeWidgetItem::setId(" << id << "," << column << ")"; //std::cerr << std::endl; - if (mIdFound) { - if (mColumn == column && mId == id) { + if (mIdFound) + { + if (mColumn == column && mId == id && (mBannedState == rsReputations->isIdentityBanned(mId))) return; - } } mIdFound = false; @@ -110,14 +110,14 @@ void GxsIdRSTreeWidgetItem::setId(const RsGxsId &id, int column, bool retryWhenF void GxsIdRSTreeWidgetItem::updateBannedState() { - if(mBannedState != (rsReputations->overallReputationLevel(mId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE)) + if(mBannedState != rsReputations->isIdentityBanned(mId)) forceUpdate() ; } void GxsIdRSTreeWidgetItem::forceUpdate() { mIdFound = false; - mBannedState = (rsReputations->overallReputationLevel(mId) == RsReputations::REPUTATION_LOCALLY_NEGATIVE); + mBannedState = (rsReputations->isIdentityBanned(mId)); startProcess(); } diff --git a/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.h b/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.h index dde64fdc6..e5c4002fd 100644 --- a/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.h +++ b/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.h @@ -67,6 +67,7 @@ private: bool mIdFound; bool mBannedState ; bool mRetryWhenFailed; + RsReputations::ReputationLevel mReputationLevel ; uint32_t mIconTypeMask; RsGxsImage mAvatar; }; diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index 581fe43ed..0a078e56c 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -574,15 +574,40 @@ void GxsForumThreadWidget::threadListCustomPopupMenu(QPoint /*point*/) contextMnu.addAction(expandAll); contextMnu.addAction(collapseAll); - contextMnu.addSeparator(); + QList selectedItems = ui->threadTreeWidget->selectedItems(); - QMenu *submenu1 = contextMnu.addMenu(tr("Author's reputation")) ; - submenu1->addAction(flagaspositiveAct); - submenu1->addAction(flagasneutralAct); - submenu1->addAction(flagasnegativeAct); - contextMnu.addAction(showinpeopleAct); + if(selectedItems.size() == 1) + { + QTreeWidgetItem *item = *selectedItems.begin(); + GxsIdRSTreeWidgetItem *gxsIdItem = dynamic_cast(item); - contextMnu.addAction(replyauthorAct); + RsGxsId author_id; + if(gxsIdItem && gxsIdItem->getId(author_id)) + { + std::cerr << "Author is: " << author_id << std::endl; + + contextMnu.addSeparator(); + + RsReputations::Opinion op ; + + if(!rsIdentity->isOwnId(author_id) && rsReputations->getOwnOpinion(author_id,op)) + { + QMenu *submenu1 = contextMnu.addMenu(tr("Author's reputation")) ; + + if(op != RsReputations::OPINION_POSITIVE) + submenu1->addAction(flagaspositiveAct); + + if(op != RsReputations::OPINION_NEUTRAL) + submenu1->addAction(flagasneutralAct); + + if(op != RsReputations::OPINION_NEGATIVE) + submenu1->addAction(flagasnegativeAct); + } + + contextMnu.addAction(showinpeopleAct); + contextMnu.addAction(replyauthorAct); + } + } contextMnu.exec(QCursor::pos()); } From c4695e3341d8d684e8a1b53feeac7edb05d0cce1 Mon Sep 17 00:00:00 2001 From: csoler Date: Fri, 13 Jan 2017 22:41:54 +0100 Subject: [PATCH 15/21] fixed compilation of unittests --- .../libretroshare/gxs/nxs_test/nxsdummyservices.cc | 2 +- .../unittests/libretroshare/gxs/nxs_test/nxsdummyservices.h | 2 ++ tests/unittests/libretroshare/services/gxs/GxsPeerNode.cc | 6 +++--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/unittests/libretroshare/gxs/nxs_test/nxsdummyservices.cc b/tests/unittests/libretroshare/gxs/nxs_test/nxsdummyservices.cc index 7b22d4e3a..216188145 100644 --- a/tests/unittests/libretroshare/gxs/nxs_test/nxsdummyservices.cc +++ b/tests/unittests/libretroshare/gxs/nxs_test/nxsdummyservices.cc @@ -62,7 +62,7 @@ bool rs_nxs_test::RsNxsSimpleDummyReputation::loadReputation(const RsGxsId& id, bool rs_nxs_test::RsNxsSimpleDummyReputation::getReputation(const RsGxsId& id, GixsReputation& rep) { - rep.score = 5; + rep.reputation_level = 5; return true; } diff --git a/tests/unittests/libretroshare/gxs/nxs_test/nxsdummyservices.h b/tests/unittests/libretroshare/gxs/nxs_test/nxsdummyservices.h index 23420bad1..2a7f3aeff 100644 --- a/tests/unittests/libretroshare/gxs/nxs_test/nxsdummyservices.h +++ b/tests/unittests/libretroshare/gxs/nxs_test/nxsdummyservices.h @@ -110,6 +110,8 @@ namespace rs_nxs_test bool loadReputation(const RsGxsId &id, const std::list& peers); bool getReputation(const RsGxsId &id, GixsReputation &rep); + virtual RsReputations::ReputationLevel overallReputationLevel(const RsGxsId&) { return RsReputations::REPUTATION_NEUTRAL ; } + private: RepMap mRepMap; diff --git a/tests/unittests/libretroshare/services/gxs/GxsPeerNode.cc b/tests/unittests/libretroshare/services/gxs/GxsPeerNode.cc index 85039d2fb..18156a693 100644 --- a/tests/unittests/libretroshare/services/gxs/GxsPeerNode.cc +++ b/tests/unittests/libretroshare/services/gxs/GxsPeerNode.cc @@ -67,7 +67,7 @@ GxsPeerNode::GxsPeerNode(const RsPeerId &ownId, const std::list &frien mGxsIdNs = new RsGxsNetService( RS_SERVICE_GXS_TYPE_GXSID, mGxsIdDs, nxsMgr, mGxsIdService, mGxsIdService->getServiceInfo(), - mGxsIdService, mGxsCircles,mGxsIdService, + NULL, mGxsCircles,mGxsIdService, mPgpAuxUtils, false); // don't synchronise group automatic (need explicit group request) @@ -81,7 +81,7 @@ GxsPeerNode::GxsPeerNode(const RsPeerId &ownId, const std::list &frien #endif (RS_SERVICE_GXS_TYPE_GXSCIRCLE, mGxsCirclesDs, nxsMgr, mGxsCircles, mGxsCircles->getServiceInfo(), - mGxsIdService, mGxsCircles,NULL, + NULL, mGxsCircles,NULL, mPgpAuxUtils); } else @@ -107,7 +107,7 @@ GxsPeerNode::GxsPeerNode(const RsPeerId &ownId, const std::list &frien #endif (RS_SERVICE_GXS_TYPE_TEST, mTestDs, nxsMgr, mTestService, mTestService->getServiceInfo(), - mGxsIdService, mGxsCircles,mGxsIdService, + NULL, mGxsCircles,mGxsIdService, mPgpAuxUtils); if (mUseIdentityService) From 14352220ab341405bf4cc33ae22840baadc92f54 Mon Sep 17 00:00:00 2001 From: csoler Date: Sat, 14 Jan 2017 10:19:31 +0100 Subject: [PATCH 16/21] fixed compilation in tests --- .../libretroshare/services/gxs/GxsIsolatedServiceTester.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unittests/libretroshare/services/gxs/GxsIsolatedServiceTester.cc b/tests/unittests/libretroshare/services/gxs/GxsIsolatedServiceTester.cc index 8d92889b0..38e47e862 100644 --- a/tests/unittests/libretroshare/services/gxs/GxsIsolatedServiceTester.cc +++ b/tests/unittests/libretroshare/services/gxs/GxsIsolatedServiceTester.cc @@ -50,7 +50,7 @@ GxsIsolatedServiceTester::GxsIsolatedServiceTester(const RsPeerId &ownId, const mTestNs = new RsGxsNetService( RS_SERVICE_GXS_TYPE_TEST, mTestDs, nxsMgr, mTestService, mTestService->getServiceInfo(), - mGxsIdService, mGxsCircles); + NULL, mGxsCircles); node->AddService(mTestNs); From 57f22e976438297d06ef50cbc4727f90394ac7fc Mon Sep 17 00:00:00 2001 From: csoler Date: Sat, 14 Jan 2017 10:51:14 +0100 Subject: [PATCH 17/21] removed costly size() call in QList --- retroshare-gui/src/gui/gxsforums/GxsForumsFillThread.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumsFillThread.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumsFillThread.cpp index 045495bd7..1a3cf5f57 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumsFillThread.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumsFillThread.cpp @@ -193,8 +193,10 @@ void GxsForumsFillThread::run() } /* process messages */ - while (msgs.size()) { - while (threadList.size() > 0) { + while (msgs.size()) + { + while (!threadList.empty()) + { if (wasStopped()) { break; } From 730869591d108ef1c4529c26c8a55fbae8a82289 Mon Sep 17 00:00:00 2001 From: csoler Date: Sat, 14 Jan 2017 11:02:02 +0100 Subject: [PATCH 18/21] fixed reloading of GXS id icon when banned/unbanned in forums --- retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp b/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp index fd83547ce..660cd47e6 100644 --- a/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp +++ b/retroshare-gui/src/gui/gxs/GxsIdTreeWidgetItem.cpp @@ -93,12 +93,10 @@ void GxsIdRSTreeWidgetItem::setId(const RsGxsId &id, int column, bool retryWhenF //std::cerr << " GxsIdRSTreeWidgetItem::setId(" << id << "," << column << ")"; //std::cerr << std::endl; - if (mIdFound) - { - if (mColumn == column && mId == id && (mBannedState == rsReputations->isIdentityBanned(mId))) + if (mIdFound && mColumn == column && mId == id && (mBannedState == rsReputations->isIdentityBanned(mId))) return; - } + mBannedState = rsReputations->isIdentityBanned(mId); mIdFound = false; mRetryWhenFailed = retryWhenFailed; From 25680d586cc011bde689904e9dfae83b53ed1380 Mon Sep 17 00:00:00 2001 From: csoler Date: Sat, 14 Jan 2017 17:29:37 +0100 Subject: [PATCH 19/21] removed quadratic cost in forum thread fill --- .../gui/gxsforums/GxsForumThreadWidget.cpp | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index 0a078e56c..18240d984 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -106,7 +106,11 @@ public: virtual void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const { - Q_ASSERT(index.isValid()); + if(!index.isValid()) + { + std::cerr << "(EE) attempt to draw an invalid index." << std::endl; + return ; + } QStyleOptionViewItemV4 opt = option; initStyleOption(&opt, index); @@ -122,6 +126,8 @@ public: // get pixmap unsigned int warning_level = qvariant_cast(index.data(Qt::DecorationRole)); + std::cerr << "Drawing item with warning level " << warning_level << std::endl; + switch(warning_level) { case 0: icon = QIcon(IMAGE_VOID); break; @@ -1357,49 +1363,45 @@ void GxsForumThreadWidget::fillThreads(QList &threadList, boo int index = 0; QTreeWidgetItem *threadItem; - QList::iterator newThread; + + // store new items in a map, so as to all + std::map newThreadMap ; + + for(QList::iterator newThread = threadList.begin (); newThread != threadList.end (); ++newThread) + newThreadMap[(*newThread)->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString()] = *newThread ; // delete not existing - while (index < ui->threadTreeWidget->topLevelItemCount()) { + while (index < ui->threadTreeWidget->topLevelItemCount()) + { threadItem = ui->threadTreeWidget->topLevelItem(index); - // search existing new thread - int found = -1; - for (newThread = threadList.begin (); newThread != threadList.end (); ++newThread) { - if (threadItem->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID) == (*newThread)->data (COLUMN_THREAD_DATA, ROLE_THREAD_MSGID)) { - // found it - found = index; - break; - } - } - if (found >= 0) { - ++index; - } else { + if(newThreadMap.find(threadItem->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString()) == newThreadMap.end()) delete(ui->threadTreeWidget->takeTopLevelItem(index)); - } + else + ++index; } // iterate all new threads - for (newThread = threadList.begin (); newThread != threadList.end (); ++newThread) { + for (QList::iterator newThread = threadList.begin (); newThread != threadList.end (); ++newThread) { // search existing thread - int found = -1; - int count = ui->threadTreeWidget->topLevelItemCount(); - for (index = 0; index < count; ++index) { - threadItem = ui->threadTreeWidget->topLevelItem(index); - if (threadItem->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID) == (*newThread)->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID)) { - // found it - found = index; - break; - } - } + QList items_found = ui->threadTreeWidget->findItems((*newThread)->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString(),Qt::MatchExactly,COLUMN_THREAD_DATA); + QTreeWidgetItem *threadItem = NULL ; + + if(!items_found.empty()) + { + if(items_found.size() != 1) + std::cerr << "(EE) weird. Found multiple tree items for msg ID " << (*newThread)->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString().toStdString() << std::endl; + + threadItem = *items_found.begin(); - if (found >= 0) { // set child data copyItem(threadItem, *newThread); // fill recursive fillChildren(threadItem, *newThread, expandNewMessages, itemToExpand); - } else { + } + else + { // add new thread ui->threadTreeWidget->addTopLevelItem (*newThread); threadItem = *newThread; From 318a3720c693767626f6ff733ff734c3e0cd2fb1 Mon Sep 17 00:00:00 2001 From: csoler Date: Sat, 14 Jan 2017 19:04:14 +0100 Subject: [PATCH 20/21] fixed reloading of distribution icons when opinion is changed over an ID in forums. Improved efficiency of forum loading in general. --- .../gui/gxsforums/GxsForumThreadWidget.cpp | 100 +++++++++--------- 1 file changed, 49 insertions(+), 51 deletions(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index 18240d984..b1ca69454 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -85,8 +85,9 @@ #define COLUMN_THREAD_SIGNED 5 #define COLUMN_THREAD_CONTENT 6 #define COLUMN_THREAD_COUNT 7 +#define COLUMN_THREAD_MSGID 8 -#define COLUMN_THREAD_DATA 0 // column for storing the userdata like msgid and parentid +#define COLUMN_THREAD_DATA 0 // column for storing the userdata like parentid #define ROLE_THREAD_MSGID Qt::UserRole #define ROLE_THREAD_STATUS Qt::UserRole + 1 @@ -126,8 +127,6 @@ public: // get pixmap unsigned int warning_level = qvariant_cast(index.data(Qt::DecorationRole)); - std::cerr << "Drawing item with warning level " << warning_level << std::endl; - switch(warning_level) { case 0: icon = QIcon(IMAGE_VOID); break; @@ -683,7 +682,7 @@ void GxsForumThreadWidget::changedThread() if (!item || !item->isSelected()) { mThreadId.clear(); } else { - mThreadId = RsGxsMessageId(item->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString().toStdString()); + mThreadId = RsGxsMessageId(item->data(COLUMN_THREAD_MSGID, Qt::DisplayRole).toString().toStdString()); } if (mFillThread) { @@ -1045,7 +1044,7 @@ void GxsForumThreadWidget::fillThreadFinished() while ((item = *itemIterator) != NULL) { ++itemIterator; - if (item->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString().toStdString() == thread->mFocusMsgId) { + if (item->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString().toStdString() == thread->mFocusMsgId) { ui->threadTreeWidget->setCurrentItem(item); ui->threadTreeWidget->setFocus(); break; @@ -1223,7 +1222,7 @@ QTreeWidgetItem *GxsForumThreadWidget::convertMsgToThreadWidget(const RsGxsForum item->setText(COLUMN_THREAD_CONTENT, doc.toPlainText().replace(QString("\n"), QString(" "))); } - item->setData(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID, QString::fromStdString(msg.mMeta.mMsgId.toStdString())); + item->setData(COLUMN_THREAD_MSGID,Qt::DisplayRole, QString::fromStdString(msg.mMeta.mMsgId.toStdString())); //#TODO #if 0 if (IS_GROUP_SUBSCRIBED(subscribeFlags) && !(msginfo.mMsgFlags & RS_DISTRIB_MISSING_MSG)) { @@ -1245,7 +1244,7 @@ QTreeWidgetItem *GxsForumThreadWidget::generateMissingItem(const RsGxsMessageId GxsIdRSTreeWidgetItem *item = new GxsIdRSTreeWidgetItem(mThreadCompareRole,GxsIdDetails::ICON_TYPE_AVATAR); item->setText(COLUMN_THREAD_TITLE, tr("[ ... Missing Message ... ]")); - item->setData(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID, QString::fromStdString(msgId.toStdString())); + item->setData(COLUMN_THREAD_MSGID,Qt::DisplayRole, QString::fromStdString(msgId.toStdString())); item->setData(COLUMN_THREAD_DATA, ROLE_THREAD_MISSING, true); item->setId(RsGxsId(), COLUMN_THREAD_AUTHOR, false); // fixed up columnId() @@ -1353,6 +1352,10 @@ static void copyItem(QTreeWidgetItem *item, const QTreeWidgetItem *newItem) for (i = 0; i < ROLE_THREAD_COUNT; ++i) { item->setData(COLUMN_THREAD_DATA, Qt::UserRole + i, newItem->data(COLUMN_THREAD_DATA, Qt::UserRole + i)); } + + item->setData(COLUMN_THREAD_DISTRIBUTION,Qt::DecorationRole,newItem->data(COLUMN_THREAD_DISTRIBUTION,Qt::DecorationRole)); + item->setData(COLUMN_THREAD_DISTRIBUTION,Qt::ToolTipRole, newItem->data(COLUMN_THREAD_DISTRIBUTION,Qt::ToolTipRole )); + item->setData(COLUMN_THREAD_MSGID, Qt::DisplayRole, newItem->data(COLUMN_THREAD_MSGID, Qt::DisplayRole )); } void GxsForumThreadWidget::fillThreads(QList &threadList, bool expandNewMessages, QList &itemToExpand) @@ -1364,35 +1367,39 @@ void GxsForumThreadWidget::fillThreads(QList &threadList, boo int index = 0; QTreeWidgetItem *threadItem; - // store new items in a map, so as to all + // store new items in a map, so as to allow a fast search + std::map newThreadMap ; for(QList::iterator newThread = threadList.begin (); newThread != threadList.end (); ++newThread) - newThreadMap[(*newThread)->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString()] = *newThread ; + newThreadMap[(*newThread)->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString()] = *newThread ; // delete not existing while (index < ui->threadTreeWidget->topLevelItemCount()) { threadItem = ui->threadTreeWidget->topLevelItem(index); - if(newThreadMap.find(threadItem->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString()) == newThreadMap.end()) + if(newThreadMap.find(threadItem->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString()) == newThreadMap.end()) delete(ui->threadTreeWidget->takeTopLevelItem(index)); else ++index; } + // (csoler) QTreeWidget::findItems apparently does not always work so I need to make the search manually, which I do using a map for efficiency reasons. + std::map oldThreadMap ; + for(uint32_t i=0;ithreadTreeWidget->topLevelItemCount();++i) + oldThreadMap[ui->threadTreeWidget->topLevelItem(i)->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString()] = ui->threadTreeWidget->topLevelItem(i) ; + // iterate all new threads for (QList::iterator newThread = threadList.begin (); newThread != threadList.end (); ++newThread) { // search existing thread - QList items_found = ui->threadTreeWidget->findItems((*newThread)->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString(),Qt::MatchExactly,COLUMN_THREAD_DATA); - QTreeWidgetItem *threadItem = NULL ; + std::cerr << "Makign a search for string \"" << (*newThread)->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString().toStdString() << "\"" << std::endl; - if(!items_found.empty()) + std::map::const_iterator it = oldThreadMap.find((*newThread)->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString()) ; + + if(it != oldThreadMap.end()) { - if(items_found.size() != 1) - std::cerr << "(EE) weird. Found multiple tree items for msg ID " << (*newThread)->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString().toStdString() << std::endl; - - threadItem = *items_found.begin(); + threadItem = it->second ; // set child data copyItem(threadItem, *newThread); @@ -1433,51 +1440,42 @@ void GxsForumThreadWidget::fillChildren(QTreeWidgetItem *parentItem, QTreeWidget QTreeWidgetItem *childItem; QTreeWidgetItem *newChildItem; + std::map newParentItemMap, parentItemMap ; + + for(index = 0; index < newParentItem->childCount(); ++index) newParentItemMap[newParentItem->child(index)->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString()] = newParentItem->child(index); + for(index = 0; index < parentItem->childCount(); ++index) parentItemMap[ parentItem->child(index)->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString()] = parentItem->child(index); + // delete not existing - while (index < parentItem->childCount()) { + while (index < parentItem->childCount()) + { childItem = parentItem->child(index); - // search existing new child - int found = -1; - int count = newParentItem->childCount(); - for (newIndex = 0; newIndex < count; ++newIndex) { - newChildItem = newParentItem->child(newIndex); - if (newChildItem->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID) == childItem->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID)) { - // found it - found = index; - break; - } - } - if (found >= 0) { - ++index; - } else { + if(newParentItemMap.find(childItem->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString()) == newParentItemMap.end()) delete(parentItem->takeChild (index)); - } - } + else + ++index; + } // iterate all new children - for (newIndex = 0; newIndex < newCount; ++newIndex) { + for (newIndex = 0; newIndex < newParentItem->childCount(); ++newIndex) + { newChildItem = newParentItem->child(newIndex); // search existing child - int found = -1; - int count = parentItem->childCount(); - for (index = 0; index < count; ++index) { - childItem = parentItem->child(index); - if (childItem->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID) == newChildItem->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID)) { - // found it - found = index; - break; - } - } - if (found >= 0) { + std::map::const_iterator it = parentItemMap.find(newChildItem->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString()) ; + + if(it != parentItemMap.end()) + { // set child data - copyItem(childItem, newChildItem); + copyItem(it->second, newChildItem); // fill recursive - fillChildren(childItem, newChildItem, expandNewMessages, itemToExpand); - } else { + fillChildren(it->second, newChildItem, expandNewMessages, itemToExpand); + childItem = it->second; + } + else + { // add new child childItem = newParentItem->takeChild(newIndex); parentItem->addChild(childItem); @@ -1751,7 +1749,7 @@ void GxsForumThreadWidget::setMsgReadStatus(QList &rows, bool if (status != statusNew) // is it different? { - std::string msgId = (*row)->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString().toStdString(); + std::string msgId = (*row)->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString().toStdString(); // NB: MUST BE PART OF ACTIVE THREAD--- OR ELSE WE MUST STORE GROUPID SOMEWHERE!. // LIKE THIS BELOW... @@ -1890,7 +1888,7 @@ bool GxsForumThreadWidget::navigate(const RsGxsMessageId &msgId) while ((item = *itemIterator) != NULL) { ++itemIterator; - if (item->data(COLUMN_THREAD_DATA, ROLE_THREAD_MSGID).toString() == msgIdString) { + if (item->data(COLUMN_THREAD_MSGID,Qt::DisplayRole).toString() == msgIdString) { ui->threadTreeWidget->setCurrentItem(item); ui->threadTreeWidget->setFocus(); return true; From dd408773c766b28501845deafcbd59808415784f Mon Sep 17 00:00:00 2001 From: csoler Date: Sat, 14 Jan 2017 19:08:14 +0100 Subject: [PATCH 21/21] removed debug info --- retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp index b1ca69454..2ba84ac18 100644 --- a/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp +++ b/retroshare-gui/src/gui/gxsforums/GxsForumThreadWidget.cpp @@ -1149,7 +1149,6 @@ QTreeWidgetItem *GxsForumThreadWidget::convertMsgToThreadWidget(const RsGxsForum rep_warning_level = 0 ; rep_tooltip_str = tr("Message will be forwarded to your friends.") ; } - std::cerr << "Inserting post from ID " << msg.mMeta.mAuthorId << ", group flags=" << std::hex << mForumGroup.mMeta.mSignFlags << " Identity flags = " << iddetails.mFlags << ": warning level = " << rep_warning_level << std::dec << std::endl; item->setData(COLUMN_THREAD_DISTRIBUTION,Qt::ToolTipRole,rep_tooltip_str) ; item->setData(COLUMN_THREAD_DISTRIBUTION,Qt::DecorationRole,rep_warning_level) ;