improve(inventory-manager): Check ERC20 balance before submitting cross chain bridge #972
Conversation
…ss chain bridge Signed-off-by: nicholaspai <[email protected]>
Signed-off-by: nicholaspai <[email protected]>
nicholaspai
requested review from
mrice32,
pxrl and
james-a-morris
as code owners
Co-authored-by: Paul <[email protected]>
There was a problem hiding this comment.
Just to double check, the skipRelays flag is really a secondary change to permit the InventoryClient to run in a standalone instance, right? I otherwise don't think this change should add any noticeable delay to the existing relayer run (as I read the commit message, it seems to indicate that this change slows the bot down).
There was a problem hiding this comment.
(So basically, I think it'd be sufficient to just have the final erc20.balanceOf() check before submitting any rebalance transactions, as a mitigation for the fact that we don't know whether there are concurrent relayer instances running)
There was a problem hiding this comment.
Just to double check, the skipRelays flag is really a secondary change to permit the InventoryClient to run in a standalone instance, right?
Yes exactly, it should not add any delay to the existing relayer run. The slowdown is moreso related to the ERC20.balanceOf check added in the inventory manager
| @@ -131,7 +133,8 @@ export class RelayerConfig extends CommonConfig { | |||
| } | |||
| this.debugProfitability = DEBUG_PROFITABILITY === "true"; | |||
| this.relayerGasMultiplier = toBNWei(RELAYER_GAS_MULTIPLIER || Constants.DEFAULT_RELAYER_GAS_MULTIPLIER); | |||
| this.sendingRelaysEnabled = SEND_RELAYS === "true"; | |||
There was a problem hiding this comment.
FWIW I think it's worth calling this out in the commit message and I probably think it's better to just do it in a separate commit, since it is a breaking change for existing relayer configs.
There was a problem hiding this comment.
fair point reverted here 9818120
The
TokenClientfetches ERC20 balances for the relayer prior to carrying out Relayer duties. This means that there could be a multiple minute delay between balances between fetched and the inventorymanager executing, exposing the relayer to the risk of sending a duplicate rebalance.
This PR adds a simple check for the ERC20 balance as close to sending the inventory rebalance as possible. The downside is a slower inventory manager run, which is mitigated by a side effect of allowing the bot-runner to refactor the inventory manager out of the relayer. This can be done by setting the new environment variable
SKIP_RELAYSwhich will skip the relayer portion of therelayer/index.tsrun.Signed-off-by: nicholaspai [email protected]
Fixes ACX1595