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] Implement difficulty proportional rewards #880

Merged
merged 24 commits into from
Oct 14, 2024

Conversation

red-0ne
Copy link
Contributor

@red-0ne red-0ne commented 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

Type of change

Select one or more from the following:

Testing

  • 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

…onal-rewards' into feat/difficulty-proportional-rewards
…onal-rewards' into feat/difficulty-proportional-rewards
@red-0ne red-0ne added the tokenomics Token Economics - what else do you need? label Oct 14, 2024
@red-0ne red-0ne added this to the Shannon Beta TestNet Launch milestone Oct 14, 2024
@red-0ne red-0ne self-assigned this Oct 14, 2024
Copy link

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 880)
Grafana network dashboard for devnet-issue-880

@github-actions github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels Oct 14, 2024
@red-0ne red-0ne merged commit da56f04 into main Oct 14, 2024
13 checks passed
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)
// because this code path is only reached if that has already been validated.
numClaimComputeUnits, err = claim.GetNumClaimedComputeUnits()
if err != nil {
return settledResult, expiredResult, err
}

// TODO(@red-0ne, #781): Convert numClaimedComputeUnits to numEstimatedComputeUnits to reflect reward/payment based on real usage.
// Get the relay mining difficulty for the service that this claim is for.
Copy link
Member

Choose a reason for hiding this comment

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

This whole section is super clean. Great job!

@@ -235,12 +237,28 @@ func (k Keeper) ProcessTokenLogicModules(
return tokenomicstypes.ErrTokenomicsServiceNotFound.Wrapf("service with ID %q not found", sessionHeader.ServiceId)
}

// Ensure the number of compute units claimed is equal to the number of relays * CUPR
expectedClaimComputeUnits := numRelays * service.ComputeUnitsPerRelay
Copy link
Member

Choose a reason for hiding this comment

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

Is there an opportunity to have a helper for this?

okdas pushed a commit that referenced this pull request Nov 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devnet devnet-test-e2e push-image CI related - pushes images to ghcr.io tokenomics Token Economics - what else do you need?
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants