-
Notifications
You must be signed in to change notification settings - Fork 32
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] Relay #414
Comments
This submission is approved by the wallet product team. |
Hi @pedromcunha, for the code review, could you please point me to the parts of your app's source code that integrates with Safe? I am not able to find the corresponding code in the provided repo: https://github.com/reservoirprotocol/relay-kit |
Hi, we use dynamic in our private repo, which uses the kit but the connection aspect is powered by dynamic. It's my understanding that dynamic uses the safe sdk under the hood |
Ok can you invite @tmjssz to the private repo so I can review it as well? |
@pedromcunha We’re still waiting for you to grant us access to the private repository so we can review the code. |
Hi, I Invited this user: https://github.com/tmjssz Is that not the right user or do I need to add someone else? |
@pedromcunha |
I reviewed the code and the only suggestion for improvement would be to introduce basic unit test coverage. Otherwise it looks good to me and we can proceed with QA. |
I've done a couple of tx and it works fine. Still discussing things with the team. I'll be done shortly |
Ok great, we do have unit tests for the inner workings of the client (relay sdk and ui kit), this is where the main app logic is housed to make it open source and easier to port into other applications: https://github.com/reservoirprotocol/relay-kit |
@pedromcunha Hi there. question, how plausible would be to implement an "empty option" for the address recipient? the reason is that most of our users have safes that are unique to that network, so is a risk for them having the recipient to be that same safe address in another network. Another question. I see that the "Audit document" section in this ticket says that it wasn't audited yet. Is that so? Thanks |
Oh that's interesting. We require a recipient and the logic is such that it will automatically preset the destination recipient. So you're saying that these addresses are locked down to a particular chain? For instance one address for base and another address for ethereum? Re audit, yes we haven't done any audits on our contracts but it's on our roadmap. Likely sometime next year we'll make some actions towards that but I don't have any concrete timelines. |
Eveb tho we currently allow safes to be deployed in different networks (same address in different networks I mean) most of the old safes were created in a single network and don't have (and cannot have) that same address deployed in another network. That's why our suggestion would be to be able to leave that field empty so the user don't use the feature without checking that address first. Regarding the audit, is something that I need to approve the app from my side. |
So are you saying that audits are required before you can approve us? |
@pedromcunha Yes, Audits are required to approve a new Safe app. |
Entry type
App info
URL: https://relay.link
Manifest.json URL: https://relay.link/manifest.json
Name: Relay
Description: Instant, low-cost bridging and swapping, powered by Reservoir
Icon (PNG, 180x180): https://mintlify.s3-us-west-1.amazonaws.com/unevenlabs/logo/relay-black.svg
It's minified via https://tinypng.com: no
Homepage:
Twitter: https://x.com/RelayProtocol
GitHub: https://github.com/reservoirprotocol
App supports batching multiple transactions via Safe: yes (Assuming dynamic support is automatically handling this)
Supported networks
and many more!
Revision checks
manifest.json
file at the root with the required dataAudit document
Not yet audited
Code for review
SDK/UI kit: https://github.com/reservoirprotocol/relay-kit
Team information
Company: Reservoir
Official website: https://reservoir.tools
Point of contact: Pedro Cunha https://github.com/pedromcunha
Email/Telegram: [email protected]
The text was updated successfully, but these errors were encountered: