Toggleable discard for failed revertible transaction hashes

builder/builder.go

@@ -30,6 +30,8 @@ import (
 )
 
 const (
+	DiscardRevertedHashesDefault = false
+
 	RateLimitIntervalDefault     = 500 * time.Millisecond
 	RateLimitBurstDefault        = 10
 	BlockResubmitIntervalDefault = 500 * time.Millisecond
@@ -72,6 +74,7 @@ type Builder struct {
 	builderPublicKey            phase0.BLSPubKey
 	builderSigningDomain        phase0.Domain
 	builderResubmitInterval     time.Duration
+	discardRevertedHashes       bool
 
 	limiter                       *rate.Limiter
 	submissionOffsetFromEndOfSlot time.Duration
@@ -91,6 +94,7 @@ type BuilderArgs struct {
 	relay                         IRelay
 	builderSigningDomain          phase0.Domain
 	builderBlockResubmitInterval  time.Duration
+	discardRevertedHashes         bool
 	eth                           IEthereumService
 	dryRun                        bool
 	ignoreLatePayloadAttributes   bool
@@ -136,6 +140,7 @@ func NewBuilder(args BuilderArgs) (*Builder, error) {
 		builderPublicKey:              pk,
 		builderSigningDomain:          args.builderSigningDomain,
 		builderResubmitInterval:       args.builderBlockResubmitInterval,
+		discardRevertedHashes:         args.discardRevertedHashes,
 		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"`
+	DiscardRevertedHashes            bool          `toml:",omitempty"`
 	EnableCancellations              bool          `toml:",omitempty"`
 }
 

builder/service.go

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

cmd/geth/main.go

@@ -179,6 +179,7 @@ var (
 		utils.BuilderRateLimitMaxBurst,
 		utils.BuilderBlockResubmitInterval,
 		utils.BuilderSubmissionOffset,
+		utils.BuilderDiscardRevertedHashes,
 		utils.BuilderEnableCancellations,
 	}
 

cmd/utils/flags.go

@@ -845,6 +845,16 @@ var (
 		Category: flags.BuilderCategory,
 	}
 
+	BuilderDiscardRevertedHashes = &cli.BoolFlag{
+		Name: "builder.discard_reverted_hashes",
+		Usage: "When enabled, if a transaction submitted as part of a bundle in a send bundle request is reverted, " +
+			"and its hash is specified as one that can revert in the request body, the builder will discard the hash of the reverted 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_REVERTED_HASHES"},
+		Value:    builder.DiscardRevertedHashesDefault,
+		Category: flags.BuilderCategory,
+	}
+
 	BuilderEnableCancellations = &cli.BoolFlag{
 		Name:     "builder.cancellations",
 		Usage:    "Enable cancellations for the builder",
@@ -1668,6 +1678,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.DiscardRevertedHashes = ctx.Bool(BuilderDiscardRevertedHashes.Name)
 	cfg.EnableCancellations = ctx.IsSet(BuilderEnableCancellations.Name)
 }
 

core/state/statedb.go

@@ -129,6 +129,12 @@ type StateDB struct {
 	StorageDeleted int
 }
 
+// OriginalRoot returns the pre-state root hash before any changes were made.
+// NOTE: This method is not safe for concurrent use.
+func (s *StateDB) OriginalRoot() common.Hash {
+	return s.originalRoot
+}
+
 // New creates a new state from a given trie.
 func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) {
 	tr, err := db.OpenTrie(root)

miner/algo_common.go

@@ -27,11 +27,10 @@ const (
 const defaultProfitPercentMinimum = 70
 
 var (
-	defaultProfitThreshold = big.NewInt(defaultProfitPercentMinimum)
 	defaultAlgorithmConfig = algorithmConfig{
-		EnforceProfit:          false,
-		ExpectedProfit:         common.Big0,
-		ProfitThresholdPercent: defaultProfitThreshold,
+		DropTransactionOnRevert: false,
+		EnforceProfit:           false,
+		ProfitThresholdPercent:  defaultProfitPercentMinimum,
 	}
 )
 
@@ -56,13 +55,14 @@ func (e *lowProfitError) Error() string {
 }
 
 type algorithmConfig struct {
+	// DropTransactionOnRevert is set when a transaction is allowed to revert and we wish to not only revert the transaction
+	// but discard it entirely
+	DropTransactionOnRevert 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
 }
 
 type chainData struct {
@@ -124,7 +124,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()
@@ -181,21 +185,40 @@ 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
+func (envDiff *environmentDiff) commitTx(tx *types.Transaction, chData chainData, algoConf algorithmConfig) (*types.Receipt, int, error) {
+	var (
+		header   = envDiff.header
+		coinbase = &envDiff.baseEnvironment.coinbase
+		signer   = envDiff.baseEnvironment.signer
+		root     = envDiff.state.OriginalRoot()
+	)
 
 	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)
 
+	// when drop transaction is enabled:
+	// 1. if there was an error applying the transaction OR
+	// 2. if the transaction receipt status is failed
+	// we don't apply the transaction to the state and we don't add it to the newTxs, return early
+	if algoConf.DropTransactionOnRevert && (err != nil || (receipt != nil && receipt.Status == types.ReceiptStatusFailed)) {
+		// the StateDB for environment diff is already modified at this point, since it gets mutated when passed in to
+		// applyTransactionWithBlacklist, so we need to revert it
+		s, sdbErr := state.New(root, envDiff.state.Database(), chData.chain.Snapshots())
+		// if we cannot create a new state from the existing database, snapshots, and intermediate root hash,
+		// we panic because this is a fatal error
+		if sdbErr != nil {
+			panic(sdbErr)
+		}
+		envDiff.state = s
+		return receipt, shiftTx, err
+	}
+
 	envDiff.state = newState
 	if err != nil {
 		switch {
@@ -240,13 +263,23 @@ 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 (
+		// Store the initial value for DropTransactionOnRevert as it will be mutated in the loop depending on whether a given transaction is revertible or not.
+		// Only sbundles and bundles currently support specifying revertible transactions.
+		discard    = algoConf.DropTransactionOnRevert
+		coinbase   = envDiff.baseEnvironment.coinbase
+		tmpEnvDiff = envDiff.copy()
+
+		coinbaseBalanceBefore = tmpEnvDiff.state.GetBalance(coinbase)
 
-	coinbaseBalanceBefore := tmpEnvDiff.state.GetBalance(coinbase)
+		profitBefore = new(big.Int).Set(tmpEnvDiff.newProfit)
 
-	profitBefore := new(big.Int).Set(tmpEnvDiff.newProfit)
-	var gasUsed uint64
+		gasUsed uint64
+
+		hasErr           bool
+		hasReceiptFailed bool
+		canDiscard       bool
+	)
 
 	for _, tx := range bundle.OriginalBundle.Txs {
 		if tmpEnvDiff.header.BaseFee != nil && tx.Type() == types.DynamicFeeTxType {
@@ -281,14 +314,36 @@ func (envDiff *environmentDiff) commitBundle(bundle *types.SimulatedBundle, chDa
 			return errInterrupt
 		}
 
-		receipt, _, err := tmpEnvDiff.commitTx(tx, chData)
+		// if drop transaction on revert is enabled and the transaction is found in list of reverting transaction hashes,
+		// we can consider the transaction for discarding
+		canDiscard = discard && bundle.OriginalBundle.RevertingHash(tx.Hash())
+		algoConf.DropTransactionOnRevert = canDiscard
 
-		if err != nil {
+		receipt, _, err := tmpEnvDiff.commitTx(tx, chData, algoConf)
+
+		// reset the drop transaction on revert flag for subsequent transactions
+		algoConf.DropTransactionOnRevert = discard
+
+		hasErr = err != nil
+		hasReceiptFailed = receipt != nil && receipt.Status == types.ReceiptStatusFailed
+
+		// if drop transaction on revert is enabled and the transaction is found in list of reverting transaction hashes:
+		// 1. when there was an error applying the transaction OR
+		// 2. the transaction failed
+		//
+		// we skip the transaction and continue with the next one
+		if canDiscard && (hasErr || hasReceiptFailed) {
+			log.Trace("Failed to commit bundle transaction, but drop transaction on revert is enabled, skipping",
+				"bundle", bundle.OriginalBundle.Hash, "tx", tx.Hash(), "err", err, "receipt-failed", hasReceiptFailed)
+			continue
+		}
+
+		if hasErr {
 			log.Trace("Bundle tx error", "bundle", bundle.OriginalBundle.Hash, "tx", tx.Hash(), "err", err)
 			return err
 		}
 
-		if receipt.Status != types.ReceiptStatusSuccessful && !bundle.OriginalBundle.RevertingHash(tx.Hash()) {
+		if hasReceiptFailed && !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")
 		}
@@ -301,7 +356,16 @@ func (envDiff *environmentDiff) commitBundle(bundle *types.SimulatedBundle, chDa
 
 	bundleProfit := coinbaseBalanceDelta
 
-	bundleActualEffGP := bundleProfit.Div(bundleProfit, big.NewInt(int64(gasUsed)))
+	var (
+		gasUsedBigInt     = new(big.Int).SetUint64(gasUsed)
+		bundleActualEffGP *big.Int
+	)
+	if gasUsed == 0 {
+		bundleActualEffGP = common.Big0
+	} else {
+		bundleActualEffGP = bundleProfit.Div(bundleProfit, gasUsedBigInt)
+	}
+
 	bundleSimEffGP := new(big.Int).Set(bundle.MevGasPrice)
 
 	// allow >-1% divergence
@@ -319,11 +383,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 {
@@ -450,7 +514,7 @@ func (envDiff *environmentDiff) commitPayoutTx(amount *big.Int, sender, receiver
 		return nil, errors.New("incorrect sender private key")
 	}
 
-	receipt, _, err := envDiff.commitTx(tx, chData)
+	receipt, _, err := envDiff.commitTx(tx, chData, defaultAlgorithmConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -468,7 +532,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
 	}
 
@@ -497,13 +561,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 {
@@ -519,7 +583,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
@@ -536,11 +602,12 @@ func (envDiff *environmentDiff) commitSBundleInner(b *types.SBundle, chData chai
 	}
 
 	var (
+		// Store the initial value for DropTransactionOnRevert as it will be mutated in the loop depending on whether a given transaction is revertible or not.
+		// Only sbundles and bundles currently support specifying revertible transactions.
+		discard                   = algoConf.DropTransactionOnRevert
 		totalProfit      *big.Int = new(big.Int)
 		refundableProfit *big.Int = new(big.Int)
-	)
 
-	var (
 		coinbaseDelta  = new(big.Int)
 		coinbaseBefore *big.Int
 	)
@@ -550,15 +617,22 @@ func (envDiff *environmentDiff) commitSBundleInner(b *types.SBundle, chData chai
 		coinbaseBefore = envDiff.state.GetBalance(envDiff.header.Coinbase)
 
 		if el.Tx != nil {
-			receipt, _, err := envDiff.commitTx(el.Tx, chData)
+			// We only want to drop reverted transactions if they are specified as ones that can revert
+			// when they are submitted to the builder. Only bundles and sbundles currently support specifying
+			// revertible transactions.
+			algoConf.DropTransactionOnRevert = discard && el.CanRevert
+			receipt, _, err := envDiff.commitTx(el.Tx, chData, algoConf)
+			// reset value for subsequent transactions or bundles
+			algoConf.DropTransactionOnRevert = discard
+
 			if err != nil {
 				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
 			}