Liquidator & disputer bot: Improve bot gas estimation #1801
Conversation
Signed-off-by: Nick Pai <npai.nyc@gmail.com>
Signed-off-by: Nick Pai <npai.nyc@gmail.com>
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
chrismaree
requested review from
nicholaspai and
mrice32
and removed request for
nicholaspai
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
disputer/disputer.js
Outdated
| try { | ||
| await dispute.call({ from: this.account, gasPrice: this.gasEstimator.getCurrentFastPrice() }); | ||
| await dispute.call({ from: this.account }); | ||
| gasLimit = Math.floor((await dispute.estimateGas({ from: this.account })) * 1.5); |
There was a problem hiding this comment.
Do you think we really need the 1.5 buffer? If so, maybe define this in the config or as a constant above the file
There was a problem hiding this comment.
I think truffle uses 1.25 by default: trufflesuite/truffle#2539. Not sure of all the details around why that's necessary, but I've always been unclear if it's estimating the gas required or the gas used (since the used gas may be lower than the gas that you must send to get a successful txn).
There was a problem hiding this comment.
I do think we need the buffer. Often the gas used by the tx is different from what the estimation provides. It could be a config but I dont think there are any situations in which the user should be messing with this. I have set it to the same that Truffle estimates to at 1.25. PTAL and let me know.
There was a problem hiding this comment.
Makes sense. I think you should set the 1.25x multiple early as a constant and then re-use that so you and future devs don't fat finger any multiples
disputer/disputer.js
Outdated
| try { | ||
| await dispute.call({ from: this.account, gasPrice: this.gasEstimator.getCurrentFastPrice() }); | ||
| await dispute.call({ from: this.account }); | ||
| gasLimit = Math.floor((await dispute.estimateGas({ from: this.account })) * 1.5); |
There was a problem hiding this comment.
I think truffle uses 1.25 by default: trufflesuite/truffle#2539. Not sure of all the details around why that's necessary, but I've always been unclear if it's estimating the gas required or the gas used (since the used gas may be lower than the gas that you must send to get a successful txn).
…PClient once in sponsor reporter Signed-off-by: Nick Pai <npai.nyc@gmail.com>
Signed-off-by: Nick Pai <npai.nyc@gmail.com>
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
…quidator-bot-gass-price Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
…om:UMAprotocol/protocol into chrismaree/patch-liquidator-bot-gass-price Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
disputer/disputer.js
Outdated
| try { | ||
| await dispute.call({ from: this.account, gasPrice: this.gasEstimator.getCurrentFastPrice() }); | ||
| totalPaid = await dispute.call({ from: this.account }); | ||
| gasLimit = Math.floor((await dispute.estimateGas({ from: this.account })) * 1.25); |
There was a problem hiding this comment.
Perhaps const GAS_LIMIT_BUFFER = 1.25 would be safer
Signed-off-by: Christopher Maree <christopher.maree@gmail.com>
Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>
Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>
|
LGTM! |
Motivation
Right now the bots do not implement any gas estimation before sending liquidation or dispute transactions. This means that a transaction will default to the bots
txnGasLimitwhich is 9 million. This makes the transactions inefficient as they require to send way more gas than they will actually use.Summary
This PR uses the
estimateGasfunction before sending the transaction. If this value is less than the bots configuredtxnGasLimit, it uses 1.5x this value as the gas limit.close #1791