fix issues in gxstrans#308
Merged
Merged
Conversation
jolavillette
commented
Jun 8, 2026
jolavillette
commented
Contributor
- ACK stamp - processRecvdMessages now advertises messages injected via receiveNewMessages (the GxsTrans receipt) to friends; without it the ACK never propagates back.
- notifyGxsTransSendStatus - break inside the if (x2): the linear search over msgOutgoing no longer bails out after the 1st bucket.
- notifyGxsTransSendStatus - it->first instead of msg_id (x2): the GUI event references the original message, not its per-recipient copy.
- notifyDataStatus - found flag instead of the premature return: same linear-search bug.
- checkOutgoingMessages - emits a change notification when the PENDING flag is cleared (GUI refresh).
csoler
requested changes
Jun 11, 2026
csoler
left a comment
csoler
left a comment
Contributor
There was a problem hiding this comment.
- for debugging purpose you added a log of RsDbg(). That's fine, but you should use #ifdefs around (GROUTER_DEBUG, GXSTRANS_DEBUG, etc), so we can easily activate them service by service. For most debug blocks (some are large!) that actually helped you find the problem, just remove them.
| << "with: msgId: " << receipt->msgId << std::endl; | ||
| #endif | ||
|
|
||
| RsGxsTransId mailId = 0; |
Contributor
There was a problem hiding this comment.
This entire block is doing nothing except printing dbg info. Please use a ifdef around.
|
|
||
| void p3GRouter::locked_collectAvailableFriends(const GRouterKeyId& gxs_id,const std::set<RsPeerId>& incoming_routes,uint32_t duplication_factor, std::map<RsPeerId,uint32_t>& friend_peers_and_duplication_factors) | ||
| { | ||
| return; // FORCE NO DIRECT FRIENDS AVAILABLE TO TEST TUNNEL FALLBACK ROUTING! |
Contributor
There was a problem hiding this comment.
I think you do not want to keep that debugging hack.
| { | ||
| if(mTurtle->isTurtlePeer(pid)) | ||
| { | ||
| if (trans_item.PacketSubType() == RS_PKT_SUBTYPE_GROUTER_TRANSACTION_CHUNK) |
Contributor
There was a problem hiding this comment.
same, this blobk is for debug only and is fairly large. If temporary, jusr remove it.
| } | ||
| else | ||
| { | ||
| if (trans_item.PacketSubType() == RS_PKT_SUBTYPE_GROUTER_TRANSACTION_CHUNK) |
|
|
||
| if(delete_entry) | ||
| { | ||
| if( it->second.data_status != RS_GROUTER_DATA_STATUS_DONE && it->second.data_status != RS_GROUTER_DATA_STATUS_RECEIPT_OK ) |
Contributor
There was a problem hiding this comment.
same. I won't comment on the remaining ones.
| for(std::list<RsGxsMessageId>::const_iterator it(messages_to_reject.begin());it!=messages_to_reject.end();++it) | ||
| mNetService->rejectMessage(*it) ; | ||
|
|
||
| // MAIL: stamp the server-side msg update TS for groups that received new messages, so that friends |
Contributor
There was a problem hiding this comment.
We're in rsgenexchange here so there's nothing specific to each service. Update the comment.
| { | ||
| RsDbg() << "MAIL (" << AuthSSL::getAuthSSL()->getOwnLocation() << "): GXS - Stamping server msg update TS for group " << grpId | ||
| << " after injecting new message(s) via receiveNewMessages() (e.g. GxsTrans ACK), so friends get notified and re-sync"; | ||
| mNetService->stampMsgServerUpdateTS(grpId) ; |
Contributor
There was a problem hiding this comment.
ok this is probably where the problem happens. Good catch.
…o 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 <noreply@anthropic.com>
jolavillette
force-pushed
the
fix/gxstrans
branch
from
4f198c2 to
861ce24
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.