-
Notifications
You must be signed in to change notification settings - Fork 0
feat_: introduce erc20 balance fetcher #5
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
Conversation
a832825
to
b974066
Compare
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.
Looks great!
f8628bd
to
3cc4dfd
Compare
3cc4dfd
to
179c2c3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5 +/- ##
===========================================
+ Coverage 45.07% 87.28% +42.21%
===========================================
Files 7 5 -2
Lines 264 236 -28
===========================================
+ Hits 119 206 +87
+ Misses 142 26 -116
- Partials 3 4 +1
🚀 New features to boost your workflow:
|
179c2c3
to
bea2c6f
Compare
@@ -44,3 +44,57 @@ func FetchNativeBalancesWithBalanceScanner( | |||
|
|||
return balances, nil | |||
} | |||
|
|||
func FetchErc20BalancesWithBalanceScanner( |
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.
We have a separate package name for tests, which leads us to having functions like FetchErc20BalancesWithBalanceScanner
exported. Is this a deliberate approach to not hide the internals of fetcher
logic? Do we allow those functions to be called directly?
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 saw no good reason not to export this function (some dev might have their own private deployment of BalanceScanner, or want to ensure that either the more efficient way is used or an error is returned), which allowed me to write the tests in a separate package (which I always find preferable to ensure we're testing behavior and not implementation).
46062d7
to
85ceaad
Compare
Add ERC20 balance fetching capabilities to the balance fetcher package.
Adjusted example app
balance-fetcher-web
to showcase new functionality.