blockscout / blockscout-rs

Microservices for blockscout indexer
MIT License
162 stars 115 forks source link

Can't verify simple Yul contract due to parser error (that only occurs on Blockscout) #449

Closed haltman-at closed 1 year ago

haltman-at commented 1 year ago

Hi! I saw that Blockscout now supports Yul contract verification, so I figured I'd test it out with a dead simple Yul contract. However, verification failed with a compile error, even though the contract compiles fine on my machine. No idea what's going wrong here.

(For context, if you're wondering: I work on Truffle and I'm testing this out because I'm trying to finally write that Blockscout integration for Truffle that has been talked about for a while. I filed a whole bunch of issues a while back that were blocking it, but those are finally solved, so I figured it was time to take another stab at it! Of course, first I had to get up to date on Blockscout's current workings, and make sure there weren't any cases I was unaware of, which is when I encountered the Yul case and realized I had to make sure I knew how it worked before doing anything. But, as you can see, my test of this case hit a snag! So for now I still don't know how this case works.)

Environment

I'm using the Goerli Blockscout deployment at https://eth-goerli.blockscout.com/

Steps to reproduce

I'm attempting to verify the contract on Goerli at address 0x8F95fc67146d6c70D872FA53def7bB053FC7f1a1. I'm doing this via the web interface (link).

The contract verification method is Solidity (flattened). The Yul box is checked. Compiler version is 0.8.19. EVM version is default. Optimization is turned off. There are no libraries. The contract code is:

//SPDX-License-Identifier: MIT

object "ExternalTestYul" {
  code {
    let size := datasize("runtime")
    datacopy(0, dataoffset("runtime"), size)
    return(0, size)
  }
  object "runtime" {
    code {
      mstore(0, 1)
      return(0, 0x20)
    }
  }
}

This is, to be clear, the actual Yul code that I compiled the contract with on my machine.

Expected behaviour

The contract should verify.

Actual behaviour

I get the following error message:

Compilation error: ["ParserError: Expected identifier but got 'StringLiteral'\n --> .yul:3:8:\n |\n3 | object \"ExternalTestYul\" {\n | ^^^^^^^^^^^^^^^^^\n\n"]

I mean, I guess it's expecting object to be followed by an identifier rather than a string literal, but I'm very confused because that is not how Yul works in 0.8.19, and indeed to my knowledge it has not ever worked that way; object names are, as far as I'm aware, supposed to be given as string literals and not identifiers. And, again, this is the actual Yul code I compiled the contract from. So I'm very confused as to why I'm getting an error here.

Thanks!

sevenzing commented 1 year ago

Hello @haltman-at, it seems that Blockscout is trying to compile this contract as Solidity instead of YUL for some reason. We will investigate this error as soon as possible and let you know.

rimrakhimov commented 1 year ago

Hi, @haltman-at! Thank you for the issue. We've fixed that in #451 and I believe will update our instances tomorrow. Will write you, when instances will be updated

haltman-at commented 1 year ago

Oh, OK, nice!

Can I ask a tangential question, as long as I have your attention? I notice the multi-part input says it allows Yul... does this work in any meaningful way? The Solidity compiler only allows single-file input when in Yul mode (including if you do it as standard JSON; if there are multiple sources given it'll reject). What case is multi-part Yul meant for? Like I wanted to test that case out, but I couldn't think of how to come up with a test case given that solc doesn't actually accept multi-file Yul input atm.

Thanks again!

rimrakhimov commented 1 year ago

@haltman-at Sorry for long reply. We've redeployed verification service with fixed Yul verification. You can try it now

Regarding your question, I've checked it once again and you are right, it is not possible to compile several Yul contracts at once. I didn't know about that. What our service does now is just split incoming files into solidity and yul contracts (based on the file extensions), and then generates two separate standard json-s which are compiled separately. Thus, in case if more than one yul contract are supplied, the service tries to verify all of them at once, and eventually the compiler returns an error. I'm not sure if we will do something with that, but in general there is no point to use multi-part method for yul contracts.

Thank you for letting us know about that issue!

haltman-at commented 1 year ago

Thanks, I'll give it a spin!

I guess this raises another question, which is, what's the point of allowing simultaneous Solidity and Yul? I mean, as you say, they can't be compiled together; rather you're splitting it into two and compiling them separately. But the contract being verified will necessarily come from one of those compilations -- anything in the other will then be unnecessary! So why even allow this? Is there something I'm missing? I think I'm definitely going to have to test that case out...

(Actually the fact that you're allowing a mix of Solidity and Yul could pose a problem for our integration, hm, but probably nothing we can't handle...)

haltman-at commented 1 year ago

Well, it successfully verified now, thanks! :) I'm going to close this issue, as the original issue is now solved, although I'm still quite curious about these other matters...

haltman-at commented 1 year ago

Trying things some more -- it seems that if you put in both Solidity and Yul sources, the verified source code for your contract will only include the language that's relevant, not the other! To be clear, I think that's a good thing -- I'm not filing an issue for that, I'm glad it works that way! -- but it does further raise the question of what the point is of allowing mixed Solidity and Yul in the first place.

rimrakhimov commented 1 year ago

it does further raise the question of what the point is of allowing mixed Solidity and Yul in the first place.

Actually, that is just a sub-product of our verification service design and ethers-rs library we use internally :)

The first thing is that our service does not have flattened contract verification endpoint, but only supports multi-part and standard json verification methods (you can see corresponding api definitions in swagger). So, internally blockscout instance wraps flattened contract into a multi-part version and makes a request to the verifier.

The second thing is that we use ethers-solc library to compile contracts internally. And the library supports creation of compiler input from the mix of Solidity and Yul contracts (https://github.com/gakonst/ethers-rs/blob/master/ethers-solc/src/artifacts/mod.rs#L78).

So, mostly it was just a natural choice to allow mixed Solidity and Yul contracts for multi-part verification. There is no any significant meaning besides of that 🙃