flashbots / mev-share-node

GNU Affero General Public License v3.0
94 stars 13 forks source link

Bundle passes validation step but fails to be sent to builder using `refund-recipient` #20

Open ghost opened 9 months ago

ghost commented 9 months ago

Description

There is a special case of nested bundles that can be sent to builders using the v1.0 format but will fail to be sent to builders using refund-recipient

I identified this potential bug during a code review. I haven't tested it yet but my observations lead me to believe it's a bug.

Reproduction Steps

  1. Send a bundle B1 from searcher S1 that contains an inner bundle but all elements are matched. Set privacy hints so that the bundle is shared in the hint stream

    body:
      bundle: 
        tx: "0x1..."
      tx: "0x2..."
  2. Create a new bundle B2 from searcher S2 that backruns bundle B1. Make sure to set a builder in the privacy settings that relies on the refund-recipient format

    body:
      hash: "0x3..."
      tx: "0x4...

Expected Behavior

The bundle is sent to both the flashbots builder and the external builder.

Actual Behavior:

The bundle will be sent to the flashbots builder, since it relies on the v1.0 format, but not to the builder relying on the refund-recipient format.

Additional Context:

The bundle B2 passes the validation, because the max nesting level is 0.

The unmatched bundle gets matched.

body:
  bundle: 
     bundle: 
       tx: "0x1..."
     tx: "0x2..."
  tx: "0x4...

When the bundle gets sent to a builder that relies on the refund-recipient format the bundle must be converted. In the conversion the transactions are extracted, but this time the bundle will throw an error because the nesting level is 2.

ghost commented 9 months ago

I'd be interested in contributing to this repo

ghost commented 9 months ago

I also posted a question about why a searcher would actually create a bundle like B1. Any guidance would be appreciated.

ghost commented 9 months ago

Pining you @dvush, since you edited those files most recently

dvush commented 9 months ago

Thanks, taking a look!

dvush commented 9 months ago

You are correct! Not all nested bundles can be converted to bundle with a fee recipient.

Even though theoretically it makes sense in this case we did not implement this.

First of all this case would be rare because such bundle is not the best representation of what you want (plain bundle with 2 txs).

Second, I don't know how refund recipient handles more than two txs in a bundle, since it needs to distinguish between what is a backrun and what is user txs.

Given that I don't think it's should be fixed unless there is a use case that you think is unsupported if we don't implement that.