Closes #3291

As presented in the issue, we might end up in situation when parallel-processing and accepting two neutral+ trade offers will result in unwanted inventory state, because while they're both neutral+ and therefore OK to accept standalone, the combination of them both causes active badge progress degradation.

Considering the requirements we have, e.g. still processing trades in parallel, being performant, low on resources and with limited Steam servers overhead, the solution that I came up with in regards to this issue is quite simple:

- After we determine the trade to be neutral+, but before we tell the parse trade routine to accept it, we check if shared with other parallel processes set of handled sets contains any sets that we're currently processing.
- If no, we update that set to include everything we're dealing with, and tell the caller to accept this trade.
- If yes, we tell the caller to retry this trade after (other) accepted trades are confirmed and handled as usual.

This solves some issues and creates some optimistic assumptions:
- First of all, it solves the original issue, since if trade A and B both touch set S, then only one of them will be accepted. It's not deterministic which one (the one that gets to the check first), and not important anyway.
- We do not "lock" the sets before we determine that trade is neutral+, because otherwise unrelated users could spam us with non-neutral+ trades in order to lock the bot in infinite retry. This way they can't, as if the trade is determined to not be neutral+ then it never checks for concurrent processing.
- We are optimistic about resources usage. This routine could be made much more complicated to be more synchronous in order to avoid unnecessary calls to inventory and matching, however, that'd slow down the whole process only because the next call MAYBE will be determined as unneeded. Due to that, ASF is optimistic that trades will (usually) be unrelated, and can be processed in parallel, and if the conflict happens then simply we end up in a situation where we did some extra work for no reason, which is better than waiting with the work till all previous trades are processed.
- As soon as the conditions are met, the conflicting trades are retried to check if the conditions allow to accept them. If yes, they'll be accepted almost immediately after previous ones, if not, they'll be rejected as non-neutral+ anymore.

This way the additional code does not hurt the performance, parallel processing or anything else in usually expected optimistic scenarios, while adding some additional overhead in pessimistic ones, which is justified considering we don't want to degrade the badge progress.

ArchiSteamFarm/Steam/Exchange/ParseTradeResult.cs

@@ -65,6 +65,7 @@ public sealed class ParseTradeResult {
 		Blacklisted,
 		Ignored,
 		Rejected,
-		TryAgain
+		TryAgain,
+		RetryAfterOthers
 	}
 }

ArchiSteamFarm/Steam/Exchange/Trading.cs

@@ -34,6 +34,7 @@ using ArchiSteamFarm.Steam.Cards;
 using ArchiSteamFarm.Steam.Data;
 using ArchiSteamFarm.Steam.Storage;
 using ArchiSteamFarm.Storage;
+using ArchiSteamFarm.Web;
 using JetBrains.Annotations;
 using SteamKit2;
 
