Skip to content

[Part2] Finish RootChainStakingPool#101

Open
FreshmanQ wants to merge 16 commits intomasterfrom
rc_posw
Open

[Part2] Finish RootChainStakingPool#101
FreshmanQ wants to merge 16 commits intomasterfrom
rc_posw

Conversation

@FreshmanQ
Copy link
Contributor

No description provided.

@FreshmanQ FreshmanQ requested a review from a team October 6, 2020 07:33
Copy link
Contributor

@qizhou qizhou left a comment

Choose a reason for hiding this comment

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

I think we need to add a general description of how it works such as requirements. Further, I feel the code lacks necessary comments.

@qizhou
Copy link
Contributor

qizhou commented Oct 12, 2020

In general, it should be self-contained - a reader without too much context can understand the code after reading the comments.

@FreshmanQ
Copy link
Contributor Author

FreshmanQ commented Oct 12, 2020

Gas consumption of two contracts, RootChainStakingPool saves about 150000 gas for one loop.

Contract: RootChainStakingPool
    ✓ should deploy correctly
    ✓ should work with minStakes correctly (180870 gas)
    ✓ should handle adding stakes properly (151872 gas)
    ✓ should handle withdrawing stakes properly (233731 gas)
    ✓ should calculate dividends correctly (402467 gas)
    ✓ should accept new stakers (614927 gas)
    ✓ should handle no staker case (347180 gas)
    ✓ should allow admin to update fee rate (73340 gas)
    ✓ should handle maintainer fee correctly (2319969 gas)
    ✓ should handle no staker case with pool maintainer (2117808 gas)
    ✓ should update miner and admin contact infomation correctly (116458 gas)
    ✓ should update admin and pool maintainer correctly (105154 gas)

  Contract: StakingPool
    ✓ should deploy correctly
    ✓ should work with minStakes correctly (197832 gas)
    ✓ should handle adding stakes properly (204512 gas)
    ✓ should handle withdrawing stakes properly (239672 gas)
    ✓ should calculate dividends correctly (366392 gas)
    ✓ should accept new stakers (527661 gas)
    ✓ should handle no staker case (366006 gas)
    ✓ should allow admin to update fee rate (73274 gas)
    ✓ should handle maintainer fee correctly (2462598 gas)
    ✓ should handle no staker case with pool maintainer (2255520 gas)
    ✓ should update miner and admin contact infomation correctly (116414 gas)
    ✓ should update admin and pool maintainer correctly (105110 gas)

