Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check Coingecko prices separately from BatchRequesterLoop #40

Open
albertchon opened this issue Jul 18, 2021 · 0 comments
Open

Check Coingecko prices separately from BatchRequesterLoop #40

albertchon opened this issue Jul 18, 2021 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@albertchon
Copy link
Member

In the current BatchRequesterLoop, if the Coingecko API is down AND orchestrators have a non-zero minFeeInUSD, then no batches will be requested aka no withdrawals back to Ethereum will occur.

// send batch request only if fee threshold is met.
if s.CheckFeeThreshod(tokenAddr, unbatchedToken.TotalFees, s.minBatchFeeUSD) {
logger.WithFields(log.Fields{"tokenContract": tokenAddr, "denom": denom}).Infoln("sending batch request")
_ = s.peggyBroadcastClient.SendRequestBatch(ctx, denom)
}

func (s *peggyOrchestrator) CheckFeeThreshod(erc20Contract common.Address, totalFee cosmtypes.Int, minFeeInUSD float64) bool {
if minFeeInUSD == 0 {
return true
}
tokenPriceInUSD, err := s.priceFeeder.QueryUSDPrice(erc20Contract)
if err != nil {
return false
}

Empirically I've noticed the Coingecko public API can be a bit flaky sometimes (at least in the trading bot's usage) so we shouldn't depend on it to function in order to process withdrawals. It's nice that we have retries currently at least but ideally, we should make this flow independent of whether Coingecko works or not, as it's entirely plausible that their API goes down for a day.

I propose the following change:

  • add a separate goroutine that periodically checks Coingecko prices in the background (e.g. once per hour) and updates it in a coingeckoPrices map[common.Address]float64 defined on the peggyOrchestrator struct. This will keep a reasonably up to date mapping of the prices from Coingecko. The denoms for which this should be fetched are for any ERC-20 denom in the bank supply
  • Change the CheckFeeThreshold function to return the price from coingeckoPrices if the s.priceFeeder.QueryUSDPrice returns an error. So
	tokenPriceInUSD, err := s.priceFeeder.QueryUSDPrice(erc20Contract)
	if err != nil {
	        price, ok := s.coingeckoPrices[erc20Contract]
	        if ok {
	            tokenPriceInUSD = price
	        } else {
                    return false
	        }
	}

Note: currently this isn't an issue since we're running the orchestrators with a 0 minFeeInUSD setting but in the future we should change this in order to increase the economic robustness of the bridge.

@albertchon albertchon added the enhancement New feature or request label Jul 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants