Deduplicate or remove SPDX license identifier comments

[frangio]

Solidity 0.6.8 introduced // SPDX-License-Identifier comments that will now become quite common because their omission produces a compiler warning. These comments cannot be duplicated or they will produce a compiler error. They need to be combined into a single comment. This is easy if all comments are exactly the same (although hopefully the compiler can be changed so that this is no longer an error...) but it's not clear what should be done when there are multiple licenses across the file. I've asked in the Solidity repository for guidance. ethereum/solidity#8989

[frangio]

The suggestion by the Solidity team is to remove the comments entirely from the flattened file. I think this is easy and reasonable. In fact, I don't think deduplicating would be a good idea because even if all of the license identifiers that are found are the same, there may be unlicensed files and we can't assume that the same license applies to them.

truffle-flattener should emit a warning if it finds these statements and removes them, and it could have an optional command line flag to specify a license for the output file, so that the user doesn't have to edit the flattened file through other means.

[abcoathup]

I don't think any tool such as truffle-flattener should by default remove license identifiers.
This could be in violation of the terms of some licenses.

My preference would be an error if there are a mix of license identifiers.
A compromise could be a --force flag to forcibly remove license identifiers in the flattened output. Though I would suggest that a warning is shown that this action could be in violation of the terms of some licenses.

For verification on block explorers we need a tool (verification-copy?) that copies all of the contracts that a contract imports to a single temporary directory so that they can be verified easily using a block explorers multi file verification.

I have posted about SPDX license identifiers in the OpenZeppelin Community Forum: https://forum.openzeppelin.com/t/solidity-0-6-8-introduces-spdx-license-identifiers/2859

[frangio]

My preference would be an error if there are a mix of license identifiers.

The error should also be emitted when some of the files don't have any license identifier at all.

[k06a]

@frangio removing comments entirely is not a good idea, since some devs use flatten version to upload to etherscan.

[frangio]

@k06a Do you have a preference for any of the other strategies mentioned?

[k06a]

@frangio Let's remove all the SPDX comments and specify set of licenses over AND:

// SPDX-License-Identifier MIT AND GPL-3.0-only

[yudilevi]

I tend to agree with the single comment / multiple licenses approach

[k06a]

I use the following script to keep only first entries of license and ABIEncoderV2:

truffle-flattener ./contracts/OneRouter.sol | awk '/SPDX-License-Identifier/&&c++>0 {next} 1' | awk '/pragma experimental ABIEncoderV2;/&&c++>0 {next} 1' > ./OneRouter.full.sol

[sambacha]

using SPDX's conjunctive AND operator shouldn't be done. it should either return (as an example): [ 'MIT', 'Apache-2.0', 'GPL-3.0'] or MultipleLicenses as the SPDX-License-Identifier: and then a secondary comment with all licenses used in the compiled output or an index of licensed where applied to contracts.

AND, WITH, and OR have special meaning in SPDX, and downstream products use these terms in determining license requirements.

[rkalis]

What I did in v0.4 of truffle-plugin-verify (before moving to Standard Input JSON) was output an error message when the flattened output contained >1 SPDX identifier. The user could then use a flag --license <SPDX Identifier> to specifically and consciously choose one license which then overrides all other SPDX identifiers (and removes all existing identifiers). You could even call the flag --force-license to convey put an emphasis on the fact that they need to be sure of what they're doing.

[alcuadrado]

What about modifying the comments a bit so that solidity ignores them, but we keep the lincense of each file?

For example, we could turn// SPDX-License-Identifier into // -- SPDX-License-Identifier

[krasi-georgiev]

+1 against removing all comments.

I have things like

// solhint-disable-next-line no-empty-blocks

which control the behavior for solhint and slither.

[jiaochangyang]

Is there a plan to implement the modification of the SPDX-License comments?

[Thanushpen]

Iam getting this error anyone can help me please

Compiler debug log:
Error! Unable to generate Contract ByteCode and ABI
Found the following ContractName(s) in source code : Context, Counters, DigitalInternetMoney, ECDSA, EIP712, ERC20, ERC20FlashMint, ERC20Permit, ERC20Votes, IERC20, IERC20Metadata, IERC20Permit, IERC3156FlashBorrower, IERC3156FlashLender, Math, MyContract, Ownable, SafeCast
But we were unable to locate a matching bytecode (err_code_2)
For troubleshooting, you can try compiling your source code with the Remix - Solidity IDE and check for exceptions

Compiler Warning(s):

Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: " to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
--> myc

Compiler Version: v0.8.7+commit.e28d00a7
Optimization Enabled: True
Runs: 200
ByteCode (what we are looking for):
6101406040527f6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9610120908152503480156200003a57600080fd5b506040518060400160405280601481526020017f4469676974616c496e7465726e65744d6f6e6579000000000000000000000000815250806040518060400160405280600181526020017f31000000000000000000000000000000000000000000000000000000000000008152506040518060400160405280601481526020017f4469676974616c496e7465726e65744d6f6e65790000000000000000000000008152506040518060400160405280600381526020017f44494d000000000000000000000000000000000000000000000000000000000081525081600390805190602001906200012c92919062000c6b565b5080600490805190602001906200014592919062000c6b565b505050620001686200015c6200023060201b60201c565b6200023860201b60201c565b60008280519060200120905060008280519060200120905060007f8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f90508260c081815250508160e081815250504660a08181525050620001d0818484620002fe60201b60201c565b608081815250508061010081815250505050505050506200022a33620001fb6200033a60201b60201c565b601262000209919062000fe2565b67016345785d8a00006200021e91906200111f565b6200034360201b60201c565b620013f6565b600033905090565b6000600560009054906101000a900473ffffffffffffffffffffffffffffffffffffffff16905081600560006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055508173ffffffffffffffffffffffffffffffffffffffff168173ffffffffffffffffffffffffffffffffffffffff167f8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e060405160405180910390a35050565b600083838346306040516020016200031b95949392919062000dea565b6040516020818303038152906040528051906020012090509392505050565b60006012905090565b6200035a82826200035e60201b620016751760201c565b5050565b6200037582826200041c60201b620017021760201c565b620003856200059560201b60201c565b7bffffffffffffffffffffffffffffffffffffffffffffffffffffffff16620003b3620005b960201b

[alcuadrado]

Closing to clean up the repository. If this problem persists, please open a new issue with a clear explanation.

[SoftwareAngels]

Still a bug.

[ghost]

@alcuadrado This is still a bug and it would create a valid error for contract verifiers. It is 2022 and I think we have no reason to remove those comment lines manually.

[rkalis]

The recommended way of verifying source code nowadays is multi-file verification (using Standard Input JSON), which retains license information for individual files. This is supported by Etherscan as well as truffle-plugin-verify, hardhat-etherscan and hardhat-deploy.

So imo it seems fair enough to say this issue is out of scope for truffle-flattener, and if you still want to verify through a flattened file for any reason, dealing with the license identifiers should be handled in the code of those verification plugins, rather than here.