mm201 / pkmn-classic-framework

Pokémon application logic for Generation IV and V, including servers
http://pkmnclassic.net/
Other
219 stars 43 forks source link

Comm error when retracting a Pokémon in the process of being traded #117

Open mm201 opened 1 year ago

mm201 commented 1 year ago

https://github.com/mm201/pkmn-classic-framework/blob/master/gts/pokemondpds.ashx.cs#L268 https://github.com/mm201/pkmn-classic-framework/blob/master/gts/syachi2ds.ashx.cs#L188

When you retract a Pokémon that someone else is in the process of trading, you will see a comm error. This is the only remaining GTS concurrency issue that isn't handled ideally. I'm filing this issue because it's both painful to fix and low-priority and I don't want to lose track of it. Previously, the GTS allowed the retraction to complete, leading to a comm error on the trader's side. That comm error had a risk of save file corruption, which the new error does not.

One alternative strategy would be to assume the trade will complete successfully and write the traded record in get.asp, but that would open us up to cloning exploits.

The ideal fix would be to await on the trade to either complete or expire, then generate a correct response depending, but this would require refactoring the GTS to use HttpTaskAsyncHandler which is too big a task right now for small reward.