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

NEP-536: Reduce the number of refund receipts #536

Merged
merged 8 commits into from
Oct 1, 2024
Merged

Conversation

bowenwang1996
Copy link
Collaborator

This proposal aims to reduce the number of refund receipts and optimize the performance of the protocol. An implementation can be found here

Copy link

render bot commented Mar 13, 2024

@bowenwang1996 bowenwang1996 marked this pull request as ready for review March 13, 2024 04:46
@bowenwang1996 bowenwang1996 requested a review from a team as a code owner March 13, 2024 04:46
neps/nep-0536.md Outdated

Pessimistic gas pricing is removed as a part of this change. This means that transactions that do not involve function calls will not generate gas refund receipts as a result. For function calls, this proposal changes the gas refund to be
```
refund = max(0, gas_refund - 5Tgas);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please mention how 5T was selected.

And how will this impact the final pricing of a simple transfer and of a "simple" function call.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah 5 Tgas is curious to me. Especially since a refund is just a transfer and the cost of a transfer is only 230 Ggas.

5 Tgas seems like overcharging a lot to incentives developers to fiddle with their gas numbers. If this means it's considered essential for contract developers to do exact gas estimates from now on, that's a massive decline in Devx if you ask me and I am not sure if this is really worth it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, that the gas refunds also touch the access key from which they were issued to refund the gas. And the transfer is about 460 (since it's send + exec)

The penalty should be large enough to also give developers estimation range. For example, if a developer estimates their call to be from 7 to 9Tgas. They can attach 10Tgas and no refunds will be generated. This will decrease the load on the shard by removing extra refunds.

@jakmeier
Copy link
Contributor

Looks like a positive change to me and has my support in principle.

Although it has a different goal, the proposal aligns nicely with my proposal to simplify gas pricing (#500) where option 1 is just removing pessimistic gas pricing.

As I read it, the only difference is that on top of removing pessimistic gas pricing, you also reduce refunds by a fixed amount (5 Tgas). I'm not so sure about this change. If developers care about the lost gas value, they will start to estimate exactly how much gas a call needs, which can be difficult and may lead to spurious failures in their apps. And because there are no ways to query gas parameters on chain, this is bound to lead developers to use even more hard coded gas numbers in their smart contracts and relying tightly on them. Which makes it even harder for protocol changes to change gas costs.
So yeah, I don't like it because I believe it makes life of contract developers and protocol developers more difficult in the long term.


## Summary

[Gas refund](https://docs.near.org/concepts/basics/transactions/gas#attach-extra-gas-get-refunded) is a mechanism that allows users to get refunded for gas that is not used during the execution of a smart contract. Due to [pessimistic gas pricing](https://docs.near.org/concepts/basics/transactions/gas-advanced#pessimistic-gas-price-inflation), however, even transactions that do not involve function calls generate refunds because users need to pay at a high price and get refunded the difference. Gas refunds lead to nontrivial overhead in runtime and other places, which hurts the performance of the protocol. This proposal aims to reduce the number of gas refunds and prepare for future changes that completely remove gas refunds.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gas refunds lead to nontrivial overhead in runtime and other places, which hurts the performance of the protocol.

I'm curious how much this really is, do we have numbers on it? I would expect there is about 1 refund (0.23 Tgas) for 1 function call (at least 4.8 Tgas). So I would think it's less than 5% of total workload in terms of gas.
Also, the size of a transfer is tiny and shouldn't be a problem for state witness sizes.

Am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also curious about which percent of the gas refunds are above 5T vs which percent are below.

It would be nice to have a graph where we see the percentage of transactions in some period of time (say last month) of the refund amount vs the percent of transactions below this amount. This could show the impact of the NEP on users and network performance.


## Summary

[Gas refund](https://docs.near.org/concepts/basics/transactions/gas#attach-extra-gas-get-refunded) is a mechanism that allows users to get refunded for gas that is not used during the execution of a smart contract. Due to [pessimistic gas pricing](https://docs.near.org/concepts/basics/transactions/gas-advanced#pessimistic-gas-price-inflation), however, even transactions that do not involve function calls generate refunds because users need to pay at a high price and get refunded the difference. Gas refunds lead to nontrivial overhead in runtime and other places, which hurts the performance of the protocol. This proposal aims to reduce the number of gas refunds and prepare for future changes that completely remove gas refunds.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also curious about which percent of the gas refunds are above 5T vs which percent are below.

It would be nice to have a graph where we see the percentage of transactions in some period of time (say last month) of the refund amount vs the percent of transactions below this amount. This could show the impact of the NEP on users and network performance.


## Security Implications

This change may lead to less correct charging for transactions when there is congestion. However, the entire gas price mechanism needs to be rethought any ways and when only one or two shards are congested, the gas price wouldn't change so there is no difference.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This brings back the known attack of a malicious user clogging the network for a low gas price.

The attack:
The malicious user sends multiple transactions at a low gas price enough to fill a block. Each transaction calls the same contract in a loop until it runs out of gas, spanning a significant period of time.

The cost of the attack with this change will be smaller since the gas is paid at the cost of the gas in the beginning.

This attack can be done repeatedly by waiting until the gas price decreases, keeping the network from the POV of genuine users as either congested or with high gas prices.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid concern which should be addressed with receipt execution order based either on gas price or priority fee, but it's a separate NEP. Even right now the attack above is not mitigated with the pessimistic gas because you can spawn multiple transactions in a short period of time with low attached gas. It only makes it marginally more expensive which is not worth sacrificing for better UX.

neps/nep-0536.md Outdated
```
refund = max(0, gas_refund - 5Tgas);
```
per receipt. In other words, for each gas refund, there is a 5Tgas penalty that is applied. The penalty is added to gas burnt. This means that for receipts that involve gas refund less than 5Tgas, there will be no gas refunds.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to not include the full penalty into the gas_burnt since it would decrease the throughput of the network by artificially taking extra 5Tgas from every function call receipt.

Instead we include this cost into tx_burnt_amount. The gas_burnt should include only send+exec fee for action_receipt+transfer which is the cost of the actual refund receipt. But we burn exec fee immediately, because the refund receipts don't have the burnt gas.

In this case we account for gas refunds at the time of generation, but discourage them from happening by burning the fee.
In the future NEP, the 5Tgas penalty should be changed to be a dynamic fee that grows with the block height until it reaches 300Tgas in which case no new gas refunds are generated.

neps/nep-0536.md Outdated

Pessimistic gas pricing is removed as a part of this change. This means that transactions that do not involve function calls will not generate gas refund receipts as a result. For function calls, this proposal changes the gas refund to be
```
refund = max(0, gas_refund - 5Tgas);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, that the gas refunds also touch the access key from which they were issued to refund the gas. And the transfer is about 460 (since it's send + exec)

The penalty should be large enough to also give developers estimation range. For example, if a developer estimates their call to be from 7 to 9Tgas. They can attach 10Tgas and no refunds will be generated. This will decrease the load on the shard by removing extra refunds.

@evgenykuzyakov
Copy link
Contributor

We've had a discussion with @bowenwang1996 and came with slightly different mechanism.

We should introduce a gas refund receipt cost (about 650Ggas, see below) and "Gas over-attaching penalty".
The penalty will be up to 5% of the refund gas. The goal is to discourage developers from over attaching too much gas. Examples:

  • If the contract wants to refund 280Tgas, burning 5% of it would about 14Tgas which is significant cost and developers would be encouraged to optimize it on the front-end.
  • If refund is 100Tgas, then 5% is 5Tgas - still a lot and achieves the goal
  • If the refund is <10Tgas (very common case for cross-contract call self-callbacks), the penalty should be just 500Ggas, which is less than the gas refund cost. So only the fixed refund cost will be charged from gas to spawn the gas refund receipt. No UX will be broken for legacy cross-contract call contracts, so long as frontend correctly estimates the required gas in worst case scenario.

The 650Ggas comes from the following (based on https://github.com/near/nearcore/blob/master/core/parameters/res/runtime_configs/parameters.snap#L13)

  • action_receipt_creation (send_not_sir + execution): 216119000000
  • action_transfer (send_not_sir + execution): 230246125000
  • action_add_function_call_key (send_not_sir + execution): 204435250000

We need action_add_function_call_key because gas refund might refund the allowance for a given limited signer access key, so should expect it to be included.

Note, for implicit accounts, there is no extra cost, because it system can't create a new implicit account during a refund.

@birchmd
Copy link
Contributor

birchmd commented Mar 18, 2024

Thanks for continuing the discussion here @evgenykuzyakov . I am not sure I fully understand your proposal. I'll rephrase it below and please correct me if I get parts of it wrong.

  1. We keep @bowenwang1996 's current proposal of removing pessimistic gas cost so that only FunctionCall actions can have refunds (all other actions will be charged exactly the right amount of gas at execution so no refunds are needed).
  2. Any FunctionCall action which would have a refund of less than 650 Ggas will no longer receive any refund. This contributes to the goal of reducing the number of refund receipts.
  3. Any FunctionCall action which would have a refund of more than 650 Ggas is now charged the additional 650 Ggas (the cost of running the refund) as well as a 5% "over-estimation fee". The purpose of the additional fee is to encourage front-end developers and end-users interacting via cli to submit accurate gas estimates which do not trigger refunds.

If I understand this correctly then I have a question about the 650 Ggas refund cost. Does the current gas estimation logic for receipts and transfers already include the cost of running the refund? I know these costs are estimated by executing the runtime in some kind of metered environment, but I'm not sure what exactly is included. If the refund cost is already rolled in then I don't think we want to double-charge for it and perhaps other costs could go down. Of course if we are currently not charging for refunds at all then this is an extra important change!

No UX will be broken for legacy cross-contract call contracts, so long as frontend correctly estimates the required gas in worst case scenario.

If my understanding of the proposal as I have written it above is correct then I don't think we need to worry about backwards compatibility because the only change will be to the amount of refunded gas (and this cannot have any impact on the amount of gas attached to promises).

@bowenwang1996
Copy link
Collaborator Author

Does the current gas estimation logic for receipts and transfers already include the cost of running the refund? I know these costs are estimated by executing the runtime in some kind of metered environment, but I'm not sure what exactly is included. If the refund cost is already rolled in then I don't think we want to double-charge for it and perhaps other costs could go down.

It does. So as part of this change, we will rerun the estimator to reduce the action gas costs. This has the benefit that for non-function-call actions, because refunds are removed, they will be cheaper.

@bowenwang1996
Copy link
Collaborator Author

I updated the proposal to reflect #536 (comment). @mfornet @mm-near @birchmd please take a look again

Copy link
Contributor

@birchmd birchmd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the changes that have been made to the proposal. I think it would still be good to justify the 5% part of the new receipt cost.

neps/nep-0536.md Outdated Show resolved Hide resolved
Pessimistic gas pricing is removed as a part of this change. This means that transactions that do not involve function calls will not generate gas refund receipts as a result. For function calls, this proposal introduces cost of refund to be
```
REFUND_FIXED_COST = action_receipt_creation + action_transfer + action_add_function_call_key
refund_cost = max(REFUND_FIXED_COST, 0.05 * gas_refund);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to justify this 5% number. How is it chosen instead of 6% or 4%? What network data is used to make this decision?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an arbitrary number. The goal here is to start penalizing users for attaching too much gas. The percentage should be large enough so that someone attaching 100Tgas or more is penalized for doing so but also small enough so that a callback with a fixed amount of gas reserved are not heavily impacted because the contract is potentially already locked. I think a smaller percentage would work well too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small enough so that a callback with a fixed amount of gas reserved are not heavily impacted

Shouldn't fixed gas callbacks be unaffected by this change? This change effectively makes refunds smaller, but there is no refund for gas reserved for promises until after those receipts have already executed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The callbacks themselves are unaffected, but the amount of gas refund would be

neps/nep-0536.md Outdated Show resolved Hide resolved
@birchmd
Copy link
Contributor

birchmd commented Apr 10, 2024

As a reviewer and WG member, I lean towards approving this proposal. I still think it would be nice to have a data-driven approach to choosing protocol parameters (as opposed to picking 5% because it looks ok), but I understand there are time pressures which prevent us from living in an ideal world. Since this proposal also punishes overestimating gas I think it is important to improve the tooling around gas estimation. I understand this is more challenging in Near's async, sharded system than in the EVM where any RPC node can fully simulate a transaction, but DEVX is still important to the success in the Near ecosystem.

@mm-near
Copy link

mm-near commented Apr 26, 2024

As a working group member, I lean towards approving this proposal.

One important outcome of this proposal, is that "attaching MAX possible gas is now penalised" (as AFAIK in the past it was not). This is a large discrepancy from many of the other blockchains (for example Ethereum), so it should be clearly documented.

I'd also like to make sure that this new fee is clearly shown in the explorer -- so that users can see, the consequences of their gas overestimation - and exert some pressure on the app developers/wallets to improve the gas estimations (otherwise, if this is displayed together with all the other fees, users will not notice, and will just consider NEAR "expensive").

## Security Implications

This change may lead to less correct charging for transactions when there is congestion. However, the entire gas price mechanism needs to be rethought any ways and when only one or two shards are congested, the gas price wouldn't change so there is no difference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's consider the following scenario:

  • tx with 300TG is included block b1, and converted into receipts (containing function calls).
  • Eventually, only 50TG was used to process all receipts.

How does this impact the block limit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't affect how gas used is computed, so there is no impact on block limit

@flmel flmel changed the title Reduce the number of refund receipts NEP-536: Reduce the number of refund receipts Sep 27, 2024
@flmel flmel added WG-protocol Protocol Standards Work Group should be accountable A-NEP A NEAR Enhancement Proposal (NEP). S-review/needs-sme-review A NEP in the REVIEW stage is waiting for Subject Matter Expert review. labels Sep 27, 2024
@flmel flmel removed the S-review/needs-sme-review A NEP in the REVIEW stage is waiting for Subject Matter Expert review. label Sep 27, 2024
@render render bot temporarily deployed to reduce-refunds - nomicon PR #536 October 1, 2024 10:41 Destroyed
@flmel flmel merged commit 87fdb8b into master Oct 1, 2024
4 checks passed
@flmel flmel deleted the reduce-refunds branch October 1, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NEP A NEAR Enhancement Proposal (NEP). S-approved A NEP that was approved by a working group. WG-protocol Protocol Standards Work Group should be accountable
Projects
Status: APPROVED NEPs
Development

Successfully merging this pull request may close these issues.

9 participants