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

fillGasPrice fundamentally changed transactions, should have been a breaking change? #6801

Closed
Quicksaver opened this issue Feb 8, 2024 · 13 comments · Fixed by #7050
Closed
Assignees
Labels

Comments

@Quicksaver
Copy link

Quicksaver commented Feb 8, 2024

Expected behavior

Up until v4.0.3, Contract transactions would go through without explicitly estimating gas prices on the caller side; web3js did it automatically. We could do this and everything would work fine:

  const contract = provider && new provider.eth.Contract(abi, address);

Actual behavior

Since v4.1.0, this behavior changed as the default value for fillGasPrice is set as false. It becomes mandatory to do this:

  const contract = provider && new provider.eth.Contract(abi, address, {
    gasPrice: await provider.eth.getGasPrice(), // <--- this line becomes mandatory
  });

Also, the prepareTransactionForSigning is never called with this parameter (or with fillGasLimit BTW), meaning the default Contract object instances cannot be given those flags without a custom transactionBuilder method in the web3Context of the caller, which is overkill just to set simple boolean flags.

In practice, unless I am missing something obvious (I could be, but definitely need someone to point it out and I would greatly appreciate it!), it seems we are stuck to manually estimating gas for each contract/transaction beforehand.

E.g. every single transaction, processed via metamask on either BNB Mainnet or Testnet, for which we do not estimate gas price explicitly, fails, as it defaults to 2,5 GWEI which is below the minimum necessary on both networks for txs to go through.

This was buried in the changelog in the line:

sendTransaction will have gas filled by default using method estimateGas unless transaction builder options.fillGas is false

Please note that

  • there is no fillGas option anywhere;
  • fillGasPrice and fillGasLimit are not easily set without a custom transactionBuilder in the Contract's web3Context;
  • and this actually seems to be a lie because the default value for the option is false and not true.

As mentioned, if I am wrong I definitely need clarification here as documentation and examples are scarce around this.

@SantiagoDevRel
Copy link

Boa tarde Luis!! @Quicksaver thanks a lot for submitting this.
Just to understand this properly, you want to set the gas manually on a transaction, so Metamask doesn't set it automatically to 2,5 GWEI, right?
If i'm not mistaken, I think the estimageGas is what you are looking for?

const { Web3 } = require("web3");

const web3 = new Web3("https://polygon-mumbai-pokt.nodies.app");

async function main() {
  const wallet = web3.eth.accounts.wallet.add("0xPrivateKey");

  //create tx object
  const tx = {
    from: wallet[0].address,
    to: "0xfB6eE09bb37311813e474Ea19a963d6891689179",
    value: 1,
    input: "0x00",
  };

  //estimate gas & gasPrice
  const gasEstimated = await web3.eth.estimateGas(tx);
  const gasPrice = await web3.eth.getGasPrice();

  //update tx object
  tx.gas = gasEstimated;
  tx.gasPrice = gasPrice;

  console.log(tx);
}

main();

If you are calling a specific contract, you can also set the gas/gasPrice for all the txs:

myContract.options.gasPrice = '20000000000000';
myContract.options.gas = 5000000;

Let me know if this solves your issue, otherwise I'll ping the devs to check it out!

@Quicksaver
Copy link
Author

Oi Santiago! 😄

Actually it's the other way around, with v4.0.3 I could send transactions without setting any kind of gas options like those, as web3js would automatically estimate gas and gas price correctly so that on metamask the gas prices would already be correctly set (i.e. 3 gwei on bnb mainnet, or 10 gwei on bnb testnet).

From v4.1.0 onwards, web3js no longer estimas gas fees properly, and using gasPrice() just like you suggested for every single transaction seems like the only way to get transactions to work; without it everything is set to default minimums on metamask and transactions never go through (i.e. without using gasPrice() explicitly, it sets 2,5 gwei on both bnb mainnet and testnet). There's no way (apart from apparently a custom transactionBuilder method) to "tell web3js to set the gas price" by itself like it used to.

(Sorry if my first post was confusing, I tried to be as thorough and detailed as possible but may have gotten things lost in so much info.)

@SantiagoDevRel SantiagoDevRel added the 4.x 4.0 related label Feb 8, 2024
@Quicksaver
Copy link
Author

I've updated the original description with simple examples of what I mean, where before I could do this:

  const contract = provider && new provider.eth.Contract(abi, address);

I am now forced to do this explicitly:

  const contract = provider && new provider.eth.Contract(abi, address, {
    gasPrice: await provider.eth.getGasPrice(), // <--- this line
  });

@SantiagoDevRel
Copy link

@Quicksaver Really clear now! I'll ask the devs to take a look at it, obrigado Luis!

@jdevcs
Copy link
Contributor

jdevcs commented Feb 9, 2024

@Quicksaver if you want gasPrice set by default (want to send default legacy 0x0 tx ) , instead of 0x2 Txs gas params, use following:

const web3 = new Web3({
    provider: 'https://mainnet.infura.io/v3/KEY',
  config: {
    defaultTransactionType: '0x0',
  }}
  );

@jdevcs
Copy link
Contributor

jdevcs commented Feb 9, 2024

web3.js sends 0x2 Txs by default and only sets maxPriorityFeePerGas and maxFeePerGas
{ ...
gasPrice: undefined,
maxPriorityFeePerGas: "0x9502f900",
maxFeePerGas: "0x21c2c77fc0",
..
}

@Quicksaver
Copy link
Author

@jdevcs I don't see how that change, introduced in v4.2.0, would be related here. In my testing the issue started with v4.1.0, that means even 0x0 transactions were not setting gasPrice.

I tried your suggestion and it does not work, gasPrice is not set with 0x0 transactions same as with 0x2. So far it's as I mentioned, the only way to get transactions to go through is by setting gasPrice manually via getGasPrice() as in my example above.

@luu-alex
Copy link
Contributor

luu-alex commented Feb 20, 2024

sorry for the long response, it seems this issue can be fixed with a fillPrice, is this issue also occuring with the latest release as well?

@Quicksaver
Copy link
Author

Nothing has changed, still the same behavior

@avkos avkos self-assigned this May 13, 2024
@avkos
Copy link
Contributor

avkos commented May 16, 2024

Example without gas in tx
type=0x2
image.png
type=0x1
image.png

type=0x0
image.png

contract examples
type =0x2
image.png

type=0x0
image.png

version [email protected]
backend ethereum/client-go:v1.13.14-amd64

I am trying to reproduce the problem. Could you please provide which backend you use?

@Quicksaver
Copy link
Author

@avkos there's no backend, we're using web3js on a React frontend app, using the methods mentioned above: get a provider via new Web3(), instantiate a contract, use methods of that contract.

@avkos
Copy link
Contributor

avkos commented May 16, 2024

@avkos there's no backend, we're using web3js on a React frontend app, using the methods mentioned above: get a provider via new Web3(), instantiate a contract, use methods of that contract.

I mean what blockchain do u use?

@Quicksaver
Copy link
Author

@avkos it was on the first post, issue happens on BNB Mainnet and Testnet, I know it happens on others but I didn't document those at the time as BNB ones are our main ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants