Skip to content

Commit

Permalink
Closes #3291
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
JustArchi committed Sep 19, 2024
1 parent 85e90bb commit 1dff9a4
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 27 deletions.
3 changes: 2 additions & 1 deletion ArchiSteamFarm/Steam/Exchange/ParseTradeResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public enum EResult : byte {
Blacklisted,
Ignored,
Rejected,
TryAgain
TryAgain,
RetryAfterOthers
}
}
88 changes: 62 additions & 26 deletions ArchiSteamFarm/Steam/Exchange/Trading.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
using ArchiSteamFarm.Steam.Data;
using ArchiSteamFarm.Steam.Storage;
using ArchiSteamFarm.Storage;
using ArchiSteamFarm.Web;
using JetBrains.Annotations;
using SteamKit2;

Expand Down Expand Up @@ -261,48 +262,68 @@ internal async Task OnNewTrade() {
}

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;
}

if (HandledTradeOfferIDs.Count > 0) {
HandledTradeOfferIDs.IntersectWith(tradeOffers.Select(static tradeOffer => tradeOffer.TradeOfferID));
}

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);
HashSet<(uint RealAppID, EAssetType Type, EAssetRarity Rarity)> handledSets = [];

if (Bot.HasMobileAuthenticator) {
HashSet<ParseTradeResult> mobileTradeResults = results.Where(static result => result is { Result: ParseTradeResult.EResult.Accepted, Confirmed: false }).ToHashSet();
IEnumerable<Task<ParseTradeResult>> tasks = tradeOffers.Where(tradeOffer => (tradeOffer.State == ETradeOfferState.Active) && HandledTradeOfferIDs.Add(tradeOffer.TradeOfferID)).Select(tradeOffer => ParseTrade(tradeOffer, handledSets));

if (mobileTradeResults.Count > 0) {
HashSet<ulong> mobileTradeOfferIDs = mobileTradeResults.Select(static tradeOffer => tradeOffer.TradeOfferID).ToHashSet();
IList<ParseTradeResult> tradeResults = await Utilities.InParallel(tasks).ConfigureAwait(false);

(bool twoFactorSuccess, _, _) = await Bot.Actions.HandleTwoFactorAuthenticationConfirmations(true, Confirmation.EConfirmationType.Trade, mobileTradeOfferIDs, true).ConfigureAwait(false);
if (Bot.HasMobileAuthenticator) {
HashSet<ParseTradeResult> mobileTradeResults = tradeResults.Where(static result => result is { Result: ParseTradeResult.EResult.Accepted, Confirmed: false }).ToHashSet();

if (twoFactorSuccess) {
foreach (ParseTradeResult mobileTradeResult in mobileTradeResults) {
mobileTradeResult.Confirmed = true;
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 (results.Count > 0) {
await PluginsCore.OnBotTradeOfferResults(Bot, results as IReadOnlyCollection<ParseTradeResult> ?? results.ToHashSet()).ConfigureAwait(false);
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;
}

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) {
Expand Down Expand Up @@ -360,6 +381,7 @@ private async Task<ParseTradeResult> ParseTrade(TradeOffer tradeOffer) {
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);
Expand All @@ -374,8 +396,9 @@ private async Task<ParseTradeResult> ParseTrade(TradeOffer tradeOffer) {
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));
Expand Down Expand Up @@ -527,6 +550,19 @@ private async Task<ParseTradeResult> ParseTrade(TradeOffer tradeOffer) {

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;

Expand Down

0 comments on commit 1dff9a4

Please sign in to comment.