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

Fix duplicate SPDX and ABIEncoderV2 lines #61

Closed
wants to merge 3 commits into from

Conversation

zemse
Copy link

@zemse zemse commented Sep 22, 2020

A quick fix for #48 and #55. This fixed errors on my flattened contract that had a lot of SPDX's and a couple of ABIEncoderV2's.

Before:

// SPDX-License-Identifier: MIT
// SPDX-License-Identifier: MIT
// SPDX-License-Identifier: GNU

pragma experimental ABIEncoderV2;
pragma experimental ABIEncoderV2;

After:

// SPDX-License-Identifier: MIT AND GNU

pragma experimental ABIEncoderV2;

Please let me know if any changes are required!

Edit: If anyone wants to try if this PR fixes errors in your flattened contract, you can do so with the temporary package npm install @zemse/solidity-flattener -g.

This commit combines the licenses in multiple SPDX declarations as also
reported in NomicFoundation#55.
This commit removes duplicate ABIEncoderV2 pragma declarations
from the output file as also reported in NomicFoundation#48
@7--
Copy link

7-- commented Oct 15, 2020

Would be nice if this was reviewed and merged. // SPDX-License-Identifier always gives me issues.

@alcuadrado
Copy link
Member

I'm really hesitant about merging this, as I suspect it leads to violating the licenses most of the time. Maybe if the identifiers aren't move around?

@zemse
Copy link
Author

zemse commented Oct 16, 2020

I suspect it leads to violating the licenses most of the time.

I see what you mean. This would be very inappropriate.

I think only allowing flattening when all files have the same licenses should be fine? And throw if mixed licenses are there as commented in #55 (comment). I feel what best could be done in case of mixed license is Axic's second point in ethereum/solidity#8989 (comment), i.e. the least permissive could be picked, but I doubt if all kinds of licenses listed on https://spdx.org/licenses/ can be really arranged in an order of precedence. I'm not sure if there is any solution for mixed licenses, but for same licences is not a problem.

So to sum up, only files with same licenses would be flattened, mixed licenses would throw. Could such be acceptable?

@7--
Copy link

7-- commented Oct 16, 2020

Maybe make it a flag so users have to ask for is specifically like truffle-flattener file.sol --flatten-licenses.

Or maybe throw a warning and a disclaimer if multiple licenses are detected and ask if they want licenses to be flattened.

Truffle-flattener shouldn't be liable if the user specifically requests it. User will remove manually if they want to. It's a lot of manual work to deploy and verify on etherscan without truffle flattener so I think it's appropriate.

@alcuadrado alcuadrado closed this Feb 12, 2022
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.

4 participants