fix: force thorchain swapper to wait for outbound tx confirmations on-chain #6143
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
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
woodenfurniture
commented
Feb 7, 2024
woodenfurniture
commented
Feb 7, 2024
gomesalexandre
approved these changes
Feb 7, 2024
There was a problem hiding this comment.
Runtime testing:
Swap to RUNE
- Confirmed this is still happy and completes pretty much instantly after inbound is seen since inner swaps to RUNE are effectively instant and do not trigger L1 outbounds
Swaps to EVM chain
isEvmCoinAssetnow properly returnstruefor EVM chains 🎉
- outbound confirmations are properly detected
- trade properly goes to confirmed state after on-chain confirmation
Swaps to UTXOs
- Trade is properly detected as complete after on-chain confirmation
Swap to ATOM
Note, I wasn't able to get to a confirmed state after outbound signed and on-chain confirmation, but logic is the same here, the issue here is there's a transient issue with unchained:
gomesalexandre
pushed a commit
that referenced
this pull request
Feb 7, 2024
0xApotheosis
added a commit
that referenced
this pull request
Feb 7, 2024
* feat: hide virtual pools in Thorchain LP "Available Pools" view * feat: thorchain LP halted pool tag * fix: force thorchain swapper to wait for outbound tx confirmations on-chain (#6143) * fix: import --------- Co-authored-by: woody <125113430+woodenfurniture@users.noreply.github.com> Co-authored-by: Apotheosis <97164662+0xApotheosis@users.noreply.github.com>
This was referenced Feb 8, 2024
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.
Description
Prevents thor trades from being displayed as complete until the outbound tx is confirmated on-chain.
Pull Request Type
Issue (if applicable)
closes #6086
Risk
Low risk, trade confirmation is already somewhat broken in that it completes early.
Testing
Check that thorchain trades only show as completed in the UI when they are in fact complete, not "pending outbound". Specifically thorchain trades that have an outbound tx, e.g
RUNE -> something, orBTC -> AVAX. Notsomething -> RUNE.NOTE: dont use viewblock to test this, use our app tx history
App can show as complete, while viewblock shows
Pending: Outbound. This is because they index the blockchain slower than us:And sure enough after giving them a moment to catch up, it's confirmed there too
Note @shapeshift/operations - we can also use THORNode /tx/ endpoint to test that the outbound Tx has been signed
Engineering
Operations
Screenshots (if applicable)
Outbound tx hash first seen, 0 confirmations. :
Trade continues to wait, with buy tx hash rendered and clickable. After some time, a confirmation is seen:
Trade is now shown as complete, in app and in viewblock for both txs:
