Skip to content
This repository has been archived by the owner on Oct 6, 2023. It is now read-only.

Update AccountsStrategy tests and address issues found in the contract #267

Merged
merged 80 commits into from
Aug 15, 2023

Conversation

0xNeshi
Copy link
Contributor

@0xNeshi 0xNeshi commented Aug 7, 2023

Ticket: #207

Explanation of the solution:

  • added missing require checks
  • added missing emit EndowmentRedeemed events
  • fixed unintended large balance reduction in _payForGasWithAccountBalance
  • fixed locked balance reduction logic in _payForGasWithAccountBalance
  • write tests for all (most) of AccountsStrategy paths

Instructions on making this work

  • run yarn or yarn install to install npm dependencies

@0xNeshi 0xNeshi added the enhancement New feature or request label Aug 7, 2023
@0xNeshi 0xNeshi self-assigned this Aug 7, 2023
@0xNeshi 0xNeshi added the WIP label Aug 7, 2023
@0xNeshi 0xNeshi force-pushed the update-acc-strat-tests branch 2 times, most recently from 6ab6172 to 4743a83 Compare August 8, 2023 06:37
@0xNeshi 0xNeshi force-pushed the update-acc-strat-tests branch 3 times, most recently from 1d3c7bf to 468882a Compare August 8, 2023 10:41
@0xNeshi 0xNeshi changed the title [WIP] Update AccountStrategy tests Update AccountStrategy tests Aug 9, 2023
@0xNeshi 0xNeshi removed the WIP label Aug 9, 2023
@0xNeshi 0xNeshi changed the title Update AccountStrategy tests Update AccountStrategy tests Aug 9, 2023
@0xNeshi 0xNeshi changed the title Update AccountStrategy tests Update AccountsStrategy tests and address issues found in the contract Aug 9, 2023
@0xNeshi
Copy link
Contributor Author

0xNeshi commented Aug 9, 2023

cc: @SovereignAndrey @stevieraykatz

@0xNeshi 0xNeshi changed the title Update AccountsStrategy tests and address issues found in the contract [WIP] Update AccountsStrategy tests and address issues found in the contract Aug 10, 2023
@0xNeshi 0xNeshi changed the title [WIP] Update AccountsStrategy tests and address issues found in the contract Update AccountsStrategy tests and address issues found in the contract Aug 10, 2023
@0xNeshi 0xNeshi changed the title Update AccountsStrategy tests and address issues found in the contract [WIP] Update AccountsStrategy tests and address issues found in the contract Aug 11, 2023
@0xNeshi 0xNeshi merged commit c1e7c5c into update-tests Aug 15, 2023
1 check passed
@0xNeshi 0xNeshi deleted the update-acc-strat-tests branch August 15, 2023 07:42
0xNeshi pushed a commit that referenced this pull request Aug 15, 2023
* Remove DummyERC20 use from core tests

* Clean redundant awaits in AccountsStrategy

* Update Router wrong Promise<string> arg

* Rename GasFwd > user to accounts

* Fix GasFwd tests

* Set Accounts as default signer in GasFwd tests > deployGasFwdAsProxy

* Fix wrong await expects

* Refactor some tests in AccountsCreateEndowment

* Fix deploy error tests

* Update 'unauthorized' error tests

* Fix IndexFund tests

* Wrap all IndexFund txs in wait

* Move 'wait' to test/utils

* Wait all unwaited txs

* Fix Vaults tests

* Fix AccountsSwapRouter tests

* Add missing hre param to packActionData in Router & AccountsStrategy

* Update deployer/proxyAdmin in .env.template

* Remove temp_task

* Remove hardhat/console.sol from AccountsSwapRouter

* Move IndexFund errors to IIndexFund

