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

allow gas tip estimate to drop when blocks are mostly empty #68

Conversation

roberto-bayardo
Copy link
Contributor

@roberto-bayardo roberto-bayardo commented Mar 9, 2023

Description

Tweak the gas tip estimation algorithm to allow the gas tip estimate to drop when there is plenty of block space by including low dummy prices in the price array that gets sampled when mostly empty blocks are encountered. A block is deemed "mostly empty" when the gas it consumes is less than half its target.

Tests

Updated unit test to exercise both the near-full and mostly-empty cases. This change is also running on our Base goerli nodes in production here: https://goerli.base.org

Additional context

This fixes an issue we are encountering on Base goerli testnet, and I imagine affects other chains where blocks are not (yet) near full. Once a node sees a few transactions with a given priority fee, it effectively locks in that first-seen priority fee as its tip estimate even when there is plenty of blockspace. Without a bot or some other activity forcing txs with lower-than-estimated gas tip, it never drops.

@roberto-bayardo roberto-bayardo force-pushed the tweak-gas-estimate branch 5 times, most recently from 96b8e64 to 26e691f Compare March 10, 2023 18:31
@roberto-bayardo roberto-bayardo changed the title allow gas tip estimate to drop when blocks are empty or mostly empty allow gas tip estimate to drop when blocks are far from full Mar 10, 2023
@roberto-bayardo roberto-bayardo marked this pull request as ready for review March 10, 2023 18:38
@roberto-bayardo
Copy link
Contributor Author

This change should probably upstreamed into geth, not just op-geth, but I assume it makes sense to start here?
cc: @protolambda @mdehoog

@roberto-bayardo roberto-bayardo changed the title allow gas tip estimate to drop when blocks are far from full allow gas tip estimate to drop when blocks are mostly empty Mar 10, 2023
@sebastianst sebastianst requested a review from protolambda March 11, 2023 13:21
@ajsutton
Copy link
Contributor

Thanks for this. My understanding is that this will only affect clients using the old eth_gasPrice method and that the results from eth_feeHistory are unaffected (the spec for that specifically requires returning 0s for empty blocks). MetaMask should be using eth_feeHistory these days as part of sending EIP-1559 transactions, but there are still quite a few non-EIP1559 transactions around which likely use this old API.

There's a lot of latitude in how eth_gasPrice calculates its result, so this looks ok to me. However I suspect there's also a lot of experience that's gone into the current Geth implementation and it would be worth at least starting the conversation about upstreaming this with them as they may know some reason it hasn't been done before.

@roberto-bayardo
Copy link
Contributor Author

Interesting, I confirmed the issue using Coinbase Wallet which must be using this function instead of fee history. Let me experiment with Metamask a bit...

@ajsutton
Copy link
Contributor

Interestingly we've had MetaMask users report incorrect high fee predictions as well, I think because there were a lot of non-1559 transactions being sent which wound up paying absurdly high inclusion fees which then affected even MetaMask's algorithm. For 1559 tx, if blocks aren't consistently full the inclusion fee probably should just be a fixed value anyway - it just needs to be enough to cover the nominal cost of inclusion as it's the baseFee that manages demand for block space. For some reason wallets don't seem to have actually gone that route though.

@roberto-bayardo
Copy link
Contributor Author

roberto-bayardo commented Mar 13, 2023

@holiman I see you have reviewed previous PRs updating this code in geth, was wondering if you could take a look and let me know if we are missing any context?

@roberto-bayardo roberto-bayardo force-pushed the tweak-gas-estimate branch 4 times, most recently from 25ce9de to 63d6e96 Compare March 14, 2023 18:56
@holiman
Copy link
Contributor

holiman commented Mar 15, 2023

The mechanics here are a bit complex, so I'd be curious to see how it behaves rather than try to predict it. However, some observations:

  • Due to the 1559 basefee, rougly half the blocks will use <50% of the block space, since that's a basic premise of 1559. Therefore, this gas estimator will return minimal gas tip half the time.
  • Is it really correct to denominate half the blocks as "mostly empty", when that is in fact the norm?

@roberto-bayardo
Copy link
Contributor Author

@holiman the mechanism here is to designate the block as mostly empty when gas is less than half of gas target, not the block limit. So post 1559 this is block limit / 4.

@roberto-bayardo
Copy link
Contributor Author

Acknowledging the point though that behavior might be hard to predict. For highly active chains like eth mainnet it should be a no-op, but let me confirm.

@ajsutton
Copy link
Contributor

