rkalis/truffle-plugin-verify

Failure to verify smart contracts using solidity ^0.6.0

Closed this issue · 26 comments

Amxx commented

Query:
truffle run verify NFWalletFactory --network rinkeby --debug

Output:

DEBUG logging is turned ON
Verifying NFWalletFactory
Reading artifact file at /home/amxx/Work/Projects/NFWallets/solidity/build/contracts/NFWalletFactory.json
Retrieving constructor parameters from https://api-rinkeby.etherscan.io/api?module=account&action=txlist&address=0x19631cB609b85FE81A00b4935A044E3A7d30F8db&page=1&sort=asc&offset=1
Constructor parameters received: 0x
Flattening source file /home/amxx/Work/Projects/NFWallets/solidity/contracts/NFWalletFactory.sol
Sending verify request with POST arguments:
{
  "apikey": "......",
  "module": "contract",
  "action": "verifysourcecode",
  "contractaddress": "0x19631cB609b85FE81A00b4935A044E3A7d30F8db",
  "sourceCode": "pragma solidity ^0.6.0;",
  "codeformat": "solidity-single-file",
  "contractname": "NFWalletFactory",
  "compilerversion": "v0.6.6+commit.6c089d02",
  "optimizationUsed": 1,
  "runs": 200,
  "constructorArguements": ""
}
Checking status of verification request uxjyzgtg9qgmhrstkbcdnaiqbhkhsk7bgkrg8tccq173zzfi1y
Fail - Unable to verify
Failed to verify 1 contract(s): NFWalletFactory

My guess is that the flatening fails. Source code and artefacts are here: https://github.com/Amxx/NFWallet/tree/master/solidity

Thanks for opening this issue! I was also able to replicate these results. I also tried using sol-merger directly to flatten the source code, which also resulted in a file with just the initial pragma statement. I'm not sure what is causing this, can try out some things later. @RyuuGan, do you have any idea what could be happening?

I found out, that in library what I'm using to parse contract and other stuff in file there is an error:

sol-merger:debug Processing file D:/projects/NFWallet/solidity/contracts/NFWalletFactory.sol +0ms
  sol-merger:error [
  sol-merger:error   {
  sol-merger:error     message: "extraneous input 'payable' expecting {'~', 'from', '(', '[', 'address', 'calldata', 'var', 'bool', 'string', 'byte', '++', '--', 'new', '+', '-', 'after', 'delete', '!', Int, Uint, Byte, Fixed, Ufixed, BooleanLiteral, DecimalNumber, HexNumber, HexLiteral, 'type', Identifier, StringLiteral}",
  sol-merger:error     line: 30,
  sol-merger:error     column: 45
  sol-merger:error   },
  sol-merger:error   { message: "missing ';' at '{'", line: 30, column: 65 },
  sol-merger:error   {
  sol-merger:error     message: "no viable alternative at input 'value:'",
  sol-merger:error     line: 30,
  sol-merger:error     column: 71
  sol-merger:error   }
  sol-merger:error ]

But looks like this library is not maintained:

https://github.com/federicobond/solidity-parser-antlr

I'll try to think how to remove this dependency. It means that I have to return back to old code, when I was parsing manually (maybe), or use another library for that.

To find this comments do next

DEBUG="sol-merger*" <command to parse contracts>

Could solc-js be used for parsing? It looks like they conform to the solc input and output JSON formats, which should include an AST object.

Hello @rkalis,

I'll take a look on this weekend.

Best regards

I actually just had the same issue with my own contracts. After running sol-merger with @RyuuGan's suggestion DEBUG="sol-merger*" set it appears that in my case the trouble was mainly in the v0.6.0 way of passing value (i.e. { value: ... } rather than .value(...)). Since the old way of value is still supported in v0.6.0, I changed the occurrences to the old way. I was able to verify after doing that.

It's definitely not ideal, but it still beats manual verification 😅

Can you test that it is working now ? sol-merger@3.0.0

I updated the grammar, so it should support structs and enums and also new syntax.

Best regards

I tried running DEBUG="sol-merger*" sol-merger NFWalletFactory.sol again using sol-merger@3.0.0. It doesn't log any errors:

  sol-merger:debug Output directory undefined +0ms
  sol-merger:debug RemoveComments? false +2ms
  sol-merger:debug [ '/Users/rosco/repos/ethereum/NFWallet/solidity/contracts/NFWallet.sol' ] +46ms
  sol-merger:debug Processing file /Users/rosco/repos/ethereum/NFWallet/solidity/contracts/NFWallet.sol +0ms
  sol-merger:debug /Users/rosco/repos/ethereum/NFWallet/solidity/contracts/NFWallet.sol -> /Users/rosco/repos/ethereum/NFWallet/solidity/contracts/NFWallet_merged.sol +2s

But the full output file is just this:

pragma solidity ^0.6.0;


contract NFWalletFactory is CounterfactualTokenRegistry, ENSReverseRegistration
{
	constructor()
	public CounterfactualTokenRegistry(
		address(new NFWallet()),
		'NonFungibleWallet',
		'NFW')
	{
	}

	function encodeInitializer(bytes32 _salt)
	internal view returns (bytes memory)
	{
		return abi.encodeWithSignature('initialize(address)', address(this), _salt);
	}

	function createWallet(address _owner, bytes32 _salt)
	external payable returns (address)
	{
		address wallet = address(_mintCreate(_owner, encodeInitializer(_salt)));
		if (msg.value > 0)
		{
			(bool success, bytes memory returndata) = payable(wallet).call{value: msg.value}('');
			require(success, string(returndata));
		}
		return wallet;
	}

	function predictWallet(address _owner, bytes32 _salt)
	external view returns (address)
	{
		return address(_mintPredict(_owner, encodeInitializer(_salt)));
	}

	function setName(address _ens, string calldata _name)
	external onlyOwner()
	{
		_setName(ENS(_ens), _name);
	}
}

