-
Notifications
You must be signed in to change notification settings - Fork 51
Adds FlowCallbackUtils contract #523
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
Conversation
|
In general I really liked the util, but I am also a bit hesitant on this; I think my main issue is I am not clear in my head, who is the actor ? is it the user ? wallet as agent of user ? or is it the app ? till now I was always thinking app as the actor, but this utility also makes a good case for user / wallet to be the actor. |
|
@bluesign I think the goal is to design it so that it can work well with any actor. I think this construct definitely makes it easier for individual users or wallets to manage callbacks, but the jury is still out on if this is a good construct for apps to manage callbacks. I think it could be good for that, but I'm not totally sure. |
|
I can see the utility of this, but don't like the need to change the scheduler contract. This could easily be just a util contract composed on top without changing the contract. |
|
This is a great addition. It gives both devs and apps a simple, standardized way to list and manage all callbacks for an account, basically a Flow-native “Task Manager” without third-party indexing. On the schedule function: making it public is useful since it allows usage with or without the manager that can fit most complicated use cases as mentioned by @sisyphusSmiling. The downside is that callbacks scheduled directly won’t show up in the manager, so extra code would be needed to surface them. If we’re fine with only "standard" callbacks being discoverable, I’m in favor of making it public, otherwise I’d keep it restricted. Either way, adding the manager feels essential to set a standard for callbacks retrieval for an account. |
|
Looks like the consensus is that the schedule function should be |
20b5041 to
ee250db
Compare
|
Closing this in favor of #530 |
It is important to remember that since handlers still need to be created for the actual logic of the callback, this does not allow us to completely standardize the callback transactions because you still need to have code in your transactions to deal with your specific callback handler
I'm still looking for feedback on these changes, so please take a look and let me know what you think. I especially need feedback about usability. I think this improves usability, but I don't always have the best instincts about that.