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.

Sign up for GitHub

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

Jump to bottom

1063 add rewards runtime api #1124

Merged
merged 22 commits into from
Dec 14, 2022
Merged

1063 add rewards runtime api #1124

merged 22 commits into from
Dec 14, 2022

Conversation

cdamian
Copy link
Contributor

@cdamian cdamian commented Dec 8, 2022 •
edited
Loading

Description

Fixes # (issue)

#1063

Changes and Descriptions

Added runtime API for:

  • listing reward currencies of account
  • computing reward for an account

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Adjusted the current tests that we have in the rewards pallet to confirm that listing the currencies behaves correctly.
  • Tested API functionality using the newly added integration tests for runtime APIs.

Checklist:

  • I have added Rust doc comments to structs, enums, traits and functions
  • I have made corresponding changes to the documentation
  • I have performed a self-review of my code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I rebased on the latest main branch

@cdamian cdamian force-pushed the 1063-add-rewards-runtime-api branch from 723e65d to afb7b83 Compare December 8, 2022 17:46
@cdamian cdamian force-pushed the 1063-add-rewards-runtime-api branch from af8f6e5 to 745c6af Compare December 9, 2022 09:10
@cdamian cdamian marked this pull request as ready for review December 9, 2022 10:46
NunoAlexandre
NunoAlexandre reviewed Dec 9, 2022
View reviewed changes
Copy link
Contributor

@NunoAlexandre NunoAlexandre left a comment

Choose a reason for hiding this comment

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

Looks good overall 👌 a few comments only

NunoAlexandre
NunoAlexandre previously approved these changes Dec 9, 2022
View reviewed changes
Copy link
Contributor

@NunoAlexandre NunoAlexandre left a comment

Choose a reason for hiding this comment

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

LGTM 💯

lemunozm
lemunozm requested changes Dec 12, 2022
View reviewed changes
Copy link
Contributor

@lemunozm lemunozm left a comment

Choose a reason for hiding this comment

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

LGTM! 😃 Only few comments

lemunozm
lemunozm requested changes Dec 12, 2022
View reviewed changes
Copy link
Contributor

@lemunozm lemunozm left a comment

Choose a reason for hiding this comment

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

Sorry for this late comment! 🙏🏻

lemunozm
lemunozm previously approved these changes Dec 12, 2022
View reviewed changes
Copy link
Contributor

@lemunozm lemunozm left a comment

Choose a reason for hiding this comment

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

LGTM! I leave two minor comments, but for me it is approved. Very clean PR @cosmin!

@cdamian cdamian force-pushed the 1063-add-rewards-runtime-api branch from a254087 to 21e74bf Compare December 12, 2022 18:37
lemunozm
lemunozm previously approved these changes Dec 12, 2022
View reviewed changes
mikiquantum
mikiquantum previously approved these changes Dec 12, 2022
View reviewed changes
Copy link
Contributor

@mikiquantum mikiquantum left a comment

Choose a reason for hiding this comment

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

👍

@cdamian cdamian marked this pull request as draft December 13, 2022 12:23
@cdamian
Copy link
Contributor Author

cdamian commented Dec 13, 2022

Marked as draft to avoid an accidental merge before the update to polkadot-0.9.32 is done.

I will rebase this branch as well.

@cdamian cdamian dismissed stale reviews from mikiquantum and lemunozm via 10d92d0 December 13, 2022 15:24
@cdamian cdamian force-pushed the 1063-add-rewards-runtime-api branch 2 times, most recently from 44a448d to d2dbf53 Compare December 13, 2022 16:05
@cdamian cdamian self-assigned this Dec 14, 2022
@gruberb gruberb mentioned this pull request Dec 14, 2022
Closed
12 tasks
@cdamian cdamian force-pushed the 1063-add-rewards-runtime-api branch 2 times, most recently from de6d017 to 38b6c2f Compare December 14, 2022 15:18
@cdamian cdamian marked this pull request as ready for review December 14, 2022 15:24
mikiquantum and others added 3 commits December 14, 2022 17:49
@mikiquantum @cdamian
fix weightlimit + mocks
0fb4f34
@mikiquantum @cdamian
attempt to adapt pools integration builder to new version of fudge
0f0259c
@NunoAlexandre @cdamian
Skip e2e / pools for now
e617094 
Lots of compilation errors in the way. Let's comment those out and
address everything else first.
mustermeiszer and others added 19 commits December 14, 2022 17:49
@mustermeiszer @cdamian
Fixing integrations test
3672bae
@cdamian
rewards: Rename CurrencyId to RewardCurrencyId
70e1ffc
@cdamian
rewards: Add tests for list currencies
ac38cc4
@cdamian
runtime: Add rewards API
c22d86e
@cdamian
rpc: Add rewards API
259492f
@cdamian
deps: Add rewards pallet to runtime-common
23473cd
@cdamian
scripts: Change log level to info when starting parachain
5d2e3ef
@cdamian
rewards-api: Rename param and change param order
e3b7d49
@cdamian
lint: Obey
7d29205
@cdamian
rewards-api: Convert result to option
89f2f98
@cdamian
rewards: Revert CurrencyId name change
68f81cc
@cdamian
rewards: Drop turbofish from collect
e59ed2e
@cdamian
rewards-test: Drop func that checks user currencies and use assert
944cc5e
@cdamian
rewards: Adjust CurrencyId comment
e1e09d8
@cdamian
rewards-test: Add test for associating different currencies
e0b861d
@cdamian
rewards: Return vector when listing currencies
7b5b1eb
@mustermeiszer @cdamian
Start testing runtime apis: Setup
75714ee
@cdamian
integration-test: Add tests for rewards runtime API
3d1f684
@cdamian
nix: Update flake.nix
b173d35
@cdamian cdamian force-pushed the 1063-add-rewards-runtime-api branch from 38b6c2f to b173d35 Compare December 14, 2022 15:57
mikiquantum
mikiquantum approved these changes Dec 14, 2022
View reviewed changes
Copy link
Contributor

@mikiquantum mikiquantum left a comment

Choose a reason for hiding this comment

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

Awesome on the integration tests for runtime APIs!

@cdamian cdamian enabled auto-merge (squash) December 14, 2022 16:59
@cdamian cdamian merged commit 2260d6e into main Dec 14, 2022
@cdamian cdamian deleted the 1063-add-rewards-runtime-api branch December 14, 2022 17:07
@mikiquantum mikiquantum mentioned this pull request Dec 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikiquantum mikiquantum mikiquantum approved these changes

@NunoAlexandre NunoAlexandre NunoAlexandre left review comments

@lemunozm lemunozm lemunozm left review comments

Assignees

@cdamian cdamian

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cdamian @NunoAlexandre @mikiquantum @lemunozm @mustermeiszer