Skip to content

Instantly share code, notes, and snippets.

@ngmachado
Created October 5, 2021 10:53
Show Gist options
  • Save ngmachado/edb5f800a0edfeac1355cc775415b91b to your computer and use it in GitHub Desktop.
Save ngmachado/edb5f800a0edfeac1355cc775415b91b to your computer and use it in GitHub Desktop.
Drip.md

Drip Contract Friendly Review

Notice

This is a friendly code review and my personal comments.

  • Lock the contracts to a specific compiler version
  • Add license to all contracts
  • Think about splitting the _updateOutflow function in two smaller functions. flowUpdated and flowDeleted cases. Beware that any revert in the terminationCallback will jail the app. Keep that part the most simple possible.

Line: drip/RedirectAll.sol at 3bae478304fc891b7d3ea6a511ea349519a8238c · ngmachado/drip · GitHub

Obs: Depending on the network you may need to register a appKey

Solution: Link to example


Line: drip/RedirectAll.sol at 3bae478304fc891b7d3ea6a511ea349519a8238c · ngmachado/drip · GitHub

Obs: The if statement is redundant, can be removed


Line: drip/RedirectAll.sol at 3bae478304fc891b7d3ea6a511ea349519a8238c · ngmachado/drip · GitHub

Obs: Could be inside the next if after checking if affiliate address exist


Link: drip/RedirectAll.sol at 3bae478304fc891b7d3ea6a511ea349519a8238c · ngmachado/drip · GitHub

Obs: check if tokenId exist, affCode could be not registered, later in the function you are using that information: drip/RedirectAll.sol at 3bae478304fc891b7d3ea6a511ea349519a8238c · ngmachado/drip · GitHub Could end up updating something like mapping => 0 => superToken


Link: drip/RedirectAll.sol at 3bae478304fc891b7d3ea6a511ea349519a8238c · ngmachado/drip · GitHub

Obs: Using modifier onlyHost that check the msg.sender, could remove this check. Only host contract can call this function.


Link: drip/RedirectAll.sol at ed63791a4f4aeb05f1e2accc6deeca8c560a8f56 · ngmachado/drip · GitHub

Obs: This assumption is not correct, you can update a flow with the same value. FlowRate = 100 and the update FlowRate also 100. changeInFlowSubscriber will be zero.


Link: drip/RedirectAll.sol at ed63791a4f4aeb05f1e2accc6deeca8c560a8f56 · ngmachado/drip · GitHub

Obs: Should in this case always deleting a flow if exist, and point to the new affiliate? What is the case to do a flow update?


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment