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

feat(rpc): implementing staking reads and replace sdk.Address #1060

Merged
merged 4 commits into from
Sep 15, 2022

Conversation

distractedm1nd
Copy link
Collaborator

@distractedm1nd distractedm1nd commented Sep 8, 2022

Closes #986.

Also removes the abstraction of sdk.Address to make it explicit that our queries only support their corresponding address type, AccAddress or ValAddress

@distractedm1nd distractedm1nd added enhancement New feature or request area:rpc labels Sep 8, 2022
@distractedm1nd distractedm1nd self-assigned this Sep 8, 2022
@distractedm1nd distractedm1nd marked this pull request as ready for review September 8, 2022 06:57
@distractedm1nd distractedm1nd added kind:improvement and removed enhancement New feature or request labels Sep 8, 2022
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Rebase on main 🙏🏻

service/rpc/state.go Outdated Show resolved Hide resolved
service/rpc/state.go Outdated Show resolved Hide resolved
service/state/core_access.go Outdated Show resolved Hide resolved
service/state/core_access.go Outdated Show resolved Hide resolved
service/state/core_access.go Outdated Show resolved Hide resolved
service/rpc/state.go Outdated Show resolved Hide resolved
walldiss
walldiss previously approved these changes Sep 9, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1060 (ec665b6) into main (8657edc) will decrease coverage by 0.53%.
The diff coverage is 7.85%.

@@            Coverage Diff             @@
##             main    #1060      +/-   ##
==========================================
- Coverage   56.56%   56.02%   -0.54%     
==========================================
  Files         135      135              
  Lines        8965     9071     +106     
==========================================
+ Hits         5071     5082      +11     
- Misses       3359     3454      +95     
  Partials      535      535              
Impacted Files Coverage Δ
service/rpc/state.go 0.00% <0.00%> (ø)
service/state/service.go 29.72% <0.00%> (-5.76%) ⬇️
service/state/core_access.go 12.61% <8.10%> (+1.02%) ⬆️
service/rpc/endpoints.go 100.00% <100.00%> (ø)
ipld/get_namespaced_shares.go 88.32% <0.00%> (-2.19%) ⬇️
fraud/bad_encoding.go 66.00% <0.00%> (+3.00%) ⬆️

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

Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

The RPC part is LGTM to me. I am a bit concerned about types change here

service/state/state.go Show resolved Hide resolved
@renaynay renaynay requested a review from Wondertan September 13, 2022 08:11
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Implementation looks clean, thank you @distractedm1nd

Let's please test every endpoint against arabica before we merge. 🙏🏻

@distractedm1nd distractedm1nd changed the title feat(rpc): implementing staking reads feat(rpc): implementing staking reads and replace sdk.Address Sep 13, 2022
@distractedm1nd
Copy link
Collaborator Author

All endpoints including queries have been tested on arabica, BUT:

  • Redelegation and Redelegation Query are not 100% tested because I can't do a redelegation with only one ValAddr. But the errors returned when redelegating to the same ValAddr are reassuring enough.

@distractedm1nd distractedm1nd merged commit 1803ca0 into celestiaorg:main Sep 15, 2022
distractedm1nd added a commit to renaynay/celestia-node that referenced this pull request Sep 19, 2022
…stiaorg#1060)

* feat: staking reads + linting errors

* feat: rewriting to not take delAddr as a parameter

* refactor(rpc): renaming local variables

* refactor(rpc): using AccAddress and ValAddress instead of the generic Address type
distractedm1nd added a commit to distractedm1nd/celestia-node that referenced this pull request Sep 21, 2022
…stiaorg#1060)

* feat: staking reads + linting errors

* feat: rewriting to not take delAddr as a parameter

* refactor(rpc): renaming local variables

* refactor(rpc): using AccAddress and ValAddress instead of the generic Address type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Implement staking module reads
5 participants