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

✨ Add Entrypoint That Sends Funds To a Recover Addr Upon Failure #70

Merged
merged 17 commits into from
Oct 17, 2023

Conversation

brimigs
Copy link
Contributor

@brimigs brimigs commented Oct 8, 2023

Since there are multiple contracts and entry points involved in the overall end to end transactions of the skip api, if an error occurs midway through the funds get "stuck" because the error will only revert to the beginning of the specific message that failed, not the entire transaction.

IMO the best way to handle this is by implementing sub-messages and replies, allowing us to handle the result of executing another contract.

Wrapping each returned message with a SubMsg, then handling the reply.id in the reply entry point gives us a way to properly handle all msgs that error. If an error is sent back to the contract, the reply message will return the tokens to the recovery address.

@brimigs brimigs marked this pull request as ready for review October 13, 2023 01:54
Copy link
Member

@NotJeremyLiu NotJeremyLiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is great!

I left some comments/suggestions on some places where I believe changes need to be made.

After those changes, would love one more run through to try and standardize some of the formatting / structure with the rest of the contract, the place I noticed this was that you defined the logic of AxelarSwapAndAction in contract.rs, whereas the rest of the contract defines the function that handles the logic in execute.rs.

I also noticed some unnecessary clones which when I run make clippy locally they're flagged but CI did not catch them so something in our CI is funky (I'd expect clippy CI to fail here), I'll look into CI later but if you could fix the clippy warnings running locally that would be great!

@NotJeremyLiu NotJeremyLiu changed the title add submsgs and replys in entry point contract for error handling Add Entrypoint That Always Sends Funds To a Recover Addr Upon Failure Oct 16, 2023
@NotJeremyLiu NotJeremyLiu changed the title Add Entrypoint That Always Sends Funds To a Recover Addr Upon Failure ✨ Add Entrypoint That Sends Funds To a Recover Addr Upon Failure Oct 17, 2023
Copy link
Member

@NotJeremyLiu NotJeremyLiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, let's start recovering!

@NotJeremyLiu NotJeremyLiu merged commit e3ece7f into main Oct 17, 2023
5 checks passed
@NotJeremyLiu NotJeremyLiu deleted the error-handling branch October 17, 2023 16:39
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

Successfully merging this pull request may close these issues.

2 participants