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

cli: Add rewards:show #2160

Merged
merged 64 commits into from
Dec 26, 2019
Merged

cli: Add rewards:show #2160

merged 64 commits into from
Dec 26, 2019

Conversation

jfoutts-celo
Copy link
Contributor

@jfoutts-celo jfoutts-celo commented Dec 10, 2019

Description

Add rewards:show command that shows rewards for the last N epochs.
If --address is supplied: voter, validator, and validator group rewards applying to address are filtered.

Tested

Tested that rewards are correctly displayed by the CLI on my testnet.

Related issues

@codecov
Copy link

codecov bot commented Dec 10, 2019

Codecov Report

Merging #2160 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #2160    +/-   ##
========================================
  Coverage   74.66%   74.66%            
========================================
  Files         274      274            
  Lines        7732     7732            
  Branches      701      985   +284     
========================================
  Hits         5773     5773            
  Misses       1847     1847            
  Partials      112      112
Flag Coverage Δ
#mobile 74.66% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ae5627...3a21f7e. Read the comment docs.

Copy link
Contributor

@nategraf nategraf left a comment

Choose a reason for hiding this comment

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

General note before I did into this again. We really want to keep type safety, so any should be used sparingly. contractkit especially has gone to great lengths for type safety. A lot of the helper functions you introduce here break type safety by using any and you'll need to refactor it to to avoid that. Honestly I find too many helpers makes code hard to read, and I think many of these could be cut because they are only needed in one place anyway.

packages/cli/src/commands/rewards/show.ts Outdated Show resolved Hide resolved
packages/cli/src/commands/rewards/show.ts Outdated Show resolved Hide resolved
packages/cli/src/commands/rewards/show.ts Outdated Show resolved Hide resolved
@nategraf nategraf removed their assignment Dec 11, 2019
@jmrossy jmrossy removed their request for review December 16, 2019 18:17
@nategraf nategraf requested review from nategraf and removed request for nategraf December 20, 2019 02:15
@nategraf nategraf dismissed their stale review December 20, 2019 02:16

Going on vacation

@nategraf
Copy link
Contributor

I'm going to be gone on vacation. I have no more concerns. Thanks for all the iterations on this! 👍

const validators = await this.contracts.getValidators()
const epochSize = await validators.getEpochSize()
const currentBlock = await this.web3.eth.getBlockNumber()
return Math.floor(currentBlock / epochSize)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this correct? I recall the conversation where you stated that it should be:
epochNumber(blockNumber) => blockNumber / epochSize + (blockNumber % epochSize != 0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to call smart contract getEpochNumber(). Note that definition ended up being off by one. I'm going off the definition in master UsingPrecompiles.

packages/contractkit/src/wrappers/Election.ts Outdated Show resolved Hide resolved
packages/contractkit/src/wrappers/Election.ts Show resolved Hide resolved
packages/contractkit/src/wrappers/Election.ts Outdated Show resolved Hide resolved
packages/contractkit/src/wrappers/Election.ts Outdated Show resolved Hide resolved
packages/contractkit/src/wrappers/Election.ts Outdated Show resolved Hide resolved
packages/contractkit/src/kit.ts Outdated Show resolved Hide resolved
packages/contractkit/src/wrappers/Validators.ts Outdated Show resolved Hide resolved
packages/contractkit/src/wrappers/Validators.ts Outdated Show resolved Hide resolved
packages/utils/src/async.ts Outdated Show resolved Hide resolved
@mcortesi mcortesi removed their assignment Dec 20, 2019
Copy link
Contributor

@mcortesi mcortesi left a comment

Choose a reason for hiding this comment

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

Great Work! Thanks for all the changes!

@mcortesi mcortesi merged commit c66548d into master Dec 26, 2019
@mcortesi mcortesi deleted the jfoutts/cli-show-rewards branch December 26, 2019 12:23
aaronmgdr added a commit that referenced this pull request Jan 2, 2020
* master: (26 commits)
  Added new lint rule (#2349)
  [Slashing] Slash locked gold (#2317)
  [Slashing] Allow voting gold to be slashable (#2302)
  1666 precompiles assembly rewrite (#2324)
  Small fixes to deploy web (#2343)
  Governance, downtime and double signing slasher contracts (#2267)
  Added daily limit for reserve spending (#2303)
  Fixing multisig tests (#2342)
  [Wallet] Implement new send & import flow (#2332)
  Limechain/16xx (#1982)
  Add infinite pagination to Leaderboard (#2339)
  cli: Add rewards:show (#2160)
  Correct broken link formatting
  Celotool command for deploying hotfixes (#2142)
  Governance ContractKit + CLI changes (#2139)
  Complete codependent slashing precompile PRs (#2333)
  Add new and modified precompiles to UsingPrecompiles.sol (#2318)
  Adding error messages to contracts (#1182)
  Upgrade i18next and react-i18next to latest across monorepo (#2311)
  [Wallet] Fix exchange input on iOS and feed item display (#2319)
  ...

# Conflicts:
#	yarn.lock
aaronmgdr added a commit that referenced this pull request Jan 2, 2020
* master: (35 commits)
  Remove rep sentence from brand kit page (#2350)
  Added new lint rule (#2349)
  [Slashing] Slash locked gold (#2317)
  [Slashing] Allow voting gold to be slashable (#2302)
  1666 precompiles assembly rewrite (#2324)
  Small fixes to deploy web (#2343)
  Governance, downtime and double signing slasher contracts (#2267)
  Added daily limit for reserve spending (#2303)
  Fixing multisig tests (#2342)
  [Wallet] Implement new send & import flow (#2332)
  Limechain/16xx (#1982)
  Add infinite pagination to Leaderboard (#2339)
  cli: Add rewards:show (#2160)
  Correct broken link formatting
  Celotool command for deploying hotfixes (#2142)
  Governance ContractKit + CLI changes (#2139)
  Complete codependent slashing precompile PRs (#2333)
  Add new and modified precompiles to UsingPrecompiles.sol (#2318)
  Adding error messages to contracts (#1182)
  Upgrade i18next and react-i18next to latest across monorepo (#2311)
  ...
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.

Validators and Groups SBAT see payments received
4 participants