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

[DifficultyHash] Prepare for difficulty multiplier usage #836

Merged
merged 9 commits into from
Oct 10, 2024

Conversation

red-0ne
Copy link
Contributor

@red-0ne red-0ne commented Sep 24, 2024

Summary

This PR prepares for claimed amount calculation to use the RelayDifficultyMultiplier.

  • It removes the RelayDifficultyTargetHash proof param.
  • Replace prooftypes.DefaultRelayDifficultyTargetHash occurrences with protocol.BaseRelayDifficultyHashBz.
  • Moves the RleayMiningDifficulty logic to the service module due to cyclic dependencies.
  • Rename num_compute_units to num_claimed_compute_units and add num_estimated_compute_units and claimed_amount_upokt
  • Remove the no longer used tokenomics querier to retrieve a service difficulty target hash.

The PR is only renaming and moving of files most (~13200LOC) changes are auto-generated code.

Type of change

Select one or more from the following:

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

@red-0ne red-0ne added code health Cleans up some code consensus-breaking IMPORTANT! If the PR with this tag is merged, next release WILL HAVE TO BE an upgrade. labels Sep 24, 2024
@red-0ne red-0ne added this to the Shannon Beta TestNet Launch milestone Sep 24, 2024
@red-0ne red-0ne self-assigned this Sep 24, 2024
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.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Please merge main, this feature doesn't exist anymore. 😅

Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

Will TAL after merged with main

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.

image

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! 🙌

Just a couple questions and small suggestions - otherwise this LGTM! 🚢

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for spotting this! 🙏

I've opened #864 to fix the underlying issue (as well as another).

testutil/integration/app.go Outdated Show resolved Hide resolved
Comment on lines +99 to +101
t.Cleanup(func() {
delete(relayDifficultyTargets, serviceId)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

👌 Nice. 😎

x/proof/types/message_update_param_test.go Outdated Show resolved Hide resolved
// this line is used by starport scaffolding # types/genesis/validField
},
expectedErr: nil,
},
{
desc: "invalid - duplicated relayMiningDifficulty",
Copy link
Contributor

Choose a reason for hiding this comment

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

👌

x/service/types/genesis.go Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Oct 9, 2024

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks.

You may need to run make trigger_ci to submit an empty commit that'll trigger the tests.

GCP workloads (requires changing the namespace to 836)
Grafana network dashboard for devnet-issue-836

@github-actions github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels Oct 9, 2024
@red-0ne red-0ne dismissed Olshansk’s stale review October 10, 2024 02:32

Olshansky declared code review bankruptcy

@red-0ne red-0ne merged commit 879401e into main Oct 10, 2024
13 checks passed
bryanchriswhite added a commit that referenced this pull request Oct 10, 2024
…burning

* pokt/main:
  [DifficultyHash] Prepare for difficulty multiplier usage (#836)
  [Application] Enforce minimum stake when staking (#847)
bryanchriswhite added a commit that referenced this pull request Oct 10, 2024
…application

* issues/612/application/burning: (94 commits)
  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
  chore: regenerate protobufs
  fix: linter error
  Empty commit
  chore: review feedback improvements
  Empty commit
  Empty commit
  fix: linter errors
  fix: typo
  chore: review feedback improvements
  chore: reconcile PreGeneratedAccountIterator#MustNext()
  chore: add review feedback TODOs
  Empty commit
  test: simplify coin equality assertions
  ...

# Conflicts:
#	x/application/keeper/msg_server_stake_application_test.go
#	x/application/keeper/msg_server_unstake_application.go
#	x/application/keeper/msg_server_unstake_application_test.go
#	x/application/keeper/unbond_applications.go
red-0ne added a commit that referenced this pull request Oct 11, 2024
## Summary

This PR incorporated the mining difficulty into the claim reward
calculation.
* Encapsulates the claimed tokens into a `claim.GetClaimeduPOKT(params,
difficulty)`
* Implements a test to assert that the difficulty based rewards are
proportional to the off-chain relays served.
* Removes unused `min_relay_difficulty_bits` gov. param.

It is based on the preparation work of PR #836.

## Issue

- #781 
- #758 

## 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

- [x] **Unit Tests**: `make go_develop_and_test`
- [x] **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: Bryan White <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: red-0ne <[email protected]>
bryanchriswhite added a commit that referenced this pull request Oct 14, 2024
* 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
  ...
bryanchriswhite added a commit that referenced this pull request Oct 14, 2024
* 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
bryanchriswhite added a commit that referenced this pull request Oct 14, 2024
…-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
  ...
red-0ne added a commit that referenced this pull request Oct 14, 2024
**This is a repost of PR #840 which did not get merged into main**

## Summary

This PR incorporated the mining difficulty into the claim reward
calculation.
* Encapsulates the claimed tokens into a `claim.GetClaimeduPOKT(params,
difficulty)`
* Implements a test to assert that the difficulty based rewards are
proportional to the off-chain relays served.
* Removes unused `min_relay_difficulty_bits` gov. param.

It is based on the preparation work of PR #836.

## Issue

- #781 
- #758 

## 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

- [x] **Unit Tests**: `make go_develop_and_test`
- [x] **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
bryanchriswhite added a commit that referenced this pull request Oct 15, 2024
…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)
bryanchriswhite added a commit that referenced this pull request Oct 15, 2024
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health Cleans up some code consensus-breaking IMPORTANT! If the PR with this tag is merged, next release WILL HAVE TO BE an upgrade. devnet devnet-test-e2e push-image CI related - pushes images to ghcr.io
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants