Skip to content

[Vyper support] Add VyperCheckedContract in lib-sourcify and the new endpoint /verify/vyper in server#1760

Merged
marcocastignoli merged 15 commits into
vyper-verification-mainfrom
vyper-support
Dec 9, 2024
Merged

[Vyper support] Add VyperCheckedContract in lib-sourcify and the new endpoint /verify/vyper in server#1760
marcocastignoli merged 15 commits into
vyper-verification-mainfrom
vyper-support

Conversation

@marcocastignoli

@marcocastignoli marcocastignoli commented Nov 28, 2024 •
edited by manuelwedler
Loading

Copy link
Copy Markdown
Member

This PR currently:

  • Implements VyperCheckedContract and SolidityCheckedContract. VyperCheckedContract generates a metadata file for Vyper contracts.
  • Implements useVyperCompiler, a function similar to useCompiler which handles dynamic download for Vyper versions (need some more work for the platform architectures), and vyper compilation.
  • verifyDeployed now takes as parameter CheckedContract, an abstract class extended by both VyperCheckedContract and SolidityCheckedContract. It skips metadata-related functions for VyperCheckedContract.
  • Stores match in all StorageServices (I still haven't checked everything in detail)

TODO:

  • - Review what's stored in the database
  • - Create a common folder for the compilers code used by both lib-sourcify and server (idk if it fits in this new feature, imo yes because it's a lot of duplicated code now that we have also vyper stuff)
  • - Rewrite platform architecture management (vyper versions for x86, arm, ...)
  • - Add documentation
  • - Handle vyper transformations (I still have to understand what we can do about it)
  • - Add more tests, and finalize lib-sourcify test
  • - Finalize /verify/vyper API errors + openapi responses
  • - Documentation, updating readme files

@marcocastignoli marcocastignoli mentioned this pull request Nov 28, 2024
Closed
@manuelwedler manuelwedler self-assigned this Nov 29, 2024
manuelwedler
manuelwedler requested changes Dec 3, 2024
View reviewed changes

@manuelwedler manuelwedler left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found it quite impressive how minimal the changes to lib-sourcify are for this feature.

Some general feedback:

  • Why do we need a metadata file mock for VyperCheckContract? I guess this is due to the verifyDeployed method which expects a metadata. This could be documented better in some way. Is it possible to get rid of it when we refactor the verification flow?
  • lib-sourcify README needs to be updated due to renaming of CheckedContract.

I also tried to verify a vyper contract on a local server instance. Unfortunately, it didn't work due to Vyper path not found. The server logs show that vyper was downloaded. But I cannot find the binary on my machine.

2024-12-03T11:19:24.889Z [debug] [Server] Downloaded vyper - version=0.4.0 | vyperPath=/home/manuel/.vyper-bin/vyper.0.4.0.linux | platform=linux - [traceId=db3e02b8-8701-47ad-9deb-c84b6af76241]
2024-12-03T11:19:24.889Z [info] [Server] safeHandler - errorMessage=Vyper path not found | errorStack=Error: Vyper path not found
    at useVyperCompiler (/home/manuel/Projects/sourcify/sourcify/services/server/dist/server/services/compiler/local/vyperCompiler.js:46:15)

Comment thread packages/lib-sourcify/src/lib/AbstractCheckedContract.ts Outdated
Comment thread services/server/src/server/services/compiler/local/vyperCompiler.ts
Comment thread packages/lib-sourcify/src/lib/validation.ts
Comment thread packages/lib-sourcify/src/lib/IVyperCompiler.ts Outdated
Comment thread packages/lib-sourcify/src/lib/VyperCheckedContract.ts Outdated
Comment thread services/server/src/server/controllers/verification/vyper/stateless/vyper.stateless.handlers.ts
Comment thread services/server/src/server/controllers/verification/vyper/stateless/vyper.stateless.paths.yaml
Comment thread services/server/src/server/cli.ts

logger.info("Using local vyper compiler");
const vyperRepoPath =
(config.get("vyperRepo") as string) || path.join("/tmp", "vyper-repo");

@manuelwedler manuelwedler Dec 3, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be documented in the server's README.

@manuelwedler manuelwedler Dec 5, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this fixed yet

Comment thread services/server/test/integration/verification-handlers/verify.stateless.spec.ts
Comment thread services/server/test/integration/verification-handlers/verify.stateless.spec.ts Outdated
@marcocastignoli

marcocastignoli commented Dec 3, 2024

Copy link
Copy Markdown
Member Author

Why do we need a metadata file mock for VyperCheckContract? I guess this is due to the verifyDeployed method which expects a metadata. This could be documented better in some way. Is it possible to get rid of it when we refactor the verification flow?

The main reason why we still need the metadata here is because we want to store it in RepositoryV1/V2 to align the repository fs to solidity contracts. I think @kuzdogan had this idea first.

@marcocastignoli

marcocastignoli commented Dec 3, 2024

Copy link
Copy Markdown
Member Author

I also tried to verify a vyper contract on a local server instance. Unfortunately, it didn't work due to Vyper path not found. The server logs show that vyper was downloaded. But I cannot find the binary on my machine.

That's probably related to the fact that I still have to work on rewriting platform architecture management (vyper versions for x86, arm, ...)

@kuzdogan

kuzdogan commented Dec 3, 2024

Copy link
Copy Markdown
Member

Why do we need a metadata file mock for VyperCheckContract? I guess this is due to the verifyDeployed method which expects a metadata. This could be documented better in some way. Is it possible to get rid of it when we refactor the verification flow?

The main reason why we still need the metadata here is because we want to store it in RepositoryV1/V2 to align the repository fs to solidity contracts. I think @kuzdogan had this idea first.

We discussed this during the call but to recap, the reason is all of the APIv1 is based on the existence of the metadata.json. Some examples:

So it's needed for backwards compatiility

manuelwedler
manuelwedler reviewed Dec 5, 2024
View reviewed changes

@manuelwedler manuelwedler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The additions until now look good. There are still some things missing from above.

Comment thread services/server/src/server/controllers/verification/vyper/stateless/vyper.stateless.handlers.ts Outdated
Comment thread packages/lib-sourcify/src/lib/types.ts Outdated
manuelwedler
manuelwedler approved these changes Dec 9, 2024
View reviewed changes

@manuelwedler manuelwedler left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

We'll address the last comments afterwards

@marcocastignoli marcocastignoli merged commit 71320f8 into vyper-verification-main Dec 9, 2024
@marcocastignoli marcocastignoli mentioned this pull request Dec 12, 2024
Merged
9 tasks
manuelwedler pushed a commit that referenced this pull request Jan 2, 2025
@marcocastignoli @manuelwedler
[Vyper support] Add VyperCheckedContract in lib-sourcify and the …
3df622f 
…new endpoint `/verify/vyper` in server (#1760)

* add VyperCheckedContract

* rename CheckedContract.solidity to sources

* refactor checkedcontract into an abstract class

* refactor AbstractCheckedContract in server

* use AbstractCheckedContract type in StorageServices

* add /verify/vyper endpoint

* successfully store is database and repository vyper match

* dynamically download the vyper compiler

* fix lib-sourcify-tests

* enable all tests in server

* improve comments for IVyperCompiler

* fix pr suggestions

* rename checkFiles to CheckFilesWithMetadata

* rename useCompiler to useSolidityCompiler

* fix pr suggestions
manuelwedler added a commit that referenced this pull request Jan 2, 2025
@manuelwedler
Add Vyper support to Sourcify (#1784)
5c0c1bd 
Adds vyper support, here the initial PR: #1760. Below the related tasks:

- [x] Review what's stored in the database
- [x] ~~Create a common folder for the compilers code used by both
lib-sourcify and server (idk if it fits in this new feature, imo yes
because it's a lot of duplicated code now that we have also vyper
stuff)~~ postponed
- [x] Rewrite platform architecture management (vyper versions for x86,
arm, ...): #1776
- [x] Update READMEs #1786
- [x] Update documentation sourcifyeth/docs#27
- [x] Handle vyper transformations: #1783
- [x] Add more tests, and finalize lib-sourcify test: #1781
- [x] Finalize `/verify/vyper` API errors + openapi responses: #1774
- [x] Check Vyper immutable field to see if we can support immutable
transformation: #1790

After merging:
- publish new docs
@kuzdogan kuzdogan deleted the vyper-support branch January 2, 2025 18:18
@stackenbotten3000 stackenbotten3000 moved this to COMPLETED in Sourcify Public Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@manuelwedler manuelwedler manuelwedler approved these changes

Assignees

@manuelwedler manuelwedler

Labels

None yet

Projects

Sourcify Public
Status: COMPLETED

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@marcocastignoli @kuzdogan @manuelwedler @r0qs @stackenbotten3000