From 861ce247e8561bb922b964cb31e1de450bd28f9f Mon Sep 17 00:00:00 2001 From: jolavillette Date: Thu, 11 Jun 2026 19:53:06 +0200 Subject: [PATCH] fix(gxstrans): advertise messages injected via receiveNewMessages() to friends When a GxsTrans message (e.g. a presigned ACK receipt) is re-injected on the recipient side via RsGenExchange::receiveNewMessages(), it is stored but the server-side message-update timestamp is never stamped - unlike the regular netservice transaction path (processCompletedTransactions). As a result friends are not notified and the message is not re-synced, so distant-mail ACKs can be lost and the sender never sees the message as delivered. - rsgenexchange: after storing, stamp the server msg-update TS for every group that received genuinely-new (post-deduplication) messages, off-mutex. - p3msgservice: notifyDataStatus now scans all outgoing boxes before giving up (found-flag instead of returning on the first non-matching box); notifyGxsTransSendStatus flags the correct message id (it->first) and breaks. Co-Authored-By: Claude Opus 4.8 --- src/gxs/rsgenexchange.cc | 17 +++++++++++++++++ src/services/p3msgservice.cc | 35 ++++++++++++++++++++--------------- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/gxs/rsgenexchange.cc b/src/gxs/rsgenexchange.cc index 21b4eff0c..f55cfa5aa 100644 --- a/src/gxs/rsgenexchange.cc +++ b/src/gxs/rsgenexchange.cc @@ -3094,6 +3094,7 @@ void RsGenExchange::computeHash(const RsTlvBinaryData& data, RsFileHash& hash) void RsGenExchange::processRecvdMessages() { std::list messages_to_reject ; + std::set grps_with_new_msgs ; // groups that received new messages, to stamp their server update TS off-mutex below { RS_STACK_MUTEX(mGenMtx) ; @@ -3273,6 +3274,12 @@ void RsGenExchange::processRecvdMessages() for(auto& nxs_msg: msgs_to_store) { + // msgs_to_store is now post-deduplication (removeDeleteExistingMessages above), so this only + // contains genuinely new messages. Mark their groups for the server-TS stamp here, NOT from + // groups_last_post_update (which is populated pre-dedup and would also fire for already-known + // messages re-received via sync, causing redundant stamps/advertisements). + grps_with_new_msgs.insert(nxs_msg->grpId); + RsGxsMsgItem *item = dynamic_cast(mSerialiser->deserialise(nxs_msg->msg.bin_data,&nxs_msg->msg.bin_len)); if(!item) @@ -3305,8 +3312,18 @@ void RsGenExchange::processRecvdMessages() // Done off-mutex to avoid cross deadlocks in the netservice that might call the RsGenExchange as an observer.. if(mNetService != NULL) + { for(std::list::const_iterator it(messages_to_reject.begin());it!=messages_to_reject.end();++it) mNetService->rejectMessage(*it) ; + + // Stamp the server-side msg update TS for groups that received new messages, so that friends get + // notified and re-synchronise. For messages received through the regular netservice transaction path + // this is already done in RsGxsNetService::processCompletedTransactions(), but messages injected + // directly via receiveNewMessages() (e.g. presigned receipts) bypass that path and would otherwise be + // stored but never advertised to friends. + for(const RsGxsGroupId& grpId : grps_with_new_msgs) + mNetService->stampMsgServerUpdateTS(grpId) ; + } } bool RsGenExchange::acceptNewGroup(const RsGxsGrpMetaData* /*grpMeta*/ ) { return true; } diff --git a/src/services/p3msgservice.cc b/src/services/p3msgservice.cc index 472fbe37f..8a488057a 100644 --- a/src/services/p3msgservice.cc +++ b/src/services/p3msgservice.cc @@ -440,6 +440,7 @@ int p3MsgService::checkOutgoingMessages() { if(to.toRsPeerId() == ownId || mServiceCtrl->isPeerConnected(getServiceInfo().mServiceType, to.toRsPeerId()) ) { + auto msg_item = createOutgoingMessageItem(*sit->second,to); // Use the msg_id of the outgoing message copy. @@ -476,7 +477,6 @@ int p3MsgService::checkOutgoingMessages() changed = true; #ifdef DEBUG_DISTANT_MSG - RsDbg() << "Message id " << mit->first << " is distant: kept in outgoing, and marked as ROUTED" << std::endl; #endif Dbg3() << __PRETTY_FUNCTION__ << " Sending out message" << std::endl; auto msg_item = createOutgoingMessageItem(*sit->second,to); @@ -510,6 +510,7 @@ int p3MsgService::checkOutgoingMessages() if(mit->second.empty()) { sit->second->msg.msgFlags &= ~RS_MSG_FLAGS_PENDING; + pEvent->mChangedMsgIds.insert(std::to_string(sit->first)); auto tmp = mit; ++tmp; msgOutgoing.erase(mit); @@ -526,7 +527,6 @@ int p3MsgService::checkOutgoingMessages() if(changed) { - // RsDbg() << "MAIL: checkOutgoingMessages() triggering IndicateConfigChanged(SAVE_NOW)" << std::endl; IndicateConfigChanged(RsConfigMgr::CheckPriority::SAVE_NOW); } @@ -2474,6 +2474,8 @@ void p3MsgService::notifyDataStatus( const GRouterMsgPropagationId& id, << " could not be delivered on time to " << signer_id << ". Message id: " << msg_id << std::endl; + bool found = false; + for(auto it=msgOutgoing.begin();it!=msgOutgoing.end();++it) { auto mit = it->second.find(msg_id); @@ -2482,15 +2484,17 @@ void p3MsgService::notifyDataStatus( const GRouterMsgPropagationId& id, { std::cerr << " reseting the ROUTED flag so that the message is requested again" << std::endl; mit->second.flags &= ~RS_MSG_FLAGS_ROUTED; + found = true; break; } - else - { - std::cerr << "(ii) message has been notified as delivered, but it's" - << " not in outgoing list. probably it has been delivered" - << " successfully by other means." << std::endl; - return; - } + } + + if(!found) + { + std::cerr << "(ii) message has been notified as delivered, but it's" + << " not in outgoing list. probably it has been delivered" + << " successfully by other means." << std::endl; + return; } } else if(data_status == GROUTER_CLIENT_SERVICE_DATA_STATUS_RECEIVED) @@ -2579,6 +2583,7 @@ bool p3MsgService::receiveGxsTransMail( const RsGxsId& authorId, const RsGxsId& recipientId, const uint8_t* data, uint32_t dataSize ) { + Dbg2() << __PRETTY_FUNCTION__ << " " << authorId << ", " << recipientId << ",, " << dataSize << std::endl; @@ -2660,7 +2665,6 @@ bool p3MsgService::notifyGxsTransSendStatus( RsGxsTransId mailId, } std::cerr << " message id = " << msg_id << std::endl; - if( status == GxsTransSendStatus::RECEIPT_RECEIVED ) { pEvent->mMailStatusEventCode = RsMailStatusEventCode::MESSAGE_RECEIVED_ACK; @@ -2679,11 +2683,10 @@ bool p3MsgService::notifyGxsTransSendStatus( RsGxsTransId mailId, { it->second.erase(mit); - pEvent->mChangedMsgIds.insert(std::to_string(msg_id)); + pEvent->mChangedMsgIds.insert(std::to_string(it->first)); found = true; + break; } - - break; } if(!found) @@ -2710,10 +2713,10 @@ bool p3MsgService::notifyGxsTransSendStatus( RsGxsTransId mailId, { mit->second.flags &= ~RS_MSG_FLAGS_ROUTED; // forces re-send. - pEvent->mChangedMsgIds.insert(std::to_string(msg_id)); + pEvent->mChangedMsgIds.insert(std::to_string(it->first)); found = true; + break; } - break; } if(!found) @@ -2737,6 +2740,7 @@ void p3MsgService::receiveGRouterData( const RsGxsId &destination_key, GRouterServiceId &/*client_id*/, uint8_t *data, uint32_t data_size ) { + std::cerr << "p3MsgService::receiveGRouterData(): received message item of" << " size " << data_size << ", for key " << destination_key << std::endl; @@ -2822,6 +2826,7 @@ void p3MsgService::locked_sendDistantMsgItem(RsMsgItem *msgitem,const RsGxsId& s mGRouter->sendData( destination_key_id, GROUTER_CLIENT_ID_MESSAGES, msg_serialized_data, msg_serialized_rssize, signing_key_id, grouter_message_id ); + RsGxsTransId gxsMailId; mGxsTransServ.sendData( gxsMailId, GxsTransSubServices::P3_MSG_SERVICE, signing_key_id, destination_key_id,