So it looks like something is still going wrong somewhere.

Yea, I found one bug that will is fixed on develop. In this commit:

RyuuGan/sol-merger@4155290

Looks like it it is working from develop. So I'll finish the comments and publish new version.

Comments are also working, but I want to introduce the different delimiter for comments.

With command

npx sol-merger@3.0.1 NFWalletFactory.sol

It merged the file and the result had more then 1900 lines of code. Can you check it one more time ?

Now I'm getting

Error: ENOENT: no such file or directory, stat '/Users/rosco/repos/ethereum/NFWallet/solidity/node_modules/@iexec/solidity/contracts/ENStools/ENSReverseRegistration.sol'

Could that have something to do with the Yarn workspaces setup? But it didn't complain about any of the other contracts in node_modules/.

@rkalis it might be, because I have this contracts when installed it with npm. I need a reproduction repo to debug this.

image

image

Looks like dependecy is not installed by yarn: @iexec\solidity, that is why this file doesn't exist.

But it exists in parent directory. That is the problem. But since npm root returned sol-merger the child directory (but not the parent one, where the dependency is installed). It can not find it. The temporary workaround would be to install dependecies in folder solidity.

Thanks for investigating further @RyuuGan. I copy-pasted the correct node_modules folder so it worked. The flattening appears to work and results in a 2700+ LOC flattened file. But when I try to verify it on Etherscan, I get the following error:

myc:2647:24: TypeError: This type is only supported in ABIEncoderV2. Use "pragma experimental ABIEncoderV2" to enable the feature.
	function forwardBatch(Call[] calldata calls)

When investigating further, I see that the original unflattened code has a pragma experimental ABIEncoderV2;, but this is not included in the flattened code, so this gets removed somewhere in the process.

Hello @rkalis,

It is hard to say what you are talking about because contract NFWalletFactory.sol doesn't have this expermental pragma:

NFWalletFactory.sol:

pragma solidity ^0.6.0;

import "@iexec/solidity/contracts/ENStools/ENSReverseRegistration.sol";
import "@opengsn/gsn/contracts/BaseRelayRecipient.sol";
import "./core/CounterfactualTokenRegistry.sol";
import "./NFWallet.sol";

But NFWallet.sol has, and it compiles correctly with 2 pragmas:

image

image

NFWalletFactory imports NFWallet, so when you're flattening NFWalletFactory it includes the code for NFWallet, but it doesn't include the experimental pragma.

Only pragma from the root contract is included.

It was like this all the time.

So if you are compiling NFWalletFactory then pragma from this contract will be included.

Is that the expected behaviour though? Since the NFWallet contract uses the experimental functionality, and NFWallet is included in NFWalletFactory, it would make sense that the flattened code would also include this pragma. The workaround is probably to just add the pragma statement to the "main" file (in this case NFWalletFactory), right?

Hello @rkalis

Yes workaround is just to add this statement to NFWalletFactory.

But I wouldn't say it is workaround, it is expected bahaviour from my side. Because otherwise sol-merger will magically decide which pragma to include and which not. So for example if one of the contract has

pragma solidity ^0.4.0;

And the other

pragma solidity 0.6.0;

They are incompatible what should sol-merger use ? Should the error be thrown ?

Just not to make it magical there simple decission was done: root contract knows what pragmas are required and taking only them. As a bonus it is controllable by developer.

Fair enough! I'll do that and test some more today.

Amxx commented

Thanks for the time you are spending on this !

So for some reason the merged code I'm getting from sol-merger for NFWalletFactory is not getting verified on Etherscan. I've confirmed that the new sol-merger version works with other v0.6.0 contracts, so I assume there's a different issue going on here.

In any case I'll publish a new truffle-plugin-verify release with the new sol-merger that should work for most v0.6.0 contracts, and we'll have to investigate this one later.

Amxx commented

So, I've retried verifying a "small" contract:

  • It gets rejected
  • using --debug I can see the constructor arguments are empty. This might be caused by the way I did deploy it. Should they be in the .json artefact ?
  • I've used the flattened code (from the --debug) AND the constructor arguments to etherscan, etherscan refuses to verify the code, when I'm comparing the produced vs expected bytecode they are identical ...

WTF?!?

So the way truffle-plugin-verify retrieves the constructor arguments is through the Etherscan API. Now it can only retrieve them that way if the contract is deployed directly (i.e. not from a factory contract) - see #8. So if you're deploying the contract through a factory and it has constructor arguments, then it currently cannot be verified through truffle-plugin-verify. Is your contract deployed through a factory?

I'm open to a PR for something to manually pass args into the command for these cases, but I'm not sure yet how that could be done in a user-friendly way.

But your third point sounds weirder. If you're passing in the flattened source code and manually inputting the constructor arguments in the etherscan form, I don't see why that shouldn't work. Especially if you're saying that the produced and expected bytecode are in fact identical.

@Enigmatic331 do you have an idea what might be happening with @Amxx' contract?

Hey thanks for the tag @rkalis, looking into it.

@Amxx - Think my friend Kaven have reached out to you on Twitter - If you'd like, you could also pass along a copy of the simple contract to me here I can jump right into it as well? In the meantime am also taking a look at the NFWalletFactory contracts you have at the top.

Addendum: I think Kaven mostly sorted things out for you @Amxx - Though if there still are any issues feel free to ping and we can look into it + see how to improve things. Merci beaucoup!

It sounds like the remaining issue here is the constructor arguments. That issue is captured in #8, so I'll close this issue. If there is something else, feel free to reopen it @Amxx!