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

Compiler flag to set bytecode size limit #10981

Closed
cameel opened this issue Feb 19, 2021 · 8 comments
Closed

Compiler flag to set bytecode size limit #10981

cameel opened this issue Feb 19, 2021 · 8 comments
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. stale The issue/PR was marked as stale because it has been open for too long.

Comments

@cameel
Copy link
Member

cameel commented Feb 19, 2021

From #2691 (comment)

Custom chains may allow deploying contracts bigger than 24 kB but the limit is currently hard-coded in the compiler and exceeding it triggers a warning that can't be suppressed. In #2691 it was proposed that the solution might be to introduce a compiler flag that allows the user to change the limit to an arbitrary value and avoid the warning.

@axic
Copy link
Member

axic commented Feb 24, 2021

During today's design meeting we have discussed two alternative approaches: #11006/#11008 and #11010. It seems we are leaning towards #11006 and having filtering in frameworks (such as hardhat and truffle).

As a trivia, this warning was added as per discussions on #2100.

@cameel
Copy link
Member Author

cameel commented Feb 24, 2021

I wonder if it might be better to offload this check to frameworks after all. In Hardhat, Truffle and Brownie (and likely most others) you define a list of networks, each with specific settings like gas limit. Hardhat's network settings even have allowUnlimitedContractSize which looks like a flag made specifically to ignore this warning :) These network settings rather than the compiler are the best place to configure something like that in my opinion.

Maybe just removing the warning and coordinating with frameworks to add a check there (with a good default) would be best?

@axic
Copy link
Member

axic commented Feb 24, 2021

I wonder if it might be better to offload this check to frameworks after all.

To be fair the warning was encouraged by hardhat:

I know this could be implemented in each tool (e.g. truffle, remix, embark, buidler, etc), but I think implementing it as a solc warning would be the simplest path to improve the situation for most users.

Perhaps @alcuadrado changed his mind since?

However @cameel, on the other hand I think that frameworks which have a setting like allowUnlimitedContractSize, when set, could also just filter out the warning based on the error code (a la #11006).

@cameel
Copy link
Member Author

cameel commented Feb 24, 2021

True. They could. @gorgos, are using the compiler directly or via a framework?

To be fair the warning was encouraged by hardhat

Yeah, that's what I actually referring to. It was encouraged back then but maybe it would be better to talk it over again given that we're now discovering that we don't really have enough info in the compiler to decide if it's warranted or not (at least with adding extra flags for the user to provide that info).

@gorgos
Copy link
Contributor

gorgos commented Feb 24, 2021

@cameel Via framework, both Truffle and Hardhat.

@cameel
Copy link
Member Author

cameel commented Jun 14, 2021

Looks like we'll probably address this problem in a different way: #11508.

@cameel cameel added low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. labels Sep 27, 2022
@github-actions
Copy link

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 17, 2023
@github-actions
Copy link

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Mar 25, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. stale The issue/PR was marked as stale because it has been open for too long.
Projects
None yet
Development

No branches or pull requests

4 participants