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

Reward pallet RPC #705

Merged
merged 5 commits into from
Sep 1, 2022
Merged

Reward pallet RPC #705

merged 5 commits into from
Sep 1, 2022

Conversation

theodorebugnet
Copy link
Contributor

@theodorebugnet theodorebugnet commented Aug 25, 2022

A few questions I'm blocked on at this point:

  1. Build failure. This looks very similar to what I was getting when failing to implement the RPC in the runtimes, but here I've definitely added it to all four parachain runtimes and the standalone. Could you take a look please?
  2. See comment below regarding possibly cleaning up the cast into Balance
  3. We have two instances of the reward pallet, once using VaultId as the RewardId for vault block rewards, one using AccountId for escrow rewards. Right now I'm just trying to get the VaultId one to work for starters, as it's also going to be the main one useful for simplifying the lib, but I think it would be good to have both available as RPCs - how would I define that? The naive way of just adding a second impl to the runtimes doesn't work, and I'm not familiar enough with the inner workings of substrate to figure out how to approach this.

Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
@gregdhill
Copy link
Member

  1. Missing trait bound on RuntimeApiCollection - see this commit.
  2. Missing cast on pallet to Rewards trait - see this commit.
  3. Split compute_reward on RewardApi into compute_escrow_reward and compute_vault_reward - see this commit.

@theodorebugnet theodorebugnet marked this pull request as ready for review August 26, 2022 10:59
@theodorebugnet theodorebugnet enabled auto-merge (squash) August 26, 2022 11:05
Copy link
Member

@sander2 sander2 left a comment

Choose a reason for hiding this comment

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

Minor nits. Did you test this? When adding a new pallet to the rpcs, it's easy to miss required additions in all of the boilerplate

crates/reward/rpc/src/lib.rs Outdated Show resolved Hide resolved
crates/reward/rpc/src/lib.rs Outdated Show resolved Hide resolved
@theodorebugnet theodorebugnet merged commit 9f37048 into master Sep 1, 2022
@gregdhill gregdhill deleted the theo/rewards-rpc branch September 1, 2022 07:26
@sander2 sander2 added this to the Release 1.19 milestone Sep 7, 2022
@gregdhill gregdhill mentioned this pull request Jan 18, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants