flashbots / ethers-provider-flashbots-bundle

Flashbots provider for ethers.js
549 stars 216 forks source link

`wait` function only looks back one block; reorgs have a high probability #39

Open Randall4 opened 3 years ago

Randall4 commented 3 years ago

bombardier got frustrated in the Discord since his MEV transaction was uncled but his FlashbotsBundleResolution said that the tx was included.

Suggestion: specify an optional number of blocks to wait (= confirmations) until resolving the promise. By default I'd recommend 3 confirmations based on the current state of the network.

sambacha commented 3 years ago

should the resolution really just say pending for n+3 blocks?

Randall4 commented 3 years ago

should the resolution really just say pending for n+3 blocks?

No, to be more specific I think this would just be passing a new argument called "confirmations" to the wait function here: https://github.com/flashbots/ethers-provider-flashbots-bundle/blob/0d404bb041b82c12789bd62b18e218304a095b6f/src/index.ts#L307

Ideally you would just propagate the confirmations argument to this other wait function and wait for block "targetBlockNumber + confirmations" to be mined immediately before this line: https://github.com/flashbots/ethers-provider-flashbots-bundle/blob/0d404bb041b82c12789bd62b18e218304a095b6f/src/index.ts#L385

sambacha commented 3 years ago

I would think using the term finality would be more apt, as this is the attribute that is actually being used (finalization of transaction meaning X amount of blocks have passed since it was mined on the canonical chain)

Randall4 commented 2 years ago

Interesting that you mention that. I've personally only heard the term "confirmations" to refer to the number of blocks to wait before deciding that a transaction is truly on the chain. Ethers' argument name for it is "confirms": https://docs.ethers.io/v5/api/providers/types/#providers-TransactionResponse and Etherscan uses "block confirmations"