Skip to content
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.

Sign up for GitHub

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

refactor(baseapp): set vote info to latest votes #15930

Merged
merged 4 commits into from
Apr 25, 2023
Merged

refactor(baseapp): set vote info to latest votes #15930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
59ab732
set vote info to latest votes
tac0turtle Apr 24, 2023
067d8a6
godoc
tac0turtle Apr 24, 2023
0c58740
add changelog entry
tac0turtle Apr 25, 2023
eef4351
Merge branch 'main' into marko/set_vote_info
tac0turtle Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/feegrant) [#14294](https://github.com/cosmos/cosmos-sdk/pull/14294) Moved the logic of rejecting duplicate grant from `msg_server` to `keeper` method.
* (x/staking) [#14590](https://github.com/cosmos/cosmos-sdk/pull/14590) `MsgUndelegateResponse` now includes undelegated amount. `x/staking` module's `keeper.Undelegate` now returns 3 values (completionTime,undelegateAmount,error) instead of 2.
* (x/staking) (#15731) (https://github.com/cosmos/cosmos-sdk/pull/15731) Introducing a new index to retrieve the delegations by validator efficiently.
* (baseapp) [#15930](https://github.com/cosmos/cosmos-sdk/pull/15930) change vote info provided by prepare and process proposal to the one in the block

### API Breaking Changes

Expand Down Expand Up @@ -173,9 +174,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/slashing) [#15875](https://github.com/cosmos/cosmos-sdk/pull/15875) `x/slashing.NewAppModule` now requires an `InterfaceRegistry` parameter.
* (client) [#15822](https://github.com/cosmos/cosmos-sdk/pull/15822) The return type of the interface method `TxConfig.SignModeHandler` has been changed to `x/tx/signing.HandlerMap`.
* (x/auth) [#15822](https://github.com/cosmos/cosmos-sdk/pull/15822) The type of struct field `ante.HandlerOptions.SignModeHandler` has been changed to `x/tx/signing.HandlerMap`.
* The signature of `NewSigVerificationDecorator` has been changed to accept a `x/tx/signing.HandlerMap`.
* The signature of `VerifySignature` has been changed to accept a `x/tx/signing.HandlerMap` and other structs from `x/tx` as arguments.
* The signature of `NewTxConfigWithTextual` has been deprecated and its signature changed to accept a `SignModeOptions`.
* The signature of `NewSigVerificationDecorator` has been changed to accept a `x/tx/signing.HandlerMap`.
* The signature of `VerifySignature` has been changed to accept a `x/tx/signing.HandlerMap` and other structs from `x/tx` as arguments.
* The signature of `NewTxConfigWithTextual` has been deprecated and its signature changed to accept a `SignModeOptions`.

### Client Breaking Changes

Expand Down
20 changes: 18 additions & 2 deletions baseapp/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.
}

app.prepareProposalState.ctx = app.getContextForProposal(app.prepareProposalState.ctx, req.Height).
WithVoteInfos(app.voteInfos).
WithVoteInfos(toVoteInfo(req.LocalLastCommit.Votes)). // this is a set of votes that are not finalized yet, wait for commit
WithBlockHeight(req.Height).
WithBlockTime(req.Time).
WithProposer(req.ProposerAddress)
Expand Down Expand Up @@ -341,7 +341,7 @@ func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) (resp abci.
app.setState(runTxProcessProposal, emptyHeader)

app.processProposalState.ctx = app.getContextForProposal(app.processProposalState.ctx, req.Height).
WithVoteInfos(app.voteInfos).
WithVoteInfos(req.ProposedLastCommit.Votes). // this is a set of votes that are not finalized yet, wait for commit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just straight up remove VoteInfo from context?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but that's orthogonal, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. This is backportable

Copy link
Member Author

@tac0turtle tac0turtle Apr 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may not be a usecase for this today but for things like optimistic execution it will be needed. Might as well leave it

EDIT: it could be removed but this would be a larger breaking chnage. I would be up for removing it in cometinfo pr though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im worried about backporting this if anyone is using votes, it could cause unforseen changes. Lets leave it for 0.50. Ill add a changelog then merge

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would it be used for optimistic execution?

WithBlockHeight(req.Height).
WithBlockTime(req.Time).
WithHeaderHash(req.Hash).
Expand Down Expand Up @@ -1036,3 +1036,19 @@ func (app *BaseApp) getContextForProposal(ctx sdk.Context, height int64) sdk.Con

return ctx
}

// toVoteInfo converts the new ExtendedVoteInfo to VoteInfo.
func toVoteInfo(votes []abci.ExtendedVoteInfo) []abci.VoteInfo {
legacyVotes := make([]abci.VoteInfo, len(votes))
for i, vote := range votes {
legacyVotes[i] = abci.VoteInfo{
Validator: abci.Validator{
Address: vote.Validator.Address,
Power: vote.Validator.Power,
},
SignedLastBlock: vote.SignedLastBlock,
}
}

return legacyVotes
}