go staking: three-way fee split

.changelog/2794.breaking.md

@@ -0,0 +1,16 @@
+go/staking: Three-way fee split
+
+We should give more to the previous block proposer, which is the block
+that first ran the transactions that paid the fees in the
+`LastBlockFees`.
+Currently they only get paid as a voter.
+
+See
+[oasis-core#2794](https://github.com/oasislabs/oasis-core/pull/2794)
+for a description of the new fee split.
+
+Instructions for genesis document maintainers:
+
+1. Rename `fee_split_vote` to `fee_split_weight_vote` and
+   `fee_split_propose` to `fee_split_weight_next_propose` and
+   add `fee_split_weight_propose` in `.staking.params`.

go/consensus/tendermint/apps/staking/fees.go

@@ -9,124 +9,165 @@ import (
 	stakingState "github.com/oasislabs/oasis-core/go/consensus/tendermint/apps/staking/state"
 )
 
-// disburseFees disburses fees.
+// disburseFeesP disburses fees to the proposer and persists the voters' and next proposer's shares of the fees.
 //
 // In case of errors the state may be inconsistent.
-func (app *stakingApplication) disburseFees(ctx *abci.Context, stakeState *stakingState.MutableState, proposerEntity *signature.PublicKey, numEligibleValidators int, signingEntities []signature.PublicKey) error {
-	totalFees, err := stakeState.LastBlockFees()
+func (app *stakingApplication) disburseFeesP(ctx *abci.Context, stakeState *stakingState.MutableState, proposerEntity *signature.PublicKey, totalFees *quantity.Quantity) error {
+	ctx.Logger().Debug("disbursing proposer fees",
+		"total_amount", totalFees,
+	)
+	if totalFees.IsZero() {
+		stakeState.SetLastBlockFees(totalFees)
+		return nil
+	}
+
+	consensusParameters, err := stakeState.ConsensusParameters()
+	if err != nil {
+		return fmt.Errorf("ConsensusParameters: %w", err)
+	}
+
+	// Compute how much to persist for voters and the next proposer.
+	weightVQ := consensusParameters.FeeSplitWeightVote.Clone()
+	if err = weightVQ.Add(&consensusParameters.FeeSplitWeightNextPropose); err != nil {
+		return fmt.Errorf("add FeeSplitWeightNextPropose: %w", err)
+	}
+	weightPVQ := weightVQ.Clone()
+	if err = weightPVQ.Add(&consensusParameters.FeeSplitWeightPropose); err != nil {
+		return fmt.Errorf("add FeeSplitWeightPropose: %w", err)
+	}
+	feePersistAmt := totalFees.Clone()
+	if err = feePersistAmt.Mul(weightVQ); err != nil {
+		return fmt.Errorf("multiply feePersistAmt: %w", err)
+	}
+	if feePersistAmt.Quo(weightPVQ) != nil {
+		return fmt.Errorf("divide feePersistAmt: %w", err)
+	}
+
+	// Persist voters' and next proposer's shares of the fees.
+	feePersist := quantity.NewQuantity()
+	if err = quantity.Move(feePersist, totalFees, feePersistAmt); err != nil {
+		return fmt.Errorf("move feePersist: %w", err)
+	}
+	stakeState.SetLastBlockFees(feePersist)
+
+	// Pay the proposer.
+	feeProposerAmt := totalFees.Clone()
+	if proposerEntity != nil {
+		acct := stakeState.Account(*proposerEntity)
+		if err = quantity.Move(&acct.General.Balance, totalFees, feeProposerAmt); err != nil {
+			return fmt.Errorf("move feeProposerAmt: %w", err)
+		}
+		stakeState.SetAccount(*proposerEntity, acct)
+	}
+
+	// Put the rest into the common pool (in case there is no proposer entity to pay).
+	if !totalFees.IsZero() {
+		remaining := totalFees.Clone()
+		commonPool, err := stakeState.CommonPool()
+		if err != nil {
+			return fmt.Errorf("CommonPool: %w", err)
+		}
+		if err = quantity.Move(commonPool, totalFees, remaining); err != nil {
+			return fmt.Errorf("move remaining: %w", err)
+		}
+		stakeState.SetCommonPool(commonPool)
+	}
+
+	return nil
+}
+
+// disburseFeesVQ disburses persisted fees to the voters and next proposer.
+//
+// In case of errors the state may be inconsistent.
+func (app *stakingApplication) disburseFeesVQ(ctx *abci.Context, stakeState *stakingState.MutableState, proposerEntity *signature.PublicKey, numEligibleValidators int, votingEntities []signature.PublicKey) error {
+	lastBlockFees, err := stakeState.LastBlockFees()
 	if err != nil {
 		return fmt.Errorf("staking: failed to query last block fees: %w", err)
 	}
 
-	ctx.Logger().Debug("disbursing fees",
-		"total_amount", totalFees,
-		"numEligibleValidators", numEligibleValidators,
-		"numSigningEntities", len(signingEntities),
+	ctx.Logger().Debug("disbursing signer and next proposer fees",
+		"total_amount", lastBlockFees,
+		"num_eligible_validators", numEligibleValidators,
+		"num_voting_entities", len(votingEntities),
 	)
-	if totalFees.IsZero() {
+	if lastBlockFees.IsZero() {
 		// Nothing to disburse.
 		return nil
 	}
 
-	// (Replicated in staking `ConsensusParameters` struct. Keep both explanations in sync.)
-	// A block's fees are split into $n$ portions, one corresponding to each validator.
-	// For each validator $V$ that signs the block, $V$'s corresponding portion is disbursed between $V$ and the
-	// proposer $P$. The ratio of this split are controlled by `FeeSplitVote` and `FeeSplitPropose`.
-	// Portions corresponding to validators that don't sign the block go to the common pool.
-
 	consensusParameters, err := stakeState.ConsensusParameters()
 	if err != nil {
-		return fmt.Errorf("staking: failed to load consensus parameters: %w", err)
+		return fmt.Errorf("ConsensusParameters: %w", err)
 	}
 
-	// Calculate denom := (FeeSplitVote + FeeSplitPropose) * numEligibleValidators.
-	// This will be used to calculate each portion's vote fee and propose fee.
-	denom := consensusParameters.FeeSplitVote.Clone()
-	if err = denom.Add(&consensusParameters.FeeSplitPropose); err != nil {
-		return fmt.Errorf("add fee splits: %w", err)
-	}
+	// Compute the portion associated with each eligible validator's share of the fees, and within that, how much goes
+	// to the voter and how much goes to the next proposer.
+	perValidator := lastBlockFees.Clone()
 	var nEVQ quantity.Quantity
 	if err = nEVQ.FromInt64(int64(numEligibleValidators)); err != nil {
 		return fmt.Errorf("import numEligibleValidators %d: %w", numEligibleValidators, err)
 	}
-	if err = denom.Mul(&nEVQ); err != nil {
-		return fmt.Errorf("multiply denom: %w", err)
+	if err = perValidator.Quo(&nEVQ); err != nil {
+		return fmt.Errorf("divide perValidator: %w", err)
 	}
-
-	// Calculate each portion's vote fee and propose fee.
-	// Portion vote fee. perVIVote := (totalFees * FeeSplitVote) / denom.
-	perVIVote := totalFees.Clone()
-	if err = perVIVote.Mul(&consensusParameters.FeeSplitVote); err != nil {
-		return fmt.Errorf("multiply perVIVote: %w", err)
+	denom := consensusParameters.FeeSplitWeightVote.Clone()
+	if err = denom.Add(&consensusParameters.FeeSplitWeightNextPropose); err != nil {
+		return fmt.Errorf("add FeeSplitWeightNextPropose: %w", err)
 	}
-	if err = perVIVote.Quo(denom); err != nil {
-		return fmt.Errorf("divide perVIVote: %w", err)
+	shareNextProposer := perValidator.Clone()
+	if err = shareNextProposer.Mul(&consensusParameters.FeeSplitWeightNextPropose); err != nil {
+		return fmt.Errorf("multiply shareNextProposer: %w", err)
 	}
-	// Portion propose fee. perVIPropose := (totalFees * FeeSplitPropose) / denom.
-	perVIPropose := totalFees.Clone()
-	if err = perVIPropose.Mul(&consensusParameters.FeeSplitPropose); err != nil {
-		return fmt.Errorf("multiply perVIPropose: %w", err)
+	if err = shareNextProposer.Quo(denom); err != nil {
+		return fmt.Errorf("divide shareNextProposer: %w", err)
 	}
-	// The per-VoteInfo proposer share is first rounded (down), then multiplied by the number of shares.
-	// This keeps incentives from having nonuniform breakpoints at certain signature counts.
-	if err = perVIPropose.Quo(denom); err != nil {
-		return fmt.Errorf("divide perVIPropose: %w", err)
+	shareVote := perValidator.Clone()
+	if err = shareVote.Sub(shareNextProposer); err != nil {
+		return fmt.Errorf("subtract shareVote: %w", err)
 	}
-	numSigningEntities := len(signingEntities)
-	var nSEQ quantity.Quantity
-	if err = nSEQ.FromInt64(int64(numSigningEntities)); err != nil {
-		return fmt.Errorf("import numSigningEntities %d: %w", numSigningEntities, err)
+
+	// Multiply to get the next proposer's total payment.
+	numVotingEntities := len(votingEntities)
+	var nVEQ quantity.Quantity
+	if err = nVEQ.FromInt64(int64(numVotingEntities)); err != nil {
+		return fmt.Errorf("import numVotingEntities %d: %w", numVotingEntities, err)
 	}
-	// Overall propose fee. proposeTotal := perVIPropose * numSigningEntities.
-	proposeTotal := perVIPropose.Clone()
-	if err = proposeTotal.Mul(&nSEQ); err != nil {
-		return fmt.Errorf("multiply proposeTotal: %w", err)
+	nextProposerTotal := shareNextProposer.Clone()
+	if err = nextProposerTotal.Mul(&nVEQ); err != nil {
+		return fmt.Errorf("multiply nextProposerTotal: %w", err)
 	}
 
-	// Pay proposer.
-	if !proposeTotal.IsZero() {
+	// Pay the next proposer.
+	if !nextProposerTotal.IsZero() {
 		if proposerEntity != nil {
-			// Perform the transfer.
 			acct := stakeState.Account(*proposerEntity)
-			if err = quantity.Move(&acct.General.Balance, totalFees, proposeTotal); err != nil {
-				ctx.Logger().Error("failed to disburse fees (propose)",
-					"err", err,
-					"to", *proposerEntity,
-					"amount", proposeTotal,
-				)
-				return fmt.Errorf("staking: failed to disburse fees (propose): %w", err)
+			if err = quantity.Move(&acct.General.Balance, lastBlockFees, nextProposerTotal); err != nil {
+				return fmt.Errorf("move nextProposerTotal: %w", err)
 			}
 			stakeState.SetAccount(*proposerEntity, acct)
 		}
 	}
-	// Pay voters.
-	if !perVIVote.IsZero() {
-		for _, voterEntity := range signingEntities {
-			// Perform the transfer.
+
+	// Pay the voters.
+	if !shareVote.IsZero() {
+		for _, voterEntity := range votingEntities {
 			acct := stakeState.Account(voterEntity)
-			if err = quantity.Move(&acct.General.Balance, totalFees, perVIVote); err != nil {
-				ctx.Logger().Error("failed to disburse fees (vote)",
-					"err", err,
-					"to", voterEntity,
-					"amount", perVIVote,
-				)
-				return fmt.Errorf("staking: failed to disburse fees (vote): %w", err)
+			if err = quantity.Move(&acct.General.Balance, lastBlockFees, shareVote); err != nil {
+				return fmt.Errorf("move shareVote: %w", err)
 			}
 			stakeState.SetAccount(voterEntity, acct)
 		}
 	}
-	// Any remainder goes to the common pool.
-	if !totalFees.IsZero() {
+
+	// Put the rest into the common pool.
+	if !lastBlockFees.IsZero() {
+		remaining := lastBlockFees.Clone()
 		commonPool, err := stakeState.CommonPool()
 		if err != nil {
-			return fmt.Errorf("staking: failed to query common pool: %w", err)
+			return fmt.Errorf("CommonPool: %w", err)
 		}
-		if err := quantity.Move(commonPool, totalFees, totalFees); err != nil {
-			ctx.Logger().Error("failed to move remainder to common pool",
-				"err", err,
-				"amount", totalFees,
-			)
-			return fmt.Errorf("staking: failed to move to common pool: %w", err)
+		if err = quantity.Move(commonPool, lastBlockFees, remaining); err != nil {
+			return fmt.Errorf("move remaining: %w", err)
 		}
 		stakeState.SetCommonPool(commonPool)
 	}

go/consensus/tendermint/apps/staking/staking.go

@@ -59,24 +59,27 @@ func (app *stakingApplication) BeginBlock(ctx *abci.Context, request types.Reque
 	// Look up the proposer's entity.
 	proposingEntity := app.resolveEntityIDFromProposer(regState, request, ctx)
 
-	// Go through all signers of the previous block and resolve entities.
+	// Go through all voters of the previous block and resolve entities.
 	// numEligibleValidators is how many total validators are in the validator set, while
-	// signingEntities is from the validators which actually signed.
+	// votingEntities is from the validators which actually voted.
 	numEligibleValidators := len(request.GetLastCommitInfo().Votes)
-	signingEntities := app.resolveEntityIDsFromVotes(ctx, regState, request.GetLastCommitInfo())
+	votingEntities := app.resolveEntityIDsFromVotes(ctx, regState, request.GetLastCommitInfo())
 
 	// Disburse fees from previous block.
-	if err := app.disburseFees(ctx, stakeState, proposingEntity, numEligibleValidators, signingEntities); err != nil {
-		return fmt.Errorf("staking: failed to disburse fees: %w", err)
+	if err := app.disburseFeesVQ(ctx, stakeState, proposingEntity, numEligibleValidators, votingEntities); err != nil {
+		return fmt.Errorf("disburse fees voters and next proposer: %w", err)
 	}
 
+	// Save block proposer for fee disbursements.
+	stakingState.SetBlockProposer(ctx, proposingEntity)
+
 	// Add rewards for proposer.
-	if err := app.rewardBlockProposing(ctx, stakeState, proposingEntity, numEligibleValidators, len(signingEntities)); err != nil {
+	if err := app.rewardBlockProposing(ctx, stakeState, proposingEntity, numEligibleValidators, len(votingEntities)); err != nil {
 		return fmt.Errorf("staking: block proposing reward: %w", err)
 	}
 
 	// Track signing for rewards.
-	if err := app.updateEpochSigning(ctx, stakeState, signingEntities); err != nil {
+	if err := app.updateEpochSigning(ctx, stakeState, votingEntities); err != nil {
 		return fmt.Errorf("staking: failed to update epoch signing info: %w", err)
 	}
 
@@ -147,8 +150,10 @@ func (app *stakingApplication) ForeignExecuteTx(ctx *abci.Context, other abci.Ap
 }
 
 func (app *stakingApplication) EndBlock(ctx *abci.Context, request types.RequestEndBlock) (types.ResponseEndBlock, error) {
-	// Persist any block fees so we can transfer them in the next block.
-	stakingState.PersistBlockFees(ctx)
+	fees := stakingState.BlockFees(ctx)
+	if err := app.disburseFeesP(ctx, stakingState.NewMutableState(ctx.State()), stakingState.BlockProposer(ctx), &fees); err != nil {
+		return types.ResponseEndBlock{}, fmt.Errorf("disburse fees proposer: %w", err)
+	}
 
 	if changed, epoch := app.state.EpochChanged(ctx); changed {
 		return types.ResponseEndBlock{}, app.onEpochChange(ctx, epoch)

go/consensus/tendermint/apps/staking/state/gas.go

@@ -100,11 +100,24 @@ func AuthenticateAndPayFees(
 	return nil
 }
 
-// PersistBlockFees persists the accumulated fee balance for the current block.
-func PersistBlockFees(ctx *abci.Context) {
+// BlockFees returns the accumulated fee balance for the current block.
+func BlockFees(ctx *abci.Context) quantity.Quantity {
 	// Fetch accumulated fees in the current block.
-	fees := ctx.BlockContext().Get(feeAccumulatorKey{}).(*feeAccumulator).balance
+	return ctx.BlockContext().Get(feeAccumulatorKey{}).(*feeAccumulator).balance
+}
 
-	state := NewMutableState(ctx.State())
-	state.SetLastBlockFees(&fees)
+// proposerKey is the block context key.
+type proposerKey struct{}
+
+func (pk proposerKey) NewDefault() interface{} {
+	var empty *signature.PublicKey
+	return empty
+}
+
+func SetBlockProposer(ctx *abci.Context, p *signature.PublicKey) {
+	ctx.BlockContext().Set(proposerKey{}, p)
+}
+
+func BlockProposer(ctx *abci.Context) *signature.PublicKey {
+	return ctx.BlockContext().Get(proposerKey{}).(*signature.PublicKey)
 }

go/genesis/genesis_test.go

@@ -130,7 +130,7 @@ func TestGenesisChainContext(t *testing.T) {
 	//       on each run.
 	stableDoc.Staking = staking.Genesis{}
 
-	require.Equal(t, "813021fca91663966fa73935b88165713d133ab2073f205050749e5b279fd869", stableDoc.ChainContext())
+	require.Equal(t, "77bc3d261fd95ff38712293ce9c8b93f20464ab8bc1f99c8f9cdf6aff4706730", stableDoc.ChainContext())
 }
 
 func TestGenesisSanityCheck(t *testing.T) {

go/oasis-node/cmd/genesis/genesis.go

@@ -475,7 +475,7 @@ func AppendStakingState(doc *genesis.Document, state string, l *logging.Logger)
 	stakingSt := staking.Genesis{
 		Ledger: make(map[signature.PublicKey]*staking.Account),
 	}
-	if err := stakingSt.Parameters.FeeSplitVote.FromInt64(1); err != nil {
+	if err := stakingSt.Parameters.FeeSplitWeightVote.FromInt64(1); err != nil {
 		return fmt.Errorf("couldn't set default fee split: %w", err)
 	}
 

go/oasis-test-runner/scenario/e2e/gas_fees_staking.go

@@ -190,6 +190,22 @@ func (sc *gasFeesImpl) runTests(ctx context.Context) error {
 		return err
 	}
 
+	// As of the last time this comment was updated:
+	// - total fees has 150 tokens from genesis common pool
+	// - total fees has 50 tokens from genesis last block fees
+	// - for each of 12 transactions that pay for gas:
+	//   - 10 tokens paid for gas in a block on its own
+	//   - (2+2)/(1+2+2) = 80% => 8 tokens persisted for VQ share
+	//   - 10 - 8 = 2 tokens paid to P
+	//   - VQ share divided into 3 validator portions, for 2 tokens each
+	//   - (2)/(2+2) = 50% => 1 token per validator for Q
+	//   - 2 - 1 = 1 token per validator for V
+	//   - remaining 2 tokens moved to common pool
+	// - 150 + 50 + 12 * 10 = 320 tokens `total_fees`
+	// - 12 * 2 = 24 tokens paid for P role
+	// - 12 * 1 * 3 = 36 tokens paid for V roles
+	// - 12 * 1 * 3 = 36 tokens paid for Q role
+	// - 24 + 36 + 36 = 96 tokens `disbursed_fees`
 	sc.logger.Info("making sure that fees have been disbursed",
 		"total_fees", totalFees,
 		"disbursed_fees", newTotalEntityBalance,