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

feat: fixed accounting bug and refactoring #767

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

shotaronowhere
Copy link

There was an accounting bug when updating params this is fixed.

I'm comfortable with the refactor in #766 , so i'm building off of that and just keeping this branch separate until there's another review. If we toss out the #766 refactor I can migrate the fix.

  • simplified the naming of totalCharge to chargePerProof
  • removed an "optimization" in feat: stakeroot cleanup #766 of copying storage to memory to avoid multiple sloads, but with some profiling the gas cost of memory copying doesn't make up for fewer reads from the same slot. In general KISS. Let's not over optimize here. I understand avoiding reading from new slots and storage packing, but reading the same slot is cheap. better not to sweat the small stuff
  • removed the assembly and replaced with readable code

Copy link
Member

@0xClandestine 0xClandestine left a comment

Choose a reason for hiding this comment

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

lgtm

@shotaronowhere shotaronowhere merged commit 3dd31a5 into feat-stakeroot-cleanup Sep 18, 2024
12 of 16 checks passed
@shotaronowhere shotaronowhere deleted the feat-stakeroot-more-cleanup branch September 18, 2024 19:20
shotaronowhere added a commit that referenced this pull request Sep 19, 2024
* 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 20, 2024
* 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
* 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
* 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants