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

feat: stakeroot cleanup #766

Merged
merged 20 commits into from
Sep 19, 2024
Merged

feat: stakeroot cleanup #766

merged 20 commits into from
Sep 19, 2024

Conversation

0xClandestine
Copy link
Member

No description provided.

@0xClandestine
Copy link
Member Author

added some todo comments

@0xClandestine
Copy link
Member Author

need help renaming

@0xClandestine 0xClandestine self-assigned this Sep 18, 2024
@shotaronowhere shotaronowhere mentioned this pull request Sep 18, 2024
Merged
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 18, 2024
View reviewed changes
src/contracts/core/StakeRootCompendiumStorage.sol
uint256 public immutable MIN_PREPAID_PROOFS;

/// @notice the total number of strategies among all operator sets (with duplicates)
uint256 public totalStrategies;

Check warning

Code scanning / Slither

State variables that could be declared constant

StakeRootCompendiumStorage.totalStrategies (src/contracts/core/StakeRootCompendiumStorage.sol#37) should be constant
src/contracts/core/StakeRootCompendiumStorage.sol
/// @dev Contains info about cumulative charges.
CumulativeChargeParams public cumulativeChargeParams;
/// @notice deposit balance to be deducted for operatorSets
mapping(address => mapping(uint32 => DepositInfo)) public depositInfos;

Check failure

Code scanning / Slither

Uninitialized state variables

StakeRootCompendiumStorage.depositInfos (src/contracts/core/StakeRootCompendiumStorage.sol#56) is never initialized. It is used in: - StakeRootCompendium.deposit(OperatorSet) (src/contracts/core/StakeRootCompendium.sol#63-97)
gpsanant
gpsanant reviewed Sep 18, 2024
View reviewed changes
src/contracts/core/StakeRootCompendium.sol

return depositInfo.balance > pendingCharge ? depositInfo.balance - pendingCharge : 0;
}

/// @inheritdoc IStakeRootCompendium
function proofIntervalSeconds() external view returns (uint32) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just delete this fn?

Copy link
Contributor

Choose a reason for hiding this comment

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

add memory returning ones for params?

Copy link

Choose a reason for hiding this comment

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

I left it in so proof submitters can easily query the proof submission rate. I think we will add some events for parameter changes. can leave this in for now?

github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 19, 2024
View reviewed changes
src/contracts/core/StakeRootCompendium.sol

function initialize(
address _owner,
address _rootConfirmer,

Check notice

Code scanning / Slither

Missing zero address validation

StakeRootCompendium.initialize(address,address,uint32,IStakeRootCompendium.ChargeParams)._rootConfirmer (src/contracts/core/StakeRootCompendium.sol#36) lacks a zero-check on : - rootConfirmer = _rootConfirmer (src/contracts/core/StakeRootCompendium.sol#43)
0xClandestine and others added 17 commits September 19, 2024 09:27
@shotaronowhere shotaronowhere merged commit 0546a0a into stake-root Sep 19, 2024
7 of 12 checks passed
@shotaronowhere shotaronowhere deleted the feat-stakeroot-cleanup branch September 19, 2024 16:34
gpsanant pushed a commit that referenced this pull request Sep 20, 2024
@0xClandestine @shotaronowhere @gpsanant
feat: stakeroot cleanup (#766)
97a8874 
* chore: forge fmt src/contracts

* nit: organize imports

* fix: bump pragma -> ^0.8.27

* refactor(optimization): significantly reduce sloads/sstores

* refactor: custom errors

* refactor(optimization): significantly reduce sloads/sstores

missed some stuff

* refactor: rename compendium -> manager

* refactor: more storage optmizations

* feat: safe eth transfer helper

* refactor: manager -> compendium

* refactor: variable renaming

* refactor: rename compendium -> manager

* feat: add `proofIntervalSeconds` getter

* feat: fixed accounting bug and refactoring (#767)

* refactor: review reconciliations

* refactor: review reconciliations

* fix: rename colluding param

* fix: types and naming

* fix: revert stakeroot calculation changes

* fix: revert stakeRoot calc views

---------

Co-authored-by: shotaro <10378902+shotaronowhere@users.noreply.github.com>
gpsanant pushed a commit that referenced this pull request Sep 21, 2024
@0xClandestine @shotaronowhere @gpsanant
feat: stakeroot cleanup (#766)
89962e6 
* chore: forge fmt src/contracts

* nit: organize imports

* fix: bump pragma -> ^0.8.27

* refactor(optimization): significantly reduce sloads/sstores

* refactor: custom errors

* refactor(optimization): significantly reduce sloads/sstores

missed some stuff

* refactor: rename compendium -> manager

* refactor: more storage optmizations

* feat: safe eth transfer helper

* refactor: manager -> compendium

* refactor: variable renaming

* refactor: rename compendium -> manager

* feat: add `proofIntervalSeconds` getter

* feat: fixed accounting bug and refactoring (#767)

* refactor: review reconciliations

* refactor: review reconciliations

* fix: rename colluding param

* fix: types and naming

* fix: revert stakeroot calculation changes

* fix: revert stakeRoot calc views

---------

Co-authored-by: shotaro <10378902+shotaronowhere@users.noreply.github.com>
ypatil12 pushed a commit that referenced this pull request Sep 24, 2024
@0xClandestine @shotaronowhere @ypatil12
feat: stakeroot cleanup (#766)
4b56b8c 
* chore: forge fmt src/contracts

* nit: organize imports

* fix: bump pragma -> ^0.8.27

* refactor(optimization): significantly reduce sloads/sstores

* refactor: custom errors

* refactor(optimization): significantly reduce sloads/sstores

missed some stuff

* refactor: rename compendium -> manager

* refactor: more storage optmizations

* feat: safe eth transfer helper

* refactor: manager -> compendium

* refactor: variable renaming

* refactor: rename compendium -> manager

* feat: add `proofIntervalSeconds` getter

* feat: fixed accounting bug and refactoring (#767)

* refactor: review reconciliations

* refactor: review reconciliations

* fix: rename colluding param

* fix: types and naming

* fix: revert stakeroot calculation changes

* fix: revert stakeRoot calc views

---------

Co-authored-by: shotaro <10378902+shotaronowhere@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shotaronowhere shotaronowhere shotaronowhere left review comments

@gpsanant gpsanant gpsanant left review comments

Assignees

@0xClandestine 0xClandestine

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@0xClandestine @shotaronowhere @gpsanant