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.

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: deprecate Voteinfo in favour of Cometinfo on Context #17670

Merged
merged 19 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,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
5 changes: 3 additions & 2 deletions tests/integration/distribution/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,9 @@ 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 i := 0; i < f.sdkCtx.CometInfo().GetLastCommit().Votes().Len(); i++ {
vote := f.sdkCtx.CometInfo().GetLastCommit().Votes().Get(i)
previousTotalPower += vote.Validator().Power()
}
assert.Equal(t, previousTotalPower, int64(100))
})
Expand Down
152 changes: 136 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
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
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 Cometinfo.GetLastCommit().Votes() instead, will be removed after 0.51
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
func (c Context) WithVoteInfos(voteInfo []abci.VoteInfo) Context {
c.voteInfo = voteInfo
return c
Expand Down Expand Up @@ -393,3 +393,123 @@ func UnwrapSDKContext(ctx context.Context) Context {
}
return ctx.Value(SdkContextKey).(Context)
}

// CometInfo defines the properties provided by comet to the application
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
type CometInfo struct {
Misbehavior []abci.Misbehavior
ValidatorsHash []byte
ProposerAddress []byte
LastCommit abci.CommitInfo
}

func (r CometInfo) GetEvidence() comet.EvidenceList {
return evidenceWrapper{evidence: r.Misbehavior}
}

func (r CometInfo) GetValidatorsHash() []byte {
return r.ValidatorsHash
}

func (r CometInfo) GetProposerAddress() []byte {
return r.ProposerAddress
}

func (r CometInfo) GetLastCommit() comet.CommitInfo {
return commitInfoWrapper{r.LastCommit}
}

type evidenceWrapper struct {
evidence []abci.Misbehavior
}

func (e evidenceWrapper) Len() int {
return len(e.evidence)
}

func (e evidenceWrapper) Get(i int) comet.Evidence {
return misbehaviorWrapper{e.evidence[i]}
}

// commitInfoWrapper is a wrapper around abci.CommitInfo that implements CommitInfo interface
type commitInfoWrapper struct {
abci.CommitInfo
}

var _ comet.CommitInfo = (*commitInfoWrapper)(nil)

func (c commitInfoWrapper) Round() int32 {
return c.CommitInfo.Round
}

func (c commitInfoWrapper) Votes() comet.VoteInfos {
return AbciVoteInfoWrapper{c.CommitInfo.Votes}
}

// AbciVoteInfoWrapper is a wrapper around abci.VoteInfo that implements VoteInfos interface
type AbciVoteInfoWrapper struct {
Votes []abci.VoteInfo
}

var _ comet.VoteInfos = (*AbciVoteInfoWrapper)(nil)

func (e AbciVoteInfoWrapper) Len() int {
return len(e.Votes)
}

func (e AbciVoteInfoWrapper) Get(i int) comet.VoteInfo {
return VoteInfoWrapper{e.Votes[i]}
}

// VoteInfoWrapper is a wrapper around abci.VoteInfo that implements VoteInfo interface
type VoteInfoWrapper struct {
abci.VoteInfo
}

var _ comet.VoteInfo = (*VoteInfoWrapper)(nil)

func (v VoteInfoWrapper) GetBlockIDFlag() comet.BlockIDFlag {
return comet.BlockIDFlag(v.VoteInfo.BlockIdFlag)
}

func (v VoteInfoWrapper) Validator() comet.Validator {
return ValidatorWrapper{v.VoteInfo.Validator}
}

// ValidatorWrapper is a wrapper around abci.Validator that implements Validator interface
type ValidatorWrapper struct {
abci.Validator
}

var _ comet.Validator = (*ValidatorWrapper)(nil)

func (v ValidatorWrapper) Address() []byte {
return v.Validator.Address
}

func (v ValidatorWrapper) Power() int64 {
return v.Validator.Power
}

type misbehaviorWrapper struct {
abci.Misbehavior
}

func (m misbehaviorWrapper) Type() comet.MisbehaviorType {
return comet.MisbehaviorType(m.Misbehavior.Type)
}

func (m misbehaviorWrapper) Height() int64 {
return m.Misbehavior.Height
}

func (m misbehaviorWrapper) Validator() comet.Validator {
return ValidatorWrapper{m.Misbehavior.Validator}
}

func (m misbehaviorWrapper) Time() time.Time {
return m.Misbehavior.Time
}

func (m misbehaviorWrapper) TotalVotingPower() int64 {
return m.Misbehavior.TotalVotingPower
}
7 changes: 4 additions & 3 deletions x/distribution/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ 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 i := 0; i < ctx.CometInfo().GetLastCommit().Votes().Len(); i++ {
vote := ctx.CometInfo().GetLastCommit().Votes().Get(i)
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().GetLastCommit().Votes()); err != nil {
Fixed Show fixed Hide fixed
return err
}
}
Expand Down
12 changes: 6 additions & 6 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.VoteInfos) 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 @@ -57,16 +56,17 @@ func (k Keeper) AllocateTokens(ctx context.Context, totalPreviousPower int64, bo
// TODO: Consider parallelizing later
//
// Ref: https://github.com/cosmos/cosmos-sdk/pull/3099#discussion_r246276376
for _, vote := range bondedVotes {
validator, err := k.stakingKeeper.ValidatorByConsAddr(ctx, vote.Validator.Address)
for i := 0; i < bondedVotes.Len(); i++ {
vote := bondedVotes.Get(i)
validator, err := k.stakingKeeper.ValidatorByConsAddr(ctx, vote.Validator().Address())
if err != nil {
return err
}

// TODO: Consider micro-slashing for missing votes.
//
// Ref: https://github.com/cosmos/cosmos-sdk/issues/2525#issuecomment-430838701
powerFraction := math.LegacyNewDec(vote.Validator.Power).QuoTruncate(math.LegacyNewDec(totalPreviousPower))
powerFraction := math.LegacyNewDec(vote.Validator().Power()).QuoTruncate(math.LegacyNewDec(totalPreviousPower))
reward := feeMultiplier.MulDecTruncate(powerFraction)

err = k.AllocateTokensToValidator(ctx, validator, reward)
Expand Down
10 changes: 6 additions & 4 deletions x/distribution/keeper/allocation_test.go
Original file line number Diff line number Diff line change
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 := sdk.AbciVoteInfoWrapper{Votes: []abci.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 @@ -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 := sdk.AbciVoteInfoWrapper{Votes: []abci.VoteInfo{
{
Validator: abciValA,
},
Expand All @@ -313,7 +314,8 @@ 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 i := 0; i < sdkCtx.CometInfo().GetLastCommit().Votes().Len(); i++ {
vote := sdkCtx.CometInfo().GetLastCommit().Votes().Get(i)
err := k.HandleValidatorSignature(ctx, vote.Validator().Address(), vote.Validator().Power(), vote.GetBlockIDFlag())
Fixed Show fixed Hide fixed
if err != nil {
return err
}
Expand Down
25 changes: 13 additions & 12 deletions x/slashing/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,14 @@ func TestBeginBlocker(t *testing.T) {
Power: power,
}

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

ctx = ctx.WithCometInfo(comet)

err = slashing.BeginBlocker(ctx, slashingKeeper)
require.NoError(t, err)
Expand All @@ -91,11 +95,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 +105,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(sdk.CometInfo{
LastCommit: abci.CommitInfo{Votes: []abci.VoteInfo{{
Validator: abciVal,
BlockIdFlag: cmtproto.BlockIDFlagAbsent,
}})
}}},
})

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