(Specifications) A bug in specifications with no direct impact on client implementations
Description
Brief/Intro
The SendRawTransaction() allows users to submit signed raw transactions to the local Erigon nodes, which contains a transaction fee check to ensure the transaction does not provide too many fees.
However, the transaction fees calculation is incorrect due to incorrect fetch of gas price for the transaction types, DynamicFeeTxType, BlobTxType and SetCodeTxType. Specifically, the gas price fetched from these types of transactions is the Tip instead of the FeeCap. This overlook makes this transaction fees ineffective as the fee cap is normally much larger than the tip.
// SendRawTransaction implements eth_sendRawTransaction. Creates new message call transaction or a contract creation for previously-signed transactions.
func (api *APIImpl) SendRawTransaction(ctx context.Context, encodedTx hexutility.Bytes) (common.Hash, error) {
txn, err := types.DecodeWrappedTransaction(encodedTx)
if err != nil {
return common.Hash{}, err
}
// If the transaction fee cap is already specified, ensure the
// fee of the given transaction is _reasonable_.
if err := checkTxFee(txn.GetPrice().ToBig(), txn.GetGas(), api.FeeCap); err != nil {
return common.Hash{}, err
}
if !txn.Protected() && !api.AllowUnprotectedTxs {
return common.Hash{}, errors.New("only replay-protected (EIP-155) transactions allowed over RPC")
}
// this has been moved to prior to adding of transactions to capture the
// pre state of the db - which is used for logging in the messages below
tx, err := api.db.BeginTemporalRo(ctx)
if err != nil {
return common.Hash{}, err
}
defer tx.Rollback()
cc, err := api.chainConfig(ctx, tx)
if err != nil {
return common.Hash{}, err
}
if txn.Protected() {
txnChainId := txn.GetChainID()
chainId := cc.ChainID
if chainId.Cmp(txnChainId.ToBig()) != 0 {
return common.Hash{}, fmt.Errorf("invalid chain id, expected: %d got: %d", chainId, *txnChainId)
}
}
hash := txn.Hash()
res, err := api.txPool.Add(ctx, &txPoolProto.AddRequest{RlpTxs: [][]byte{encodedTx}})
if err != nil {
return common.Hash{}, err
}
if res.Imported[0] != txPoolProto.ImportResult_SUCCESS {
return hash, fmt.Errorf("%s: %s", txPoolProto.ImportResult_name[int32(res.Imported[0])], res.Errors[0])
}
return txn.Hash(), nil
}
It first decodes the raw transaction and calls the function checkTxFee() to ensure the provided transaction fee does not exceed the pre-configured 1 Ether.
// checkTxFee is an internal function used to check whether the fee of
// the given transaction is _reasonable_(under the cap).
func checkTxFee(gasPrice *big.Int, gas uint64, gasCap float64) error {
// Short circuit if there is no gasCap for transaction fee at all.
if gasCap == 0 {
return nil
}
feeEth := new(big.Float).Quo(new(big.Float).SetInt(new(big.Int).Mul(gasPrice, new(big.Int).SetUint64(gas))), new(big.Float).SetInt(big.NewInt(params.Ether)))
feeFloat, _ := feeEth.Float64()
if feeFloat > gasCap {
return fmt.Errorf("tx fee (%.2f ether) exceeds the configured cap (%.2f ether)", feeFloat, gasCap)
}
return nil
}
The issue is that the gas price fetched from the transaction is mistakenly set as the tip in the transaction types, DynamicFeeTxType, BlobTxType and SetCodeTxType. For example, if the transaction if of type DynamicFeeTxType:
The function GetPrice() returns the Tip in the transaction, which should be the FeeCap.
In this case, checkTxFee() does not validate the transaction fees correctly, as the Tip in a transaction is much less than the FeeCap.
For example, even though the FeeCap exceeds the limit, the Tip does not. Consequently, the check does not fail.
Impact Details
The incorrect fetch of gas price of transactions makes the transaction fee validation ineffective. For example, a transaction with a transaction fee exceeding the limit does not fail.
For simplicity, we modified the test function TestSendRawTransaction() in the file: https://github.com/erigontech/erigon/blob/v3.0.0-alpha7/turbo/jsonrpc/send_transaction_test.go#L93
Create a transaction of type DynamicFeeTxType and submit it:
func TestSendRawTransaction(t *testing.T) {
mockSentry, require := mock.MockWithTxPool(t), require.New(t)
logger := log.New()
oneBlockStep(mockSentry, require, t)
expectedValue := uint64(1234)
//txn, err := types.SignTx(types.NewTransaction(0, common.Address{1}, uint256.NewInt(expectedValue), params.TxGas, uint256.NewInt(10*params.GWei), nil), *types.LatestSignerForChainID(mockSentry.ChainConfig.ChainID), mockSentry.Key)
txn, err := types.SignTx(types.NewEIP1559Transaction(uint256.Int{1337}, 0, common.Address{1}, uint256.NewInt(expectedValue), 1_000_000, uint256.NewInt(2000_000), uint256.NewInt(3000_000), uint256.NewInt(4000_000), nil), *types.LatestSignerForChainID(mockSentry.ChainConfig.ChainID), mockSentry.Key)
require.NoError(err)
fmt.Printf("gas limit is %d \n", txn.GetGas())
fmt.Printf("gas price is %d \n", txn.GetPrice())
fmt.Printf("tip is %d \n", txn.GetTip())
fmt.Printf("fee cap is %d \n", txn.GetFeeCap())
fmt.Printf("gas price is equal to tip: %t \n", txn.GetPrice() == txn.GetTip())
fmt.Printf("gas price is equal to fee cap: %t \n", txn.GetPrice() == txn.GetFeeCap())
ctx, conn := rpcdaemontest.CreateTestGrpcConn(t, mockSentry)
txPool := txpool.NewTxpoolClient(conn)
ff := rpchelper.New(ctx, rpchelper.DefaultFiltersConfig, nil, txPool, txpool.NewMiningClient(conn), func() {}, mockSentry.Log)
api := jsonrpc.NewEthAPI(newBaseApiForTest(mockSentry), mockSentry.DB, nil, txPool, nil, 5000000, 1e18, 100_000, false, 100_000, 128, logger)
buf := bytes.NewBuffer(nil)
err = txn.MarshalBinary(buf)
require.NoError(err)
txsCh, id := ff.SubscribePendingTxs(1)
defer ff.UnsubscribePendingTxs(id)
txHash, err := api.SendRawTransaction(ctx, buf.Bytes())
require.NoError(err)
select {
case got := <-txsCh:
require.Equal(expectedValue, got[0].GetValue().Uint64())
case <-time.After(20 * time.Second): // Sometimes the channel times out on github actions
t.Log("Timeout waiting for txn from channel")
jsonTx, err := api.GetTransactionByHash(ctx, txHash)
require.NoError(err)
require.Equal(expectedValue, jsonTx.Value.Uint64())
}
//send same txn second time and expect error
_, err = api.SendRawTransaction(ctx, buf.Bytes())
require.NotNil(err)
expectedErr := txpool_proto.ImportResult_name[int32(txpool_proto.ImportResult_ALREADY_EXISTS)] + ": " + txpoolcfg.AlreadyKnown.String()
require.Equal(expectedErr, err.Error())
mockSentry.ReceiveWg.Wait()
//TODO: make propagation easy to test - now race
//time.Sleep(time.Second)
//sent := m.SentMessage(0)
//require.Equal(eth.ToProto[m.MultiClient.Protocol()][eth.NewPooledTransactionHashesMsg], sent.Id)
}
The test result shows that the GetPrice() fetches the Tip instead of the FeeCap.
=== RUN TestSendRawTransaction
gas limit is 1000000
gas price is 3000000
tip is 3000000
fee cap is 4000000
gas price is equal to tip: true
gas price is equal to fee cap: false
--- PASS: TestSendRawTransaction (0.18s)
PASS
Process finished with the exit code 0
