-
Notifications
You must be signed in to change notification settings - Fork 56
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
improve(inventory-manager): Check ERC20 balance before submitting cross chain bridge #972
Changes from all commits
888b78d
1c47ac7
152c468
b90a4fa
7474f91
38763ca
9818120
37ece85
518e692
955ed09
526cb5f
51ffb60
f0a036d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ export class RelayerConfig extends CommonConfig { | |
readonly debugProfitability: boolean; | ||
// Whether token price fetch failures will be ignored when computing relay profitability. | ||
// If this is false, the relayer will throw an error when fetching prices fails. | ||
readonly skipRelays: boolean; | ||
readonly sendingRelaysEnabled: boolean; | ||
readonly sendingSlowRelaysEnabled: boolean; | ||
readonly sendingRefundRequestsEnabled: boolean; | ||
|
@@ -44,6 +45,7 @@ export class RelayerConfig extends CommonConfig { | |
RELAYER_INVENTORY_CONFIG, | ||
RELAYER_TOKENS, | ||
SEND_RELAYS, | ||
SKIP_RELAYS, | ||
SEND_SLOW_RELAYS, | ||
SEND_REFUND_REQUESTS, | ||
MIN_RELAYER_FEE_PCT, | ||
|
@@ -132,6 +134,7 @@ 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fair point reverted here 9818120 |
||
this.skipRelays = SKIP_RELAYS === "true"; | ||
this.sendingRefundRequestsEnabled = SEND_REFUND_REQUESTS !== "false"; | ||
this.sendingSlowRelaysEnabled = SEND_SLOW_RELAYS === "true"; | ||
this.acceptInvalidFills = ACCEPT_INVALID_FILLS === "true"; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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