·--------------------------------------------------------------|---------------------------|--------------|----------------------------·
|             Solc version: 0.5.8+commit.23d335f2              ·  Optimizer enabled: true  ·  Runs: 9999  ·  Block limit: 6721975 gas  │
·······························································|···························|··············|·····························
|  Methods                                                                                                                             │
··································|····························|·············|·············|··············|··············|··············
|  Contract                       ·  Method                    ·  Min        ·  Max        ·  Avg         ·  # calls     ·  eur (avg)  │
··································|····························|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager      ·  depositGasReserve         ·          -  ·          -  ·       27577  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager      ·  payAsGas                  ·      35459  ·      50459  ·       47459  ·           5  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager      ·  proposeNewExchangeRate    ·      40728  ·      85395  ·       64853  ·           8  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager      ·  requireTokenRegistration  ·          -  ·          -  ·       13551  ·           4  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager      ·  setFrozen                 ·          -  ·          -  ·       42209  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager      ·  setMinGasReserve          ·          -  ·          -  ·       28078  ·           6  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager      ·  setRefundPercentage       ·          -  ·          -  ·       27643  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager      ·  withdrawGasReserve        ·      19955  ·      20249  ·       20053  ·           3  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  Migrations                     ·  setCompleted              ·          -  ·          -  ·       41927  ·           4  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  accelerate                ·      47653  ·      68185  ·       52941  ·           4  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  bidNewToken               ·      41991  ·     127828  ·      102507  ·          16  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  changeAccelerator         ·      13565  ·      28410  ·       20988  ·           2  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  endAuction                ·          -  ·          -  ·       42573  ·           8  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  pauseAuction              ·          -  ·          -  ·       26954  ·           3  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  resumeAuction             ·      13720  ·      27440  ·       18418  ·           8  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  setAuctionParams          ·          -  ·          -  ·       28090  ·           5  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  whitelistTokenId          ·      13698  ·      42459  ·       28079  ·           2  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager  ·  withdraw                  ·          -  ·          -  ·       19590  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  RootChainStakingPool           ·  adjustMinerFeeRate        ·          -  ·          -  ·       29006  ·           2  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  RootChainStakingPool           ·  transferMaintainerFee     ·          -  ·          -  ·       21144  ·           2  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  RootChainStakingPool           ·  updateAdmin               ·          -  ·          -  ·       28408  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  RootChainStakingPool           ·  updateAdminContactInfo    ·          -  ·          -  ·       34327  ·           2  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  RootChainStakingPool           ·  updateMinerContactInfo    ·          -  ·          -  ·       34371  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  RootChainStakingPool           ·  updatePoolMaintainer      ·          -  ·          -  ·       30145  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  RootChainStakingPool           ·  withdrawMinerReward       ·      21165  ·      90081  ·       38394  ·           4  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  RootChainStakingPool           ·  withdrawStakes            ·      35123  ·      94355  ·       51535  ·           7  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  SelfDestruct                   ·  forceSend                 ·      13893  ·      13925  ·       13923  ·          19  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  StakingPool                    ·  adjustMinerFeeRate        ·          -  ·          -  ·       28984  ·           2  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  StakingPool                    ·  transferMaintainerFee     ·          -  ·          -  ·       21144  ·           2  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  StakingPool                    ·  updateAdmin               ·          -  ·          -  ·       28386  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  StakingPool                    ·  updateAdminContactInfo    ·          -  ·          -  ·       34305  ·           2  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  StakingPool                    ·  updateMinerContactInfo    ·          -  ·          -  ·       34371  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  StakingPool                    ·  updatePoolMaintainer      ·          -  ·          -  ·       30145  ·           1  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  StakingPool                    ·  withdrawMinerReward       ·      21143  ·      76210  ·       34910  ·           4  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  StakingPool                    ·  withdrawStakes            ·      38763  ·      58829  ·       42437  ·           7  ·          -  │
··································|····························|·············|·············|··············|··············|··············
|  Deployments                                                 ·                                          ·  % of limit  ·             │
·······························································|·············|·············|··············|··············|··············
|  GeneralNativeTokenManager                                   ·          -  ·          -  ·     2295370  ·      34.1 %  ·          -  │
·······························································|·············|·············|··············|··············|··············
|  Migrations                                                  ·          -  ·          -  ·      209550  ·       3.1 %  ·          -  │
·······························································|·············|·············|··············|··············|··············
|  NonReservedNativeTokenManager                               ·          -  ·          -  ·     2470057  ·      36.7 %  ·          -  │
·······························································|·············|·············|··············|··············|··············
|  RootChainStakingPool                                        ·    1991092  ·    2006220  ·     1993109  ·      29.7 %  ·          -  │
·······························································|·············|·············|··············|··············|··············
|  SelfDestruct                                                ·          -  ·          -  ·       97663  ·       1.5 %  ·          -  │
·······························································|·············|·············|··············|··············|··············
|  StakingPool                                                 ·    2128804  ·    2143932  ·     2130821  ·      31.7 %  ·          -  │
·--------------------------------------------------------------|-------------|-------------|--------------|--------------|-------------·

Copy link
Contributor

@ninjaahhh ninjaahhh left a comment

Choose a reason for hiding this comment

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

Gas consumption of two contracts, RootChainStakingPool saves about 150000 gas for one loop.

what are you comparing with?

@FreshmanQ
Copy link
Contributor Author

Gas consumption of two contracts, RootChainStakingPool saves about 150000 gas for one loop.

what are you comparing with?

@FreshmanQ FreshmanQ closed this Oct 12, 2020
@FreshmanQ FreshmanQ reopened this Oct 12, 2020
@FreshmanQ
Copy link
Contributor Author

Gas consumption of two contracts, RootChainStakingPool saves about 150000 gas for one loop.

what are you comparing with?

compare the same test of two contracts, like handle maintainer fee correctly

@FreshmanQ FreshmanQ changed the title Add RootChainStakingPool contract & test [Part2] Finish RootChainStakingPool Oct 13, 2020
@FreshmanQ FreshmanQ changed the base branch from master to rc_posw_part1 October 13, 2020 00:39
Base automatically changed from rc_posw_part1 to master October 14, 2020 00:43
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.

3 participants