bancorprotocol / fastlane-bot

Fast Lane, an open-source arbitrage protocol, allows any user to perform arbitrage between Bancor ecosystem protocols and external exchanges and redirect arbitrage profits back to the protocol.
https://github.com/bancorprotocol/fastlane-bot
MIT License
138 stars 58 forks source link

ABI Cleanup #415

Open barakman opened 8 months ago

barakman commented 8 months ago

Enhancement Description

  1. Rewrite every ABI as a dictionary declared in a properly-formatted manner, rather than as a (very long) string which is json-loaded into a dictionary during runtime
  2. Keep in every ABI only the functions and events that the bot relies on
  3. Merge the ABIs of all DEX contracts into a single ABI

Bullet 3 is doable only if no two functions which differ only by the return-value type exist within the different DEX contracts. Even the potential of this possibility in the future (as more DEXs are added) might be a good reason for avoiding it altogether. The main advantage here is that it would revoke the need to interact with different DEX contracts using different ABIs.

Rationale

The ABI of a contract doesn't need to include all the functions and events in the contract. It is sufficient for every ABI to include only the functions and events that are actually used in the bot. The main advantage in the current implementation is that every ABI can be copied as is from etherscan or similar, and then pasted directly into the code without "having to think about it any further". The main disadvantage is a ton of redundant information which needs to be maintained in the code. And keeping it as a string makes it even more difficult, both in terms of readability and in terms of maintenance. For example, searching through functions or events in an ABI is extremely difficult under the current implementation.

Impact Analysis

As a example for the overloaded-functions issue, suppose a ABI with the following two functions:

  1. function getFee(uint256 id, bool stable) external returns (uint256)
  2. function getFee(uint128 id) external returns (uint256)

The way to call each one of these functions with id = 5, for example, is something like:

  1. contract.getFee["uint256,bool"](5)
  2. contract.getFee["uint128"](5)
barakman commented 7 months ago

Supplemental - parameter names in an ABI are generally optional:

Function parameter names have no impact whatsoever, so they are only "nice to have".

Event parameter names impact how the client's side web3 package provides the event to the caller. They do not need to be identical to the ones in the contract, which means that the programmers on the client side can rename them as desired.

This becomes useful when several events on different ABIs are identical with the exception of parameter names. By aligning these names under a single convention, one can simplify the handling of the related events.

For example, the PairCreated events in the Uniswap V2 factory ABI and in the Pancakeswap V2 factory ABI, are identical with the exception of the 1st parameter name (tkn0_address vs token0) and the 2nd parameter name (tkn1_address vs token1).

Aligning these parameter names shall allow to: