Add walletconnect provider #114

devceline · 2023-09-28T11:23:11Z

Changes

Add walletconnectProvider
Tested by hitting the /api/test endpoint - it is kept in there for testing convenience, will be reverted once everything is confirmed to work
Updated dev dev for @types/node to prevent build error

src/providers/walletconnectNotify.ts

ChaituVR · 2023-10-04T18:56:21Z

Hey @devceline @Cali93 Are there any docs on how I can test this? :)

devceline · 2023-10-05T10:16:26Z

Hey @devceline @Cali93 Are there any docs on how I can test this? :)

Hi - yes of course. As mentioned in the description I personally tested it by modifying the /api/test endpoint to use our provider directly.

preliminary setup for that is:

Either enable notify in your current WalletConnect Cloud project or create a different project and make sure to enable notify under "APIs". From there you can get the project secret.
Make sure to upload the did.json and wc-notify-config to the URL configured in the project above under https://your.domain/.well-known/
Using the variables in env.example, copy the ones prefixed with WALLETCONNECT into your .env file. Modifying the project id and project secret from the info retrieved in step 1
From there - you can connect to https://app.web3inbox.com/ and subscribe to your project from https://app.web3inbox.com/notifications/new-app
After you are subscribed and the env vars are configured - run dev in snapshot-hooks & hit the /api/test endpoint.

After that, you should see a notification in https://app.web3inbox.com in your subscription.

If you do not get one, double check that the notification type in the wc-notify-config.json exists in the getNotificationType function.

If you have any questions please feel free to reach out.

ChaituVR · 2023-10-06T18:20:29Z

Hi @devceline thanks for the reply, i have seen the WC cloud projects but that needs verification. Is there some way to test with out touching https://your.domain/.well-known/ or snapshot.org directly

chris13524 · 2023-10-06T20:12:11Z

@ChaituVR you can deploy a testing domain to host the .well-known for a new testing WalletConnect Cloud project, e.g. snapshot-wc-notify-test.vercel.app. It doesn't have to be tied to the snapshot.org domain until you move to prod.

ChaituVR · 2023-10-17T19:37:27Z

Hey guys, how can i see my test app here https://app.web3inbox.com/notifications/new-app ?

devceline · 2023-10-17T20:07:26Z

Hey guys, how can i see my test app here https://app.web3inbox.com/notifications/new-app ?

Hey! We'll send you a full video on this tomorrow. However, for now

You can follow the following instructions to see your app there.

On https://app.web3inbox.com/notifications/new-app, go to the settings page by clicking the cog on the sidebar

Then clicking the "Notifications" tab

And toggle "Display all"

Then navigate back to https://app.web3inbox.com/notifications/new-app and you should see your app there.

ChaituVR · 2023-10-24T19:13:46Z

Hi @devceline I could get my app on this list. here is my app https://cloud.walletconnect.com/app/project?uuid=1b17f525-1b76-40d7-86e4-66821b538885 should i have to publish it?

chris13524 · 2023-10-24T20:00:18Z

@ChaituVR you don't need to publish your app to test it, just enable Notify API in Cloud and also toggle the switch in app.web3inbox.com from Celine's post above.

ChaituVR · 2023-10-25T03:21:51Z

I did that @chris13524 but sadly i don't see it. I see other test ones but not mine. Maybe i miss some step ,😥

ChaituVR · 2023-10-25T06:23:59Z

Or is there some other way i can test this ? maybe i can build a small front end like here https://docs.walletconnect.com/api/notify/usage

devceline · 2023-10-25T10:41:24Z

Or is there some other way i can test this ? maybe i can build a small front end like here https://docs.walletconnect.com/api/notify/usage

Just checked your project, looks like Notify is disabled, can you go ahead and tick that toggle for Notify API?

chris13524 · 2023-10-25T15:39:19Z

@ChaituVR I don't think Ngrok will work for hosting the did.json because Ngrok requires the user to press a button in order to view the site (which a JS fetch will not be able to do). So you will probably need to host the did.json on another platform such as Cloudflare Pages or Vercel. Key is hitting that switch and getting a green toast, and it doesn't look like that was setup yet.

ChaituVR · 2023-10-25T20:01:52Z

Weird it actually shows that i have enabled it, ngrok works without any "press a button"

ChaituVR · 2023-10-25T20:03:41Z

Oh i retried disabling and enabling it again and i see my app on the list now 🥳

ChaituVR · 2023-10-29T18:40:10Z

I get this as response when i try to trigger test endpoint

[WalletConnect] there are 1 addresses that are not subscribed through WalletConnect
[WalletConnect] could not get matching notification body for event proposal/created
Failed to deserialize the JSON body into the target type: missing field `notification` at line 1 column 15

ChaituVR · 2023-10-29T18:41:11Z

Tried adding proposal/created to notifications of my test app, but i always see these somehow

devceline · 2023-10-30T08:59:21Z

Tried adding proposal/created to notifications of my test app, but i always see these somehow

Ah yes, there was a spec change over the past week. Now notification types need to be passed by ID. I checked your config and got the ID for proposal_update and pushed that up.

Also realized I didn't document this above but like other providers that take a list of subscribers, the WalletConnect Notify provider does as well.

You should pass your address on line 32 in src/api.

We can remove this and just use notify server's list of subscribers directly, of course, but I thought targeting an address would be easier to test with.

ChaituVR · 2023-11-01T10:27:31Z

whatever way i try, I always get this error
[WalletConnect] there are 1 addresses that are not subscribed through WalletConnect

😢

Anyway, what are the next steps to proceed?

devceline · 2023-11-01T10:31:24Z

whatever way i try, I always get this error [WalletConnect] there are 1 addresses that are not subscribed through WalletConnect

😢

Anyway, what are the next steps to proceed?

Well - can you tell me what address you're putting in line 32 of src/api? Did you subscribe with this address on web3inbox? Any extra details to help us debug would be awesome 🙏

Next steps would be choosing whether or not you'd want to maintain a list of subscribers on your database or go with the discord provider approach and just depend on external data. In this case, external data is notify servers' list of subscribers.

Once we have that squared away I can update the PR to match, but if you can't test this maybe we should arrange a meeting to actually tackle the issue.

ChaituVR · 2023-11-10T17:47:46Z

Well - can you tell me what address you're putting in line 32 of src/api? Did you subscribe with this address on web3inbox? Any extra details to help us debug would be awesome 🙏

I tried passing multiple addresses, for example 0x24F15402C6Bb870554489b2fd2049A85d75B982f, i tried metamask, mobile and trust wallet using walletconnect, maybe they are not supported yet?

Next steps would be choosing whether or not you'd want to maintain a list of subscribers on your database or go with the discord provider approach and just depend on external data. In this case, external data is notify servers' list of subscribers.

@bonustrack Could you help me with this plz?

chris13524 · 2023-11-10T19:54:25Z

@ChaituVR the address needs to be a CAIP-10 account ID. Try prefixing the address with eip165:1: for example eip155:1:0x24F15402C6Bb870554489b2fd2049A85d75B982f

ChaituVR · 2023-11-11T08:39:39Z

Hi @chris13524 thank you!

Is there some issue with
https://app.web3inbox.com/ i am getting these errors from yesterday

Also tried in incognito

chris13524 · 2023-11-12T21:31:47Z

@ChaituVR this looks like a temporary network error, and we don't commonly see this. Could you perhaps try again?

src/providers/walletconnectNotify.ts

chris13524 · 2023-12-22T14:41:22Z

src/providers/walletconnectNotify.ts

+  } catch (e) {
+    capture('[WalletConnect] failed to fetch subscribers');
+    return [];
+  }


Why are we catching and suppressing this error? If we can't get subscribers then the functions that call this should fail as well, not proceed with an empty list.

the other providers like discord and xmtp don't throw and just end up being a noop so I did the same here

chris13524 · 2023-12-22T14:43:07Z

src/providers/walletconnectNotify.ts

