-
Notifications
You must be signed in to change notification settings - Fork 59
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
Only log when trying to map restricted swap transfers #1732
Conversation
`Unsupported App: ${order.fullAppData?.appCode}`, | ||
); | ||
// Don't throw in order to fallback to "standard" transfer mapping | ||
return null; |
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.
Nit: while I see null
returns in many other places related to transactions/swap/twaps mappings to indicate a failure, I also think we should refactor the null
returns and change them for actual errors and do proper error handling in the upper layers/caller functions, wdyt? (This is just a general comment, I'm totally fine with this PR 🙂)
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.
I agree. There is a common pattern with the finding then mapping of swap-related transactions that needs to be refactored in general. I think it would also solve this.
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.
"Proper error handling" has been included in #1733, but it should definitely be refactored as mentioned.
Pull Request Test Coverage Report for Build 9806877275Details
💛 - Coveralls |
Summary
When mapping a swap transfer, we are currently throwing if the transfer was created by a disallowed app. This means that the mapping breaks when we should be falling back to "standard" transfer mapping.
This switches from throwing to instead logging and returning
null
instead of a mapped swap transfer. This means that the "standard" mapping will be done instead.Changes
null
instead of throwing when mapping a disallowed swap transfer