-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(x/gov): grpc query tally for failed proposal #19725
Conversation
WalkthroughWalkthroughThe modification introduces a new status condition, Changes
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (1)
- x/gov/keeper/grpc_query.go (1 hunks)
Additional comments: 1
x/gov/keeper/grpc_query.go (1)
- 322-322: The addition of
proposal.Status == v1.StatusFailed
to the switch case in theTallyResult
function correctly addresses the issue of tallying votes for failed proposals. By fetching the final tally result directly fromproposal.FinalTallyResult
, this change ensures that the tally accurately reflects the final outcome without attempting to recount votes, which aligns with the intended behavior and the PR objectives.
Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yml
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- x/gov/keeper/grpc_query_test.go (2 hunks)
Additional comments: 4
x/gov/keeper/grpc_query_test.go (3)
- 1643-1684: The test case "proposal status failed" in
TestGRPCQueryTallyResult
method correctly sets up a failed proposal and queries its tally result. This test is crucial for ensuring that the tally results of failed proposals are accurately fetched fromFinalTallyResult
. The setup, execution, and assertion are correctly implemented, ensuring the functionality works as expected.- 1826-1858: The test case "proposal status failed" in
TestLegacyGRPCQueryTallyResult
method is well-constructed for verifying the legacy gRPC query functionality for failed proposals. It ensures backward compatibility and correctness of the tally result fetching in the legacy system. The use ofv1beta1
types and the alignment with the expected behavior are correctly handled.- 1823-1861: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1859-1899]
The test case "multiple choice proposal" in
TestProposalVoteOptions
method effectively verifies the retrieval of vote options for a multiple choice proposal. It demonstrates the system's ability to handle proposals with multiple voting options and ensures that the query returns the correct set of options. The setup, query execution, and result validation are properly implemented, showcasing the functionality's correctness.CHANGELOG.md (1)
- 88-88: The changelog entry for the bug fix in the
x/gov
module is clear, concise, and correctly formatted. It accurately describes the change made and provides a link to the PR for more details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!!
(cherry picked from commit d961aef) # Conflicts: # CHANGELOG.md
(cherry picked from commit d961aef) # Conflicts: # CHANGELOG.md
…19727) Co-authored-by: David Tumcharoen <[email protected]> Co-authored-by: Julien Robert <[email protected]>
…19728) Co-authored-by: David Tumcharoen <[email protected]> Co-authored-by: Julien Robert <[email protected]>
…0.50.5 * fix: in-place-testnet edgecases (backport cosmos#19516) (cosmos#19526) Co-authored-by: Adam Tucker <[email protected]> * fix(simapp): typo in GetStoreKeys (cosmos#19544) * build(deps): Bump cosmossdk.io/math from 1.2.0 to 1.3.0 (cosmos#19562) * fix(depinject): Authtx was not accepting custom signers (backport cosmos#19549) (cosmos#19551) Co-authored-by: Devon Bear <[email protected]> Co-authored-by: Qt <[email protected]> Co-authored-by: Julien Robert <[email protected]> * build(deps): Bump github.com/cosmos/cosmos-db from 1.0.0 to 1.0.2 (cosmos#19566) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Julien Robert <[email protected]> * Merge pull request from GHSA-86h5-xcpx-cfqc * fix slashing hole with test * release notes + changelog * word * date * updates --------- Co-authored-by: Julien Robert <[email protected]> * ci: run test pipeline on merge v0.50 branch (cosmos#19582) * fix(staking): fix impossible conditions (cosmos#19621) * docs: add section on creating a testnets from mainnet exports (backport cosmos#19475) (cosmos#19648) Co-authored-by: Marko <[email protected]> * build(deps): Bump cosmossdk.io/x/tx from 0.13.0 to 0.13.1 (cosmos#19665) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * feat(client/v2): marshal enum as string (cosmos#19653) * refactor(x/auth): allow empty public keys for GetSignBytesAdapter (backport cosmos#19651) (cosmos#19675) Co-authored-by: mmsqe <[email protected]> Co-authored-by: Julien Robert <[email protected]> * fix(types): check for HasABCIGenesis in CoreAppModuleBasicAdaptor (cosmos#19709) * build(deps): Bump deps (backport cosmos#19655) (cosmos#19711) Co-authored-by: Julien Robert <[email protected]> * Merge pull request from GHSA-95rx-m9m5-m94v * validate ExtendedCommit against LastCommit test cases * account for core.comet types * logging * linting * cherry-pick staking fix * nits * linting fix * run tests --------- Co-authored-by: Marko <[email protected]> Co-authored-by: Marko Baricevic <[email protected]> * feat(baseapp): add option to disable block gas meter (cosmos#19626) * feat(x/distribution): add rewards-by-validator autocli config (backport cosmos#19707) (cosmos#19714) Co-authored-by: Julien Robert <[email protected]> * fix(x/gov): grpc query tally for failed proposal (backport cosmos#19725) (cosmos#19727) Co-authored-by: David Tumcharoen <[email protected]> Co-authored-by: Julien Robert <[email protected]> * chore: prepare v0.50.5 (cosmos#19715) --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: yihuang <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Devon Bear <[email protected]> Co-authored-by: Qt <[email protected]> Co-authored-by: Julien Robert <[email protected]> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: khanh <[email protected]> Co-authored-by: Tom <[email protected]> Co-authored-by: Marko <[email protected]> Co-authored-by: mmsqe <[email protected]> Co-authored-by: Nikhil Vasan <[email protected]> Co-authored-by: Marko Baricevic <[email protected]> Co-authored-by: David Tumcharoen <[email protected]>
* fix(server): consensus failure while restart node with wrong `chainId` in genesis (cosmos#18920) * test: add NodeURI for clientCtx (backport cosmos#18930) (cosmos#18988) Co-authored-by: mmsqe <[email protected]> Co-authored-by: Julien Robert <[email protected]> * chore: clean-up buf workspace (backport cosmos#18993) (cosmos#18998) * build(deps): Bump cosmossdk.io/log from 1.2.1 to 1.3.0 (cosmos#19024) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * build(deps): Bump cosmossdk.io/errors from 1.0.0 to 1.0.1 (cosmos#19025) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * fix: allow empty public keys when setting signatures (backport cosmos#19106) (cosmos#19108) Co-authored-by: Callum Waters <[email protected]> Co-authored-by: Julien Robert <[email protected]> * chore: prepare v0.47.8 (cosmos#19162) * docs: fix typo in 06-grpc_rest.md (backport cosmos#19192) (cosmos#19194) Co-authored-by: Yoksirod <[email protected]> Co-authored-by: Julien Robert <[email protected]> * fix: skip same-sender non-sequential sequence and then add others txs new solution (backport cosmos#19177) (cosmos#19250) Co-authored-by: Brann Bronzebeard <[email protected]> Co-authored-by: Facundo <[email protected]> Co-authored-by: Facundo Medica <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]> * test(baseapp): Refactor tx selector tests + better comments (backport cosmos#19284) (cosmos#19288) Co-authored-by: Facundo Medica <[email protected]> Co-authored-by: Facundo <[email protected]> * build(deps): Bump cosmossdk.io/log from 1.3.0 to 1.3.1 (cosmos#19359) * chore: prepare v0.47.9 (cosmos#19451) * build(deps): Bump github.com/cosmos/cosmos-proto from 1.0.0-beta.2 to 1.0.0-beta.4 (cosmos#19472) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Julien Robert <[email protected]> * Merge pull request from GHSA-4j93-fm92-rp4m * fix(x/auth/vesting): Add `BlockedAddr` check in `CreatePeriodicVestingAccount` * updates * build(deps): Bump cosmossdk.io/math from 1.2.0 to 1.3.0 (cosmos#19564) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Julien Robert <[email protected]> * fix: use proper `db_backend` type when reading chain-id (cosmos#19573) * Merge pull request from GHSA-86h5-xcpx-cfqc * fix slashing logic * add test * changelog + release notes * word --------- Co-authored-by: Julien Robert <[email protected]> * build(deps): Bump deps (backport cosmos#19655) (cosmos#19712) Co-authored-by: Julien Robert <[email protected]> * fix(x/gov): grpc query tally for failed proposal (backport cosmos#19725) (cosmos#19728) Co-authored-by: David Tumcharoen <[email protected]> Co-authored-by: Julien Robert <[email protected]> * fix(crypto): error if incorrect ledger public key (backport cosmos#19691) (cosmos#19746) Co-authored-by: Rootul P <[email protected]> Co-authored-by: sontrinh16 <[email protected]> * build(deps): Bump github.com/cometbft/cometbft from 0.37.4 to 0.37.5 (cosmos#19752) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Julien Robert <[email protected]> * fix: Implement gogoproto customtype to secp256r1 keys (backport cosmos#20027) (cosmos#20032) Co-authored-by: Facundo Medica <[email protected]> Co-authored-by: marbar3778 <[email protected]> Co-authored-by: Marko <[email protected]> * fix: secp256r1 json missing quotes (backport cosmos#20060) (cosmos#20070) Co-authored-by: Facundo Medica <[email protected]> * build(deps): Bump github.com/cosmos/cosmos-proto from 1.0.0-beta.4 to 1.0.0-beta.5 (cosmos#20094) * chore: prepare v0.47.11 (cosmos#20088) * fix: use timestamp for sim log file name (backport cosmos#20108) (cosmos#20112) Co-authored-by: mmsqe <[email protected]> * fix(x/authz,x/feegrant): check blocked address (backport cosmos#20102) (cosmos#20114) Co-authored-by: Julien Robert <[email protected]> * fix(testsuite/sims): set all signatures (backport cosmos#20151) (cosmos#20186) Co-authored-by: Leon <[email protected]> * build(deps): Bump github.com/cometbft/cometbft from 0.37.5 to 0.37.6 (cosmos#20205) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * go mod tidy * chore: downgrade to go 1.19 (cosmos#20211) * chore: tidy with go 1.19 (cosmos#20220) * chore: revert comet 0.37.6 upgrade due to go version bump (cosmos#20247) * fix: remove txs from mempool when antehandler fails in recheck (backport cosmos#20144) (cosmos#20252) Co-authored-by: Marko <[email protected]> Co-authored-by: marbar3778 <[email protected]> * Revert "chore: downgrade to go 1.19 (cosmos#20211)" This reverts commit aba4e40. * Revert "chore: revert comet 0.37.6 upgrade due to go version bump (cosmos#20247)" This reverts commit 00e4273. * bump go in ci * update docker file --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: atheeshp <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: mmsqe <[email protected]> Co-authored-by: Julien Robert <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Callum Waters <[email protected]> Co-authored-by: Yoksirod <[email protected]> Co-authored-by: Brann Bronzebeard <[email protected]> Co-authored-by: Facundo <[email protected]> Co-authored-by: Facundo Medica <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]> Co-authored-by: Kevin Yang <[email protected]> Co-authored-by: khanh <[email protected]> Co-authored-by: David Tumcharoen <[email protected]> Co-authored-by: Rootul P <[email protected]> Co-authored-by: sontrinh16 <[email protected]> Co-authored-by: marbar3778 <[email protected]> Co-authored-by: Marko <[email protected]> Co-authored-by: mmsqe <[email protected]> Co-authored-by: Leon <[email protected]>
Description
Closes: #XXXX
The query for a failed proposal tally should fetch from proposal.FinalTallyResult rather than tallying from votes again, as they are all removed after the proposal has failed.
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.md
Reviewers 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...
Summary by CodeRabbit
Bug Fixes
x/gov
module related to fetching failed proposal tallies.New Features