@@ -261,48 +262,68 @@ public sealed class Trading : IDisposable {
 	}
 
 	private async Task<bool> ParseActiveTrades() {
-		HashSet<TradeOffer>? tradeOffers = await Bot.ArchiWebHandler.GetTradeOffers(true, true, false, true).ConfigureAwait(false);
+		bool lootableTypesReceived = false;
 
-		if ((tradeOffers == null) || (tradeOffers.Count == 0)) {
-			return false;
-		}
+		for (byte i = 0; i < WebBrowser.MaxTries; i++) {
+			HashSet<TradeOffer>? tradeOffers = await Bot.ArchiWebHandler.GetTradeOffers(true, true, false, true).ConfigureAwait(false);
 
-		if (HandledTradeOfferIDs.Count > 0) {
-			HandledTradeOfferIDs.IntersectWith(tradeOffers.Select(static tradeOffer => tradeOffer.TradeOfferID));
-		}
+			if ((tradeOffers == null) || (tradeOffers.Count == 0)) {
+				return false;
+			}
 
-		IEnumerable<Task<ParseTradeResult>> tasks = tradeOffers.Where(tradeOffer => (tradeOffer.State == ETradeOfferState.Active) && HandledTradeOfferIDs.Add(tradeOffer.TradeOfferID)).Select(ParseTrade);
-		IList<ParseTradeResult> results = await Utilities.InParallel(tasks).ConfigureAwait(false);
+			if (HandledTradeOfferIDs.Count > 0) {
+				HandledTradeOfferIDs.IntersectWith(tradeOffers.Select(static tradeOffer => tradeOffer.TradeOfferID));
+			}
 
-		if (Bot.HasMobileAuthenticator) {
-			HashSet<ParseTradeResult> mobileTradeResults = results.Where(static result => result is { Result: ParseTradeResult.EResult.Accepted, Confirmed: false }).ToHashSet();
+			HashSet<(uint RealAppID, EAssetType Type, EAssetRarity Rarity)> handledSets = [];
 
-			if (mobileTradeResults.Count > 0) {
-				HashSet<ulong> mobileTradeOfferIDs = mobileTradeResults.Select(static tradeOffer => tradeOffer.TradeOfferID).ToHashSet();
+			IEnumerable<Task<ParseTradeResult>> tasks = tradeOffers.Where(tradeOffer => (tradeOffer.State == ETradeOfferState.Active) && HandledTradeOfferIDs.Add(tradeOffer.TradeOfferID)).Select(tradeOffer => ParseTrade(tradeOffer, handledSets));
 
-				(bool twoFactorSuccess, _, _) = await Bot.Actions.HandleTwoFactorAuthenticationConfirmations(true, Confirmation.EConfirmationType.Trade, mobileTradeOfferIDs, true).ConfigureAwait(false);
+			IList<ParseTradeResult> tradeResults = await Utilities.InParallel(tasks).ConfigureAwait(false);
 
-				if (twoFactorSuccess) {
-					foreach (ParseTradeResult mobileTradeResult in mobileTradeResults) {
-						mobileTradeResult.Confirmed = true;
+			if (Bot.HasMobileAuthenticator) {
+				HashSet<ParseTradeResult> mobileTradeResults = tradeResults.Where(static result => result is { Result: ParseTradeResult.EResult.Accepted, Confirmed: false }).ToHashSet();
+
+				if (mobileTradeResults.Count > 0) {
+					HashSet<ulong> mobileTradeOfferIDs = mobileTradeResults.Select(static tradeOffer => tradeOffer.TradeOfferID).ToHashSet();
+
+					(bool twoFactorSuccess, _, _) = await Bot.Actions.HandleTwoFactorAuthenticationConfirmations(true, Confirmation.EConfirmationType.Trade, mobileTradeOfferIDs, true).ConfigureAwait(false);
+
+					if (twoFactorSuccess) {
+						foreach (ParseTradeResult mobileTradeResult in mobileTradeResults) {
+							mobileTradeResult.Confirmed = true;
+						}
+					} else {
+						HandledTradeOfferIDs.ExceptWith(mobileTradeOfferIDs);
 					}
-				} else {
-					HandledTradeOfferIDs.ExceptWith(mobileTradeOfferIDs);
 				}
 			}
+
+			if (tradeResults.Count > 0) {
+				await PluginsCore.OnBotTradeOfferResults(Bot, tradeResults as IReadOnlyCollection<ParseTradeResult> ?? tradeResults.ToHashSet()).ConfigureAwait(false);
+			}
+
+			if (!lootableTypesReceived && tradeResults.Any(tradeResult => tradeResult is { Result: ParseTradeResult.EResult.Accepted, Confirmed: true } && (tradeResult.ItemsToReceive?.Any(receivedItem => Bot.BotConfig.LootableTypes.Contains(receivedItem.Type)) == true))) {
+				lootableTypesReceived = true;
+			}
+
+			// If any trade asked to be retried, we do have ASF 2FA and actually managed to confirm something (else), then now is the time for retry
+			if (Bot.HasMobileAuthenticator && tradeResults.Any(static tradeResult => tradeResult.Result == ParseTradeResult.EResult.RetryAfterOthers) && tradeResults.Any(static tradeResult => tradeResult is { Result: ParseTradeResult.EResult.Accepted, Confirmed: true })) {
+				continue;
+			}
+
+			return lootableTypesReceived;
 		}
 
-		if (results.Count > 0) {
-			await PluginsCore.OnBotTradeOfferResults(Bot, results as IReadOnlyCollection<ParseTradeResult> ?? results.ToHashSet()).ConfigureAwait(false);
-		}
-
-		return results.Any(result => result is { Result: ParseTradeResult.EResult.Accepted, Confirmed: true } && (result.ItemsToReceive?.Any(receivedItem => Bot.BotConfig.LootableTypes.Contains(receivedItem.Type)) == true));
+		// The remaining trade offers we'll handle at later time
+		return lootableTypesReceived;
 	}
 
-	private async Task<ParseTradeResult> ParseTrade(TradeOffer tradeOffer) {
+	private async Task<ParseTradeResult> ParseTrade(TradeOffer tradeOffer, ISet<(uint RealAppID, EAssetType Type, EAssetRarity Rarity)> handledSets) {
 		ArgumentNullException.ThrowIfNull(tradeOffer);
+		ArgumentNullException.ThrowIfNull(handledSets);
 
-		ParseTradeResult.EResult result = await ShouldAcceptTrade(tradeOffer).ConfigureAwait(false);
+		ParseTradeResult.EResult result = await ShouldAcceptTrade(tradeOffer, handledSets).ConfigureAwait(false);
 		bool tradeRequiresMobileConfirmation = false;
 
 		switch (result) {
@@ -360,6 +381,7 @@ public sealed class Trading : IDisposable {
 				Bot.ArchiLogger.LogGenericInfo(Strings.FormatIgnoringTrade(tradeOffer.TradeOfferID));
 
 				break;
+			case ParseTradeResult.EResult.RetryAfterOthers:
 			case ParseTradeResult.EResult.TryAgain:
 				// We expect to see this trade offer again and we intend to retry it
 				HandledTradeOfferIDs.Remove(tradeOffer.TradeOfferID);
@@ -374,8 +396,9 @@ public sealed class Trading : IDisposable {
 		return new ParseTradeResult(tradeOffer.TradeOfferID, result, tradeRequiresMobileConfirmation, tradeOffer.ItemsToGive, tradeOffer.ItemsToReceive);
 	}
 
-	private async Task<ParseTradeResult.EResult> ShouldAcceptTrade(TradeOffer tradeOffer) {
+	private async Task<ParseTradeResult.EResult> ShouldAcceptTrade(TradeOffer tradeOffer, ISet<(uint RealAppID, EAssetType Type, EAssetRarity Rarity)> handledSets) {
 		ArgumentNullException.ThrowIfNull(tradeOffer);
+		ArgumentNullException.ThrowIfNull(handledSets);
 
 		if (Bot.Bots == null) {
 			throw new InvalidOperationException(nameof(Bot.Bots));
@@ -527,6 +550,19 @@ public sealed class Trading : IDisposable {
 
 		bool accept = IsTradeNeutralOrBetter(inventory, tradeOffer.ItemsToGive, tradeOffer.ItemsToReceive);
 
+		if (accept) {
+			// Ensure that accepting this trade offer does not create conflicts with other
+			lock (handledSets) {
+				if (wantedSets.Any(handledSets.Contains)) {
+					Bot.ArchiLogger.LogGenericDebug(Strings.FormatBotTradeOfferResult(tradeOffer.TradeOfferID, ParseTradeResult.EResult.RetryAfterOthers, nameof(handledSets)));
+
+					return ParseTradeResult.EResult.RetryAfterOthers;
+				}
+
+				handledSets.UnionWith(wantedSets);
+			}
+		}
+
 		// We're now sure whether the trade is neutral+ for us or not
 		ParseTradeResult.EResult acceptResult = accept ? ParseTradeResult.EResult.Accepted : ParseTradeResult.EResult.Rejected;