ENGN-4969 x/scheduler integration - tokenomics recalculation #881
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf Linter / buf (pull_request).
|
amimart
force-pushed
the
arnaud/engn-4255-expose-events-on-xscheduler-module
branch
3 times, most recently
from
c5936b1 to
c487365
Compare
guilherme-brandao
force-pushed
the
guilherme/tokenomics-scheduler-rebased2
branch
from
771e522 to
eec79a6
Compare
guilherme-brandao
changed the base branch from
arnaud/engn-4255-expose-events-on-xscheduler-module
to
dev
guilherme-brandao
changed the title
guilherme-brandao
requested review from
guilherme-brandao,
spooktheducks,
xmariachi and
zale144
as code owners
xmariachi
changed the title
There was a problem hiding this comment.
Hey @guilherme-brandao Here's some remarks mainly regarding on the articulation with the x/scheduler.
Btw maybe you should open a new PR, I can't request changes as this one is in my name, and I see some commit from dev in that one so maybe the occasion to clean that
| // Initialize recurring tasks after state is loaded | ||
| if loadLatest { | ||
| app.InitializeRecurringTasks() | ||
| } | ||
|
|
There was a problem hiding this comment.
| // Initialize recurring tasks after state is loaded | |
| if loadLatest { | |
| app.InitializeRecurringTasks() | |
| } |
That shouldn't be managed here, the x/mint tasks must be managed by the x/mint module
There was a problem hiding this comment.
No need to create a task type if there's no arguments
| // TaskHandlers returns the task handlers for the mint module | ||
| func (k *Keeper) TaskHandlers() schedulertypes.TaskHandlers { | ||
| return schedulertypes.TaskHandlers{ | ||
| schedulertypes.NewTaskHandler( |
There was a problem hiding this comment.
| schedulertypes.NewTaskHandler( | |
| schedulertypes.NewNoArgsTaskHandler( |
Let's declare it as a no args task
There was a problem hiding this comment.
The scheduler keeper should be added as module input and put in the mint's keeper
| return vm, err | ||
| } | ||
|
|
||
| if appKeepers != nil && appKeepers.SchedulerKeeper != nil { |
There was a problem hiding this comment.
All this migration should be achieved in the x/mint module migration
There was a problem hiding this comment.
I think the initial registering of the recalculation task when starting a brand new chain should occur in the begin blocker at the first block
| return schedulerKeeper.ScheduleTask( | ||
| ctx, | ||
| types.TaskEmissionRecalculation, | ||
| schedulertypes.TaskID("emission_recalculation"), |
There was a problem hiding this comment.
| schedulertypes.TaskID("emission_recalculation"), | |
| types.TaskEmissionRecalculation, |
| @@ -9,7 +9,7 @@ import ( | |||
| upgradekeeper "cosmossdk.io/x/upgrade/keeper" | |||
| emissionsKeeper "github.com/allora-network/allora-chain/x/emissions/keeper" | |||
There was a problem hiding this comment.
good alignment - however we can align the emissionsKeeper too for completeness :P
Purpose of Changes and their Description
This PR integrates the
x/mintmodule with thex/schedulermodule to enable periodic emission recalculation via scheduled tasks instead of the previous block-by-block approach in BeginBlocker.BeginBlockerand moved it to a scheduler-based task that runs approximately every 30 daysRecalculateEmissionkeeper method: Extracted the emission recalculation logic into a standalone method that can be invoked by the schedulerLink(s) to Ticket(s) or Issue(s) resolved by this PR
https://linear.app/alloralabs/issue/ENGN-4969/tokenomics-recalculation-with-xscheduler-integration
Are these changes tested and documented?
Unreleasedsection ofCHANGELOG.md?