release / v0.5.0 sync dev => master #52
Conversation
* merge balancer & uniswap endpoint to under /eth * consolidate functions on balancer, uniswap swaps price lookup * cache swap pool as class property * replace token contract address with token symbol on request param * add mainnet token list (1inch) * add maxswaps, ethGasStation api, gas level param to .env
* Consolidate log into `info`. Remove debug, error, and rejection. * Enable debug mode logging for development * Check for undefined erc20 token list url source * Fix token list conversion for allowance & approval
* accept balancer exchange proxy in .env * accept balancer gas base, gas per swap overwrite in .env * accept eth approval gas limit, manual gas fee setting in .env when ethgasstation not set to true * accept uniswap router setting in .env * accept uniswap gas limit, ttl, update period overwrite * update .env.example
* Add initiate one-time gas retrieval
* Add price check debugging
* get on-chain pool data update to prevent change in pool balance prior to trade execution * remove pool cache loop during strategy initialisation
* update start function to use POST to standard request body param (json in particular).
* reduce redundant params being sent over stand POST api call in client
(feat) optimize Uniswap connector
…into feat/optimize_protocol_endpoints
(feat) standard endpoints & consolidate protocol library functions
evseevnn
left a comment
evseevnn
left a comment
There was a problem hiding this comment.
Just my 5 cents =)
Actually, I can find more problems with this code, but have not enought time for that.
src/services/balancer.js
Outdated
| this.gasBase = GAS_BASE | ||
| this.gasPerSwap = GAS_PER_SWAP | ||
| this.maxSwaps = MAX_SWAPS | ||
| this.maxSwaps = process.env.BALANCER_MAX_SWAPS |
There was a problem hiding this comment.
I think better move all configuration to one config file and then take data from this file, because so often need prepare data before using. Like example, sometimes need check value on undefined or convert to type what will used later in code. So, using env variables like this look bad.
src/services/eth.js
Outdated
| uniswap: process.env.UNISWAP_ROUTER | ||
| } | ||
| this.tokenListUrl = process.env.ETHEREUM_TOKEN_LIST_URL | ||
| if (network === 'kovan') { |
There was a problem hiding this comment.
Names of networks should be stored on one file, it's can be enum collection, in case of using Typescript you can use enum items in code. In case of using Javascript you can use simply constants.
src/services/eth.js
Outdated
| // get remote token list | ||
| async getTokenList (tokenListUrl) { | ||
| try { | ||
| axios.get(tokenListUrl) |
There was a problem hiding this comment.
try catch construction should be use in case if you using async/await. IN another case error will in order execution flow and you cannot catch it.
You should write await before axios.get and use async await flow for this operation.
src/services/eth.js
Outdated
| } catch (err) { | ||
| logger.error(err) | ||
| let reason | ||
| err.reason ? reason = err.reason : reason = 'error remote token list retrieval' |
There was a problem hiding this comment.
return err.reason || 'error remote token list retrieval';
src/services/eth.js
Outdated
| } catch (err) { | ||
| logger.error(err) | ||
| let reason | ||
| err.reason ? reason = err.reason : reason = 'error remote token list retrieval' |
There was a problem hiding this comment.
Better put 'error remote token list retrieval' into constants.
src/services/fees.js
Outdated
| .then(function (response) { | ||
| // handle success | ||
| const gasFee = response.data[gasLevel] | ||
| // console.log(`getETHGasStationFee(${gasLevel})`, gasFee) |
There was a problem hiding this comment.
Pls not leave unusable comments or code in code
No description provided.