docs: ADR-048 a multi-tier in-protocol gas price system #10653

yihuang · 2021-12-01T16:04:53Z

Description

Discussion thread about in-protocol gas price: #8224

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

docs/architecture/adr-047-consensus-fees.md

tomtau · 2021-12-03T01:14:19Z

docs/architecture/adr-047-consensus-fees.md

+- No priority conflict with custom priority based on transaction types, since this proposal only occupy three priority levels.
+
+### Negative
+


this is perhaps the case for all dynamic fee systems, but one negative or neutral consequence will be that Tendermint's create-empty-blocks = false option won't work, because the state / app hash will keep changing at every block

When create-empty-blocks = false, the validator doesn't propose new blocks when there's no tx, the consequence is the gas price is not adjusted in the meantime?

robert-zaremba · 2021-12-05T12:21:17Z

We already have ADR-47: #10602 (It was created earlier).
Please rename to ADr-048

hxrts · 2021-12-08T00:10:48Z

Is there any more literature on this IOHK mechanism? The linked blog post is very light on detail. EIP-1559 is well modelled and studied in the wild. Even if this other mechanism has some desirable properties EIP-1559 seems like a good SDK default if only due to available information. Willing to be convinced otherwise so please share further info.

Also, FYI, we want fees to be customisable and moved to a middleware design to make this more straightforward.

yihuang · 2021-12-08T00:16:41Z

Is there any more literature on this IOHK mechanism? The linked blog post is very light on detail. EIP-1559 is well modelled and studied in the wild. Even if this other mechanism has some desirable properties EIP-1559 seems like a good SDK default if only due to available information. Willing to be convinced otherwise so please share further info.

There's no more detailed information other than the blog yet.

yihuang · 2021-12-08T00:57:40Z

We already have ADR-47: #10602 (It was created earlier). Please rename to ADr-048

Thanks, renamed.

tac0turtle · 2021-12-09T14:04:35Z

docs/architecture/adr-048-consensus-fees.md

+
+##### Negative
+
+- Can't include urgent transactions with priority.


this would be built into the middleware imo. If a chain needs a special tx they would be aware of this and preprogram it

tac0turtle · 2021-12-09T14:08:26Z

Reading through this, it makes me think if we could have real world numbers in cosmos that would allude to a decision. EIP-1559 while nice doesn't bring much value to chains that have little volume as the fee may end up at 0 anyways?

Basic tx priority is currently merged into the sdk, past this, if a chain has multiple tokens as fees, they would have to implement some kind of oracle to get the price of this token compared to others.

adu-web3 · 2021-12-10T04:25:25Z

docs/architecture/adr-048-consensus-fees.md

+##### Negative
+
+- Unpredictable.
+- Overpricing the tip price in theory TODO I guess we don't obverse this in real world on ethereum ?


if there is a sudden congestion owing to huge price volatility of assets, EIP-1559 will degrade to first price auction

yihuang · 2021-12-10T07:09:47Z

Reading through this, it makes me think if we could have real world numbers in cosmos that would allude to a decision. EIP-1559 while nice doesn't bring much value to chains that have little volume as the fee may end up at 0 anyways?

Actually, this is one of the places where multi-tier design is better than EIP-1559:

The gas price in the base tier is constant.
Only the gas price in the higher tier is adjusted based on block load, I guess it should also have a lower bound which is larger than lower-tier price.

Basic tx priority is currently merged into the sdk, past this, if a chain has multiple tokens as fees, they would have to implement some kind of oracle to get the price of this token compared to others.

The current implementation seems to fit in the "first price auction" category, right?

hxrts · 2021-12-10T13:09:00Z

kind of. since there's no prioritisation you're not auctioning anything really. will change soon.

maybe @dogemos has an opinion on supporting a multi-tier fee model.

yihuang · 2021-12-10T13:11:49Z

kind of. since there's no prioritisation you're not auctioning anything really. will change soon.

maybe @dogemos has an opinion on supporting a multi-tier fee model.

I think the current master already prioritize tx based on the fee it pays: https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/middleware/priority.go#L38.

adu-web3 · 2021-12-15T09:26:26Z

docs/architecture/adr-048-consensus-fees.md

+### Positive
+
+- The default tier keeps the same predictable gas price experience for client.
+- No priority conflict with custom priority based on transaction types, since this proposal only occupy three priority levels.


This multi-tier pricing system should be considered as only a default one for Cosmos-SDK.

If the application wants customized priority based on transaction types, they could implement their own pricing mechanism by overriding the code or reusing the code, for example:

