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

EIP-1559 Transaction Support #1610

Closed
ricmoo opened this issue May 26, 2021 · 44 comments
Closed

EIP-1559 Transaction Support #1610

ricmoo opened this issue May 26, 2021 · 44 comments
Assignees
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@ricmoo
Copy link
Member

ricmoo commented May 26, 2021

This has already been started locally, but I'm throwing together a quick issue to help track it (in commits) and to include relevant literature.

For those unfamiliar with EIP-1559, it uses a new fee structure to provide more stable gas fees across block, improving the user experience and attempting to minimize economic inefficiencies which lead to many users overpaying for their transactions.

It's goal is not necessarily to make transactions cheaper, but will make it easier to know how much to pay for a reasonable inclusion duration, and cause fees paid for transactions to more accurately reflect fair-market value.

More links: (this will be updated as I find more resources):

Old Links:

@ricmoo ricmoo added enhancement New feature or improvement. on-deck This Enhancement or Bug is currently being worked on. labels May 26, 2021
@ricmoo ricmoo self-assigned this May 26, 2021
@FrederikBolding
Copy link

We are definitely eagerly awaiting this at MyCrypto to start playing around with it. Let me know if you need any help at all!

@wolovim
Copy link

wolovim commented May 26, 2021

hi from web3.py! following along too as we'll be starting down this path either this or next week. 👀

ricmoo added a commit that referenced this issue May 30, 2021
@ricmoo
Copy link
Member Author

ricmoo commented Jun 15, 2021

So, one problems that has come up is the default choices for maxFeeGas and maxPriorityFeePerGas.

It makes total sense that maxFeeGas >= maxPriorityFeePerGas, however the current advice is:

  • maxFeePerGas = 2 * getBlock(-1).baseFee
  • maxPriorityFeePerGas = 1 gwei

This has the following consequence. On Calaveras, right now the baseFee is 7 wei (not gwei). This means:

  • maxFeePerGas = 14 wei
  • maxPriorityFeePerGas = 1 gwei

And 14 wei < 1 gwei. So, what should be do?

Option 1:

  • maxFeePerGas = min(2 * getBlock(-1).baseFee, 1 gwei)
  • maxPriorityFeePerGas = 1 gwei

Option 2:

  • maxFeePerGas = 2 * getBlock(-1).baseFee + 1 gwei
  • maxPriorityFeePerGas = 1 gwei

Options 3

  • ???

Both options will likely massively overpay (100's of millions times more to the miner than the burn) on a network with a low base fee.

Ideas?

@zemse
Copy link
Collaborator

zemse commented Jun 15, 2021

What if we have default maxPriorityFeePerGas as 0?

In pre-EIP1559 system, there has been an option to set a minimum gas price for eth clients, and mostly it has been 1 gwei across testnets and mainnet (before last year's increase). I assume this is to prevent spam txs when network has less traffic. That's why if ethers.js had 0 default gas price tx (which is less than the min gas price by nodes), then it'd not be possible to demo ethers.js without using a gasPrice override.

However, post-EIP1559, this minimim gas price is in the form of baseFee (unless miners also want to utilize their ability to set a minimum priority fee). As long as the baseFee is set sufficient, i.e 2x of latest block's base fee, having a priority fee as 0 should still work (at least on less crowded testnets). For mainnet use, almost everyone sets a manual gasPrice (from some gas price oracle).

Edit: since we are setting maxFeePerGas as 2x base fee, and if that is 14 wei. If baseFee turned out to be 8 wei, then even if maxPriorityFee is set as 1 gwei, the priority fee cannot be more than 14 - 8 = 6 wei ?

@ricmoo
Copy link
Member Author

ricmoo commented Jun 15, 2021

If you set a maxPriorityFee of 0 though, you will 100% never get mined though. Miners will not mine a transaction that does not cover their costs for uncle risk. We need priority fees to prevent mining empty blocks.

Micah suggested we go with Option 2, and the discussions on discord have convinced me. :)

I've made those changes to the eip-1559 branch, if anyone wants to check it out. :)

One nice thing of EIP-1559 is we will no longer require any gas oracle. And it looks like any oracle data we might need to make better decisions in the future may be included in the Geth JSON-RPC, such as a histogram of priority fees paid. Then we can add some nice logic to ethers to "just work" for anyone doing normal things and overriding will only be necessary for developers doing more advanced things. :)

@ricmoo
Copy link
Member Author

ricmoo commented Jun 15, 2021

In this case, what we would end up with is:

  • maxFeePerGas = 14 wei
  • maxPriorityFeePerGas = 1000000000 wei

I was originally concerned, because I was thinking of priority fee like a tip, and I would never tip $500 million on a $3 coffee. But Micah pointed out that we shouldn't think of it like a tip. The priorityFee is the costs the miner must absorb (operation cost + needed profit) to justify their continued work and the baseFee is more of a mechanism to figure out "who wants it more" and weed out spam/low-value transactions. So it is ok to have a large discrepancy between these values, since they represent different things entirely. :)

@marceljay
Copy link

marceljay commented Jun 23, 2021

