Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

baseapp: fix sender events accumulation #6307

Merged
merged 5 commits into from
Jun 2, 2020
Merged

baseapp: fix sender events accumulation #6307

merged 5 commits into from
Jun 2, 2020

Conversation

fedekunze
Copy link
Collaborator

Description

closes: #6306

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@codecov
Copy link

codecov bot commented May 29, 2020 •
edited
Loading

Codecov Report

Merging #6307 into master will decrease coverage by 0.00%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master    #6307      +/-   ##
==========================================
- Coverage   55.70%   55.70%   -0.01%     
==========================================
  Files         446      446              
  Lines       26931    26934       +3     
==========================================
+ Hits        15003    15004       +1     
- Misses      10857    10859       +2     
  Partials     1071     1071

@fedekunze fedekunze marked this pull request as ready for review May 29, 2020 18:02
@fedekunze fedekunze added the R4R label May 29, 2020
alexanderbez
alexanderbez reviewed May 29, 2020
View reviewed changes
x/ibc/ante/ante.go
@@ -37,6 +37,9 @@ func (pvr ProofVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim
_, err = pvr.channelKeeper.AcknowledgePacket(ctx, msg.Packet, msg.Acknowledgement, msg.Proof, msg.ProofHeight)
case channel.MsgTimeout:
_, err = pvr.channelKeeper.TimeoutPacket(ctx, msg.Packet, msg.Proof, msg.ProofHeight, msg.NextSequenceRecv)
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Does this actually fix anything?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, but it prevents adding undesired events to non-IBC messages

alexanderbez
alexanderbez approved these changes May 29, 2020
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Mind adding a unit test for this in BaseApp, please? We should've done this in the original PR.

@fedekunze fedekunze removed the WIP label May 29, 2020
@fedekunze fedekunze added the A:automerge Automatically merge PR once all prerequisites pass. label May 31, 2020
@fedekunze fedekunze changed the title fix sender events accumulation baseapp: fix sender events accumulation May 31, 2020
AdityaSripal
AdityaSripal approved these changes Jun 2, 2020
View reviewed changes
Copy link
Member

@AdityaSripal AdityaSripal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

zmanian
zmanian approved these changes Jun 2, 2020
View reviewed changes
Copy link
Member

@zmanian zmanian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works on Game of Zones Hub 3

@mergify mergify bot merged commit 1f5626b into master Jun 2, 2020
@mergify mergify bot deleted the fedekunze/6306-sender-events branch June 2, 2020 04:26
@alexanderbez
Copy link
Contributor

@fedekunze do you think you can cherry-pick this as a PR to backport/release/0.38.x?

ref: #6306 (comment)

@Codegnosis Codegnosis mentioned this pull request Jun 30, 2020
Closed
4 tasks
@alessio alessio mentioned this pull request Jul 10, 2020
Merged
9 tasks
alessio pushed a commit that referenced this pull request Jul 10, 2020
@fedekunze
baseapp: fix sender events accumulation
c47c1b4 
closes: #6306
original PR: #6307
alessio pushed a commit that referenced this pull request Jul 13, 2020
baseapp: fix sender events accumulation (#6683)
807ea26 
closes: #6306
original PR: #6307
Raumo0 pushed a commit to mapofzones/cosmos-sdk that referenced this pull request Feb 13, 2022
@jinmannwong @alexanderbez
@ethanfrey @fedekunze @jgimeno @tac0turtle
Port sdk v0.39.1 (cosmos#39)
c2c8529 
* v0.38.4-RC1

* add release date

* client/keys/parse: honor config changes (cosmos#6340)

`keys parse` uses the global configuration before
before client applications have had a chance to
apply their settings.

This change adds a `GetSealedConfig()` helper
that waits for the config to be sealed before
returning it.

Fixes: cosmos#5091
Origin: cosmos@4e328d7
Author: Adam Bozanich <[email protected]>
Reviewed-by: Alessio Treglia <[email protected]>

* Merge PR cosmos#6505: Backport v0.38.5: IAVL Bump (RC) & Pruning Refactor

* Fix cl

* Fix cl

* Merge PR cosmos#6551: Ethanfrey/fix trace flag

* Merge PR cosmos#6552: Add sender info to bank transfer event

* Merge PR cosmos#6581: v0.38.5-RC1

* Add release date

* deps: bump IAVL to 0.14

* cl++

* Merge PR cosmos#6618: backport 0.39.0 (launchpad): cherry pick cosmos#5839

* launchpad: bump tendermint to v0.33.6 (cosmos#6673)

* launchpad: bump tendermint to v0.33.6

* cha-cha-cha

* fix types.ChainAnteDecorators() panic (cosmos#5742) (cosmos#6669)

ChainAnteDecorators() panics when no arguments are supplied.
This change its behaviour and the function now returns a nil
AnteHandler in case no AnteDecorator instances are supplied.

Closes: cosmos#5741

* launchpad: register MsgFundCommunityPool to distribution codec (cosmos#6675)

Closes: cosmos#6210

* client: backport IBC additions (cosmos#6682)

* launchpad: backport cliCtx.QueryABCI

* add prove flag

* Save account for multi sending (cosmos#6674)

Include changes from PR cosmos#6283

Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>

* baseapp: fix sender events accumulation (cosmos#6683)

closes: cosmos#6306
original PR: cosmos#6307

* run go mod tidy

* add changelog line re: issue that was not included in v0.38.5

* Fix changelog

* add release notes

* update CODEOWNERS as per STABLE_RELEASES.md

Launchpad's Stable Release Managers team's current composition as
approved by @okwme (ICF):

* @alessio
* @clevinson
* @ethanfrey

* Add milestone's URL

* Revert "update CODEOWNERS as per STABLE_RELEASES.md"

This reverts commit f384592.

* fix typo

* add example patch

* launchpad: backport account sequence stuck (cosmos#6721)

Launchpad fix for cosmos#6287

* update release notes

* make explicit that the regression is fixed in 0.39

* update release notes

* Update CHANGELOG.md

* update release notes

@ethanfrey

* remove pruning info from changelog

They have been moved to NEWS.md

* Update NEWS.md

Co-authored-by: Alexander Bezobchuk <[email protected]>

* Mege PR cosmos#6749: auth: remove custom JSON marshaling

* mv NEWS.md -> RELEASE_NOTES.md

Self-explanatory naming convention.

* Update changelog

* add reference to gaia software upgrade

* x/staking: add call to iterator Close() method (cosmos#6794)

Original pull request: cosmos#6791

* [ci] fix linter (cosmos#6795)

* Merge PR cosmos#6842: Remove custom acc JSON marshl

* Backport cosmos#5671: Add a Flag for CORS (cosmos#6853)

This adds the --unsafe-cors flag functionality.

From: cosmos#5671
Co-authored-by: Marko <[email protected]>

* Merge PR cosmos#6855: Allow passing through Content-Type (needed for POST)

* Merge PR cosmos#6861: Revert "Backport 0.39.1: Remove Custom JSON Marshaling for Accounts"

* Merge PR cosmos#6869: Backport 0.39.1: Launchpad Migration cosmos#6829

* Update changelog and release notes

* Update x/auth/legacy/v0_39/types.go

* Update x/auth/legacy/v0_38/types.go

* Merge PR cosmos#6938: Fix v0.39 migrate test

* Merge PR cosmos#6937: Bump Tendermint to v0.33.7

* Update x/auth/legacy/v0_39/types.go

Co-authored-by: Alexander Bezobchuk <[email protected]>

* update RELEASE_NOTES.md

* update changelog

* test case with real genesis data (cosmos#6995)

* incorporate Ethan Frey's suggestion

* make format

* Update go mod

* Remove some ci files

* Modify data for test

Co-authored-by: Alexander Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: Ethan Frey <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: Jonathan Gimeno <[email protected]>
Co-authored-by: Marko <[email protected]>
Raumo0 pushed a commit to mapofzones/cosmos-sdk that referenced this pull request Feb 13, 2022
@jinmannwong @alexanderbez
@ethanfrey @fedekunze @jgimeno @tac0turtle @boz
Port telemetry (cosmos#41)
938d376 
* v0.38.4-RC1

* add release date

* client/keys/parse: honor config changes (cosmos#6340)

`keys parse` uses the global configuration before
before client applications have had a chance to
apply their settings.

This change adds a `GetSealedConfig()` helper
that waits for the config to be sealed before
returning it.

Fixes: cosmos#5091
Origin: cosmos@4e328d7
Author: Adam Bozanich <[email protected]>
Reviewed-by: Alessio Treglia <[email protected]>

* Merge PR cosmos#6505: Backport v0.38.5: IAVL Bump (RC) & Pruning Refactor

* Fix cl

* Fix cl

* Merge PR cosmos#6551: Ethanfrey/fix trace flag

* Merge PR cosmos#6552: Add sender info to bank transfer event

* Merge PR cosmos#6581: v0.38.5-RC1

* Add release date

* deps: bump IAVL to 0.14

* cl++

* Merge PR cosmos#6618: backport 0.39.0 (launchpad): cherry pick cosmos#5839

* launchpad: bump tendermint to v0.33.6 (cosmos#6673)

* launchpad: bump tendermint to v0.33.6

* cha-cha-cha

* fix types.ChainAnteDecorators() panic (cosmos#5742) (cosmos#6669)

ChainAnteDecorators() panics when no arguments are supplied.
This change its behaviour and the function now returns a nil
AnteHandler in case no AnteDecorator instances are supplied.

Closes: cosmos#5741

* launchpad: register MsgFundCommunityPool to distribution codec (cosmos#6675)

Closes: cosmos#6210

* client: backport IBC additions (cosmos#6682)

* launchpad: backport cliCtx.QueryABCI

* add prove flag

* Save account for multi sending (cosmos#6674)

Include changes from PR cosmos#6283

Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>

* baseapp: fix sender events accumulation (cosmos#6683)

closes: cosmos#6306
original PR: cosmos#6307

* run go mod tidy

* add changelog line re: issue that was not included in v0.38.5

* Fix changelog

* add release notes

* update CODEOWNERS as per STABLE_RELEASES.md

Launchpad's Stable Release Managers team's current composition as
approved by @okwme (ICF):

* @alessio
* @clevinson
* @ethanfrey

* Add milestone's URL

* Revert "update CODEOWNERS as per STABLE_RELEASES.md"

This reverts commit f384592.

* fix typo

* add example patch

* launchpad: backport account sequence stuck (cosmos#6721)

Launchpad fix for cosmos#6287

* update release notes

* make explicit that the regression is fixed in 0.39

* update release notes

* Update CHANGELOG.md

* update release notes

@ethanfrey

* remove pruning info from changelog

They have been moved to NEWS.md

* Update NEWS.md

Co-authored-by: Alexander Bezobchuk <[email protected]>

* Mege PR cosmos#6749: auth: remove custom JSON marshaling

* mv NEWS.md -> RELEASE_NOTES.md

Self-explanatory naming convention.

* Update changelog

* add reference to gaia software upgrade

* x/staking: add call to iterator Close() method (cosmos#6794)

Original pull request: cosmos#6791

* [ci] fix linter (cosmos#6795)

* Merge PR cosmos#6842: Remove custom acc JSON marshl

* Backport cosmos#5671: Add a Flag for CORS (cosmos#6853)

This adds the --unsafe-cors flag functionality.

From: cosmos#5671
Co-authored-by: Marko <[email protected]>

* Merge PR cosmos#6855: Allow passing through Content-Type (needed for POST)

* Merge PR cosmos#6861: Revert "Backport 0.39.1: Remove Custom JSON Marshaling for Accounts"

* Merge PR cosmos#6869: Backport 0.39.1: Launchpad Migration cosmos#6829

* Update changelog and release notes

* Update x/auth/legacy/v0_39/types.go

* Update x/auth/legacy/v0_38/types.go

* Merge PR cosmos#6938: Fix v0.39 migrate test

* Merge PR cosmos#6937: Bump Tendermint to v0.33.7

* Update x/auth/legacy/v0_39/types.go

Co-authored-by: Alexander Bezobchuk <[email protected]>

* update RELEASE_NOTES.md

* update changelog

* test case with real genesis data (cosmos#6995)

* incorporate Ethan Frey's suggestion

* make format

* Update go mod

* Remove some ci files

* Modify data for test

* API server

* Port telemetry

* Initial metrics

* Fix measure

* Merge PR cosmos#6761: telemetry: use UTC() in wrappers

* Remove file

* Make format

* Revert changes to client

* Revert changes to client/lcd/root

* Revert renames in client

* Fix int64 panics

* Revert "Revert renames in client"

This reverts commit cc0a95d14c3212fa1c49f93789dd94e167427a57.

* Revert "Revert changes to client/lcd/root"

This reverts commit e3bb87bbacae13676c3a1f86f8d441d576b1f2ba.

* Revert "Revert changes to client"

This reverts commit 332cacba3242e503bcc4bffc3f6b25c1906efca4.

Co-authored-by: Alexander Bezobchuk <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: Ethan Frey <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: Jonathan Gimeno <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: Adam Bozanich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdityaSripal AdityaSripal AdityaSripal approved these changes

@alexanderbez alexanderbez alexanderbez approved these changes

@zmanian zmanian zmanian approved these changes

@aaronc aaronc Awaiting requested review from aaronc

@alessio alessio Awaiting requested review from alessio

@clevinson clevinson Awaiting requested review from clevinson

@jgimeno jgimeno Awaiting requested review from jgimeno

@jackzampolin jackzampolin Awaiting requested review from jackzampolin

Assignees
No one assigned
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:baseapp T:Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Events are accumulating in msg.sender across trs
4 participants
@fedekunze @alexanderbez @zmanian @AdityaSripal