type PrioritizedTx interface { Priority(ctx) int }
Given tier is an optional field for transaction, the application could custom priorities for different transaction types without considering tier by implementing this interface for customized transaction type in application level.

I'd like to see some sort of fallback gas policy that applies to all transaction types. This could be a possible instantiation of the fall back policy.

Takes for instance Terra Oracle Msgs or or GravityEthereum messages.

For example, the Terra ante handler checks to see if all the msgs in the multimsg are oracle messages and applies one fee policy and otherwise applies the fallback fee policy. https://github.com/terra-money/core/blob/main/custom/auth/ante/spamming_prevention.go#L50-L104

We really want to enable use cases like Terra to operate without having to override the entire interface and instead provide middleware that composes with this system. If all the middleware before the multi tier fee module returns nil, then the multi tier fee model applies. In the terra example, if a multi msg combines both oracle msg and bank send messages than the multi tier fee model would apply to both. If the multi msg is all Oracle messages than the messages would have max priority or post ABCI++ use reserved oracle message block space.

@Muggle-Du totally - we don't want to have a single choice.

@zmanian good point - this should definitely compose with other functionalities.

robert-zaremba · 2021-12-16T23:06:32Z

docs/architecture/adr-048-consensus-fees.md

+
+#### First price auction
+
+Before EIP-1559, ethereum miners select transactions ranked by the highest fees, which creates a first price auction market.


this is what we are using now.

robert-zaremba

ADR must have more clarity in the design:

how are we going to update the transaction structure?
I think we need a design which enables optional activation (so chains can update to newer SDK without being forced to use the new fee model).

robert-zaremba · 2021-12-16T23:07:09Z

docs/architecture/adr-048-consensus-fees.md

+
+Since tendermint 0.35 has supported mempool prioritization, we can take advantage of that to implement more sophisticated gas fee system.
+
+### Compare Different Designs


I don't think ADR is the right place for making comparisons.

removed now.

docs/architecture/adr-048-consensus-fees.md

robert-zaremba · 2021-12-16T23:15:59Z

docs/architecture/adr-048-consensus-fees.md

+- The default tier keeps the same predictable gas price experience for client.
+- No priority conflict with custom priority based on transaction types, since this proposal only occupy three priority levels.
+
+### Negative


there is a negative consequence: the update will require wallets & tools update.

Also, it might need the TM core protocol upgrades support.

robert-zaremba · 2021-12-16T23:17:56Z

docs/architecture/adr-048-consensus-fees.md

+def txPriorityHandler_checkTx(ctx, tx):
+    res, err := next_CheckTx(ctx, tx)
+    # pass priority to tendermint
+    res.Priority = Params.tiers[tx_tier(tx)].priority


we still need to order transactions in a tire. Probably we still need to add gas price component here.

I think the point in this design is to not order tx by price directly. Of course, we still have the issue if people all flooded to the higher tier, the lower tier won't have a chance to get included, the potential solution reserves block area for each tier, but that'll need to change tendermint.

We know that ordering only by price is not the perfect solution, but it's the most general one which is good for simapp example, until we have something better. We want transactions prioritized by type , price and and transaction distribution (by type).

That being said, an ADR proposal must be more complete.

Some of the tires can definitely be full, so we should decide about the order. It can be FIFO or or anything... I think ordering by gas price makes most of the sense in this design.

the order would be FIFO under the same tier/priority.

OK, so let's add it to the doc

this ADR needs the TM core to change the tx behavior in mempool. Maybe needs TM core to support the plug-in mempool(separate the mempool component from TM core) for supporting the ADR requirement.

Tendermint 0.35 can already prioritize transactions, and we also have a primitive example in the SDK how to use it.

sunnya97 · 2021-12-18T17:59:17Z

docs/architecture/adr-048-consensus-fees.md

+
+The original proposal reserve block segments for different tiers, we can't do that without modify tendermint, but it should be good enough to just specify higher priority for higher tier transactions. 
+
+The lowest tier can use constant gas price, the higher tiers adjust gas price based on block load at different speed.


If blocks are consistently full, then won't the lowest tier txs just never make it in, removing the "predictable pricing"?

Once we have ABCI++, we reserve a portion of block space for medium and low fee tiers to ensure that this pool of transaction is slowly drained even in full block space environment.

Wouldn't that just create an out-of-band tx fee market then?

Wouldn't that just create an out-of-band tx fee market then?

within the same tier, the txs are processed in FIFO manner, avoiding the competition on gas price.

hxrts · 2021-12-21T18:40:50Z

I'm still thinking we should stick to EIP-1559 as the sdk default. We want to make it easy for people to augment and experiment with alternatives, but simplicity and a well understood mechanism is pretty important as a baseline.

I'd like to push to get typed transactions that can reserve block space, but I'm still of the opinion that this should be future work.

docs/architecture/adr-048-consensus-fees.md

robert-zaremba

decide if we want to do comparison in the ADR
improve design to allow priority composability
update consequences

robert-zaremba · 2022-01-20T10:54:00Z

docs/architecture/adr-048-consensus-fees.md

+    consensus_price = State.gas_prices[tx_tier(tx)]
+    min_price = max(validator_price, consensus_price)
+
+    if tx.gas_price() < min_price:


how the user will know what gas price to use? Maybe if we use 0then we should automatically adjust and set the min_price?

I tried to keep exiting tx fields and semantics in this proposal, but maybe it makes more sense to change it to gasPriceCap, so it'll always be charged with what's computed on-chain as long as the computed price <= gasPriceCap. And we shouldn't use the per-validator minimal-gas-price anymore, it's not compatible with the consensus logic.

updated proposal to change gasPrice to gasPriceCap, the actual gas price used is the one computed in consensus.

robert-zaremba · 2022-01-20T10:54:27Z

docs/architecture/adr-048-consensus-fees.md

+### Positive
+
+- The default tier keeps the same predictable gas price experience for client.
+- No priority conflict with custom priority based on transaction types, since this proposal only occupy three priority levels.


@Muggle-Du totally - we don't want to have a single choice.

@zmanian good point - this should definitely compose with other functionalities.

alexanderbez · 2022-01-24T18:50:45Z

docs/architecture/adr-048-consensus-fees.md

+- Tier 2: a dynamic gas price which is adjusted according to previous block load.
+- Tier 3: a dynamic gas price which is adjusted according to previous block load.


These are identical BTW

added: "at a higher speed".

docs/architecture/adr-048-consensus-fees.md

alexanderbez · 2022-01-24T18:51:45Z

docs/architecture/adr-048-consensus-fees.md

+
+Add a new `tier` field in `Tx` to specify the preferred tier of service.
+
+Change the semantic of `gasPrice` to `gasPriceCap`, which is the maximum gas price can be charged, if the consensus gas price is larger than `gasPriceCap`, the transaction should be rejected.


So the consensus gas price acts as a floor?

the consensus gas price is the actual gas price user is charged, since it's dynamic, the gasPriceCap is to prevent that price is unexpectedly high to user.

should we rephrase:
"the transaction should be rejected" -> "the transaction won't be included and will stay in the mempool until the gas will go to the gasPriceCap level. The mempool can eventually prune old transactions"

yeah, that's better, changed.
implementation-wise, that'll need abci++ right?

I don't think so, the mempool management is already there. Let's double check with @cmwaters

yihuang · 2022-03-14T15:46:35Z

Let's add tire configuration to the spec.

done

robert-zaremba

Almost there:

This snippet should be updated: https://github.com/cosmos/cosmos-sdk/pull/10653/files#r788646099
Expand information about FIFO
- that this is the default mechanism, delivered by Tendermint
- FIFO within a tire is not ensured by consensus so a validator can tweak the Tendermint code to order txs in a tire differently
add a note about composability:
- we should be able to create tires by Tx type: this way we can easily prioritize both by user tire selection and tx type (eg to prioritize evidence or IBC transactions)

robert-zaremba · 2022-03-17T08:47:27Z

docs/architecture/adr-048-consensus-fees.md

+def txPriorityHandler_checkTx(ctx, tx):
+    res, err := next_CheckTx(ctx, tx)
+    # pass priority to tendermint
+    res.Priority = Params.tiers[tx_tier(tx)].priority


tx_tier returns number, so we should remove .priority.

Suggested change

res.Priority = Params.tiers[tx_tier(tx)].priority

res.Priority = Params.tiers[tx_tier(tx)]

hmm, the original one is good I think, tx_tier(tx) returns the gas_tier, which is used to index the tier parameter list Params.tiers to get the tier parameters, then use the configured .priority field of it as the tx priority.

yihuang · 2022-03-17T10:50:22Z

we should be able to create tires by Tx type: this way we can easily prioritize both by user tire selection and tx type (eg to prioritize evidence or IBC transactions)

I'm not sure about this part, we can use different priorities based on tx types, but that takes completely different tx type, not just msg types.
An alternative is just to configure the relayer to always pick a higher tier.
Another approach is to reserve some high tiers for permission-ed usage, which only certain registered senders can use.

robert-zaremba

