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

Replace solc dependency with a thin solidity_compile wrapper #6073

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

galargh
Copy link
Member

@galargh galargh commented Dec 23, 2024

  • Because this PR includes a bug fix, relevant tests have been included.
  • Because this PR includes a new feature, the change was previously discussed on an Issue or with someone from the team.
  • I didn't do anything of this.

Resolves #6071

This is a follow-up to #6056

In this PR, I replace the solc-js dependency with a thin wrapper over the solidity_compile method.

It was created by extracting the parts of the solc-js package (https://github.com/ethereum/solc-js) that we were actually using.

Copy link

vercel bot commented Dec 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ❌ Failed (Inspect) Jan 3, 2025 11:16am

Copy link

changeset-bot bot commented Dec 23, 2024

⚠️ No Changeset found

Latest commit: 7652f97

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

@github-actions github-actions bot added the status:ready This issue is ready to be worked on label Dec 23, 2024
Copy link
Contributor

hardhat

Total size of the bundle: 219M
Total number of dependencies (including transitive): 53

List of dependencies (sorted by size)
214M	total
30M	@ignored/edr-optimism-linux-x64-musl
30M	@ignored/edr-optimism-linux-x64-gnu
27M	@ignored/edr-optimism-linux-arm64-musl
27M	@ignored/edr-optimism-linux-arm64-gnu
22M	@ignored/edr-optimism-win32-x64-msvc
21M	@ignored/edr-optimism-darwin-x64
20M	esbuild
20M	@ignored/edr-optimism-darwin-arm64
2.8M	@sentry/tracing
2.4M	micro-eth-signer
1.9M	@noble/curves
1.7M	undici
1.2M	@sentry/types
1.2M	@noble/hashes
932K	@sentry/node
920K	@sentry/utils
856K	zod
844K	@ignored/hardhat-vnext-utils
616K	micro-packed
576K	tsx
548K	@sentry/core
504K	fast-equals
492K	@scure/bip39
460K	@ignored/edr
384K	@ignored/edr-optimism
368K	ethereum-cryptography
344K	@sentry/hub
320K	enquirer
320K	@ignored/hardhat-vnext-errors
284K	semver
192K	ws
168K	@scure/base
136K	adm-zip
128K	get-tsconfig
96K	@scure/bip32
92K	chalk
88K	tslib
88K	@sentry/minimal
76K	agent-base
72K	@nomicfoundation/solidity-analyzer
68K	debug
64K	lru_map
64K	https-proxy-agent
60K	@ignored/hardhat-vnext-zod-utils
56K	rfdc
48K	ansi-colors
40K	resolve-pkg-maps
36K	p-map
32K	cookie
24K	strip-ansi
24K	env-paths
24K	ansi-regex
20K	ms

type Compile = (
input: string,
callbackPtr: number | null,
callbackContextPtr?: null,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an odd type, but we either want to skip this argument or provide an explicit null. That depends on the version of solc that we're going to be using.

Comment on lines 59 to 62
const output = isVersion6OrNewer
? compile(input, null, null)
: compile(input, null);
Copy link
Member Author

Choose a reason for hiding this comment

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

The underlying compile functions expect explicit values as far as I understand. Also, their arguments differ depending on the version being used.

}

function versionToSemver(version: string): string {
// FIXME: parse more detail, but this is a good start
Copy link
Member Author

Choose a reason for hiding this comment

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

The implementation of this function is a direct copy from solc-js.

@galargh galargh marked this pull request as ready for review December 24, 2024 13:36
@schaable schaable added the v-next A Hardhat v3 development task label Dec 30, 2024
Copy link
Member

@schaable schaable left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd wait for @alcuadrado's review on this one since he's more familiar with solc

Base automatically changed from robust-compiler-downloader to v-next January 3, 2025 10:57
@galargh galargh added this pull request to the merge queue Jan 3, 2025
Merged via the queue into v-next with commit 217c35f Jan 3, 2025
69 of 70 checks passed
@galargh galargh deleted the solcjs-compile-wrapper branch January 3, 2025 11:24
@galargh
Copy link
Member Author

galargh commented Jan 3, 2025

LGTM, but I'd wait for @alcuadrado's review on this one since he's more familiar with solc

I've only just realised I merged the solc replacement as part of #6056. Now, I also merged this one because after merging the other one, this one ended up consisting only of the changes requested in the review here.

@alcuadrado I put together a branch on which I revert the solc wrapper changes altogether in case we want to roll them back for now: v-next...revert-solcjs-compile-wrapper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready This issue is ready to be worked on v-next A Hardhat v3 development task
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants