From 7773e9a2809018766133a1d8f80b5a103b878787 Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Mon, 31 Aug 2020 16:15:57 +1000 Subject: [PATCH 1/3] fix bugs for open group time handling --- .../ConversationView/Cells/OWSMessageFooterView.m | 2 +- .../ConversationView/ConversationViewController.m | 8 ++++---- .../ConversationView/ConversationViewModel.m | 4 ++-- .../src/Messages/Interactions/TSInteraction.h | 3 +++ .../src/Messages/Interactions/TSInteraction.m | 10 ++++------ 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageFooterView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageFooterView.m index d63a8baa8..09782265d 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageFooterView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageFooterView.m @@ -195,7 +195,7 @@ NS_ASSUME_NONNULL_BEGIN timestampLabelText = NSLocalizedString(@"MESSAGE_STATUS_SEND_FAILED", @"Label indicating that a message failed to send."); } else { - timestampLabelText = [DateUtil formatMessageTimestamp:viewItem.interaction.timestamp]; + timestampLabelText = [DateUtil formatMessageTimestamp:viewItem.interaction.timestampForUI]; } TSMessage *message = [viewItem.interaction as:TSMessage.class]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 96f84c3ff..1d8a91499 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -4683,7 +4683,7 @@ typedef enum : NSUInteger { OWSAssertDebug(left <= mid); OWSAssertDebug(mid < right); id viewItem = self.viewItems[mid]; - if (viewItem.interaction.timestamp >= viewHorizonTimestamp) { + if (viewItem.interaction.timestampForUI >= viewHorizonTimestamp) { right = mid; } else { // This is an optimization; it also ensures that we converge. @@ -4692,7 +4692,7 @@ typedef enum : NSUInteger { } OWSAssertDebug(left == right); id viewItem = self.viewItems[left]; - if (viewItem.interaction.timestamp >= viewHorizonTimestamp) { + if (viewItem.interaction.timestampForUI >= viewHorizonTimestamp) { OWSLogInfo(@"firstIndexPathAtViewHorizonTimestamp: %zd / %zd", left, self.viewItems.count); return [NSIndexPath indexPathForRow:(NSInteger) left inSection:0]; } else { @@ -5402,7 +5402,7 @@ typedef enum : NSUInteger { __block TSInteraction *targetInteraction; [LKStorage writeSyncWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self.thread enumerateInteractionsWithTransaction:transaction usingBlock:^(TSInteraction *interaction, YapDatabaseReadTransaction *t) { - if (interaction.timestamp == timestamp.unsignedLongLongValue) { + if (interaction.timestampForUI == timestamp.unsignedLongLongValue) { targetInteraction = interaction; } }]; @@ -5426,7 +5426,7 @@ typedef enum : NSUInteger { { __block TSInteraction *targetInteraction; [self.thread enumerateInteractionsUsingBlock:^(TSInteraction *interaction) { - if (interaction.timestamp == timestamp.unsignedLongLongValue) { + if (interaction.timestampForUI == timestamp.unsignedLongLongValue) { targetInteraction = interaction; } }]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m index 693392413..1b438a869 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewModel.m @@ -1350,7 +1350,7 @@ static const int kYapDatabaseRangeMaxLength = 25000; break; } - uint64_t viewItemTimestamp = viewItem.interaction.timestamp; + uint64_t viewItemTimestamp = viewItem.interaction.timestampForUI; OWSAssertDebug(viewItemTimestamp > 0); BOOL shouldShowDate = NO; @@ -1417,7 +1417,7 @@ static const int kYapDatabaseRangeMaxLength = 25000; NSAttributedString *_Nullable senderName = nil; OWSInteractionType interactionType = viewItem.interaction.interactionType; - NSString *timestampText = [DateUtil formatTimestampShort:viewItem.interaction.timestamp]; + NSString *timestampText = [DateUtil formatTimestampShort:viewItem.interaction.timestampForUI]; if (interactionType == OWSInteractionType_OutgoingMessage) { TSOutgoingMessage *outgoingMessage = (TSOutgoingMessage *)viewItem.interaction; diff --git a/SignalServiceKit/src/Messages/Interactions/TSInteraction.h b/SignalServiceKit/src/Messages/Interactions/TSInteraction.h index b53c71b17..2d230db56 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInteraction.h +++ b/SignalServiceKit/src/Messages/Interactions/TSInteraction.h @@ -39,11 +39,14 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value); @property (nonatomic, readonly) uint64_t timestamp; @property (nonatomic, readonly) uint64_t sortId; @property (nonatomic, readonly) uint64_t receivedAtTimestamp; +@property (nonatomic, readonly) BOOL shouldUseServerTime; /// Used for public chats where a message sent from a slave device is interpreted as having been sent from the master device. @property (nonatomic) NSString *actualSenderHexEncodedPublicKey; - (void)setServerTimestampToReceivedTimestamp:(uint64_t)receivedAtTimestamp; +- (uint64_t)timestampForUI; + - (NSDate *)receivedAtDate; - (OWSInteractionType)interactionType; diff --git a/SignalServiceKit/src/Messages/Interactions/TSInteraction.m b/SignalServiceKit/src/Messages/Interactions/TSInteraction.m index 502c55d35..2d818d150 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInteraction.m +++ b/SignalServiceKit/src/Messages/Interactions/TSInteraction.m @@ -177,13 +177,10 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value) #pragma mark Date operations -- (uint64_t)timestamp +- (uint64_t)timestampForUI { - if (self.thread.isGroupThread) { - TSGroupThread *thread = (TSGroupThread *)self.thread; - if (thread.isPublicChat) { - return _receivedAtTimestamp; - } + if (_shouldUseServerTime) { + return _receivedAtTimestamp; } return _timestamp; } @@ -195,6 +192,7 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value) - (void)setServerTimestampToReceivedTimestamp:(uint64_t)receivedAtTimestamp { + _shouldUseServerTime = YES; _receivedAtTimestamp = receivedAtTimestamp; } From 78e6651e8785125f43cbfd7a753fa52fafe32e0e Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 1 Sep 2020 13:31:11 +1000 Subject: [PATCH 2/3] Fix missed error handling case --- .../Loki/API/Onion Requests/OnionRequestAPI.swift | 8 +++++++- SignalServiceKit/src/Loki/API/SnodeAPI.swift | 14 ++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift b/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift index 45a7c0e41..f0b90bac5 100644 --- a/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift +++ b/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift @@ -339,7 +339,13 @@ public enum OnionRequestAPI { } } promise.catch2 { error in // Must be invoked on LokiAPI.workQueue - guard case HTTP.Error.httpRequestFailed(_, _) = error else { return } + guard case HTTP.Error.httpRequestFailed(let statusCode, let json) = error else { return } + // Marking all the snodes in the path as unreliable here is aggressive, but otherwise users + // can get stuck with a failing path that just refreshes to the same path. + let path = paths.first { $0.contains(guardSnode) } + path?.forEach { snode in + SnodeAPI.handleError(withStatusCode: statusCode, json: json, forSnode: snode) // Intentionally don't throw + } dropAllPaths() // A snode in the path is bad; retry with a different path dropGuardSnode(guardSnode) } diff --git a/SignalServiceKit/src/Loki/API/SnodeAPI.swift b/SignalServiceKit/src/Loki/API/SnodeAPI.swift index 9e24ba37f..5c2255574 100644 --- a/SignalServiceKit/src/Loki/API/SnodeAPI.swift +++ b/SignalServiceKit/src/Loki/API/SnodeAPI.swift @@ -296,7 +296,7 @@ public final class SnodeAPI : NSObject { // MARK: Error Handling /// - Note: Should only be invoked from `LokiAPI.workQueue` to avoid race conditions. - internal static func handleError(withStatusCode statusCode: UInt, json: JSON?, forSnode snode: Snode, associatedWith publicKey: String) -> Error? { + internal static func handleError(withStatusCode statusCode: UInt, json: JSON?, forSnode snode: Snode, associatedWith publicKey: String? = nil) -> Error? { #if DEBUG assertOnQueue(SnodeAPI.workQueue) #endif @@ -307,7 +307,9 @@ public final class SnodeAPI : NSObject { print("[Loki] Couldn't reach snode at: \(snode); setting failure count to \(newFailureCount).") if newFailureCount >= SnodeAPI.snodeFailureThreshold { print("[Loki] Failure threshold reached for: \(snode); dropping it.") - SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) + if let publicKey = publicKey { + SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) + } SnodeAPI.dropSnodeFromSnodePool(snode) SnodeAPI.snodeFailureCount[snode] = 0 } @@ -321,8 +323,12 @@ public final class SnodeAPI : NSObject { return SnodeAPI.SnodeAPIError.clockOutOfSync case 421: // The snode isn't associated with the given public key anymore - print("[Loki] Invalidating swarm for: \(publicKey).") - SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) + if let publicKey = publicKey { + print("[Loki] Invalidating swarm for: \(publicKey).") + SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) + } else { + print("[Loki] Got a 421 without an associated public key.") + } case 432: // The proof of work difficulty is too low if let powDifficulty = json?["difficulty"] as? UInt { From 2b3e97f6cc8928d1583aea9c56bdf56a5faaf948 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Wed, 2 Sep 2020 09:17:01 +1000 Subject: [PATCH 3/3] Tweak error handling --- .../src/Loki/API/Onion Requests/OnionRequestAPI.swift | 9 ++------- SignalServiceKit/src/Loki/API/SnodeAPI.swift | 5 +++-- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift b/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift index f0b90bac5..e9f2f9035 100644 --- a/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift +++ b/SignalServiceKit/src/Loki/API/Onion Requests/OnionRequestAPI.swift @@ -6,11 +6,6 @@ public enum OnionRequestAPI { public static var guardSnodes: Set = [] public static var paths: [Path] = [] // Not a set to ensure we consistently show the same path to the user - private static var snodePool: Set { - let unreliableSnodes = Set(SnodeAPI.snodeFailureCount.keys) - return SnodeAPI.snodePool.subtracting(unreliableSnodes) - } - // MARK: Settings /// The number of snodes (including the guard snode) in a path. private static let pathSize: UInt = 3 @@ -84,7 +79,7 @@ public enum OnionRequestAPI { } else { print("[Loki] [Onion Request API] Populating guard snode cache.") return SnodeAPI.getRandomSnode().then2 { _ -> Promise> in // Just used to populate the snode pool - var unusedSnodes = snodePool // Sync on LokiAPI.workQueue + var unusedSnodes = SnodeAPI.snodePool // Sync on LokiAPI.workQueue guard unusedSnodes.count >= guardSnodeCount else { throw Error.insufficientSnodes } func getGuardSnode() -> Promise { // randomElement() uses the system's default random generator, which is cryptographically secure @@ -115,7 +110,7 @@ public enum OnionRequestAPI { } return SnodeAPI.getRandomSnode().then2 { _ -> Promise<[Path]> in // Just used to populate the snode pool return getGuardSnodes().map2 { guardSnodes -> [Path] in - var unusedSnodes = snodePool.subtracting(guardSnodes) + var unusedSnodes = SnodeAPI.snodePool.subtracting(guardSnodes) let pathSnodeCount = guardSnodeCount * pathSize - guardSnodeCount guard unusedSnodes.count >= pathSnodeCount else { throw Error.insufficientSnodes } // Don't test path snodes as this would reveal the user's IP to them diff --git a/SignalServiceKit/src/Loki/API/SnodeAPI.swift b/SignalServiceKit/src/Loki/API/SnodeAPI.swift index 5c2255574..420a8aa22 100644 --- a/SignalServiceKit/src/Loki/API/SnodeAPI.swift +++ b/SignalServiceKit/src/Loki/API/SnodeAPI.swift @@ -15,10 +15,10 @@ public final class SnodeAPI : NSObject { // MARK: Settings private static let maxRetryCount: UInt = 4 - private static let minimumSnodePoolCount = 32 + private static let minimumSnodePoolCount = 64 private static let minimumSwarmSnodeCount = 2 private static let seedNodePool: Set = [ "https://storage.seed1.loki.network", "https://storage.seed3.loki.network", "https://public.loki.foundation" ] - private static let snodeFailureThreshold = 2 + private static let snodeFailureThreshold = 1 private static let targetSwarmSnodeCount = 2 internal static var powDifficulty: UInt = 1 @@ -311,6 +311,7 @@ public final class SnodeAPI : NSObject { SnodeAPI.dropSnodeFromSwarmIfNeeded(snode, publicKey: publicKey) } SnodeAPI.dropSnodeFromSnodePool(snode) + print("[Loki] Snode pool count: \(snodePool.count).") SnodeAPI.snodeFailureCount[snode] = 0 } }