staking: Don't emit zero-amount staking events

.changelog/2983.bugfix.md

@@ -0,0 +1,8 @@
+staking: Don't emit zero-amount staking events
+
+If fees were set to 0, staking events related to the fee accumulator
+would still get emitted with zero amounts, which is pointless.
+
+This fix affects only events related to the internal fee accumulator
+and common pool accounts, manual transfers with 0 as the amount will
+still get emitted.

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

@@ -63,7 +63,7 @@ func (app *stakingApplication) disburseFeesP(
 
 	// Pay the proposer.
 	feeProposerAmt := totalFees.Clone()
-	if proposerEntity != nil {
+	if proposerEntity != nil && !feeProposerAmt.IsZero() {
 		acct, err := stakeState.Account(ctx, *proposerEntity)
 		if err != nil {
 			return fmt.Errorf("failed to fetch proposer account: %w", err)

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

@@ -98,13 +98,15 @@ func AuthenticateAndPayFees(
 		return fmt.Errorf("failed to set account: %w", err)
 	}
 
-	// Emit transfer event.
-	ev := cbor.Marshal(&staking.TransferEvent{
-		From:   id,
-		To:     staking.FeeAccumulatorAccountID,
-		Tokens: fee.Amount,
-	})
-	ctx.EmitEvent(abciAPI.NewEventBuilder(AppName).Attribute(KeyTransfer, ev))
+	// Emit transfer event if fee is non-zero.
+	if !fee.Amount.IsZero() {
+		ev := cbor.Marshal(&staking.TransferEvent{
+			From:   id,
+			To:     staking.FeeAccumulatorAccountID,
+			Tokens: fee.Amount,
+		})
+		ctx.EmitEvent(abciAPI.NewEventBuilder(AppName).Attribute(KeyTransfer, ev))
+	}
 
 	// Configure gas accountant on the context.
 	ctx.SetGasAccountant(abciAPI.NewCompositeGasAccountant(

go/staking/tests/tester.go

@@ -181,20 +181,18 @@ func testTransfer(t *testing.T, state *stakingTestsState, backend api.Backend, c
 	err = consensusAPI.SignAndSubmitTx(context.Background(), consensus, srcSigner, tx)
 	require.NoError(err, "Transfer")
 
-	var gotCommon bool
-	var gotFeeAcc bool
 	var gotTransfer bool
 
 TransferWaitLoop:
 	for {
 		select {
 		case ev := <-ch:
 			if ev.From.Equal(api.CommonPoolAccountID) || ev.To.Equal(api.CommonPoolAccountID) {
-				gotCommon = true
+				require.False(ev.Tokens.IsZero(), "CommonPool xfer: amount should be non-zero")
 				continue
 			}
 			if ev.From.Equal(api.FeeAccumulatorAccountID) || ev.To.Equal(api.FeeAccumulatorAccountID) {
-				gotFeeAcc = true
+				require.False(ev.Tokens.IsZero(), "FeeAccumulator xfer: amount should be non-zero")
 				continue
 			}
 
@@ -218,16 +216,14 @@ TransferWaitLoop:
 				require.True(gotTransfer, "GetEvents should return transfer event")
 			}
 
-			if (gotCommon || gotFeeAcc) && gotTransfer {
+			if gotTransfer {
 				break TransferWaitLoop
 			}
 		case <-time.After(recvTimeout):
 			t.Fatalf("failed to receive transfer event")
 		}
 	}
 
-	require.True(gotCommon || gotFeeAcc, "WatchTransfers should also return transfers related to the common pool and/or the fee accumulator")
-
 	_ = srcAcc.General.Balance.Sub(&xfer.Tokens)
 	newSrcAcc, err := backend.AccountInfo(context.Background(), &api.OwnerQuery{Owner: SrcID, Height: consensusAPI.HeightLatest})
 	require.NoError(err, "src: AccountInfo - after")
@@ -267,8 +263,6 @@ func testTransferWatchEvents(t *testing.T, state *stakingTestsState, backend api
 	err = consensusAPI.SignAndSubmitTx(context.Background(), consensus, srcSigner, tx)
 	require.NoError(err, "SignAndSubmitTx")
 
-	var gotCommon bool
-	var gotFeeAcc bool
 	var gotTransfer bool
 
 TransferWaitLoop:
@@ -282,14 +276,15 @@ TransferWaitLoop:
 
 			from := ev.TransferEvent.From
 			to := ev.TransferEvent.To
+			tokens := ev.TransferEvent.Tokens
 
 			// We should also get some traffic to/from the common pool and fee accumulator.
 			if from.Equal(api.CommonPoolAccountID) || to.Equal(api.CommonPoolAccountID) {
-				gotCommon = true
+				require.False(tokens.IsZero(), "CommonPool xfer: amount should be non-zero")
 				continue
 			}
 			if from.Equal(api.FeeAccumulatorAccountID) || to.Equal(api.FeeAccumulatorAccountID) {
-				gotFeeAcc = true
+				require.False(tokens.IsZero(), "FeeAccumulator xfer: amount should be non-zero")
 				continue
 			}
 
@@ -307,15 +302,13 @@ TransferWaitLoop:
 				gotTransfer = true
 			}
 
-			if (gotCommon || gotFeeAcc) && gotTransfer {
+			if gotTransfer {
 				break TransferWaitLoop
 			}
 		case <-time.After(recvTimeout):
 			t.Fatalf("failed to receive transfer event")
 		}
 	}
-
-	require.True(gotCommon || gotFeeAcc, "WatchEvents should also return transfer events related to the common pool and/or the fee accumulator")
 }
 
 func testSelfTransfer(t *testing.T, state *stakingTestsState, backend api.Backend, consensus consensusAPI.Backend) {
@@ -336,20 +329,18 @@ func testSelfTransfer(t *testing.T, state *stakingTestsState, backend api.Backen
 	err = consensusAPI.SignAndSubmitTx(context.Background(), consensus, srcSigner, tx)
 	require.NoError(err, "Transfer")
 
-	var gotCommon bool
-	var gotFeeAcc bool
 	var gotTransfer bool
 
 TransferWaitLoop:
 	for {
 		select {
 		case ev := <-ch:
 			if ev.From.Equal(api.CommonPoolAccountID) || ev.To.Equal(api.CommonPoolAccountID) {
-				gotCommon = true
+				require.False(ev.Tokens.IsZero(), "CommonPool xfer: amount should be non-zero")
 				continue
 			}
 			if ev.From.Equal(api.FeeAccumulatorAccountID) || ev.To.Equal(api.FeeAccumulatorAccountID) {
-				gotFeeAcc = true
+				require.False(ev.Tokens.IsZero(), "FeeAccumulator xfer: amount should be non-zero")
 				continue
 			}
 
@@ -360,16 +351,14 @@ TransferWaitLoop:
 				gotTransfer = true
 			}
 
-			if (gotCommon || gotFeeAcc) && gotTransfer {
+			if gotTransfer {
 				break TransferWaitLoop
 			}
 		case <-time.After(recvTimeout):
 			t.Fatalf("failed to receive transfer event")
 		}
 	}
 
-	require.True(gotCommon || gotFeeAcc, "WatchTransfers should also return transfers related to the common pool and/or the fee accumulator")
-
 	newSrcAcc, err := backend.AccountInfo(context.Background(), &api.OwnerQuery{Owner: SrcID, Height: consensusAPI.HeightLatest})
 	require.NoError(err, "src: AccountInfo - after")
 	require.Equal(srcAcc.General.Balance, newSrcAcc.General.Balance, "src: general balance - after")