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

Internal Audit #22

Closed
wants to merge 193 commits into from
Closed

Internal Audit #22

wants to merge 193 commits into from

Conversation

nxqbao
Copy link
Contributor

@nxqbao nxqbao commented Sep 30, 2022

Review Progress:

  • Staking
  • StakingManager
  • RewardCalculation: Partially. The formulae are not reviewed.
  • StakingVesting
  • CandidateManager
  • ValidatorSet
  • Maintainance
  • Slashing

import "../../precompile-usages/PrecompileUsagePickValidatorSet.sol";
import "./CandidateManager.sol";

contract RoninValidatorSet is
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this contract support withdraw function to withdraw jailed reward?

ducthotran2010 and others added 3 commits November 2, 2022 14:43
* Add draft contract for credit score and bail out

* Fix maintenance at current period

* Update comment

* Fix check in maintainingAtCurrentPeriod

* Replace maintainingAtCurrentPeriod by maintainingInBlockRange

* Refactor credit score to an abstract contract

* Fix bailout function

* Refactor test for slash

* Fix contract

* Refactor slash indicator test

* Add first test

* Fix fixture id

* Add more test for credit score

* Fix contracts

* Fix test helper. Add more tests.

* Add jailedAtBlock methods

* Fix test

* Fix rebase error. Update contracts.

* Fix contracts

* Fix credit score test

* Fix test

* Refactor _setUnavailabilityIndicator method

* Change fixture name

* Fix docs

* Add gap for CreditScore contract

* Mark helper contracts of SlashIndicator as abstract

* Refactor slash indicator interfaces

* Update contracts/interfaces/slash-indicator/ICreditScore.sol

Co-authored-by: Duc Tho Tran <ducthotran2010@gmail.com>

* Update contracts/interfaces/slash-indicator/ICreditScore.sol

Co-authored-by: Duc Tho Tran <ducthotran2010@gmail.com>

* Update test/helpers/ronin-validator-set.ts

Co-authored-by: Duc Tho Tran <ducthotran2010@gmail.com>

* Update contracts/libraries/Math.sol

Co-authored-by: Duc Tho Tran <ducthotran2010@gmail.com>

* Update contracts/libraries/Math.sol

Co-authored-by: Duc Tho Tran <ducthotran2010@gmail.com>

* Add comments

Co-authored-by: nxqbao <quocbao300@gmail.com>
* Merge two bonus requests bonus into one

* Rename in tests

* Rename deploy scripts

* Use unsafeSendRON for staking vesting

* Add tests

* Fix rebase

* Add contract balance for transfer failed event

* Handle no request bonus for deprecated block producer
Co-authored-by: Bao <quocbao300@gmail.com>
* Update reward calculation mechanism

* Change balance to stakingAmount

* Fix rebase issues

* Update pool shares syncing mechanism
Copy link
Collaborator

@ducthotran2010 ducthotran2010 left a comment

Choose a reason for hiding this comment

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

Add reserved space is put in place to allow future versions to add new variables without shifting down storage in the inheritance chain

contracts/ronin/staking/StakingManager.sol Show resolved Hide resolved
contracts/ronin/staking/RewardCalculation.sol Show resolved Hide resolved
@ducthotran2010
Copy link
Collaborator

ducthotran2010 commented Nov 7, 2022

The discussions put in this PR this quite hard to follow. If any further bugs/issues are found or something you need to discuss please make it an issue.

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.

4 participants