sourcify icon indicating copy to clipboard operation
sourcify copied to clipboard

Intermittent Upload Failure of Hardhat Artifact Files to Sourcify Verifier

Open mshakeg opened this issue 1 year ago • 4 comments

Description

I've been experiencing intermittent failures when trying to verify contracts by uploading Hardhat artifact files (build-info/{hash}.json) to the Sourcify Verifier (https://sourcify.dev/#/verifier). This issue has only started occurring recently; I had been able to upload files successfully in the past without any problem. The failures are sporadic, with some attempts succeeding while others fail(or never complete "Checking contracts") without a clear pattern or identifiable cause.

Steps to Reproduce

  1. Go to https://sourcify.dev/#/verifier.
  2. Click on "Verify Contracts".
  3. Select the appropriate network.
  4. Upload the build-info/{hash}.json file generated by Hardhat. Such as the attached file.
  5. Observe that it never completes "Checking contracts"

Expected Behavior

The expected behavior is for the file to be uploaded successfully every time, allowing the contract to be verified without issue.

Actual Behavior

The upload process intermittently fails. Sometimes, it proceeds as expected, but at other times, the upload does not complete, and the verification process cannot proceed. There is no error message displayed on the UI.

4be0a250f2e0bdcedfcf348751eb11c6.json

mshakeg avatar Feb 15 '24 21:02 mshakeg

Sometimes the above error popup shows up

sourcify upload artifact issue

mshakeg avatar Feb 15 '24 21:02 mshakeg

Thanks for the detailed description!

From what I see we have some performance issues when adding large files. At first sight the hash function we use to get identifiers for the files in session seems to take quite a long time. I couldn't reproduce it on https://staging.sourcify.dev/#/verifier fully but there it also seems to take a long time.

We should replace the keccaks with more lightweight identifier generators.

kuzdogan avatar Feb 16 '24 12:02 kuzdogan

I can save couple seconds locally by changing how we create identifiers for the session: https://github.com/ethereum/sourcify/commit/580064317f5716d6e578af9943bf437d68a8026a

But the main issue seems to come in lib-sourcify when we are generating variations: https://github.com/ethereum/sourcify/blob/580064317f5716d6e578af9943bf437d68a8026a/packages/lib-sourcify/src/lib/validation.ts#L303-L308

Here we need to use keccak256 because in the metadata file the source file identifiers are in keccak. Here we generate variations of each source file extracted (adding whitespaces etc.) and hashing every single one of them. This results in a whopping 1360 variations meaning 1360 hashes. Locally it takes couple seconds but in a busy server I guess this takes a long time, longer than the request timeout which is why you are having that error..

In the meantime I can recommend you to use hardhat-verify directly if possible. That way you don't have to use the session API and you don't have to send the source files that are not needed which might be happening by dumping the whole build artifact.

kuzdogan avatar Feb 16 '24 15:02 kuzdogan

@kuzdogan ok, thanks for the update. Not sure what the sourcify cloud set up is like, but if each instance has multiple cores available you could look to parallelize the execution of keccak256str across multiple worker threads.

mshakeg avatar Feb 16 '24 17:02 mshakeg

@kuzdogan not much has changed in my experience, not sure if autoscaling is enabled to handle increased load? maybe tighten the per IP rate limits, as the service really hasn't been functional since I created this issue.

mshakeg avatar Feb 24 '24 14:02 mshakeg

Yes sorry we found out a general performance issue with the session APIs, affecting not only this contract but almost all. Since the session APIs are not used much, it seemed to have gone unnoticed. If the second keccak generation was the issue this would have manifested itself in the non-session too, so the problem is not there.

In the meantime, can you please share the chainId and the address of the contract you want to verify?

If just verifying this contract anyway solves your issue you can verify via the non session API or we can verify for you.

I'm off this week so @marcocastignoli will take over this issue.

kuzdogan avatar Feb 26 '24 14:02 kuzdogan

@kuzdogan sure the contracts are on avalanche c-chain(chainId 43114). I've verified them on the https://snowtrace.io/ explorer.

ICHIVaultFactory: 0xDD2346e0dA9540792C2F2E86016bc44Ba39DC72d UV3Math: 0x921aCCA39e8D3519A503EE4A11b56d6eEACbb2Aa ICHIVaultDeployer: 0xf3145E8Cd87E94B65cF5Ba336292d557aD380e5B

mshakeg avatar Feb 26 '24 15:02 mshakeg

I was able to manually verify the ICHIVaultFactory at 0xDD2346e0dA9540792C2F2E86016bc44Ba39DC72d but the other two have nested auxdatas and that's why the verification failed. See #851

We are soon going to handle these cases it's just not added to the verification yet cc: @marcocastignoli

We still need to fix the performance issue in the session verification.

kuzdogan avatar Mar 05 '24 17:03 kuzdogan

Hey @kuzdogan I just wanted to follow up on the progress of this issue. Sourcify is basically not functional at least whenever I try to use it.

mshakeg avatar Mar 18 '24 11:03 mshakeg

@mshakeg Sorry for the late response. I'm trying to further debug into this.

Could you please try verifying in an incognite browser window, or after deleting the cookies?

The issue seems to be with the existing sessions

kuzdogan avatar Mar 28 '24 13:03 kuzdogan

I discovered that we are creating a new session for every request (not only the ones calling "/session" endpoints).

I think that in production, a new session is created for each of the hundreds of requests we receive every second causing the session storage to become very slow. Before moving on with https://github.com/ethereum/sourcify/issues/1321 I would try to limit the session usage to only "/session" endpoints and see if it fixes the problem.

marcocastignoli avatar Apr 03 '24 09:04 marcocastignoli

@marcocastignoli Great, let's see if this solves the issue, and if so do a quick release.

We should move away from the MemorySession regardless but the fixing this should be ASAP

kuzdogan avatar Apr 03 '24 10:04 kuzdogan

The issue should be fixed with the latest release @mshakeg Sorry for taking too long to fix this.

kuzdogan avatar Apr 04 '24 15:04 kuzdogan

@kuzdogan thanks, it seems to be much better now.

mshakeg avatar Apr 04 '24 17:04 mshakeg