near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.33k stars 632 forks source link

add_key_cost.function_call_cost_per_byte is not correctly calculated from #3037

Open ailisp opened 4 years ago

ailisp commented 4 years ago

We found https://github.com/nearprotocol/nearcore/pull/2621 introduced a typo, which cause add_key_cost.function_call_cost_per_byte 1000x more than correct. But besides that, there probably a copy-paste error in https://github.com/nearprotocol/nearcore/commit/f024a28d9b0f1b2238f04d288ddebc8ac12ff4d1, this cause add_key_cost.function_call_cost_per_byte 10000x less than correct. As a result add_key_cost.function_call_cost_per_byte now is about 10x less than correct.

For correct I mean the result i got from rerun param estimator several time in between f024a289 and current master, in both cloud&locally, result is almostly not changed, so it's very likely correct

10x in add function call access key per byte cost is acceptable, we need to correct it some time in future, but must do it in a backward compatible way. In rust check protocol version and apply different fees.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

bowenwang1996 commented 3 years ago

@ailisp what is the status of this issue?

ailisp commented 3 years ago

@bowenwang1996 Examined it's still not fixed. Most recent fee update to add_key_cost.function_call_cost_per_byte is this https://github.com/near/nearcore/commits/master/scripts/migrations/11-runtime-cost-adjustment.py, the number of this cost matches fees.rs, which is prior to this issue. Should be fixable by run runtime param estimator and update cost in a protocol version upgrade I think

bowenwang1996 commented 3 years ago

Okay. @Longarithm please keep this issue in mind when you recalculate the gas costs.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

bowenwang1996 commented 3 years ago

@longarithm is the recalculation done?

Longarithm commented 3 years ago

@bowenwang1996 yes, together with data receipt costs: https://hackmd.io/w6ODyKjUReuuofXTuqdyFQ I think nothing prevents us from updating add key costs. The costs difference in add_key_cost.function_call_cost_per_byte is ~6.25x, which is similar to what is stated in the issue. At the same time base costs can be reduced.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.