[Relayminer] Query for on-chain session param `num_blocks_per_session` #538

bryanchriswhite · 2024-05-17T07:07:03Z

Summary

Refactors the relayminer to query for the current num_blocks_per_session each (or even multiple) time(s) it's used. This is a naive implementation to just get the relayminer using the on-chain param instead of the const that's still defined in the session keeper pkg but which will be removed in forthcoming work.

Next Steps

Implement ModuleParamsClient[P any] (see: notion - ModuleParamsClient).
Refactor sessionQuerier to use ModuleParamsClient[sessiontypes.Params] to avoid excessive querying.
...

Issue

[Protocol] Add a NumBlocksPerSession governance parameter #517

Type of change

Select one or more:

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)

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

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

🚀 This description was created by Ellipsis for commit `bf24e6c`

Summary:

Refactors the relayminer to dynamically query the num_blocks_per_session parameter from the blockchain, updating relevant classes to utilize this new approach.

Key points:

Refactor relayminer to dynamically query num_blocks_per_session.
Update SessionQueryClient interface with new methods for session parameters.
Implement new methods in sessionQuerier.
Utilize new session parameter methods in relayerProxy and related classes.
Prepare for future enhancements in session handling and grace period calculations.

Generated with ❤️ by ellipsis.dev

Co-authored-by: Daniel Olshansky <[email protected]>

github-actions · 2024-05-17T07:21:58Z

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.

…n E2E test

…/refactor/move-num_blocks_per_session * refs/remotes/pokt/main: [On-chain] scaffold: module shared (#545) # Conflicts: # api/poktroll/shared/params.pulsar.go # proto/poktroll/shared/params.proto # x/shared/keeper/msg_update_params.go # x/shared/keeper/msg_update_params_test.go # x/shared/types/errors.go # x/shared/types/genesis.go # x/shared/types/genesis_test.go # x/shared/types/msg_update_params.go # x/shared/types/params.go # x/shared/types/params.pb.go

…ssues/517/refactor/session-params_relayminer

Olshansk

Just a few minor comments but the only blocking ones are in x/shared/session.go

Olshansk · 2024-05-27T22:17:44Z

pkg/client/query/sharedquerier.go

+// GetParams queries & returns the shared module on-chain parameters.
+//
+// TODO_TECHDEBT(#543): We don't really want to have to query the params for every method call.
+// Once `ModuleParamsClient` is implemented, use its replay observable's `#Last` method


When will we have this?

Depends on how we prioritize it. From our discussion about historical params, I got the impression that ModuleParamsClient wouldn't be a high priority for the same reason. It would reduce background network overhead but until we have a reason to believe that that's significantly impacting performance, I don't see another reason that needs to be a high priority.

pkg/client/query/sharedquerier.go

pkg/relayer/session/session.go

x/shared/session.go

…ssues/517/refactor/session-params_relayminer * issues/517/refactor/move-num_blocks_per_session: fix: update_params.feature reset

…sion-params_relayminer * pokt/main: refactor: move `num_blocks_per_session` param to shared module (#546)

red-0ne

Looks good to me! Many parts are waiting for #516 and #517, Eager to see how all of these will fit together.

…sessions * pokt/main: [Relayminer] Query for on-chain session param `num_blocks_per_session` (#538)

bryanchriswhite · 2024-05-29T07:50:40Z

🚨 Do not delete this branch until #555 changes bases! 🚨

…-module * pokt/main: [Relayminer] Query for on-chain session param `num_blocks_per_session` (#538)

…r/ring-client * issues/517/refactor/app-module: [Relayminer] Query for on-chain session param `num_blocks_per_session` (#538) chore: udpate comments refactor: rename methods which use default shared params with suffix `WithDefaultParams` [Relay Mining] Relay Mining math helpers (#549) [Relay Mining] Scaffold the RelayMiningDifficulty type (#548)

…to issues/517/refactor/session-module * pokt/issues/517/refactor/app-module: [Code Health] test: add makefile target for E2E param update tests (#558) [Relayminer] Query for on-chain session param `num_blocks_per_session` (#538) chore: udpate comments refactor: rename methods which use default shared params with suffix `WithDefaultParams` [Relay Mining] Relay Mining math helpers (#549) [Relay Mining] Scaffold the RelayMiningDifficulty type (#548)

…re/cleanup * issues/517/refactor/session-module: chore: review improvements chore: review improvements [Code Health] test: add makefile target for E2E param update tests (#558) [Relayminer] Query for on-chain session param `num_blocks_per_session` (#538) chore: udpate comments refactor: rename methods which use default shared params with suffix `WithDefaultParams` [Relay Mining] Relay Mining math helpers (#549) [Relay Mining] Scaffold the RelayMiningDifficulty type (#548)

…le-param-update-msg * issues/517/chore/cleanup: chore: rename localnet load test makefile target to improve discoverability refactor: relay stress test to use queried shared params chore: review improvements chore: review improvements [Code Health] test: add makefile target for E2E param update tests (#558) [Relayminer] Query for on-chain session param `num_blocks_per_session` (#538) chore: udpate comments refactor: rename methods which use default shared params with suffix `WithDefaultParams` [Relay Mining] Relay Mining math helpers (#549) [Relay Mining] Scaffold the RelayMiningDifficulty type (#548) Empty commit

#538) Co-authored-by: Daniel Olshansky <[email protected]>

bryanchriswhite and others added 12 commits May 15, 2024 14:10

feat: add num_blocks_per_session session module param

a3d3d17

feat: add make target to update all session params

99455d5

chore: use default param values in param update msgs

05e631a

Empty commit

bf3bff3

fixup! feat: add num_blocks_per_session session module param

3aa53ec

test: add coverage for sessiontypes.Params#ValidateNumBlocksPerSession()

d7a04dc

fix: #ValidateNumBlocksPerSession()

56ae9a8

chore: review feedback improvements

b291bc1

Co-authored-by: Daniel Olshansky <[email protected]>

Merge branch 'main' into issues/517/param-num_blocks_per_session

e9afc4c

refactor: rename param name vars

40e1e7f

fix: unit tests

2c33381

Merge branch 'main' into issues/517/param-num_blocks_per_session

9597876

bryanchriswhite added relayminer Changes related to the Relayminer off-chain Off-chain business logic labels May 17, 2024

bryanchriswhite added this to the Shannon Public TestNet milestone May 17, 2024

bryanchriswhite self-assigned this May 17, 2024

bryanchriswhite force-pushed the issues/517/refactor/session-params_relayminer branch from d2e94e5 to a635c4e Compare May 17, 2024 07:21

bryanchriswhite linked an issue May 17, 2024 that may be closed by this pull request

[Protocol] Add a NumBlocksPerSession governance parameter #517

Closed

10 tasks

bryanchriswhite added the devnet-test-e2e label May 17, 2024

github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels May 17, 2024

bryanchriswhite added 2 commits May 21, 2024 08:39

Merge branch 'main' into issues/517/param-num_blocks_per_session

1eae173

Empty commit

86e114d

bryanchriswhite force-pushed the issues/517/refactor/session-params_relayminer branch from f9355f2 to 163f9ba Compare May 21, 2024 11:45

bryanchriswhite added 5 commits May 22, 2024 07:59

Empty commit

eaf5f15

Empty commit

a757d5a

chore: add "unauthorized" account to config.yml instead of creating i…

6c6ec62

…n E2E test

chore: add client.SessionQueryClient#GetParams interface method

638f0c9

chore: add ErrQuerySessionParams

a6667eb

bryanchriswhite added 9 commits May 24, 2024 15:11

Empty commit

03a4079

fix: add omitted pb.go file

89114f3

fix: linting

5db7c9b

fix: go imports script: exclude *.pulsar.go files & fix buffer size

0613764

Merge branch 'issues/517/refactor/move-num_blocks_per_session' into i…

c269dc1

…ssues/517/refactor/session-params_relayminer

Empty commit

77f704e

Merge branch 'issues/517/refactor/move-num_blocks_per_session' into i…

1ccb926

…ssues/517/refactor/session-params_relayminer

chore: review improvements

c18ec7b

Olshansk requested changes May 27, 2024

View reviewed changes

bryanchriswhite added 3 commits May 28, 2024 08:22

fix: update_params.feature reset

203de35

Merge branch 'issues/517/refactor/move-num_blocks_per_session' into i…

7d281a6

…ssues/517/refactor/session-params_relayminer * issues/517/refactor/move-num_blocks_per_session: fix: update_params.feature reset

chore: review improvements

6352a5f

bryanchriswhite requested a review from Olshansk May 28, 2024 06:52

Merge remote-tracking branch 'pokt/main' into issues/517/refactor/ses…

0d55060

…sion-params_relayminer * pokt/main: refactor: move `num_blocks_per_session` param to shared module (#546)

bryanchriswhite changed the base branch from issues/517/refactor/move-num_blocks_per_session to main May 28, 2024 06:54

bryanchriswhite mentioned this pull request May 28, 2024

[Protocol] Add a NumBlocksPerSession governance parameter #517

Closed

10 tasks

red-0ne approved these changes May 28, 2024

View reviewed changes

Olshansk approved these changes May 28, 2024

View reviewed changes

Merge branch 'main' into issues/517/refactor/session-params_relayminer

a0a23b7

bryanchriswhite merged commit 7507dc5 into main May 29, 2024
10 checks passed

bryanchriswhite added a commit that referenced this pull request May 29, 2024

Merge remote-tracking branch 'pokt/main' into fix/relayminer/on-time-…

4e3812d

…sessions * pokt/main: [Relayminer] Query for on-chain session param `num_blocks_per_session` (#538)

bryanchriswhite added a commit that referenced this pull request May 29, 2024

Merge remote-tracking branch 'pokt/main' into issues/517/refactor/app…

8ce12f5

…-module * pokt/main: [Relayminer] Query for on-chain session param `num_blocks_per_session` (#538)

bryanchriswhite deleted the issues/517/refactor/session-params_relayminer branch May 29, 2024 07:59

okdas pushed a commit that referenced this pull request Nov 14, 2024

[Relayminer] Query for on-chain session param num_blocks_per_session (

6235360

#538) Co-authored-by: Daniel Olshansky <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Relayminer] Query for on-chain session param `num_blocks_per_session` #538

[Relayminer] Query for on-chain session param `num_blocks_per_session` #538

bryanchriswhite commented May 17, 2024 •

edited by ellipsis-dev bot

Loading

github-actions bot commented May 17, 2024

Olshansk left a comment

Olshansk May 27, 2024

bryanchriswhite May 28, 2024 •

edited

Loading

red-0ne left a comment

bryanchriswhite commented May 29, 2024 •

edited

Loading

[Relayminer] Query for on-chain session param num_blocks_per_session #538

[Relayminer] Query for on-chain session param num_blocks_per_session #538

Conversation

bryanchriswhite commented May 17, 2024 • edited by ellipsis-dev bot Loading

Summary

Next Steps

Issue

Type of change

Testing

Sanity Checklist

Summary:

github-actions bot commented May 17, 2024

Olshansk left a comment

Choose a reason for hiding this comment

Olshansk May 27, 2024

Choose a reason for hiding this comment

bryanchriswhite May 28, 2024 • edited Loading

Choose a reason for hiding this comment

red-0ne left a comment

Choose a reason for hiding this comment

bryanchriswhite commented May 29, 2024 • edited Loading

[Relayminer] Query for on-chain session param `num_blocks_per_session` #538

[Relayminer] Query for on-chain session param `num_blocks_per_session` #538

bryanchriswhite commented May 17, 2024 •

edited by ellipsis-dev bot

Loading

bryanchriswhite May 28, 2024 •

edited

Loading

bryanchriswhite commented May 29, 2024 •

edited

Loading