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

Remove dependency on solidity-stringutils #91

Merged
merged 11 commits into from
Jan 27, 2025

Conversation

ericglau
Copy link
Member

  • Remove dependency on solidity-stringutils. Instead, uses a combination of OpenZeppelin's Strings library, Forge's string cheatcodes, and an internal library for additional string finder functions.
  • Sets the assembly block in Core._deployFromBytecode as /// @solidity memory-safe-assembly. This annotation is used instead of assembly ("memory-safe") so that it remains compatible with versions of Solidity used in both OpenZeppelin Contracts v4 and v5.

Fixes #89

@ericglau ericglau requested a review from a team January 14, 2025 23:03
Copy link

@Amxx Amxx left a comment

Choose a reason for hiding this comment

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

I'm thinking that there may be stuff from solidity-stringutils that we want to put in our String.sol

@ericglau ericglau requested a review from Amxx January 24, 2025 22:32
@ericglau ericglau merged commit cbce1e0 into OpenZeppelin:main Jan 27, 2025
4 checks passed
@ericglau ericglau deleted the removestringutils branch January 27, 2025 18:20
@tomw1808
Copy link

I must be missing something, I get:

[⠊] Compiling...
[⠑] Compiling 99 files with Solc 0.8.26
[⠘] Solc 0.8.26 finished in 666.39ms
Error: Compiler run failed:
Error (9582): Member "contains" not found or not visible after argument-dependent lookup in contract Vm.
  --> lib/openzeppelin-foundry-upgrades/src/internal/StringFinder.sol:17:16:
   |
17 |         return vm.contains(subject, search);
   |                ^^^^^^^^^^^
❯ forge --version
forge Version: 1.0.0-nightly
Commit SHA: 4974a0891bf01511993f49138ebf81f4be207cdf
Build Timestamp: 2025-02-25T09:01:16.709526000Z (1740474076)
Build Profile: maxperf

main branch from openzeppelin-foundry-upgrades
release-v5.3 branch from both openzeppelin-contracts and openzeppelin-contracts-upgradeable

foundry.toml

[profile.default]
src = "src"
out = "out"
libs = ["lib"]

# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options

optimizer = true
optimizer_runs = 20_000

# Required for OpenZeppelin Foundry Upgrades
ffi = true
ast = true
build_info = true
extra_output = ["storageLayout"]

# Allow reading the deployments directory
fs_permissions = [{ access = "read-write", path = "./deployments" }]

Can you let me know what I am missing?

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.

solidity-stringutils dependency can cause problems with compilation
3 participants