refactor: migrate consensus to collections #15553
Conversation
github-actions
bot
added
C:x/consensus
C:x/evidence
C:x/genutil
x/consensus/keeper/keeper.go
Outdated
| return Keeper{ | ||
| storeService: storeService, | ||
| cdc: cdc, | ||
| authority: authority, | ||
|
|
||
| Params: collections.NewItem(sb, ParamsPrefix, StoreKey, codec.CollValue[cmtproto.ConsensusParams](cdc)), |
There was a problem hiding this comment.
before we were not using a store prefix so we should verify this
There was a problem hiding this comment.
tested on a local testnet it works
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
tac0turtle
force-pushed
the
marko/consensus_collections
branch
from
4c18799
to
05fecfa
Compare
| if cp := app.GetConsensusParams(app.deliverState.ctx); cp != nil { | ||
| res.ConsensusParamUpdates = cp | ||
| } | ||
| cp := app.GetConsensusParams(app.deliverState.ctx) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
| } | ||
|
|
||
| cp, err := app.paramStore.Get(ctx) | ||
| if err != nil { | ||
| panic(err) | ||
| panic(fmt.Errorf("consensus key is nil: %w", err)) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
|
TestDeliverTx (server/mock/app_test.go) is failing because the paramStore is not set |
tac0turtle
force-pushed
the
marko/consensus_collections
branch
from
e343cd7
to
52f8f06
Compare
tac0turtle
force-pushed
the
marko/consensus_collections
branch
from
85eefca
to
8f27bd5
Compare
|
|
||
| // QueryParamsResponse defines the response type for querying x/consensus parameters. | ||
| message QueryParamsResponse { | ||
| message QueryGetParamsResponse { |
There was a problem hiding this comment.
Is that proto breaking warning ok to be ignored? cc @AmauryM
| } | ||
|
|
||
| cp, err := app.paramStore.Get(ctx) | ||
| if err != nil { | ||
| panic(err) | ||
| panic(fmt.Errorf("consensus key is nil: %w", err)) |
There was a problem hiding this comment.
nit: The key isn't nil, the value is empty/nil.
There was a problem hiding this comment.
Thanks. I'll amend in another pr.
This pr migrates consensus module to consensus and puts everything related to the keeper in a single file
Closes: #16275
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...
!to 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...
!in the type prefix if API or client breaking change