Make lock-tx id available in redeem/punish state to be able to assert exact fees #145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
D4nte
reviewed
Jan 18, 2021
swap/src/protocol/bob/state.rs
Outdated
| @@ -35,8 +35,8 @@ pub enum BobState { | |||
| CancelTimelockExpired(State4), | |||
| BtcCancelled(State4), | |||
| BtcRefunded(State4), | |||
| XmrRedeemed, | |||
| BtcPunished, | |||
| XmrRedeemed(State6), | |||
There was a problem hiding this comment.
I don't think we should define a State6. Let me try something locally see if it looks better.
bors bot
added a commit
that referenced
this pull request
Jan 18, 2021
144: Test refactor r=da-kami a=da-kami This PR is pure refactoring, keeping the logic of the tests we had before. No production code is touched besides re-exports in early commits (no logic changes). In the follow ups improvements will be introduced, that touch the production code as well. All remaining tasks actioned since Friday: - [x] `happy_path_bob _restart` (trivial) - [x] add refund assertions to harnesses (trivial) - [x] convert all refund scenarios currently being tested (trivial) - [x] remove dead test init code once all old tests are converted - [ ] ~~(optional) move alice and bob harness code into separate files~~ -> might action this once re-using test code in production. Out of scope, follow up: - [x] #145 - We can do exact assertions for Bob's redeem as well, but have to store Bob's `tx_lock` id in the respective final state. Make `tx_lock` available in `BtcRedeemed` and `BtcPunished` to have better assertions / harmonize test behaviour. - [ ] update the production code to use the `Alice` and `Bob` structs to bundle the params - update tests to use the production struct. - [ ] Re-use test swap setup in production (i.e. `Alice-/BobHarness::new`) to setup the swap. - [ ] add additional tests - [ ] re-try moving the tests from `test` to `src` (if the peer_id was the only problem this should be trivial now - but should be done after the refactor is finished) - [ ] creating new wallets upon restart - [ ] aborting the old event loop after restart Co-authored-by: rishflab <rishflab@hotmail.com> Co-authored-by: Daniel Karzel <daniel@comit.network>
|
bors r+ |
abraham-nixon
added a commit
to abraham-nixon/xmr-btc-swap
that referenced
this pull request
Feb 15, 2022
144: Test refactor r=da-kami a=da-kami This PR is pure refactoring, keeping the logic of the tests we had before. No production code is touched besides re-exports in early commits (no logic changes). In the follow ups improvements will be introduced, that touch the production code as well. All remaining tasks actioned since Friday: - [x] `happy_path_bob _restart` (trivial) - [x] add refund assertions to harnesses (trivial) - [x] convert all refund scenarios currently being tested (trivial) - [x] remove dead test init code once all old tests are converted - [ ] ~~(optional) move alice and bob harness code into separate files~~ -> might action this once re-using test code in production. Out of scope, follow up: - [x] comit-network/xmr-btc-swap#145 - We can do exact assertions for Bob's redeem as well, but have to store Bob's `tx_lock` id in the respective final state. Make `tx_lock` available in `BtcRedeemed` and `BtcPunished` to have better assertions / harmonize test behaviour. - [ ] update the production code to use the `Alice` and `Bob` structs to bundle the params - update tests to use the production struct. - [ ] Re-use test swap setup in production (i.e. `Alice-/BobHarness::new`) to setup the swap. - [ ] add additional tests - [ ] re-try moving the tests from `test` to `src` (if the peer_id was the only problem this should be trivial now - but should be done after the refactor is finished) - [ ] creating new wallets upon restart - [ ] aborting the old event loop after restart Co-authored-by: rishflab <rishflab@hotmail.com> Co-authored-by: Daniel Karzel <daniel@comit.network>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We can do exact assertions for Bob's redeem as well, but have to store Bob's tx_lock id in the respective final state. Make tx_lock available in BtcRedeemed and BtcPunished to have better assertions / harmonize test behaviour.
Storing this information is strictly speaking not needed for the production environment. But it is static information that can be seen as additional information that can be handy for a user. We could potentially extract it inside the tests as well (for redeem without restart would be a bit tricky), but I think this solution is more elegant.