Use constant minimum_new_receipt_gas #10941
Conversation
There was a problem hiding this comment.
Requesting changes for the issue in fetching the latest protocol version. I’m not an expert in how our blocks look like either, so I’d be happy to yield to anyone knowledgeable who’d confirm the current code is correct and my understanding of the other fetches of the current protocol version are wrong :)
aborg-dev
force-pushed
the
akashin/bounded_refunds
branch
2 times, most recently
from
871ee4e
to
923078e
Compare
|
@Akashin I am confused about the number. Why don't we use the exact same number as |
runtime/runtime/src/config.rs
Outdated
| // congestion when receipts are delayed before they execute. Hence there is not much | ||
| // value to tie this limit to the function call base cost. Making it constant limits | ||
| // overcharging to 6x, which was the value before the cost increase. | ||
| 4_850_000_000_000 // 4.85TGas. |
There was a problem hiding this comment.
nit. is there some central place (eg. a file containing important constants) that this number be defined with a constant (for discoverability)?
There was a problem hiding this comment.
In general, I agree with introducing dedicated constants. In this particular case, I don't think there is much value to be gained by doing this - we don't plan to reuse it anywhere (if we need to, adding a constant would make sense, otherwise, YAGNI applies), we already have a docstring that explains it and seeing it in a separate file without a usage context will yield very little insight to the reader and will introduce unnecessary indirection when reading this code.
aborg-dev
force-pushed
the
akashin/bounded_refunds
branch
from
3e63d7a
to
e721559
Compare
aborg-dev
changed the title
I've updated the code to avoid the protocol update. We still need to look at the protocol version to keep the behavior consistent for past blocks where the gas costs were different. |
This addresses the issue from #10829. Because we plan to decrease the function call cost, we need to modify the refund estimation logic that relies on function call base cost. This PR sets minimum_new_receipt_gas to 108_059_500_000 + 2_319_861_500_000 + 2_319_861_500_000 ~= 4.7TGas which would limit the overcharging to 6x which is the current level.
aborg-dev
force-pushed
the
akashin/bounded_refunds
branch
from
e721559
to
0116edd
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10941 +/- ##
==========================================
+ Coverage 71.56% 71.57% +0.01%
==========================================
Files 758 758
Lines 152127 152135 +8
Branches 152127 152135 +8
==========================================
+ Hits 108871 108892 +21
+ Misses 38752 38736 -16
- Partials 4504 4507 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This addresses the issue from #10829. Because we plan to decrease the function call cost, we need to modify the refund estimation logic that [relies](https://near.zulipchat.com/#narrow/stream/295306-contract-runtime/topic/major.20function.20call.20fees.20today/near/427527995) on function call base cost. This PR sets [minimum_new_receipt_gas](https://github.com/near/nearcore/blob/32ef06752f3702e22bd69542df07a4e6e123477e/runtime/runtime/src/config.rs#L270) to 108_059_500_000 + 2_319_861_500_000 + 2_319_861_500_000 ~= 4.7TGas which would limit the overcharging to 6x which is the current level.
This addresses the issue from #10829. Because we plan to decrease the function call cost, we need to modify the refund estimation logic that [relies](https://near.zulipchat.com/#narrow/stream/295306-contract-runtime/topic/major.20function.20call.20fees.20today/near/427527995) on function call base cost. This PR sets [minimum_new_receipt_gas](https://github.com/near/nearcore/blob/32ef06752f3702e22bd69542df07a4e6e123477e/runtime/runtime/src/config.rs#L270) to 108_059_500_000 + 2_319_861_500_000 + 2_319_861_500_000 ~= 4.7TGas which would limit the overcharging to 6x which is the current level.
This PR decreases the function call base execution cost to 1TGas. It is stacked on #10941.
This addresses the issue from #10829.
Because we plan to decrease the function call cost, we need to modify the refund estimation logic that relies on function call base cost.
This PR sets minimum_new_receipt_gas to 108_059_500_000 + 2_319_861_500_000 + 2_319_861_500_000 ~= 4.7TGas which would limit the overcharging to 6x which is the current level.