[draft]Flag for throw on partial match

[max-wickham]

For our use case we want to be able to ensure its possible to check contracts pass verification even if the contract has already been verified and stored in the database. Not sure if this is a use case others may share or not. Also not sure that this is the best way to go about this.

This PR adds a flag to config that allows for disabling the exception thrown if a match already exists for a contract (at the same verification level).

If the flag is disabled then the reverification will pass, (but wont update the database).

[kuzdogan]

Hi and thanks for the PR :)

The idea looks good to me but this is only covering one of these two cases:

1. (This PR covers) Contract was verified match. We received a new verification for the same contract. We try verifying again because the match might improve (can be exact_match). But the verification yielded a match again. Here the verification request POST /v2/verify/ will be a success 202, as it is received successfully. But if you check the verificationJob with GET /v2/verify/{jobId} you will get a 409 as this job ended with a "conflict", the match did not improve.
2. Contract was verified exact_match. We received a new verification for the same contract. We can immediately ignore the verification request since the match can't be improved. Here POST /v2/verify will return a 409 immediately, without creating a verification job.

Which one do you need? Both?

The code as is does not work because you are not providing the throwOnPartialMatch option in the Server class and then not passing to to StorageService's when they are initialized. It would also be nice to see a test case for when throwOnPartialMatch is false (true by default). But again, the other case is not covered anyway. If you want to do the implementation please go ahead. Otherwise we can implement this too.

I also notice our test case only covers the case 2 above and not 1. We need a test case for this too.

Also curious about your use case. Are you running your self-hosted Sourcify and what purposes? Why do you need the verification job to return a success when the match does not improve? We're always looking into understanding how people use Sourcify behind the scenes.

[max-wickham]

Hi thanks for the comments. I think we need both cases, we always want to retry the actual verification, then only store if improving the match. Will update the PR with the implementation comments.

The reason we want to do this is at some frequency internally we want to reverify all our contracts against the original source repo (using a self hosted sourcify instance). If a developer modified the source code before the original verification slightly then with the current system when we reverify the contract would already be marked as verified as so sourcify would not retry the verification iiuc. It is also slightly hard for us to just download the source code of the verified contract as we change the contract name with each deploy so would have to add logic to account for this. I think there are other ways of getting around this, but generally we would like to just ensure the bytecode deployed matches the source repo whenever we need to.

[max-wickham]

Reading through now realised a couple of things.
1.

The code as is does not work because you are not providing the throwOnPartialMatch option in the Server class

I though I was doing this in line 157 of cli.ts but very likely I've misunderstood.

In terms of the second case, rerunning verification if a full match exists. iiuc the checkIfAlreadyVerified function in middleware.ts would need to receive the flag in order to implement this. However, I'm not sure what the expected way to pass configs to this file would be. Or maybe better to remove this function from the route in verification.router.ts dependant on a flag?

Apologies not particularly familiar with typescript.

[kuzdogan]

Hi thanks for the comments. I think we need both cases, we always want to retry the actual verification, then only store if improving the match. Will update the PR with the implementation comments.

The reason we want to do this is at some frequency internally we want to reverify all our contracts against the original source repo (using a self hosted sourcify instance). If a developer modified the source code before the original verification slightly then with the current system when we reverify the contract would already be marked as verified as so sourcify would not retry the verification iiuc. It is also slightly hard for us to just download the source code of the verified contract as we change the contract name with each deploy so would have to add logic to account for this. I think there are other ways of getting around this, but generally we would like to just ensure the bytecode deployed matches the source repo whenever we need to.

Can you reach out to me so I can ask more questions :) same username on all platforms and @kuzdogan:matrix.org on Matrix.

[kuzdogan]

I though I was doing this in line 157 of cli.ts but very likely I've misunderstood.

Yes so cli.ts is for initiating a Sourcify instance on cli like node cli.ts with given options. The actualy Server class and instance is at server.ts which takes options as constr. args. That is used e.g. in tests or you want to run a Sourcify instance within your project. Like a code debugging tool for VSCode would have new Server() in their project.

In terms of the second case, rerunning verification if a full match exists. iiuc the checkIfAlreadyVerified function in middleware.ts would need to receive the flag in order to implement this. However, I'm not sure what the expected way to pass configs to this file would be. Or maybe better to remove this function from the route in verification.router.ts dependant on a flag?

You can pass this config as a request variable similar to these

https://github.com/ethereum/sourcify/blob/612ed66ad617933e2526b2b0703dd34be55efaa3/services/server/src/server/server.ts#L131-L137

and call as such:

https://github.com/ethereum/sourcify/blob/612ed66ad617933e2526b2b0703dd34be55efaa3/services/server/src/server/apiv2/middlewares.ts#L200-L201

Then if it's both exact_matches it should respond with the response of the /v2/contract/ when ?fields= is empty.

Apologies not particularly familiar with typescript.

Happy to help! As said we can also implement this if you ever feel it's too much but if you want go for it!

[kuzdogan]

Is this still needed?

[manuelwedler]

closing due to inactivity