Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction:inferType wrong behaviour #4858

Open
krlosMata opened this issue Oct 11, 2024 · 3 comments
Open

Transaction:inferType wrong behaviour #4858

krlosMata opened this issue Oct 11, 2024 · 3 comments
Assignees
Labels
investigate Under investigation and may be a bug. on-deck This Enhancement or Bug is currently being worked on. v6 Issues regarding v6

Comments

@krlosMata
Copy link

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

@krlosMata krlosMata added investigate Under investigation and may be a bug. v6 Issues regarding v6 labels Oct 11, 2024
@krlosMata krlosMata changed the title Add Bug Title Here Transaction:inferType wrong behaviour Oct 11, 2024
@ricmoo
Copy link
Member

ricmoo commented Oct 16, 2024

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

@ricmoo ricmoo added the on-deck This Enhancement or Bug is currently being worked on. label Oct 16, 2024
@krlosMata
Copy link
Author

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
Copy link
Author

I was about to do the PR, but someone already did it 😄
#4859

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigate Under investigation and may be a bug. on-deck This Enhancement or Bug is currently being worked on. v6 Issues regarding v6
Projects
None yet
Development

No branches or pull requests

2 participants