Skip to content

Commit

Permalink
refactor: deprecate Voteinfo in favour of Cometinfo on Context (#17670)
Browse files Browse the repository at this point in the history
  • Loading branch information
tac0turtle authored Sep 14, 2023
1 parent e90e5e1 commit 36a9330
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 61 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (rpc) [#17470](https://github.com/cosmos/cosmos-sdk/pull/17470) Avoid open 0.0.0.0 to public by default and add `listen-ip-address` argument for `testnet init-files` cmd.
* (types/module) [#17554](https://github.com/cosmos/cosmos-sdk/pull/17554) Introduce `HasABCIGenesis` which is implemented by a module only when a validatorset update needs to be returned
* (baseapp) [#17667](https://github.com/cosmos/cosmos-sdk/pull/17667) Close databases opened by cosmos-sdk during BaseApp shutdown
* (types) [#17670](https://github.com/cosmos/cosmos-sdk/pull/17670) Use `ctx.CometInfo` in place of `ctx.VoteInfos`
* [#17733](https://github.com/cosmos/cosmos-sdk/pull/17733) Ensure `buf export` exports all proto dependencies

### Bug Fixes
Expand Down Expand Up @@ -143,6 +144,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (types) [#17426](https://github.com/cosmos/cosmos-sdk/pull/17426) `NewContext` does not take a `cmtproto.Header{}` any longer.
* `WithChainID` / `WithBlockHeight` / `WithBlockHeader` must be used to set values on the context
* (x/bank) [#17569](https://github.com/cosmos/cosmos-sdk/pull/17569) `BurnCoins` takes an address instead of a module name
* (x/distribution) [#17670](https://github.com/cosmos/cosmos-sdk/pull/17670) `AllocateTokens` takes `comet.VoteInfos` instead of `[]abci.VoteInfo`

### CLI Breaking Changes

Expand Down
23 changes: 13 additions & 10 deletions tests/integration/distribution/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@ import (
"fmt"
"testing"

cmtabcitypes "github.com/cometbft/cometbft/abci/types"
"github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/stretchr/testify/require"
"gotest.tools/v3/assert"

"cosmossdk.io/collections"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/core/comet"
"cosmossdk.io/log"
"cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
Expand Down Expand Up @@ -116,13 +115,17 @@ func initFixture(tb testing.TB) *fixture {
valConsAddr := sdk.ConsAddress(valConsPk0.Address())

// set proposer and vote infos
ctx := newCtx.WithProposer(valConsAddr).WithVoteInfos([]cmtabcitypes.VoteInfo{
{
Validator: cmtabcitypes.Validator{
Address: valAddr,
Power: 100,
ctx := newCtx.WithProposer(valConsAddr).WithCometInfo(comet.Info{
LastCommit: comet.CommitInfo{
Votes: []comet.VoteInfo{
{
Validator: comet.Validator{
Address: valAddr,
Power: 100,
},
BlockIDFlag: comet.BlockIDFlagCommit,
},
},
BlockIdFlag: types.BlockIDFlagCommit,
},
})

Expand Down Expand Up @@ -322,8 +325,8 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) {
assert.Assert(t, prevProposerConsAddr.Empty() == false)
assert.DeepEqual(t, prevProposerConsAddr, valConsAddr)
var previousTotalPower int64
for _, voteInfo := range f.sdkCtx.VoteInfos() {
previousTotalPower += voteInfo.Validator.Power
for _, vote := range f.sdkCtx.CometInfo().LastCommit.Votes {
previousTotalPower += vote.Validator.Power
}
assert.Equal(t, previousTotalPower, int64(100))
})
Expand Down
2 changes: 1 addition & 1 deletion testutil/integration/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func (app *App) RunMsg(msg sdk.Msg, option ...Option) (*codectypes.Any, error) {

if cfg.AutomaticFinalizeBlock {
height := app.LastBlockHeight() + 1
if _, err := app.FinalizeBlock(&cmtabcitypes.RequestFinalizeBlock{Height: height}); err != nil {
if _, err := app.FinalizeBlock(&cmtabcitypes.RequestFinalizeBlock{Height: height, DecidedLastCommit: cmtabcitypes.CommitInfo{Votes: []cmtabcitypes.VoteInfo{{}}}}); err != nil {
return nil, fmt.Errorf("failed to run finalize block: %w", err)
}
}
Expand Down
32 changes: 16 additions & 16 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,14 @@ but please do not over-use it. We try to keep all data structured
and standard additions here would be better just to add to the Context struct
*/
type Context struct {
baseCtx context.Context
ms storetypes.MultiStore
// Deprecated: Use HeaderService for height, time, and chainID and CometService for the rest
header cmtproto.Header
// Deprecated: Use HeaderService for hash
headerHash []byte
// Deprecated: Use HeaderService for chainID and CometService for the rest
chainID string
baseCtx context.Context
ms storetypes.MultiStore
header cmtproto.Header // Deprecated: Use HeaderService for height, time, and chainID and CometService for the rest
headerHash []byte // Deprecated: Use HeaderService for hash
chainID string // Deprecated: Use HeaderService for chainID and CometService for the rest
txBytes []byte
logger log.Logger
voteInfo []abci.VoteInfo
voteInfo []abci.VoteInfo // Deprecated: use Cometinfo.GetLastCommit().Votes() instead, will be removed in 0.51
gasMeter storetypes.GasMeter
blockGasMeter storetypes.GasMeter
checkTx bool
Expand All @@ -69,13 +66,15 @@ type Context struct {
type Request = Context

// Read-only accessors
func (c Context) Context() context.Context { return c.baseCtx }
func (c Context) MultiStore() storetypes.MultiStore { return c.ms }
func (c Context) BlockHeight() int64 { return c.header.Height }
func (c Context) BlockTime() time.Time { return c.header.Time }
func (c Context) ChainID() string { return c.chainID }
func (c Context) TxBytes() []byte { return c.txBytes }
func (c Context) Logger() log.Logger { return c.logger }
func (c Context) Context() context.Context { return c.baseCtx }
func (c Context) MultiStore() storetypes.MultiStore { return c.ms }
func (c Context) BlockHeight() int64 { return c.header.Height }
func (c Context) BlockTime() time.Time { return c.header.Time }
func (c Context) ChainID() string { return c.chainID }
func (c Context) TxBytes() []byte { return c.txBytes }
func (c Context) Logger() log.Logger { return c.logger }

// Deprecated: use Cometinfo.GetLastCommit().Votes() instead, will be removed after 0.51
func (c Context) VoteInfos() []abci.VoteInfo { return c.voteInfo }
func (c Context) GasMeter() storetypes.GasMeter { return c.gasMeter }
func (c Context) BlockGasMeter() storetypes.GasMeter { return c.blockGasMeter }
Expand Down Expand Up @@ -216,6 +215,7 @@ func (c Context) WithLogger(logger log.Logger) Context {
}

// WithVoteInfos returns a Context with an updated consensus VoteInfo.
// Deprecated: use WithCometinfo() instead, will be removed after 0.51
func (c Context) WithVoteInfos(voteInfo []abci.VoteInfo) Context {
c.voteInfo = voteInfo
return c
Expand Down
6 changes: 3 additions & 3 deletions x/distribution/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ func BeginBlocker(ctx sdk.Context, k keeper.Keeper) error {

// determine the total power signing the block
var previousTotalPower int64
for _, voteInfo := range ctx.VoteInfos() {
previousTotalPower += voteInfo.Validator.Power
for _, vote := range ctx.CometInfo().LastCommit.Votes {
previousTotalPower += vote.Validator.Power
}

// TODO this is Tendermint-dependent
// ref https://github.com/cosmos/cosmos-sdk/issues/3095
if ctx.BlockHeight() > 1 {
if err := k.AllocateTokens(ctx, previousTotalPower, ctx.VoteInfos()); err != nil {
if err := k.AllocateTokens(ctx, previousTotalPower, ctx.CometInfo().LastCommit.Votes); err != nil {
return err
}
}
Expand Down
6 changes: 3 additions & 3 deletions x/distribution/keeper/allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ import (
"context"
"errors"

abci "github.com/cometbft/cometbft/abci/types"

"cosmossdk.io/collections"
"cosmossdk.io/core/comet"
"cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -16,7 +15,7 @@ import (

// AllocateTokens performs reward and fee distribution to all validators based
// on the F1 fee distribution specification.
func (k Keeper) AllocateTokens(ctx context.Context, totalPreviousPower int64, bondedVotes []abci.VoteInfo) error {
func (k Keeper) AllocateTokens(ctx context.Context, totalPreviousPower int64, bondedVotes []comet.VoteInfo) error {
// fetch and clear the collected fees for distribution, since this is
// called in BeginBlock, collected fees will be from the previous block
// (and distributed to the previous proposer)
Expand Down Expand Up @@ -58,6 +57,7 @@ func (k Keeper) AllocateTokens(ctx context.Context, totalPreviousPower int64, bo
//
// Ref: https://github.com/cosmos/cosmos-sdk/pull/3099#discussion_r246276376
for _, vote := range bondedVotes {

validator, err := k.stakingKeeper.ValidatorByConsAddr(ctx, vote.Validator.Address)
if err != nil {
return err
Expand Down
18 changes: 10 additions & 8 deletions x/distribution/keeper/allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import (
"testing"
"time"

abci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"

"cosmossdk.io/collections"
"cosmossdk.io/core/comet"
"cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"

Expand Down Expand Up @@ -128,11 +128,11 @@ func TestAllocateTokensToManyValidators(t *testing.T) {
val1.Commission = stakingtypes.NewCommission(math.LegacyNewDec(0), math.LegacyNewDec(0), math.LegacyNewDec(0))
stakingKeeper.EXPECT().ValidatorByConsAddr(gomock.Any(), sdk.GetConsAddress(valConsPk1)).Return(val1, nil).AnyTimes()

abciValA := abci.Validator{
abciValA := comet.Validator{
Address: valConsPk0.Address(),
Power: 100,
}
abciValB := abci.Validator{
abciValB := comet.Validator{
Address: valConsPk1.Address(),
Power: 100,
}
Expand Down Expand Up @@ -165,14 +165,15 @@ func TestAllocateTokensToManyValidators(t *testing.T) {
bankKeeper.EXPECT().GetAllBalances(gomock.Any(), feeCollectorAcc.GetAddress()).Return(fees)
bankKeeper.EXPECT().SendCoinsFromModuleToModule(gomock.Any(), "fee_collector", disttypes.ModuleName, fees)

votes := []abci.VoteInfo{
votes := []comet.VoteInfo{
{
Validator: abciValA,
},
{
Validator: abciValB,
},
}

require.NoError(t, distrKeeper.AllocateTokens(ctx, 200, votes))

// 98 outstanding rewards (100 less 2 to community pool)
Expand Down Expand Up @@ -262,15 +263,15 @@ func TestAllocateTokensTruncation(t *testing.T) {
val2.Commission = stakingtypes.NewCommission(math.LegacyNewDecWithPrec(1, 1), math.LegacyNewDecWithPrec(1, 1), math.LegacyNewDec(0))
stakingKeeper.EXPECT().ValidatorByConsAddr(gomock.Any(), sdk.GetConsAddress(valConsPk2)).Return(val2, nil).AnyTimes()

abciValA := abci.Validator{
abciValA := comet.Validator{
Address: valConsPk0.Address(),
Power: 11,
}
abciValB := abci.Validator{
abciValB := comet.Validator{
Address: valConsPk1.Address(),
Power: 10,
}
abciValC := abci.Validator{
abciValC := comet.Validator{
Address: valConsPk2.Address(),
Power: 10,
}
Expand Down Expand Up @@ -303,7 +304,7 @@ func TestAllocateTokensTruncation(t *testing.T) {
bankKeeper.EXPECT().GetAllBalances(gomock.Any(), feeCollectorAcc.GetAddress()).Return(fees)
bankKeeper.EXPECT().SendCoinsFromModuleToModule(gomock.Any(), "fee_collector", disttypes.ModuleName, fees)

votes := []abci.VoteInfo{
votes := []comet.VoteInfo{
{
Validator: abciValA,
},
Expand All @@ -314,6 +315,7 @@ func TestAllocateTokensTruncation(t *testing.T) {
Validator: abciValC,
},
}

require.NoError(t, distrKeeper.AllocateTokens(ctx, 31, votes))

val0OutstandingRewards, err := distrKeeper.ValidatorOutstandingRewards.Get(ctx, valAddr0)
Expand Down
7 changes: 3 additions & 4 deletions x/slashing/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"context"
"time"

"cosmossdk.io/core/comet"

"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/slashing/keeper"
Expand All @@ -21,8 +19,9 @@ func BeginBlocker(ctx context.Context, k keeper.Keeper) error {
// store whether or not they have actually signed it and slash/unbond any
// which have missed too many blocks in a row (downtime slashing)
sdkCtx := sdk.UnwrapSDKContext(ctx)
for _, voteInfo := range sdkCtx.VoteInfos() {
err := k.HandleValidatorSignature(ctx, voteInfo.Validator.Address, voteInfo.Validator.Power, comet.BlockIDFlag(voteInfo.BlockIdFlag))
for _, vote := range sdkCtx.CometInfo().LastCommit.Votes {

err := k.HandleValidatorSignature(ctx, vote.Validator.Address, vote.Validator.Power, vote.BlockIDFlag)
if err != nil {
return err
}
Expand Down
30 changes: 14 additions & 16 deletions x/slashing/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ import (
"testing"
"time"

abci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
"github.com/stretchr/testify/require"

"cosmossdk.io/core/comet"
"cosmossdk.io/depinject"
"cosmossdk.io/log"

Expand Down Expand Up @@ -65,15 +64,17 @@ func TestBeginBlocker(t *testing.T) {
require.NoError(t, err)
require.Equal(t, amt, val.GetBondedTokens())

abciVal := abci.Validator{
abciVal := comet.Validator{
Address: pk.Address(),
Power: power,
}

ctx = ctx.WithVoteInfos([]abci.VoteInfo{{
Validator: abciVal,
BlockIdFlag: cmtproto.BlockIDFlagCommit,
}})
ctx = ctx.WithCometInfo(comet.Info{
LastCommit: comet.CommitInfo{Votes: []comet.VoteInfo{{
Validator: abciVal,
BlockIDFlag: comet.BlockIDFlagCommit,
}}},
})

err = slashing.BeginBlocker(ctx, slashingKeeper)
require.NoError(t, err)
Expand All @@ -91,11 +92,7 @@ func TestBeginBlocker(t *testing.T) {
require.NoError(t, err)
// for 100 blocks, mark the validator as having signed
for ; height < signedBlocksWindow; height++ {
ctx = ctx.WithBlockHeight(height).
WithVoteInfos([]abci.VoteInfo{{
Validator: abciVal,
BlockIdFlag: cmtproto.BlockIDFlagCommit,
}})
ctx = ctx.WithBlockHeight(height)

err = slashing.BeginBlocker(ctx, slashingKeeper)
require.NoError(t, err)
Expand All @@ -105,11 +102,12 @@ func TestBeginBlocker(t *testing.T) {
require.NoError(t, err)
// for 50 blocks, mark the validator as having not signed
for ; height < ((signedBlocksWindow * 2) - minSignedPerWindow + 1); height++ {
ctx = ctx.WithBlockHeight(height).
WithVoteInfos([]abci.VoteInfo{{
ctx = ctx.WithBlockHeight(height).WithCometInfo(comet.Info{
LastCommit: comet.CommitInfo{Votes: []comet.VoteInfo{{
Validator: abciVal,
BlockIdFlag: cmtproto.BlockIDFlagAbsent,
}})
BlockIDFlag: comet.BlockIDFlagAbsent,
}}},
})

err = slashing.BeginBlocker(ctx, slashingKeeper)
require.NoError(t, err)
Expand Down

0 comments on commit 36a9330

Please sign in to comment.