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

[Tokenomics] Post-scaffold cleanup & parameter updates foundation #287

Merged
merged 55 commits into from
Jan 23, 2024

Conversation

Olshansk
Copy link
Member

@Olshansk Olshansk commented Dec 20, 2023

Summary

  • Scaffolded the MsgUpdateParams in the tokenomics module with tests
  • Researched x/gov, x/upgrade and x/authz to set a pattern that follows Cosmos SDK best practices
    for governance parameter maintenance
  • Reviewers: Please look through the description in this ticket to get some context as well [Governance] Align on how we want to manage Governance Parameter Updates #322
  • Deferring all imports and lining errors to the tooling we have today: make go_lint && make go_imports
  • A few minor documentation NITs outside the scope of this PR that made it in accidently

Issue

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • Run all unit tests: make go_develop_and_test
  • Run E2E tests locally: make test_e2e
  • Run E2E tests on DevNet: Add the devnet-test-e2e label to the PR. This is VERY expensive, only do it after all the reviews are complete.

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have performed a self-review of my own code
  • I have commented my code, updated documentation and left TODOs throughout the codebase

@Olshansk Olshansk added protocol General core protocol related changes tokenomics Token Economics - what else do you need? labels Dec 20, 2023
@Olshansk Olshansk self-assigned this Dec 20, 2023
@Olshansk Olshansk added this to the Shannon TestNet milestone Dec 20, 2023
@Olshansk Olshansk force-pushed the issues/243/update_tokenomics_module branch from e327dd5 to 86cca4f Compare December 21, 2023 18:24
@Olshansk Olshansk changed the title [WIP] Tokenomics - post scaffold cleanup [WIP] Tokenomics - Post-scaffold cleanup & parameter-based MVP Jan 8, 2024
app/app.go Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
x/tokenomics/client/cli/tx_update_params_test.go Outdated Show resolved Hide resolved
x/tokenomics/client/cli/tx_update_params.go Outdated Show resolved Hide resolved
x/tokenomics/keeper/settle_session_accounting_test.go Outdated Show resolved Hide resolved
x/tokenomics/keeper/keeper.go Outdated Show resolved Hide resolved
@Olshansk
Copy link
Member Author

@bryanchriswhite @red-0ne Please see the updated PR description. Replied to most comments and just left a couple open.

Copy link
Contributor

@red-0ne red-0ne left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Just left a small typo change request

Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@bryanchriswhite bryanchriswhite left a comment

Choose a reason for hiding this comment

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

Preemptively approving. I think we should prefer to reference the nft and/or group modules as examples of modern cosmos-sdk best practices as they are the newest.

(see: #287 (comment))

x/tokenomics/keeper/keeper.go Show resolved Hide resolved
Copy link

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. If you just created a pull request, you might need to push another commit to produce a container image DevNet can utilize to spin up infrastructure. You can use make trigger_ci to push an empty commit.

@github-actions github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels Jan 23, 2024
@Olshansk Olshansk merged commit 30c23e6 into main Jan 23, 2024
10 checks passed
@Olshansk Olshansk deleted the issues/243/update_tokenomics_module branch January 23, 2024 17:33
Olshansk added a commit that referenced this pull request Jan 23, 2024
@Olshansk
Copy link
Member Author

@bryanchriswhite Missed your last comment but I made the update directly on main: f8b44a2

@bryanchriswhite bryanchriswhite removed push-image CI related - pushes images to ghcr.io devnet-test-e2e labels May 16, 2024
@github-actions github-actions bot removed the devnet label May 16, 2024
okdas added a commit that referenced this pull request Nov 14, 2024
- Scaffolded the `MsgUpdateParams` in the `tokenomics` module with tests
- Researched `x/gov`, `x/upgrade` and `x/authz` to set a pattern that follows Cosmos SDK best practices
   for governance parameter maintenance
- Reviewers: Please look through the description in this ticket to get some context as well #322 
- Deferring all imports and lining errors to the tooling we have today: `make go_lint && make go_imports`
- A few minor documentation NITs outside the scope of this PR that made it in accidently

---

Co-authored-by: Redouane Lakrache <[email protected]>
Co-authored-by: Dima Kniazev <[email protected]>
Co-authored-by: Bryan White <[email protected]>
okdas pushed a commit that referenced this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol General core protocol related changes tokenomics Token Economics - what else do you need?
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants