Callback scheduler #485
Callback scheduler #485
Conversation
joshuahannan
left a comment
joshuahannan
left a comment
There was a problem hiding this comment.
I haven't really started looking at the SharedScheduler resource yet, but I figured I'd leave these comments now so you can start looking at them while I'm reviewing that part and you can start answering my questions
| } | ||
|
|
||
| let amount = self.fees.balance * multiplier | ||
| return <- self.fees.withdraw(amount: amount) as! @FlowToken.Vault |
There was a problem hiding this comment.
what happens to fees that aren't withdrawn? Are they just burnt?
There was a problem hiding this comment.
Good question, not sure what is the pattern for that on the service accounts, should we just destroy the vault?
There was a problem hiding this comment.
this is probably something that should be brought up with the governance/tokenomics people. I would imagine that the right call would just be to destroy them, but anything that has to do with minting/destroying FLOW by the protocol should be a bigger discussion that we get more approvals on. Can you add a section to the FLIP about this? I think we should propose to destroy the leftover FLOW
There was a problem hiding this comment.
Will add a section. They are destroyed now.
There was a problem hiding this comment.
If it's easy, we should put them into the fee pool, if that's hard, we can just burn them.
turbolent
left a comment
turbolent
left a comment
There was a problem hiding this comment.
Great work! Mostly looked at the PR from a Cadence code perspective, didn't review the logic
joshuahannan
left a comment
joshuahannan
left a comment
There was a problem hiding this comment.
I'm just leaving my comments here as notes for myself for when I make my changes in my PR. I am updating the target branch to the feature branch and merging it into that and not into master so I can more easily work on it
| // minimum execution effort is the minimum effort that can be | ||
| // used for any priority | ||
| access(self) var minimumExecutionEffort: UInt64 |
There was a problem hiding this comment.
Is this cumulative or per callback?
There was a problem hiding this comment.
@devbugging Just flagging this for you to answer. I'll update the comment once I get an answer from you
| // priority fee multipliers are values we use to calculate the added | ||
| // processing fee for each priority | ||
| access(self) var priorityFeeMultipliers: {Priority: UFix64} | ||
| // refund multiplier is the portion of the fees that are refunded when a callback is cancelled |
There was a problem hiding this comment.
| // refund multiplier is the portion of the fees that are refunded when a callback is cancelled | |
| // refund multiplier is the portion of the fees that are refunded when a callback is cancelled. Must be between zero and one |
Is this true?
joshuahannan
merged commit 8fc371f
into
onflow:feature/callback-scheduling
|
Was the merge an accident? All the feedback was just marked as resolved but there are no commits? |
|
I'm taking over a lot of this work from Gregor, so I merged this into a feature branch on this repo so I could make the changes myself here instead of Gregor's fork. The open PR is here: #486 |
4 participants
Implementation of callback scheduler contract following the FLIP onflow/flips#331