The priorityFee is the costs the miner must absorb (operation cost + needed profit) to justify their continued work

I don't get why this is assumed, because there is a block reward. I thought the tip is really just to be mined quickly when there is congestion to give the miners an way to prioritize. And the mechanism is supposed to make ether more valuable.

@ricmoo
Copy link
Member Author

ricmoo commented Jun 23, 2021

@marceljay Check out the EIP in the OP. After the London Hardfork (which will include EIP-1559) the subsidy reward is destroyed, not given to the miner like it is today. That is the new incentive model which is why “tips” (or bribes) are necessary.

The economic model is a lot different and quite unrelated to the gasPrice auctions today, so there is a lot of additional modelling and economic ballet to study up on. :)

Sorry, I don’t know what I was thinking when I wrote that. I need to revise my answer as that last one is entirely incorrect. :s

The block subsidy reward only rewards the miner for the resources used to mine a block, but there must be an incentive to include each transaction, otherwise it would be easier to just mine empty blocks, and no reason to bother including anything else.

Each transaction also requires some additional time to process, and increases the processing time of each other node to verify a successful block before that block gets accepted and propagated. As a result, including additional transactions has an extra cost; each transaction increases the odds that a block will get uncled (or worse orphaned), so the priority fee is required to offset the cost of that risk.

@ricmoo
Copy link
Member Author

ricmoo commented Jun 24, 2021

@marceljay Sorry. My previous answer was wrong. I updated it and am tagging you to make sure you see it. Sorry about that.

@ricmoo
Copy link
Member Author

ricmoo commented Jun 26, 2021

This has been published in 5.4.0. Try it out and let me know how it works for you! :)

@jmrossy
Copy link

jmrossy commented Jul 5, 2021

@ricmoo Do these updates for EIP-1559 break any compat with old txs (i.e. can 5.4.0+ be used on Mainnet still)?
And related, is the intention to keep backwards compat after EIP-1559 is live on Mainnet?

@ricmoo
Copy link
Member Author

ricmoo commented Jul 5, 2021

@jmrossy Everything is 100% backwards compatible. It has to be this way to continue supporting other networks (e.g. matic, xDai, etc).

The new provider.getFeeData can be used to detect if a network supports EIP-1559. If a transaction does not have an explicit type, ethers will use this to detect the best option to use on a given network, transparently to the developer. :)

@Macket
Copy link

Macket commented Sep 8, 2021

Thank you for your comments, sir! And for all job you do :)

@ricmoo
Copy link
Member Author

ricmoo commented Sep 8, 2021

Thanks for your patience with my typos. ;)

@fulldecent
Copy link

I disagree with the general statement:

But you shouldn't be explicitly passing gasPrice or any fee data to MetaMask anyways, it is far more qualified to choose these for you, and if you notice MetaMask now throws up a big red box if the gas price (or fee data) was provided by the dapp.

I am working with one specific function where MetaMask is, for some reason, setting max priority fee to 185 and max fee to 185. (Today gas prices are about priority = 2 and base = 180).

Here I am working on a function that I will be calling my self many times. (Manually dropping Su Square NFTs, FML). So I would like a way here to specify that no, I do not want a crazy high priority fee.

@rperez89
Copy link

@ricmoo Xdai now implemented EIP-1559, should you add compatibility from your side?

@ricmoo
Copy link
Member Author

ricmoo commented Nov 15, 2021

@rperez89 The ethers provider will automatically detect if a network supports EIP-1559 and use it if it does, so there shouldn't be any need to do anything as long as Xdai correctly adds a .baseFee property on the Block object. :)

@rperez89
Copy link

@rperez89 The ethers provider will automatically detect if a network supports EIP-1559 and use it if it does, so there shouldn't be any need to do anything as long as Xdai correctly adds a .baseFee property on the Block object. :)

ohh i see thanks!

@mohammed-shameem
Copy link

@ricmoo Does this support wallet.signTransaction(transaction) ?

var transaction = {
      to: address,
      data: transactionData,
      nonce: txCount,
      maxFeePerGas,
      maxPriorityFeePerGas,
      gasLimit: 7000000,
      chainId
    };

I am getting this error during signing. "invalid object key - maxFeePerGas (argument="transaction:maxFeePerGas"

@ricmoo
Copy link
Member Author

ricmoo commented Nov 17, 2021

@mohammed-shameem Yes, it should work fine. What version are you using? What signer?

@mohammed-shameem
Copy link

mohammed-shameem commented Nov 17, 2021

@ricmoo
"ethers": "^5.4.5"

var transaction = {
     to: address,
     data: transactionData,
     nonce: txCount,
     maxFeePerGas,
     maxPriorityFeePerGas,
     gasLimit: 7000000,
     chainId
   };
const provider =  new ethers.providers.InfuraProvider(chainId, infuraProjectId);
 const wallet = new ethers.Wallet(privateKey, provider);
 return wallet.signTransaction(transaction);

Ah is the error because of InfuraProvider ?

@DefiCake
Copy link

@ricmoo Does this support wallet.signTransaction(transaction) ?

