From e7615d69fdc08d7545639f26dd297dec4836522f Mon Sep 17 00:00:00 2001 From: Carlton Hanna Date: Wed, 24 Jan 2024 22:07:32 -0700 Subject: [PATCH] Add filtering of nil events from begin blocker (#1823) * add filtering of nil events from begin blocker * Add tests and refactor function * add comments * fix lint --- CHANGELOG.md | 1 + app/app.go | 28 ++++++++++++- app/app_test.go | 102 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d66ccbd880..a073255071 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * Add upgrade handler for 1.18 [#1756](https://github.com/provenance-io/provenance/pull/1756). * Remove the rust upgrade handlers [PR 1774](https://github.com/provenance-io/provenance/pull/1774). * Allow bypassing the config warning wait using an environment variable [PR 1810](https://github.com/provenance-io/provenance/pull/1810). +* Filter out empty distribution events from begin blocker [#1822](https://github.com/provenance-io/provenance/pull/1822). ### Bug Fixes diff --git a/app/app.go b/app/app.go index 8c50610c23..604376f5d7 100644 --- a/app/app.go +++ b/app/app.go @@ -1124,7 +1124,33 @@ func (app *App) Name() string { return app.BaseApp.Name() } // BeginBlocker application updates every begin block func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock { - return app.mm.BeginBlock(ctx, req) + responseBeginBlock := app.mm.BeginBlock(ctx, req) + responseBeginBlock.Events = filterBeginBlockerEvents(responseBeginBlock) + return responseBeginBlock +} + +// filterBeginBlockerEvents filters out events from a given abci.ResponseBeginBlock according to the criteria defined in shouldFilterEvent. +func filterBeginBlockerEvents(responseBeginBlock abci.ResponseBeginBlock) []abci.Event { + filteredEvents := make([]abci.Event, 0) + for _, e := range responseBeginBlock.Events { + if shouldFilterEvent(e) { + continue + } + filteredEvents = append(filteredEvents, e) + } + return filteredEvents +} + +// shouldFilterEvent checks if an abci.Event should be filtered based on its type and attributes. +func shouldFilterEvent(e abci.Event) bool { + if e.Type == distrtypes.EventTypeCommission || e.Type == distrtypes.EventTypeRewards || e.Type == distrtypes.EventTypeProposerReward || e.Type == banktypes.EventTypeTransfer || e.Type == banktypes.EventTypeCoinSpent || e.Type == banktypes.EventTypeCoinReceived { + for _, a := range e.Attributes { + if string(a.Key) == sdk.AttributeKeyAmount && len(a.Value) == 0 { + return true + } + } + } + return false } // EndBlocker application updates every end block diff --git a/app/app_test.go b/app/app_test.go index cbf4126006..7f21e23171 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -14,6 +14,8 @@ import ( sdksim "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" @@ -322,3 +324,103 @@ func assertAddrNotInAccounts(t *testing.T, addr sdk.AccAddress, addrName string, } return true } + +func createEvent(eventType string, attributes []abci.EventAttribute) abci.Event { + return abci.Event{ + Type: eventType, + Attributes: attributes, + } +} + +func TestShouldFilterEvent(t *testing.T) { + tests := []struct { + name string + event abci.Event + expect bool + }{ + {"Empty commission event", createEvent(distrtypes.EventTypeCommission, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("")}}), true}, + {"Nil commission event", createEvent(distrtypes.EventTypeCommission, []abci.EventAttribute{{Key: []byte("amount"), Value: nil}}), true}, + {"Non-empty commission event", createEvent(distrtypes.EventTypeCommission, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("100")}}), false}, + + {"Empty rewards event", createEvent(distrtypes.EventTypeRewards, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("")}}), true}, + {"Nil rewards event", createEvent(distrtypes.EventTypeRewards, []abci.EventAttribute{{Key: []byte("amount"), Value: nil}}), true}, + {"Non-empty rewards event", createEvent(distrtypes.EventTypeRewards, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("100")}}), false}, + + {"Empty proposer_reward event", createEvent(distrtypes.EventTypeProposerReward, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("")}}), true}, + {"Nil proposer_reward event", createEvent(distrtypes.EventTypeProposerReward, []abci.EventAttribute{{Key: []byte("amount"), Value: nil}}), true}, + {"Non-empty proposer_reward event", createEvent(distrtypes.EventTypeProposerReward, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("100")}}), false}, + + {"Empty transfer event", createEvent(banktypes.EventTypeTransfer, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("")}}), true}, + {"Nil transfer event", createEvent(banktypes.EventTypeTransfer, []abci.EventAttribute{{Key: []byte("amount"), Value: nil}}), true}, + {"Non-empty transfer event", createEvent(banktypes.EventTypeTransfer, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("100")}}), false}, + + {"Empty coin_spent event", createEvent(banktypes.EventTypeCoinSpent, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("")}}), true}, + {"Nil coin_spent event", createEvent(banktypes.EventTypeCoinSpent, []abci.EventAttribute{{Key: []byte("amount"), Value: nil}}), true}, + {"Non-empty coin_spent event", createEvent(banktypes.EventTypeCoinSpent, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("100")}}), false}, + + {"Empty coin_received event", createEvent(banktypes.EventTypeCoinReceived, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("")}}), true}, + {"Nil coin_received event", createEvent(banktypes.EventTypeCoinReceived, []abci.EventAttribute{{Key: []byte("amount"), Value: nil}}), true}, + {"Non-empty coin_received event", createEvent(banktypes.EventTypeCoinReceived, []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("100")}}), false}, + + {"Unhandled event type with empty amount", createEvent("unhandled_type", []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("")}}), false}, + {"Unhandled event type with nil amount", createEvent("unhandled_type", []abci.EventAttribute{{Key: []byte("amount"), Value: nil}}), false}, + {"Unhandled event type with non-empty amount", createEvent("unhandled_type", []abci.EventAttribute{{Key: []byte("amount"), Value: []byte("100")}}), false}, + {"Event with no attributes", createEvent(distrtypes.EventTypeCommission, []abci.EventAttribute{}), false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := shouldFilterEvent(tc.event) + assert.Equal(t, tc.expect, result, "Test %v failed: expected %v, got %v", tc.name, tc.expect, result) + }) + } +} + +func TestFilterBeginBlockerEvents(t *testing.T) { + tests := []struct { + name string + events []abci.Event + expected []abci.Event + }{ + { + name: "Filter out events with empty amounts", + events: []abci.Event{ + createEvent(distrtypes.EventTypeCommission, []abci.EventAttribute{{Key: []byte(sdk.AttributeKeyAmount), Value: []byte("")}}), + createEvent(distrtypes.EventTypeRewards, []abci.EventAttribute{{Key: []byte(sdk.AttributeKeyAmount), Value: []byte("100")}}), + }, + expected: []abci.Event{ + createEvent(distrtypes.EventTypeRewards, []abci.EventAttribute{{Key: []byte(sdk.AttributeKeyAmount), Value: []byte("100")}}), + }, + }, + { + name: "No filtering when all events are valid", + events: []abci.Event{ + createEvent(banktypes.EventTypeTransfer, []abci.EventAttribute{{Key: []byte(sdk.AttributeKeyAmount), Value: []byte("100")}}), + }, + expected: []abci.Event{ + createEvent(banktypes.EventTypeTransfer, []abci.EventAttribute{{Key: []byte(sdk.AttributeKeyAmount), Value: []byte("100")}}), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + responseBeginBlock := abci.ResponseBeginBlock{Events: tc.events} + actualEvents := filterBeginBlockerEvents(responseBeginBlock) + assert.Equal(t, len(tc.expected), len(actualEvents), "Number of events mismatch") + + for i, expectedEvent := range tc.expected { + actualEvent := actualEvents[i] + assert.Equal(t, expectedEvent.Type, actualEvent.Type, "Event types mismatch") + + assert.Equal(t, len(expectedEvent.Attributes), len(actualEvent.Attributes), "Number of attributes mismatch in event %v", expectedEvent.Type) + + for j, expectedAttribute := range expectedEvent.Attributes { + actualAttribute := actualEvent.Attributes[j] + assert.Equal(t, expectedAttribute.Key, actualAttribute.Key, "Attribute keys mismatch in event %v", expectedEvent.Type) + assert.Equal(t, expectedAttribute.Value, actualAttribute.Value, "Attribute values mismatch in event %v", expectedEvent.Type) + } + } + }) + } +}