add test to swap bought asset from maker

[Arshia-r-m]

The test added shows an unexpected behavior of the nodes after the 2nd node swapped (bought) 2 assets with the 1st node and then tried to swapped these assets with the 1st node again.

[nicbus]

Swap quantities (qty_from, qty_to) behave the opposite way, as documented for MakerInitRequest:

// "from" and "to" are seen from the taker's perspective, so:
// - "from" is what the taker will send and the maker will receive
// - "to" is what the taker will receive and the maker will send

Therefore, when swapping from 5 asset 1 to 5 asset 2:

• the maker will send 5 asset 2 and receive 5 asset 1
• the taker will send 5 asset 1 and receive 5 asset 2

The expected off-chain balances after the last swap in the test then are:

• maker: 595 asset 1, 585 asset 2
• taker: 5 asset 2, 15 asset 2

With these balances the test passes.

That being said, I've observed the test still failing sometimes due to balances not updating after the last swap (asset 1 remaining 590/10), will look into that.

PS: a dedicated issue_second_asset_nia helper isn't needed, the existing one (issue_asset_nia) can be used for both assets; even if ticker and name are the same, the asset ID will be unique so they behave as two distinct assets.

[Arshia-r-m]

Thanks for the clear explanations, the expected amounts after the last swap is wrong and I'll modify it, but the fact that the the balances aren't updated remains the same.

The test tries to replicate the exact situation I faced multiple times in my testing where the maker marks the swap as failed, taker marks it as pending and the amounts will not change.

Let me know if I could provide more details.

[nicbus]

Thanks for the clear explanations, the expected amounts after the last swap is wrong and I'll modify it, but the fact that the the balances aren't updated remains the same.

From some local test runs I did, with the correct balances the test passes most of the time.
As I was mentioning in my previous comment, it doesn't work 100% of the time, which is odd and we'll be looking into that.

The test tries to replicate the exact situation I faced multiple times in my testing where the maker marks the swap as failed, taker marks it as pending and the amounts will not change.

From this I get that you expect the test to fail because of the maker failing the swap, is my understanding correct?
Anyway, will do more local runs and check for this kind of behavior.

[Arshia-r-m]

From this I get that you expect the test to fail because of the maker failing the swap, is my understanding correct? Anyway, will do more local runs and check for this kind of behavior.

To be exact, from my understanding, the maker marks the swap as failed because it fails to pay to taker, so the swap it self is marked as failed, but on the taker side it is stuck in pending status and also the balances doesn't change on both end.
I'll update the test asap and push it again.

[nicbus]

To be exact, from my understanding, the maker marks the swap as failed because it fails to pay to taker, so the swap it self is marked as failed, but on the taker side it is stuck in pending status and also the balances doesn't change on both end.

Both maker and taker mark the swap as succeeded, but the balance fails to update to the expected values because the second part of the swap sometimes happens on the wrong channel.

The underlying cause of the bug has been identified and fixed.

We're going to merge this test and will then improve on it to add more checks.

Before we merge it, though, we'd like for the name of the test to be changed into swap_assets_liquidity_both_ways (including the name and directory inside the test file) and the commit to be signed. Can you please apply these changes?

[Arshia-r-m]

Of-course, thanks for resolving this.

[zoedberg]

Merged, thanks!