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 App] Coinrule Multiswap #409

Open
7 tasks done
ogiberstein opened this issue Oct 31, 2024 · 8 comments
Open
7 tasks done

[Add App] Coinrule Multiswap #409

ogiberstein opened this issue Oct 31, 2024 · 8 comments

Comments

@ogiberstein
Copy link

Entry type

  • New addition

App info

URL: https://safeapp.coinrule.com/

Manifest.json URL: https://web.coinrule.com/manifest.json

Name: Coinrule Multiswap

Description: Coinrule's Multiswap lets you sell dozens and hundreds of dust tokens in just a few clicks.

Icon (PNG, 180x180): Icon
It's minified via https://tinypng.com: yes

Homepage: coinrule.com
Twitter: coinrulehq
GitHub: We use Gitlab
Discord: Discord

App supports batching multiple transactions via Safe: yes

Supported networks

- Mainnet

Revision checks

  • [n/a - no smart contracts ] 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

N/A - No Smart Contracts used

Code for review

TBC - this is on Gitlab

Team information

Company: Coinrule

Official website: Coinrule

Point of contact: Oleg Giberstein

Email/Telegram: [email protected] / @oleggiberstein

@kirkkonen
Copy link

This submission was reviewed and approved by the product team.

@tmjssz
Copy link

tmjssz commented Nov 25, 2024

I reviewed the code and it looks good to me in general. There are a couple of things I would suggest for improvement:

Frontend

  • Remove unused/deprecated dependencies, such as safe-global/auth-kit or @safe-global/safe-core-sdk
  • @safe-global/safe-apps-sdk imports are present in the project but missing in the dependency list
  • Avoid console.log ’s for debugging in production code (e.g. in SafeConvertCoins.tsx line 207-208)
  • Use useSafeAppsSDK hook in components to get the sdk instance. It can be imported from the @safe-global/safe-apps-react-sdk package which is already in the dependency list, but not used
  • Import from the top-level package if possible, instead of using internal paths:
    // e.g. instead of this
    import { ChainInfo } from '@safe-global/safe-apps-sdk/src/types';
    import { SafeInfoExtended } from '@safe-global/safe-apps-sdk/src/types/sdk';
    
    // just do this
    import { ChainInfo, SafeInfoExtended } from '@safe-global/safe-apps-sdk'
  • Increase test coverage

Backend

  • Migrate from @safe-global/safe-core-sdk-types (deprecated) to @safe-global/types-kit

@ogiberstein
Copy link
Author

ogiberstein commented Nov 25, 2024 via email

@tmjssz
Copy link

tmjssz commented Nov 26, 2024

@ogiberstein They are not critical. So we can proceed and our QA will test the app next.

@github-project-automation github-project-automation bot moved this to New issues in Web Squad Nov 26, 2024
@liliya-soroka liliya-soroka moved this from New issues to Ready for QA in Web Squad Nov 26, 2024
@liliya-soroka
Copy link
Member

Trying to use the app :

  1. I have create an account and login
  2. got the info message that I should sign a message to continue as a safe
    Why the message is 1 only ?
image
  1. signed the message and got the error on UI in the app
image

@liliya-soroka
Copy link
Member

What feature will you suggest using to check the tx from the safe app to the connected safe?
image

@ogiberstein
Copy link
Author

ogiberstein commented Nov 26, 2024 via email

@ogiberstein
Copy link
Author

ogiberstein commented Dec 3, 2024 via email

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

No branches or pull requests

4 participants