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.

Sign up for GitHub

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

Jump to bottom

Racing condition when replacing txs #8

Closed
PaulRBerg opened this issue Jan 21, 2021 · 2 comments
Closed

Racing condition when replacing txs #8

PaulRBerg opened this issue Jan 21, 2021 · 2 comments
Labels
bug Something isn't working

Comments

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Jan 21, 2021 •
edited
Loading

There is a bug with the tx replacement logic.

  1. Sentinel triggers liquidation 0x123
  2. Sentinel sees that 0x123 wasn't mined quickly enough, so it broadcasts a replacement tx 0xabc with a higher gas price.
  3. At the same time with ending step Restrict monitoring to vaults with at least a minimum amount of collateral #2, tx 0x123 gets mined.
  4. 0xabc will revert with the reason "ERR_ACCOUNT_NOT_UNDERWATER"
  5. The "if let" expression on line 208 in liquidations.rs won't return true, because the 0xabc has been dropped and (I assume) Ethereum gives no receipt for these txs.
  6. The sentinel will keep attempting to replace 0xabc forever, even if 0x123 was mined, but it will fail to replace it. The Err variant will be used when broadcasting the replacement of the replacement tx.
@PaulRBerg PaulRBerg added the bug Something isn't working label Jan 21, 2021
@PaulRBerg PaulRBerg changed the title Dangling replacement txs Racing condition when replacing txs Jan 21, 2021
@PaulRBerg
Copy link
Contributor Author

PaulRBerg commented Jan 21, 2021 •
edited
Loading

Refer to the Racing condition when replacing transactions post on StackExchange for more information on the problem and the eventual solution that I found.

PaulRBerg added a commit that referenced this issue Jan 21, 2021
@PaulRBerg
fix: racing condition (gh issue #8) by checking for the nonce
8cc4b1a 
feat: "get_vault" method in "VaultsContainer"
refactor: iterate over pending tx tuples in "sentinel.rs" instead of "liquidations.rs"
refactor: "get_vault" to "query_vault"
refactor: "hifi_flash_swap" to "hifi_flash_swap_address"
refactor: "remove_pending_tx_tuple_or_bump_gas_price" to "remove_or_replace_tx"
@PaulRBerg
Copy link
Contributor Author

Logs:

ReplacementTxBug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@PaulRBerg