var transaction = {
      to: address,
      data: transactionData,
      nonce: txCount,
      maxFeePerGas,
      maxPriorityFeePerGas,
      gasLimit: 7000000,
      chainId
    };

I am getting this error during signing. "invalid object key - maxFeePerGas (argument="transaction:maxFeePerGas"

Getting this too

@DefiCake
Copy link

DefiCake commented Nov 19, 2021

@ricmoo "ethers": "^5.4.5"

var transaction = {
     to: address,
     data: transactionData,
     nonce: txCount,
     maxFeePerGas,
     maxPriorityFeePerGas,
     gasLimit: 7000000,
     chainId
   };
const provider =  new ethers.providers.InfuraProvider(chainId, infuraProjectId);
 const wallet = new ethers.Wallet(privateKey, provider);
 return wallet.signTransaction(transaction);

Ah is the error because of InfuraProvider ?

Managed to workaround it: put type 2 transaction

var transaction = {
      to: address,
      data: transactionData,
      nonce: txCount,
      maxFeePerGas,
      maxPriorityFeePerGas,
      type: 2,
      gasLimit: 7000000,
      chainId
};

@ricmoo problem lies at packages/transactions/src.ts/index.ts function serialize. It is routing through legacy transaction because tx.type == null fulfills the condition. I don't know if the following fix @ packages/wallet/src.ts/index.ts 's signTransaction would be OK for you. Tests are passing on my machine. If so lmk and I 'll open a PR

signTransaction(transaction: TransactionRequest): Promise<string> {
        return resolveProperties(transaction).then((tx) => {
            if (tx.from != null) {
                if (getAddress(tx.from) !== this.address) {
                    logger.throwArgumentError("transaction from address mismatch", "transaction.from", transaction.from);
                }
                delete tx.from;
            }

            if(tx.type == null && (tx.maxFeePerGas || tx.maxPriorityFeePerGas)) tx.type = 2

            const signature = this._signingKey().signDigest(keccak256(serialize(<UnsignedTransaction>tx)));
            return serialize(<UnsignedTransaction>tx, signature);
        });
}

@akwodkiewicz
Copy link

What will happen if I only define a maxPriorityFeePerGas (leaving gasPrice === undefined) and try to call different chains: both implementing EIP-1559 and those that do not implement it?

@zemse
Copy link
Collaborator

zemse commented Nov 29, 2021

@akwodkiewicz Along with maxPriorityFeePerGas, you also need to provide maxFeePerGas. If network doesn't support EIP1559, ethers will detect it and give you an error. If network supports then it should work. Are you facing any problem?

@akwodkiewicz
Copy link

@zemse, thanks for the reply. I knew about the maxFeePerGas, but was just curious how would the library behave if I tried to force type:2 transaction

@akwodkiewicz
Copy link

Is there a programmable way of checking EIP1559 support other than parsing a potential error after a failed EIP1559 transaction?

@ricmoo
Copy link
Member Author

ricmoo commented Dec 7, 2021

If you have a provider connected to a network, you can use provider.getFeeData(). If there is a maxFeePerGas On the FeeData object returned, the network supports EIP-1559.

@ihorbond
Copy link

If you have a provider connected to a network, you can use provider.getFeeData(). If there is a maxFeePerGas On the FeeData object returned, the network supports EIP-1559.

It may need to be clarified that the maxFeePerGas property will always be returned but will be null in case of no eip-1559 support

@tzapu
Copy link

tzapu commented Jan 12, 2023

is there a way to force type 2 transactions on all transactions? using ethers through hardhat-ethers to send a lot of transactions for some tests on filecoin s evm compatible jsonrpc (https://wallaby.node.glif.io/rpc/v0) and it always fails to send the transaction with legacy transaction is not supported.

i can force it one by one to use type 2 through overrides but that s quite tedious for loads of deploys and txs.
the block response from getBlock seems to contain baseFeePerGas which i saw above was required for detection.

thank you very much for everything

@zemse
Copy link
Collaborator

zemse commented Jan 13, 2023

is there a way to force type 2 transactions on all transactions?

The Signer class that you are using (Wallet or JsonRpcSigner), you can override it’s sendTransaction method link and set the tx.type to 2 if it is not set.

But if you are getting the signer from somewhere and if it’s challange to have your new class being used over there, you may try this hacky solution:

const sendTransactionOld = contract.signer.sendTransaction;

contract.signer.sendTransaction = async(tx) => {
    if(tx.type === undefined) {
       tx.type = 2; 
    }
    return sendTransactionOld.bind(contract.signer)(tx);
}

@tzapu
Copy link

tzapu commented Jan 13, 2023

i am sending tx s through hardhat-ethers and the contract interface it provides so i end up doing something like

oracle.initialize({
      time: 0,
      tick: 0,
      liquidity: 0,
    }, {
      maxPriorityFeePerGas: feeData.maxPriorityFeePerGas?.mul(2), 
      maxFeePerGas: feeData. maxFeePerGas?.mul(2), 
      type: 2,
    })

thanks @zemse will give it a try and see if i manage something

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests