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

Fresh runtime api instance per call estimation #565

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions client/rpc/src/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1524,9 +1524,21 @@ where
used_gas: U256,
}

// Create a helper to check if a gas allowance results in an executable transaction
// Create a helper to check if a gas allowance results in an executable transaction.
//
// A new ApiRef instance needs to be used per execution to avoid the overlayed state to affect
// the estimation result of subsequent calls.
//
// Note that this would have a performance penalty if we introduce gas estimation for past
// blocks - and thus, past runtime versions. Substrate has a default `runtime_cache_size` of
// 2 slots LRU-style, meaning if users were to access multiple runtime versions in a short period
// of time, the RPC response time would degrade a lot, as the VersionedRuntime needs to be compiled.
//
// To solve that, and if we introduce historical gas estimation, we'd need to increase that default.
#[rustfmt::skip]
let executable = move |request, gas_limit, api_version, estimate_mode| -> Result<ExecutableResult> {
let executable = move |
request, gas_limit, api_version, api: sp_api::ApiRef<'_, C::Api>, estimate_mode
| -> Result<ExecutableResult> {
let CallRequest {
from,
to,
Expand Down Expand Up @@ -1689,7 +1701,13 @@ where
data,
exit_reason,
used_gas,
} = executable(request.clone(), highest, api_version, estimate_mode)?;
} = executable(
request.clone(),
highest,
api_version,
client.runtime_api(),
estimate_mode,
)?;
match exit_reason {
ExitReason::Succeed(_) => (),
ExitReason::Error(ExitError::OutOfGas) => {
Expand All @@ -1714,6 +1732,7 @@ where
request.clone(),
get_current_block_gas_limit().await?,
api_version,
client.runtime_api(),
estimate_mode,
)?;
match exit_reason {
Expand Down Expand Up @@ -1755,7 +1774,13 @@ where
data,
exit_reason,
used_gas: _,
} = executable(request.clone(), mid, api_version, estimate_mode)?;
} = executable(
request.clone(),
mid,
api_version,
client.runtime_api(),
estimate_mode,
)?;
match exit_reason {
ExitReason::Succeed(_) => {
highest = mid;
Expand Down