search

near / nearcore

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

Rethink gas spending logic #5148

Open olonho opened 2 years ago

olonho commented 2 years ago

During work on https://github.com/near/nearcore/pull/5121 we identified two questionable places in gas metering.

  1. When transaction fails we still require (by tests) that used amount of gas is sum of promised and burnt gas. Seems more natural to zero out promised part.
  2. When charging for gas, we always burn the gas even if go the error path. It looks strange and not match with normal paying logic: no operation - no payment.
matklad commented 2 years ago

If we protocol change this, we should also consider changing how we limit promises gas. Rather than checking gas limit on every deduct_gas operation, we can check it once, after wasm execution finishes. Note that we already postpone actual promise creation in the Rust SDK.

olonho commented 2 years ago

It's not hard to keep exact promises gas counter IMO.

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.

jakmeier commented 1 year ago

When transaction fails we still require (by tests) that used amount of gas is sum of promised and burnt gas. Seems more natural to zero out promised part.

I agree in principle with that statement but want to make clear that it doesn't matter, since on an error, we refund prepaid - burnt and the promised gas is not relevant:

https://github.com/near/nearcore/blob/cd7150ecf3a4ea368b55c80a60044684a13dc327/runtime/runtime/src/lib.rs#L761-L765