Skip to content

Restrict visibilities to external#747

Merged
maurelian merged 1 commit intomasterfrom
chore/externalVisibility
May 4, 2021
Merged

Restrict visibilities to external#747
maurelian merged 1 commit intomasterfrom
chore/externalVisibility

Conversation

@maurelian
Copy link
Copy Markdown
Contributor

If a function does not need to be accessed from within the contract, it should be labelled external because:

  1. The use of the function is made semantically clearer to reviewers
  2. It should save gas, though the optimizer may address that (but fewer optimizer changes is also a benefit).

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented May 4, 2021

⚠️ No Changeset found

Latest commit: bacf06e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@maurelian
Copy link
Copy Markdown
Contributor Author

Shouldn't require a changeset, does not change functionality.

@maurelian maurelian merged commit ae1ac05 into master May 4, 2021
@maurelian maurelian deleted the chore/externalVisibility branch May 4, 2021 18:59
OptimismBot pushed a commit that referenced this pull request Dec 10, 2025
* refactor: add helper function to match instruction by key only

* chore: pre-pr ready

* test: refactor key matching test

* test: add test suite for IsMatchingKey helper function

* chore: pre-pr ready
theochap pushed a commit that referenced this pull request Dec 10, 2025
* chore(ci): Don't run `online` tests

* no online tests
github-merge-queue bot pushed a commit that referenced this pull request Dec 11, 2025
* feat: add cgt to opcmv2

* feat: add cgt to opcmv2

* fix: change semver

* feat: add cgt to opcmv2

* feat: add cgt to opcmv2

* fix: change semver

* refactor: change disputeGameConfigs order in _loadFullConfig

* fix: comment slash rules

* feat: add opcmv2-cgt matrix in CI

* fix: enable cgt overrides on OPCMv2 (#724)

* fix: check CGT is not enabled before enabling it

* fix: enable CGT overrides

* fix: add extra instruction overrides for useCustomGasToken on OPCMv2 (#725)

* chore: link TODO to issue (#727)

* fix: opcm2 revert upgrade cgt (#728)

* fix: add check enabled CGT during upgrades

* test: set CGT false by default during forked tests

* chore: expand comment on OPCMv2 regarding CGT

* chore: pre-pr ready

* refactor: remove unnecessary check for CGT feature (#735)

* fix: cgt upgrade in opcm v2 and semver (#738)

* fix: Full checkout for FFI build (#18527)

* chore(op-acceptance-tests): delete very old logs (#18529)

* chore(op-acceptance-tests): op-acceptor v3.8.0 (#18530)

* jovian: remove feature toggles (#17978)

* jovian: remove feature toggles

scope is now fixed

* Updated op-geth to v1.101604.0-synctest.0.0.20251120150812-e50f80a16afc

* lint

* just update-op-geth f48f382

* Use Jovian in test error message

* just update-op-geth ba6bdcfef42341fe2b5ce124c31ff2d6b264e9e4

* chore(ai-eng): add ReinitializableBase test to exclusion list (#18531)

* feat: have OPCM upgrade allowances be upgrade specific (#18462)

* feat: have OPCM upgrade allowances be upgrade specific

Updates the OPCMv2 check for allowed extra instructions to be
specific to releases. When release versions are bumped, the
allowances become automatically invalid and would reveal
anywhere in the codebase where the allowance is being used.

* fix: semver lock

* fix: emit instruction key in error

* fix: better semantics

* docs: proper OPCM versioning policy

* fix: final version tweaks

* fix(op-acceptance-test): flake-shake; empty slack notifications. (#18542)

* feat: add cgt to opcmv2

* feat: add cgt to opcmv2

* fix: change semver

* feat: add cgt to opcmv2

* feat: add cgt to opcmv2

* fix: change semver

* refactor: change disputeGameConfigs order in _loadFullConfig

* fix: enable cgt overrides on OPCMv2 (#724)

* fix: check CGT is not enabled before enabling it

* fix: enable CGT overrides

* fix: opcm2 revert upgrade cgt (#728)

* fix: add check enabled CGT during upgrades

* test: set CGT false by default during forked tests

* chore: expand comment on OPCMv2 regarding CGT

* fix: cgt upgrade in opcm v2 and semver (#738)

* fix: merge conflicts

* fix: pre-pr

* fix: add missing isMatchingInstruction for cgt in false

* refactor: opcm2 extra instruction keymatch (#747)

* refactor: add helper function to match instruction by key only

* chore: pre-pr ready

* test: refactor key matching test

* test: add test suite for IsMatchingKey helper function

* chore: pre-pr ready

* fix: opcmv2 semver

---------

Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com>
Co-authored-by: Ján Jakub Naništa <jan.jakub.nanista@gmail.com>
Co-authored-by: Stefano Charissis <stefano@oplabs.co>
Co-authored-by: George Knee <georgeknee@googlemail.com>
Co-authored-by: Ariel Diaz <65925295+aliersh@users.noreply.github.com>
Co-authored-by: smartcontracts <14298799+smartcontracts@users.noreply.github.com>
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.

2 participants