Deprecate gateway fee (#2120)

* Discard tx with gateway fee in state processor

* Reject txs with gateway fee in tx pools

* Deprecate gateway fee APIs in JS after hardfork

* Fix failing LES tests

* cleanup: Use available field

* Move `light.gatewayfee` flag to deprecated section

* Don't set gateway fee fields in tx after g fork

* Fix tx_pool tests

* Fix e2e transfer tests

* Pull test changes from monorepo

Author: palango

cmd/geth/usage.go

@@ -61,7 +61,6 @@ var AppHelpFlagGroups = []flags.FlagGroup{
 			utils.LightIngressFlag,
 			utils.LightEgressFlag,
 			utils.LightMaxPeersFlag,
-			utils.LightGatewayFeeFlag,
 			utils.UltraLightServersFlag,
 			utils.UltraLightFractionFlag,
 			utils.UltraLightOnlyAnnounceFlag,
@@ -231,6 +230,7 @@ var AppHelpFlagGroups = []flags.FlagGroup{
 			utils.LegacyWSApiFlag,
 			utils.LegacyGraphQLListenAddrFlag,
 			utils.LegacyGraphQLPortFlag,
+			utils.LightGatewayFeeFlag,
 		},
 	},
 	{

core/error.go

@@ -113,4 +113,8 @@ var (
 	// ErrUnprotectedTransaction is returned if replay protection is required (post-Donut) but the transaction doesn't
 	// use it.
 	ErrUnprotectedTransaction = errors.New("replay protection is required")
+
+	// ErrGatewayFeeDeprecated is returned when a transaction containing a gateway fee is encountered after the
+	// G hardfork
+	ErrGatewayFeeDeprecated = errors.New("gateway fee is deprecated")
 )

core/state_processor.go

@@ -85,17 +85,17 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg
 		baseFee *big.Int
 		sysCtx  *SysContractCallCtx
 	)
-	if p.bc.Config().IsEspresso(blockNumber) {
+	if p.config.IsEspresso(blockNumber) {
 		sysCtx = NewSysContractCallCtx(header, statedb, p.bc)
-		if p.bc.Config().FakeBaseFee != nil {
+		if p.config.FakeBaseFee != nil {
 			sysCtx = MockSysContractCallCtx(p.bc.Config().FakeBaseFee)
 		}
 	}
 	blockContext := NewEVMBlockContext(header, p.bc, nil)
 	vmenv := vm.NewEVM(blockContext, vm.TxContext{}, statedb, p.config, cfg)
 	// Iterate over and process the individual transactions
 	for i, tx := range block.Transactions() {
-		if p.bc.chainConfig.IsEspresso(header.Number) {
+		if p.config.IsEspresso(header.Number) {
 			baseFee = sysCtx.GetGasPriceMinimum(tx.FeeCurrency())
 		}
 		msg, err := tx.AsMessage(types.MakeSigner(p.config, header.Number), baseFee)
@@ -124,6 +124,15 @@ func applyTransaction(msg types.Message, config *params.ChainConfig, gp *GasPool
 		return nil, ErrUnprotectedTransaction
 	}
 
+	// CIP 57 deprecates full node incentives
+	// Check that neither `GatewayFeeRecipient` nor `GatewayFee` are set, otherwise reject the transaction
+	if config.IsGFork(blockNumber) {
+		gatewayFeeSet := !(msg.GatewayFee() == nil || msg.GatewayFee().Cmp(common.Big0) == 0)
+		if msg.GatewayFeeRecipient() != nil || gatewayFeeSet {
+			return nil, ErrGatewayFeeDeprecated
+		}
+	}
+
 	// Create a new context to be used in the EVM environment
 	txContext := NewEVMTxContext(msg)
 	evm.Reset(txContext, statedb)

core/tx_pool.go

@@ -276,6 +276,7 @@ type TxPool struct {
 	istanbul  bool // Fork indicator whether we are in the istanbul stage.
 	donut     bool // Fork indicator for the Donut fork.
 	espresso  bool // Fork indicator for the Espresso fork.
+	gfork     bool // Fork indicator for the G fork.
 
 	currentState    *state.StateDB // Current state in the blockchain head
 	currentVMRunner vm.EVMRunner   // Current EVMRunner
@@ -659,6 +660,12 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error {
 		return err
 	}
 
+	// CIP 57 deprecates full node incentives
+	gatewayFeeSet := !(tx.GatewayFee() == nil || tx.GatewayFee().Cmp(common.Big0) == 0)
+	if pool.gfork && (tx.GatewayFeeRecipient() != nil || gatewayFeeSet) {
+		return ErrGatewayFeeDeprecated
+	}
+
 	// Accept only legacy transactions until EIP-2718/2930 activates.
 	if !pool.espresso && tx.Type() != types.LegacyTxType {
 		return ErrTxTypeNotSupported
@@ -1412,6 +1419,7 @@ func (pool *TxPool) reset(oldHead, newHead *types.Header) {
 	pool.istanbul = pool.chainconfig.IsIstanbul(next)
 	pool.donut = pool.chainconfig.IsDonut(next)
 	pool.espresso = pool.chainconfig.IsEspresso(next)
+	pool.gfork = pool.chainconfig.IsGFork(next)
 }
 
 // promoteExecutables moves transactions that have become processable from the

core/tx_pool_test.go

@@ -341,7 +341,57 @@ func TestInvalidTransactions(t *testing.T) {
 		t.Error("expected", ErrIntrinsicGas, "got", err)
 	}
 
-	// TODO(joshua): Convert this to testAddGatewayFee
+	// Adding a gateway fee should result in deprecation error.
+	tx = lesTransaction(0, 100, big.NewInt(50), key)
+	if err := pool.AddRemote(tx); err != ErrGatewayFeeDeprecated {
+		t.Error("expected", ErrGatewayFeeDeprecated, "got", err)
+	}
+
+	// Should still return a deprecation error.
+	pool.currentState.AddBalance(from, tx.GatewayFee())
+	if err := pool.AddRemote(tx); err != ErrGatewayFeeDeprecated {
+		t.Error("expected", ErrGatewayFeeDeprecated, "got", err)
+	}
+
+	testSetNonce(pool, from, 1)
+	testAddBalance(pool, from, big.NewInt(0xffffffffffffff))
+
+	tx = transaction(0, 100000, key)
+	if err := pool.AddRemote(tx); !errors.Is(err, ErrNonceTooLow) {
+		t.Error("expected", ErrNonceTooLow)
+	}
+
+	tx = transaction(1, 100000, key)
+	pool.gasPrice = big.NewInt(1000)
+	if err := pool.AddRemote(tx); err != ErrUnderpriced {
+		t.Error("expected", ErrUnderpriced, "got", err)
+	}
+	if err := pool.AddLocal(tx); err != nil {
+		t.Error("expected", nil, "got", err)
+	}
+}
+
+func TestInvalidTransactionsPreGFork(t *testing.T) {
+	t.Parallel()
+
+	pool, key := setupTxPool()
+	defer pool.Stop()
+	pool.gfork = false
+
+	tx := transaction(0, 100, key)
+	from, _ := deriveSender(tx)
+
+	testAddBalance(pool, from, big.NewInt(1))
+	if err := pool.AddRemote(tx); !errors.Is(err, ErrInsufficientFunds) {
+		t.Error("expected", ErrInsufficientFunds)
+	}
+
+	balance := new(big.Int).Add(tx.Value(), new(big.Int).Mul(new(big.Int).SetUint64(tx.Gas()), tx.GasPrice()))
+	testAddBalance(pool, from, balance)
+	if err := pool.AddRemote(tx); !errors.Is(err, ErrIntrinsicGas) {
+		t.Error("expected", ErrIntrinsicGas, "got", err)
+	}
+
 	// Adding a gateway fee should result in insufficient funds again.
 	tx = lesTransaction(0, 100, big.NewInt(50), key)
 	if err := pool.AddRemote(tx); err != ErrInsufficientFunds {

e2e_test/e2e_transfer_test.go

@@ -11,6 +11,7 @@ import (
 	ethereum "github.com/celo-org/celo-blockchain"
 	"github.com/celo-org/celo-blockchain/common"
 	"github.com/celo-org/celo-blockchain/common/hexutil"
+	"github.com/celo-org/celo-blockchain/core"
 	"github.com/celo-org/celo-blockchain/core/types"
 	"github.com/celo-org/celo-blockchain/ethclient"
 	"github.com/celo-org/celo-blockchain/internal/ethapi"
@@ -31,7 +32,191 @@ const (
 func TestTransferCELO(t *testing.T) {
 	ac := test.AccountConfig(1, 3)
 	gc, ec, err := test.BuildConfig(ac)
-	gc.Hardforks.EspressoBlock = big.NewInt(0)
+	require.NoError(t, err)
+	network, shutdown, err := test.NewNetwork(ac, gc, ec)
+	require.NoError(t, err)
+	defer shutdown()
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
+	defer cancel()
+
+	node := network[0] // validator node
+	client := node.WsClient
+	devAccounts := test.Accounts(ac.DeveloperAccounts(), gc.ChainConfig())
+	sender := devAccounts[0]
+	recipient := devAccounts[1]
+	gateWayFeeRecipient := devAccounts[2]
+
+	// Get datum to set GasPrice/MaxFeePerGas/MaxPriorityFeePerGas to sensible values
+	header, err := network[0].WsClient.HeaderByNumber(ctx, common.Big0)
+	require.NoError(t, err)
+	datum, err := network[0].Eth.APIBackend.GasPriceMinimumForHeader(ctx, nil, header)
+	require.NoError(t, err)
+
+	testCases := []struct {
+		name        string
+		txArgs      *ethapi.TransactionArgs
+		expectedErr error
+	}{
+		{
+			name: "eth compatible LegacyTxType",
+			txArgs: &ethapi.TransactionArgs{
+				To:       &recipient.Address,
+				Value:    (*hexutil.Big)(new(big.Int).SetInt64(oneCelo)),
+				GasPrice: (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "eth incompatible LegacyTxType",
+			txArgs: &ethapi.TransactionArgs{
+				To:                  &recipient.Address,
+				Value:               (*hexutil.Big)(new(big.Int).SetInt64(oneCelo)),
+				GasPrice:            (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+				GatewayFee:          (*hexutil.Big)(new(big.Int).SetInt64(oneCelo / 10)),
+				GatewayFeeRecipient: &gateWayFeeRecipient.Address,
+			},
+			expectedErr: core.ErrGatewayFeeDeprecated,
+		},
+		{
+			name: "AccessListTxType",
+			txArgs: &ethapi.TransactionArgs{
+				To:         &recipient.Address,
+				Value:      (*hexutil.Big)(new(big.Int).SetInt64(oneCelo)),
+				GasPrice:   (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+				AccessList: &types.AccessList{},
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "DynamicFeeTxType - tip = MaxFeePerGas - BaseFee",
+			txArgs: &ethapi.TransactionArgs{
+				To:                   &recipient.Address,
+				Value:                (*hexutil.Big)(new(big.Int).SetInt64(oneCelo)),
+				MaxFeePerGas:         (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+				MaxPriorityFeePerGas: (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "DynamicFeeTxType - tip = MaxPriorityFeePerGas",
+			txArgs: &ethapi.TransactionArgs{
+				To:                   &recipient.Address,
+				Value:                (*hexutil.Big)(new(big.Int).SetInt64(oneCelo)),
+				MaxFeePerGas:         (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+				MaxPriorityFeePerGas: (*hexutil.Big)(datum),
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "CeloDynamicFeeTxType - gas = MaxFeePerGas - BaseFee",
+			txArgs: &ethapi.TransactionArgs{
+				To:                   &recipient.Address,
+				Value:                (*hexutil.Big)(new(big.Int).SetInt64(oneCelo)),
+				MaxFeePerGas:         (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+				MaxPriorityFeePerGas: (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+				GatewayFee:           (*hexutil.Big)(new(big.Int).SetInt64(oneCelo / 10)),
+				GatewayFeeRecipient:  &gateWayFeeRecipient.Address,
+			},
+			expectedErr: core.ErrGatewayFeeDeprecated,
+		},
+		{
+			name: "CeloDynamicFeeTxType - MaxPriorityFeePerGas",
+			txArgs: &ethapi.TransactionArgs{
+				To:                   &recipient.Address,
+				Value:                (*hexutil.Big)(new(big.Int).SetInt64(oneCelo)),
+				MaxFeePerGas:         (*hexutil.Big)(datum.Mul(datum, new(big.Int).SetInt64(4))),
+				MaxPriorityFeePerGas: (*hexutil.Big)(datum),
+				GatewayFee:           (*hexutil.Big)(new(big.Int).SetInt64(oneCelo / 10)),
+				GatewayFeeRecipient:  &gateWayFeeRecipient.Address,
+			},
+			expectedErr: core.ErrGatewayFeeDeprecated,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			watcher := test.NewBalanceWatcher(client, []common.Address{sender.Address, recipient.Address, gateWayFeeRecipient.Address, node.Address})
+			blockNum, err := client.BlockNumber(ctx)
+			require.NoError(t, err)
+			signer := types.MakeSigner(devAccounts[0].ChainConfig, new(big.Int).SetUint64(blockNum))
+			tx, err := prepareTransaction(*tc.txArgs, sender.Key, sender.Address, signer, client)
+			require.NoError(t, err)
+			err = client.SendTransaction(ctx, tx)
+			if tc.expectedErr != nil {
+				// Once the error is checked, there's nothing more to do
+				if err.Error() != tc.expectedErr.Error() {
+					t.Error("Expected error", tc.expectedErr, "got", err)
+				}
+				return
+			}
+
+			require.NoError(t, err, "SendTransaction failed", "tx", *tx)
+			err = network.AwaitTransactions(ctx, tx)
+			require.NoError(t, err)
+			watcher.Update()
+			receipt, err := client.TransactionReceipt(ctx, tx.Hash())
+			require.NoError(t, err)
+
+			// check value goes to recipient
+			expected := tx.Value()
+			actual := watcher.Delta(recipient.Address)
+			assert.Equal(t, expected, actual, "Recipient's balance increase unexpected", "expected", expected.Int64(), "actual", actual.Int64())
+
+			// Check tip goes to validator
+			header, err := network[0].WsClient.HeaderByNumber(ctx, receipt.BlockNumber)
+			require.NoError(t, err)
+			gpm, err := network[0].Eth.APIBackend.GasPriceMinimumForHeader(ctx, nil, header)
+			require.NoError(t, err)
+			baseFee := new(big.Int).Mul(gpm, new(big.Int).SetUint64(receipt.GasUsed))
+			switch tx.Type() {
+			case types.LegacyTxType, types.AccessListTxType:
+				fee := new(big.Int).Mul(tx.GasPrice(), new(big.Int).SetUint64(receipt.GasUsed))
+				expected = new(big.Int).Sub(fee, baseFee)
+			case types.DynamicFeeTxType, types.CeloDynamicFeeTxType:
+				expected = tx.EffectiveGasTipValue(gpm)
+				expected.Mul(expected, new(big.Int).SetUint64(receipt.GasUsed))
+			}
+			actual = watcher.Delta(node.Address)
+			assert.Equal(t, expected, actual, "Validator's balance increase unexpected", "expected", expected.Int64(), "actual", actual.Int64())
+
+			// check value + tx fee + gateway fee are subtracted from sender
+			var fee *big.Int
+			switch tx.Type() {
+			case types.LegacyTxType, types.AccessListTxType:
+				fee = new(big.Int).Mul(tx.GasPrice(), new(big.Int).SetUint64(receipt.GasUsed))
+			case types.DynamicFeeTxType, types.CeloDynamicFeeTxType:
+				tip := tx.EffectiveGasTipValue(gpm)
+				tip.Mul(tip, new(big.Int).SetUint64(receipt.GasUsed))
+				fee = new(big.Int).Add(tip, baseFee)
+			}
+			consumed := new(big.Int).Add(tx.Value(), fee)
+			if tx.GatewayFeeRecipient() != nil && tx.GatewayFee() != nil {
+				consumed.Add(consumed, tx.GatewayFee())
+			}
+			expected = new(big.Int).Neg(consumed)
+			actual = watcher.Delta(sender.Address)
+			assert.Equal(t, expected, actual, "Sender's balance decrease unexpected", "expected", expected.Int64(), "actual", expected.Int64())
+
+			// Check gateway fee
+			if tx.GatewayFeeRecipient() != nil && tx.GatewayFee() != nil {
+				expected = tx.GatewayFee()
+				actual = watcher.Delta(gateWayFeeRecipient.Address)
+				assert.Equal(t, expected, actual, "gateWayFeeRecipient's balance increase unexpected", "expected", expected.Int64(), "actual", actual.Int64())
+			}
+		})
+	}
+}
+
+// TestTransferCELO checks following accounts:
+// - Sender account has transfer value, transaction fee deducted
+// - Receiver account has transfer value added.
+// - Governance account has base fee added.
+// - validator account has tip fee added.
+func TestTransferCELOPreGFork(t *testing.T) {
+	ac := test.AccountConfig(1, 3)
+	gc, ec, err := test.BuildConfig(ac)
+	gc.Hardforks.GForkBlock = nil
+
 	require.NoError(t, err)
 	network, shutdown, err := test.NewNetwork(ac, gc, ec)
 	require.NoError(t, err)

internal/ethapi/transaction_args.go

@@ -86,12 +86,14 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error {
 		return errors.New("both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified")
 	}
 	// After london, default to 1559 unless gasPrice is set
-	head := b.CurrentHeader()
+	currentBlockNumber := b.CurrentHeader().Number
+	isGFork := b.ChainConfig().IsGFork(currentBlockNumber)
+
 	// If user specifies both maxPriorityfee and maxFee, then we do not
 	// need to consult the chain for defaults. It's definitely a London tx.
 	if args.MaxPriorityFeePerGas == nil || args.MaxFeePerGas == nil {
 		// In this clause, user left some fields unspecified.
-		if b.ChainConfig().IsEspresso(head.Number) {
+		if b.ChainConfig().IsEspresso(currentBlockNumber) {
 			if args.GasPrice == nil || args.GasPrice.ToInt().Cmp(big.NewInt(0)) == 0 {
 				if args.MaxPriorityFeePerGas == nil {
 					tip, err := b.SuggestGasTipCap(ctx, args.FeeCurrency)
@@ -149,7 +151,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error {
 	if args.To == nil && len(args.data()) == 0 {
 		return errors.New(`contract creation without any data provided`)
 	}
-	if args.GatewayFeeRecipient == nil && !args.EthCompatible {
+	if args.GatewayFeeRecipient == nil && !args.EthCompatible && !isGFork {
 		recipient := b.GatewayFeeRecipient()
 		if (recipient != common.Address{}) {
 			args.GatewayFeeRecipient = &recipient
@@ -181,7 +183,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error {
 		args.Gas = &estimated
 		log.Trace("Estimate gas usage automatically", "gas", args.Gas)
 	}
-	if args.GatewayFeeRecipient != nil && args.GatewayFee == nil {
+	if args.GatewayFeeRecipient != nil && args.GatewayFee == nil && !isGFork {
 		args.GatewayFee = (*hexutil.Big)(b.GatewayFee())
 	}
 	if args.ChainID == nil {