Support EIP-1559 transaction #177
Conversation
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 |
There was a problem hiding this comment.
the comment seems to mention the incorrect eip number
There was a problem hiding this comment.
thank you for your review, it's fixed now.
| 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I see, didn't know the populateTransaction method does that by default
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
sendTransaction does not work with all clients, and therefore should not be static. The main issue I have with doing any of these integrations is whether or not a joint library should be created to merge both ethers and ethers-quorum together.
There was a problem hiding this comment.
I apologize for missing my point. There are several methods of doing things to accomplish the goals in mind, and where I really was going with my comment was that the use of: populateTransaction should not be allowed if there are not corresponding calls in the other main clients.
|
@rmeissner We would like to request your review on our PR as your feedback would be greatly appreciated 😄 |
|
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 |
|
Can I be informed on the context of this message, please?
T.R.H. P.C. Walker, Esq
Roy Walker PLLC / Roy Walker Syndicate
USA: GVC / LAC / MBF / NYC / SEA
INTL: Dubai / Hong Kong / London / Zug
ICCoC, Interim Chief Justice
The Hague, Netherlands
From: Ivan ***@***.***>
Date: Wednesday, March 13, 2024 at 04:48
To: safe-global/safe-singleton-factory ***@***.***>
Cc: Pandora Walker ***@***.***>, Comment ***@***.***>
Subject: Re: [safe-global/safe-singleton-factory] Support EIP-1559 transaction (PR #177)
Hi, @rmeissner<https://github.com/rmeissner>, @rmeissner<https://github.com/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).
—
Reply to this email directly, view it on GitHub<#177 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A33O7TKIWVWML2O2CQOYUJTYYA4I5AVCNFSM6AAAAAAYYBPMI2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJUGE4TOMJWGM>.
You are receiving this because you commented.Message ID: ***@***.***>
|
|
Thank you!
On Sat, 2 Nov 2024 at 3:08 PM DashaBochkar wrote:
@DashaBochkar approved this pull request.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>
[
{
***@***.***": "http://schema.org",
***@***.***": "EmailMessage",
"potentialAction": {
***@***.***": "ViewAction",
"target": "#177 (review)",
"url": "#177 (review)",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
***@***.***": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]
|
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.