Transaction:inferType wrong behaviour

[krlosMata]

Ethers Version

6.8.0

Search Terms

inferType

Describe the Problem

Rationale

inferTypes() function adds type eip2930 incorrectly: https://github.com/ethers-io/ethers.js/blob/main/src.ts/transaction/transaction.ts#L1024

Error description

if the transaction has a gasPrice set, it directly adds to a possible transaction type the type: 1 (eip2930). Later on, it checks if it has an accessList, so it adds type: 0 (legacy).

Logic should be the other way around. If it has gasPrice, add type: 0 (legacy). And if it also has accessList, then add type: 1 (eip2930).

Since the function sorts the types and later on returns the highest one, it will always return type: 1 (eip2930) if it has gasPrice set.

Correct code

// Explicit type
        if (this.type != null) {
            types.push(this.type);

        } else {
            if (hasFee) {
                types.push(2);
            } else if (hasGasPrice) {
                types.push(0); // CORRECTED
                if (hasAccessList) { types.push(1); } // CORRECTED
            } else if (hasAccessList) {
                types.push(1);
                types.push(2);
            } else if (hasBlob && this.to) {
                types.push(3);
            } else {
                types.push(0);
                types.push(1);
                types.push(2);
                types.push(3);
            }
        }

Code Snippet

// Explicit type
        if (this.type != null) {
            types.push(this.type);

        } else {
            if (hasFee) {
                types.push(2);
            } else if (hasGasPrice) {
                types.push(1);
                if (!hasAccessList) { types.push(0); }
            } else if (hasAccessList) {
                types.push(1);
                types.push(2);
            } else if (hasBlob && this.to) {
                types.push(3);
            } else {
                types.push(0);
                types.push(1);
                types.push(2);
                types.push(3);
            }
        }

Contract ABI

No response

Errors

No response

Environment

node.js (v12 or newer)

Environment (Other)

No response

[ricmoo]

Thanks! Could you provide a short code snippet (like Transaction.from( { blah })) that demonstrates the issue?

[krlosMata]

Sure!

Code snippet

const ethers = require('ethers');

async function main() {
    // parse transaction with ethercv6
    const tx = {
        chainId: 0,
        nonce: 0,
        gasLimit: 21000,
        gasPrice: 0,
        value: 0,
        to: '0xb0f8fE07921F65dCf696016D0FbA45A697c0D536',
        value: ethers.parseEther('1.0'),
        signature: {
            v: "0x1b",
            r: "0x00000000000000000000000000000000000000000000000000000005ca1ab1e0",
            s: "0x000000000000000000000000000000000000000000000000000000005ca1ab1e"
        }
    };

    const txObject = ethers.Transaction.from(tx);
    console.log(txObject.unsignedSerialized); // internally call `inferType`
}

main();

Behaviour

It logs 0x01e680808082520894b0f8fe07921f65dcf696016d0fba45a697c0d536880de0b6b3a764000080c0 which is an eip2930 rlp encoded type

Expected behaviour

Since it has not accessList attribute, I would expect to be serialized as a legacy transaction.

[krlosMata]

I was about to do the PR, but someone already did it :smile: https://github.com/ethers-io/ethers.js/pull/4859

[dumbeng]

@ricmoo I was having the same issue with transaction type inferring (but with different transaction type). When I was creating a blob transaction, the type will always be inferred as dynamic fee transaction.

The following code snippet is the definition of blob transaction in geth, it shows that a blob transaction can only use EIP1559 fees.

// BlobTx represents an EIP-4844 transaction.
type BlobTx struct {
	ChainID    *uint256.Int
	Nonce      uint64
	GasTipCap  *uint256.Int // a.k.a. maxPriorityFeePerGas
	GasFeeCap  *uint256.Int // a.k.a. maxFeePerGas
	Gas        uint64
	To         common.Address
	Value      *uint256.Int
	Data       []byte
	AccessList AccessList
	BlobFeeCap *uint256.Int // a.k.a. maxFeePerBlobGas
	BlobHashes []common.Hash

	// A blob transaction can optionally contain blobs. This field must be set when BlobTx
	// is used to create a transaction for signing.
	Sidecar *BlobTxSidecar `rlp:"-"`

	// Signature values
	V *uint256.Int `json:"v" gencodec:"required"`
	R *uint256.Int `json:"r" gencodec:"required"`
	S *uint256.Int `json:"s" gencodec:"required"`
}

So which means, in the above transaction inferring code, the if statement for inferring the blob transaction should be at the top:

// Explicit type
        if (this.type != null) {
            types.push(this.type);

        } else {
            if (hasFee) {
                types.push(2);
            } else if (hasGasPrice) {
                types.push(1);
                if (!hasAccessList) { types.push(0); }
            } else if (hasAccessList) {
                types.push(1);
                types.push(2);
            } else if (hasBlob && this.to) { // <-- this should be at the top
                types.push(3);
            } else {
                types.push(0);
                types.push(1);
                types.push(2);
                types.push(3);
            }
        }

[ricmoo]

But if it has an gasPrice and an accessList it cannot be type 0. If it has a gasPrice I think we still want to infer type 1. We want to infer the highest possible type from the provided parameters.

I think the current logic accomplishes the intended outcome? But to be fair, inferring transaction type is already undecidable, so it is important to execute the action of least surprise. If a given TX is a valid type 0 and type 1, I want the highest type to be selected.

Does that make sense?