Skip to content

Commit

Permalink
staking: Don't emit zero-amount staking events
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
abukosek committed Jun 8, 2020
1 parent dc893d3 commit 9f625d2
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 29 deletions.
2 changes: 1 addition & 1 deletion go/consensus/tendermint/apps/staking/fees.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 9 additions & 7 deletions go/consensus/tendermint/apps/staking/state/gas.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
31 changes: 10 additions & 21 deletions go/staking/tests/tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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")
Expand Down Expand Up @@ -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:
Expand All @@ -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
}

Expand All @@ -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) {
Expand All @@ -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
}

Expand All @@ -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")
Expand Down

0 comments on commit 9f625d2

Please sign in to comment.