Better tx replacement logic for solution submission #3941
Conversation
MartinquaXD
changed the title
crates/driver/src/domain/mempools.rs
Outdated
| /// pending transaction at the same nonce. | ||
| const GAS_PRICE_BUMP: f64 = 1.125; | ||
| /// pending transaction at the same nonce. The correct factor is actually | ||
| /// 1.25 but to avoid rounding issues on chains with very low gas prices |
There was a problem hiding this comment.
1.25 or 1.125 as it was? Where's the correct number coming from? 🤔
| let submission_block = self.ethereum.current_block().borrow().number; | ||
| let blocks_until_deadline = submission_deadline.saturating_sub(submission_block); | ||
| let estimated_gas_price = | ||
| current_gas_price * GAS_PRICE_BUMP.powi(blocks_until_deadline as i32); |
There was a problem hiding this comment.
This might get pretty aggressive on chains with low block time, right?
There was a problem hiding this comment.
Agree, this exponential progression can go crazy pretty fast. It seems like the GAS_PRICE_BUMP needs to be configurable, then.
There was a problem hiding this comment.
We already had this logic before this PR. In fact we currently apply this multiplication logic in 2 places. Will update the PR to move all the magic gas multiplication logic into 1 file.
| tracing::info!( | ||
| settle_tx_hash = ?hash, | ||
| deadline = submission_deadline, | ||
| current_block = block.number, | ||
| ?cancellation_tx_hash, | ||
| "tx not confirmed in time, cancelling", | ||
| ); |
There was a problem hiding this comment.
Why is this log not needed anymore?
squadgazzz
left a comment
squadgazzz
left a comment
There was a problem hiding this comment.
Very nice! It should help to mitigate some of the nonce issues.
Description
With the recent gnosis chain issues we saw a lot of issues with replacing transactions. Additionally it was not very easy to understand what went wrong due to insufficient logging. This PR addresses both issues.
Changes
Logic:
inframoduledomainmoduleLogging:
submit()functionHow to test
TODO