[WIP][Utility] Feat: Chain-specific compute unit to tokens multipliers

rBurgett commented 1 month ago

Summary

Added a compute_units_per_relay param to new services
Updated SettleSessionAccounting() to compute units per relay along with the general compute units to tokens multipliers
Added new tests.
Updated existing tests
Added comments where necessary

Issue

494

Type of change

Select one or more:

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

Testing

Documentation changes (only if making doc changes)

[ ] make docusaurus_start; only needed if you make doc changes

Local Testing (only if making code changes)

[x] Unit Tests: make go_develop_and_test
[ ] LocalNet E2E Tests: make test_e2e
See quickstart guide for instructions

PR Testing (only if making code changes)

[ ] DevNet E2E Tests: Add the devnet-test-e2e label to the PR.
- THIS IS VERY EXPENSIVE, so only do it after all the reviews are complete.
- Optionally run make trigger_ci if you want to re-trigger tests without any code changes
- If tests fail, try re-running failed tests only using the GitHub UI as shown here

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
[x] I create and reference any new tickets, if applicable
[x] I have left TODOs throughout the codebase, if applicable

rBurgett commented 3 weeks ago

@Olshansk I updated my PR moving away from the chain-specific multiplier to a service-based compute units to relay which is combined with the compute units to tokens multiplier when generating rewards. I updated the PR description with my changes. The tests are still all over the place on my computer. I can run the tests individually and they all pass, but when I run them all together with the make file they randomly fail. The e2e tests fail as well, although I am not sure if that is because of my changes or because of the same reasons the other tests randomly fail. I am currently looking at the e2e tests to try and figure out what is going on, but the main functionality has been pushed up and all of the regular tests pass so feel free to look it over and let me know if it is implemented as you expected based on the updated requirements. Thank you!

On a related note, I still cannot get the e2e tests working. I am not sure how to debug it. I thought that I would try running the localnet in order to make sure that works, but that doesn't work either. Following the quickstart guide, I have never been able to get the localnet up since beginning work on this and I don't have the slightest idea how to debug it due to my lack of familiarity with the tools. All attempts so far have been fruitless. The Tilt ui lists everything as "Update pending" except for an error on protocol-dashboards-label that says: The connection to the server 127.0.0.1:37189 was refused - did you specify the right host or port? Build Failed: Command "kubectl label configmap protocol-dashboards grafana_dashboard=1 --overwrite" failed: exit status 1.

Olshansk commented 2 weeks ago

@rBurgett

Did an initial review - PTAL at the comments.
Please merge w/ main and update the branch
General direction LGTM
In the future, please re-request review when you need another look

Re tests: Can you open up a new GitHub issue / bug and provide more details on running E2E and uni tests from main on your machine, we can debug there and cc others.

Olshansk commented 1 week ago

@rBurgett What do you think your ETA on finishing this is?

Trying to understand if it'll make it into Alpha TestNet #2
- Assume (hypothetically) hard cutoff is next Friday to be MERGED in
- This implies code be complete by Monday at the latest (to continue further reviews by the team)
Just looking for a confidence level between 1 - 10 provided:
- Other priorities you have
- The open feedback on this ticket

rBurgett commented 1 week ago

@Olshansk the only two remaining things I know are (unless something got lost in all these GitHub comments):

1) the documentation update 2) the e2e test.

I can update documentation, but until I can run a localnet and e2e tests, I can't add a new e2e test. I'm still trying to figure out what is wrong there, but it's a losing battle so far.

If I could successfully run a localnet and e2e tests, for sure I could have it ready by Monday. Otherwise, I can't say.

Olshansk commented 1 week ago

@rBurgett

I can't add a new e2e test. I'm still trying to figure out what is wrong there, but it's a losing battle so far.

Comments:

See the video I embedded in Quickstart for reference.
I'm not going to be reviewing any PRs for the next 3 weeks, so someone else from the team (cc @bryanchriswhite @red-0ne) will have to help out.
In my comment last week, we talked about creating a new github issue dedicated to this to unblock you. Have you had a chance to create it?

Call to action:

Please confirm you've seen this
👍 ?
Please create a new issue w/ details

rBurgett commented 1 week ago

@Olshansk

I have watched the video, but the validators do not start up like in the video or the text instructions.
Thanks for the heads up.
I'll create that issue now.

Olshansk commented 1 week ago

@okdas Please see @rBurgett's comment here: https://github.com/pokt-network/poktroll/pull/552#issuecomment-2181658737

I won't be around to provide support on this next week.

rBurgett commented 1 week ago

@Olshansk I created an issue. There was all sorts of stuff in the issue template, but it's not my project so I'm not sure what all that was for, I just wanted to put up an issue. I put up what little information I have.

https://github.com/pokt-network/poktroll/issues/630

pokt-network / poktroll