[Application] Enforce minimum stake when burning #848

bryanchriswhite · 2024-10-01T13:23:01Z

Summary

Updates ProcessTokenLogicModules() logic to unbond applications whose stake is below the minimum after processing all TLMs.

Dependencies

Dependents

Issue

[Utility] Minimum staking values for each actor #612

Type of change

Select one or more from the following:

New feature, functionality or library
Consensus breaking; add the consensus-breaking label if so. See [Infra] Automatically add the consensus-breaking label #791 for details
Bug fix
Code health or cleanup
Documentation
Other (specify)

Testing

Documentation: make docusaurus_start; only needed if you make doc changes
Unit Tests: make go_develop_and_test
LocalNet E2E Tests: make test_e2e
DevNet E2E Tests: Add the devnet-test-e2e label to the PR.

Sanity Checklist

I have tested my changes using the available tooling
I have commented my code
I have performed a self-review of my own code; both comments & source code
I create and reference any new tickets, if applicable
I have left TODOs throughout the codebase, if applicable

…me as_type --response params

… name as_type --response params

## Summary - Adds minimum stake validation to the application stake message handler (i.e. total stake must be >= minimum stake). - Updates error returns in the same handler to ensure all error paths return appropriate gRPC status errors. - Replaces some warn and error level logs with info level, which I believe is more appropriate (until we have a practical debug level, at which point they should become debug logs). ## Dependencies - #809 - #843 - #844 - #845 ## Dependents - #848 - #849 - #850 - #857 - #852 - #861 - #851 - #863 ## Issue - #612 ## Type of change Select one or more from the following: - [x] New feature, functionality or library - [x] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [x] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [x] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: Redouane Lakrache <r3d0ne@gmail.com> Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com> Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