+  const notifyUrl = `${WALLETCONNECT_NOTIFY_SERVER_URL}/${WALLETCONNECT_PROJECT_ID}/notify`;
+
+  const body = {
+    accounts,


accounts has a max length of 500, so we need to call /notify in a loop.

Also has a rate limit of 2 per second so you may need to add some artificial timing e.g. with date math and await new Promise(resolve => setTimeout(resolve, time))

I'll refactor to fix this

bonustrack

Can you run the lint:fix command too?

src/helpers/utils.ts

src/providers/walletconnectNotify.ts

bonustrack · 2024-01-09T05:51:41Z

src/providers/walletconnectNotify.ts

+
+// Cross Reference subscribers from Snapshot to the ones in Notify
+export async function getSubscriberFromDb(space: string) {
+  const subs: string[] = await getSubscribers(space);


You don't need to run this query, you should instead use the "subscribers" variable from the send function

Awesome - updating to use subscribers.

Co-authored-by: Less <[email protected]>

src/providers/walletconnectNotify.ts

ChaituVR · 2024-01-15T11:34:20Z

src/providers/walletconnectNotify.ts

+    case 'proposal/end':
+      return `A proposal has closed for ${space.name}`;


I talked to team about this, better we should be jusing just 'proposal/start' instead of created/end

ChaituVR · 2024-01-15T10:47:36Z

src/providers/walletconnectNotify.ts

+// That should be defined in the wc-notify-config.json
+function getNotificationType(event) {
+  if (event.includes('proposal/')) {
+    return 'ed2fd071-65e1-440d-95c5-7d58884eae43';


I think this one is for new app

Suggested change

return 'ed2fd071-65e1-440d-95c5-7d58884eae43';

return 'f3c55113-06fb-47c7-87da-28ee21b9114a';

Yes - please feel free to adjust the notification types per your cloud configuration

src/providers/walletconnectNotify.ts

ChaituVR · 2024-01-15T12:16:28Z

src/providers/walletconnectNotify.ts

+  const url = new URL(proposal.link);
+  url.searchParams.append('app', 'walletconnect');


This is resulting in https://snapshot.org/?app=walletconnect#/thanku.eth/proposal/0x8c2a67398c9912f7c2ba15a68caf94388ea98a88074079103d48e3eae83e2ea7 I think it is better to add string at the end of URL

Maybe this is fine

What's the purpose of ?app=walletconnect @devceline ?

Reason for new URL was to properly parse and construct the string to properly use query params. If we appended at end of the URL it also wouldn't be a valid query param as it would actually be part of the fragment. What would the string at the end of the URL be used for?

What would the string at the end of the URL be used for?

It is used by snapshot to know that someone voted from web3inbox, changed it to web3inbox in the other PR, hope it is fine

ChaituVR · 2024-01-15T12:23:07Z

src/providers/walletconnectNotify.ts

+}
+
+// Transform proposal event into notification format.
+async function formatMessage(event, proposal) {


Suggested change

async function formatMessage(event, proposal) {

function formatMessage(event, proposal) {

devceline · 2024-01-16T13:57:11Z

Closed in favor of #162

Add walletconnect provider

d09c062

Cali93 reviewed Sep 28, 2023

View reviewed changes

src/providers/walletconnectNotify.ts Outdated Show resolved Hide resolved

devceline added 6 commits September 28, 2023 14:23

chore: fix bug causing an invalid array to be passed as subscribers

09d1870

Remove unused console log

db410a7

chore: Add documentation

003b98c

chore: add env vars to example file

4e850f2

Add notification body gen

bd78c18

chore: remove switch statement

2262352

zzuziak requested a review from ChaituVR October 3, 2023 07:17

fix: use new notification id

152fc4a

feat: cross reference subscribers

aec2484

chris13524 suggested changes Dec 22, 2023

View reviewed changes

chore: account for rate limiting

feed1c6

bonustrack reviewed Jan 9, 2024

View reviewed changes

devceline and others added 3 commits January 9, 2024 12:22

Apply suggestions from code review

d0431a2

Co-authored-by: Less <[email protected]>

feat: use subscribers coming from the function, less query

a26ce70

chore: run lint fix

c33b8ff

ChaituVR requested changes Jan 15, 2024

View reviewed changes

ChaituVR reviewed Jan 15, 2024

View reviewed changes

ChaituVR mentioned this pull request Jan 16, 2024

feat: Add walletconnect notify provider for web3Inbox #162

Merged

1 task

devceline closed this Jan 16, 2024

		case 'proposal/end':
		return `A proposal has closed for ${space.name}`;

	return 'ed2fd071-65e1-440d-95c5-7d58884eae43';
	return 'f3c55113-06fb-47c7-87da-28ee21b9114a';

		const url = new URL(proposal.link);
		url.searchParams.append('app', 'walletconnect');

	async function formatMessage(event, proposal) {
	function formatMessage(event, proposal) {

Add walletconnect provider #114

Add walletconnect provider #114

Conversation

devceline commented Sep 28, 2023 • edited Loading

Changes

ChaituVR commented Oct 4, 2023

devceline commented Oct 5, 2023

ChaituVR commented Oct 6, 2023

chris13524 commented Oct 6, 2023

ChaituVR commented Oct 17, 2023

devceline commented Oct 17, 2023

ChaituVR commented Oct 24, 2023

chris13524 commented Oct 24, 2023

ChaituVR commented Oct 25, 2023

ChaituVR commented Oct 25, 2023

devceline commented Oct 25, 2023

chris13524 commented Oct 25, 2023

ChaituVR commented Oct 25, 2023 • edited Loading

ChaituVR commented Oct 25, 2023

ChaituVR commented Oct 29, 2023

ChaituVR commented Oct 29, 2023

devceline commented Oct 30, 2023

ChaituVR commented Nov 1, 2023

devceline commented Nov 1, 2023

ChaituVR commented Nov 10, 2023

chris13524 commented Nov 10, 2023

ChaituVR commented Nov 11, 2023 • edited Loading

chris13524 commented Nov 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bonustrack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris13524 Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devceline commented Jan 16, 2024

devceline commented Sep 28, 2023 •

edited

Loading

ChaituVR commented Oct 25, 2023 •

edited

Loading

ChaituVR commented Nov 11, 2023 •

edited

Loading

chris13524 Jan 15, 2024 •

edited

Loading