Skip to content

feat(ctb): Remove division from hasMinGas for precision#5540

Merged
OptimismBot merged 1 commit intodevelopfrom
clabby/ctb/safe-call-rounding
Apr 26, 2023
Merged

feat(ctb): Remove division from hasMinGas for precision#5540
OptimismBot merged 1 commit intodevelopfrom
clabby/ctb/safe-call-rounding

Conversation

@clabby
Copy link
Contributor

@clabby clabby commented Apr 26, 2023

Overview

Removes the division from hasMinGas for extra precision.

Old

$$ g \geq \text{minGas} \times \frac{64}{63} + 40,000 + \text{reservedGas} $$

New

$$ g \times 63 \geq \text{minGas} \times 64 + 63(40,000 + \text{reservedGas}) $$

Metadata
Fixes CLI-3892

@changeset-bot
Copy link

changeset-bot bot commented Apr 26, 2023

🦋 Changeset detected

Latest commit: 6eb0543

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@eth-optimism/contracts-bedrock Minor
@eth-optimism/actor-tests Patch
@eth-optimism/sdk Patch
@eth-optimism/chain-mon Patch
@eth-optimism/message-relayer Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Apr 26, 2023

Deploy Preview for opstack-docs canceled.

Name Link
🔨 Latest commit 6eb0543
🔍 Latest deploy log https://app.netlify.com/sites/opstack-docs/deploys/644942b310169c0008feb3b2

@clabby
Copy link
Contributor Author

clabby commented Apr 26, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@mergify
Copy link
Contributor

mergify bot commented Apr 26, 2023

Hey @clabby! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Apr 26, 2023
@tynes
Copy link
Contributor

tynes commented Apr 26, 2023

How do you feel about putting the formula into the contract? Would help future reviewers understand.

@tynes
Copy link
Contributor

tynes commented Apr 26, 2023

Going to need to rebase + update semver for portal + both messengers

Add changeset

Semver bump

Resolve conflicts
@clabby clabby force-pushed the clabby/ctb/safe-call-rounding branch from 9808498 to 6eb0543 Compare April 26, 2023 15:26
@mergify mergify bot removed the conflict label Apr 26, 2023
@clabby clabby marked this pull request as ready for review April 26, 2023 15:27
@clabby clabby requested a review from a team as a code owner April 26, 2023 15:27
@clabby clabby requested a review from tynes April 26, 2023 15:27
@OptimismBot OptimismBot merged commit 4e97fad into develop Apr 26, 2023
@OptimismBot OptimismBot deleted the clabby/ctb/safe-call-rounding branch April 26, 2023 16:58
@mergify
Copy link
Contributor

mergify bot commented Apr 26, 2023

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Apr 26, 2023

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot removed the on-merge-train label Apr 26, 2023
seolaoh added a commit to kroma-network/kroma that referenced this pull request Aug 9, 2023
…Gas`

Adjusts `SafeCall.callWithMinGas` to account for the worst-case cost of 3/5 of
the `CALL` opcode's dynamic gas factors:
  - `address_access_cost`
  - `positive_value_cost`
  - `value_to_empty_account_cost`
As for `memory_expansion_cost` & `code_execution_cost`, these are expected to be
covered by the `minGasLimit` itself.
Also removes the division from `hasMinGas` for extra precision.

See:
  - ethereum-optimism/optimism#5470
  - ethereum-optimism/optimism#5540
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants