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

Introduce custom staking module that update validators voting power each N blocks #320

Merged
merged 59 commits into from
Jan 2, 2024

Conversation

RustNinja
Copy link
Collaborator

@RustNinja RustNinja commented Dec 22, 2023

2 modules introduced:

  • custom staking module that actually override logic of cosmos staking.
  • staking middleware that store params how often update voting power for validators

BlockValidatorUpdates
method taken from cosmosSDK/x/staking module
and if line introduced that basically now execute ApplyAndReturnValidatorSetUpdates each N blocks

if shouldExecuteBatch {
        v, err := k.ApplyAndReturnValidatorSetUpdates(ctx)
        if err != nil {
	        panic(err)
        }
        validatorUpdates = v
}

RustNinja and others added 30 commits December 12, 2023 23:14
…rver-func

Reimplement delegate msg server func
fix custom staking keeper msg server issue with nil keeper
…e-module-into-cosmos-chain

integrate staking middleware module into cosmos chain
…ustom-staking-module

use staking middleware in custom staking module to batch delegation.
…-staking

override end block custom staking module implementation
…atching

add logic to app module to batch delegation.
@joe-bowman
Copy link
Collaborator

An aside and perhaps for another time:

Why is the custom staking module in custom/staking and not under x/staking or x/customstaking?

@RustNinja
Copy link
Collaborator Author

An aside and perhaps for another time:

Why is the custom staking module in custom/staking and not under x/staking or x/customstaking?

thanks for question sir.
the reason why custom/staking is not under /x bz of the same reason that custom/bank is not under /x
custom staking it is just a wrapper around cosmos staking that does not have any storage, not any changes in query or tx service. so imo it should be in /custom folder because custom bask is there as well.

@RustNinja
Copy link
Collaborator Author

Looks good for the most part.

I don't understand the point of the stakingmiddleware - it appears to exist to manage params only, which are then consumed by the custom staking module. Why are the params not part of the custom staking module?

Otherwise, many references in comments to 'mint' from presumably using mint module as the template. Should be updated to reflect reality.

thanks. initially this module was created to collect all staking bond/unbond request from users and execute them later each N block in EndBlock. but later we changed the approach to execute initial staking End block validator set update each N block. so i would like to keep this logic with storing params separately to not introduce extra storage key into the initial staking module and not introduce any changes into tx/query service into cosmos staking module that not represented as wrapper module custom-staking.

@faddat faddat mentioned this pull request Dec 27, 2023
3 tasks
@joe-bowman
Copy link
Collaborator

An aside and perhaps for another time:
Why is the custom staking module in custom/staking and not under x/staking or x/customstaking?

thanks for question sir. the reason why custom/staking is not under /x bz of the same reason that custom/bank is not under /x custom staking it is just a wrapper around cosmos staking that does not have any storage, not any changes in query or tx service. so imo it should be in /custom folder because custom bask is there as well.

I'd argue that all your modules in x/ are 'custom', and both bank and staking ought to be in x/, but it's a personal preference thing :)

@RustNinja
Copy link
Collaborator Author

update:
Tested with @rjonczy on devnet with a 2 validators
worked as expected:

  • unbond
  • validator set did not changed immediately
  • validator set voting power changed later on block X that % N without rest

cc @blasrodri @JafarAz

@blasrodri
Copy link

If @faddat and @joe-bowman are happy we merge

app/keepers/keepers.go Outdated Show resolved Hide resolved
@faddat
Copy link
Contributor

faddat commented Dec 29, 2023

@blasrodri I think we may need to rebase this on v7 but I agree with you on readiness, pending addressing Joe's questions.

Copy link
Collaborator

@joe-bowman joe-bowman left a comment

Choose a reason for hiding this comment

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

lgtm.

@RustNinja RustNinja merged commit a463217 into master Jan 2, 2024
15 of 16 checks passed
RustNinja added a commit that referenced this pull request Jan 8, 2024
…leware-module

Introduce custom staking module that update validators voting power each N blocks
RustNinja added a commit that referenced this pull request Jan 15, 2024
…leware-module

Introduce custom staking module that update validators voting power each N blocks
tungleanh0902 pushed a commit to notional-labs/composable-cosmos that referenced this pull request Mar 7, 2024
…/stakingmiddleware-module

Introduce custom staking module that update validators voting power each N blocks
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.

5 participants