Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Vaultcraft App #413

Open
8 tasks done
Andreadinenno opened this issue Nov 8, 2024 · 7 comments
Open
8 tasks done

Add Vaultcraft App #413

Andreadinenno opened this issue Nov 8, 2024 · 7 comments

Comments

@Andreadinenno
Copy link

Entry type

New addition

App info

URL: https://app.vaultcraft.io/

Manifest.json URL: https://app.vaultcraft.io/manifest.json

Name: Vaultcraft

Description:
Easily deploy ERC4626 and ERC7540 vaults that can be managed via Gnosis Safe

Icon (PNG, 180x180): https://app.vaultcraft.io/images/tokens/vcx.svg
It's minified via https://tinypng.com: no

Homepage:
Twitter: https://app.vaultcraft.io/manifest.json
GitHub: https://github.com/Popcorn-Limited
Discord: https://discord.com/invite/kgCun7SbkR

App supports batching multiple transactions via Safe: yes

Supported networks

  • Mainnet
  • Base
  • Abitrum
  • Optimism
  • Fraxtal
  • Avalanche
  • Polygon
  • XLayer
  • BNB

Revision checks

  • Used smart contracts were audited.
  • You have implemented the app using the Safe Apps SDK
  • Your Safe App includes a manifest.json file at the root with the required data
  • The app can be loaded as a custom Safe App in the Apps section of https://app.safe.global.
  • The app auto-connects to the Safe as a wallet
  • It doesn't try to connect to the browser wallet (e.g. MetaMask)
  • You are able to trigger and execute one transaction with a Safe.
  • RPC requests are optimized (not triggering many requests in a very short time period).

Audit document

https://github.com/Popcorn-Limited/audit-04-24
https://github.com/Popcorn-Limited/audit2

Code for review

https://github.com/Popcorn-Limited/

Team information

Company: Popcorn Limited

Official website: https://vaultcraft.io/

Point of contact:

Telegram: t.me/vaultcraft

@kirkkonen
Copy link

This app was reviewed and approved by the product team.

@Andreadinenno
Copy link
Author

What are the next steps here?

@parius
Copy link
Collaborator

parius commented Nov 29, 2024

Tech team needs to review the code and audit results and QA to test it

@iamacook
Copy link
Member

@Andreadinenno, thank you for the submission.

We’ve reviewed the code and have a few points for clarification and adjustment:

  1. Could you confirm the test and review process for the project? We noticed that contributors are responsible for their own PRs and are merging without review. Additionally, we observed that there are currently no tests in the project.
  2. Could you update the manifest.json to use "Safe" instead of "Gnosis".

Additionally, to support the submission, could you:

  1. Provide an example flow to test batch support?
  2. Confirm your Twitter/audit links, as they were not provided. We found the following via Google:

We look forward to your response.

@RedVeil
Copy link

RedVeil commented Dec 11, 2024

@iamacook Thank you for your feedback!
To your clarifications and adjusments:

  1. We review our PRs but minor fixes are often reviewed in person and pushed to main to react quickly to user feedback.
  2. Updated it.

For the additional questions:

  1. Do you mean a sample to show multicall-write support or what do you refer to with batch support?
  2. Those are the correct links :) Sorry for not including them before.

@iamacook
Copy link
Member

@RedVeil, thank you for the clarifications.

Do you mean a sample to show multicall-write support or what do you refer to with batch support?

What's the easiest way to trigger a multicall? The flows I looked at were stepped, ergo not batchable.

I am otherwise satisfied and happy to pass this onto QA. cc @francovenica

@RedVeil
Copy link

RedVeil commented Dec 13, 2024

@iamacook Ah damn you are right. With the solver transactions in between for zaps it will be hard for us to make it properly batchable. I guess we marked that wrong in the PR. Apologies for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants