onchainification/great-ape-safe

An extension of the legendary Ape Safe web3 package. Adding extra functions and making it protocol aware, saving time getting familiar with the source code of every individual smart contract.

great-ape-safe's Issues

[Gitcoin Bounty] Great Ape Safe: Write Unit Tests for a DeFi On-Chain API Wrapper & Tx Builder

Multisig Txs

Over the last year, Badger DAO and its smart contracts have made thousands of transactions on the EVM and EVM-compatible chains in order to operate and maintain the Badger system. The dev multisig alone has executed 900+ custom transactions in less than a year. Every single one of these multisig transactions has to be written up in code in advance; a lot of times there is no UI available and even if there is it is not used for security reasons.

The speed and frequency with which we need to execute transactions from our multisigs is increasing. On top of that, we found ourselves copypasting a lot of logic from old scripts for platforms we interact with on a regular basis. We are in the process of giving this more structure.

Great Ape Safe

Building on top of the legendary Ape Safe, we have released an alpha of Great Ape Safe. Besides doing everything Ape Safe can, it contains wrappers for on-chain APIs for some commonly used protocols. This simplifies interaction with these protocols immensely, since no more time has to be spent on researching documentation and deployment addresses. A simple call to the wrapper function safe.aave.deposit(usdc, amount) is now all it takes.

Task

As of now, the API library is limited to some of the main functions of Aave, Compound and Convex. We plan to extend this in the future, but for obvious reasons need to know that the underlying logic is bulletproof.

Help us by writing unit tests for each one of these wrapper functions. Using brownie and its built-in pytest framework, it should be fairly straight-forward to make sure each wrapper function actually does what it needs to do---by asserting the necessary mutations took place on-fork. Did the wallet actually receive a token back after a deposit? Was the contract's return value successful? Do the amounts match? Etc.

There are some known bugs in the current logic; the more you are able to break and the tighter your tests are the better!

Getting Started

Everything you need for your first run can be found in the repo's README.md. A toy example of a unit test can be found under tests/test_aave.py and can be run through brownie test.

Brownie's documentation has a whole chapter dedicated to Writing Unit Tests.

Curve.fi has a great video tutorial series on brownie, which includes some very relevant items:

• Brownie Tutorial 8 -- Fixtures I (https://www.youtube.com/watch?v=YrGuC45GkQc)
• Brownie Tutorial 9 -- Fixtures II (https://www.youtube.com/watch?v=fhPXJtQBF54)
• Brownie Tutorial 10 -- Parametrization (https://www.youtube.com/watch?v=Ogomm9Up0cA)
• Brownie Tutorial 22 -- Unit Testing I (https://www.youtube.com/watch?v=AzAEw9ZzqZo)

Bounty

There is a budget of up to 100 $BADGER available for this task. It will be distributed based on the amount, the quality and the usability of the submissions.

Bonus

Unit tests are what we are after, but if you are able to also fix some of the wrappers or even add new protocol specific logic, great! Please do so in a separate PR.

Links

• the repo you will be working on: https://github.com/gosuto-ai/great-ape-safe
• the og: https://github.com/banteg/ape-safe
• brownie docs: https://eth-brownie.readthedocs.io/en/stable/tests-pytest-intro.html
• pytest general docs: https://docs.pytest.org/en/6.2.x/
• curve.fi video tutorial series on brownie: https://www.youtube.com/playlist?list=PLVOHzVzbg7bFUaOGwN0NOgkTItUAVyBBQ
• overview of our multisigs: https://app.ens.domains/name/badgerdao.eth/subdomains

Contact

Come join our community of Badger builders on Discord at https://discord.gg/BucHcc57FS. For questions specific to this bounty you can reach out to @gosuto#7559 there.

Gitcoin

This issue is a copypaste of the original Gitcoin bounty, which can be found here: https://gitcoin.co/issue/badger-finance/gitcoin/38/100027216

[Q] Beginner contribution

Hi, I want to learn by doing. Can I contribute something? I am proficient with advanced python but I am newbee in this domain.

support only running updated tests instead of the full suite all over again

would be great to be able to run only the tests that were altered, and remember the last result for all other tests:

After the test suite has been run once, you can use the --update flag to only repeat tests where changes have occured:

$ brownie test --update

A module must use the module_isolation or fn_isolation fixture in every test function in order to be skipped in this way.

https://eth-brownie.readthedocs.io/en/stable/tests-pytest-intro.html#only-running-updated-tests

use github actions to run full test suite on every pr

after encountering #11, it makes total sense to automatically run full test suite on every pr.

see https://github.com/brownie-mix/github-actions-mix

catch non-existing convex pools gracefully

there should be a check in withdraw_all to make sure the passed asset has a pool.

current behaviour is a bit ugly:

>>> safe.convex.withdraw_all(usdc)
  File "<console>", line 1, in <module>
  File "./great_ape_safe/api_safe/convex.py", line 75, in withdraw_all
    pool_id = self.get_pool_info(underlying)[0]
TypeError: 'NoneType' object is not subscriptable

i am sure other functions have similar issues, maybe a helper function (eg convex._is_valid_underlying(...)) makes sense here.

safe fixture in conftest should be a neutral/random (pref freshly deployed) gnosis safe

currently still using Badger DAO's multisig:

@pytest.fixture
def safe():
    return GreatApeSafe('dev.badgerdao.eth')

move away from safe.contract(...) and use interface.I...(...) instead

currently almost all contracts are initialised using self.safe.contract(...). this creates a dependency on etherscan or another explorer, and can become incorrect when the local abi db lags behind the abi of an upgradable proxy (see eth-brownie/brownie#1314).

instead, proper use of brownie's interfaces folder should be implemented. this redirects the dependency to the interfaces folder, for which the source is included in the repo and lets itself be easily adjusted. this also guarantees uniform underlying abis for all users, independent of their local brownie db.

implement brownie-token-tester to mint tokens needed in tests

https://github.com/iamdefinitelyahuman/brownie-token-tester is a great tool for 'minting' tokens on-fork for testing purposes.

all tests currently needing some form of a token balance should make use of this package; this removes dependency on real world balance of a token.

the package has already been added to requirements-core.txt in 81bc431.

get rid of rounding error in test_aave_withdraw

>       assert aUSDC.balanceOf(safe) >= bal_before_ausdc - to_withdraw
E       assert 8999999999 >= (10000000000 - 1000000000)
E        +  where 8999999999 = <ContractCall 'balanceOf(address user)'>(ApeSafe("0xB65cef03b9B89f99517643226d76e286ee999e77"))
E        +    where <ContractCall 'balanceOf(address user)'> = <InitializableImmutableAdminUpgradeabilityProxy Contract '0xBcca60bB61934080951369a648Fb03DF4F96263C'>.balanceOf

tests/aave/test_aave_withdraw.py:18: AssertionError

i am suggesting assert aUSDC.balanceOf(safe) >= bal_before_ausdc - to_withdraw - 1, but am just wondering if bigger deviations than 1 could occur? eg if chain is left to sleep for longer, leaving ausdc rebase more?