…to issues/612/application/burning * pokt/issues/612/application/staking: Empty commit [Tokenomics] Prevent GMR to produce zero values (#866) Empty commit

…burning * pokt/main: [DifficultyHash] Prepare for difficulty multiplier usage (#836) [Application] Enforce minimum stake when staking (#847)

red-0ne

Added a small comment request but everything looks good to me.

Preemptively approving.

red-0ne · 2024-10-11T05:09:02Z

x/tokenomics/keeper/token_logic_modules.go

+	// start checking the denom here.
+	if application.Stake.Amount.LT(apptypes.DefaultMinStake.Amount) {
+		// Unbond the application because it has less than the minimum stake.
+		if err = k.applicationKeeper.UnbondApplication(ctx, &application); err != nil {


Could you please add a comment that unbonding is only viable when an application is only staked for a single service.

If it is staked for multiple one, this would prevent other settlements to burn tokens.

Set application UnbondingSessionEndHeight = -1 and save here.

In SettlePendingClaims(), after ProcessTokenLogicModules() loop, unstake apps from 1 by lookup via claims.

https://www.notion.so/buildwithgrove/Claim-Settlement-Safety-11ca36edfff6809aa951fe70a65443da

NOTE: UnbondingSessionEndHeight is a uint64 so I went with 1 instead of -1 to keep the change small. I think this should be fine as 1 is not really a practical value for an actual unbonding session end height.

Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

bryanchriswhite · 2024-10-11T15:24:52Z

E2E tests passing locally but timing out in CI, waiting for devnet to come up.

…-update-param * pokt/main: [Application] Enforce minimum stake when burning (#848)

* issues/612/shared/refresh: (46 commits) test: supplier up-staking from below min. stake fix: linter error fix: flaky test [Application] Enforce minimum stake when burning (#848) Empty commit chore: review feedback improvements chore: regenerate protobufs chore: review feedback improvements test: fix application min stake integration test chore: review feedback improvements Empty commit fix: linter error chore: quick fixes [DifficultyHash] Prepare for difficulty multiplier usage (#836) [Application] Enforce minimum stake when staking (#847) Empty commit [Tokenomics] Prevent GMR to produce zero values (#866) Empty commit [Application] Add `min_stake` application module param (#845) chore: regenerate protobufs ...

* issues/612/proof/refresh: (46 commits) test: supplier up-staking from below min. stake fix: linter error fix: flaky test [Application] Enforce minimum stake when burning (#848) Empty commit chore: review feedback improvements chore: regenerate protobufs chore: review feedback improvements test: fix application min stake integration test chore: review feedback improvements Empty commit fix: linter error chore: quick fixes [DifficultyHash] Prepare for difficulty multiplier usage (#836) [Application] Enforce minimum stake when staking (#847) Empty commit [Tokenomics] Prevent GMR to produce zero values (#866) Empty commit [Application] Add `min_stake` application module param (#845) chore: regenerate protobufs ... # Conflicts: # x/service/types/errors.go

## Summary ```bash ignite scaffold message update-param --module supplier --signer authority name as_type --response params ``` Adds the `MsgUpdateParam` message so that the supplier module may update individual parameters following section 0 from the updated docs (see #839). ## Dependencies - #809 - #843 - #844 - #845 - #847 - #848 ## Dependents - #850 - #857 - #852 - #861 - #851 - #863 ## Issue - #612 ## Type of change Select one or more from the following: - [x] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [x] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [x] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: Redouane Lakrache <r3d0ne@gmail.com> Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com> Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

…-uint64 * issues/612/service/refresh: (46 commits) test: supplier up-staking from below min. stake fix: linter error fix: flaky test [Application] Enforce minimum stake when burning (#848) Empty commit chore: review feedback improvements chore: regenerate protobufs chore: review feedback improvements test: fix application min stake integration test chore: review feedback improvements Empty commit fix: linter error chore: quick fixes [DifficultyHash] Prepare for difficulty multiplier usage (#836) [Application] Enforce minimum stake when staking (#847) Empty commit [Tokenomics] Prevent GMR to produce zero values (#866) Empty commit [Application] Add `min_stake` application module param (#845) chore: regenerate protobufs ...

## Summary - Adds the `min_stake` param to the supplier module following section 0 from the updated docs (see #839). - Adds missing validation unit test coverage around existing application module param, max_delegated_gateways. - Pushes validation from the MsgUpdateParam handler down into MsgUpdateParam#ValidateBasic(). - Ensures Params#Valid() is called prior to setting params in MsgUpdateParam handler. - Updates error returns in the same handler to ensure all error paths return appropriate gRPC status errors. ## Dependencies - #809 - #843 - #844 - #845 - #847 - #848 - #849 ## Dependents - #857 - #852 - #861 - #851 - #863 ## Issue - #612 ## Type of change Select one or more from the following: - [x] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [x] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [x] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: Redouane Lakrache <r3d0ne@gmail.com> Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com> Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

## Summary 1. Adds minimum stake validation to the supplier stake message handler (i.e. total stake must be >= minimum stake). 2. Updates error returns in the same handler to ensure all error paths return appropriate gRPC status errors. ## Dependencies - #809 - #843 - #844 - #845 - #847 - #848 - #849 - #850 ## Dependents - #852 - #861 - #851 ## Issue - #612 ## Type of change Select one or more from the following: - [x] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [ ] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [x] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: Redouane Lakrache <r3d0ne@gmail.com> Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com> Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

…cation * pokt/main: [Supplier] Enforce minimum stake when staking (#857) [Supplier] Add `min_stake` supplier module param (#850) [Supplier] Add `MsgUpdateParam` to application module (#849) [Application] Enforce minimum stake when burning (#848)

## Summary The "adding_params.md" doc has (or will have) changed substantially (see #839) since the shared module's param logic was last updated. This PR aligns the shared module's param logic with the snippets in the updated docs. Specifically, this refresh includes: - Moving validation logic from `msgServer#UpdateParam()` to `MsgUpdateParam#ValidateBasic()`. - Replacing magic strings of param names with their standard variable counterparts (i.e. `sharedtypes.Param...`). - Improving some local variable names. - Replacing usages of `interface{}` with `any`. ## Dependencies - #809 - #843 - #844 - #845 - #847 - #848 - #849 - #850 - #857 ## Dependents - #851 - #861 ## Issue - #612 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: Redouane Lakrache <r3d0ne@gmail.com> Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com> Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

## Summary The "adding_params.md" doc has (or will have) changed substantially (see #839) since the proof module's param logic was last updated. This PR aligns the proof module's param logic with the snippets in the updated docs. Specifically, this refresh includes: - Moving validation logic from `msgServer#UpdateParam()` to `MsgUpdateParam#ValidateBasic()`. - Replacing magic strings of param names with their standard variable counterparts (i.e. `prooftypes.Param...`). - Improving some local variable names. - Replacing usages of `interface{}` with `any`. - Improving validation for all coin type params to be consistent with application, gateway, and supplier coin type validation. # Dependencies - #809 - #843 - #844 - #845 - #847 - #848 - #849 - #850 - #857 - #852 ## Dependents - #861 ## Issue - #612 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: Redouane Lakrache <r3d0ne@gmail.com> Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com> Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

## Summary The "adding_params.md" doc has (or will have) changed substantially (see #839) since the shared module's param logic was last updated. This PR aligns the service module's param logic with the snippets in the updated docs. Specifically, this refresh includes: - Moving validation logic from `msgServer#UpdateParam()` to `MsgUpdateParam#ValidateBasic()`. - Replacing magic strings of param names with their standard variable counterparts (i.e. `prooftypes.Param...`). - Improving some local variable names. - Replacing usages of `interface{}` with `any`. - Improving validation for all coin type params to be consistent with application, gateway, and supplier coin type validation. ## Dependencies - #809 - #843 - #844 - #845 - #847 - #848 - #849 - #850 - #857 - #852 - #851 ## Issue - #612 ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [ ] **Unit Tests**: `make go_develop_and_test` - [ ] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [ ] I have tested my changes using the available tooling - [ ] I have commented my code - [ ] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: Redouane Lakrache <r3d0ne@gmail.com> Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com> Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

…ed-params * pokt/main: [On-Chain] Refactor `uint64` type individual param update logic (#863) [Service] Refresh service module params logic (#861) [Proof] Refresh proof module params logic (#851) [Shared] Refresh shared module params logic (#852) [Docs] Fix example of how to set rev share precentage (#877) [Tokenomics] Implement difficulty proportional rewards (#880) [Supplier] Enforce minimum stake when staking (#857) [Supplier] Add `min_stake` supplier module param (#850) [Supplier] Add `MsgUpdateParam` to application module (#849) [Application] Enforce minimum stake when burning (#848) [DifficultyHash] Prepare for difficulty multiplier usage (#836) [Application] Enforce minimum stake when staking (#847) [Tokenomics] Prevent GMR to produce zero values (#866)

* pokt/main: [On-Chain] Refactor `uint64` type individual param update logic (#863) [Service] Refresh service module params logic (#861) [Proof] Refresh proof module params logic (#851) [Shared] Refresh shared module params logic (#852) [Docs] Fix example of how to set rev share precentage (#877) [Tokenomics] Implement difficulty proportional rewards (#880) [Supplier] Enforce minimum stake when staking (#857) [Supplier] Add `min_stake` supplier module param (#850) [Supplier] Add `MsgUpdateParam` to application module (#849) [Application] Enforce minimum stake when burning (#848) [DifficultyHash] Prepare for difficulty multiplier usage (#836) [Application] Enforce minimum stake when staking (#847) [Tokenomics] Prevent GMR to produce zero values (#866)

bryanchriswhite added 30 commits September 26, 2024 08:56

scaffold: message update-param --module gateway --signer authority na…

d1cd5e6

…me as_type --response params

chore: update gateway MsgUpdateParam fields

91eecfc

chore: update MsgUpdateParamResponse fields

ef62280

chore: comment out autocli for gateway module param updates

f7a6e5b

fix: linter error

8957f1c

revert: premature changes

3016ca3

feat: add min_stake gateway module param

899effb

chore: cleanup makefiles and param update JSON files

83c1d74

chore: add gateway module param make targets

55e6a31

refctor: consolidate param config types

ddd68bb

tests: improve error messaging

fa8caf6

tests: add coverage over gateway min stake param updates

ae006e2

chore: add as_coin as_type type 😉

e92e3b1

chore: add app MsgUpdateParam to genesis authorizations

b2e1267

fix: typo

0373fa2

chore: cleanup makefiles

da7894a

wip: fixing tests

015c537

tests: add tests

1ac6c48

tests: remove erroneous case

db366f6

test: gateway staking below minimum fails

60bef17

feat: gateway min stake validation & grpc status error returns

a21687f

chore: cleanup comments

7e9f857

scaffold: message update-param --module examplemod --signer authority…

4140710

… name as_type --response params

chore: update MsgUpdateParam fields

0a8d922

chore: comment out application MsgUpdateParam autocli

c88edd6

chore: add application MsgUpdateParam to dao genesis authorizations

da51239

chore: update NewMsgUpdateParam constructor

45c5a99

fix: simulations generated code

d987e09

chore: add min_stake param to application module params

0da255d

chore: update params config for param updates test suite

4542628

This was referenced Oct 9, 2024

[Shared] Refresh shared module params logic #852

Merged

[Proof] Refresh proof module params logic #851

Merged

[Service] Refresh service module params logic #861

Merged

bryanchriswhite added 3 commits October 10, 2024 09:10

Merge remote-tracking branch 'pokt/issues/612/application/staking' in…

0fa9ab8

…to issues/612/application/burning * pokt/issues/612/application/staking: Empty commit [Tokenomics] Prevent GMR to produce zero values (#866) Empty commit

Merge remote-tracking branch 'pokt/main' into issues/612/application/…

d91671e

…burning * pokt/main: [DifficultyHash] Prepare for difficulty multiplier usage (#836) [Application] Enforce minimum stake when staking (#847)

chore: quick fixes

c89ee93

bryanchriswhite changed the base branch from issues/612/application/staking to main October 10, 2024 07:24

bryanchriswhite added 2 commits October 10, 2024 13:54

fix: linter error

ed0d795

Empty commit

0be55c6

red-0ne approved these changes Oct 11, 2024

View reviewed changes

bryanchriswhite and others added 3 commits October 11, 2024 12:27

chore: review feedback improvements

14a5c79

Co-authored-by: red-0ne <red-0ne@users.noreply.github.com>

test: fix application min stake integration test

735ca2a

Empty commit

b220177

bryanchriswhite merged commit b60dac9 into main Oct 11, 2024
9 of 10 checks passed

bryanchriswhite added a commit that referenced this pull request Oct 11, 2024

Merge remote-tracking branch 'pokt/main' into issues/612/supplier/msg…

788d506

…-update-param * pokt/main: [Application] Enforce minimum stake when burning (#848)

bryanchriswhite deleted the issues/612/application/burning branch October 14, 2024 12:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Application] Enforce minimum stake when burning #848

[Application] Enforce minimum stake when burning #848

bryanchriswhite commented Oct 1, 2024 •

edited

Loading

red-0ne left a comment

red-0ne Oct 11, 2024

bryanchriswhite Oct 11, 2024 •

edited

Loading

bryanchriswhite Oct 11, 2024

bryanchriswhite Oct 11, 2024

bryanchriswhite commented Oct 11, 2024

[Application] Enforce minimum stake when burning #848

[Application] Enforce minimum stake when burning #848

Conversation

bryanchriswhite commented Oct 1, 2024 • edited Loading

Summary

Dependencies

Dependents

Issue

Type of change

Testing

Sanity Checklist

red-0ne left a comment

Choose a reason for hiding this comment

red-0ne Oct 11, 2024

Choose a reason for hiding this comment

bryanchriswhite Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

bryanchriswhite Oct 11, 2024

Choose a reason for hiding this comment

bryanchriswhite Oct 11, 2024

Choose a reason for hiding this comment

bryanchriswhite commented Oct 11, 2024

bryanchriswhite commented Oct 1, 2024 •

edited

Loading

bryanchriswhite Oct 11, 2024 •

edited

Loading