Skip to content

Support EIP-1559 transaction - #177

Open
samuerio wants to merge 2 commits into
safe-fndn:mainfrom
froghub-io:main
Open

Support EIP-1559 transaction#177
samuerio wants to merge 2 commits into
safe-fndn:mainfrom
froghub-io:main

Conversation

@samuerio

@samuerio samuerio commented Jun 2, 2023

Copy link
Copy Markdown

Allows creation of EIP-1559 deployment transaction if the blockchain supports EIP-1559.

Note: Filecoin network only supports EIP-1559 transaction and does not support legacy transaction. This feature is added for adaptation.

@FroghubMan FroghubMan mentioned this pull request Jun 2, 2023
Comment thread scripts/compile.ts Outdated
})
const provider = new ethers.providers.JsonRpcProvider(process.env.RPC)
const signer = ethers.Wallet.fromMnemonic(process.env.MNEMONIC!!).connect(provider)
//if the network supports EIP-1155 transaction, use the EIP-1155 transaction type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the comment seems to mention the incorrect eip number

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for your review, it's fixed now.

@Uxio0
Uxio0 requested a review from rmeissner June 2, 2023 10:18
Comment thread scripts/compile.ts
const signer = ethers.Wallet.fromMnemonic(process.env.MNEMONIC!!).connect(provider)
//if the network supports EIP-1559 transaction, use the EIP-1559 transaction type
const tx = await signer.populateTransaction({
nonce, gasPrice, gasLimit, value, data, chainId

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you using EIP-1559 here? You haven't specified the transaction type and you are not using the maxPriorityFeePerGas or maxBaseFeePerGas here, right

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also confused for a while, but the key here seems to be using the .populateTransaction method that adds all the necessary fields, including EIP-1559 gas fields. So the comment could be better 100%.

Also, possibly we'd need to support multiple transaction types because, like networks only supporting eip-1559 transactions, there are networks that only support legacy ones.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, didn't know the populateTransaction method does that by default

@FroghubMan FroghubMan Jun 3, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function sendTransaction in ethers.js' abstract Signer is as follows:

// Populates all fields in a transaction, signs it and sends it to the network
async sendTransaction(transaction: Deferrable<TransactionRequest>): Promise<TransactionResponse> {
    this._checkProvider("sendTransaction");
    const tx = await this.populateTransaction(transaction);
    const signedTx = await this.signTransaction(tx);
    return await this.provider.sendTransaction(signedTx);
}

Before signing the transaction, it calls the populateTransaction method to fill in the relevant transaction fields, including the identification of the transaction type. If the type field is not provided, it checks whether the current chain supports EIP-1559. If it does, it populates the maxFeePerGas and maxPriorityFeePerGas fields and generates an EIP-1559 transaction. Otherwise, it generates a Legacy Transaction.

The current adaptation is made to align with the default behavior of the ethers.js library in order to support all EVM-compatible chains.

@samuerio

Copy link
Copy Markdown
Author

@rmeissner We would like to request your review on our PR as your feedback would be greatly appreciated 😄

@AlcibiadesCleinias

AlcibiadesCleinias commented Mar 13, 2024

Copy link
Copy Markdown

Hi, @rmeissner, @rmeissner I kindly want to up this thread, as I and my colleagues from FluenceLabs want to manage our contract deployment and contract calls based on chains without legacy transaction support (like Calibration and Filecoin mainnet).

Also, I want to note, that it seems that for now I and my colleagues will go with our-self fork, but I am ready to contribute to this PR to be delivered to main

@rmeissner
rmeissner removed their request for review July 8, 2025 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants