Database & Resume Swap

[rishflab]

Rebased @D4nte work on the database onto current master.
Update tests to use event-loop and properly restart.

Notable changes:

• Refactored the e2e test to have re-usable building blocks to init containers, wallets, and initial state
• Refactored the network behavior to be stateless (we finally did that 🎉) so we can overcome re-starting issues where the behaviour would need an initial state that is not available
• Add [role]::recover function next to [role]::swap, (used in tests, to be plugged into CLI so we can recover :)

[da-kami]

Database and recovery done :)
Will adapt the CLI in the morning as small follow-up PR, the recover functions are already in place and just have to be called with the correct params which includes the additional param to pass a db dir in.

For completion we should add balance checks to the happy_path_restart tests. @rishflab would be cool if you could tackle that in the morning. I expect the balances to be correct given that we basically end a happy_path scenario and that test passes with balance checks - but better add them :)

[da-kami]

Let's not forget to squash the earlier commits on this branch 😬

[da-kami]

The tests pass locally when run individually. When running in CI there was a failure when running the tests that are in same file. Previously those tests had already passed, when there was just one test per file. It could be an initialization issue with the containers, but needs more investigation. I split the tests into multiple files for now to see if that resolves the issue. Maybe @thomaseizinger has an idea what could cause this?

Given that it fails early:

---- given_alice_restarts_after_encsig_is_learned_resume_swap stdout ----
thread 'given_alice_restarts_after_encsig_is_learned_resume_swap' panicked at 'called `Result::unwrap()` on an `Err` value: Failed to receive dial connection from Bob

I would guess it is a setup issue, maybe related to using the same port for listening,

[thomaseizinger]

Yeah you can't have them listen on the same port :)
I would suggest to use the MemoryTransport like we did in comit-rs to not run into this issue.
Alternatively you can listen on TCP port 0 to get one automatically assigned by the OS but that is a bit tricky because you have to get it out again of the NewListenAddress event before you can dial.

Note: I did not read any of the code :)

[D4nte]

I don't think "recovery" is a good terminology because if possible, the swap will continue. I suggest "Resume" instead.

Also, my work was on #83 I will see if I can combine our changes.

Can you please clean-up your commit tree?

[D4nte]

Why the file name change? this does more than just negotiation.

[da-kami]

Does it? I think negotiate is more accurate than execute. We can rename it again, but I think it is not the execution.

[D4nte]

It provides an abstraction that allows to do one function call per step for the protocol execution.

Look at the functions present in the file, they are not related to "negotiation":

• wait_for_locked_bitcoin
• lock_xmr
• etc

would steps.rs make more sense to you?

[D4nte]

What is recover for? Do you mean, resume?

[da-kami]

This is the terminology that was previously used in the project. I am happy to rename both the CLI option and the function to resume.

[D4nte]

The terminology previously used represented a different feature.

[rishflab]

my vote goes for resume

[D4nte]

This only tests for Alice to restart and refund. We need a test for Bob to restart to ensure that database save/restore works as expected.

[D4nte]

In my try I replaced this with futures::future::select. It allows to return a value, the state, removing the triplicate db save and run_until call.

[da-kami]

Closing this PR in favor of #85