-
Notifications
You must be signed in to change notification settings - Fork 320
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
Improve gas checks in Randomness precompile #2051
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,13 +45,33 @@ pub const INCREASE_REQUEST_FEE_ESTIMATED_COST: u64 = 16995; | |
pub const EXECUTE_EXPIRATION_ESTIMATED_COST: u64 = 22201; | ||
|
||
/// Fulfillment overhead cost, which takes input weight hint -> weight -> return gas | ||
pub fn fulfillment_overhead_gas_cost<T: pallet_evm::Config>(num_words: u8) -> u64 { | ||
pub fn prepare_and_finish_fulfillment_gas_cost<T: pallet_evm::Config>(num_words: u8) -> u64 { | ||
<T as pallet_evm::Config>::GasWeightMapping::weight_to_gas( | ||
SubstrateWeight::<T>::prepare_fulfillment(num_words.into()) | ||
.saturating_add(SubstrateWeight::<T>::finish_fulfillment()), | ||
) | ||
} | ||
|
||
pub fn subcall_overhead_gas_costs<T: pallet_evm::Config>() -> EvmResult<u64> { | ||
// cost of log don't depend on specific address. | ||
let log_cost = log_fulfillment_failed(H160::zero()) | ||
.compute_cost() | ||
.map_err(|_| revert("failed to compute log cost"))?; | ||
let call_cost = call_cost(U256::zero(), <T as pallet_evm::Config>::config()); | ||
log_cost | ||
.checked_add(call_cost) | ||
.ok_or(revert("overflow when computing overhead gas")) | ||
} | ||
|
||
pub fn transaction_gas_refund<T: pallet_evm::Config>() -> u64 { | ||
// 21_000 for the transaction itself | ||
// we also include the fees to pay for input request id which is 32 bytes, which is in practice | ||
// a u64 and thus can only occupy 8 non zero bytes. | ||
21_000 | ||
+ 8 * T::config().gas_transaction_non_zero_data | ||
+ 24 * T::config().gas_transaction_zero_data | ||
} | ||
|
||
pub const LOG_FULFILLMENT_SUCCEEDED: [u8; 32] = keccak256!("FulFillmentSucceeded()"); | ||
pub const LOG_FULFILLMENT_FAILED: [u8; 32] = keccak256!("FulFillmentFailed()"); | ||
|
||
|
@@ -65,37 +85,44 @@ pub fn log_fulfillment_failed(address: impl Into<H160>) -> Log { | |
|
||
/// Reverts if fees and gas_limit are not sufficient to make subcall and cleanup | ||
fn ensure_can_provide_randomness<Runtime>( | ||
code_address: H160, | ||
gas_limit: u64, | ||
remaining_gas: u64, | ||
request_gas_limit: u64, | ||
request_fee: BalanceOf<Runtime>, | ||
clean_up_cost: u64, | ||
subcall_overhead_gas_costs: u64, | ||
prepare_and_finish_fulfillment_gas_cost: u64, | ||
) -> EvmResult<()> | ||
where | ||
Runtime: pallet_randomness::Config + pallet_evm::Config, | ||
BalanceOf<Runtime>: Into<U256>, | ||
{ | ||
// assert fee > gasLimit * base_fee | ||
let gas_limit_as_u256: U256 = gas_limit.into(); | ||
let (base_fee, _) = <Runtime as pallet_evm::Config>::FeeCalculator::min_gas_price(); | ||
if let Some(gas_limit_times_base_fee) = gas_limit_as_u256.checked_mul(base_fee) { | ||
if gas_limit_times_base_fee >= request_fee.into() { | ||
return Err(revert( | ||
"Gas limit at current price must be less than fees allotted", | ||
)); | ||
} | ||
} else { | ||
return Err(revert("Gas limit times base fee overflowed U256")); | ||
let request_gas_limit_with_overhead = request_gas_limit | ||
.checked_add(subcall_overhead_gas_costs) | ||
.ok_or(revert( | ||
"overflow when computing request gas limit + overhead", | ||
))?; | ||
|
||
// Ensure precompile have enough gas to perform subcall with the overhead. | ||
if remaining_gas < request_gas_limit_with_overhead { | ||
return Err(revert("not enough gas to perform the call")); | ||
} | ||
let log_cost = log_fulfillment_failed(code_address) | ||
.compute_cost() | ||
.map_err(|_| revert("failed to compute log cost"))?; | ||
// Cost of the call itself that the batch precompile must pay. | ||
let call_cost = call_cost(U256::zero(), <Runtime as pallet_evm::Config>::config()); | ||
// assert gasLimit > overhead cost | ||
let overhead = call_cost + log_cost + clean_up_cost; | ||
if gas_limit <= overhead { | ||
return Err(revert("Gas limit must exceed overhead call cost")); | ||
|
||
// Ensure request fee is enough to refund the fulfiller. | ||
let total_refunded_gas = prepare_and_finish_fulfillment_gas_cost | ||
.checked_add(request_gas_limit_with_overhead) | ||
.ok_or(revert("overflow when computed max amount of refunded gas"))? | ||
.checked_add(transaction_gas_refund::<Runtime>()) | ||
.ok_or(revert("overflow when computed max amount of refunded gas"))?; | ||
|
||
let total_refunded_gas: U256 = total_refunded_gas.into(); | ||
let (base_fee, _) = <Runtime as pallet_evm::Config>::FeeCalculator::min_gas_price(); | ||
let execution_max_fee = total_refunded_gas.checked_mul(base_fee).ok_or(revert( | ||
"gas limit (with overhead) * base fee overflowed U256", | ||
))?; | ||
|
||
if execution_max_fee > request_fee.into() { | ||
return Err(revert("request fee cannot pay for execution cost")); | ||
} | ||
|
||
Ok(()) | ||
} | ||
|
||
|
@@ -384,38 +411,65 @@ where | |
) -> EvmResult { | ||
let request_id = request_id.converted(); | ||
|
||
// Since we cannot compute `prepare_and_finish_fulfillment_cost` now (we don't | ||
// know the number of words), we compute the cost for the maximum allowed number of | ||
// words. | ||
let max_prepare_and_finish_fulfillment_cost = | ||
prepare_and_finish_fulfillment_gas_cost::<Runtime>( | ||
<Runtime as pallet_randomness::Config>::MaxRandomWords::get(), | ||
); | ||
|
||
if handle.remaining_gas() < max_prepare_and_finish_fulfillment_cost { | ||
return Err(revert(alloc::format!( | ||
"provided gas must be at least {max_prepare_and_finish_fulfillment_cost}" | ||
))); | ||
} | ||
Comment on lines
+414
to
+426
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So now the fulfill_request caller must overestimate the gas_limit just in case request.num_words = MaxWords:::get()? Will the UI automatically do this or do we need to document is it somewhere There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it must overestimate because it can be this value. If it is under it will revert, thus the gas estimator will increase the gas limit automatically. |
||
|
||
let pallet_randomness::FulfillArgs { | ||
request, | ||
deposit, | ||
randomness, | ||
} = Pallet::<Runtime>::prepare_fulfillment(request_id) | ||
.map_err(|e| revert(alloc::format!("{:?}", e)))?; | ||
|
||
let prepare_and_finish_fulfillment_cost = | ||
prepare_and_finish_fulfillment_gas_cost::<Runtime>(request.num_words); | ||
handle.record_cost(prepare_and_finish_fulfillment_cost)?; | ||
|
||
let subcall_overhead_gas_costs = subcall_overhead_gas_costs::<Runtime>()?; | ||
|
||
// check that randomness can be provided | ||
ensure_can_provide_randomness::<Runtime>( | ||
handle.code_address(), | ||
handle.remaining_gas(), | ||
request.gas_limit, | ||
request.fee, | ||
fulfillment_overhead_gas_cost::<Runtime>(request.num_words), | ||
subcall_overhead_gas_costs, | ||
prepare_and_finish_fulfillment_cost, | ||
)?; | ||
|
||
// get gas before subcall | ||
let before_remaining_gas = handle.remaining_gas(); | ||
// We meter this section to know how much gas was actually used. | ||
// It contains the gas used by the subcall and the overhead actually | ||
// performing a call. It doesn't contain `prepare_and_finish_fulfillment_cost`. | ||
let remaining_gas_before = handle.remaining_gas(); | ||
provide_randomness( | ||
handle, | ||
request_id, | ||
request.gas_limit, | ||
request.contract_address.clone().into(), | ||
randomness.into_iter().map(|x| H256(x)).collect(), | ||
)?; | ||
let remaining_gas_after = handle.remaining_gas(); | ||
|
||
// get gas after subcall | ||
let after_remaining_gas = handle.remaining_gas(); | ||
let gas_used: U256 = before_remaining_gas | ||
.checked_sub(after_remaining_gas) | ||
// We compute the actual gas used to refund the caller. | ||
// It is the metered gas + `prepare_and_finish_fulfillment_cost`. | ||
let gas_used: U256 = remaining_gas_before | ||
.checked_sub(remaining_gas_after) | ||
.ok_or(revert("Before remaining gas < After remaining gas"))? | ||
.checked_add(prepare_and_finish_fulfillment_cost) | ||
.ok_or(revert("overflow when adding real call cost + overhead"))? | ||
.checked_add(transaction_gas_refund::<Runtime>()) | ||
.ok_or(revert("overflow when adding real call cost + overhead"))? | ||
.into(); | ||
// cost of execution is before_remaining_gas less after_remaining_gas | ||
let (base_fee, _) = <Runtime as pallet_evm::Config>::FeeCalculator::min_gas_price(); | ||
let cost_of_execution: BalanceOf<Runtime> = gas_used | ||
.checked_mul(base_fee) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in theory we should include the second parameter (it is the amount of weight for calling
min_gas_price()
itself). we currently use a value of 0 for this, though.