walletconnect-docs icon indicating copy to clipboard operation
walletconnect-docs copied to clipboard

feat: speed up pairing

Open arein opened this issue 1 year ago • 2 comments

Currently wallet waits to send settlement payload until dapp confirms that they received the proposal.

In the worst case e.g. for users in Australia far away from our writer nodes, steps 6 and 7 each take up to a second.

They don't need to be sequential. Hence, parallelizing this speeds up pairing experience by up to a second.

arein avatar Sep 30 '22 18:09 arein

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
walletconnect-docs ✅ Ready (Inspect) Visit Preview Sep 30, 2022 at 6:27PM (UTC)

vercel[bot] avatar Sep 30 '22 18:09 vercel[bot]

I think we could specify "parallelizing" Relay client encodes every publish request into a string and sends it as a websocket text frame, so sending one text frame after another will never be parallelized. In best case we can execute send() function at the same time.

Other approach(more parallelized) would be batching publish requests and sending it as one ws frame but then I believe relay needs to be able to handle it.

llbartekll avatar Oct 03 '22 07:10 llbartekll

@llbartekll "parallelize" here means that we should not wait for the response of one websocket frame before we send the other one. We should truly parallelize these things on the application layer. If the transport layer does not in fact send the messages in parallel that's ok. Most of the latency is currently wasted by sequentially waiting on the responses.

arein avatar Oct 26 '22 09:10 arein

OK, so I have measured time between requests and it goes as follow:

will send propose response 1667392251385 did send propose response 1667392251387 will send settle request 1667392251496 did send settle request 1667392251500

I may take more on CI and on different devices and networking condition

@arein so the difference is just 125 milliseconds between first and second request do you think it is likely to cause the problems?

Edit: yeah actually we can improve, will make sense especially in bad networking conditions

llbartekll avatar Nov 02 '22 12:11 llbartekll

@llbartekll can you measure using "ap-southeast-1.relay.WalletConnect.com"?

Because our db writer instances are Europe latency is much worse in other regions.

Also: 125ms is quite a lot so yeah IMO still worth it.

arein avatar Nov 04 '22 08:11 arein

Catching up on the discussion here and just did some measurements on JS via the unit tests:

wss://relay.walletconnect.com

settle_end: 6399.335207939148
propose_end: 6459.772874832153
total latency (propose_end - settle_end):  60.43766689300537

wss://ap-southeast-1.relay.walletconnect.com

settle_end: 22783.965916633606
propose_end: 23154.623041152954
total latency (propose_end - settle_end):  370.65712451934814

So naive run against relay.walletconnect is only 60ms on avg, but against AP it's ~370ms on avg, which is significant.

cc @ganchoradkov, looping you in here so you're aware of the discussion.

Relevant code with debug measurements is here: https://github.com/WalletConnect/walletconnect-monorepo/commit/325190df739dab8efcc233a99606cfff475f0c04

bkrem avatar Nov 04 '22 09:11 bkrem

@llbartekll can you measure using "ap-southeast-1.relay.WalletConnect.com"?

Because our db writer instances are Europe latency is much worse in other regions.

Also: 125ms is quite a lot so yeah IMO still worth it.

it's around 700ms

llbartekll avatar Nov 04 '22 10:11 llbartekll

So to summarize it sounds that this is a good improvement to do. Kotlin already implements it. Hence, could I please get approvals? Not ultra urgent to implement this but end of the year would be great

arein avatar Nov 07 '22 23:11 arein