Cosmos-sdk: Proposer bonus going to the previous proposer when it should be going to the current proposer

[sunnya97]

As noticed by Warren He:

The bonus for including additional signatures should be going to the proposer of the current block. However, in the code, this proposer bonus is going to the previous block's proposer instead.

https://github.com/cosmos/cosmos-sdk/blob/v0.38.1/x/distribution/keeper/allocation.go#L46-L54

Summary of Bug

Version

Steps to Reproduce

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[cwgoes]

Hmm, I'm not sure, aren't the signatures in sdk.Context from the last commit?

[alexanderbez]

Hmm, I'm not sure, aren't the signatures in sdk.Context from the last commit?

The votes are passed directly into AllocateTokens, which are themselves fetched from Tendermint's RequestBeginBlock:

// TODO this is Tendermint-dependent
// ref https://github.com/cosmos/cosmos-sdk/issues/3095
if ctx.BlockHeight() > 1 {
    previousProposer := k.GetPreviousProposerConsAddr(ctx)
    k.AllocateTokens(ctx, sumPreviousPrecommitPower, previousTotalPower, previousProposer, req.LastCommitInfo.GetVotes())
}

---

In so far as I understand it, a current block H includes pre-commits for H-1. I assume the current proposer decides which pre-commits to include in H. However, it is the previous proposer that actually proposed the block for which the pre-commits commit to.

Is this not what the F1 spec states? If this thinking is wrong, then yes, this is a bug. But based on the distribution code, it seems like this was clearly intentional (e.g. why have an SetPreviousProposerConsAddr API?).

[cwgoes]

If the current proposer decides which commits to include, they should be paid the variable bonus.

I agree that this was definitely intentional though, alas my memory is vague, we should check with Tendermint & see who (which proposal) exactly decides what's in LastCommit.

[alexanderbez]

@ebuchman @melekes, can confirm on who/how decides what is in LastCommit?

[melekes]

proposer of the current block (H) decides on the content of LastCommit (commit for block H-1)

Requirements for LastCommit (from the spec):

Includes one vote for every current validator.
All votes must either be for the previous block, nil or absent.
All votes must have a valid signature from the corresponding validator.
The sum total of the voting power of the validators that voted
must be greater than 2/3 of the total voting power of the complete validator set.

[alexanderbez]

Thanks for confirming my assumption @melekes! That being the case, the code is indeed buggy, correct @cwgoes? The current proposer should get the (variable) bonus.

[cwgoes]

Thanks @melekes - then yes, the code seems wrong - great catch @sunnya97 / Warren He!

It should be updated to pay the proposer of this block in proportion to the precommits they include.