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.

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

Update eth_getBalance to use mirror node #529

Merged
merged 4 commits into from
Sep 20, 2022
Merged

Conversation

Nana-EC
Copy link
Collaborator

@Nana-EC Nana-EC commented Sep 17, 2022

Signed-off-by: Nana Essilfie-Conduah [email protected]

Description:
eth_getBalance currently uses the consensus nodes to retrieve balance since mirror node only update balance every 15 mins.
With mirror node fix to update on transfers relay can now point to mirror node

  • Update eth_getBalance logic to call Mirror Node /api/v1/accounts` endpoint and extract balance
  • Fix metric discrepancy on resolvedCost
  • Update sdkClient flow to remove unnecessary error check since default is UNKNOWN

Related issue(s):

Fixes #448

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: Nana Essilfie-Conduah <[email protected]>
@Nana-EC Nana-EC added this to the 0.8.0 milestone Sep 17, 2022
@Nana-EC Nana-EC self-assigned this Sep 17, 2022
@Nana-EC Nana-EC added enhancement New feature or request P1 labels Sep 17, 2022
@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2022

Codecov Report

Base: 76.67% // Head: 76.54% // Decreases project coverage by -0.12% ⚠️

Coverage data is based on head (463b976) compared to base (68f9216).
Patch coverage: 46.15% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #529      +/-   ##
==========================================
- Coverage   76.67%   76.54%   -0.13%     
==========================================
  Files          12       12              
  Lines         926      921       -5     
  Branches      145      140       -5     
==========================================
- Hits          710      705       -5     
- Misses        164      166       +2     
+ Partials       52       50       -2     
Impacted Files Coverage Δ
packages/relay/src/lib/clients/mirrorNodeClient.ts 88.73% <ø> (-1.41%) ⬇️
packages/relay/src/lib/clients/sdkClient.ts 8.25% <0.00%> (+0.07%) ⬆️
packages/relay/src/lib/eth.ts 84.95% <100.00%> (+0.30%) ⬆️
packages/relay/src/lib/errors/SDKClientError.ts 64.28% <0.00%> (-7.15%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -507,25 +507,18 @@ export class EthImpl implements Eth {

Copy link
Member

Choose a reason for hiding this comment

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

10 lines above this, there is a comment, which I think can be removed now with this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks

Copy link
Member

@georgi-l95 georgi-l95 left a comment

Choose a reason for hiding this comment

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

LG, but one acceptance test is failing.
@release should execute "eth_getBalance" for newly created account with 10 HBAR

Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Nana-EC Nana-EC requested a review from georgi-l95 September 20, 2022 01:01
Copy link
Member

@georgi-l95 georgi-l95 left a comment

Choose a reason for hiding this comment

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

LGTM

@Nana-EC Nana-EC requested a review from Ivo-Yankov September 20, 2022 14:54
@Nana-EC Nana-EC merged commit c13740f into main Sep 20, 2022
@Nana-EC Nana-EC deleted the 448-eth-get-balance-mirror branch September 20, 2022 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P1
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Update eth_getBalance with logic to call mirror node account endpoint
4 participants