-
Notifications
You must be signed in to change notification settings - Fork 50
Adds COA handler and schedule txs #553
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
base: master
Are you sure you want to change the base?
Conversation
|
EVM is accessible in the emulator, right? We don't have to do anything else to make it work properly? I just want to make sure that is true before trying to write too many tests |
The emulator should have EVM enabled in the same way as other networks. Is anything not working? |
|
I haven't tried to test with it yet. I was just checking if there was anything special I needed to do |
2bcd63d to
fd30e16
Compare
| return | ||
| } | ||
|
|
||
| if let params = data as? COAHandlerParams { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should either be [COAHandlerParams] or the params struct should be updated to include a sequenced array of calls. In either case, sequenced calls would be very helpful for even basic EVM scenarios - e.g. DEX swap == approve & swap.
I also think there should be a mechanism by which a scheduler can define whether failure between calls forces a reversion.
Take two scenarios:
- I want to schedule listing 5 NFTs for sale
- I want to perform a DCA swap
In scenario 1., I likely don't care if one of the listings fail - I just want al listings executed and I may deal with the failed listings later.
In scenario 2., I probably want to abort the whole scheduled operation if my initial approve call fails.
As I'm writing, I'm also imagining another scenario where I want to schedule an arbitrage. In this scenario, I likely need the output of one call to be input of another call. You could argue that a scheduler should just leverage a solidity contract to perform all of this which is valid. But I think it's worth at least considering the scenario where it would be helpful to pipe data between calls and making a design decision about whether it's worth supporting or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good suggestions. I'll make it an array and allow them to revert if certain transactions fail. I'll come back to potentially using the output as an input after I've finished the other changes and wrote some tests for those
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a way to allow using the results of previous transactions in a generic way that we could use for this, so I'm going to leave it out for now
0d4d7bb to
b096716
Compare
Adds a handler type that can schedule transactions for a COA.
Currently can schedule deposit FLOW, withdraw FLOW, and
call(), but could possibly do more. This would make the handler more complicated though and may not be worth the effort