Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Hard-cap the execution time of custom contract calls. #4226

Merged
merged 3 commits into from
Nov 27, 2019
Merged

Conversation

tomusdrw
Copy link
Contributor

To prevent RPC spam, make sure we disallow contract queries that may take forever to complete. The limit is now based on the assumption that:

  1. Gas cost is roughly equivalent to weight (CC @pepyakin to confirm)
  2. One weight is roughly one nano second (seems like something that is assumed in linked W3F doc).

Current default block weight limit is 1e9 as well, so this limit can be interpreted as either:

  1. Up to 5s of native execution.
  2. Or 5x max block size.

I'd rather be conservative here - if this ever creates issues we can revisit later.

@tomusdrw tomusdrw added A0-please_review Pull request needs code review. M6-rpcapi labels Nov 27, 2019
/// prevent blocking the RPC for too long.
///
/// Based on W3F research spreadsheet:
/// https://docs.google.com/spreadsheets/d/1h0RqncdqiWI4KgxO0z9JIpZEJESXjX_ZCK6LFX6veDo/view
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have permissions to view this, so I'm guessing others won't either. Who do we talk to about making this public?

@@ -148,6 +157,19 @@ where
data: None,
})?;

let max_gas_limit = 5 * GAS_PER_SECOND;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add the "interpretation" you put in the PR description as a comment here

@HCastano
Copy link
Contributor

Some nits, but looks good

Copy link
Contributor

@pepyakin pepyakin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The weight=gas PR hasn't landed yet, but still the limit appears to be fine even for the current one.

@gavofyork gavofyork merged commit a3fb0fd into master Nov 27, 2019
@gavofyork gavofyork deleted the td-hard-cap branch November 27, 2019 18:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants