Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Increase maxBuffer to 50MB when compiling using native compiler #6008

Merged
merged 1 commit into from
Apr 27, 2023

Conversation

d10r
Copy link
Contributor

@d10r d10r commented Apr 25, 2023

PR description

Same issue as already tackled in #3002, but which keeps getting up for more complex projects. Also see #4499

Testing instructions

I only can tell how to reproduce the issue before the change:

git clone https://github.com/superfluid-finance/protocol-monorepo
cd protocol-monorepo
yarn install
cd packages/ethereum-contracts
sed -i 's/version: "0.8.19",/version: "native",/g' truffle-config.js
npx truffle compile

You need to have solc version 0.8.19 in PATH.

@hellwolf
Copy link

yes please.

Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

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

Seems like this makes sense! Here's one approval of two (and I'll add a few reviewers)

@gnidan
Copy link
Contributor

gnidan commented Apr 27, 2023

(Thanks for this, by the way! Appreciate the digging :)

Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

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

Thanks @d10r! Since @gnidan's ok with this, I'll throw another approval with a caveat.

This solution doesn't solve the general case as there could be another complex contract requiring even more memory! An alternative fix would be to use spawn and collect the child's output via its event system to bypass buffer memory constraints. I'll open an PR issue for it.

Edit: s/PR/issue/

@@ -6,7 +6,7 @@ export class Native {
load() {
const versionString = this.validateAndGetSolcVersion();
const command = "solc --standard-json";
const maxBuffer = 1024 * 1024 * 10;
const maxBuffer = 1024 * 1024 * 50;
Copy link
Member

Choose a reason for hiding this comment

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

we can has more memory!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants