-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
remove-unnecessary-events #126
base: main
Are you sure you want to change the base?
Conversation
@@ -233,7 +223,7 @@ contract ICS20Transfer is IIBCApp, IICS20Transfer, IICS20Errors, Ownable, Reentr | |||
string calldata destChannel, | |||
ICS20Lib.FungibleTokenPacketData memory packetData | |||
) | |||
private | |||
public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note this, I changed the visibility here to have access to this function to properly test the logic of the foreignDenoms test without relying on the events. However it may not be the ideal solutions.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #126 +/- ##
==========================================
- Coverage 93.89% 93.77% -0.13%
==========================================
Files 9 9
Lines 311 305 -6
==========================================
- Hits 292 286 -6
Misses 19 19 ☔ View full report in Codecov by Sentry. |
Noted that currently our code:
My take is that we should add the updateClient event and remove the Committed events. Committed events will be emitted right before the router events and, like those, they emit the packet, thus they appear to be redundant. |
Description
WIP on #108. Currently this PR :
PacketCommited
andAckCommitted
events from IBCStoreWhat is missing :
closes: #108
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.
godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.