let's make the final round and get approvals. cc: @aaronc , @hxrts , @zmanian , @alexanderbez

robert-zaremba · 2022-03-17T11:55:28Z

I've added a commit explaining tx priority composability: c85d663

yihuang · 2022-03-17T13:30:43Z

I've added a commit explaining tx priority composability: c85d663

LGTM

yihuang · 2022-03-18T15:30:26Z

tire -> tier? there are several other terms in this section.

good catch, fixed.

tac0turtle · 2022-04-22T08:41:32Z

Hey, on one of the previous architecture calls we talked about the maintain of this feature and how it would be done on top of the feature's inclusion in the sdk. At the end there was a larger portion of people agreeing that we can merge the Protobuf changes to allow applications to use this but we shouldn't merge this into the sdk itself. It is unclear who all wants this structure and I would opt for users writing their own or using this feature which would be hosted in a different repo.

robert-zaremba · 2022-04-26T23:17:50Z

I think it's still good to have this ADR and merge it.

tac0turtle · 2022-04-26T23:35:33Z

I think it's still good to have this ADR and merge it.

merging this is a requirement, but should mark as rejected for being housed in the sdk or something along those lines.

docs/architecture/adr-048-consensus-fees.md

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

robert-zaremba · 2022-07-11T23:44:11Z

Any reason this was merged as rejected instead of proposed?

tomtau · 2022-07-12T05:51:25Z

#10653 (comment)
afaik the consensus seemed that the mechanism wasn't that well understood, so it could be good to be implemented, but not to be offered as a default one in SDK

robert-zaremba · 2022-07-12T16:05:11Z

the mechanism requires some tx proto customization, which we added.
Probably we don't have a clear difference between Proposed and Rejected. I think something can be well designed for SDK or even partially implemented (as an example or a template) but not integrated by default.

yihuang requested review from aaronc and alexanderbez as code owners December 1, 2021 16:04

github-actions bot added the T: ADR An issue or PR relating to an architectural decision record label Dec 1, 2021

yihuang changed the title ~~adr: Propose a multi-tier in-protocol gas price system~~ adr: ADR-047 a multi-tier in-protocol gas price system Dec 1, 2021

alexanderbez reviewed Dec 2, 2021

View reviewed changes

docs/architecture/adr-047-consensus-fees.md Outdated Show resolved Hide resolved

tomtau reviewed Dec 3, 2021

View reviewed changes

yihuang changed the title ~~adr: ADR-047 a multi-tier in-protocol gas price system~~ adr: ADR-048 a multi-tier in-protocol gas price system Dec 8, 2021

yihuang force-pushed the adr-consensus-fees branch from e1995c0 to 2dc17f9 Compare December 8, 2021 00:57

tac0turtle reviewed Dec 9, 2021

View reviewed changes

adu-web3 reviewed Dec 10, 2021

View reviewed changes

adu-web3 reviewed Dec 15, 2021

View reviewed changes

robert-zaremba reviewed Dec 16, 2021

View reviewed changes

sunnya97 reviewed Dec 18, 2021

View reviewed changes

robert-zaremba reviewed Jan 20, 2022

View reviewed changes

docs/architecture/adr-048-consensus-fees.md Outdated Show resolved Hide resolved

robert-zaremba reviewed Jan 20, 2022

View reviewed changes

yihuang force-pushed the adr-consensus-fees branch 2 times, most recently from f6b4384 to f0fce5a Compare January 24, 2022 04:21

yihuang requested a review from robert-zaremba January 24, 2022 04:27

alexanderbez reviewed Jan 24, 2022

View reviewed changes

yihuang force-pushed the adr-consensus-fees branch from 45ebb87 to fae48fe Compare January 25, 2022 01:38

yihuang force-pushed the adr-consensus-fees branch from ce29ba7 to e2024d3 Compare March 14, 2022 15:42

minor updates

ff773ae

fix review suggestions

e263882

robert-zaremba reviewed Mar 17, 2022

View reviewed changes

fix review suggestions

712011f

Add more information about transaction composability.

c85d663

robert-zaremba approved these changes Mar 17, 2022

View reviewed changes

yihuang mentioned this pull request Mar 18, 2022

Implement in-protocol gas price system in an extensible way #11411

Closed

7 tasks

tire -> tier

3598e72

yihuang mentioned this pull request Mar 29, 2022

Implement ADR-048 multi-tier gas price system #11494

Closed

4 tasks

robert-zaremba requested review from zmanian and alexanderbez April 26, 2022 23:18

yihuang mentioned this pull request Apr 27, 2022

Problem: Pending decision on adding feemarket module to chain-main crypto-org-chain/chain-main#758

