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

N02 gas inefficiencies #1786

Conversation

sparrowDom
Copy link
Member

@sparrowDom sparrowDom commented Aug 25, 2023

notes:

  • I haven't converted the function signature of internal functions (_deposit, withdrawal, getBPTExpected) to use calldata. Since other functions can call those with in memory constructed arrays that can not be converted to calldata.

Have also tested if storing memory array length into a uint256 provides any gas gains. Have ran it twice, with reading length each time inside the loop and storing the length separately. There seemed to be no differences. If the array would be a storage type array, then the gas gains should be more noticeable. In both cases the min/max/average gas usage were the same:

  • depositToStrategy · 1294871 · 1320278 · 1307819

From audit report:
There are several places across the codebase where changes can be made to improve gas
consumption.
The amount = 0; assignment is unnecessary.
There is another, more efficient version of the fromPoolAsset function that can be
used when the amount is not needed. Consider switching to it instead.
The assetAmount variable is unused, consider removing it.
In the withdraw function, the conversion to a pool asset depends only on
poolAssets[i] . Consider doing it only once per pool asset, outside of the for loop
that goes through the strategy assets.
The asset variable can be reused here, to avoid another array access through
_assets[i] .
There are several places where function parameters could be marked as calldata ,
consider changing them for future gas optimizations: deposit , _deposit ,
withdraw , _withdraw , and getBPTExpected .
There are multiple instances of an array's length being accessed on every iteration of a
loop. This can be made more efficient by saving off the length of the array in a local
variable and using that in the for loop instead.
When performing these changes, aim to reach an optimal trade-off between gas optimization
and readability. Having a codebase that is easy to understand reduces the chance of errors in
the future and improves transparency for the community.

@rafaelugolini rafaelugolini temporarily deployed to preview-ousd-sparrowdom-vdqdec August 25, 2023 11:25 Inactive
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-sparrowdom-n6jjoc August 25, 2023 11:25 Inactive
Copy link
Collaborator

@naddison36 naddison36 left a comment

Choose a reason for hiding this comment

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

Looks good

@sparrowDom sparrowDom merged commit 852afa4 into sparrowDom/balancer-sfrxETH-stETH-rETH Aug 30, 2023
6 of 9 checks passed
@sparrowDom sparrowDom deleted the sparrowDom/N02-gas-optimisation branch August 30, 2023 20:15
sparrowDom added a commit that referenced this pull request Sep 21, 2023
* initail commit

* intermediary commit

* commit research files

* balancer booster abi

* intermittent commit

* add base balancer contract that implements checkBalance functionality

* add some additional initial integration

* intermittent commit

* add deployment file

* add fork test fixture

* intermittent commit

* prettier

* add basic withdrawal / deposit functionality

* correct the BPT calculation

* prettier + lint

* simplify the BPT price calculation

* add read-only re-entrancy protection

* add some missing tests and adjust existing. Deparate deposit and withdrawal slippage

* fix check balance implementation

* Balancer review changes (#1726)

* Generated contract docs

* Refactor Balancer storage variables

* Small Balancer changes

* Natspec updates
Added missing licensing
getPoolAssets gas optimized

* Updated generated Balancer strategy contract diagrams

* fix contract linter

* Removed restrictions on tests

* Small gas improvements
Fixed Slither

* Change BalancerError version

* Updated constant names
Addresses to use checksum format

* JS lint tasks

* Updated Balancer and Aura pool id constants

* Removed getRateProviderRate as it wasn't being used
Refactored to remove poolAssetsMapped storage variable

* Updated OETH Contracts diagrams
Generated new Balancer contract diagrams

* Fix failing test

* Fix merge conflict

* Restored getRateProviderRate

* Natspec updates
Added toPoolAsset override

* Removed unused getRateProviderRate

* Natspec updates
Gas optimization of InitializableAbstractStrategy

* Abstract strategy gas improvements (#1719)

* Refactor base strategy to use immutables

* Fixed strategy deployments in 001_core and fixtures

* Generated new strategy diagrams

* Deploy rETH instead of the stETH Balancer MetaStable Pool

* removed unused Aura config

* Balancer fork tests

* Added check that BPT amount equals Aura LP amount
Added rETH conversion to ETH value

* Updated balancer strat fork tests

* Updated Balancer fork tests

* Added optional deposit with multiple assets to the strategy

* Single asset deposit to use multi asset deposit

* Added optional checkBalance to Balancer strategy

* Added checkBalance() to BaseBalancerStrategy

* Fix slither
Fix curve HH task

* Added multi-asset withdraw to balancer strategy

* Fix multi-asset withdraw

* Updated Balancer and Vault diagrams

* Fix js linter

* Fixed checkBalance of rETH asset in Balancer strategy

* Only wrap assets if amount > 0
Added depositAll fork test for Balancer strat

* Removed Vault changes for multi-asset strategy support

* Updated generated docs

* Add tests for wstETH/WETH Balancer pool (#1725)

* Split deployment and fix fixtures

* Deposit tests for wstETH/WETH pool

* Add withdraw test

* prettier

* remove .only in fork tests

---------

Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>

* [ DFD-1 ] Balancer's checkBalance (#1730)

* add alternative implementation of Balancer's checkBalance

* correct the checkBalance implementation

* Balancer fork tests (#1727)

* Added large withdraw tests for Balancer strategy

* fix test log

* Balancer withdraw to handle close to BPT limit

* Small change to Balancer withdraw fork test

* add some comments

* change implementation

---------

Co-authored-by: Domen Grabec <grabec@gmail.com>

* Add read-only reentrancy test (#1731)

* Added large withdraw tests for Balancer strategy

* fix test log

* Balancer withdraw to handle close to BPT limit

* Small change to Balancer withdraw fork test

* add some comments

* Add test for read-only reentrancy

* add reentrancy protection to checkBalance functions

* add documentation

* remove the only

---------

Co-authored-by: Nicholas Addison <nick@addisonbrown.com.au>
Co-authored-by: Domen Grabec <grabec@gmail.com>

* Balancer fixes (#1734)

* prettier

* adjust how checkBalance is calculated

* Balancer withdrawal fix (#1739)

* fix balancer withdrawals

* lint

* prettier

* use only 1 safeApprove when applicable

* some renames and more correct application of approves

* renames, additional requires, move initializer to a better location, slither

* bug fix

* Generated latest Balancer strategy diagrams

* re-deploy BPT tokens sitting in the strategy

* fix re-entrancy test

* fixture fix

* bug fix

* prettier

* L02 improve naming  (#1783)

* improve naming

* one more rename

* buf fix

* do a check that supported assets are being withdrawn (#1784)

* set uint256 max instead of magic number (#1782)

* remove unused files (#1785)

* fix renaming bug

* correct safe approve all tokens and adjust the documentation (#1776)

* prettier

* M04 - minBptFunction robustness (#1795)

* change bptExpected to ignore Oracle prices and assume assets are always pegged. (Vault value checker shall be used to mitigate issues with said assumption)

* fix all the tests

* add test for pool manipulation

* prettier & lint

* minor change

* add withdrawal test

* update documentation

* pick string error length that is smaller than 32 characters

* prettier

* correct comment

* better comments

* prettier

* M02 withdrawal fuzzing tests (#1801)

* add more withdrawal tests

* add more withdrawal cases

* N02 gas inefficiencies  (#1786)

* gas optimisation

* fix bad merge and prettier

* remove todo comments (#1796)

* use a more appropriate array initialisation length (#1800)

* more consistant function naming (#1797)

* fix typo (#1798)

* simplify the way we withdrawAll. no need to pass along min amonts (#1777)

* M03 - missing or misleading documentation (#1781)

* improve documentation

* add comma

* M01 More comprehensive test suite (#1780)

* add tests for approving tokens and harvesting rewards

* prettier and lint

* fix bad merge + prettier & lint

* fix fork tests remove .only

* remove only

* lint

* fix unit tests

* add more tests to see how checkBalance behaves

* remove console log

* improve checkBalance test by testing that checkBalance amount doesn't decrease after the attack comaring to the middle of the attack.

* confirm that yield gained by 3rd party tilting the pool can be extracted by withdrawing the funds

* rename internal functions by prepending them with underscore

* Generated latest Balancer strategy diagrams (#1805)

* bug fix

* bug fix

* Minor Balancer changes from final review (#1819)

* Removed unused imports

* Generated updated contract diagram

---------

Co-authored-by: Nick Addison <nick@addisonbrown.com.au>
Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>
sparrowDom added a commit that referenced this pull request Sep 27, 2023
* initail commit

* intermediary commit

* commit research files

* balancer booster abi

* intermittent commit

* add base balancer contract that implements checkBalance functionality

* add some additional initial integration

* intermittent commit

* add deployment file

* add fork test fixture

* intermittent commit

* prettier

* add basic withdrawal / deposit functionality

* correct the BPT calculation

* prettier + lint

* simplify the BPT price calculation

* add read-only re-entrancy protection

* add some missing tests and adjust existing. Deparate deposit and withdrawal slippage

* fix check balance implementation

* Balancer review changes (#1726)

* Generated contract docs

* Refactor Balancer storage variables

* Small Balancer changes

* Natspec updates
Added missing licensing
getPoolAssets gas optimized

* Updated generated Balancer strategy contract diagrams

* fix contract linter

* Removed restrictions on tests

* Small gas improvements
Fixed Slither

* Change BalancerError version

* Updated constant names
Addresses to use checksum format

* JS lint tasks

* Updated Balancer and Aura pool id constants

* Removed getRateProviderRate as it wasn't being used
Refactored to remove poolAssetsMapped storage variable

* Updated OETH Contracts diagrams
Generated new Balancer contract diagrams

* Fix failing test

* Fix merge conflict

* Restored getRateProviderRate

* Natspec updates
Added toPoolAsset override

* Removed unused getRateProviderRate

* Natspec updates
Gas optimization of InitializableAbstractStrategy

* Abstract strategy gas improvements (#1719)

* Refactor base strategy to use immutables

* Fixed strategy deployments in 001_core and fixtures

* Generated new strategy diagrams

* Deploy rETH instead of the stETH Balancer MetaStable Pool

* removed unused Aura config

* Balancer fork tests

* Added check that BPT amount equals Aura LP amount
Added rETH conversion to ETH value

* Updated balancer strat fork tests

* Updated Balancer fork tests

* Added optional deposit with multiple assets to the strategy

* Single asset deposit to use multi asset deposit

* Added optional checkBalance to Balancer strategy

* Added checkBalance() to BaseBalancerStrategy

* Fix slither
Fix curve HH task

* Added multi-asset withdraw to balancer strategy

* Fix multi-asset withdraw

* Updated Balancer and Vault diagrams

* Fix js linter

* Fixed checkBalance of rETH asset in Balancer strategy

* Only wrap assets if amount > 0
Added depositAll fork test for Balancer strat

* Removed Vault changes for multi-asset strategy support

* Updated generated docs

* Add tests for wstETH/WETH Balancer pool (#1725)

* Split deployment and fix fixtures

* Deposit tests for wstETH/WETH pool

* Add withdraw test

* prettier

* remove .only in fork tests

---------

Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>

* [ DFD-1 ] Balancer's checkBalance (#1730)

* add alternative implementation of Balancer's checkBalance

* correct the checkBalance implementation

* Balancer fork tests (#1727)

* Added large withdraw tests for Balancer strategy

* fix test log

* Balancer withdraw to handle close to BPT limit

* Small change to Balancer withdraw fork test

* add some comments

* change implementation

---------

Co-authored-by: Domen Grabec <grabec@gmail.com>

* Add read-only reentrancy test (#1731)

* Added large withdraw tests for Balancer strategy

* fix test log

* Balancer withdraw to handle close to BPT limit

* Small change to Balancer withdraw fork test

* add some comments

* Add test for read-only reentrancy

* add reentrancy protection to checkBalance functions

* add documentation

* remove the only

---------

Co-authored-by: Nicholas Addison <nick@addisonbrown.com.au>
Co-authored-by: Domen Grabec <grabec@gmail.com>

* Balancer fixes (#1734)

* prettier

* adjust how checkBalance is calculated

* Balancer withdrawal fix (#1739)

* fix balancer withdrawals

* lint

* prettier

* use only 1 safeApprove when applicable

* some renames and more correct application of approves

* renames, additional requires, move initializer to a better location, slither

* bug fix

* Generated latest Balancer strategy diagrams

* re-deploy BPT tokens sitting in the strategy

* fix re-entrancy test

* fixture fix

* bug fix

* prettier

* L02 improve naming  (#1783)

* improve naming

* one more rename

* buf fix

* do a check that supported assets are being withdrawn (#1784)

* set uint256 max instead of magic number (#1782)

* remove unused files (#1785)

* fix renaming bug

* correct safe approve all tokens and adjust the documentation (#1776)

* prettier

* M04 - minBptFunction robustness (#1795)

* change bptExpected to ignore Oracle prices and assume assets are always pegged. (Vault value checker shall be used to mitigate issues with said assumption)

* fix all the tests

* add test for pool manipulation

* prettier & lint

* minor change

* add withdrawal test

* update documentation

* pick string error length that is smaller than 32 characters

* prettier

* correct comment

* better comments

* prettier

* M02 withdrawal fuzzing tests (#1801)

* add more withdrawal tests

* add more withdrawal cases

* N02 gas inefficiencies  (#1786)

* gas optimisation

* fix bad merge and prettier

* remove todo comments (#1796)

* use a more appropriate array initialisation length (#1800)

* more consistant function naming (#1797)

* fix typo (#1798)

* simplify the way we withdrawAll. no need to pass along min amonts (#1777)

* M03 - missing or misleading documentation (#1781)

* improve documentation

* add comma

* M01 More comprehensive test suite (#1780)

* add tests for approving tokens and harvesting rewards

* prettier and lint

* fix bad merge + prettier & lint

* fix fork tests remove .only

* remove only

* lint

* fix unit tests

* add more tests to see how checkBalance behaves

* remove console log

* improve checkBalance test by testing that checkBalance amount doesn't decrease after the attack comaring to the middle of the attack.

* confirm that yield gained by 3rd party tilting the pool can be extracted by withdrawing the funds

* rename internal functions by prepending them with underscore

* Generated latest Balancer strategy diagrams (#1805)

* bug fix

* bug fix

* Minor Balancer changes from final review (#1819)

* Removed unused imports

* Generated updated contract diagram

* Deploy Balancer Meta stable pool RETH strategy

* update deploy description

* fix typo

* add proposal Id to deploy script

* prettier

---------

Co-authored-by: Nick Addison <nick@addisonbrown.com.au>
Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>
sparrowDom added a commit that referenced this pull request Sep 27, 2023
* initail commit

* intermediary commit

* commit research files

* balancer booster abi

* intermittent commit

* add base balancer contract that implements checkBalance functionality

* add some additional initial integration

* intermittent commit

* add deployment file

* add fork test fixture

* intermittent commit

* prettier

* add basic withdrawal / deposit functionality

* correct the BPT calculation

* prettier + lint

* simplify the BPT price calculation

* add read-only re-entrancy protection

* add some missing tests and adjust existing. Deparate deposit and withdrawal slippage

* fix check balance implementation

* Balancer review changes (#1726)

* Generated contract docs

* Refactor Balancer storage variables

* Small Balancer changes

* Natspec updates
Added missing licensing
getPoolAssets gas optimized

* Updated generated Balancer strategy contract diagrams

* fix contract linter

* Removed restrictions on tests

* Small gas improvements
Fixed Slither

* Change BalancerError version

* Updated constant names
Addresses to use checksum format

* JS lint tasks

* Updated Balancer and Aura pool id constants

* Removed getRateProviderRate as it wasn't being used
Refactored to remove poolAssetsMapped storage variable

* Updated OETH Contracts diagrams
Generated new Balancer contract diagrams

* Fix failing test

* Fix merge conflict

* Restored getRateProviderRate

* Natspec updates
Added toPoolAsset override

* Removed unused getRateProviderRate

* Natspec updates
Gas optimization of InitializableAbstractStrategy

* Abstract strategy gas improvements (#1719)

* Refactor base strategy to use immutables

* Fixed strategy deployments in 001_core and fixtures

* Generated new strategy diagrams

* Deploy rETH instead of the stETH Balancer MetaStable Pool

* removed unused Aura config

* Balancer fork tests

* Added check that BPT amount equals Aura LP amount
Added rETH conversion to ETH value

* Updated balancer strat fork tests

* Updated Balancer fork tests

* Added optional deposit with multiple assets to the strategy

* Single asset deposit to use multi asset deposit

* Added optional checkBalance to Balancer strategy

* Added checkBalance() to BaseBalancerStrategy

* Fix slither
Fix curve HH task

* Added multi-asset withdraw to balancer strategy

* Fix multi-asset withdraw

* Updated Balancer and Vault diagrams

* Fix js linter

* Fixed checkBalance of rETH asset in Balancer strategy

* Only wrap assets if amount > 0
Added depositAll fork test for Balancer strat

* Removed Vault changes for multi-asset strategy support

* Updated generated docs

* Add tests for wstETH/WETH Balancer pool (#1725)

* Split deployment and fix fixtures

* Deposit tests for wstETH/WETH pool

* Add withdraw test

* prettier

* remove .only in fork tests

---------

Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>

* [ DFD-1 ] Balancer's checkBalance (#1730)

* add alternative implementation of Balancer's checkBalance

* correct the checkBalance implementation

* Balancer fork tests (#1727)

* Added large withdraw tests for Balancer strategy

* fix test log

* Balancer withdraw to handle close to BPT limit

* Small change to Balancer withdraw fork test

* add some comments

* change implementation

---------

Co-authored-by: Domen Grabec <grabec@gmail.com>

* Add read-only reentrancy test (#1731)

* Added large withdraw tests for Balancer strategy

* fix test log

* Balancer withdraw to handle close to BPT limit

* Small change to Balancer withdraw fork test

* add some comments

* Add test for read-only reentrancy

* add reentrancy protection to checkBalance functions

* add documentation

* remove the only

---------

Co-authored-by: Nicholas Addison <nick@addisonbrown.com.au>
Co-authored-by: Domen Grabec <grabec@gmail.com>

* Balancer fixes (#1734)

* prettier

* adjust how checkBalance is calculated

* Balancer withdrawal fix (#1739)

* fix balancer withdrawals

* lint

* prettier

* use only 1 safeApprove when applicable

* some renames and more correct application of approves

* renames, additional requires, move initializer to a better location, slither

* bug fix

* Generated latest Balancer strategy diagrams

* re-deploy BPT tokens sitting in the strategy

* fix re-entrancy test

* fixture fix

* bug fix

* prettier

* L02 improve naming  (#1783)

* improve naming

* one more rename

* buf fix

* do a check that supported assets are being withdrawn (#1784)

* set uint256 max instead of magic number (#1782)

* remove unused files (#1785)

* fix renaming bug

* correct safe approve all tokens and adjust the documentation (#1776)

* prettier

* M04 - minBptFunction robustness (#1795)

* change bptExpected to ignore Oracle prices and assume assets are always pegged. (Vault value checker shall be used to mitigate issues with said assumption)

* fix all the tests

* add test for pool manipulation

* prettier & lint

* minor change

* add withdrawal test

* update documentation

* pick string error length that is smaller than 32 characters

* prettier

* correct comment

* better comments

* prettier

* M02 withdrawal fuzzing tests (#1801)

* add more withdrawal tests

* add more withdrawal cases

* N02 gas inefficiencies  (#1786)

* gas optimisation

* fix bad merge and prettier

* remove todo comments (#1796)

* use a more appropriate array initialisation length (#1800)

* more consistant function naming (#1797)

* fix typo (#1798)

* simplify the way we withdrawAll. no need to pass along min amonts (#1777)

* M03 - missing or misleading documentation (#1781)

* improve documentation

* add comma

* M01 More comprehensive test suite (#1780)

* add tests for approving tokens and harvesting rewards

* prettier and lint

* fix bad merge + prettier & lint

* fix fork tests remove .only

* remove only

* lint

* fix unit tests

* add more tests to see how checkBalance behaves

* remove console log

* improve checkBalance test by testing that checkBalance amount doesn't decrease after the attack comaring to the middle of the attack.

* confirm that yield gained by 3rd party tilting the pool can be extracted by withdrawing the funds

* rename internal functions by prepending them with underscore

* Generated latest Balancer strategy diagrams (#1805)

* bug fix

* bug fix

* Minor Balancer changes from final review (#1819)

* Removed unused imports

* Generated updated contract diagram

* Deploy Balancer Meta stable pool RETH strategy

* update deploy description

* fix typo

* add proposal Id to deploy script

* prettier

* create a deploy file

* file rename

* improve wording

* fix name redundancy

---------

Co-authored-by: Nick Addison <nick@addisonbrown.com.au>
Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>
sparrowDom added a commit that referenced this pull request Oct 3, 2023
* initail commit

* intermediary commit

* commit research files

* balancer booster abi

* intermittent commit

* add base balancer contract that implements checkBalance functionality

* add some additional initial integration

* intermittent commit

* add deployment file

* add fork test fixture

* intermittent commit

* prettier

* add basic withdrawal / deposit functionality

* correct the BPT calculation

* prettier + lint

* simplify the BPT price calculation

* add read-only re-entrancy protection

* add some missing tests and adjust existing. Deparate deposit and withdrawal slippage

* fix check balance implementation

* Balancer review changes (#1726)

* Generated contract docs

* Refactor Balancer storage variables

* Small Balancer changes

* Natspec updates
Added missing licensing
getPoolAssets gas optimized

* Updated generated Balancer strategy contract diagrams

* fix contract linter

* Removed restrictions on tests

* Small gas improvements
Fixed Slither

* Change BalancerError version

* Updated constant names
Addresses to use checksum format

* JS lint tasks

* Updated Balancer and Aura pool id constants

* Removed getRateProviderRate as it wasn't being used
Refactored to remove poolAssetsMapped storage variable

* Updated OETH Contracts diagrams
Generated new Balancer contract diagrams

* Fix failing test

* Fix merge conflict

* Restored getRateProviderRate

* Natspec updates
Added toPoolAsset override

* Removed unused getRateProviderRate

* Natspec updates
Gas optimization of InitializableAbstractStrategy

* Abstract strategy gas improvements (#1719)

* Refactor base strategy to use immutables

* Fixed strategy deployments in 001_core and fixtures

* Generated new strategy diagrams

* Deploy rETH instead of the stETH Balancer MetaStable Pool

* removed unused Aura config

* Balancer fork tests

* Added check that BPT amount equals Aura LP amount
Added rETH conversion to ETH value

* Updated balancer strat fork tests

* Updated Balancer fork tests

* Added optional deposit with multiple assets to the strategy

* Single asset deposit to use multi asset deposit

* Added optional checkBalance to Balancer strategy

* Added checkBalance() to BaseBalancerStrategy

* Fix slither
Fix curve HH task

* Added multi-asset withdraw to balancer strategy

* Fix multi-asset withdraw

* Updated Balancer and Vault diagrams

* Fix js linter

* Fixed checkBalance of rETH asset in Balancer strategy

* Only wrap assets if amount > 0
Added depositAll fork test for Balancer strat

* Removed Vault changes for multi-asset strategy support

* Updated generated docs

* Add tests for wstETH/WETH Balancer pool (#1725)

* Split deployment and fix fixtures

* Deposit tests for wstETH/WETH pool

* Add withdraw test

* prettier

* remove .only in fork tests

---------

Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>

* [ DFD-1 ] Balancer's checkBalance (#1730)

* add alternative implementation of Balancer's checkBalance

* correct the checkBalance implementation

* Balancer fork tests (#1727)

* Added large withdraw tests for Balancer strategy

* fix test log

* Balancer withdraw to handle close to BPT limit

* Small change to Balancer withdraw fork test

* add some comments

* change implementation

---------

Co-authored-by: Domen Grabec <grabec@gmail.com>

* Add read-only reentrancy test (#1731)

* Added large withdraw tests for Balancer strategy

* fix test log

* Balancer withdraw to handle close to BPT limit

* Small change to Balancer withdraw fork test

* add some comments

* Add test for read-only reentrancy

* add reentrancy protection to checkBalance functions

* add documentation

* remove the only

---------

Co-authored-by: Nicholas Addison <nick@addisonbrown.com.au>
Co-authored-by: Domen Grabec <grabec@gmail.com>

* Balancer fixes (#1734)

* prettier

* adjust how checkBalance is calculated

* Balancer withdrawal fix (#1739)

* fix balancer withdrawals

* lint

* prettier

* use only 1 safeApprove when applicable

* some renames and more correct application of approves

* renames, additional requires, move initializer to a better location, slither

* bug fix

* Generated latest Balancer strategy diagrams

* re-deploy BPT tokens sitting in the strategy

* fix re-entrancy test

* fixture fix

* bug fix

* prettier

* L02 improve naming  (#1783)

* improve naming

* one more rename

* buf fix

* do a check that supported assets are being withdrawn (#1784)

* set uint256 max instead of magic number (#1782)

* remove unused files (#1785)

* fix renaming bug

* correct safe approve all tokens and adjust the documentation (#1776)

* prettier

* M04 - minBptFunction robustness (#1795)

* change bptExpected to ignore Oracle prices and assume assets are always pegged. (Vault value checker shall be used to mitigate issues with said assumption)

* fix all the tests

* add test for pool manipulation

* prettier & lint

* minor change

* add withdrawal test

* update documentation

* pick string error length that is smaller than 32 characters

* prettier

* correct comment

* better comments

* prettier

* M02 withdrawal fuzzing tests (#1801)

* add more withdrawal tests

* add more withdrawal cases

* N02 gas inefficiencies  (#1786)

* gas optimisation

* fix bad merge and prettier

* remove todo comments (#1796)

* use a more appropriate array initialisation length (#1800)

* more consistant function naming (#1797)

* fix typo (#1798)

* simplify the way we withdrawAll. no need to pass along min amonts (#1777)

* M03 - missing or misleading documentation (#1781)

* improve documentation

* add comma

* M01 More comprehensive test suite (#1780)

* add tests for approving tokens and harvesting rewards

* prettier and lint

* fix bad merge + prettier & lint

* fix fork tests remove .only

* remove only

* lint

* fix unit tests

* add more tests to see how checkBalance behaves

* remove console log

* improve checkBalance test by testing that checkBalance amount doesn't decrease after the attack comaring to the middle of the attack.

* confirm that yield gained by 3rd party tilting the pool can be extracted by withdrawing the funds

* rename internal functions by prepending them with underscore

* Generated latest Balancer strategy diagrams (#1805)

* bug fix

* bug fix

* Minor Balancer changes from final review (#1819)

* Removed unused imports

* Generated updated contract diagram

* Deploy Balancer Meta stable pool RETH strategy

* update deploy description

* fix typo

* add proposal Id to deploy script

* prettier

* create a deploy file

* file rename

* improve wording

* fix name redundancy

* re-deploy Balancer rETH/WETH strategy

* add proposal id

* improve deploy proposal wording

* update proposal id

* Prettify

---------

Co-authored-by: Nick Addison <nick@addisonbrown.com.au>
Co-authored-by: Shahul Hameed <10547529+shahthepro@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.

None yet

4 participants