Fix issue with trade failures

[Quexington]

There was a sneaky issue here where trade could get set to None and thus fail to cancel even though it had previously been identified as needing to be cancelled.

[arvidn]

I think it would be good to have test coverage for this, perhaps not for the release, but in main at least

[Quexington]

I think it would be good to have test coverage for this, perhaps not for the release, but in main at least

I was trying to add a test for this but the thing is, I couldn't understand why this was ever working in the first place and then I realized it had to do with the order in which the coins were processed. This is kind of random and could have happened with any number of offers and so adding a test for it now once it has been fixed doesn't really make a ton of sense, because you could shift the order of the coins around to hit this edge case but you'd be neglecting to test the case that actually runs in production and it would be a very confusing test to exist anyways due to the fix already being in place.

I think the best way to think of this is as a small "refactor" where the existing test coverage should be leaned on as the bad code was just replaced with slightly better code. Adding a test for something that is now conceptually impossible just because it was at one point doesn't make a lot of sense to me.