refactor(consensus messages): add consensus messages to various modules #19602
Conversation
|
Important Auto Review SkippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
x/staking/keeper/historical_info.go
Outdated
| @@ -2,9 +2,8 @@ package keeper | |||
|
|
|||
| import ( | |||
| "context" | |||
|
|
|||
| consensusv1 "cosmossdk.io/api/cosmos/consensus/v1" | |||
There was a problem hiding this comment.
since the hybrid router and proto v2 is deprecated in server v2 these should be gogo type imports right?
| if evidence == nil { | ||
| continue // skip if no evidence | ||
| } | ||
| e := types.FromABCIEvidence(*evidence, k.stakingKeeper.ConsensusAddressCodec()) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| continue // skip if no evidence | ||
| } | ||
| e := types.FromABCIEvidence(*evidence, k.stakingKeeper.ConsensusAddressCodec()) | ||
| err := k.handleEquivocationEvidence(ctx, e) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
github-actions
bot
removed
C:x/distribution
| "cosmossdk.io/x/evidence/types" | ||
|
|
||
| "github.com/cosmos/cosmos-sdk/telemetry" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| consensusv1 "github.com/cosmos/cosmos-sdk/x/consensus/types" |
There was a problem hiding this comment.
this usage of types seems correct, the gogo types in x/consensus, but slashing and staking are using the API module?
|
after working to get these tests passing it occurred to me that there is a simpler way to resolve this via IoC instead of conditionally deciding where to fetch cometInfo from in the keeper call and adding more boilerplate to every place in the code we need to fetch it. #20238 demonstrates a Service which fetches cometInfo from context for server v1. a different implementation would be used for server v2, probably a (caching) keeper call. I don't think we need to expose this on the grpc router either and wouldn't have to in the design in #20238 |
|
replaced |
Description
Closes: #19599
this pr adds consensus messages to consensus module and makes other modules query it for comet info data
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...