Support Settings (a.k.a. Input JSON Description) in @nomiclabs/hardhat-vyper #4872

ChristopherDedominici · 2024-02-15T16:46:13Z

Added properties evmVersion and optimize.
Follow this issue to track additional settings properties that we might consider adding.

changeset-bot · 2024-02-15T16:46:17Z

🦋 Changeset detected

Latest commit: 598acb2

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

This PR includes changesets to release 1 package

Name	Type
@nomiclabs/hardhat-vyper	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

vercel · 2024-02-15T16:46:19Z

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

Name	Status	Preview	Comments	Updated (UTC)
hardhat	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 8, 2024 8:27pm

…omicFoundation/hardhat into feature/4566-support-settings-in-vyper

packages/hardhat-vyper/README.md

packages/hardhat-vyper/test/tests.ts

packages/hardhat-vyper/src/compiler.ts

packages/hardhat-vyper/src/index.ts

schaable

LGTM

packages/hardhat-vyper/src/compiler.ts

packages/hardhat-vyper/src/index.ts

packages/hardhat-vyper/src/types.ts

fvictorio · 2024-04-02T09:36:12Z

I pushed a commit using VyperPluginError instead of plain errors. This is necessary to get proper error output in the CLI.

Before:

After:

fvictorio · 2024-04-02T09:45:42Z

I'm trying several combinations of versions and settings. This is what I got:

0.3.0
- optimize: true: throws a proper error ✅
- optimize: false: compiler error ❌
- optimize: "gas": throws a proper error ✅
0.3.1
- optimize: true: works ✅
- optimize: false: works ✅
- optimize: "gas": throws a proper error ✅
0.3.10
- optimize: true: throws a proper error ✅
- optimize: false: works ✅
- optimize: "gas": works ✅

So mostly good. We should just fix that combination that produces a compiler error.

…e/4566-support-settings-in-vyper

fvictorio

LGTM. Just some minor comments. I'll let @kanej decide if he wants to review it or if @schaable's previous code review is enough (my guess is that it is).

fvictorio · 2024-04-05T08:34:49Z

packages/hardhat-vyper/test/tests.ts

+      );
+      useEnvironment();
+
+      it("should compile successfully", async function () {


Suggested change

it("should compile successfully", async function () {

it("should fail to compile", async function () {

There are other test descriptions below like this one that don't match what the test does.

fvictorio · 2024-04-05T08:57:07Z

packages/hardhat-vyper/src/compiler.ts

+        semver.lt(compilerVersion, "0.3.1")
+      ) {
+        throw new VyperPluginError(
+          `The 'optimize' setting with value 'true' is not supported for versions of the Vyper compiler older than 0.3.1 or newer than 0.3.10. You are currently using version ${compilerVersion}.`


This message has an off-by-one error, right? You should either say newer than 0.3.9 or newer than or equal to 0.3.10. (I prefer the former.)

packages/hardhat-vyper/src/compiler.ts

schaable · 2024-04-05T16:21:02Z

packages/hardhat-vyper/src/compiler.ts

+        semver.lte(compilerVersion, "0.3.0")
+      ) {
+        throw new VyperPluginError(
+          `The 'optimize' setting with value 'true' is not supported for versions of the Vyper compiler older than or equal to 0.3.0 or newer than or equal to 0.3.10. You are currently using version ${compilerVersion}.`


Maybe it's just me, but I find that error message a bit too verbose, how about something like: "The 'optimize' setting with value 'true' is supported only for Vyper versions >0.3.0 and <0.3.10. You are currently using version ${compilerVersion}."
Feel free to ignore it if you think the current message is better 😁

(same for the rest of the error messages)

packages/hardhat-vyper/src/compiler.ts

schaable

Included a few minor suggestions, but none are merge blockers.

kanej · 2024-04-08T13:57:26Z

packages/hardhat-vyper/src/compiler.ts

@@ -7,10 +10,18 @@ export class Compiler {
   *
   * @param inputPaths array of paths to contracts to be compiled


We should add the new parameters.

kanej

I had one small comment on some missing param docs, but pre-approving. Looks good!

ChristopherDedominici added 4 commits February 14, 2024 23:08

add settings in the compiler options

89b5571

fix readme

e09dfad

fix test

0bbfd00

fix VyperSettings type

5f078bc

ChristopherDedominici self-assigned this Feb 15, 2024

github-actions bot added the status:ready This issue is ready to be worked on label Feb 15, 2024

ChristopherDedominici linked an issue Feb 15, 2024 that may be closed by this pull request

Support Settings (a.k.a. Input JSON Description) in @nomiclabs/hardhat-vyper #4566

Closed

vercel bot deployed to Preview February 15, 2024 16:47 View deployment

ChristopherDedominici mentioned this pull request Feb 15, 2024

Support all the vyper settings (a.k.a. Input JSON Description) in @nomiclabs/hardhat-vyper #4873

Open

ChristopherDedominici requested review from fvictorio and schaable February 15, 2024 16:56

ChristopherDedominici added 2 commits February 15, 2024 17:57

Create fluffy-cups-clap.md

bb9a402

Merge branch 'feature/4566-support-settings-in-vyper' of github.com:N…

593d897

…omicFoundation/hardhat into feature/4566-support-settings-in-vyper

vercel bot deployed to Preview February 15, 2024 16:58 View deployment

schaable reviewed Feb 15, 2024

View reviewed changes

packages/hardhat-vyper/README.md Outdated Show resolved Hide resolved

schaable reviewed Feb 15, 2024

View reviewed changes

packages/hardhat-vyper/test/tests.ts Outdated Show resolved Hide resolved

vercel bot deployed to Preview February 15, 2024 18:51 View deployment

schaable reviewed Feb 15, 2024

View reviewed changes

packages/hardhat-vyper/src/compiler.ts Outdated Show resolved Hide resolved

schaable reviewed Feb 15, 2024

View reviewed changes

packages/hardhat-vyper/src/index.ts Outdated Show resolved Hide resolved

schaable reviewed Feb 15, 2024

View reviewed changes

packages/hardhat-vyper/src/index.ts Outdated Show resolved Hide resolved

vercel bot deployed to Preview February 15, 2024 18:59 View deployment

schaable approved these changes Feb 15, 2024

View reviewed changes

vercel bot deployed to Preview February 15, 2024 19:17 View deployment

fvictorio requested changes Feb 16, 2024

View reviewed changes

packages/hardhat-vyper/src/compiler.ts Outdated Show resolved Hide resolved

packages/hardhat-vyper/src/index.ts Outdated Show resolved Hide resolved

packages/hardhat-vyper/src/types.ts Outdated Show resolved Hide resolved

improve handling of optimize setting

e7ddc3e

vercel bot deployed to Preview February 19, 2024 11:45 View deployment

ChristopherDedominici requested a review from fvictorio February 20, 2024 14:06

Use VyperPluginError instead of Error

da8b0ec

vercel bot deployed to Preview April 2, 2024 09:27 View deployment

ChristopherDedominici added 2 commits April 2, 2024 19:39

Merge branch 'main' of github.com:NomicFoundation/hardhat into featur…

de57883

…e/4566-support-settings-in-vyper

fix optimize flag for versions < 0.3.1

87e2516

vercel bot deployed to Preview April 2, 2024 19:19 View deployment

fvictorio reviewed Apr 5, 2024

View reviewed changes

small PR's comments fixes

d466a56

ChristopherDedominici requested review from schaable and kanej April 5, 2024 13:54

vercel bot deployed to Preview April 5, 2024 13:54 View deployment

schaable reviewed Apr 5, 2024

View reviewed changes

packages/hardhat-vyper/src/compiler.ts Outdated Show resolved Hide resolved

schaable reviewed Apr 5, 2024

View reviewed changes

packages/hardhat-vyper/src/compiler.ts Outdated Show resolved Hide resolved

schaable approved these changes Apr 5, 2024

View reviewed changes

smaller fixes based on PR's comments

2f1dafe

vercel bot deployed to Preview April 8, 2024 10:02 View deployment

fvictorio approved these changes Apr 8, 2024

View reviewed changes

increase mocha timeout

b85ff41

vercel bot deployed to Preview April 8, 2024 13:52 View deployment

kanej reviewed Apr 8, 2024

View reviewed changes

kanej approved these changes Apr 8, 2024

View reviewed changes

add params in doc

598acb2

vercel bot deployed to Preview April 8, 2024 20:27 View deployment

ChristopherDedominici merged commit f0e6389 into main Apr 8, 2024
32 checks passed

ChristopherDedominici deleted the feature/4566-support-settings-in-vyper branch April 8, 2024 21:15

github-actions bot locked as resolved and limited conversation to collaborators Jul 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Settings (a.k.a. Input JSON Description) in @nomiclabs/hardhat-vyper #4872

Support Settings (a.k.a. Input JSON Description) in @nomiclabs/hardhat-vyper #4872

ChristopherDedominici commented Feb 15, 2024 •

edited

Loading

changeset-bot bot commented Feb 15, 2024 •

edited

Loading

vercel bot commented Feb 15, 2024 •

edited

Loading

schaable left a comment

fvictorio commented Apr 2, 2024

fvictorio commented Apr 2, 2024

fvictorio left a comment

fvictorio Apr 5, 2024

fvictorio Apr 5, 2024

fvictorio Apr 5, 2024

schaable Apr 5, 2024

schaable Apr 5, 2024

schaable left a comment

kanej Apr 8, 2024

kanej left a comment

	it("should compile successfully", async function () {
	it("should fail to compile", async function () {

		@@ -7,10 +10,18 @@ export class Compiler {
		*
		* @param inputPaths array of paths to contracts to be compiled

Support Settings (a.k.a. Input JSON Description) in @nomiclabs/hardhat-vyper #4872

Support Settings (a.k.a. Input JSON Description) in @nomiclabs/hardhat-vyper #4872

Conversation

ChristopherDedominici commented Feb 15, 2024 • edited Loading

changeset-bot bot commented Feb 15, 2024 • edited Loading

🦋 Changeset detected

vercel bot commented Feb 15, 2024 • edited Loading

schaable left a comment

Choose a reason for hiding this comment

fvictorio commented Apr 2, 2024

fvictorio commented Apr 2, 2024

fvictorio left a comment

Choose a reason for hiding this comment

fvictorio Apr 5, 2024

Choose a reason for hiding this comment

fvictorio Apr 5, 2024

Choose a reason for hiding this comment

fvictorio Apr 5, 2024

Choose a reason for hiding this comment

schaable Apr 5, 2024

Choose a reason for hiding this comment

schaable Apr 5, 2024

Choose a reason for hiding this comment

schaable left a comment

Choose a reason for hiding this comment

kanej Apr 8, 2024

Choose a reason for hiding this comment

kanej left a comment

Choose a reason for hiding this comment

ChristopherDedominici commented Feb 15, 2024 •

edited

Loading

changeset-bot bot commented Feb 15, 2024 •

edited

Loading

vercel bot commented Feb 15, 2024 •

edited

Loading