Toggleable discard for failed revertible transaction hashes

README.md

@@ -59,6 +59,15 @@ $ geth --help
     --builder.cancellations        (default: false)
           Enable cancellations for the builder
 
+    --builder.discard_revertible_tx_on_error (default: false)
+          When enabled, if a transaction submitted as part of a bundle in a send bundle
+          request has error on commit, and its hash is specified as one that can revert in
+          the request body, the builder will discard the hash of the failed transaction
+          from the submitted bundle.For additional details on the structure of the request
+          body, see
+          https://docs.flashbots.net/flashbots-mev-share/searchers/understanding-bundles#bundle-definition
+          [$FLASHBOTS_BUILDER_DISCARD_REVERTIBLE_TX_ON_ERROR]
+
     --builder.dry-run              (default: false)
           Builder only validates blocks without submission to the relay
 

builder/builder.go

@@ -72,6 +72,7 @@ type Builder struct {
 	builderPublicKey            phase0.BLSPubKey
 	builderSigningDomain        phase0.Domain
 	builderResubmitInterval     time.Duration
+	discardRevertibleTxOnErr    bool
 
 	limiter                       *rate.Limiter
 	submissionOffsetFromEndOfSlot time.Duration
@@ -91,6 +92,7 @@ type BuilderArgs struct {
 	relay                         IRelay
 	builderSigningDomain          phase0.Domain
 	builderBlockResubmitInterval  time.Duration
+	discardRevertibleTxOnErr      bool
 	eth                           IEthereumService
 	dryRun                        bool
 	ignoreLatePayloadAttributes   bool
@@ -136,6 +138,7 @@ func NewBuilder(args BuilderArgs) (*Builder, error) {
 		builderPublicKey:              pk,
 		builderSigningDomain:          args.builderSigningDomain,
 		builderResubmitInterval:       args.builderBlockResubmitInterval,
+		discardRevertibleTxOnErr:      args.discardRevertibleTxOnErr,
 		submissionOffsetFromEndOfSlot: args.submissionOffsetFromEndOfSlot,
 
 		limiter:       args.limiter,

builder/config.go

@@ -25,6 +25,7 @@ type Config struct {
 	BuilderRateLimitMaxBurst         int           `toml:",omitempty"`
 	BuilderRateLimitResubmitInterval string        `toml:",omitempty"`
 	BuilderSubmissionOffset          time.Duration `toml:",omitempty"`
+	DiscardRevertibleTxOnErr         bool          `toml:",omitempty"`
 	EnableCancellations              bool          `toml:",omitempty"`
 }
 
@@ -50,6 +51,7 @@ var DefaultConfig = Config{
 	ValidationBlocklist:           "",
 	BuilderRateLimitDuration:      RateLimitIntervalDefault.String(),
 	BuilderRateLimitMaxBurst:      RateLimitBurstDefault,
+	DiscardRevertibleTxOnErr:      false,
 	EnableCancellations:           false,
 }
 

builder/service.go

@@ -289,6 +289,7 @@ func Register(stack *node.Node, backend *eth.Ethereum, cfg *Config) error {
 		builderSigningDomain:          builderSigningDomain,
 		builderBlockResubmitInterval:  builderRateLimitInterval,
 		submissionOffsetFromEndOfSlot: submissionOffset,
+		discardRevertibleTxOnErr:      cfg.DiscardRevertibleTxOnErr,
 		ignoreLatePayloadAttributes:   cfg.IgnoreLatePayloadAttributes,
 		validator:                     validator,
 		beaconClient:                  beaconClient,

cmd/geth/main.go

@@ -181,6 +181,7 @@ var (
 		utils.BuilderRateLimitMaxBurst,
 		utils.BuilderBlockResubmitInterval,
 		utils.BuilderSubmissionOffset,
+		utils.BuilderDiscardRevertibleTxOnErr,
 		utils.BuilderEnableCancellations,
 	}
 

cmd/utils/flags.go

@@ -871,6 +871,16 @@ var (
 		Category: flags.BuilderCategory,
 	}
 
+	BuilderDiscardRevertibleTxOnErr = &cli.BoolFlag{
+		Name: "builder.discard_revertible_tx_on_error",
+		Usage: "When enabled, if a transaction submitted as part of a bundle in a send bundle request has error on commit, " +
+			"and its hash is specified as one that can revert in the request body, the builder will discard the hash of the failed transaction from the submitted bundle." +
+			"For additional details on the structure of the request body, see https://docs.flashbots.net/flashbots-mev-share/searchers/understanding-bundles#bundle-definition",
+		EnvVars:  []string{"FLASHBOTS_BUILDER_DISCARD_REVERTIBLE_TX_ON_ERROR"},
+		Value:    builder.DefaultConfig.DiscardRevertibleTxOnErr,
+		Category: flags.BuilderCategory,
+	}
+
 	BuilderEnableCancellations = &cli.BoolFlag{
 		Name:     "builder.cancellations",
 		Usage:    "Enable cancellations for the builder",
@@ -1704,6 +1714,7 @@ func SetBuilderConfig(ctx *cli.Context, cfg *builder.Config) {
 	cfg.BuilderRateLimitDuration = ctx.String(BuilderRateLimitDuration.Name)
 	cfg.BuilderRateLimitMaxBurst = ctx.Int(BuilderRateLimitMaxBurst.Name)
 	cfg.BuilderSubmissionOffset = ctx.Duration(BuilderSubmissionOffset.Name)
+	cfg.DiscardRevertibleTxOnErr = ctx.Bool(BuilderDiscardRevertibleTxOnErr.Name)
 	cfg.EnableCancellations = ctx.IsSet(BuilderEnableCancellations.Name)
 	cfg.BuilderRateLimitResubmitInterval = ctx.String(BuilderBlockResubmitInterval.Name)
 }

miner/algo_common.go

@@ -23,20 +23,20 @@ const (
 )
 
 const (
-	// defaultProfitPercentMinimum is to ensure committed transactions, bundles, sbundles don't fall below this threshold
+	// defaultProfitThresholdPercent is to ensure committed transactions, bundles, sbundles don't fall below this threshold
 	// when profit is enforced
-	defaultProfitPercentMinimum = 70
+	defaultProfitThresholdPercent = 70
 
 	// defaultPriceCutoffPercent is for bucketing transactions by price, used for greedy buckets algorithm
 	defaultPriceCutoffPercent = 50
 )
 
 var (
-	defaultProfitThreshold = big.NewInt(defaultProfitPercentMinimum)
 	defaultAlgorithmConfig = algorithmConfig{
+		DropRevertibleTxOnErr:  false,
 		EnforceProfit:          false,
-		ExpectedProfit:         common.Big0,
-		ProfitThresholdPercent: defaultProfitThreshold,
+		ProfitThresholdPercent: defaultProfitThresholdPercent,
+		PriceCutoffPercent:     defaultPriceCutoffPercent,
 	}
 )
 
@@ -61,13 +61,18 @@ func (e *lowProfitError) Error() string {
 }
 
 type algorithmConfig struct {
+	// DropRevertibleTxOnErr is used when a revertible transaction has error on commit, and we wish to discard
+	// the transaction and continue processing the rest of a bundle or sbundle.
+	// Revertible transactions are specified as hashes that can revert in a bundle or sbundle.
+	DropRevertibleTxOnErr bool
+
 	// EnforceProfit is true if we want to enforce a minimum profit threshold
 	// for committing a transaction based on ProfitThresholdPercent
 	EnforceProfit bool
-	// ExpectedProfit should be set on a per transaction basis when profit is enforced
-	ExpectedProfit *big.Int
+
 	// ProfitThresholdPercent is the minimum profit threshold for committing a transaction
-	ProfitThresholdPercent *big.Int
+	ProfitThresholdPercent int // 0-100, e.g. 70 means 70%
+
 	// PriceCutoffPercent is the minimum effective gas price threshold used for bucketing transactions by price.
 	// For example if the top transaction in a list has an effective gas price of 1000 wei and PriceCutoffPercent
 	// is 10 (i.e. 10%), then the minimum effective gas price included in the same bucket as the top transaction
@@ -134,7 +139,11 @@ func checkInterrupt(i *int32) bool {
 
 // Simulate bundle on top of current state without modifying it
 // pending txs used to track if bundle tx is part of the mempool
-func applyTransactionWithBlacklist(signer types.Signer, config *params.ChainConfig, bc core.ChainContext, author *common.Address, gp *core.GasPool, statedb *state.StateDB, header *types.Header, tx *types.Transaction, usedGas *uint64, cfg vm.Config, blacklist map[common.Address]struct{}) (*types.Receipt, *state.StateDB, error) {
+func applyTransactionWithBlacklist(
+	signer types.Signer, config *params.ChainConfig, bc core.ChainContext, author *common.Address, gp *core.GasPool,
+	statedb *state.StateDB, header *types.Header, tx *types.Transaction, usedGas *uint64,
+	cfg vm.Config, blacklist map[common.Address]struct{},
+) (*types.Receipt, *state.StateDB, error) {
 	// short circuit if blacklist is empty
 	if len(blacklist) == 0 {
 		snap := statedb.Snapshot()
@@ -192,17 +201,18 @@ func applyTransactionWithBlacklist(signer types.Signer, config *params.ChainConf
 
 // commit tx to envDiff
 func (envDiff *environmentDiff) commitTx(tx *types.Transaction, chData chainData) (*types.Receipt, int, error) {
-	header := envDiff.header
-	coinbase := &envDiff.baseEnvironment.coinbase
-	signer := envDiff.baseEnvironment.signer
+	var (
+		header   = envDiff.header
+		coinbase = &envDiff.baseEnvironment.coinbase
+		signer   = envDiff.baseEnvironment.signer
+	)
 
 	gasPrice, err := tx.EffectiveGasTip(header.BaseFee)
 	if err != nil {
 		return nil, shiftTx, err
 	}
 
 	envDiff.state.SetTxContext(tx.Hash(), envDiff.baseEnvironment.tcount+len(envDiff.newTxs))
-
 	receipt, newState, err := applyTransactionWithBlacklist(signer, chData.chainConfig, chData.chain, coinbase,
 		envDiff.gasPool, envDiff.state, header, tx, &header.GasUsed, *chData.chain.GetVMConfig(), chData.blacklist)
 
@@ -250,15 +260,19 @@ func (envDiff *environmentDiff) commitTx(tx *types.Transaction, chData chainData
 
 // Commit Bundle to env diff
 func (envDiff *environmentDiff) commitBundle(bundle *types.SimulatedBundle, chData chainData, interrupt *int32, algoConf algorithmConfig) error {
-	coinbase := envDiff.baseEnvironment.coinbase
-	tmpEnvDiff := envDiff.copy()
+	var (
+		coinbase   = envDiff.baseEnvironment.coinbase
+		tmpEnvDiff = envDiff.copy()
 
-	coinbaseBalanceBefore := tmpEnvDiff.state.GetBalance(coinbase)
+		coinbaseBalanceBefore = tmpEnvDiff.state.GetBalance(coinbase)
 
-	profitBefore := new(big.Int).Set(tmpEnvDiff.newProfit)
-	var gasUsed uint64
+		profitBefore = new(big.Int).Set(tmpEnvDiff.newProfit)
+
+		gasUsed uint64
+	)
 
 	for _, tx := range bundle.OriginalBundle.Txs {
+		txHash := tx.Hash()
 		if tmpEnvDiff.header.BaseFee != nil && tx.Type() == types.DynamicFeeTxType {
 			// Sanity check for extremely large numbers
 			if tx.GasFeeCap().BitLen() > 256 {
@@ -294,13 +308,28 @@ func (envDiff *environmentDiff) commitBundle(bundle *types.SimulatedBundle, chDa
 		receipt, _, err := tmpEnvDiff.commitTx(tx, chData)
 
 		if err != nil {
-			log.Trace("Bundle tx error", "bundle", bundle.OriginalBundle.Hash, "tx", tx.Hash(), "err", err)
+			isRevertibleTx := bundle.OriginalBundle.RevertingHash(txHash)
+			// if drop enabled, and revertible tx has error on commit, we skip the transaction and continue with next one
+			if algoConf.DropRevertibleTxOnErr && isRevertibleTx {
+				log.Trace("Found error on commit for revertible tx, but discard on err is enabled so skipping.",
+					"tx", txHash, "err", err)
+				continue
+			}
+
+			log.Trace("Bundle tx error", "bundle", bundle.OriginalBundle.Hash, "tx", txHash, "err", err)
 			return err
 		}
 
-		if receipt.Status != types.ReceiptStatusSuccessful && !bundle.OriginalBundle.RevertingHash(tx.Hash()) {
-			log.Trace("Bundle tx failed", "bundle", bundle.OriginalBundle.Hash, "tx", tx.Hash(), "err", err)
-			return errors.New("bundle tx revert")
+		if receipt != nil {
+			if receipt.Status == types.ReceiptStatusFailed && !bundle.OriginalBundle.RevertingHash(txHash) {
+				// if transaction reverted and isn't specified as reverting hash, return error
+				log.Trace("Bundle tx failed", "bundle", bundle.OriginalBundle.Hash, "tx", txHash, "err", err)
+				return errors.New("bundle tx revert")
+			}
+		} else {
+			// NOTE: The expectation is that a receipt is only nil if an error occurred.
+			//  If there is no error but receipt is nil, there is likely a programming error.
+			return errors.New("invalid receipt when no error occurred")
 		}
 
 		gasUsed += receipt.GasUsed
@@ -311,7 +340,8 @@ func (envDiff *environmentDiff) commitBundle(bundle *types.SimulatedBundle, chDa
 
 	bundleProfit := coinbaseBalanceDelta
 
-	bundleActualEffGP := bundleProfit.Div(bundleProfit, big.NewInt(int64(gasUsed)))
+	gasUsedBigInt := new(big.Int).SetUint64(gasUsed)
+	bundleActualEffGP := bundleProfit.Div(bundleProfit, gasUsedBigInt)
 	bundleSimEffGP := new(big.Int).Set(bundle.MevGasPrice)
 
 	// allow >-1% divergence
@@ -329,11 +359,11 @@ func (envDiff *environmentDiff) commitBundle(bundle *types.SimulatedBundle, chDa
 	if algoConf.EnforceProfit {
 		// if profit is enforced between simulation and actual commit, only allow ProfitThresholdPercent divergence
 		simulatedBundleProfit := new(big.Int).Set(bundle.TotalEth)
-		actualBundleProfit := new(big.Int).Mul(bundleActualEffGP, big.NewInt(int64(gasUsed)))
+		actualBundleProfit := new(big.Int).Mul(bundleActualEffGP, gasUsedBigInt)
 
 		// We want to make simulated profit smaller to allow for some leeway in cases where the actual profit is
 		// lower due to transaction ordering
-		simulatedProfitMultiple := new(big.Int).Mul(simulatedBundleProfit, algoConf.ProfitThresholdPercent)
+		simulatedProfitMultiple := common.PercentOf(simulatedBundleProfit, algoConf.ProfitThresholdPercent)
 		actualProfitMultiple := new(big.Int).Mul(actualBundleProfit, common.Big100)
 
 		if simulatedProfitMultiple.Cmp(actualProfitMultiple) > 0 {
@@ -478,7 +508,7 @@ func (envDiff *environmentDiff) commitSBundle(b *types.SimSBundle, chData chainD
 	coinbaseBefore := tmpEnvDiff.state.GetBalance(tmpEnvDiff.header.Coinbase)
 	gasBefore := tmpEnvDiff.gasPool.Gas()
 
-	if err := tmpEnvDiff.commitSBundleInner(b.Bundle, chData, interrupt, key); err != nil {
+	if err := tmpEnvDiff.commitSBundleInner(b.Bundle, chData, interrupt, key, algoConf); err != nil {
 		return err
 	}
 
@@ -507,13 +537,13 @@ func (envDiff *environmentDiff) commitSBundle(b *types.SimSBundle, chData chainD
 	}
 
 	if algoConf.EnforceProfit {
-		// if profit is enforced between simulation and actual commit, only allow >-1% divergence
+		// if profit is enforced between simulation and actual commit, only allow ProfitThresholdPercent divergence
 		simulatedSbundleProfit := new(big.Int).Set(b.Profit)
 		actualSbundleProfit := new(big.Int).Set(coinbaseDelta)
 
 		// We want to make simulated profit smaller to allow for some leeway in cases where the actual profit is
 		// lower due to transaction ordering
-		simulatedProfitMultiple := new(big.Int).Mul(simulatedSbundleProfit, algoConf.ProfitThresholdPercent)
+		simulatedProfitMultiple := common.PercentOf(simulatedSbundleProfit, algoConf.ProfitThresholdPercent)
 		actualProfitMultiple := new(big.Int).Mul(actualSbundleProfit, common.Big100)
 
 		if simulatedProfitMultiple.Cmp(actualProfitMultiple) > 0 {
@@ -529,7 +559,9 @@ func (envDiff *environmentDiff) commitSBundle(b *types.SimSBundle, chData chainD
 	return nil
 }
 
-func (envDiff *environmentDiff) commitSBundleInner(b *types.SBundle, chData chainData, interrupt *int32, key *ecdsa.PrivateKey) error {
+func (envDiff *environmentDiff) commitSBundleInner(
+	b *types.SBundle, chData chainData, interrupt *int32, key *ecdsa.PrivateKey, algoConf algorithmConfig,
+) error {
 	// check inclusion
 	minBlock := b.Inclusion.BlockNumber
 	maxBlock := b.Inclusion.MaxBlockNumber
@@ -548,9 +580,7 @@ func (envDiff *environmentDiff) commitSBundleInner(b *types.SBundle, chData chai
 	var (
 		totalProfit      *big.Int = new(big.Int)
 		refundableProfit *big.Int = new(big.Int)
-	)
 
-	var (
 		coinbaseDelta  = new(big.Int)
 		coinbaseBefore *big.Int
 	)
@@ -561,14 +591,22 @@ func (envDiff *environmentDiff) commitSBundleInner(b *types.SBundle, chData chai
 
 		if el.Tx != nil {
 			receipt, _, err := envDiff.commitTx(el.Tx, chData)
+
 			if err != nil {
+				// if drop enabled, and revertible tx has error on commit,
+				// we skip the transaction and continue with next one
+				if algoConf.DropRevertibleTxOnErr && el.CanRevert {
+					log.Trace("Found error on commit for revertible tx, but discard on err is enabled so skipping.",
+						"tx", el.Tx.Hash(), "err", err)
+					continue
+				}
 				return err
 			}
 			if receipt.Status != types.ReceiptStatusSuccessful && !el.CanRevert {
 				return errors.New("tx failed")
 			}
 		} else if el.Bundle != nil {
-			err := envDiff.commitSBundleInner(el.Bundle, chData, interrupt, key)
+			err := envDiff.commitSBundleInner(el.Bundle, chData, interrupt, key, algoConf)
 			if err != nil {
 				return err
 			}

miner/algo_common_test.go

@@ -280,6 +280,7 @@ func TestTxCommit(t *testing.T) {
 }
 
 func TestBundleCommit(t *testing.T) {
+	algoConf := defaultAlgorithmConfig
 	statedb, chData, signers := genTestSetup()
 
 	env := newEnvironment(chData, statedb, signers.addresses[0], GasLimit, big.NewInt(1))
@@ -298,7 +299,7 @@ func TestBundleCommit(t *testing.T) {
 		t.Fatal("Failed to simulate bundle", err)
 	}
 
-	err = envDiff.commitBundle(&simBundle, chData, nil, defaultAlgorithmConfig)
+	err = envDiff.commitBundle(&simBundle, chData, nil, algoConf)
 	if err != nil {
 		t.Fatal("Failed to commit bundle", err)
 	}

miner/algo_greedy.go

@@ -20,17 +20,22 @@ type greedyBuilder struct {
 	chainData        chainData
 	builderKey       *ecdsa.PrivateKey
 	interrupt        *int32
+	algoConf         algorithmConfig
 }
 
 func newGreedyBuilder(
-	chain *core.BlockChain, chainConfig *params.ChainConfig,
+	chain *core.BlockChain, chainConfig *params.ChainConfig, algoConf *algorithmConfig,
 	blacklist map[common.Address]struct{}, env *environment, key *ecdsa.PrivateKey, interrupt *int32,
 ) *greedyBuilder {
+	if algoConf == nil {
+		algoConf = &defaultAlgorithmConfig
+	}
 	return &greedyBuilder{
 		inputEnvironment: env,
 		chainData:        chainData{chainConfig, chain, blacklist},
 		builderKey:       key,
 		interrupt:        interrupt,
+		algoConf:         *algoConf,
 	}
 }
 
@@ -66,7 +71,7 @@ func (b *greedyBuilder) mergeOrdersIntoEnvDiff(
 			}
 		} else if bundle := order.Bundle(); bundle != nil {
 			//log.Debug("buildBlock considering bundle", "egp", bundle.MevGasPrice.String(), "hash", bundle.OriginalBundle.Hash)
-			err := envDiff.commitBundle(bundle, b.chainData, b.interrupt, defaultAlgorithmConfig)
+			err := envDiff.commitBundle(bundle, b.chainData, b.interrupt, b.algoConf)
 			orders.Pop()
 			if err != nil {
 				log.Trace("Could not apply bundle", "bundle", bundle.OriginalBundle.Hash, "err", err)
@@ -79,7 +84,7 @@ func (b *greedyBuilder) mergeOrdersIntoEnvDiff(
 			usedEntry := types.UsedSBundle{
 				Bundle: sbundle.Bundle,
 			}
-			err := envDiff.commitSBundle(sbundle, b.chainData, b.interrupt, b.builderKey, defaultAlgorithmConfig)
+			err := envDiff.commitSBundle(sbundle, b.chainData, b.interrupt, b.builderKey, b.algoConf)
 			orders.Pop()
 			if err != nil {
 				log.Trace("Could not apply sbundle", "bundle", sbundle.Bundle.Hash(), "err", err)