Eval: Move congestion monitoring and fee escalation on-chain

[jannotti]

Currently, congestion is monitored locally, by each algod and is not a part of consensus. So while individual algods may require higher fees, transaction processing is unaware of that requirement. Therefore, the extra fee can end up being "double spent" to execute inner transactions. This PR introduces two header fields, Load which indicates how full a particular block is, and CongestionTax which grows when the Load exceeds 50% and returns to 0 with less full blocks. The long term plan is that groups must pay numTxns * MinFee * (1+CongestionTax) to succeed.

As of this PR, however, groups merely state their willingness to pay extra by setting a Tip field on one transaction, and ensure the sum of their Fee fields (both outer and inner) is more than the number of txns (both outer and inner) * MinFee * (1 + Tip). At ingress time, algod will confirm that Tip exceeds the current block's CongestionTax, and at eval-time the protocol will ensure the Fees exceed the tipped value.

Note that the Fee fields are not grown by Tip. Instead Tip multiplies the costs of the group, and the Fee fields must be big enough to pay that higher price. They are not scaled, to avoid "tricking" existing code, especially logicsigs. Users that prepare groups should obtain the current congestion level, set the Tip, and adjust their Fee fields accordingly.

After this PR, transactions will be dropped when they arrive at algod if the current block's CongestionTax is higher than the submitted group's Tip, but groups will not be dropped at evaluation time unless they failed to include enough Fee for their stated Tip. The result of this policy is that once a group is admitted to the transaction queue, it will not be dropped if congestion increases before the group gets into a block. However, groups that included a Tip to get admitted will still be held to that standard.

This means that congestion is still outside the protocol in some ways (proposers could include a transaction with no Tip, even when the previous blocks have high load, and CongestionTax is high), but the groundwork was been laid to first track congestion in protocol, and eventually enforce it at evaluation time. Since we expect all algods to reject low Tips during congestion, only proposers can take advantage of this "loophole". Transactions will not propagate with Tip < CongestionTax.

Why don't we enforce CongestionTax during evaluation with this update? Because it would lead to an ugly pathology. Suppose there is no congestion. Lots of transactions arrive with MinFee fees, and no Tip. As soon as one block is created that's more than 1/2 full, the next block would have a positive CongestionTax so all those pending transactions would be dropped upon re-evaluation. We'd have an empty block, congestion would go down, and then a bunch of MinFee transactions would be sent again.

Avoid that problem requires some sort of "pay-as-you-go" or "refund" model. Transactions will need to specify that they are willing to pay more, but only if there's congestion that demands it. If congestion is less than they have indicated their willingness to pay, they will keep this part of their fee.

A Phase 2 PR is described below to help explain some of the unusual aspects of this PR

To avoid backward incompatible changes, we will leave Fee to mean, "I always pay this fee", and Tip will always mean "I am willing to be charged this extra amount". There will need to be two new transaction field, perhaps called RefundableFee and Tax. RefundableFee is an additional amount (in MicroAlgos) which does not necessarily go to the fee sink. Tax represents a factor that the transaction is willing to pay if it is included in block with CongestionTax, but otherwise will not. With these fields added, then we can confirm at evaluation time that Tax + Tip is at least CongestionTax. But only min(Tax, CongestionTax) + Tip will be charged. There are several issues with that change, so prudence leads us to put that off a bit. Which transactions receive the refund, if there are RefundableFees in multiple transactions of a group? Maybe only one transactions can specify a RefundableFee? Can more that one transaction specify Tax? Or does it apply to all? How can logicsigs be protected from unintentional spend (probably, RefundableFee is prohibited for them)? Do apps need similar protection? Maybe inners can't specify these new fields.

So: the short term solution for this PR and the next consensus release is to only enforce the changing congestion value at ingress, then lock that choice in for checking during evaluation even if the congestion level changes. This gives the community time to understand the new congestion model, since it will appear in the block. Wallets and such can begin to offer recommended fees. This consensus change will report the current CongestionTax in the /v2/transactions/params call to allow wallets to make such recommendations.

In a related cleanup, EnableFeePooling is eliminated as a consensus parameter. Since it is strictly more permissive, we can simply assume it has always been true.

Dependent PR

algorand/go-algorand-sdk#761 will be needed to sync the new fields and consensus params

Summary

Test Plan

[codecov]

Codecov Report

❌ Patch coverage is 41.02564% with 207 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.70%. Comparing base (172f19d) to head (5a85fdf).
⚠️ Report is 58 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
data/txntest/txn.go	0.00%	41 Missing ⚠️
ledger/eval/eval.go	35.18%	27 Missing and 8 partials ⚠️
data/basics/units.go	14.81%	19 Missing and 4 partials ⚠️
cmd/goal/common.go	5.26%	18 Missing ⚠️
libgoal/libgoal.go	0.00%	15 Missing ⚠️
libgoal/transactions.go	0.00%	11 Missing ⚠️
data/basics/overflow.go	64.28%	10 Missing ⚠️
data/transactions/transaction.go	16.66%	10 Missing ⚠️
cmd/goal/application.go	0.00%	8 Missing ⚠️
cmd/goal/asset.go	0.00%	6 Missing ⚠️
... and 11 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6400      +/-   ##
==========================================
- Coverage   47.90%   47.70%   -0.20%     
==========================================
  Files         662      654       -8     
  Lines       87991    87982       -9     
==========================================
- Hits        42149    41975     -174     
- Misses      43085    43238     +153     
- Partials     2757     2769      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[algorandskiy]

Okay, now CongestionTax comparison in PreCheck is not consensus gated.
As I understand random values in CongestionTax not really allowed in pre-congestion consensus? - Looks like need one more test case with prev.CongestionTax = 0 and current.CongestionTax != 0 both pre-congestion consensus, and have PreCheck to fail?

[jannotti]

I can do that if you want, but in a sense, there is a stronger check already present.

By testing this:

		prev := BlockHeader{
			Round:         1,
			GenesisID:     "test",
			GenesisHash:   crypto.Digest{0x02, 0x02},
			Load:          0,
			CongestionTax: 2_000_000, // Weird, but allowed (maybe there was a downgrade?)
		}
		prev.CurrentProtocol = protoNoCongestion

		current := BlockHeader{
			Round:         prev.Round + 1,
			GenesisID:     prev.GenesisID,
			GenesisHash:   prev.GenesisHash,
			Branch:        prev.Hash(),
			Branch512:     prev.Hash512(),
			Load:          0,
			CongestionTax: 1_700_000, // Tapers from above.  (3*.9)-1=1.7
		}
		current.CurrentProtocol = protoNoCongestion

We show that the CongestionTax must be correct, because we show it goes from 2.0 to 1.7. If it must be correct, then it must be 0, since all pre-upgrade Loads are 0.

[jannotti]

I do think I should show that the 1_700_000 is required to be exact. I forgot to do that.

I'll make that change, let me know if you want the 0s too.

[algorandskiy]

I agree exact values make sense but this test does not show reality since it is impossible to have non-zero CongestionTax when prev Load is zero (given non-congested consensus). I think we still need to have prev zero and current non-zero error case to make sure such blocks are not really possible.

[jannotti]

Sure, I'll put that in.

This test does reflect reality if we ever turn off LoadTracking in the future. The CongestionTax might be non-zero when that happens, and so this will happen and the tax will slowly get back down to zero.

[jannotti]

18ee6a5

[cce]

This 1e6 constant is because inner txns can't have a non-standard FeeFactor ... I guess that will be true forever but might want to call that out

[jannotti]

Not exactly. Yes, it is true that currently there are no legal non-minfee transactions, but in the PR that adds them as outers (for big notes, programs, etc) I did not intend to block them here.

But notice that this is when we first add a transaction to an inner group. It's currently blank. No type. No notes, nothing. So all we can do is estimate the eventual cost. We presume it is one minFee here only for the purpose of trying to set the newly created txn's Fee. We're trying to find out if the current fee surplus is enough, and if not, we default to the Fee to the amount needed. This can't be perfect for a future with non minFee txns. But:

1. It doesn't have to be. Since the user can still adjust the fee anyway, the real place where it has to be correct is opItxnSubmit where we decide if the inner group really has enough. At that point, we will know if any inners are special and need to cost more.
2. We probably don't even want it to be perfect. By being a lower estimate of the Fee required, we prevent apps from being tricked into paying for expensive inners using the default fee mechanism. They will have to explicitly increment the fee if they want to use big features.
3. Almost nobody uses this default mechanism. They zero out the Fee as a way to demand that the existing fee surplus is enough to pay for the txns. The model people have gravitated to is "Top-level transactions need to send enough surplus to make inners work".

[cce]

If we want to give the API user advice for how to get into the next block should it be NextCongestionTax(latest.Load, latest.CongestionTax)?

[jannotti]

For now, yes, it should be:
txn.Tip = NextCongestionTax(latest.Load, latest.CongestionTax)

We may want to export something like NextCongestionTax to the SDK. Maybe something that takes a number of blocks, and presumes 10% further growth?

In the future, the will want to set txn.Tax to a comfortable over estimate (they won't spend it if it's not needed). And they'll set Tip only if the want priority.

[cce]

I thought the SDK is getting / is expecting to get NextCongestionTax via this transaction params REST API implementation method here in node.go?

[jannotti]

The API will provide the current CongestionTax, but a caller may well prefer to assume further growth. They will never be able to get their transaction into exactly the same next block that they observe with an API call.

[cce]

I forget, how do heartbeats and stateproof txns get past this check now?

[algorandskiy]

I second this, both SP and HB txns must have zero Tip per WellFormed.
This check efficiently prevents SP and HB txn to be accepted into the pool

[jannotti]

Seems like a problem, thank you for catching. I think my fix will be something like
if tax > tip && usage > 0 where I get the usage with SummarizeFees, so that any free transaction automatically avoids needing a tip.

[jannotti]

I did something simpler, but also avoided repeat checks. I simply allow singleton transactions with no fee through. As described in the comment, that is safe, and avoids repeating the same checks in multiple places.
fbc33eb

[cce]

Do we need an explicit tip flag too? So you can manually set tip in addition to fee? If you don't pass the fee flag you will get the auto-suggested tip+fee but if you do pass fee flag you will get 0 tip + fee passed in? Or is anyone who would want to use this stuff going to use an SDK instead of clerk probably

[jannotti]

Added in 5a85fdf

Check it for the four cases of using or not using fee and tip, see if you agree. (I think the only one that may be contentious is using --fee but not setting --tip.

[algorandskiy]

Suggestion: temporary enable congestion fees in current consensus to see what will fail.

[algorandskiy]

should it return Fee = suggested .MinFee * (1 + nilToZero(suggested.CongestionTax)), nilToZero(suggested.CongestionTax) ?

Otherwise ConstructPayment above produces a txn with too low Fee if chain is actually congested so that txn will be rejected.

[jannotti]

Yes, that's right. I will try to think how we can test this stuff better. We should certainly have higher level tests that operate under congestion.

[algorandskiy]

Maybe make a custom proto with low MaxTxnBytesPerBlock so that even few txns per block create congestion condition

[cce]

agreed with not having multiple fee checks, but maybe we could add a Txn.Fee > params.MinFee or non-zero Fee check in WellFormed somewhere? AFAICT a singleton pay transaction with zero fee would pass WellFormed, then use up crypto signature verification CPU, then get admitted to the pool here, then finally die in Remember but maybe we could block 0 fee fields for all but stateproof & heartbeat txns in WellFormed?

[cce]

it is tricky because you really only want it for singleton 0-fee txns so maybe WellFormed is not the right place

[jannotti]

Are you concerned about intentional or unintentional 0-fee txns?

If the intent is to waste our time, they could just use an empty account, but put in a big fee.

If it's a mistake, I can't imagine it being a persistent problem.

[cce]

oh good point .. i was trying to think of reasons to add a transaction type check here but it's true there are tons of other ways to get past WellFormed and this spot.

[jannotti]

Take note, by the way, of what's below. We're not checking any fees here. At most, we're checking the Tip is set properly.

[jannotti]

Closing this, since we decided to go with a smaller PR that just adds the header fields for now, no change to congestion handling/dropping.