server/asset/eth: max fee rate needs to be a max gas cap

[chappjc]

TLDR: Server contract audits check and enforce a fee rate, but for dynamic ETH transactions there are two fees (tip and max fee) so we have to check max fee, which means that's the fee a match needs to prescribe and record in the DB per-match.

As we have discussed previously, with ETH's eip-1559 "dynamic" transactions:

• dynamic eip-1559 transactions have two fee settings (GasFeeCap and GasTipCap) while legacy fee transactions have one (GasPrice)
• the MaxFeeRate asset const needs to be something quite high, and as with other asset params, must be set in the server config, and this must be high enough for any server-prescribed max fee.

Since a server's fee "suggestions" and prescribed match fee rate in our comms protocol only supports a single rate:

• The prescribed match fee rate must specify the desired gas fee cap, which sets the transactions limit of effective base fee + tip.
• Thus, a dynamic transaction's gas fee cap should be some combination of gas tip cap plus a scaled current base fee, or even just the constant MaxFeeRate and the eip1559 gas fee cap semantics are the same as MaxFeeRate.

A sensible formula for this is used in go-ethereum/accounts/bind/(*BoundContract).createDynamicTx:

gasFeeCap = gasTipCap + 2 * headBaseFee

https://github.com/ethereum/go-ethereum/blob/356bbe343a30789e77bb38f25983c8f2f2bfbb47/accounts/abi/bind/base.go#L250-L255

gasFeeCap := opts.GasFeeCap
if gasFeeCap == nil {
	gasFeeCap = new(big.Int).Add(
		gasTipCap,
		new(big.Int).Mul(head.BaseFee, big.NewInt(2)),
	)
}

Presently the server gives a fee rate that is obtained from a "legacy" fee rate function that just sums current base fee rate with suggested tip. This is far too low for a max gas fee for a "dynamic" transaction. I started work on this, addressing prerequesites in #1356
In that PR description I've linked a draft commit that will use the base fee and tip suggestion to compute a max gas fee for clients using a formula like the one I've given above.
64a72b4

However, this does not answer the question of what a client should use for their tip, but their funding should use MaxFeeRate from the asset config, and their swap transaction should use the gas fee cap provided by the msgjson.Match fee rate. It's not clear how much the tip matters if the gas fee cap is set very high, but if it is important, we will have to establish an enforced convention for the tip (e.g. always 2), or communicate it somehow in addition to the fee cap.

EDIT: Instead, it is conceivable that the priority fee for a swap txn can come from the msgjson.Match rate, and the txn should set the GasFeeCap to MaxFeeRate, but this max fee must be checked.

[JoeGruffins]

imo.

• The server only needs to send the max tip.
  This is because the base fee is not negotiable. It is part of consensus and not up to the miner to decide to include your tx based on this fee.

• We don't need to bother with the legacy fee type.
  This isn't in use yet, so there should be no backwards compatibility issues. Also, the chain no longer understands the old tx type I believe. If you send the old type, it just sets the tip to everything - base fee. Trying this out on the harness seems to be the case as well. Also, our harness did not attempt to take the entire tip cap. Probably real miners would however.

• The fee cap should always be the user's max fee rate.
  Again the negotiable part of the fee is the tip. The fee cap is the max fee one is willing to pay.
  I suppose you could say, though, that you are negotiating for lower network usage. But then what is the resolution if it cannot be sent because base is too low? Recalculate base and try again with a higher fee cap? Give the user an option to cancel if they don't want to go higher? But that's what the user's max fee is already set for, so I think it makes just as much sense to just use that.

I think the difference here is that with other coins and the tip you are dealing with the human element, but with the base fee it's just network usage. A miner cannot screw you on the base fee. If none of this makes sense please call me out, but I think this is what's different and why only the tip cap really matters here.

[chappjc]

• The server only needs to send the max tip.
  This is because the base fee is not negotiable. It is part of consensus and not up to the miner to decide to include your tx based on this fee.

I think we need to clarify what is consensus and what is settable as part of the transaction.

Each block has a base fee that is decided by consensus. It fluctuates a great deal from block-to-block. Consider the following blocks, where base fee is the right column:

Over 10 blocks and 2 minutes, base fee went from 54 to 44 to 64. And it may not get below 50 again for quite some time.

For a transaction to be included in one of those blocks, it's GasFeeCap a.k.a. maxFeePerGas would need to be set above the base fee for the block it gets into. So while the base fee for a block is not negotiable, this cap is set by the transaction. If it is set low, it will be delayed until base fee drops low enough.

Tip / priority fee is much less important as that will only play a role when a block is full (gas used is about 30 million, as with those 3 blocks above) and the miner needs to decide which get in and which don't. The result is that base fee just goes up (see the table) until the block has space for transactions.

The gas fee cap on a transaction must be checked because if it is lower than base fee, it doesn't get mined.

[chappjc]

So the only way I can see the priority fee being the fee rate prescribed for matches is if the asset's constant MaxFeeRate in the server config is also enforced for the transaction's GasFeeCap, which would be reasonable, but server absolutely has to check the GasFeeCap on swap transactions, and it must be very high (e.g. 200 gwei / gas) so that it is acceptable in almost any ethereum block that has been witnessed since eip1559 has activated.

[JoeGruffins]

If it is set low, it will be delayed until base fee drops low enough.

If we want to use the GasFeeCap to delay a tx until the fee is lower, then this makes sense. But if this is too low, don't we just try again with a higher fee? I still think the difference here is that for other coins the fee is negotiated, so you cannot rely on the miner to be "fair" to you. We don't have that problem with the base fee, and I think it is correct to just make sure we do not go over the max.

If we want to delay the tx until the fee becomes something "reasonable" then what you propose is correct. It's like a blending of a rate check and max possible...

The gas fee cap on a transaction must be checked because if it is lower than base fee, it doesn't get mined.

I am proposing just make it the max.

[chappjc]

I am proposing just make it the max.

Yes, it's must be super high, but it also has to be checked by sever during audit. I'm not proposing to delay transaction. Nor should there ever be a need to retry a transaction with a higher fee - the whole point of dynamic transactions is to solve this. I'm saying that we can only check one value presently, thus we cannot have server prescribe tip because it has to be checking the max fee instead.

Why I'm focusing this discussion on the match fee rate communicated from server to client is because it is recorded for the match and enforced during server audit of the swap. Whatever we check needs to also be recorded per match.

[chappjc]

• Also, the chain no longer understands the old tx type I believe. If you send the old type, it just sets the tip to everything - base fee. Trying this out on the harness seems to be the case as well.

The test must be creating the transaction incorrectly. Legacy transactions still work, on mainnet anyway.
Not relevant though, except in pointing out that the current server FeeRate method isn't right yet in that it's using a method for a single fee rate best fitted for a legacy txn.

[buck54321]

Oof. The server should check that both 1) GasFeeCap == MaxFeeRate and 2) GasTipCap == Match.Rate. I don't see any way around it. We'll need an API change for server/asset.Contract or server/asset.Coin and some handling in (*Swapper).processInit.

[chappjc]

Yes, if we want to check both, it's an API change (and DB changes on both client and server to save both fee rates, uggggh). However, I'm not convinced tip needs to be enforced if the GasFeeCap is sufficiently high.

Also, I don't think that it's required that GasFeeCap == MaxFeeRate, just that GasFeeCap == something_high. In fact, I'm quite nervous about using the funding fee rate (MaxFeeRate) as every swap txn's GasFeeCap because we are very likely to hit edge cases and very bad out-of-gas failures in redemptions that can lead to lost funds (exposed secret but unable to redeem without more funding).

[chappjc]

Oh, I had a thought about tip last night. We could have tip be set by convention not dynamically. We have a lot of asset conventions such as the contract addresses, redeem script structures, etc., so it would be very reasonable to have the tip always be at least 2 gwei/gas. That would let us check the GasFeeCap is as prescribed without API or DB changes for a secondary tip fee to check.

Anecdotally, checking MetaMask, it seems to set GasFeeCap to something like 120% of current base fee, and the Tip depends on a Low/Medium/High setting that changes it between 1.41/1.5/2.0. These tips might be variable somehow, but I haven't seen Med or High be anything different. The "Max fee" seems to be the main variable here. The etherscan gas tracker also seems to suggest 2 to 4 being a "high" priority fee, with base fee being more variable and precise.

[chappjc]

Found a comment by karalabe describing geth's oracle implementation: ethereum/pm#328 (comment)

Specifically, some justification if we went with tip + 2 * base for a dynamic txn's max fee:

If the user doesn't specify maxFeePerGas, we default to maxPriorityFeePerGas + 2*baseFee

So for server/asset's FeeRate: we could pick a base multiple like 2 or whaterver, plus the tip from geth's oracle. Then the user's swap would use that for base fee, and for tip use an established-by-convention 2 or 3 gwei. This would let us keep both API and DBs dealing with just a single fee value.
We have to keep in mind that even if we use MaxFeeRate for matches' base fee instead of a more dynamic value, that still has to be recorded and communicated per-match because server config can change.

[buck54321]

It's just that Match.Rate is the only knob we have to affect priority on the fly.

[chappjc]

Maybe there's something I'm still not getting about eip 1559, but there is essentially no need to affect priority on the fly any more. The GasFeeCap only need to be above current block's base fee.

The only time tip actually plays a role is when blocks are full and there is a backlog in txpool, but with eip 1559 setting the target block utilization to 50%, that rarely happens for more than a few blocks (a minute or two) at a time. As long as your txn's GasFeeCap is higher than the current block's base fee, and the block is not full, priority fee does not matter. This is part of what I was intending to describe in my comment with the screenshot here, specifically the "Gas used" percentage: #1372 (comment) The behavior of ETH wallets and other gas oracles always suggesting 2-4 for a "high" priority fee, while varying the max fee with base fee much more, supports this I think.

[buck54321]

I think you're probably right. I just hate to use a constant, when we have e.g. SuggestGasTipCap, which by all appearances is also going to be relatively constant, but comes directly from the chain. If that did rise from 2 to say 3 or 4, we'd want the client to be using the new value. And the client really doesn't want to pay more than the suggestion. But how do you synchronize changes without new fields, db entries, or asset-specific API info. Is there some condition that we could check server-side to validate the priority fee? Something like

if tipRate >= lowestPriorityFee(tipHeight - 10, tipHeight) { ...

The GasFeeCap only need to be above current block's base fee.

And the client is going to lock up funds at MaxFeeRate. I'm just not seeing a good reason to assign anything but MaxFeeRate to GasFeeCap. We just cannot know the assessed fee rate at match time, so we can't record an accurate value in the DB without a significant update. I have no problem recording MaxFeeRate for all ETH matches.

[chappjc]

I'm just not seeing a good reason to assign anything but MaxFeeRate to GasFeeCap

I agree it's the logical setting for GasFeeCap since they have the exact same semantics of "plan for fees this high". It's just the edge cases we will hit, but might as well sort it all out so it does work OK at the funded limit.

We just cannot know the assessed fee rate at match time, so we can't record an accurate value in the DB without a significant update

On this topic, we should be realistic about the timings. Server computes some fee rates, be they the the max fee and/or priority fee. Then some time passes until a transaction is made. Even for maker this is likely to be after a block or two and thus network conditions have changed. For taker, both the priority fee and base fee are super irrelevant since they have been sitting waiting for maker's swap to hit target confs....
As such I think that's actually a good reason to use MaxFeeRate for GasFeeCap, but also a reason to use something particularly high for priority, such as 4, since the estimate geth gives us from SuggestGasTipCap is completely worthless to the taker.

I have no problem recording MaxFeeRate for all ETH matches.

Recording both rates then? This sounds like overkill to me.

[buck54321]

Recording both rates then?

What other rate are we recording for the match?

[chappjc]

There's priority and max fee. I thought you were advocating to send both rates separately in the msgjson.Match. Any rate we enforce we must record too. Or you mean it would be a heuristic based on past recent blocks for priority? I guess I'm not clear on lowestPriorityFee that you described.

[buck54321]

I thought you were advocating to send both rates separately in the msgjson.Match.

Not exactly, but I know what you mean. Let's forget about this approach for the time being.

Or you mean it would be a heuristic based on past recent blocks for priority?

Right. We can record the MaxFeeRate at match time, send it as the Match.Rate too. The server/asset.(Coin).FeeRate returns the MaxFeeCap of the transaction for validation. Internally to the (Backend).Contract implementation, we can check against a heuristic based on recent blockchain history. Just a sanity check for what is expected to be a very very slowly changing value. Then the server and client don't have to be in sync down to the block, and we can still adapt without using a constant that's probably too high.

[chappjc]

That sounds workable to me for the issue of swap txn fee rate assignment and enforcement. As long as light clients can obtain a priority fee from their chain node that is concordant with the server's.

[chappjc]

Did not mean to assign this to myself

[martonp]

I'll work on this. So seems like the final consensus is to send the max fee rate in the match request which will be used as the MaxFeeCap, and the MaxTipCap will be a conservative estimate based on the priority fees in the previous blocks that the server will also check.

[chappjc]

I'll work on this. So seems like the final consensus is to send the max fee rate in the match request which will be used as the MaxFeeCap, and the MaxTipCap will be a conservative estimate based on the priority fees in the previous blocks that the server will also check.

Yup, although I'm personally fine with a constant tip of 2 or 2.25. It seems to be fairly unimportant if your max fee is very high