@roberto-bayardo Any update on this?

I've discovered that MetaMask actually isn't using EIP1559 on Optimism so it will be using eth_gasPrice as well. That was a bit of a surprise to me...

I'm a little unsure how to test this well to be honest. Could potentially do a differential of estimates with and without this over time but you'd probably have to actually submit transactions to know for sure that the gas price estimation wasn't too low.

@ajsutton
Copy link
Contributor

Actually you said this was running on the base production nodes - if you're not seeing issues with underpriced transactions being submitted then that's certainly very helpful evidence that it works. Not quite the same as on Ethereum MainNet where the minimum tip required by nodes might differ, but I suspect even there very few people actually modify the default.

@roberto-bayardo
Copy link
Contributor Author

roberto-bayardo commented Mar 21, 2023

My tests with metamask showed it seems to be providing same estimates as coinbase wallet (suggesting both are using eth_gasPrice, at least on Base).

And yes we're still running it on our prod nodes with good results. In fact @mdehoog just asked me about the status of this PR since it's getting a little painful to keep merging it in with each update.

I haven't done the analysis against eth mainnet, but I've yet to sample a block where this change would have any impact (gas used < 1/4 block limit), so I don't expect any surprises there. But for completeness sake let me get that done.

[Edit:] there are some oddly empty blocks (MEV?), but they seem infrequent. lemme get some harder #s....

@ajsutton
Copy link
Contributor

Empty blocks can happen on Mainnet in a few ways - most commonly when the consensus node doesn't leave much time between requesting a block be built and asking for the built block because it's either running behind or there was a late block received.

@roberto-bayardo
Copy link
Contributor Author

roberto-bayardo commented Mar 27, 2023

I've been running an A/B comparison on Ethererum mainnet since last night, and for most blocks the price estimates are identical, though I do see occasional periods of divergence caused by partially empty blocks (see below). I will investigate further. Note that any block below 7.5k gas usage is considered "below target". There are three of them here prior to the divergence:

block=16,919,143 gasUsed=7,226,898  oldprice=38,720,994 newprice=38,720,994 percentDiff=0.000
block=16,919,144 gasUsed=22,813,951 oldprice=38,720,994 newprice=38,720,994 percentDiff=0.000
block=16,919,145 gasUsed=19,837,749 oldprice=38,720,994 newprice=38,720,994 percentDiff=0.000
block=16,919,146 gasUsed=9,717,249  oldprice=38,720,994 newprice=38,720,994 percentDiff=0.000
block=16,919,147 gasUsed=15,745,861 oldprice=38,720,994 newprice=38,720,994 percentDiff=0.000
block=16,919,148 gasUsed=12,118,860 oldprice=38,720,994 newprice=38,720,994 percentDiff=0.000
block=16,919,149 gasUsed=18,534,065 oldprice=38,720,994 newprice=38,720,994 percentDiff=0.000
block=16,919,150 gasUsed=13,199,334 oldprice=38,720,995 newprice=38,720,994 percentDiff=0.000
block=16,919,151 gasUsed=4,730,599  oldprice=38,720,995 newprice=38,720,994 percentDiff=0.000
block=16,919,152 gasUsed=25,755,940 oldprice=38,720,995 newprice=38,720,995 percentDiff=0.000
block=16,919,153 gasUsed=1,273,270  oldprice=38,720,995 newprice=38,720,994 percentDiff=0.000
block=16,919,154 gasUsed=25,331,575 oldprice=38,720,995 newprice=38,720,995 percentDiff=0.000
block=16,919,155 gasUsed=28,313,288 oldprice=38,720,995 newprice=38,720,995 percentDiff=0.000
block=16,919,156 gasUsed=11,292,789 oldprice=38,720,995 newprice=38,720,995 percentDiff=0.000
block=16,919,157 gasUsed=18,085,512 oldprice=38,720,995 newprice=38,720,995 percentDiff=0.000
block=16,919,158 gasUsed=14,280,337 oldprice=55,824,999 newprice=38,720,995 percentDiff=30.639
block=16,919,159 gasUsed=16,283,520 oldprice=55,824,999 newprice=38,720,995 percentDiff=30.639
block=16,919,160 gasUsed=14,950,104 oldprice=42,593,094 newprice=38,720,995 percentDiff=9.091
block=16,919,161 gasUsed=11,727,422 oldprice=42,593,094 newprice=38,720,995 percentDiff=9.091
block=16,919,162 gasUsed=13,235,916 oldprice=42,593,094 newprice=38,720,995 percentDiff=9.091
block=16,919,163 gasUsed=14,595,683 oldprice=55,824,999 newprice=42,593,094 percentDiff=23.702
block=16,919,164 gasUsed=20,643,365 oldprice=55,824,999 newprice=42,593,094 percentDiff=23.702
block=16,919,165 gasUsed=12,858,131 oldprice=55,824,999 newprice=42,593,094 percentDiff=23.702
block=16,919,166 gasUsed=14,594,618 oldprice=55,824,999 newprice=42,593,094 percentDiff=23.702
block=16,919,167 gasUsed=14,021,468 oldprice=55,824,999 newprice=44,660,000 percentDiff=20.000
block=16,919,168 gasUsed=13,288,691 oldprice=52,475,499 newprice=50,000,000 percentDiff=4.717
block=16,919,169 gasUsed=16,436,418 oldprice=52,475,499 newprice=50,000,000 percentDiff=4.717
block=16,919,170 gasUsed=12,423,604 oldprice=52,475,499 newprice=50,000,000 percentDiff=4.717
block=16,919,171 gasUsed=16,840,900 oldprice=50,000,000 newprice=50,000,000 percentDiff=0.000

@ajsutton
Copy link
Contributor

Awesome, thanks - this data is really useful. Interestingly looking at:

block=16,919,158 gasUsed=14,280,337 oldprice=55,824,999 newprice=38,720,995 percentDiff=30.639

the base fee is 33.511953698gwei Gwei and there are a bunch of transactions included for around 33.611953698 gwei. So assuming those numbers are kwei it's probably still overpaying for gas but less than it did before. It's probably reasonable to give some leeway in case the base fee is moving up at the time.

So the couple of blocks with divergence in that range where the base fee will move up the most are:

block=16,919,167 gasUsed=14,021,468 oldprice=55,824,999 newprice=44,660,000 percentDiff=20.000
block=16,919,168 gasUsed=13,288,691 oldprice=52,475,499 newprice=50,000,000 percentDiff=4.717

16919167 has a base fee of 32.145587537 Gwei and 16919168 has a base fee of 33.657331567 Gwei. The 44gwei estimate is easily covering that. I don't think there's anything that guarantees the estimate is high enough to cover the maximum increase in base fee to guarantee eligibility for the next block but I'm not sure you'd want to do that as most of the time you'd overpay a fairly significant amount.

@roberto-bayardo
Copy link
Contributor Author

roberto-bayardo commented Mar 27, 2023

I'll share a more complete analysis later, but note that these estimates are for priority fee not base fee, and they are quite low compared to base fee: 44,660,000 is .04 gwei for example, not 44 gwei. So all in all, on ethereum mainnet where base fee is almost always > 25, tweaking this logic isn't really going to materially change things enough to justify the extra complexity.

I am now thinking none of this logic makes sense for an L2 like Optimism where there is only one block producer that doesn't need to worry about what other weirdness is going on outside its view. I think what Optimism should do is simply look at the last previously produced block only, and:

  1. If the block wasn't full (e.g. within a small % off blocklimit, which is going to almost always be the case outside airdrops), return a small fixed fee as the priority fee estimate. (This value could be a config parameter.)

  2. Otherwise, take the median priority fee from the block and return as the estimate a value 10% (or so) higher than that.

This ensures priority fee suggestions rise only when they really need to. And blocks prior to the latest aren't likely to add any predictive value, in fact it's likely to only confuse things since there's only one producer and we know how it works.

[Edit: I had first suggested we use the txpool contents instead of the last produced block, but then realized we want dumb nodes without txpools to be able to fulfill the estimation requests.]

@ajsutton
Copy link
Contributor

ah sorry, I thought those were the final gas prices with base fee added back in (hence assuming they were in Kwei).

@Inphi
Copy link
Contributor

Inphi commented Mar 28, 2023

fwiw we also see inaccurate estimation on chaos net. This would be a good change to upstream too.

@roberto-bayardo
Copy link
Contributor Author

@Inphi Possibly, but you could argue looking back at a longer history might make sense when you don't have full understanding of how other block builders might operate. For example I see evidence that some are filtering all tx with less than .1 gwei priority fee. If most builders started doing that then you'd be in trouble if your fixed fee under no congestion was less than that amount.

@roberto-bayardo
Copy link
Contributor Author

roberto-bayardo commented Mar 30, 2023

Superceded by #77

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.

4 participants