* Update `AccountsStrategy` tests and address issues found in the contract (#267)

* Use getChainId in AccountsStrategy

* Make AccountsStrategy 'upon strategyInvest reverts when' tests pass with fake contracts

* Make AccountsStrategy 'upon strategyInvest and calls the local router' tests pass with fake contracts

* Make AccountsStrategy 'upon strategyInvest and calls axelar GMP' tests pass with fake contracts

* Make AccountsStrategy 'upon strategyRedeem reverts when' tests pass with fake contracts

* Add missing EndowmentRedeemed emissions in strategyRedeem

* Make AccountsStrategy 'upon strategyRedeem and calls the local router' tests pass with fake contracts

* Make AccountsStrategy 'upon strategyRedeem and calls axelar GMP' tests pass with fake contracts

* Move state/facet declaration to root describe

* Move ACCOUNT_ID up

* Move common fake contracts to root describe

* Set type of investRequest to AccountMessages.InvestRequestStruct

* Refactor strategy approval state

* Use 'const' where applicable

* Refactor registrar fakes

* Check WITHDRAW_ONLY flow in strategyRedeem

* Remove redundant token.transfer.returns(true)

* Add missing types

* Move lock/liq/gas fee consts up

* Move all common fake contracts up

* Move 'and calls the local router' outside of 'reverts when'

* Make upon strategyRedeemAll work with 'reverts when' and 'and calls the local router'

* Rename all constants to upper case

* Make upon strategyRedeemAll work with 'and calls axelar GMP'

* Add new test case when more than enough balance to pay gas

* Reorg. imports

* Move 'token.approve.returns' mock to beforeEach

* Add missing test cases related to amnt of gas covered by GasFwd

* Fix lock. bal calc. in _payForGasWithAccountBalance

* Add new test case for _payForGasWithAccountBalance

* Move balance setup up

* Add revert cases for strategyInvest _payForGasWithAccountBalance

* Move strategyInvest > _payForGasWithAccountBalance revert tests to 'and calls axelar GMP' section

* Remove redundant check in reverting test for strategyRedeem

* Add missing test cases for strategyRedeem

* Fix/add test cases for strategyRedeemAll

* Add new ext. call tests

* Move gasFwd, registrar, router and token fakes init to beforeEach

* Add test case for strategyRedeemAll > approvalState == WITHDRAW_ONLY

* Remove async func from describes

* Reorganize callback tests

* Pass 0 tokens to _payForGasWithAccountBalance on redeem funcs

* Refactor liqGas calculation

Co-authored-by: katzman <84420280+stevieraykatz@users.noreply.github.com>

* Use liq. percentage to calculate gas to pay from account bal

* Revert "Use liq. percentage to calculate gas to pay from account bal"

This reverts commit 8e53c23.

* Extract investAmt

* Add gasPercentFromLiq param to _payForGasWithAccountBalance

* Add missing require at least one of locked/liquid is passed

* Move comments

* Add missing 'Must X at least one of locked/liquid' tests

* Use ACCOUNT_ID instead of hardcoded 1s

* Add valid value check for gasPercentFromLiq in _payForGasWithAccountBalance

* Revert "Add valid value check for gasPercentFromLiq in _payForGasWithAccountBalance"

This reverts commit f7e1a0a.

* Fix tests

* Add more data to InsufficientFundsForGas error

* Add more test cases for _execute

* Fix wrongful expect awaits into await expects

* Fix 'into _execute' tests

* Fix all tests

* Add missing types to redeemRequests

* Fix typo in InsufficientFundsForGas

* Fix comments in _payForGasWithAccountBalance

* Add missing emit EndowmentRedeemed in _axelarCallbackWithToken

* Rename actionStatus to vaultStatus

* Rename gasPercentFromLiq to gasRateFromLiq_withPrecision

* Use BIG_NUMBA_BASIS instead of PERCENT_BASIS

* Add fractional calculation tests for strategy[Invest/Redeem]

* Remove extra fields from InsufficientFundsForGas

* Merge all strategyRedeem > 'makes all the correct external calls and pays for part of gas fee' tests into one

* Fix strategyRedeemAll percentage, changed basis into BIG_NUMBA_BASIS

* Add more strategyRedeemAll tests

* Add custom descriptions

* Add new strategyInvest test cases

* Update EndowmentInvested and EndowmentRedeemed events to include endowId

* Add missing expects and events to tests

* Move state updates in strategyInvest before if/else

* Emit custom error on zero amounts set to invest/redeem

* Fix _payForGasWithAccountBalance, no longer accepts lock/liq amts

* Fix tests

* Move ZeroAmount to interface

---------

Co-authored-by: katzman <84420280+stevieraykatz@users.noreply.github.com>

* Remove redundant state.config assignment in IndexFund.initialize

---------

Co-authored-by: katzman <84420280+stevieraykatz@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants