SetProtocol / set-protocol-v2

Set Protocol V2
118 stars 94 forks source link

Notional Trade Module #251

Closed ckoopmann closed 2 years ago

ckoopmann commented 2 years ago

Code Review Processes

New Feature Review

Before submitting a pull request for new review, make sure the following is done:

README Checks

Code Checks

Broader Considerations

ckoopmann commented 2 years ago

@ckoopmann This looks really nice! Integration tests look solid.

Have left some (mostly nit) comments and a couple smaller questions.

(Would it be possible to regenerate the abi JSON files added in this PR so they don't add 200k lines? They can't be inspected on Github and had trouble opening them locally as well.)

Those are abis that I didn't generate but were shared with me by @jeffywu. Maybe he can direct me to the smart contract source code from which those abis were generated so I can try to regenerate them in the repo.

EDIT: After rereading your comment it seems you were mostly concerned about the number of new lines. So for now I went for the easiest "solution" of just combining those files to a single line. Lmk if that alleviates the problem or makes it worse. (obviously the files are unreadable for humans now). Either way these files will be removed from the repo when the respective upgrades of the notional proxy were performed on mainnet.

cgewecke commented 2 years ago

:tada: This PR is included in version 0.11.0-hhat.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: