[Relayminer] refactor: query min_relay_difficulty_bits param
#655
Conversation
bryanchriswhite
added
relayminer
WalkthroughThe update integrates a new querying interface for on-chain proof module parameters, adjusts the mining logic to use this new interface, and updates dependency injections accordingly. The changes ensure relay difficulty bits are fetched and compared as Changes
Sequence Diagram(s)sequenceDiagram
participant Application
participant ProofQueryClient
participant Blockchain
Application->>ProofQueryClient: GetParams(ctx)
ProofQueryClient->>Blockchain: Query proof module params
Blockchain-->>ProofQueryClient: Return params
ProofQueryClient-->>Application: Return proof module params
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
The image is going to be pushed after the next commit. You can use If you also want to run E2E tests, please add |
|
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 GCP workloads (requires changing the namespace to 655) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- pkg/client/interface.go (3 hunks)
- pkg/client/query/proofquerier.go (1 hunks)
- pkg/deps/config/suppliers.go (1 hunks)
- pkg/relayer/cmd/cmd.go (2 hunks)
- pkg/relayer/miner/miner.go (4 hunks)
- pkg/relayer/miner/miner_test.go (2 hunks)
- pkg/relayer/miner/options.go (1 hunks)
- testutil/testclient/testqueryclients/proofquerier.go (1 hunks)
- x/proof/types/params.go (1 hunks)
Additional comments not posted (15)
pkg/relayer/miner/options.go (1)
7-7: Function parameter type change: Ensure consistency.The parameter type for
WithDifficultyhas been changed frominttouint64. Ensure that all calls toWithDifficultyand related functions handleuint64correctly.Verification successful
Function parameter type change: Verification successful.
The parameter type for
WithDifficultyhas been changed touint64, and all calls to this function handle theuint64type correctly.
testDifficultyis defined asuint64inpkg/relayer/miner/miner_test.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `WithDifficulty` handle `uint64` correctly. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type go $'WithDifficulty'Length of output: 386
Script:
#!/bin/bash # Search for the definition and type of `testDifficulty` in `pkg/relayer/miner/miner_test.go` rg 'testDifficulty' pkg/relayer/miner/miner_test.goLength of output: 230
testutil/testclient/testqueryclients/proofquerier.go (1)
1-24: Ensure mock behavior correctness.The function
NewTestProofQueryClientcreates a mockProofQueryClientwith default proof module parameters. Ensure that the mock behavior is consistent with the expected usage in tests.Verification successful
Mock behavior is consistent with test usage.
The
NewTestProofQueryClientfunction creates a mockProofQueryClientwith default proof module parameters, and its usage in theTestMiner_MinedRelaystest is consistent with this behavior.
- The
GetParamsmethod of the mock client is set up to return default proof parameters.- The test involves creating a
Minerinstance with the mock client, which likely uses theGetParamsmethod internally.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `NewTestProofQueryClient` in tests. # Test: Search for the function usage. Expect: Consistent mock behavior. rg --type go $'NewTestProofQueryClient'Length of output: 404
Script:
#!/bin/bash # Extract relevant portions of the file where NewTestProofQueryClient is used. rg -A 10 -B 10 'NewTestProofQueryClient' pkg/relayer/miner/miner_test.goLength of output: 973
Script:
#!/bin/bash # Extract more lines from the file to see the complete usage and expectations around proofQueryClientMock. rg -A 50 -B 10 'NewTestProofQueryClient' pkg/relayer/miner/miner_test.goLength of output: 2299
pkg/client/query/proofquerier.go (1)
1-50: Ensure correct dependency injection and error handling.The
NewProofQuerierfunction injects dependencies usingdepinject.Configand initializes theproofQuerierstruct. Ensure that all required dependencies are correctly injected and error handling is robust.Verification successful
Dependency injection and error handling are correctly implemented.
The
NewProofQuerierfunction is used inNewSupplyProofQueryClientFnwithinpkg/deps/config/suppliers.go. The dependency injection and error handling are correctly managed, ensuring robust functionality.
pkg/deps/config/suppliers.go: The functionNewSupplyProofQueryClientFncorrectly handles errors and injects dependencies.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `NewProofQuerier` and ensure correct dependency injection. # Test: Search for the function usage. Expect: Correct dependency injection and error handling. rg --type go $'NewProofQuerier'Length of output: 329
Script:
#!/bin/bash # Extract the context around the usage of NewProofQuerier in suppliers.go to inspect dependency injection and error handling. rg --type go -A 10 -B 10 'NewProofQuerier' pkg/deps/config/suppliers.goLength of output: 721
pkg/relayer/miner/miner_test.go (1)
15-21: Ensure correct dependency injection in tests.The
TestMiner_MinedRelaysfunction now includes dependency injection for a proof query client. Ensure that the dependency injection is correctly implemented and consistent with the rest of the codebase.Verification successful
Ensure correct dependency injection in tests.
The
TestMiner_MinedRelaysfunction inpkg/relayer/miner/miner_test.gocorrectly usesdepinject.Supplyfor dependency injection, ensuring consistency with the rest of the codebase.
pkg/relayer/miner/miner_test.go: Verified the usage ofdepinject.Supplyfor dependency injection.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of dependency injection in tests. # Test: Search for the usage of `depinject.Supply` and ensure correctness. rg --type go $'depinject.Supply'Length of output: 5627
pkg/relayer/miner/miner.go (5)
6-8: Imports look good.The new imports are necessary for dependency injection and the new proof query client.
34-35: New fieldproofQueryClientlooks good.The new field is necessary to incorporate the proof query client into the
minerstruct.
51-66: Changes inNewMinerfunction look good.The changes are necessary to inject the new proof query client into the
minerstruct.
96-106: Changes insetDefaultsfunction look good.The changes are necessary to fetch the default relay difficulty bits from the proof query client.
128-128: Changes inmapMineRelayfunction look good.The changes are necessary to handle the new
uint64type for relay difficulty bits.x/proof/types/params.go (1)
9-13: New import and interface implementation declaration look good.The new import and declaration are necessary to implement the
ProofParamsinterface.pkg/relayer/cmd/cmd.go (2)
198-198: New function call toconfig.NewSupplyProofQueryClientFn()looks good.The new function call is necessary to supply the proof query client.
221-221: Changes insupplyMinerfunction look good.The changes are necessary to create a new miner with the supplied dependencies.
pkg/deps/config/suppliers.go (1)
449-465: New functionNewSupplyProofQueryClientFnlooks good.The new function is necessary to create and supply a proof query client.
pkg/client/interface.go (2)
318-326: LGTM!The
ProofParamsinterface is well-defined and consistent with the existing codebase.
328-333: LGTM!The
ProofQueryClientinterface is consistent with the existing codebase and provides necessary functionality for querying proof module parameters.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/relayer/miner/miner.go (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- pkg/relayer/miner/miner.go
bryanchriswhite
changed the title
min_relay_difficulty_bits param
bryanchriswhite
changed the title
min_relay_difficulty_bits parammin_relay_difficulty_bits param
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/relayer/miner/miner.go (4 hunks)
Additional comments not posted (4)
pkg/relayer/miner/miner.go (4)
25-30: LGTM! New fields added tominerstruct.The fields
proofQueryClientandrelayDifficultyBitsare correctly added with appropriate comments.
42-57: LGTM! Dependency injection and defaults setting inNewMinerfunction.The
proofQueryClientdependency is correctly injected, and thesetDefaultsmethod is called to ensure proper configuration.
87-97: LGTM! Fetching parameters and setting defaults insetDefaultsmethod.The
setDefaultsmethod correctly fetches parameters fromproofQueryClientand setsrelayDifficultyBitsif it is not already set.
119-119: LGTM! Updated difficulty comparison inmapMineRelaymethod.The comparison logic is correctly updated to handle
uint64values.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/relayer/cmd/cmd.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- pkg/relayer/cmd/cmd.go
|
|
||
| // ProofParams is a go interface type which corresponds to the poktroll.proof.Params | ||
| // protobuf message. Since the generated go types don't include interface types, this | ||
| // is necessary to prevent dependency cycles. |
There was a problem hiding this comment.
#PUC what the dependency cycle is.
There was a problem hiding this comment.
I think this will need to become its own issue where we evaluate potential solutions. There are potential dependency cycles looming in pkg/client due to the importing of concrete module types. In this case, we were trying to import x/proof/types for the Params type but that package imports pkg/client for some querier interface types.
…elayer-session-manager * issues/553/fix/replay-observable: chore: go imports chore: update comment chore: update comment chore: review feedback improvements [Relayminer] refactor: query `min_relay_difficulty_bits` param (#655) [Docs] Claim expiration (#649) [LoadTest] Passing non-existing plans variable (#661) Tiny: updating labels in tiltfile [Load Testing] fix: relay stress test duration calculation (#651) [LocalNet] Grafana stress test dashboard changes (#641)
Summary
Refactors the relayminer to query for the
min_relay_difficulty_bitsfrom on-chain.Issue
Type of change
Select one or more:
Testing
Documentation changes (only if making doc changes)
make docusaurus_start; only needed if you make doc changesLocal Testing (only if making code changes)
make go_develop_and_testmake test_e2ePR Testing (only if making code changes)
devnet-test-e2elabel to the PR.make trigger_ciif you want to re-trigger tests without any code changesSanity Checklist
Summary by CodeRabbit
New Features
Improvements
minerstruct to dynamically fetch and set relay difficulty bits from on-chain parameters.NewMinerfunction to include dependency injection for greater flexibility.Bug Fixes
Tests