Open

alexanderbez approved these changes Apr 27, 2022

View reviewed changes

docs/architecture/adr-048-consensus-fees.md Outdated Show resolved Hide resolved

yihuang and others added 2 commits April 28, 2022 06:11

Update docs/architecture/adr-048-consensus-fees.md

3048168

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

Merge branch 'main' into adr-consensus-fees

f74c0f4

alexanderbez changed the title ~~adr: ADR-048 a multi-tier in-protocol gas price system~~ docs: ADR-048 a multi-tier in-protocol gas price system Apr 27, 2022

alexanderbez merged commit 9e6d2d9 into cosmos:main Apr 27, 2022

yihuang deleted the adr-consensus-fees branch April 28, 2022 00:12

		- No priority conflict with custom priority based on transaction types, since this proposal only occupy three priority levels.

		### Negative


		##### Negative

		- Can't include urgent transactions with priority.


		#### First price auction

		Before EIP-1559, ethereum miners select transactions ranked by the highest fees, which creates a first price auction market.


		Since tendermint 0.35 has supported mempool prioritization, we can take advantage of that to implement more sophisticated gas fee system.

		### Compare Different Designs


		The original proposal reserve block segments for different tiers, we can't do that without modify tendermint, but it should be good enough to just specify higher priority for higher tier transactions.

		The lowest tier can use constant gas price, the higher tiers adjust gas price based on block load at different speed.

		- Tier 2: a dynamic gas price which is adjusted according to previous block load.
		- Tier 3: a dynamic gas price which is adjusted according to previous block load.


		Add a new `tier` field in `Tx` to specify the preferred tier of service.

		Change the semantic of `gasPrice` to `gasPriceCap`, which is the maximum gas price can be charged, if the consensus gas price is larger than `gasPriceCap`, the transaction should be rejected.

	res.Priority = Params.tiers[tx_tier(tx)].priority
	res.Priority = Params.tiers[tx_tier(tx)]

docs: ADR-048 a multi-tier in-protocol gas price system #10653

docs: ADR-048 a multi-tier in-protocol gas price system #10653

Conversation

yihuang commented Dec 1, 2021 • edited Loading

Description

Author Checklist

Reviewers Checklist

Choose a reason for hiding this comment

yihuang Dec 8, 2021 • edited Loading

Choose a reason for hiding this comment

robert-zaremba commented Dec 5, 2021

hxrts commented Dec 8, 2021

yihuang commented Dec 8, 2021

yihuang commented Dec 8, 2021

Choose a reason for hiding this comment

tac0turtle commented Dec 9, 2021

Choose a reason for hiding this comment

yihuang commented Dec 10, 2021 • edited Loading

hxrts commented Dec 10, 2021

yihuang commented Dec 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robert-zaremba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yihuang Dec 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yihuang Jan 24, 2022 • edited Loading

Choose a reason for hiding this comment

hxrts commented Dec 21, 2021

robert-zaremba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yihuang Jan 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yihuang Jan 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yihuang commented Mar 14, 2022

robert-zaremba left a comment • edited by yihuang Loading

Choose a reason for hiding this comment

robert-zaremba Mar 17, 2022 • edited Loading

Choose a reason for hiding this comment

yihuang Mar 17, 2022 • edited Loading

Choose a reason for hiding this comment

yihuang commented Mar 17, 2022

robert-zaremba left a comment • edited by hxrts Loading

Choose a reason for hiding this comment

robert-zaremba commented Mar 17, 2022

yihuang commented Mar 17, 2022

yihuang commented Mar 18, 2022

tac0turtle commented Apr 22, 2022

robert-zaremba commented Apr 26, 2022

tac0turtle commented Apr 26, 2022

robert-zaremba commented Jul 11, 2022

tomtau commented Jul 12, 2022

robert-zaremba commented Jul 12, 2022

yihuang commented Dec 1, 2021 •

edited

Loading

yihuang Dec 8, 2021 •

edited

Loading

yihuang commented Dec 10, 2021 •

edited

Loading

yihuang commented Dec 10, 2021 •

edited

Loading

yihuang Dec 17, 2021 •

edited

Loading

yihuang Jan 24, 2022 •

edited

Loading

yihuang Jan 24, 2022 •

edited

Loading

yihuang Jan 25, 2022 •

edited

Loading

robert-zaremba left a comment •

edited by yihuang

Loading

robert-zaremba Mar 17, 2022 •

edited

Loading

yihuang Mar 17, 2022 •

edited

Loading

robert-zaremba left a comment •

edited by hxrts

Loading