Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call to my_swap_status rpc or implementation function occasionally may fail #2117

Open
dimxy opened this issue May 9, 2024 · 1 comment
Open

Comments

@dimxy
Copy link
Collaborator

dimxy commented May 9, 2024

Bug description
Test test_taker_completes_swap_after_taker_payment_spent_while_offline may fail with error:

thread 'docker_tests::swap_watcher_tests::test_taker_completes_swap_after_taker_payment_spent_while_offline' panicked at 'called `Result::unwrap()` on an `Err` value: "!status of 35844273-888e-42dc-9652-5a35f67ff20a: {\"error\":\"rpc:215] dispatcher_legacy:141] lp_swap:1117] saved_swap:211] fs:192] Error deserializing a swap: EOF while parsing a value at line 1 column 0\"}"', /home/runner/work/komodo-defi-framework/komodo-defi-framework/mm2src/mm2_test_helpers/src/for_tests.rs:2269:53

Commit: 9a82349
Environment: Native (non-wasm)

More Information
Apparently this may happen for swaps v1 due to concurrent access to the swap file: writing swap state to the swap file (while a swap in progress) and reading from the swap file by querying my_swap_status().
Example of this is test_taker_completes_swap_after_taker_payment_spent_while_offline test which waits for swap completion periodically calling my_swap_status().

Proposed Solution
We have several options to fix this:

  • use existing in the code solution with .tmp file when swap state is initially written to a tmp file and then renamed to its actual name. This prevents concurrent reading and writing the swap file (easiest one).
  • use RwLock to guard access to the swap file
  • migrate swap file to a field in the db for the native environment.
@dimxy
Copy link
Collaborator Author

dimxy commented May 13, 2024

I did an experiment and apparently the .tmp file option works okay both on linux and windows: I tried to open file.txt for reading and overwrite it with async_fs::rename() from file1.tmp to file1.txt to ensure that rename does not fail. I suggest use it for now and later get rid of swap files in favour db as we do it now for wasm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant